diff --git a/apps/predbat/execute.py b/apps/predbat/execute.py index 8b4d57e78..fe2e2a530 100644 --- a/apps/predbat/execute.py +++ b/apps/predbat/execute.py @@ -72,6 +72,31 @@ def resolve_multi_inverter_status(status_per_inverter, current_status): return current_status +def build_status_extra(status_extra_parts): + """Assemble the per-inverter status detail text recorded during execute_plan(). + + Each entry is ``(inverter_id, lead, label, detail)``: ``lead`` is the introductory word used by the + first inverter ("target"/"current SoC"), ``label`` is that inverter's own core charge/export state + and ``detail`` the SoC figures. + + The per-inverter ``label`` only carries information when the fleet's inverters actually disagree. + #4466 added it so a mixed fleet could be read at all, but emitting it unconditionally repeated the + headline status on every system whose inverters agree - a single-inverter install showed + "Exporting target Exporting 19%-5%". Labels are therefore dropped when every segment carries the + same one, restoring the pre-#4466 text for a single inverter and for any fleet acting in unison, + and kept when they differ. + """ + if not status_extra_parts: + return "" + show_labels = len({label for _, _, label, _ in status_extra_parts}) > 1 + status_extra = "" + for inverter_id, lead, label, detail in status_extra_parts: + # Only the first inverter introduces the text; the rest are appended after a separator. + status_extra += " {}".format(lead) if inverter_id == 0 else " /" + status_extra += " {} {}".format(label, detail) if show_labels else " {}".format(detail) + return status_extra + + class Execute: """Execution mixin for applying optimised plans to physical inverters. @@ -81,7 +106,9 @@ class Execute: """ def execute_plan(self): - status_extra = "" # extra status text added to Predbat notifications + # Per-inverter detail segments, assembled into the status text after the headline status is + # resolved - see build_status_extra() for why they can't be concatenated inline. + status_extra_parts = [] status_hold_car = "" # car hold status text status_hold_iboost = "" # iBoost hold status text status_freeze_export = "" # freeze export during demand status text @@ -249,8 +276,7 @@ def execute_plan(self): 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(status, inverter.soc_percent) + status_extra_parts.append((inverter.id, "target", status, "{}%".format(inverter.soc_percent))) # Append multi-inverter target SoC's together 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 @@ -295,8 +321,7 @@ def execute_plan(self): 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(status, inverter.soc_percent, inv_target_soc_percent) + status_extra_parts.append((inverter.id, "target", status, "{}%-{}%".format(inverter.soc_percent, inv_target_soc_percent))) # append multi-inverter target SoC's together if not self.set_discharge_during_charge and resetPause: # Do we discharge discharge during charge @@ -432,8 +457,7 @@ def execute_plan(self): 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(status, inverter.soc_percent, int(target)) + status_extra_parts.append((inverter.id, "target", status, "{}%-{}%".format(inverter.soc_percent, int(target)))) # append multi-inverter target SoC's together # Immediate export mode else: inverter.adjust_force_export(False) @@ -453,8 +477,8 @@ 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(status, inverter.soc_percent) # Discharge limit (99) is meaningless when Freeze Exporting so don't display it + # Discharge limit (99) is meaningless when Freeze Exporting so don't display it + status_extra_parts.append((inverter.id, "current SoC", status, "{}%".format(inverter.soc_percent))) # append multi-inverter target SoC's together isExporting = True target = self.export_window_best[0].get("target", self.export_limits_best[0]) self.isExporting_Target = int(target) @@ -462,8 +486,7 @@ def execute_plan(self): 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(status, inverter.soc_percent, inverter.soc_percent) + status_extra_parts.append((inverter.id, "target", status, "{}%-{}%".format(inverter.soc_percent, inverter.soc_percent))) # append multi-inverter target SoC's together 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: @@ -691,6 +714,7 @@ def execute_plan(self): # 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) + status_extra = build_status_extra(status_extra_parts) # Set the charge/discharge status information self.set_charge_export_status(isCharging, isExporting, not (isCharging or isExporting)) diff --git a/apps/predbat/predbat.py b/apps/predbat/predbat.py index 2fdd4b427..f77b6f26f 100644 --- a/apps/predbat/predbat.py +++ b/apps/predbat/predbat.py @@ -35,7 +35,7 @@ import pytz import asyncio -THIS_VERSION = "v8.48.5" +THIS_VERSION = "v8.48.6" from download import predbat_update_move, predbat_update_download, check_install, DEFAULT_PREDBAT_REPOSITORY from const import MINUTE_WATT diff --git a/apps/predbat/tests/test_execute.py b/apps/predbat/tests/test_execute.py index d8dfb51b7..9f6b7c7ae 100644 --- a/apps/predbat/tests/test_execute.py +++ b/apps/predbat/tests/test_execute.py @@ -2230,6 +2230,8 @@ def run_execute_tests(my_predbat): set_export_window=True, soc_kw=100, assert_status="Exporting", + # A fleet acting in unison must not repeat the headline status on every segment (v8.48.4 regression) + assert_status_extra=" target 100%-0% / 100%-0%", car_slot=charge_window_best_slot, car_charging_from_battery=True, assert_force_export=True, @@ -2259,6 +2261,32 @@ def run_execute_tests(my_predbat): if failed: return failed + # Single-inverter status text (v8.48.4 regression): every other case in this module runs two + # inverters, so the single-inverter rendering had no end-to-end coverage at all and shipped + # showing the headline status twice - "Exporting target Exporting 19%-5%". + two_inverters = my_predbat.inverters + my_predbat.inverters = [two_inverters[0]] + my_predbat.args["num_inverters"] = 1 + failed |= run_execute_test( + my_predbat, + "single_inverter_export_status", + export_window_best=export_window_best, + export_limits_best=export_limits_best, + set_charge_window=True, + set_export_window=True, + soc_kw=100, + assert_status="Exporting", + assert_status_extra=" target 100%-0%", + assert_force_export=True, + assert_discharge_start_time_minutes=my_predbat.minutes_now, + assert_discharge_end_time_minutes=my_predbat.minutes_now + 60 + 1, + assert_immediate_soc_target=0, + ) + my_predbat.inverters = two_inverters + my_predbat.args["num_inverters"] = 2 + if failed: + return failed + failed |= run_execute_test( my_predbat, "no_discharge_car_demand1", diff --git a/apps/predbat/tests/test_execute_multi_inverter_status.py b/apps/predbat/tests/test_execute_multi_inverter_status.py index d2f0585cd..37cca07a6 100644 --- a/apps/predbat/tests/test_execute_multi_inverter_status.py +++ b/apps/predbat/tests/test_execute_multi_inverter_status.py @@ -13,7 +13,7 @@ overwrite, which silently hid genuine cross-inverter disagreement (e.g. real cross-charging). """ -from execute import resolve_multi_inverter_status +from execute import build_status_extra, resolve_multi_inverter_status def test_multi_inverter_status(my_predbat): @@ -98,4 +98,62 @@ def test_multi_inverter_status(my_predbat): print(" ERROR: expected 'Calibration' to override a stale core state left by an earlier inverter, got {!r}".format(result)) failed = True + failed |= test_build_status_extra() + + return failed + + +def test_build_status_extra(): + """Verify the per-inverter status detail text only labels segments when inverters disagree. + + #4466 began labelling every segment with that inverter's own state so a mixed fleet could be + read. The label is redundant when the whole fleet agrees, because the headline status the detail + is appended to already says it - a single-inverter export showed + "Exporting target Exporting 19%-5%" in v8.48.4. + """ + failed = False + print("**** Testing build_status_extra ****") + + print("Test: no inverter recorded a detail segment - empty text") + result = build_status_extra([]) + if result != "": + print(" ERROR: expected '' with no segments, got {!r}".format(result)) + failed = True + + print("Test: single inverter exporting - no repeated label (regression, GH v8.48.4)") + result = build_status_extra([(0, "target", "Exporting", "19%-5%")]) + if result != " target 19%-5%": + print(" ERROR: expected ' target 19%-5%', got {!r}".format(result)) + failed = True + + print("Test: single inverter freeze exporting - uses its own lead word, still unlabelled") + result = build_status_extra([(0, "current SoC", "Freeze exporting", "19%")]) + if result != " current SoC 19%": + print(" ERROR: expected ' current SoC 19%', got {!r}".format(result)) + failed = True + + print("Test: multiple inverters all in the same state - joined, still unlabelled") + result = build_status_extra([(0, "target", "Charging", "50%-80%"), (1, "target", "Charging", "60%-80%")]) + if result != " target 50%-80% / 60%-80%": + print(" ERROR: expected ' target 50%-80% / 60%-80%', got {!r}".format(result)) + failed = True + + print("Test: inverters in different states - every segment labelled so the fleet can be read") + result = build_status_extra([(0, "target", "Charging", "90%-100.0%"), (1, "target", "Hold charging", "100%-100.0%")]) + if result != " target Charging 90%-100.0% / Hold charging 100%-100.0%": + print(" ERROR: expected labelled mixed-fleet text, got {!r}".format(result)) + failed = True + + print("Test: cross-charging (charge and export at once) - labelled, as the headline says neither") + result = build_status_extra([(0, "target", "Charging", "50%-80%"), (1, "target", "Exporting", "50%-5%")]) + if result != " target Charging 50%-80% / Exporting 50%-5%": + print(" ERROR: expected labelled cross-charging text, got {!r}".format(result)) + failed = True + + print("Test: only a non-zero inverter recorded a segment - separator used, no lead word") + result = build_status_extra([(1, "target", "Exporting", "19%-5%")]) + if result != " / 19%-5%": + print(" ERROR: expected ' / 19%-5%', got {!r}".format(result)) + failed = True + return failed