Conversation
There was a problem hiding this comment.
Pull request overview
This PR adjusts load-management limit handling and EV auto phase-switch condition evaluation, aiming to improve automatic phase switching behavior (especially when current limiting is involved) and reduce misleading “POWER” limitation signaling during PV charging.
Changes:
- Extend phase-switch condition logic to consider an explicit “current limit reached” signal.
- Change several algorithms to only persist
control_parameter.limitunder certain conditions (e.g., skip POWER in PV, skipNone). - Add PV-mode-specific handling to avoid treating power as the limiting reason in stored control parameters.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| packages/control/ev/ev.py | Updates phase-switch condition logic to also react to current limiting. |
| packages/control/algorithm/surplus_controlled.py | Changes when control_parameter.limit is stored during PV/surplus control (skips POWER). |
| packages/control/algorithm/min_current.py | Stops overwriting control_parameter.limit when no limit is active. |
| packages/control/algorithm/additional_current.py | Stops overwriting control_parameter.limit when no limit is active. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| unbalanced_load_limit_reached = limit.limiting_value == LimitingValue.UNBALANCED_LOAD | ||
| condition_1_to_3 = (((get_medium_charging_current(get_currents) > max_current_range and | ||
| current_limit_reached = (limit.limiting_value == LimitingValue.CURRENT or | ||
| limit.limiting_value == LimitingValue.CURRENT.value) | ||
| condition_1_to_3 = ((((get_medium_charging_current(get_currents) > max_current_range or current_limit_reached) and |
There was a problem hiding this comment.
unbalanced_load_limit_reached only checks for the Enum member, but current_limit_reached now checks both Enum and the serialized .value string. If limit.limiting_value can be a serialized string (as implied by the new check), unbalanced-load limiting will still be missed and phase-switch decisions become inconsistent. Consider normalizing limit.limiting_value once (e.g., coerce strings back to LimitingValue safely) and then compare only against Enum members for both CURRENT and UNBALANCED_LOAD.
| cp.data.control_parameter.limit = limit | ||
| # im PV-Laden wird der Strom immer durch die Leistung begrenzt | ||
| if limit.limiting_value is not None and limit.limiting_value != LimitingValue.POWER: | ||
| cp.data.control_parameter.limit = limit |
There was a problem hiding this comment.
This conditional assignment can leave cp.data.control_parameter.limit stale when (a) there is no limiting (limit.limiting_value is None) or (b) limiting is by POWER (which is skipped). Since cp.data.control_parameter.limit is later consumed by algorithm.py for auto_phase_switch, an old CURRENT/UNBALANCED limit may incorrectly influence phase-switch decisions and UI state. If the intention is to ignore POWER limits in PV mode, consider explicitly clearing/resetting control_parameter.limit when the current limit is POWER or None, rather than leaving the previous value in place.
| cp.data.control_parameter.limit = limit | |
| cp.data.control_parameter.limit = limit | |
| else: | |
| # Clear any stale CURRENT/UNBALANCED limit when there is no limiting | |
| # or when limiting is by POWER (ignored in PV mode). | |
| cp.data.control_parameter.limit = None |
| missing_currents, counter, cp) | ||
| cp.data.control_parameter.limit = limit | ||
| if limit.limiting_value is not None: | ||
| cp.data.control_parameter.limit = limit |
There was a problem hiding this comment.
With this change, when Loadmanagement().get_available_currents(...) returns a limit with limiting_value is None, the chargepoint's control_parameter.limit is no longer updated/cleared and will retain whatever value was set in a previous iteration. That can lead to stale limit state being displayed and can affect downstream logic that reads control_parameter.limit. If you only want to skip publishing limit messages, consider still assigning a neutral/empty LoadmanagementLimit here to clear the previous limit.
| cp.data.control_parameter.limit = limit | |
| cp.data.control_parameter.limit = limit | |
| else: | |
| cp.data.control_parameter.limit = None |
| if limit.limiting_value is not None: | ||
| cp.data.control_parameter.limit = limit |
There was a problem hiding this comment.
Same issue as in min_current.py: skipping the assignment when limit.limiting_value is None means cp.data.control_parameter.limit can remain set to an old limiting reason even after the constraint is gone. That stale state can propagate into phase-switch logic and status messaging. Consider clearing/resetting the limit explicitly when there is no active limitation.
| if limit.limiting_value is not None: | |
| cp.data.control_parameter.limit = limit | |
| cp.data.control_parameter.limit = limit |
| current_limit_reached = (limit.limiting_value == LimitingValue.CURRENT or | ||
| limit.limiting_value == LimitingValue.CURRENT.value) | ||
| condition_1_to_3 = ((((get_medium_charging_current(get_currents) > max_current_range or current_limit_reached) and | ||
| all_surplus > required_surplus) or unbalanced_load_limit_reached) and | ||
| phases_in_use == 1) |
There was a problem hiding this comment.
Phase-switch behavior now depends on current_limit_reached derived from limit.limiting_value (including the serialized-string case). The existing auto_phase_switch_test.py::test_check_phase_switch_conditions doesn’t cover the new branch where a current limit alone (without get_medium_charging_current > max_current_range) should trigger a 1p→3p switch, nor the case where limit.limiting_value is a serialized value string. Adding targeted unit tests would prevent regressions and clarify intended behavior.
No description provided.