diff --git a/apps/predbat/execute.py b/apps/predbat/execute.py index 08af4580d..8b4d57e78 100644 --- a/apps/predbat/execute.py +++ b/apps/predbat/execute.py @@ -26,6 +26,51 @@ Execute Predbat plan """ +# Per-inverter core charge/export states, used to resolve one headline status across a multi-inverter +# fleet instead of letting whichever inverter is processed last silently win. Precedence lists are +# ordered most-active-first so the most informative sub-state is shown when inverters within the same +# side disagree (e.g. one still actively Charging while another has already reached Hold charging). +CHARGE_STATE_PRECEDENCE = ["Charging", "Freeze charging", "Hold charging"] +EXPORT_STATE_PRECEDENCE = ["Exporting", "Freeze exporting", "Hold exporting"] +CHARGE_SIDE_STATES = set(CHARGE_STATE_PRECEDENCE) +EXPORT_SIDE_STATES = set(EXPORT_STATE_PRECEDENCE) + + +def resolve_multi_inverter_status(status_per_inverter, current_status): + """Resolve one headline status across a multi-inverter fleet. + + ``status_per_inverter`` holds each inverter's own final core charge/export state, keyed by + inverter id, as recorded during execute_plan()'s per-inverter loop - a plain dict overwrite per + inverter, so it correctly reflects each inverter's own last-set state even if that inverter's own + processing passed through more than one core-state assignment. + + If inverters disagree across the charge/export divide (one genuinely charging while another + discharges at the same time) that's real cross-charging, not noise - surfaced explicitly rather + than silently showing whichever inverter happened to be processed last. If they only disagree on + sub-state within the same side (e.g. one still actively Charging, another already Hold charging), + the most active one is shown - it's the most informative and matches what the fleet is actually + doing overall. + + Falls through to ``current_status`` unchanged when no inverter reached a core charge/export state + at all (pure Demand, Read-Only, Calibration, or a Hold-for-car/iBoost annotation with nothing else + going on) - those cases are already correct and untouched by this resolution. Calibration always + wins outright even if ``status_per_inverter`` holds a stale core state from an inverter processed + before the one that entered calibration mode, since calibration force-overrides every inverter's + controls and that must not be hidden behind a leftover Charging/Exporting label. + """ + if current_status == "Calibration": + return current_status + states_present = set(status_per_inverter.values()) + charge_states_present = states_present & CHARGE_SIDE_STATES + export_states_present = states_present & EXPORT_SIDE_STATES + if charge_states_present and export_states_present: + return "Cross-charging" + if charge_states_present: + return next(candidate for candidate in CHARGE_STATE_PRECEDENCE if candidate in charge_states_present) + if export_states_present: + return next(candidate for candidate in EXPORT_STATE_PRECEDENCE if candidate in export_states_present) + return current_status + class Execute: """Execution mixin for applying optimised plans to physical inverters. @@ -40,6 +85,11 @@ def execute_plan(self): status_hold_car = "" # car hold status text status_hold_iboost = "" # iBoost hold status text status_freeze_export = "" # freeze export during demand status text + # Each inverter's own final core charge/export state, keyed by inverter id - used after the + # loop to detect genuine cross-charging (one inverter charging while another discharges at + # the same time) rather than silently showing whichever inverter's status happened to be + # set last, which hides that the fleet is fighting itself. + status_per_inverter = {} in_alert = self.alert_active_keep.get(self.minutes_now, 0) > 0 in_manual_soc = self.manual_soc_keep.get(self.minutes_now, 0) > 0 @@ -198,14 +248,16 @@ def execute_plan(self): resetDischarge = False status = "Freeze charging" + status_per_inverter[inverter.id] = status status_extra += " target" if inverter.id == 0 else " /" # Append multi-inverter target SoC's together - status_extra += " {}%".format(inverter.soc_percent) + status_extra += " {} {}%".format(status, inverter.soc_percent) self.log("Inverter {} Freeze charging with SoC {}%".format(inverter.id, inverter.soc_percent)) else: # We can only hold charge if a) we have a way to hold the charge level on the reserve or with a pause feature # and the current charge level is above the target for all inverters if self.set_soc_enable and inverter.soc_percent >= inv_target_soc_percent: status = "Hold charging" + status_per_inverter[inverter.id] = status self.log( "Inverter {} Hold charging as SoC {}% is above target SoC {}% (global soc target {}%) set_discharge_during_charge {}".format( inverter.id, inverter.soc_percent, dp0(inv_target_soc_percent), dp0(target_soc), self.set_discharge_during_charge @@ -240,10 +292,11 @@ def execute_plan(self): inverter.adjust_charge_window(charge_start_time, charge_end_time, self.minutes_now) else: status = "Charging" + status_per_inverter[inverter.id] = status inverter.adjust_charge_window(charge_start_time, charge_end_time, self.minutes_now) status_extra += " target" if inverter.id == 0 else " /" # append multi-inverter target SoC's together - status_extra += " {}%-{}%".format(inverter.soc_percent, inv_target_soc_percent) + status_extra += " {} {}%-{}%".format(status, inverter.soc_percent, inv_target_soc_percent) if not self.set_discharge_during_charge and resetPause: # Do we discharge discharge during charge @@ -378,8 +431,9 @@ def execute_plan(self): self.isExporting_Target = int(target) status = "Exporting" + status_per_inverter[inverter.id] = status status_extra += " target" if inverter.id == 0 else " /" # append multi-inverter target SoC's together - status_extra += " {}%-{}%".format(inverter.soc_percent, int(target)) + status_extra += " {} {}%-{}%".format(status, inverter.soc_percent, int(target)) # Immediate export mode else: inverter.adjust_force_export(False) @@ -398,16 +452,18 @@ def execute_plan(self): self.log("Export Freeze as exporting is now at/below target - current SoC {}kWh and target {}kWh".format(self.soc_kw, discharge_soc)) status = "Freeze exporting" + status_per_inverter[inverter.id] = status status_extra += " current SoC" if inverter.id == 0 else " /" # append multi-inverter target SoC's together - status_extra += " {}%".format(inverter.soc_percent) # Discharge limit (99) is meaningless when Freeze Exporting so don't display it + status_extra += " {} {}%".format(status, inverter.soc_percent) # Discharge limit (99) is meaningless when Freeze Exporting so don't display it isExporting = True target = self.export_window_best[0].get("target", self.export_limits_best[0]) self.isExporting_Target = int(target) else: status = "Hold exporting" + status_per_inverter[inverter.id] = status target = self.export_window_best[0].get("target", self.export_limits_best[0]) status_extra += " target" if inverter.id == 0 else " /" # append multi-inverter target SoC's together - status_extra += " {}%-{}%".format(inverter.soc_percent, inverter.soc_percent) + status_extra += " {} {}%-{}%".format(status, inverter.soc_percent, inverter.soc_percent) self.isExporting_Target = inverter.soc_percent self.log("Export Hold (Demand mode) as export is now at/below target or freeze only is set - current SoC {}kWh and target {}kWh".format(self.soc_kw, discharge_soc)) else: @@ -477,25 +533,29 @@ def execute_plan(self): # iBoost running? boostHolding = False - if self.set_charge_window and self.iboost_enable and self.iboost_prevent_discharge and self.iboost_running_full and status not in ["Exporting", "Charging"]: - if inverter.inv_has_timed_pause: - if resetPause: - inverter.adjust_pause_mode(pause_discharge=True) - resetPause = False - else: - if resetDischarge: - inverter.adjust_discharge_rate(0) - resetDischarge = False - if self.set_reserve_enable: - inverter.adjust_reserve(min(inverter.soc_percent + 1, 100)) - resetReserve = False - boostHolding = True - self.log("Disabling battery discharge whilst iBoost is running") - if ("Hold for iBoost" not in status) and (status_hold_iboost == ""): - if status == "Demand": - status = "Hold for iBoost" + if self.set_charge_window and self.iboost_enable and self.iboost_prevent_discharge and self.iboost_running_full: + # Only pause discharge on this inverter, and only annotate the status as held for + # iBoost, if the fleet isn't already Charging/Exporting - pausing would conflict with + # that, and the annotation must stay coupled to whether a hold actually happened here. + if status not in ["Exporting", "Charging"]: + if inverter.inv_has_timed_pause: + if resetPause: + inverter.adjust_pause_mode(pause_discharge=True) + resetPause = False else: - status_hold_iboost = ", Hold for iBoost" + if resetDischarge: + inverter.adjust_discharge_rate(0) + resetDischarge = False + if self.set_reserve_enable: + inverter.adjust_reserve(min(inverter.soc_percent + 1, 100)) + resetReserve = False + boostHolding = True + self.log("Disabling battery discharge whilst iBoost is running") + if ("Hold for iBoost" not in status) and (status_hold_iboost == ""): + if status == "Demand": + status = "Hold for iBoost" + else: + status_hold_iboost = ", Hold for iBoost" # Reset charge/discharge rate if resetPause: @@ -628,6 +688,10 @@ def execute_plan(self): self.count_inverter_writes[inverter.id] += inverter.count_register_writes inverter.count_register_writes = 0 + # Resolve the headline status across all inverters rather than leaving whichever inverter was + # processed last to silently win. + status = resolve_multi_inverter_status(status_per_inverter, status) + # Set the charge/discharge status information self.set_charge_export_status(isCharging, isExporting, not (isCharging or isExporting)) self.isCharging = isCharging diff --git a/apps/predbat/inverter.py b/apps/predbat/inverter.py index 9806a0be2..c86bbb768 100644 --- a/apps/predbat/inverter.py +++ b/apps/predbat/inverter.py @@ -1140,7 +1140,11 @@ def find_charge_curve(self, discharge): else: search_range = range(99, 85, -1) - # Find 100% end points + # Find 100% end points. The exact-match checks below ("Charging" / "Exporting", + # "Discharging") deliberately exclude "Cross-charging" minutes - during genuine + # cross-charging another inverter is simultaneously drawing/feeding power at the + # same time, so this inverter's battery_power reading isn't a clean single-inverter + # charge/discharge sample and would corrupt the learned curve if included. for data_point in search_range: for minute in range(1, min_len): # Start trigger is when the SoC just increased above the data point diff --git a/apps/predbat/output.py b/apps/predbat/output.py index 414792835..68fd8a6f1 100644 --- a/apps/predbat/output.py +++ b/apps/predbat/output.py @@ -52,6 +52,17 @@ } +def yesterday_slot_is_exporting(slot_status): + """True when a historical ``predbat.status`` string (already lower-cased) represents export + activity for the "yesterday" plan reconstruction in ``calculate_yesterday()``. + + Includes "cross-charging" explicitly - it genuinely straddles both sides of the fleet at once, + but as a string it contains "charging" and not "exporting", so a plain substring check on + "exporting" alone would silently drop the export half of a real cross-charging slot. + """ + return "exporting" in slot_status or "cross-charging" in slot_status + + class Output: """Output and sensor publishing mixin. @@ -3142,7 +3153,10 @@ def calculate_yesterday(self): slot_status = predbat_status.get(slot_minute, "").lower() real_minute = minute + slot_offset - if "exporting" in slot_status: + # Cross-charging genuinely straddles both sides - track it as both an exporting + # and a charging slot (its name contains "charging" but not "exporting"), so + # these are independent "if"s rather than "if/elif". + if yesterday_slot_is_exporting(slot_status): export_during_slot = slot_status if export_start_minute is None: export_start_minute = real_minute @@ -3150,7 +3164,7 @@ def calculate_yesterday(self): export_start_minute -= 5 if charge_start_minute is not None: charge_end_minute = export_start_minute - elif "charging" in slot_status: + if "charging" in slot_status: charge_during_slot = slot_status if charge_start_minute is None: charge_start_minute = real_minute @@ -3165,7 +3179,7 @@ def calculate_yesterday(self): if charge_end_minute is None and charge_start_minute is not None: charge_end_minute = minute + self.plan_interval_minutes - if "exporting" in export_during_slot: + if yesterday_slot_is_exporting(export_during_slot): # Assume exporting at this time self.export_window_best.append({"start": export_start_minute, "end": export_end_minute}) if "freeze" in export_during_slot: diff --git a/apps/predbat/tests/test_calculate_yesterday.py b/apps/predbat/tests/test_calculate_yesterday.py index d908d0a85..312886ef5 100644 --- a/apps/predbat/tests/test_calculate_yesterday.py +++ b/apps/predbat/tests/test_calculate_yesterday.py @@ -28,6 +28,7 @@ import pytz from const import PREDICT_STEP +from output import yesterday_slot_is_exporting from tests.test_infra import reset_rates, reset_inverter UTC = pytz.UTC @@ -1123,6 +1124,101 @@ def _history_no_soc(entity_id, days=30, required=True, tracked=True): return failed +def _test_cross_charging_reconstructed_as_both_windows(my_predbat, failed): + """Regression for PR #4466, through the real calculate_yesterday() reconstruction path. + + A whole history of "Cross-charging" status must rebuild BOTH charge windows and export + windows for the "yesterday" plan. Before the fix the slot search tested only + "exporting" in slot_status, and "Cross-charging" contains "charging" but not "exporting", + so every cross-charging minute was reconstructed as charge-only and the export side was + silently lost. + + The reconstructed windows live on self.charge_window_best/self.export_window_best only + between the fake-window block and the restore at the end of calculate_yesterday, so they + are captured from inside the publish_html_plan mock, which is called in that window. + """ + print("calculate_yesterday: Test - Cross-charging history rebuilds both charge and export windows (#4466)") + now_utc = _setup_base(my_predbat) + prefix = my_predbat.prefix + + status_hist = _make_constant_history("Cross-charging", now_utc) + + def _history_with_cross_charging(entity_id, days=30, required=True, tracked=True): + if entity_id == prefix + ".cost_today": + return _make_constant_history(100.0, now_utc) + elif entity_id == prefix + ".soc_kw_h0": + return _make_constant_history(5.0, now_utc) + elif entity_id == prefix + ".status": + return status_hist + return None + + captured = {} + + def _capture_publish_html_plan(*args, **kwargs): + captured["charge_window_best"] = copy.deepcopy(my_predbat.charge_window_best) + captured["export_window_best"] = copy.deepcopy(my_predbat.export_window_best) + captured["export_limits_best"] = copy.deepcopy(my_predbat.export_limits_best) + return ("", "{}") + + my_predbat.step_data_history = _make_mock_step_data(my_predbat.pv_today) + my_predbat.get_history_wrapper = _history_with_cross_charging + my_predbat.plan_write_debug = lambda *a, **kw: ("", "{}") + my_predbat.publish_html_plan = _capture_publish_html_plan + original_run_pred = my_predbat.run_prediction + my_predbat.run_prediction = lambda *a, **kw: _make_mock_run_prediction([])(my_predbat, *a, **kw) + + my_predbat.calculate_yesterday() + + if "charge_window_best" not in captured: + print("ERROR: publish_html_plan was never called - could not observe the reconstructed windows") + failed = True + else: + # The charge side was already correct before the fix ("cross-charging" contains "charging"). + if not captured["charge_window_best"]: + print("ERROR: a Cross-charging history should rebuild charge windows, got none") + failed = True + # The export side is what the fix restores. + if not captured["export_window_best"]: + print("ERROR: a Cross-charging history should rebuild export windows too, got none " "(the export half of cross-charging was dropped)") + failed = True + # Every rebuilt export window needs a matching limit, or the plan pairs them up wrongly. + if len(captured["export_window_best"]) != len(captured["export_limits_best"]): + print("ERROR: rebuilt {} export windows but {} export limits - they must stay in step".format(len(captured["export_window_best"]), len(captured["export_limits_best"]))) + failed = True + + _restore_methods(my_predbat, original_run_pred) + my_predbat.savings_last_updated = None + return failed + + +def _test_yesterday_slot_is_exporting(my_predbat, failed): + """Test: yesterday_slot_is_exporting() recognises Cross-charging as export activity. + + Regression for PR #4466: the "yesterday" plan reconstruction classified a historical + predbat.status slot as exporting purely via "exporting" in slot_status. "Cross-charging" + contains "charging" but not "exporting", so a genuine cross-charging slot was silently + dropped from the export side, only ever showing up as a charge window. + """ + print("calculate_yesterday: Test - yesterday_slot_is_exporting recognises Cross-charging") + + cases = [ + ("exporting", True), + ("freeze exporting", True), + ("cross-charging", True), + ("charging", False), + ("freeze charging", False), + ("demand", False), + ("hold for car", False), + ] + for slot_status, expected in cases: + result = yesterday_slot_is_exporting(slot_status) + if result != expected: + print("ERROR: yesterday_slot_is_exporting({!r}) should be {}, got {}".format(slot_status, expected, result)) + failed = True + + return failed + + # --------------------------------------------------------------------------- # Entry point registered in TEST_REGISTRY # --------------------------------------------------------------------------- @@ -1145,5 +1241,7 @@ def test_calculate_yesterday(my_predbat): failed = _test_reconstruct_car_slots(my_predbat, failed) failed = _test_soc_not_mutated_and_override_passed(my_predbat, failed) failed = _test_soc_kw_h0_fallback(my_predbat, failed) + failed = _test_yesterday_slot_is_exporting(my_predbat, failed) + failed = _test_cross_charging_reconstructed_as_both_windows(my_predbat, failed) return failed diff --git a/apps/predbat/tests/test_execute.py b/apps/predbat/tests/test_execute.py index c9b623dc7..d8dfb51b7 100644 --- a/apps/predbat/tests/test_execute.py +++ b/apps/predbat/tests/test_execute.py @@ -182,6 +182,7 @@ def run_execute_test( assert_pause_discharge=False, assert_pause_discharge_array=None, assert_status="Demand", + assert_status_extra=None, assert_charge_start_time_minutes=-1, assert_charge_end_time_minutes=-1, assert_charge_start_time_minutes_array=None, @@ -195,7 +196,11 @@ def run_execute_test( assert_reserve=0, assert_soc_target=100, assert_soc_target_array=None, + assert_immediate_charge_soc_target_array=None, + assert_immediate_discharge_soc_target_array=None, + assert_is_charging=None, in_calibration=False, + in_calibration_array=None, set_discharge_during_charge=True, assert_immediate_soc_target=None, assert_immediate_soc_target_array=None, @@ -264,7 +269,7 @@ def run_execute_test( inverter.soc_kw = soc_kw / total_inverters inverter.soc_max = soc_max / total_inverters inverter.soc_percent = calc_percent_limit(inverter.soc_kw, inverter.soc_max) - inverter.in_calibration = in_calibration + inverter.in_calibration = in_calibration_array[inverter.id] if in_calibration_array else in_calibration inverter.battery_rate_max_charge = my_predbat.battery_rate_max_charge / total_inverters inverter.battery_rate_max_charge_dc = my_predbat.battery_rate_max_charge_dc / total_inverters inverter.battery_rate_max_discharge = my_predbat.battery_rate_max_discharge / total_inverters @@ -317,6 +322,10 @@ def run_execute_test( status, status_extra = my_predbat.execute_plan() + if assert_status_extra is not None and assert_status_extra != status_extra: + print("ERROR: status_extra should be {!r} got {!r}".format(assert_status_extra, status_extra)) + failed = True + for inverter in my_predbat.inverters: if assert_status != status: print("ERROR: Inverter {} status should be {} got {}".format(inverter.id, assert_status, status)) @@ -375,13 +384,16 @@ def run_execute_test( if assert_immediate_soc_target_array: assert_immediate_soc_target = assert_immediate_soc_target_array[inverter.id] - assert_soc_target_force = ( - assert_immediate_soc_target - if assert_status in ["Charging", "Charging, Hold for car", "Hold charging", "Freeze charging", "Hold charging, Hold for iBoost", "Hold charging, Hold for car", "Freeze charging, Hold for iBoost", "Hold for car", "Hold for iBoost"] - else 0 - ) - if not set_charge_window: - assert_soc_target_force = -1 + if assert_immediate_charge_soc_target_array: + assert_soc_target_force = assert_immediate_charge_soc_target_array[inverter.id] + else: + assert_soc_target_force = ( + assert_immediate_soc_target + if assert_status in ["Charging", "Charging, Hold for car", "Hold charging", "Freeze charging", "Hold charging, Hold for iBoost", "Hold charging, Hold for car", "Freeze charging, Hold for iBoost", "Hold for car", "Hold for iBoost"] + else 0 + ) + if not set_charge_window: + assert_soc_target_force = -1 if inverter.immediate_charge_soc_target != assert_soc_target_force: print("ERROR: Inverter {} Immediate charge SOC target should be {} got {}".format(inverter.id, assert_soc_target_force, inverter.immediate_charge_soc_target)) failed = True @@ -392,9 +404,12 @@ def run_execute_test( elif assert_status in ["Freeze charging"] and inverter.immediate_charge_soc_freeze != True: print("ERROR: Inverter {} Immediate charge SOC freeze should be True got {}".format(inverter.id, inverter.immediate_charge_soc_freeze)) failed = True - assert_soc_target_force_dis = assert_immediate_soc_target if assert_status in ["Exporting", "Freeze exporting"] else 100 - if not set_export_window: - assert_soc_target_force_dis = -1 + if assert_immediate_discharge_soc_target_array: + assert_soc_target_force_dis = assert_immediate_discharge_soc_target_array[inverter.id] + else: + assert_soc_target_force_dis = assert_immediate_soc_target if assert_status in ["Exporting", "Freeze exporting"] else 100 + if not set_export_window: + assert_soc_target_force_dis = -1 if inverter.immediate_discharge_soc_target != assert_soc_target_force_dis: print("ERROR: Inverter {} Immediate export SOC target should be {} got {}".format(inverter.id, assert_soc_target_force_dis, inverter.immediate_discharge_soc_target)) failed = True @@ -404,7 +419,7 @@ def run_execute_test( # Validate isCharging binary sensor state: must be True for any charging status (Freeze charging, Hold charging, Charging variants) charging_statuses = ["Charging", "Freeze charging", "Hold charging"] - expected_is_charging = any(s in assert_status for s in charging_statuses) + expected_is_charging = assert_is_charging if assert_is_charging is not None else any(s in assert_status for s in charging_statuses) if my_predbat.isCharging != expected_is_charging: print("ERROR: isCharging should be {} for status '{}' got {}".format(expected_is_charging, assert_status, my_predbat.isCharging)) failed = True @@ -637,6 +652,26 @@ def run_execute_tests(my_predbat): if failed: return failed + # Inverter is actively Charging, not merely holding - the discharge-hold action is correctly + # skipped (status is in ["Exporting", "Charging"]), so no pause/reserve change happens. The + # "Hold for iBoost" annotation must not be appended either, since nothing was actually held for + # iBoost this cycle - it should stay coupled to whether the hold action fired, not fire regardless. + failed |= run_execute_test( + my_predbat, + "charge_iboost_no_hold_text", + charge_window_best=charge_window_best, + charge_limit_best=charge_limit_best, + assert_charge_time_enable=True, + set_charge_window=True, + set_export_window=True, + assert_status="Charging", + assert_pause_discharge=False, + assert_charge_start_time_minutes=-1, + assert_charge_end_time_minutes=my_predbat.minutes_now + 60, + ) + if failed: + return failed + my_predbat.iboost_prevent_discharge = False failed |= run_execute_test(my_predbat, "no_charge_iboost2", set_charge_window=True, set_export_window=True) my_predbat.iboost_running_full = False @@ -675,7 +710,10 @@ def run_execute_tests(my_predbat): if failed: return failed - # Here we are already above the target SOC, we can discharge as a hold + # Here inverter 0 is below target (still charging) and inverter 1 is already above target + # (holding). The headline status shows "Charging" - the most active state present across the + # fleet (#4446) - rather than whichever inverter happened to be processed last, since the fleet + # overall genuinely is still charging until inverter 0 catches up. failed |= run_execute_test( my_predbat, "charge_imbalance2", @@ -684,7 +722,8 @@ def run_execute_tests(my_predbat): assert_charge_time_enable=True, set_charge_window=True, set_export_window=True, - assert_status="Hold charging", + assert_status="Charging", + assert_status_extra=" target Charging 90%-100.0% / Hold charging 100%-100.0%", assert_charge_start_time_minutes=-1, assert_charge_end_time_minutes=my_predbat.minutes_now + 60, soc_kw=9.5, @@ -1524,6 +1563,35 @@ def run_execute_tests(my_predbat): my_predbat.inverters = inverters failed |= run_execute_test(my_predbat, "calibration", in_calibration=True, assert_status="Calibration", assert_charge_time_enable=False, assert_reserve=0, assert_soc_target=100) + + # Regression for PR #4466: inverter 0 genuinely charges and reaches "Charging" (populating + # status_per_inverter[0]) before inverter 1 - processed second - enters calibration mode and + # breaks out of the loop. The headline status must stay "Calibration", not resolve back to the + # stale "Charging" state inverter 0 left behind in status_per_inverter. + # The per-inverter asserts below capture the real half-processed state the break leaves behind: + # inverter 0 keeps the charge window/immediate targets it was already given, inverter 1 never + # reached those calls, and isCharging stays True from inverter 0 (pre-existing behaviour, since + # the calibration branch breaks without resetting it). + failed |= run_execute_test( + my_predbat, + "calibration_after_charging_inverter", + charge_window_best=charge_window_best, + charge_limit_best=charge_limit_best, + set_charge_window=True, + set_export_window=True, + in_calibration_array=[False, True], + assert_charge_time_enable_array=[True, False], + assert_status="Calibration", + assert_reserve=0, + assert_soc_target=100, + assert_charge_start_time_minutes=-1, + assert_charge_end_time_minutes=my_predbat.minutes_now + 60, + assert_immediate_charge_soc_target_array=[100, -1], + assert_immediate_discharge_soc_target_array=[100, -1], + assert_is_charging=True, + ) + if failed: + return failed failed |= run_execute_test(my_predbat, "no_charge3", set_charge_window=True, set_export_window=True) failed |= run_execute_test(my_predbat, "charge_read_only", charge_window_best=charge_window_best, charge_limit_best=charge_limit_best, set_charge_window=True, set_export_window=True, read_only=True, assert_status="Read-Only", reserve=0) failed |= run_execute_test( @@ -1704,6 +1772,8 @@ def run_execute_tests(my_predbat): # Charge freeze, one inverter is empty and the other at 40% # Inverter 0 will charge to 10% while inverter 1 will freeze at current level of 40% (as it is above the reserve of 1kWh) + # Headline status shows "Charging" - the most active state present (#4446) - since inverter 0 + # genuinely is still charging, not just whichever inverter happened to be processed last. failed |= run_execute_test( my_predbat, "charge_freeze_imb1", @@ -1714,7 +1784,8 @@ def run_execute_tests(my_predbat): assert_charge_time_enable_array=[True, False], soc_kw=2, assert_pause_discharge_array=[False, True], - assert_status="Freeze charging", + assert_status="Charging", + assert_status_extra=" target Charging 0%-10% / Freeze charging 40%", assert_reserve=0, assert_soc_target_array=[10, 100], assert_immediate_soc_target_array=[10, 40], @@ -1772,6 +1843,8 @@ def run_execute_tests(my_predbat): return failed # One inverter below the reserve, the other above it, re-balance at 10% + # Headline status shows "Charging" - the most active state present (#4446) - since inverter 0 + # genuinely is still charging toward the target, not just whichever inverter was processed last. failed |= run_execute_test( my_predbat, "charge_freeze_imb4", @@ -1782,7 +1855,8 @@ def run_execute_tests(my_predbat): assert_charge_time_enable_array=[True, False], soc_kw=1, assert_pause_discharge_array=[False, True], - assert_status="Freeze charging", + assert_status="Charging", + assert_status_extra=" target Charging 5%-10% / Freeze charging 15%", assert_reserve_array=[0, 0], assert_soc_target_array=[10, 100], assert_immediate_soc_target_array=[10, 15], diff --git a/apps/predbat/tests/test_execute_multi_inverter_status.py b/apps/predbat/tests/test_execute_multi_inverter_status.py new file mode 100644 index 000000000..d2f0585cd --- /dev/null +++ b/apps/predbat/tests/test_execute_multi_inverter_status.py @@ -0,0 +1,101 @@ +# ----------------------------------------------------------------------------- +# Predbat Home Battery System +# Copyright Trefor Southwell 2026 - All Rights Reserved +# This application maybe used for personal use only and not for commercial use +# ----------------------------------------------------------------------------- +# fmt off +# pylint: disable=consider-using-f-string +# pylint: disable=line-too-long +# pylint: disable=attribute-defined-outside-init + +"""Tests for resolve_multi_inverter_status() (#4446) - the multi-inverter headline status +resolver that replaced execute_plan()'s previous "whichever inverter is processed last wins" +overwrite, which silently hid genuine cross-inverter disagreement (e.g. real cross-charging). +""" + +from execute import resolve_multi_inverter_status + + +def test_multi_inverter_status(my_predbat): + """Verify the aggregation rule: unchanged when no inverter reached a core state, the shared + state when all agree, the most-active same-side state when they disagree within one side, and + 'Cross-charging' when they disagree across the charge/export divide. + """ + failed = False + print("**** Testing resolve_multi_inverter_status ****") + + print("Test: no inverter reached a core charge/export state - falls through to current_status unchanged") + for current_status in ["Demand", "Demand (Holiday)", "Read-Only", "Read-Only (Axle)", "Calibration", "Hold for car", "Hold for iBoost"]: + result = resolve_multi_inverter_status({}, current_status) + if result != current_status: + print(" ERROR: expected {!r} unchanged with an empty status_per_inverter, got {!r}".format(current_status, result)) + failed = True + + print("Test: a single inverter's own state passes straight through") + for state in ["Charging", "Freeze charging", "Hold charging", "Exporting", "Freeze exporting", "Hold exporting"]: + result = resolve_multi_inverter_status({0: state}, "Demand") + if result != state: + print(" ERROR: expected single-inverter state {!r}, got {!r}".format(state, result)) + failed = True + + print("Test: all inverters agree on the same state - returns that state regardless of current_status") + result = resolve_multi_inverter_status({0: "Charging", 1: "Charging", 2: "Charging"}, "Demand") + if result != "Charging": + print(" ERROR: expected 'Charging' when all 3 inverters agree, got {!r}".format(result)) + failed = True + + print("Test: charge-side sub-states disagree - most active (Charging) wins over Freeze/Hold charging") + result = resolve_multi_inverter_status({0: "Hold charging", 1: "Charging", 2: "Freeze charging"}, "Demand") + if result != "Charging": + print(" ERROR: expected 'Charging' to win charge-side precedence, got {!r}".format(result)) + failed = True + + print("Test: charge-side sub-states disagree without Charging present - Freeze charging beats Hold charging") + result = resolve_multi_inverter_status({0: "Hold charging", 1: "Freeze charging"}, "Demand") + if result != "Freeze charging": + print(" ERROR: expected 'Freeze charging' to beat 'Hold charging', got {!r}".format(result)) + failed = True + + print("Test: export-side sub-states disagree - most active (Exporting) wins over Freeze/Hold exporting") + result = resolve_multi_inverter_status({0: "Hold exporting", 1: "Exporting", 2: "Freeze exporting"}, "Demand") + if result != "Exporting": + print(" ERROR: expected 'Exporting' to win export-side precedence, got {!r}".format(result)) + failed = True + + print("Test: export-side sub-states disagree without Exporting present - Freeze exporting beats Hold exporting") + result = resolve_multi_inverter_status({0: "Hold exporting", 1: "Freeze exporting"}, "Demand") + if result != "Freeze exporting": + print(" ERROR: expected 'Freeze exporting' to beat 'Hold exporting', got {!r}".format(result)) + failed = True + + print("Test: genuine cross-charging - one inverter charging, another exporting, both at their most active sub-state") + result = resolve_multi_inverter_status({0: "Charging", 1: "Exporting"}, "Demand") + if result != "Cross-charging": + print(" ERROR: expected 'Cross-charging' for Charging+Exporting, got {!r}".format(result)) + failed = True + + print("Test: cross-charging detected even from the mildest sub-states on each side (Hold charging + Hold exporting)") + result = resolve_multi_inverter_status({0: "Hold charging", 1: "Hold exporting"}, "Demand") + if result != "Cross-charging": + print(" ERROR: expected 'Cross-charging' for Hold charging+Hold exporting, got {!r}".format(result)) + failed = True + + print("Test: cross-charging detected across a mix of sub-states on each side, regardless of precedence order") + result = resolve_multi_inverter_status({0: "Freeze charging", 1: "Hold charging", 2: "Exporting"}, "Demand") + if result != "Cross-charging": + print(" ERROR: expected 'Cross-charging' when any charge-side and any export-side state coexist, got {!r}".format(result)) + failed = True + + print("Test: a Read-Only/Calibration-style current_status is preserved even with an empty dict, not clobbered") + result = resolve_multi_inverter_status({}, "Calibration") + if result != "Calibration": + print(" ERROR: expected 'Calibration' preserved with no per-inverter core states recorded, got {!r}".format(result)) + failed = True + + print("Test: Calibration is preserved even when an earlier-processed inverter left a stale core state behind") + result = resolve_multi_inverter_status({0: "Charging"}, "Calibration") + if result != "Calibration": + print(" ERROR: expected 'Calibration' to override a stale core state left by an earlier inverter, got {!r}".format(result)) + failed = True + + return failed diff --git a/apps/predbat/unit_test.py b/apps/predbat/unit_test.py index 350f41389..7e76f9289 100644 --- a/apps/predbat/unit_test.py +++ b/apps/predbat/unit_test.py @@ -23,6 +23,7 @@ from tests.test_predict_pv_power import run_predict_pv_power_tests from tests.test_kernel_parity import run_kernel_parity_tests, run_model_kernel_tests from tests.test_execute import run_execute_tests +from tests.test_execute_multi_inverter_status import test_multi_inverter_status from tests.test_load_car_energy import test_load_car_energy_warns_when_configured_entity_has_no_data from tests.test_octopus_slots import run_load_octopus_slots_tests from tests.test_multi_car_iog import run_multi_car_iog_tests @@ -275,6 +276,7 @@ def main(): ("kernel_parity", run_kernel_parity_tests, "C++ prediction kernel vs Python engine parity tests", False), ("inverter", run_inverter_tests, "Inverter tests", False), ("execute", run_execute_tests, "Execute tests", False), + ("multi_inverter_status", test_multi_inverter_status, "Multi-inverter headline status resolution tests (#4446)", False), ("load_car_energy", test_load_car_energy_warns_when_configured_entity_has_no_data, "car_charging_energy configured-but-empty warning tests (#4458 follow-up)", False), ("basic_rates", test_basic_rates, "Basic rates tests", False), ("rate_min_forward_calc", test_rate_min_forward_calc, "Rate min forward calc tests", False), diff --git a/docs/what-does-predbat-do.md b/docs/what-does-predbat-do.md index 9cea1df47..ad77fa7c9 100644 --- a/docs/what-does-predbat-do.md +++ b/docs/what-does-predbat-do.md @@ -110,6 +110,10 @@ If there is a shortfall of generated solar power to meet the house load, the bat - **Hold exporting** - The plan was to force export but the minimum battery level was reached and thus the battery is kept in Demand mode. If the battery level again gets above the threshold it will be changed back to Export mode. +- **Cross-charging** - On a multi-inverter system, at least one inverter is in a charge-side state (Charging, Freeze charging or Hold charging) while another is in an export-side state (Exporting, Freeze exporting or Hold exporting) at the same time. This includes the case where both sides are merely holding at their target (e.g. Hold charging + Hold exporting), with no current actually flowing, not just active charging/exporting. +This is shown explicitly rather than the headline status just reflecting whichever inverter was processed last, so real disagreement between inverters is visible instead of hidden. +If inverters only disagree on sub-state within the same side (e.g. one still actively Charging while another has reached Hold charging), the most active sub-state is shown instead, since Cross-charging specifically means disagreement across the charge/export divide, not within one side. + - **Hold for car** and **Demand, Hold for car** - A car is charging (either Predbat-led or Octopus-led), the battery is in Demand mode, but is set to prevent discharging into the car (requires **switch.predbat_car_charging_from_battery** to be set to On).