From f39e52efd3a9f10383df5bd28d5842f6c9de7cee Mon Sep 17 00:00:00 2001 From: glenn schrooyen Date: Thu, 8 Oct 2026 20:37:01 +0200 Subject: [PATCH 1/3] MAINT-14: maintenance with no SoC reading: charge times out, drain holds at 0 W Co-Authored-By: Claude Opus 5.5 --- .../MAINT-14-no-soc-in-maintenance.md | 70 ++++ goodwe_controller/app/maintenance.py | 88 ++++- goodwe_controller/test_maint14.py | 356 ++++++++++++++++++ goodwe_controller/test_maintenance.py | 3 +- 4 files changed, 507 insertions(+), 10 deletions(-) create mode 100644 goodwe_controller/DECISIONS/MAINT-14-no-soc-in-maintenance.md create mode 100644 goodwe_controller/test_maint14.py diff --git a/goodwe_controller/DECISIONS/MAINT-14-no-soc-in-maintenance.md b/goodwe_controller/DECISIONS/MAINT-14-no-soc-in-maintenance.md new file mode 100644 index 0000000..2dd4c9d --- /dev/null +++ b/goodwe_controller/DECISIONS/MAINT-14-no-soc-in-maintenance.md @@ -0,0 +1,70 @@ +# MAINT-14 - no SoC reading in a maintenance cycle + +**Status: for review.** Tests: `test_maint14.py`. No option added; the one new +number is a constant in `app/maintenance.py`. + +**The fault.** `Maintenance.tick()` returned early when the SoC was `None`, +before any timeout check. (1) A charge on a pack whose SoC reading was lost +never ended: no timeout, no back-off, no notice, and the 7-day backstop of +MAINT-13 sat behind the same return. (2) A drain kept commanding `discharge_w` +with an unknown level, which breaks the TEL-08 rule that an unknown level never +allows discharge. + +## Rules + +1. **Charge, no SoC.** The charge timeout (`CHARGE_TIMEOUT_MIN`, gate-open + minutes) and the 7-day wall backstop still run. When one fires: if the last + SoC seen was >= target - 3 the charge counts as done (the MAINT-13 "close + enough to full" rule, HOLD follows); otherwise it fails: MAINT-13 back-off + and its one notification. If no SoC was ever seen, it fails. +2. **Drain, no SoC.** DRAIN holds at 0 W and owns the setpoint, exactly as HOLD + does. The 12 h drain timeout still runs and fails as before. +3. **SoC comes back.** Nothing is recorded; the drain continues at the drain + rate, the charge carries on, and the stall wait of MAINT-13 restarts from + what it was: it paused while the reading was missing, so a gap never counts + as "no more charge". +4. **HOLD** is unchanged (0 W). +5. **Log.** One line when a drain starts holding for this reason ("the battery + level is unknown, so the drain holds at 0 W until it is back"), and one line + when the level has been missing for `NOSOC_NOTE_MIN` = 15 min. Both are said + once per episode; the old line that repeated every tick is gone. + +## Why + +- **"Last seen", not "best seen".** A pack that was at 99 and then dropped to 50 + before the reading went is not close to full. Only the last reading counts. + It is saved with the charge record (`charge_track.last`), and saved at once + when the reading first goes missing, so a restart mid-gap still knows it + (within the usual 5 min save interval otherwise). +- **Failure when nothing was seen.** With no level we cannot call a pack full. + The failure costs a 7-day wait and one notice; calling it done costs a + missed balance and a month-long false "complete". The first is the honest one. +- **Timeouts count the same minutes as with a reading.** Gate-open charging + minutes only; the wall backstop for a gate that never opens. +- **15 min.** A judgement. A poll glitch of a few minutes should not write a + line; a reading lost for a quarter of an hour is worth one. Raise it if the + action log shows noise. +- **0 W, not "stop the phase".** Aborting a drain on a short gap would throw + away the part already drained. Holding at 0 W loses nothing and the 12 h + timeout is the limit. + +## Cost + +- A drain that loses its reading stops draining, and the 12 h clock keeps + running. A gap of many hours can therefore fail a drain that was working. The + back-off then applies (7 days). +- A charge whose reading is lost early (last SoC below target - 3) fails at the + timeout, even if the pack was in fact filling. The same 7-day wait applies. + +## What this does not cover + +- **HOLD with no SoC** still holds 0 W and, as before, does not end while the + reading is missing: its time check sits behind the same early return. A HOLD + needs no level to end, so this is a candidate follow-up, not part of this fix. +- It does not tell the operator why the reading is missing (sensor, register, + link). It does not reuse the stale-input gate; it is the cycle's own rule. +- The charge itself is still driven by the control loop, which has its own + handling of a missing SoC (TEL-08 for the loop is a separate card). +- A restart loses at most 5 min of accounting, as in MAINT-13. The "unknown for + long" line may be said again after a restart: the episode start is in memory. +- Text added is two log lines, English only, like the other log lines. diff --git a/goodwe_controller/app/maintenance.py b/goodwe_controller/app/maintenance.py index 4edc5c9..8758185 100644 --- a/goodwe_controller/app/maintenance.py +++ b/goodwe_controller/app/maintenance.py @@ -84,6 +84,10 @@ FAIL_BACKOFF_DAYS = 7 CHARGE_TICK_MAX_MIN = 5 # The charge accounting is written to the state file at most this often. CHARGE_SAVE_MIN = 5 +# MAINT-14 (DECISIONS/MAINT-14-no-soc-in-maintenance.md): with no SoC reading +# the timeouts still run, a drain holds at 0 W, and the log says once, after this +# many minutes, that the level has been unknown for long. +NOSOC_NOTE_MIN = 15 # ⚠️ CFG-03. THE ONE IN-CODE MIRROR OF config.yaml FOR EVERY `maintenance_*` # OPTION, and the only place in the add-on where one of these numbers is typed. @@ -367,6 +371,10 @@ class Maintenance: # written to the state file every CHARGE_SAVE_MIN minutes. self._trk = None self._trk_saved = None + # MAINT-14: when the SoC reading went missing (None while it is there), + # and whether the "unknown for long" line was said in this episode. + self._nosoc_since = None + self._nosoc_said = False # -- state --------------------------------------------------------------- @property @@ -374,6 +382,7 @@ class Maintenance: return self.store.get("phase", IDLE) def _enter(self, phase: str, now: datetime, events: list, msg: str) -> None: + self._nosoc_since = None self._trk = None self._trk_saved = None if self.store.get("charge_track") is not None: @@ -473,7 +482,8 @@ class Maintenance: self._enter(DRAIN, now, events, "maintenance: forced start -> drain") return events - def _charge_account(self, now: datetime, soc: float, gate_open: bool) -> dict: + def _charge_account(self, now: datetime, soc: float | None, + gate_open: bool) -> dict: """MAINT-13: what this charge has used so far, as minutes of ALLOWED time. `open_min` counts only ticks with the price gate open; `flat_min` counts @@ -481,6 +491,9 @@ class Maintenance: both. One tick adds at most CHARGE_TICK_MAX_MIN, so a gap (add-on down) counts for nothing. A state file from before MAINT-13, or from another charge, starts from the wall-clock minutes so far: the old behaviour. + + MAINT-14: `soc` may be None (no reading). Time still counts, the stall + wait pauses, and `last` keeps the last SoC seen (saved with the rest). """ since = self.store.get("phase_started") trk = self._trk @@ -490,7 +503,9 @@ class Maintenance: trk = dict(raw) else: trk = {"since": since, "open_min": self._elapsed_min(now), - "flat_min": 0.0, "best": float(soc), "at": None} + "flat_min": 0.0, + "best": None if soc is None else float(soc), + "last": None if soc is None else float(soc), "at": None} self._trk = trk at = trk.get("at") try: @@ -500,10 +515,14 @@ class Maintenance: dt = 0.0 if last is None else min( CHARGE_TICK_MAX_MIN, max(0.0, (now - last).total_seconds() / 60.0)) trk["at"] = now.isoformat() - if soc > trk["best"]: + if soc is None: + pass + elif trk.get("best") is None or soc > trk["best"]: trk["best"], trk["flat_min"] = float(soc), 0.0 elif gate_open: trk["flat_min"] += dt + if soc is not None: + trk["last"] = float(soc) if gate_open: trk["open_min"] += dt saved = self._trk_saved @@ -522,6 +541,57 @@ class Maintenance: events.append(msg) return MaintResult(IDLE, True, 0.0, False, events, failed=kind) + def _no_soc(self, now: datetime, phase: str, gate_open: bool, + events: list) -> MaintResult: + """MAINT-14: one tick of a pack in a moving phase with no SoC reading. + + HOLD is unchanged (0 W). DRAIN holds at 0 W, like HOLD, and its 12 h + timeout still runs. CHARGE keeps its timeout and wall backstop: out of + time is a failure, except when the last SoC seen was within + CHARGE_NEAR_FULL_PCT of the target, which counts as done (MAINT-13). + Nothing here may start a phase change that needs the level. + """ + if self._nosoc_since is None: + self._nosoc_since = now + self._nosoc_said = False + self._trk_saved = None # save the last SoC seen, now + if phase == DRAIN: + msg = ("maintenance: the battery level is unknown, so the " + "drain holds at 0 W until it is back.") + _LOG.warning("%s", msg) + events.append(msg) + if (not self._nosoc_said and (now - self._nosoc_since).total_seconds() + >= NOSOC_NOTE_MIN * 60): + self._nosoc_said = True + msg = (f"maintenance: no battery level for {NOSOC_NOTE_MIN} min. " + f"The time limit of the {phase} still applies.") + _LOG.warning("%s", msg) + events.append(msg) + if phase == HOLD: + return MaintResult(HOLD, True, 0.0, False, events) + elapsed = self._elapsed_min(now) + if phase == DRAIN: + if elapsed > DRAIN_TIMEOUT_MIN: + return self._fail(now, "drain", + f"drain stuck, battery level unknown, after " + f"{elapsed / 60:.0f} h") + return MaintResult(DRAIN, True, 0.0, False, events) + trk = self._charge_account(now, None, gate_open) + if (trk["open_min"] > CHARGE_TIMEOUT_MIN + or elapsed > CHARGE_WALL_TIMEOUT_MIN): + last = trk.get("last") + if last is not None and last >= self.cfg.soc_target - CHARGE_NEAR_FULL_PCT: + self._enter(HOLD, now, events, + f"maintenance: charge time is up and the last level " + f"seen was {last:.0f} %, close enough to full, so the " + f"charge counts as done -> holding " + f"{self.cfg.hold_min} min to balance") + return MaintResult(HOLD, True, 0.0, False, events) + return self._fail(now, "charge", + f"charge stuck, battery level unknown, after " + f"{trk['open_min'] / 60:.0f} h of charging time") + return MaintResult(CHARGE, False, None, True, events) + def abort(self, now: datetime, why: str) -> list: events = [] self._enter(IDLE, now, events, f"maintenance: ABORT ({why})") @@ -609,13 +679,13 @@ class Maintenance: # transitions below mean anything, and a stale "100 %" would end the # charge phase having charged nothing. Hold the current phase's command # and wait; the phase timeout is the backstop. + # + # MAINT-14: "hold and wait" is not "wait for ever". The timeouts run + # here too, and a drain holds at 0 W (an unknown level never allows + # discharge, TEL-08). See `_no_soc`. if soc is None: - _LOG.warning("maintenance: no SoC reading, holding phase %s", phase) - if phase == DRAIN: - return MaintResult(DRAIN, True, self.cfg.discharge_w, False, events) - if phase == HOLD: - return MaintResult(HOLD, True, 0.0, False, events) - return MaintResult(CHARGE, False, None, True, events) + return self._no_soc(now, phase, gate_open, events) + self._nosoc_since = None elapsed = self._elapsed_min(now) diff --git a/goodwe_controller/test_maint14.py b/goodwe_controller/test_maint14.py new file mode 100644 index 0000000..37bb1cc --- /dev/null +++ b/goodwe_controller/test_maint14.py @@ -0,0 +1,356 @@ +"""MAINT-14: maintenance with no SoC reading. +`python3 test_maint14.py` + +Graded here: charge and drain with SoC None at one and two packs - the charge +timeout and 7 day backstop still run (failure, or done when the last SoC was +>= target - 3), the drain holds at 0 W and times out, SoC coming back resumes +normally with no back-off, the log lines (once per episode), a restart while the +SoC is missing, and one pack's missing SoC leaving the other pack alone. +See DECISIONS/MAINT-14-no-soc-in-maintenance.md. + +Mutating what this file grades? Read MUTATION.md first (stale bytecode). +""" + +import os +import sys +import tempfile +from datetime import datetime, timedelta, timezone + +from app.maintenance import ( + CHARGE, CHARGE_NEAR_FULL_PCT, CHARGE_TIMEOUT_MIN, CHARGE_WALL_TIMEOUT_MIN, + DRAIN, DRAIN_TIMEOUT_MIN, FAIL_BACKOFF_DAYS, HOLD, IDLE, NOSOC_NOTE_MIN, + MaintConfig, Maintenance, arbitrate, +) +from app.store import DEFAULTS, Store + +UTC = timezone.utc +fails = [] + + +def check(name, cond): + print(f" {'ok ' if cond else 'FAIL'} {name}") + if not cond: + fails.append(name) + + +CFG = MaintConfig(enabled=True, interval_days=28, start_hour=19, + discharge_w=2000, charge_w=2000, soc_floor=11, + soc_target=100, hold_min=120) +T0 = datetime(2026, 10, 4, 18, 24, tzinfo=UTC) +MIN = timedelta(minutes=1) + + +class Mem: + def __init__(self, data=None): + self.data = dict(DEFAULTS, **(data or {})) + + def set(self, k, v): + self.data[k] = v + + def set_time(self, k, when): + self.data[k] = when.astimezone(UTC).isoformat() if when else None + + def get_time(self, k): + raw = self.data.get(k) + return datetime.fromisoformat(raw) if raw else None + + def save(self): + pass + + +def in_drain(store=None, unit=None, at=T0): + m = Maintenance(CFG, store or Mem(), unit=unit, name=unit or "") + m.force_start(at) + assert m.phase == DRAIN + return m + + +def in_charge(store=None, unit=None, at=T0): + m = in_drain(store, unit, at) + m.tick(at, 11.0) + assert m.phase == CHARGE + return m + + +def run(m, start, minutes, soc, gate=True, step=1): + """Tick every `step` min; returns (last result, end time, first result that + is not the starting phase with its time, all events).""" + ph, t, res, out, ev = m.phase, start, None, None, [] + for i in range(0, minutes, step): + t = start + i * MIN + s = soc(i) if callable(soc) else soc + res = m.tick(t, s, gate_open=gate) + ev += res.events + if out is None and res.phase != ph: + out = (res, t) + break + return res, start + minutes * MIN, out, ev + + +print("1. charge with no SoC") +m = in_charge() +res, _, out, _ = run(m, T0, 600, None) +check("it keeps charging: the loop owns the setpoint, no start of a new phase", + out is None and res.phase == CHARGE and res.charge_only + and not res.owns_setpoint) + +m = in_charge() +run(m, T0, 60, 60.0) +res, _, out, ev = run(m, T0 + 60 * MIN, CHARGE_TIMEOUT_MIN + 10, None) +check("the charge timeout still runs: it fails once the time is up (last SoC 60)", + out and out[0].failed == "charge" and out[0].phase == IDLE) +check("...with the MAINT-13 back-off and its one plain line", + m.store.get_time("last_failed") is not None + and sum("did not finish" in e for e in ev) == 1 + and f"{FAIL_BACKOFF_DAYS} days" in ev[-1]) +check("...it took 36 h of allowed time from the charge start, not more", + out and out[1] <= T0 + timedelta(minutes=CHARGE_TIMEOUT_MIN + 10)) +check("...and the pack does not want a start in the back-off", + not m.wants_start((T0 + timedelta(days=3)).replace(hour=19))) + +m = in_charge() +res, _, out, _ = run(m, T0, CHARGE_TIMEOUT_MIN + 10, None) +check("never saw a SoC: the timeout is a failure, not 'done'", + out and out[0].failed == "charge") + +for last in (97.0, 99.0): + m = in_charge() + run(m, T0, 10, last) + res, _, out, ev = run(m, T0 + 10 * MIN, CHARGE_TIMEOUT_MIN + 20, None) + check(f"last SoC {last:.0f} (>= target - {CHARGE_NEAR_FULL_PCT}): time up counts as done", + out and out[0].phase == HOLD and out[0].failed == "" + and m.store.get_time("last_failed") is None) + check("...one plain line says so", + sum("counts as done" in e for e in ev) == 1 + and f"{last:.0f} %" in " ".join(ev)) +m = in_charge() +run(m, T0, 10, 96.0) +res, _, out, _ = run(m, T0 + 10 * MIN, CHARGE_TIMEOUT_MIN + 20, None) +check("last SoC 96 (below target - 3): time up is a failure", + out and out[0].failed == "charge") +m = in_charge() +run(m, T0, 10, 99.0) +run(m, T0 + 10 * MIN, 10, 50.0) +res, _, out, _ = run(m, T0 + 20 * MIN, CHARGE_TIMEOUT_MIN + 20, None) +check("'last seen' is the last reading, not the best (99 then 50: failure)", + out and out[0].failed == "charge") + +print("2. the 7 day backstop, with the gate shut") +m = in_charge() +run(m, T0, 10, 70.0) +res, _, out, _ = run(m, T0 + 10 * MIN, CHARGE_WALL_TIMEOUT_MIN + 30, None, + gate=False, step=5) +check("gate never opens, last SoC 70: failure at 7 days", + out and out[0].failed == "charge" + and out[1] >= T0 + timedelta(minutes=CHARGE_WALL_TIMEOUT_MIN) + and out[1] <= T0 + timedelta(minutes=CHARGE_WALL_TIMEOUT_MIN + 10)) +m = in_charge() +run(m, T0, 10, 98.0) +res, _, out, _ = run(m, T0 + 10 * MIN, CHARGE_WALL_TIMEOUT_MIN + 30, None, + gate=False, step=5) +check("gate never opens, last SoC 98: done at 7 days", + out and out[0].phase == HOLD and out[0].failed == "" + and out[1] >= T0 + timedelta(minutes=CHARGE_WALL_TIMEOUT_MIN)) +m = in_charge() +run(m, T0, 10, 70.0) +res, _, out, _ = run(m, T0 + 10 * MIN, 3000, None, gate=False, step=5) +check("gate shut, no SoC: the open-time clock does not run (no early end)", + out is None) + +print("3. SoC comes back mid-charge") +m = in_charge() +run(m, T0, 30, 80.0) +run(m, T0 + 30 * MIN, 600, None) +res, _, out, _ = run(m, T0 + 630 * MIN, 5, 80.0) +check("normal behaviour resumes: still charging, nothing recorded", + out is None and res.phase == CHARGE + and m.store.get_time("last_failed") is None) +res, _, out, _ = run(m, T0 + 640 * MIN, 10, 100.0) +check("...and it completes at the target as usual", + out and out[0].phase == HOLD and m.store.get_time("last_failed") is None) +m = in_charge() +run(m, T0, 30, 98.0) +run(m, T0 + 30 * MIN, 600, None) +res, _, out, _ = run(m, T0 + 630 * MIN, 30, 98.0) +check("the stall wait paused while the SoC was missing (30 min flat after the " + "gap is not 60)", out is None) +res, _, out, _ = run(m, T0 + 660 * MIN, 40, 98.0) +check("...and completes once 60 min flat are in", out and out[0].phase == HOLD) +m = in_charge() +run(m, T0, 10, 70.0) +run(m, T0 + 10 * MIN, CHARGE_TIMEOUT_MIN - 100, None) +res, _, out, _ = run(m, T0 + (CHARGE_TIMEOUT_MIN - 90) * MIN, 5, 70.0) +check("a gap just before the limit then a reading: no failure, no back-off", + out is None and m.store.get_time("last_failed") is None) + +print("4. drain with no SoC") +m = in_drain() +run(m, T0, 30, 60.0) +res, _, out, ev = run(m, T0 + 30 * MIN, 60, None) +check("DRAIN holds at 0 W, owned by maintenance, the same as HOLD", + out is None and res.phase == DRAIN and res.owns_setpoint + and res.setpoint_w == 0.0 and not res.charge_only) +hold = in_charge() +hold.tick(T0 + MIN, 100.0) +hr = hold.tick(T0 + 2 * MIN, None) +check("...the same answer as HOLD gives", + (hr.owns_setpoint, hr.setpoint_w) == (res.owns_setpoint, res.setpoint_w)) +check("never above 0 W on any missing tick", + all(m.tick(T0 + (100 + i) * MIN, None).setpoint_w == 0.0 for i in range(30))) +check("one plain line that the drain holds, once, not per tick", + sum("drain holds at 0 W" in e for e in ev) == 1) +check("one plain line after the threshold, once", + sum("no battery level for" in e for e in ev) == 1 + and f"{NOSOC_NOTE_MIN} min" in " ".join(ev)) +m = in_drain() +res, _, out, ev = run(m, T0, NOSOC_NOTE_MIN - 1, None) +check("under the threshold the 'no level for long' line is not said", + not any("no battery level for" in e for e in ev)) +m = in_drain() +res, _, out, ev = run(m, T0, DRAIN_TIMEOUT_MIN + 10, None) +check("the 12 h drain timeout still runs: failure with back-off", + out and out[0].failed == "drain" and out[0].phase == IDLE + and out[1] > T0 + timedelta(minutes=DRAIN_TIMEOUT_MIN) + and m.store.get_time("last_failed") is not None) +m = in_drain() +run(m, T0, 60, None) +res, _, out, _ = run(m, T0 + 60 * MIN, 5, 60.0) +check("SoC back: the drain continues at the drain rate", + out is None and res.phase == DRAIN and res.owns_setpoint + and res.setpoint_w == CFG.discharge_w) +res, _, out, _ = run(m, T0 + 70 * MIN, 5, 10.0) +check("...and ends at the floor as usual", + out and out[0].phase == CHARGE and m.store.get_time("last_failed") is None) +m = in_drain() +run(m, T0, 60, None) +run(m, T0 + 60 * MIN, 5, 60.0) +res, _, out, ev = run(m, T0 + 70 * MIN, 5, None) +check("a second episode says its lines again", + any("drain holds at 0 W" in e for e in ev)) + +print("5. the log is once per episode") +m = in_charge() +res, _, out, ev = run(m, T0, 300, None) +check("a 5 h charge episode: exactly one 'no level for long' line, none per tick", + len(ev) == 1 and "no battery level for" in ev[0]) +run(m, T0 + 300 * MIN, 3, 70.0) +res, _, out, ev = run(m, T0 + 303 * MIN, 60, None) +check("...and one more for a new episode", len(ev) == 1) +m = in_charge() +res, _, out, ev = run(m, T0, 5, None) +check("a 5 min gap says nothing", ev == []) + +print("6. restart while the SoC is missing") +with tempfile.TemporaryDirectory() as d: + path = os.path.join(d, "state.json") + m = Maintenance(CFG, Store(path)) + m.force_start(T0) + m.tick(T0, 11.0) + run(m, T0, 97, lambda i: 90.0 if i < 96 else 98.0) # 98 seen 1 min ago + run(m, T0 + 97 * MIN, 3, None) # SoC lost; state saved at once + t = T0 + 100 * MIN # restart 3 min after the loss, before the next 5 min save + m2 = Maintenance(CFG, Store(path)) + r = m2.tick(t, None) + check("a fresh process reads charge, and still charges", m2.phase == CHARGE + and r.phase == CHARGE and r.charge_only) + check("...with the open minutes and the last SoC seen from the file", + m2._trk is not None and m2._trk["last"] == 98.0 + and 92 <= m2._trk["open_min"] <= 105) + res, _, out, _ = run(m2, t + MIN, CHARGE_TIMEOUT_MIN, None, step=5) + check("...so the timeout counts from the first charge minute and, with the " + "last SoC 98, completes (no failure)", + out and out[0].phase == HOLD and out[0].failed == "" + and out[1] < T0 + timedelta(minutes=CHARGE_TIMEOUT_MIN + 60)) + # drain: restart keeps it at 0 W and keeps the clock + path2 = os.path.join(d, "s2.json") + md = Maintenance(CFG, Store(path2)) + md.force_start(T0) + md.tick(T0 + 100 * MIN, None) + md2 = Maintenance(CFG, Store(path2)) + r = md2.tick(T0 + 101 * MIN, None) + check("a drain restarted with no SoC holds at 0 W", md2.phase == DRAIN + and r.setpoint_w == 0.0 and r.owns_setpoint) + res, _, out, _ = run(md2, T0 + 102 * MIN, DRAIN_TIMEOUT_MIN, None, step=5) + check("...and the 12 h clock is from the drain start, not the restart", + out and out[0].failed == "drain" + and out[1] <= T0 + timedelta(minutes=DRAIN_TIMEOUT_MIN + 10)) + # a charge restarted with no record of any SoC (old state file) + old = {"phase": "charge", "phase_started": (T0 - timedelta(hours=35)).isoformat(), + "last_completed": None, "last_start_attempt": None, "auto": True} + import json + path3 = os.path.join(d, "s3.json") + json.dump(old, open(path3, "w")) + mo = Maintenance(CFG, Store(path3)) + res, _, out, _ = run(mo, T0, 120, None) + check("an old state file with no SoC seen: the charge fails at 36 h", + out and out[0].failed == "charge") + +print("7. two packs") +a, b = (Maintenance(CFG, Mem(), unit="inverter-a", name="inverter-a"), + Maintenance(CFG, Mem(), unit="inverter-b", name="inverter-b")) +a.force_start(T0) +b_alone = Maintenance(CFG, Mem(), unit="inverter-b", name="inverter-b") +same = True +for i in range(0, 120, 10): + t = T0 + i * MIN + plan = arbitrate([a, b], t) + ra = a.tick(t, None, blocked_by=plan[0]) + rb = b.tick(t, 55.0, blocked_by=plan[1]) + rb0 = b_alone.tick(t, 55.0, blocked_by=plan[1]) + same = same and (rb.phase, rb.setpoint_w, rb.deferred) == \ + (rb0.phase, rb0.setpoint_w, rb0.deferred) +check("A drains with no SoC at 0 W while B (SoC 55, idle) is as it is when A " + "is healthy", ra.setpoint_w == 0.0 and ra.owns_setpoint and rb.phase == IDLE + and rb.setpoint_w is None and not rb.owns_setpoint) +check("...B's answers are unchanged tick by tick", same) +res = None +for i in range(0, DRAIN_TIMEOUT_MIN + 30, 10): + t = T0 + 120 * MIN + i * MIN + plan = arbitrate([a, b], t) + res = a.tick(t, None, blocked_by=plan[0]) + b.tick(t, 55.0, blocked_by=plan[1]) + if res.failed: + break +check("A's drain times out and A backs off (A's keys only)", + res.failed == "drain" and a.store.get_time("last_failed") is not None + and b.store.get_time("last_failed") is None) +slot = (T0 + timedelta(days=1)).replace(hour=19, minute=5) +check("...then B, with its SoC, takes the slot as usual", + arbitrate([a, b], slot) == ["", ""] and b.wants_start(slot) + and not a.wants_start(slot)) +b.force_start(slot) +rb = b.tick(slot + 5 * MIN, 50.0) +check("...and B's drain runs at the drain rate", + rb.phase == DRAIN and rb.setpoint_w == CFG.discharge_w) +# the other way: B charges with no SoC, A has a SoC +a = Maintenance(CFG, Mem(), unit="inverter-a", name="inverter-a") +b = Maintenance(CFG, Mem(), unit="inverter-b", name="inverter-b") +b.force_start(T0) +b.tick(T0, 11.0) +run(b, T0, 5, 97.0) +a_alone = Maintenance(CFG, Mem(), unit="inverter-a", name="inverter-a") +same, t = True, T0 + 10 * MIN +while t < T0 + timedelta(minutes=CHARGE_TIMEOUT_MIN + 30): + plan = arbitrate([a, b], t) + rb = b.tick(t, None, blocked_by=plan[1]) + ra = a.tick(t, 60.0, blocked_by=plan[0]) + ra0 = a_alone.tick(t, 60.0, blocked_by=plan[0]) + same = same and (ra.phase, ra.setpoint_w, ra.deferred) == \ + (ra0.phase, ra0.setpoint_w, ra0.deferred) + if rb.phase != CHARGE: + break + t += 5 * MIN +check("B charges with no SoC (last 97): A (SoC 60) is unchanged tick by tick", + same) +check("...B ends as done (HOLD), A untouched, no failure on either", + rb.phase == HOLD and rb.failed == "" + and b.store.get_time("last_failed") is None + and a.store.get_time("last_failed") is None and a.phase == IDLE) + +print() +if fails: + print(f"{len(fails)} FAILED") + for f in fails: + print(" -", f) + sys.exit(1) +print("all passed") diff --git a/goodwe_controller/test_maintenance.py b/goodwe_controller/test_maintenance.py index f38a054..61736e8 100644 --- a/goodwe_controller/test_maintenance.py +++ b/goodwe_controller/test_maintenance.py @@ -122,7 +122,8 @@ m = Maintenance(cfg, FakeStore()) m.force_start(t0) r = m.tick(t0 + timedelta(minutes=5), soc=None) check("no SoC: holds the phase rather than guessing", r.phase == DRAIN) -check("no SoC: still commands the drain rate", r.setpoint_w == 2500) +check("no SoC: MAINT-14 the drain holds at 0 W, never discharges", + r.owns_setpoint and r.setpoint_w == 0.0) m = Maintenance(cfg, FakeStore()) m.store.set("phase", "banana") From ef35f4b925e2db39a30e5f746b9f630ebc47e86c Mon Sep 17 00:00:00 2001 From: glenn schrooyen Date: Thu, 8 Oct 2026 21:25:35 +0200 Subject: [PATCH 2/3] MAINT-14: accurate no-level wording per phase; name the limit that ended the charge Co-Authored-By: Claude Opus 5.5 --- .../MAINT-14-no-soc-in-maintenance.md | 3 ++- goodwe_controller/app/maintenance.py | 12 +++++++--- goodwe_controller/test_maint14.py | 23 +++++++++++++++++++ 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/goodwe_controller/DECISIONS/MAINT-14-no-soc-in-maintenance.md b/goodwe_controller/DECISIONS/MAINT-14-no-soc-in-maintenance.md index 2dd4c9d..0ffa494 100644 --- a/goodwe_controller/DECISIONS/MAINT-14-no-soc-in-maintenance.md +++ b/goodwe_controller/DECISIONS/MAINT-14-no-soc-in-maintenance.md @@ -60,7 +60,8 @@ allows discharge. - **HOLD with no SoC** still holds 0 W and, as before, does not end while the reading is missing: its time check sits behind the same early return. A HOLD - needs no level to end, so this is a candidate follow-up, not part of this fix. + needs no level to end, so this is a candidate follow-up, not part of this fix. It is card MAINT-15. The "no battery level" line in + HOLD says only that the hold waits for the level. - It does not tell the operator why the reading is missing (sensor, register, link). It does not reuse the stale-input gate; it is the cycle's own rule. - The charge itself is still driven by the control loop, which has its own diff --git a/goodwe_controller/app/maintenance.py b/goodwe_controller/app/maintenance.py index 8758185..fc7ecb5 100644 --- a/goodwe_controller/app/maintenance.py +++ b/goodwe_controller/app/maintenance.py @@ -563,8 +563,11 @@ class Maintenance: if (not self._nosoc_said and (now - self._nosoc_since).total_seconds() >= NOSOC_NOTE_MIN * 60): self._nosoc_said = True + tail = ("The hold waits for the level to come back." + if phase == HOLD else + f"The time limit of the {phase} still applies.") msg = (f"maintenance: no battery level for {NOSOC_NOTE_MIN} min. " - f"The time limit of the {phase} still applies.") + f"{tail}") _LOG.warning("%s", msg) events.append(msg) if phase == HOLD: @@ -587,9 +590,12 @@ class Maintenance: f"charge counts as done -> holding " f"{self.cfg.hold_min} min to balance") return MaintResult(HOLD, True, 0.0, False, events) + if elapsed > CHARGE_WALL_TIMEOUT_MIN: + lim = f"the {CHARGE_WALL_TIMEOUT_MIN // 1440}-day limit" + else: + lim = f"{trk['open_min'] / 60:.0f} h of charging time" return self._fail(now, "charge", - f"charge stuck, battery level unknown, after " - f"{trk['open_min'] / 60:.0f} h of charging time") + f"charge stuck, battery level unknown, after {lim}") return MaintResult(CHARGE, False, None, True, events) def abort(self, now: datetime, why: str) -> list: diff --git a/goodwe_controller/test_maint14.py b/goodwe_controller/test_maint14.py index 37bb1cc..7a6873e 100644 --- a/goodwe_controller/test_maint14.py +++ b/goodwe_controller/test_maint14.py @@ -240,6 +240,29 @@ m = in_charge() res, _, out, ev = run(m, T0, 5, None) check("a 5 min gap says nothing", ev == []) +print("5b. the words per phase") +m = in_charge() +m.tick(T0 + MIN, 100.0) +_, _, _, ev = run(m, T0 + 2 * MIN, 30, None) +note = [e for e in ev if "no battery level for" in e] +check("HOLD: the line exists and claims no time limit", + len(note) == 1 and "time limit" not in note[0] and "waits" in note[0]) +m = in_charge() +_, _, _, ev = run(m, T0, 30, None) +check("charge keeps its wording", any("time limit of the charge" in e for e in ev)) +m = in_charge() +run(m, T0, 10, 70.0) +_, _, out, ev = run(m, T0 + 10 * MIN, CHARGE_WALL_TIMEOUT_MIN + 30, None, + gate=False, step=5) +fail = [e for e in ev if "ABORT" in e] +check("7-day backstop: the reason names the 7-day limit, not '0 h'", + len(fail) == 1 and "7-day limit" in fail[0] and "0 h" not in fail[0]) +m = in_charge() +_, _, out, ev = run(m, T0, CHARGE_TIMEOUT_MIN + 10, None) +fail = [e for e in ev if "ABORT" in e] +check("charge-time limit: the reason gives the hours of charging time", + len(fail) == 1 and "36 h of charging time" in fail[0]) + print("6. restart while the SoC is missing") with tempfile.TemporaryDirectory() as d: path = os.path.join(d, "state.json") From 61a61941bb5c65eeac25e749ff09b9cadabc0f93 Mon Sep 17 00:00:00 2001 From: MAINT-14 review Date: Thu, 8 Oct 2026 22:14:26 +0200 Subject: [PATCH 3/3] MAINT-14 review: tests for a HOLD episode after a no-SoC charge, a MAINT-13 state file, and a true twin site at N=2 Co-Authored-By: Claude Opus 5.5 --- goodwe_controller/test_maint14.py | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/goodwe_controller/test_maint14.py b/goodwe_controller/test_maint14.py index 7a6873e..8296610 100644 --- a/goodwe_controller/test_maint14.py +++ b/goodwe_controller/test_maint14.py @@ -262,6 +262,13 @@ _, _, out, ev = run(m, T0, CHARGE_TIMEOUT_MIN + 10, None) fail = [e for e in ev if "ABORT" in e] check("charge-time limit: the reason gives the hours of charging time", len(fail) == 1 and "36 h of charging time" in fail[0]) +m = in_charge() +run(m, T0, 10, 98.0) +_, _, out, _ = run(m, T0 + 10 * MIN, CHARGE_TIMEOUT_MIN + 20, None) +_, _, _, ev = run(m, out[1] + MIN, 30, None) +check("charge done with no SoC, then HOLD with no SoC: a new episode, so the " + "HOLD line is said", out[0].phase == HOLD + and sum("hold waits" in e for e in ev) == 1) print("6. restart while the SoC is missing") with tempfile.TemporaryDirectory() as d: @@ -307,19 +314,36 @@ with tempfile.TemporaryDirectory() as d: res, _, out, _ = run(mo, T0, 120, None) check("an old state file with no SoC seen: the charge fails at 36 h", out and out[0].failed == "charge") + # a MAINT-13 state file: charge_track with a best but no `last` + since = (T0 - timedelta(hours=35)).isoformat() + m13 = dict(old, phase_started=since, charge_track={ + "since": since, "open_min": 2100.0, "flat_min": 0.0, "best": 99.0, + "at": (T0 - MIN).isoformat()}) + path4 = os.path.join(d, "s4.json") + json.dump(m13, open(path4, "w")) + mo = Maintenance(CFG, Store(path4)) + res, _, out, _ = run(mo, T0, 120, None) + check("a MAINT-13 state file (no last SoC): no crash, the charge fails at " + "36 h (best is not last)", out and out[0].failed == "charge") print("7. two packs") a, b = (Maintenance(CFG, Mem(), unit="inverter-a", name="inverter-a"), Maintenance(CFG, Mem(), unit="inverter-b", name="inverter-b")) a.force_start(T0) -b_alone = Maintenance(CFG, Mem(), unit="inverter-b", name="inverter-b") +# the twin site: the same two packs, A healthy (SoC 60, still above the floor) +a_ok, b_alone = (Maintenance(CFG, Mem(), unit="inverter-a", name="inverter-a"), + Maintenance(CFG, Mem(), unit="inverter-b", name="inverter-b")) +a_ok.force_start(T0) same = True for i in range(0, 120, 10): t = T0 + i * MIN plan = arbitrate([a, b], t) + plan0 = arbitrate([a_ok, b_alone], t) + same = same and plan[1] == plan0[1] ra = a.tick(t, None, blocked_by=plan[0]) rb = b.tick(t, 55.0, blocked_by=plan[1]) - rb0 = b_alone.tick(t, 55.0, blocked_by=plan[1]) + a_ok.tick(t, 60.0, blocked_by=plan0[0]) + rb0 = b_alone.tick(t, 55.0, blocked_by=plan0[1]) same = same and (rb.phase, rb.setpoint_w, rb.deferred) == \ (rb0.phase, rb0.setpoint_w, rb0.deferred) check("A drains with no SoC at 0 W while B (SoC 55, idle) is as it is when A "