Skip to content

error handling for set power limit#3275

Merged
benderl merged 3 commits intoopenWB:masterfrom
LKuemmel:fix_error_handling
Apr 9, 2026
Merged

error handling for set power limit#3275
benderl merged 3 commits intoopenWB:masterfrom
LKuemmel:fix_error_handling

Conversation

@LKuemmel
Copy link
Copy Markdown
Contributor

@LKuemmel LKuemmel commented Apr 8, 2026

No description provided.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_wrapper helper that executes AbstractBat.set_power_limit() within SingleComponentUpdateContext.
  • Use the new wrapper as the thread target when applying battery power_limit values in the control process.
  • Format battery state_str messages 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.

Comment on lines +133 to +138
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)
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Comment on lines 280 to 285
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}) "
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Comment on lines 297 to 302
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}) "
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@seaspotter seaspotter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

# Leistung an einzelne Speicher übergeben

@LKuemmel LKuemmel changed the title error handling for set power limit, format bat state str error handling for set power limit Apr 9, 2026
@LKuemmel
Copy link
Copy Markdown
Contributor Author

LKuemmel commented Apr 9, 2026

Ich habe das in einen eigenen PR ausgelagert. #3276

@benderl benderl merged commit 1bfe9e3 into openWB:master Apr 9, 2026
1 check passed
LKuemmel added a commit that referenced this pull request Apr 10, 2026
* error handling for set power limit, format bat state str
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants