Skip to content

Commit f65beb5

Browse files
fix(execute): stop the status detail repeating the headline state
The dashboard showed "Exporting target Exporting 19%-5%" - the status label twice. #4466 began prefixing each per-inverter detail segment with that inverter's own state so a mixed fleet could be read, but the detail text is appended directly to the headline status, so on any system whose inverters agree the label is simply repeated. A single-inverter install saw it on every Charging/Exporting/Freeze/Hold status. Defer assembly of the detail text until after the headline status is resolved and emit the per-segment label only when the segments actually differ. A fleet acting in unison, and every single-inverter install, gets the pre-#4466 text back; a mixed fleet keeps the labels that made #4466 worth doing - all three existing mixed-fleet assertions are unchanged. Every end-to-end case in test_execute.py runs two inverters, which is why the single-inverter rendering shipped broken with no test failing. Add direct build_status_extra() coverage plus a single-inverter export case; both were confirmed to fail against the pre-fix behaviour. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 2c2feea commit f65beb5

4 files changed

Lines changed: 123 additions & 13 deletions

File tree

apps/predbat/execute.py

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,31 @@ def resolve_multi_inverter_status(status_per_inverter, current_status):
7272
return current_status
7373

7474

75+
def build_status_extra(status_extra_parts):
76+
"""Assemble the per-inverter status detail text recorded during execute_plan().
77+
78+
Each entry is ``(inverter_id, lead, label, detail)``: ``lead`` is the introductory word used by the
79+
first inverter ("target"/"current SoC"), ``label`` is that inverter's own core charge/export state
80+
and ``detail`` the SoC figures.
81+
82+
The per-inverter ``label`` only carries information when the fleet's inverters actually disagree.
83+
#4466 added it so a mixed fleet could be read at all, but emitting it unconditionally repeated the
84+
headline status on every system whose inverters agree - a single-inverter install showed
85+
"Exporting target Exporting 19%-5%". Labels are therefore dropped when every segment carries the
86+
same one, restoring the pre-#4466 text for a single inverter and for any fleet acting in unison,
87+
and kept when they differ.
88+
"""
89+
if not status_extra_parts:
90+
return ""
91+
show_labels = len({label for _, _, label, _ in status_extra_parts}) > 1
92+
status_extra = ""
93+
for inverter_id, lead, label, detail in status_extra_parts:
94+
# Only the first inverter introduces the text; the rest are appended after a separator.
95+
status_extra += " {}".format(lead) if inverter_id == 0 else " /"
96+
status_extra += " {} {}".format(label, detail) if show_labels else " {}".format(detail)
97+
return status_extra
98+
99+
75100
class Execute:
76101
"""Execution mixin for applying optimised plans to physical inverters.
77102
@@ -81,7 +106,9 @@ class Execute:
81106
"""
82107

83108
def execute_plan(self):
84-
status_extra = "" # extra status text added to Predbat notifications
109+
# Per-inverter detail segments, assembled into the status text after the headline status is
110+
# resolved - see build_status_extra() for why they can't be concatenated inline.
111+
status_extra_parts = []
85112
status_hold_car = "" # car hold status text
86113
status_hold_iboost = "" # iBoost hold status text
87114
status_freeze_export = "" # freeze export during demand status text
@@ -249,8 +276,7 @@ def execute_plan(self):
249276

250277
status = "Freeze charging"
251278
status_per_inverter[inverter.id] = status
252-
status_extra += " target" if inverter.id == 0 else " /" # Append multi-inverter target SoC's together
253-
status_extra += " {} {}%".format(status, inverter.soc_percent)
279+
status_extra_parts.append((inverter.id, "target", status, "{}%".format(inverter.soc_percent))) # Append multi-inverter target SoC's together
254280
self.log("Inverter {} Freeze charging with SoC {}%".format(inverter.id, inverter.soc_percent))
255281
else:
256282
# 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):
295321
status_per_inverter[inverter.id] = status
296322
inverter.adjust_charge_window(charge_start_time, charge_end_time, self.minutes_now)
297323

298-
status_extra += " target" if inverter.id == 0 else " /" # append multi-inverter target SoC's together
299-
status_extra += " {} {}%-{}%".format(status, inverter.soc_percent, inv_target_soc_percent)
324+
status_extra_parts.append((inverter.id, "target", status, "{}%-{}%".format(inverter.soc_percent, inv_target_soc_percent))) # append multi-inverter target SoC's together
300325

301326
if not self.set_discharge_during_charge and resetPause:
302327
# Do we discharge discharge during charge
@@ -432,8 +457,7 @@ def execute_plan(self):
432457

433458
status = "Exporting"
434459
status_per_inverter[inverter.id] = status
435-
status_extra += " target" if inverter.id == 0 else " /" # append multi-inverter target SoC's together
436-
status_extra += " {} {}%-{}%".format(status, inverter.soc_percent, int(target))
460+
status_extra_parts.append((inverter.id, "target", status, "{}%-{}%".format(inverter.soc_percent, int(target)))) # append multi-inverter target SoC's together
437461
# Immediate export mode
438462
else:
439463
inverter.adjust_force_export(False)
@@ -453,17 +477,16 @@ def execute_plan(self):
453477
self.log("Export Freeze as exporting is now at/below target - current SoC {}kWh and target {}kWh".format(self.soc_kw, discharge_soc))
454478
status = "Freeze exporting"
455479
status_per_inverter[inverter.id] = status
456-
status_extra += " current SoC" if inverter.id == 0 else " /" # append multi-inverter target SoC's together
457-
status_extra += " {} {}%".format(status, inverter.soc_percent) # Discharge limit (99) is meaningless when Freeze Exporting so don't display it
480+
# Discharge limit (99) is meaningless when Freeze Exporting so don't display it
481+
status_extra_parts.append((inverter.id, "current SoC", status, "{}%".format(inverter.soc_percent))) # append multi-inverter target SoC's together
458482
isExporting = True
459483
target = self.export_window_best[0].get("target", self.export_limits_best[0])
460484
self.isExporting_Target = int(target)
461485
else:
462486
status = "Hold exporting"
463487
status_per_inverter[inverter.id] = status
464488
target = self.export_window_best[0].get("target", self.export_limits_best[0])
465-
status_extra += " target" if inverter.id == 0 else " /" # append multi-inverter target SoC's together
466-
status_extra += " {} {}%-{}%".format(status, inverter.soc_percent, inverter.soc_percent)
489+
status_extra_parts.append((inverter.id, "target", status, "{}%-{}%".format(inverter.soc_percent, inverter.soc_percent))) # append multi-inverter target SoC's together
467490
self.isExporting_Target = inverter.soc_percent
468491
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))
469492
else:
@@ -691,6 +714,7 @@ def execute_plan(self):
691714
# Resolve the headline status across all inverters rather than leaving whichever inverter was
692715
# processed last to silently win.
693716
status = resolve_multi_inverter_status(status_per_inverter, status)
717+
status_extra = build_status_extra(status_extra_parts)
694718

695719
# Set the charge/discharge status information
696720
self.set_charge_export_status(isCharging, isExporting, not (isCharging or isExporting))

apps/predbat/predbat.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
import pytz
3636
import asyncio
3737

38-
THIS_VERSION = "v8.48.5"
38+
THIS_VERSION = "v8.48.6"
3939

4040
from download import predbat_update_move, predbat_update_download, check_install, DEFAULT_PREDBAT_REPOSITORY
4141
from const import MINUTE_WATT

apps/predbat/tests/test_execute.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2230,6 +2230,8 @@ def run_execute_tests(my_predbat):
22302230
set_export_window=True,
22312231
soc_kw=100,
22322232
assert_status="Exporting",
2233+
# A fleet acting in unison must not repeat the headline status on every segment (v8.48.4 regression)
2234+
assert_status_extra=" target 100%-0% / 100%-0%",
22332235
car_slot=charge_window_best_slot,
22342236
car_charging_from_battery=True,
22352237
assert_force_export=True,
@@ -2259,6 +2261,32 @@ def run_execute_tests(my_predbat):
22592261
if failed:
22602262
return failed
22612263

2264+
# Single-inverter status text (v8.48.4 regression): every other case in this module runs two
2265+
# inverters, so the single-inverter rendering had no end-to-end coverage at all and shipped
2266+
# showing the headline status twice - "Exporting target Exporting 19%-5%".
2267+
two_inverters = my_predbat.inverters
2268+
my_predbat.inverters = [two_inverters[0]]
2269+
my_predbat.args["num_inverters"] = 1
2270+
failed |= run_execute_test(
2271+
my_predbat,
2272+
"single_inverter_export_status",
2273+
export_window_best=export_window_best,
2274+
export_limits_best=export_limits_best,
2275+
set_charge_window=True,
2276+
set_export_window=True,
2277+
soc_kw=100,
2278+
assert_status="Exporting",
2279+
assert_status_extra=" target 100%-0%",
2280+
assert_force_export=True,
2281+
assert_discharge_start_time_minutes=my_predbat.minutes_now,
2282+
assert_discharge_end_time_minutes=my_predbat.minutes_now + 60 + 1,
2283+
assert_immediate_soc_target=0,
2284+
)
2285+
my_predbat.inverters = two_inverters
2286+
my_predbat.args["num_inverters"] = 2
2287+
if failed:
2288+
return failed
2289+
22622290
failed |= run_execute_test(
22632291
my_predbat,
22642292
"no_discharge_car_demand1",

apps/predbat/tests/test_execute_multi_inverter_status.py

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
overwrite, which silently hid genuine cross-inverter disagreement (e.g. real cross-charging).
1414
"""
1515

16-
from execute import resolve_multi_inverter_status
16+
from execute import build_status_extra, resolve_multi_inverter_status
1717

1818

1919
def test_multi_inverter_status(my_predbat):
@@ -98,4 +98,62 @@ def test_multi_inverter_status(my_predbat):
9898
print(" ERROR: expected 'Calibration' to override a stale core state left by an earlier inverter, got {!r}".format(result))
9999
failed = True
100100

101+
failed |= test_build_status_extra()
102+
103+
return failed
104+
105+
106+
def test_build_status_extra():
107+
"""Verify the per-inverter status detail text only labels segments when inverters disagree.
108+
109+
#4466 began labelling every segment with that inverter's own state so a mixed fleet could be
110+
read. The label is redundant when the whole fleet agrees, because the headline status the detail
111+
is appended to already says it - a single-inverter export showed
112+
"Exporting target Exporting 19%-5%" in v8.48.4.
113+
"""
114+
failed = False
115+
print("**** Testing build_status_extra ****")
116+
117+
print("Test: no inverter recorded a detail segment - empty text")
118+
result = build_status_extra([])
119+
if result != "":
120+
print(" ERROR: expected '' with no segments, got {!r}".format(result))
121+
failed = True
122+
123+
print("Test: single inverter exporting - no repeated label (regression, GH v8.48.4)")
124+
result = build_status_extra([(0, "target", "Exporting", "19%-5%")])
125+
if result != " target 19%-5%":
126+
print(" ERROR: expected ' target 19%-5%', got {!r}".format(result))
127+
failed = True
128+
129+
print("Test: single inverter freeze exporting - uses its own lead word, still unlabelled")
130+
result = build_status_extra([(0, "current SoC", "Freeze exporting", "19%")])
131+
if result != " current SoC 19%":
132+
print(" ERROR: expected ' current SoC 19%', got {!r}".format(result))
133+
failed = True
134+
135+
print("Test: multiple inverters all in the same state - joined, still unlabelled")
136+
result = build_status_extra([(0, "target", "Charging", "50%-80%"), (1, "target", "Charging", "60%-80%")])
137+
if result != " target 50%-80% / 60%-80%":
138+
print(" ERROR: expected ' target 50%-80% / 60%-80%', got {!r}".format(result))
139+
failed = True
140+
141+
print("Test: inverters in different states - every segment labelled so the fleet can be read")
142+
result = build_status_extra([(0, "target", "Charging", "90%-100.0%"), (1, "target", "Hold charging", "100%-100.0%")])
143+
if result != " target Charging 90%-100.0% / Hold charging 100%-100.0%":
144+
print(" ERROR: expected labelled mixed-fleet text, got {!r}".format(result))
145+
failed = True
146+
147+
print("Test: cross-charging (charge and export at once) - labelled, as the headline says neither")
148+
result = build_status_extra([(0, "target", "Charging", "50%-80%"), (1, "target", "Exporting", "50%-5%")])
149+
if result != " target Charging 50%-80% / Exporting 50%-5%":
150+
print(" ERROR: expected labelled cross-charging text, got {!r}".format(result))
151+
failed = True
152+
153+
print("Test: only a non-zero inverter recorded a segment - separator used, no lead word")
154+
result = build_status_extra([(1, "target", "Exporting", "19%-5%")])
155+
if result != " / 19%-5%":
156+
print(" ERROR: expected ' / 19%-5%', got {!r}".format(result))
157+
failed = True
158+
101159
return failed

0 commit comments

Comments
 (0)