Skip to content

Commit 308f348

Browse files
chalfontchubbyclaudespringfall2008
authored
fix(execute): resolve multi-inverter status from all inverters, not just the last one processed (#4466)
* fix(execute): resolve multi-inverter status from all inverters, not just the last one processed execute_plan()'s headline status was a single variable overwritten once per inverter in the loop, so the dashboard silently showed whichever inverter happened to be processed last - hiding real disagreement between inverters, including genuine cross-charging (one inverter charging while another discharges at the same time), which gcoan confirmed is a real, known phenomenon on multi-inverter Sigenergy/GivEnergy-style systems. Track each inverter's own final core state in status_per_inverter (keyed by id, so an inverter passing through multiple assignments in its own processing still just keeps its own last value). After the loop, resolve one headline via resolve_multi_inverter_status(): if inverters disagree across the charge/export divide, surface it as "Cross-charging" rather than picking one side arbitrarily; if they only disagree on sub-state within the same side (e.g. one still Charging, another already Hold charging), show the most active one, since that's what the fleet is actually still doing overall. Extracted as a small pure function for direct unit testing rather than needing to drive execute_plan()'s full branching to construct genuine multi-inverter disagreement scenarios. Three existing execute.py tests (charge_imbalance2, charge_freeze_imb1, charge_freeze_imb4) asserted the old "last inverter wins" artifact as if it were correct (e.g. "Hold charging" when one inverter was actually still charging toward target) - updated to assert the corrected, more informative aggregate instead. Fixes #4446 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat(execute): include each inverter's own state in the status tooltip Follow-on to the #4446 headline fix. The status_extra text (the "detail" attribute on predbat.status, shown as the tooltip/more-info text) already concatenated each inverter's SoC->target numerically for multi-inverter setups ("target 80%-40% / 60%-40%") but didn't say which state each entry belonged to. Prefix each entry with that inverter's own state ("target Charging 80%-40% / Hold charging 60%-40%") - reuses the existing append pattern and status_per_inverter/status values already computed for the #4446 fix, no new machinery. Single-inverter setups are unaffected. Added assert_status_extra to the shared execute test harness and locked in real values for charge_imbalance2 and charge_freeze_imb1, which already exercise genuine multi-inverter disagreement. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs: document the new Cross-charging status gcoan asked on #4466 for the new multi-inverter Cross-charging status (introduced by resolve_multi_inverter_status()) to be documented alongside the other predbat.status values. * fix(execute): don't let a Charging/Exporting fleet state suppress the iBoost hold status text status_hold_iboost was only ever set inside the same guard (status not in ["Exporting", "Charging"]) that gates the actual discharge-pause actions, so the annotation could never be recorded once any inverter's status reached Charging/Exporting. Split the guard so it still gates the pause/reserve actions but the status text update always runs, matching the pattern already used by the car-holding block just above it. * fix(execute): keep the iBoost hold status text and Calibration headline correct on multi-inverter fleets The final commit of this PR decoupled the "Hold for iBoost" status annotation from the pause action it describes - the annotation became a sibling of the inner discharge-hold guard instead of nested inside it, so it could now fire whenever the outer iBoost condition held, regardless of whether a pause actually happened this cycle. Re-nest it so the annotation stays coupled to boostHolding actually firing. Also fixes resolve_multi_inverter_status() so a Calibration break correctly overrides any stale core state an earlier-processed inverter left in status_per_inverter, matching what the function's own docstring already claimed but didn't implement. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(output): recognise Cross-charging as export activity in the yesterday-plan reconstruction Cross-charging (#4466) contains "charging" but not "exporting" as a string, so calculate_yesterday()'s slot classification silently dropped the export half of a genuine cross-charging minute, showing it as plain charging only. Add yesterday_slot_is_exporting() and use it at both the search and the window-building call sites. Also clarifies (no behaviour change) that find_charge_curve()'s exact-match status checks deliberately exclude Cross-charging minutes from curve learning - another inverter is drawing/feeding power at the same time, so the sample isn't a clean single-inverter reading. And tightens the Cross-charging doc entry, which described it as both inverters "genuinely" charging/exporting when it also fires for two merely-holding sub-states (Hold charging + Hold exporting) with no current actually flowing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test: cover both #4466 fixes through their real integration paths Both fixes were previously only covered at the extracted-function level (resolve_multi_inverter_status / yesterday_slot_is_exporting), which proves the helper's logic but not that the surrounding code actually reaches it. Add the two integration cases and verify each genuinely fails when its fix is reverted: - execute: "calibration_after_charging_inverter" drives execute_plan() with a real two-inverter fleet where inverter 0 reaches Charging before inverter 1 enters calibration and breaks the loop. Without the fix the headline resolves back to the stale "Charging". Needed per-inverter escape hatches (in_calibration_array and the immediate-target/isCharging asserts) because the break leaves the fleet genuinely half-processed - the test now documents that real state rather than papering over it. - calculate_yesterday: a full "Cross-charging" status history must rebuild both charge AND export windows. Captured from inside a publish_html_plan mock, since the reconstructed windows only exist between the fake-window block and the restore at the end of the function. Without the fix the export side comes back empty. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Trefor Southwell <48591903+springfall2008@users.noreply.github.com> Co-authored-by: Trefor Southwell <tdlj@tdlj.net>
1 parent 9ad008e commit 308f348

8 files changed

Lines changed: 404 additions & 43 deletions

File tree

apps/predbat/execute.py

Lines changed: 87 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,51 @@
2626
Execute Predbat plan
2727
"""
2828

29+
# Per-inverter core charge/export states, used to resolve one headline status across a multi-inverter
30+
# fleet instead of letting whichever inverter is processed last silently win. Precedence lists are
31+
# ordered most-active-first so the most informative sub-state is shown when inverters within the same
32+
# side disagree (e.g. one still actively Charging while another has already reached Hold charging).
33+
CHARGE_STATE_PRECEDENCE = ["Charging", "Freeze charging", "Hold charging"]
34+
EXPORT_STATE_PRECEDENCE = ["Exporting", "Freeze exporting", "Hold exporting"]
35+
CHARGE_SIDE_STATES = set(CHARGE_STATE_PRECEDENCE)
36+
EXPORT_SIDE_STATES = set(EXPORT_STATE_PRECEDENCE)
37+
38+
39+
def resolve_multi_inverter_status(status_per_inverter, current_status):
40+
"""Resolve one headline status across a multi-inverter fleet.
41+
42+
``status_per_inverter`` holds each inverter's own final core charge/export state, keyed by
43+
inverter id, as recorded during execute_plan()'s per-inverter loop - a plain dict overwrite per
44+
inverter, so it correctly reflects each inverter's own last-set state even if that inverter's own
45+
processing passed through more than one core-state assignment.
46+
47+
If inverters disagree across the charge/export divide (one genuinely charging while another
48+
discharges at the same time) that's real cross-charging, not noise - surfaced explicitly rather
49+
than silently showing whichever inverter happened to be processed last. If they only disagree on
50+
sub-state within the same side (e.g. one still actively Charging, another already Hold charging),
51+
the most active one is shown - it's the most informative and matches what the fleet is actually
52+
doing overall.
53+
54+
Falls through to ``current_status`` unchanged when no inverter reached a core charge/export state
55+
at all (pure Demand, Read-Only, Calibration, or a Hold-for-car/iBoost annotation with nothing else
56+
going on) - those cases are already correct and untouched by this resolution. Calibration always
57+
wins outright even if ``status_per_inverter`` holds a stale core state from an inverter processed
58+
before the one that entered calibration mode, since calibration force-overrides every inverter's
59+
controls and that must not be hidden behind a leftover Charging/Exporting label.
60+
"""
61+
if current_status == "Calibration":
62+
return current_status
63+
states_present = set(status_per_inverter.values())
64+
charge_states_present = states_present & CHARGE_SIDE_STATES
65+
export_states_present = states_present & EXPORT_SIDE_STATES
66+
if charge_states_present and export_states_present:
67+
return "Cross-charging"
68+
if charge_states_present:
69+
return next(candidate for candidate in CHARGE_STATE_PRECEDENCE if candidate in charge_states_present)
70+
if export_states_present:
71+
return next(candidate for candidate in EXPORT_STATE_PRECEDENCE if candidate in export_states_present)
72+
return current_status
73+
2974

3075
class Execute:
3176
"""Execution mixin for applying optimised plans to physical inverters.
@@ -40,6 +85,11 @@ def execute_plan(self):
4085
status_hold_car = "" # car hold status text
4186
status_hold_iboost = "" # iBoost hold status text
4287
status_freeze_export = "" # freeze export during demand status text
88+
# Each inverter's own final core charge/export state, keyed by inverter id - used after the
89+
# loop to detect genuine cross-charging (one inverter charging while another discharges at
90+
# the same time) rather than silently showing whichever inverter's status happened to be
91+
# set last, which hides that the fleet is fighting itself.
92+
status_per_inverter = {}
4393

4494
in_alert = self.alert_active_keep.get(self.minutes_now, 0) > 0
4595
in_manual_soc = self.manual_soc_keep.get(self.minutes_now, 0) > 0
@@ -198,14 +248,16 @@ def execute_plan(self):
198248
resetDischarge = False
199249

200250
status = "Freeze charging"
251+
status_per_inverter[inverter.id] = status
201252
status_extra += " target" if inverter.id == 0 else " /" # Append multi-inverter target SoC's together
202-
status_extra += " {}%".format(inverter.soc_percent)
253+
status_extra += " {} {}%".format(status, inverter.soc_percent)
203254
self.log("Inverter {} Freeze charging with SoC {}%".format(inverter.id, inverter.soc_percent))
204255
else:
205256
# We can only hold charge if a) we have a way to hold the charge level on the reserve or with a pause feature
206257
# and the current charge level is above the target for all inverters
207258
if self.set_soc_enable and inverter.soc_percent >= inv_target_soc_percent:
208259
status = "Hold charging"
260+
status_per_inverter[inverter.id] = status
209261
self.log(
210262
"Inverter {} Hold charging as SoC {}% is above target SoC {}% (global soc target {}%) set_discharge_during_charge {}".format(
211263
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):
240292
inverter.adjust_charge_window(charge_start_time, charge_end_time, self.minutes_now)
241293
else:
242294
status = "Charging"
295+
status_per_inverter[inverter.id] = status
243296
inverter.adjust_charge_window(charge_start_time, charge_end_time, self.minutes_now)
244297

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

248301
if not self.set_discharge_during_charge and resetPause:
249302
# Do we discharge discharge during charge
@@ -378,8 +431,9 @@ def execute_plan(self):
378431
self.isExporting_Target = int(target)
379432

380433
status = "Exporting"
434+
status_per_inverter[inverter.id] = status
381435
status_extra += " target" if inverter.id == 0 else " /" # append multi-inverter target SoC's together
382-
status_extra += " {}%-{}%".format(inverter.soc_percent, int(target))
436+
status_extra += " {} {}%-{}%".format(status, inverter.soc_percent, int(target))
383437
# Immediate export mode
384438
else:
385439
inverter.adjust_force_export(False)
@@ -398,16 +452,18 @@ def execute_plan(self):
398452

399453
self.log("Export Freeze as exporting is now at/below target - current SoC {}kWh and target {}kWh".format(self.soc_kw, discharge_soc))
400454
status = "Freeze exporting"
455+
status_per_inverter[inverter.id] = status
401456
status_extra += " current SoC" if inverter.id == 0 else " /" # append multi-inverter target SoC's together
402-
status_extra += " {}%".format(inverter.soc_percent) # Discharge limit (99) is meaningless when Freeze Exporting so don't display it
457+
status_extra += " {} {}%".format(status, inverter.soc_percent) # Discharge limit (99) is meaningless when Freeze Exporting so don't display it
403458
isExporting = True
404459
target = self.export_window_best[0].get("target", self.export_limits_best[0])
405460
self.isExporting_Target = int(target)
406461
else:
407462
status = "Hold exporting"
463+
status_per_inverter[inverter.id] = status
408464
target = self.export_window_best[0].get("target", self.export_limits_best[0])
409465
status_extra += " target" if inverter.id == 0 else " /" # append multi-inverter target SoC's together
410-
status_extra += " {}%-{}%".format(inverter.soc_percent, inverter.soc_percent)
466+
status_extra += " {} {}%-{}%".format(status, inverter.soc_percent, inverter.soc_percent)
411467
self.isExporting_Target = inverter.soc_percent
412468
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))
413469
else:
@@ -477,25 +533,29 @@ def execute_plan(self):
477533

478534
# iBoost running?
479535
boostHolding = False
480-
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"]:
481-
if inverter.inv_has_timed_pause:
482-
if resetPause:
483-
inverter.adjust_pause_mode(pause_discharge=True)
484-
resetPause = False
485-
else:
486-
if resetDischarge:
487-
inverter.adjust_discharge_rate(0)
488-
resetDischarge = False
489-
if self.set_reserve_enable:
490-
inverter.adjust_reserve(min(inverter.soc_percent + 1, 100))
491-
resetReserve = False
492-
boostHolding = True
493-
self.log("Disabling battery discharge whilst iBoost is running")
494-
if ("Hold for iBoost" not in status) and (status_hold_iboost == ""):
495-
if status == "Demand":
496-
status = "Hold for iBoost"
536+
if self.set_charge_window and self.iboost_enable and self.iboost_prevent_discharge and self.iboost_running_full:
537+
# Only pause discharge on this inverter, and only annotate the status as held for
538+
# iBoost, if the fleet isn't already Charging/Exporting - pausing would conflict with
539+
# that, and the annotation must stay coupled to whether a hold actually happened here.
540+
if status not in ["Exporting", "Charging"]:
541+
if inverter.inv_has_timed_pause:
542+
if resetPause:
543+
inverter.adjust_pause_mode(pause_discharge=True)
544+
resetPause = False
497545
else:
498-
status_hold_iboost = ", Hold for iBoost"
546+
if resetDischarge:
547+
inverter.adjust_discharge_rate(0)
548+
resetDischarge = False
549+
if self.set_reserve_enable:
550+
inverter.adjust_reserve(min(inverter.soc_percent + 1, 100))
551+
resetReserve = False
552+
boostHolding = True
553+
self.log("Disabling battery discharge whilst iBoost is running")
554+
if ("Hold for iBoost" not in status) and (status_hold_iboost == ""):
555+
if status == "Demand":
556+
status = "Hold for iBoost"
557+
else:
558+
status_hold_iboost = ", Hold for iBoost"
499559

500560
# Reset charge/discharge rate
501561
if resetPause:
@@ -628,6 +688,10 @@ def execute_plan(self):
628688
self.count_inverter_writes[inverter.id] += inverter.count_register_writes
629689
inverter.count_register_writes = 0
630690

691+
# Resolve the headline status across all inverters rather than leaving whichever inverter was
692+
# processed last to silently win.
693+
status = resolve_multi_inverter_status(status_per_inverter, status)
694+
631695
# Set the charge/discharge status information
632696
self.set_charge_export_status(isCharging, isExporting, not (isCharging or isExporting))
633697
self.isCharging = isCharging

apps/predbat/inverter.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1152,7 +1152,11 @@ def find_charge_curve(self, discharge):
11521152
else:
11531153
search_range = range(99, 85, -1)
11541154

1155-
# Find 100% end points
1155+
# Find 100% end points. The exact-match checks below ("Charging" / "Exporting",
1156+
# "Discharging") deliberately exclude "Cross-charging" minutes - during genuine
1157+
# cross-charging another inverter is simultaneously drawing/feeding power at the
1158+
# same time, so this inverter's battery_power reading isn't a clean single-inverter
1159+
# charge/discharge sample and would corrupt the learned curve if included.
11561160
for data_point in search_range:
11571161
for minute in range(1, min_len):
11581162
# Start trigger is when the SoC just increased above the data point

apps/predbat/output.py

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,17 @@
5252
}
5353

5454

55+
def yesterday_slot_is_exporting(slot_status):
56+
"""True when a historical ``predbat.status`` string (already lower-cased) represents export
57+
activity for the "yesterday" plan reconstruction in ``calculate_yesterday()``.
58+
59+
Includes "cross-charging" explicitly - it genuinely straddles both sides of the fleet at once,
60+
but as a string it contains "charging" and not "exporting", so a plain substring check on
61+
"exporting" alone would silently drop the export half of a real cross-charging slot.
62+
"""
63+
return "exporting" in slot_status or "cross-charging" in slot_status
64+
65+
5566
class Output:
5667
"""Output and sensor publishing mixin.
5768
@@ -3183,15 +3194,18 @@ def calculate_yesterday(self):
31833194
slot_status = predbat_status.get(slot_minute, "").lower()
31843195
real_minute = minute + slot_offset
31853196

3186-
if "exporting" in slot_status:
3197+
# Cross-charging genuinely straddles both sides - track it as both an exporting
3198+
# and a charging slot (its name contains "charging" but not "exporting"), so
3199+
# these are independent "if"s rather than "if/elif".
3200+
if yesterday_slot_is_exporting(slot_status):
31873201
export_during_slot = slot_status
31883202
if export_start_minute is None:
31893203
export_start_minute = real_minute
31903204
if slot_offset == 5:
31913205
export_start_minute -= 5
31923206
if charge_start_minute is not None:
31933207
charge_end_minute = export_start_minute
3194-
elif "charging" in slot_status:
3208+
if "charging" in slot_status:
31953209
charge_during_slot = slot_status
31963210
if charge_start_minute is None:
31973211
charge_start_minute = real_minute
@@ -3206,7 +3220,7 @@ def calculate_yesterday(self):
32063220
if charge_end_minute is None and charge_start_minute is not None:
32073221
charge_end_minute = minute + self.plan_interval_minutes
32083222

3209-
if "exporting" in export_during_slot:
3223+
if yesterday_slot_is_exporting(export_during_slot):
32103224
# Assume exporting at this time
32113225
self.export_window_best.append({"start": export_start_minute, "end": export_end_minute})
32123226
if "freeze" in export_during_slot:

0 commit comments

Comments
 (0)