dimm kit: improve error handling, handle deleted output devices - #3868
Open
LKuemmel wants to merge 1 commit into
Open
dimm kit: improve error handling, handle deleted output devices#3868LKuemmel wants to merge 1 commit into
LKuemmel wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Improves I/O action resilience when configured devices are deleted and centralizes device logging.
Changes:
- Adds reusable fault-state error handling.
- Handles missing devices during logging and output assignment.
- Safely translates exceptions without arguments.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
packages/modules/io_actions/generator_systems/stepwise_control/api_io.py |
Handles missing inverter names. |
packages/modules/io_actions/generator_systems/stepwise_control/api_eebus.py |
Handles missing inverter names. |
packages/modules/io_actions/controllable_consumers/ripple_control_receiver/api.py |
Uses shared device logging. |
packages/modules/io_actions/controllable_consumers/dimming/api_io.py |
Uses shared device logging. |
packages/modules/io_actions/controllable_consumers/dimming/api_eebus.py |
Uses shared device logging. |
packages/modules/io_actions/controllable_consumers/dimming_direct_control/api.py |
Uses shared device logging. |
packages/modules/io_actions/common.py |
Adds resilient device-log formatting. |
packages/modules/common/fault_state.py |
Adds a fault-state context manager. |
packages/helpermodules/exceptions/registry.py |
Handles argument-less exceptions. |
packages/control/process.py |
Associates output errors with fault states. |
packages/control/io_device.py |
Wraps I/O actions in fault handling. |
Suppressed comments (5)
packages/control/io_device.py:78
- This lookup runs before checking whether the action is a dimming action. Consequently, a stale unrelated action whose I/O device was deleted now makes every dimming query raise, whereas it was previously ignored. Skip entries whose device is no longer present before opening the context.
io_device = data.data.system_data[f"io{action.config.configuration.io_device}"]
with FaultStateContext(io_device.fault_state, update_always=False):
packages/control/io_device.py:90
- The new unconditional dictionary lookup makes
dimming_set_import_power_leftfail on any stale action, including unrelated action types that this method previously skipped. Guard the lookup so a deleted I/O device cannot abort the whole iteration.
io_device = data.data.system_data[f"io{action.config.configuration.io_device}"]
with FaultStateContext(io_device.fault_state, update_always=False):
packages/control/io_device.py:100
- Resolving the I/O device before the action-type check means a deleted device belonging to any unrelated action now aborts direct-control lookup. Use a guarded lookup and continue past stale actions.
io_device = data.data.system_data[f"io{action.config.configuration.io_device}"]
with FaultStateContext(io_device.fault_state, update_always=False):
packages/control/io_device.py:111
- A stale action with a deleted controlling I/O device raises here before the ripple-control type check, so an unrelated stale action can disable this lookup. Skip missing devices before entering the context.
io_device = data.data.system_data[f"io{action.config.configuration.io_device}"]
with FaultStateContext(io_device.fault_state, update_always=False):
packages/control/io_device.py:122
- The unconditional lookup introduces the same stale-action regression for stepwise control: a deleted I/O device on any earlier action raises before non-stepwise actions can be skipped. Guard the lookup and continue when the device no longer exists.
io_device = data.data.system_data[f"io{action.config.configuration.io_device}"]
with FaultStateContext(io_device.fault_state, update_always=False):
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+19
to
+24
| return (f"Ladepunkt {data.data.cp_data[cp].data.config.name}: " | ||
| f"{data.data.cp_data[cp].data.get.powers}W, ") | ||
| if device["type"] == "io": | ||
| io = f"io{device['id']}" | ||
| return (f"{data.data.system_data[io].config.name}: " | ||
| "Leistung unbekannt, ") |
Comment on lines
+106
to
+115
| if isinstance(exc_value, Exception): | ||
| self.__fault_state.from_exception(exc_value) | ||
| elif self.update_always is False and self.__fault_state.fault_state == 0: | ||
| # Fehlerstatus nicht überschreiben | ||
| return True | ||
| self.__fault_state.store_error() | ||
| if self.reraise is False or exc_value is None: | ||
| return True | ||
| else: | ||
| return False |
Comment on lines
+69
to
+70
| io_device = data.data.system_data[f"io{action.config.configuration.io_device}"] | ||
| with FaultStateContext(io_device.fault_state, update_always=False): |
Contributor
There was a problem hiding this comment.
Sehe ich auch so. Fehlende Geräte sollten mit einer Fehlermeldung abgefangen werden.
Comment on lines
+76
to
+77
| io_device = data.data.system_data[f"io{action.config.configuration.io_device}"] | ||
| with FaultStateContext(io_device.fault_state, update_always=False): |
benderl
reviewed
Aug 28, 2026
Comment on lines
+69
to
+70
| io_device = data.data.system_data[f"io{action.config.configuration.io_device}"] | ||
| with FaultStateContext(io_device.fault_state, update_always=False): |
Contributor
There was a problem hiding this comment.
Sehe ich auch so. Fehlende Geräte sollten mit einer Fehlermeldung abgefangen werden.
| traceback: Optional[TracebackType]) -> bool: | ||
| if isinstance(exc_value, Exception): | ||
| self.__fault_state.from_exception(exc_value) | ||
| elif self.update_always is False and self.__fault_state.fault_state == 0: |
Contributor
There was a problem hiding this comment.
Suggested change
| elif self.update_always is False and self.__fault_state.fault_state == 0: | |
| elif self.update_always is False and self.__fault_state.fault_state == FaultStateLevel.NO_ERROR: |
| exc_type: Optional[Type[BaseException]], | ||
| exc_value: Optional[BaseException], | ||
| traceback: Optional[TracebackType]) -> bool: | ||
| if isinstance(exc_value, Exception): |
Contributor
There was a problem hiding this comment.
Wie von Copilot angemerkt, sollten wichtige Systemereignisse nicht verschluckt werden.
Suggested change
| if isinstance(exc_value, Exception): | |
| # Systemrelevante Signale niemals unterdrücken | |
| if exc_value is not None and not isinstance(exc_value, Exception): | |
| return False | |
| if isinstance(exc_value, Exception): |
Comment on lines
+19
to
+24
| return (f"Ladepunkt {data.data.cp_data[cp].data.config.name}: " | ||
| f"{data.data.cp_data[cp].data.get.powers}W, ") | ||
| if device["type"] == "io": | ||
| io = f"io{device['id']}" | ||
| return (f"{data.data.system_data[io].config.name}: " | ||
| "Leistung unbekannt, ") |
| from control import data | ||
| from control.limiting_value import LoadmanagementLimit | ||
| from helpermodules.constants import NO_ERROR | ||
| from modules.common.configurable_io import ConfigurableIo |
Contributor
There was a problem hiding this comment.
Suggested change
| from modules.common.configurable_io import ConfigurableIo |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.