Active bat control: update power_limit_mode#3169
Conversation
|
@ndrsnhs Bitte ein Rebase machen. |
benderl
left a comment
benderl
left a comment
There was a problem hiding this comment.
Bitte auch die anderen alten Bezeichnungen migrieren. "no_limit" ist nur einer von dreien.
ndrsnhs
force-pushed
the
remove-power_limit_mode
branch
from
cbd2b6d to
ce0aaa4
Compare
…core into remove-power_limit_mode
There was a problem hiding this comment.
Pull request overview
This PR bumps the datastore version and adds a new datastore migration to normalize legacy openWB/bat/config/power_limit_mode values that may still be present (e.g. no_limit).
Changes:
- Increase
UpdateConfig.DATASTORE_VERSIONfrom 119 to 120. - Add
upgrade_datastore_120to migrateopenWB/bat/config/power_limit_modeto current mode values. - Append datastore version
120after running the new migration.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if mode == "no_limit" or mode == "limit_stop": | ||
| mode = "mode_no_discharge" | ||
| return {topic: mode} | ||
| else: | ||
| mode = "mode_discharge_home_consumption" | ||
| return {topic: mode} |
There was a problem hiding this comment.
This migration introduces new mapping behavior for openWB/bat/config/power_limit_mode, but there is no test covering the conversion (and especially that already-valid modes are left unchanged). Please add a pytest case similar to existing datastore upgrade tests to prevent regressions.
| if mode == "no_limit" or mode == "limit_stop": | |
| mode = "mode_no_discharge" | |
| return {topic: mode} | |
| else: | |
| mode = "mode_discharge_home_consumption" | |
| return {topic: mode} | |
| legacy_mode_mapping = { | |
| "no_limit": "mode_no_discharge", | |
| "limit_stop": "mode_no_discharge", | |
| "limit": "mode_discharge_home_consumption", | |
| } | |
| if mode in legacy_mode_mapping: | |
| return {topic: legacy_mode_mapping[mode]} |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def upgrade_datastore_120(self) -> None: | ||
| def upgrade(topic: str, payload) -> Optional[dict]: | ||
| if re.search("^openWB/bat/config/power_limit_mode$", topic) is not None: | ||
| mode = decode_payload(payload) | ||
| if mode == "no_limit" or mode == "limit_stop": | ||
| mode = "mode_no_discharge" | ||
| return {topic: mode} | ||
| elif mode == "limit_home_consumption": | ||
| mode = "mode_discharge_home_consumption" | ||
| return {topic: mode} |
There was a problem hiding this comment.
For this new datastore migration (mapping legacy values like "no_limit"/"limit_stop"/"limit_home_consumption" to the current BatPowerLimitMode strings), please add a focused unit test (similar to existing upgrade_datastore_94 coverage in packages/helpermodules/update_config_test.py) to prevent regressions and to document the intended mapping behavior.
Regelmodus no_limit existiert in der Form nicht mehr, kann aber ggfs. noch gesetzt sein