error handling for set power limit#3275
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves robustness when applying battery power limits by ensuring set_power_limit runs inside a SingleComponentUpdateContext (so exceptions are handled via fault_state), and updates the battery status text formatting to display power in kW.
Changes:
- Add a
set_power_limit_wrapperhelper that executesAbstractBat.set_power_limit()withinSingleComponentUpdateContext. - Use the new wrapper as the thread target when applying battery
power_limitvalues in the control process. - Format battery
state_strmessages for charge/discharge using kW instead of W.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/modules/common/configurable_device.py | Adds set_power_limit_wrapper (and imports AbstractBat) to run set_power_limit inside an update context for fault handling. |
| packages/control/process.py | Switches threaded battery power-limit calls to use set_power_limit_wrapper. |
| packages/control/bat_all.py | Updates state_str strings to show charge/discharge power in kW. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def set_power_limit_wrapper(bat_component: AbstractBat, power_limit: float): | ||
| """set_power_limit innerhalb des SingleComponentUpdateContext aufrufen, | ||
| damit Fehler im fault_state-Handler behandelt werden | ||
| """ | ||
| with SingleComponentUpdateContext(bat_component.fault_state, update_always=False): | ||
| bat_component.set_power_limit(power_limit) |
There was a problem hiding this comment.
set_power_limit_wrapper is annotated as power_limit: float, but AbstractBat.set_power_limit() expects Optional[int] and callers can pass None (e.g. when battery self-regulation is desired). Please align the wrapper’s type hints (and optionally its docstring) to accept None and match the AbstractBat API (e.g. Optional[int]), to avoid misleading call sites/type checking errors.
| else: | ||
| factor = min(power / max_discharge_power_total, 1) | ||
| power_limit = bat_component_data.get.max_discharge_power * factor | ||
| bat_component_data.get.state_str = f"Entladung mit {power_limit}W" | ||
| bat_component_data.get.state_str = f"Entladung mit {round(power_limit / 1000, 3)} kW" | ||
| log.debug(("Aktive Speichersteuerung: Entladung - " | ||
| f"Speicher (ID: {bat_component.component_config.id}) " |
There was a problem hiding this comment.
The updated state_str formatting is behavior/UI-facing but isn’t covered by the existing unit tests for BatAll. Consider adding assertions for the generated state_str (including unit conversion to kW) in packages/control/bat_all_test.py, so future refactors don’t silently change the displayed status text.
| else: | ||
| factor = min(power / max_charge_power_total, 1) | ||
| power_limit = bat_component_data.get.max_charge_power * factor | ||
| bat_component_data.get.state_str = f"Ladung mit {power_limit}W" | ||
| bat_component_data.get.state_str = f"Ladung mit {round(power_limit / 1000, 3)} kW" | ||
| log.debug(("Aktive Speichersteuerung: Ladung - " | ||
| f"Speicher (ID: {bat_component.component_config.id}) " |
There was a problem hiding this comment.
The updated state_str formatting is behavior/UI-facing but isn’t covered by the existing unit tests for BatAll. Consider adding assertions for the generated state_str (including unit conversion to kW) in packages/control/bat_all_test.py, so future refactors don’t silently change the displayed status text.
seaspotter
left a comment
There was a problem hiding this comment.
Macht es nicht eher sinn statt nur den Ausgabetext gleich den übergebenen power_limit Wert zu runden auf max eine Nachkommastelle? Hintergrund es gibt auch in vielen log.debug Nachrichten immer wieder diese utopisch langen Kommawerte.
Ab hier dann:
core/packages/control/bat_all.py
Line 247 in ce0aaa4
|
Ich habe das in einen eigenen PR ausgelagert. #3276 |
* error handling for set power limit, format bat state str
No description provided.