From 138e45b53fbe6f7c8d01bf370a014550fb08a2a2 Mon Sep 17 00:00:00 2001 From: admin Date: Thu, 8 Oct 2026 02:25:55 +0200 Subject: [PATCH 01/17] SAFETY-103: no swing on old battery readings after a drain ends While a battery settles from its ended drain (SAFETY-95), the house point was grid + every battery's reading. A register can be 10 s older than the meter, so B swung 860 -> 1650 -> 870 W on A's old reading (rig check 5). Now the house point is held and moved only as far as the meter proves: each battery is between its last changed reading and its commands since (a reading that changed this cycle, or a refused side, as it reads). N=1 and 1 s registers are byte-identical to release/1.0. DECISIONS/SAFETY-103-old-readings-after-a-drain.md, test_safety103.py. Co-Authored-By: Claude Opus 5.5 --- .../SAFETY-103-old-readings-after-a-drain.md | 79 ++++ goodwe_controller/DOCS.md | 3 +- goodwe_controller/app/main.py | 57 ++- goodwe_controller/test_safety103.py | 397 ++++++++++++++++++ goodwe_controller/tools/safety103_plant.py | 195 +++++++++ 5 files changed, 727 insertions(+), 4 deletions(-) create mode 100644 goodwe_controller/DECISIONS/SAFETY-103-old-readings-after-a-drain.md create mode 100644 goodwe_controller/test_safety103.py create mode 100644 goodwe_controller/tools/safety103_plant.py diff --git a/goodwe_controller/DECISIONS/SAFETY-103-old-readings-after-a-drain.md b/goodwe_controller/DECISIONS/SAFETY-103-old-readings-after-a-drain.md new file mode 100644 index 0000000..8f2adc7 --- /dev/null +++ b/goodwe_controller/DECISIONS/SAFETY-103-old-readings-after-a-drain.md @@ -0,0 +1,79 @@ +# SAFETY-103 - no swing on old battery readings after a drain ends + +**Status: implemented 2026-10-08, for review.** Tests: `test_safety103.py`. + +**The defect** (SAFETY-95 rig test, check 5, F1). While a battery settles +from its ended drain, SAFETY-95 steers on the house's point: grid + every +battery's reading - aim. The meter is 1 s old; a battery register can be +10 s old (the SAFETY-90 register model). At E+13 A's reading said 1238 W +(6.8 s old) while A was at 616 W, so the house looked ~620 W bigger: B went +860 -> 1650 -> 870 W. B's own old reading does the same the other way. + +**The rule** (`main.py`, `_track_reads()`, `_drain_grid_w()`, N>=2, only +while a drain's tail settles): + +1. Each cycle, per battery: the range its power can be in. That is its last + changed reading, widened by every command on its wire since (a lagging + battery moves from one toward the others, never beyond). A new reading + starts the range again. +2. A reading that changed this cycle is taken as it reads. So is the side a + battery refuses (SAFETY-73): it is not heading that way. +3. The house point is held (first cycle: the loop's start from SAFETY-95). + It moves only as far as the meter proves: to the nearest value of grid + + the batteries' ranges - aim. + +So the loop no longer moves on a reading that has not caught up. A real +house step still moves it, as soon as the meter shows more than the +batteries can explain. + +**Why not the tester's rule** ("a battery whose reading is older than its +last command counts at its command"). B's command changes almost every +cycle, so B is almost always at its command. That is the plain meter, which +chases A's tail: B swings 1420 W. A lag model needs the plant's time +constant, which is not known for a client's inverter. Measured on the plant +below (N=2, 1200 W, A Off; swing = a move of the others later undone): + +| rule | 10 s lag | 5 s lag | 20 s lag | surplus, 5 s lag | +|---|---|---|---|---| +| release/1.0 (readings) | 760 | 840 | 150 | 800 | +| tester's: at the command | 1420 | 1020 | 1600 | 800 | +| lag model, tau 10 s | 70 | 140 | 80 | 580 | +| lag model, tau 5 s, plant 20 s | | | 600 | | +| hold all window | 0 | 0 | 0 | 0 | +| **this rule** | **0** | **0** | **0** | **130** | + +Holding all window ignores a house step: a 2 kW kettle at E+5 s imported +14.4 Wh over the next 30 s (release 11.5, this rule 11.3); 2 kW more sun +exported 16.9 Wh (release 8.5, this rule 8.9). + +**Before -> after** (`test_safety103.py`, `tools/safety103_plant.py`: 800 +W/s ramp, 10 s lag, meter 1 s, registers 10 s at the rig's phases, B idles +at -100 W; summed swing of the others in the 23 s after the end): + +| case | release/1.0 | after | +|---|---|---| +| rig check 5: N=2, 1200 W, A Off | B 1140 -> 850 -> 1630 -> 870, swing 760 W | B holds 1140 W | +| N=3, 1200 W / 4000 W, A Off | 680 / 630 W | 0 / 0 W | +| solar surplus, N=2 / N=3 | 890 / 740 W | 130 / 130 W | +| lag 5 or 20 s, registers 5 or 10 s, N=2 and N=3 | | at most 10 W | +| registers as fresh as the meter (1 s) | | rows byte-identical | +| refusal end | | rows byte-identical | +| N=1, every end | | rows byte-identical (also against the release/1.0 tree) | + +**Cost.** In the window, a house change smaller than what the batteries +could still be doing is not followed until a reading catches up (at most a +register period) or the window ends (23 s). Under a surplus the others +still swing up to 130 W, a third of a slew step. + +**Not covered.** +- Outside the drain tail the loop steers on the meter; readings only reach + the saturation freeze (SAFETY-96), unchanged. +- A lost link or a refusal ends the tail at once (A is out of the split, or + reads ~0 W). The swing after them is the loop's own on a lagging plant, + the same with 1 s registers and on release/1.0 (up to 1.9 kW at N=3 under + a surplus): not this card. +- A battery that overshoots its command, or a reading offset beyond the + range, can put the true power outside the range; the meter then moves the + point, as before. +- The end cycle's start (SAFETY-95 step 2) still uses the readings; the + batteries were steady during the drain, so they are current there. diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md index 6f42660..8771c2a 100644 --- a/goodwe_controller/DOCS.md +++ b/goodwe_controller/DOCS.md @@ -1704,7 +1704,8 @@ When a battery's link drops, the others take over one slew step a cycle, and control does not stall on the dropped battery's power (SAFETY-96). When a drain ends (done, Off, timeout, abort, lost link, refused), the other batteries cover the house at once but do not export its drain; then -they move one slew step a cycle (SAFETY-95). +they move one slew step a cycle (SAFETY-95). While that battery settles, +an old battery reading does not move them (SAFETY-103). #### Capacity-tariff exposure of the grid-following loop diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index 016ec58..c255a73 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -1031,6 +1031,11 @@ class Controller: # SAFETY-95: batteries settling after their drain ended; meanwhile # the loop's error counts every battery at its command. self._drain_tail = set() + # SAFETY-103: per battery, the range its power can be in since its + # reading last changed (that reading and every command since), and + # the house point held while a drain's tail settles. + self._read_span = {} + self._drain_house = None # SAFETY-96: last cycle's commandable batteries, this cycle's start # point per battery where a reseed moved it, and batteries settling # after a step the loop did not make ({name: since, monotonic}). @@ -2527,13 +2532,56 @@ class Controller: most PLANT_SETTLE_DOWN_S), the meter still shows the drain's tail and the others' climb, so the loop would chase them (B down, then up again). Meanwhile the batteries count at the site command: the error - is the house's own point minus the loop's start.""" + is the house's own point minus the loop's start. + + SAFETY-103: a reading can be up to a register period (10 s) older + than the meter, so the house point is held, and moved only as far as + the meter proves: to the nearest value the batteries can be at (each + between its last changed reading and its commands since; a reading + that changed this cycle, or the side a battery refuses, is taken as + it reads). DECISIONS/SAFETY-103-old-readings-after-a-drain.md. + """ grid = float(self.grid) self._drain_tail &= set(self._settle) & set(live) if not self._drain_tail: + self._drain_house = None return grid - return grid - float(self.target) + sum( - float(inv.batt) for inv in self.inverters if inv.name in live) + aim = self.tuning.target_grid_w + lo = hi = grid - aim + for inv in self.inverters: + if inv.name not in live: + continue + r = float(inv.batt) + span = self._read_span.get(inv.name) + if (span is None or span[2] != inv.batt_fresh.changed_mono + or span[2] == self._now_mono): + span = (r, r) + # A battery that refuses one way (SAFETY-73) is not heading + # that way, whatever it was asked: that side is its reading. + if (inv.refused or 0) > 0: + span = (min(span[0], r), r) + elif (inv.refused or 0) < 0: + span = (r, max(span[1], r)) + lo, hi = lo + span[0], hi + span[1] + held = (float(self.target) if self._drain_house is None + else self._drain_house) + self._drain_house = min(hi, max(lo, held)) + return self._drain_house - float(self.target) + aim + + def _track_reads(self) -> None: + """SAFETY-103, N>=2: per battery, the range its power can be in now: + its last changed reading and every command on its wire since (a + lagging battery moves from one toward the others, never beyond).""" + if len(self.inverters) < 2: + return + for inv in self.inverters: + if inv.batt is None: + continue + cmd, since = inv.standing_w(), inv.batt_fresh.changed_mono + lo, hi, was = self._read_span.get(inv.name, (0.0, 0.0, None)) + if was is None or since != was: + lo = hi = float(inv.batt) + self._read_span[inv.name] = (min(lo, cmd), max(hi, cmd), since) def _reseed_drop(self, live) -> None: """SAFETY-96, N>=2: a battery that drops out of the split (stale @@ -4914,6 +4962,9 @@ class Controller: # level: the two halves of §6.2's identity break in opposite # directions. See `site_batt_w` and test_alloc.py part 8. site_batt = self.site_batt_w + # SAFETY-103: what each battery can be doing (its reading may be + # older than the meter). + self._track_reads() # SAFETY-96: a battery the loop cannot move counts at its command, # so its own power does not freeze the loop (the meter has it). site_batt += self._held_out_w(_live96) diff --git a/goodwe_controller/test_safety103.py b/goodwe_controller/test_safety103.py new file mode 100644 index 0000000..f4729d3 --- /dev/null +++ b/goodwe_controller/test_safety103.py @@ -0,0 +1,397 @@ +"""Runnable check for SAFETY-103: no swing on old battery readings after a +drain ends. `python3 test_safety103.py` + +While a battery settles from its ended drain (SAFETY-95), the loop works +from the grid plus each battery's reading. A reading can be one register +period (10 s) older than the meter: on the rig (SAFETY-95 test, check 5) A's +reading still said 1238 W while A was at 616 W, and B went 860 -> 1650 -> +870 W. Now the house point is held, and moved only as far as the meter +proves (each battery between its last changed reading and its commands +since). Drives the REAL Controller on tools/safety103_plant.py: a lagging +battery, the meter every 1 s, registers every 10 s at the rig's phases, B +idling at -100 W. DECISIONS/SAFETY-103-old-readings-after-a-drain.md. + +1. The rig case and every drain end at N=2 and N=3, 10 s registers: no + swing beyond one slew step; release/1.0 swings (non-vacuous). +2. SAFETY-95/96's gains stay: no jump at the end, no give into a surplus, + one slew step a cycle, refusal and stale link still learned. +3. A house step inside the window is still followed. +4. With readings as fresh as the meter, every row is release/1.0's; N=1 is + untouched. +5. The rule's parts, one by one. + +Mutations (MUTATION.md, purged copies) are listed in the pull request. +""" + +import hashlib +import logging +import math +import os +import sys + +HERE = os.path.dirname(os.path.abspath(__file__)) +sys.path.insert(0, HERE) +sys.path.insert(0, os.path.join(HERE, "tools")) +logging.disable(logging.WARNING) + +import safety103_plant as p # noqa: E402 +from app.main import Controller # noqa: E402 +from app.maintenance import DRAIN # noqa: E402 + +fails = [] +SLEW = 400.0 # the rig's slew_w +AIM = -10.0 # the rig's target_grid_w +T = p.T +A = p.A + + +def check(name, cond): + print(f" {'ok ' if cond else 'FAIL'} {name}") + if not cond: + fails.append(name) + + +fixed = Controller._drain_grid_w +track = getattr(Controller, "_track_reads", None) + + +def _release(self, live): + """release/1.0's `_drain_grid_w` (SAFETY-95 as merged): every battery at + its reading.""" + grid = float(self.grid) + self._drain_tail &= set(self._settle) & set(live) + if not self._drain_tail: + return grid + return grid - float(self.target) + sum( + float(inv.batt) for inv in self.inverters if inv.name in live) + + +def without(fn): + """Run `fn` with SAFETY-103 taken out (release/1.0's rule).""" + Controller._drain_grid_w = _release + try: + return fn() + finally: + Controller._drain_grid_w = fixed + + +def soc_done(u, t): + return 5.0 if (u == "a" and t >= T) else 50.0 + + +# A's drain ends at T: switched Off (the rig case), completed on SoC, its +# battery refusing (gives nothing from T; SAFETY-73 learns it later), or its +# link going stale (writes refused from T, its board to 0 W 25 s later). +ENDS = { + "Off": dict(), + "complete": dict(before=None, soc=soc_done), + "refusal": dict(before=None, dead=("a", T), dur=T + 150.0), + "stale link": dict(before=None, reject={"a": T}, dead=("a", T + 25.0), + dur=T + 150.0), +} + + +def end_of(rows): + """The first cycle A's drain is over (its command left 2000 W, or its + phase left DRAIN); None if never.""" + return next((r["t"] for r in rows if r["t"] >= T and ( + r["cmd"][0] < 1990.0 or r["phase"][0] != DRAIN)), None) + + +def measure(rows, n, house): + """From the end on: the largest swing of the others' summed command in + the 23 s window (a move later undone; the slew step is the site's); their + largest command on the end cycle; the site target's largest step after + the end cycle; the others' largest command after the end (for a + surplus); the grid's range in the window.""" + te = end_of(rows) + if te is None: + return dict(end=math.nan, swing=math.nan, first=math.nan, + step=math.nan, give=math.nan, gmin=math.nan, + gmax=math.nan, rows=rows) + win = [r for r in rows if te <= r["t"] <= te + 23.0] + sw = 0.0 + w = [sum(r["cmd"][1:]) for r in win][1:] + for j in range(len(w)): + sw = max(sw, min(w[j] - min(w[:j + 1]), w[j] - min(w[j:])), + min(max(w[:j + 1]) - w[j], max(w[j:]) - w[j])) + seg = [r["target"] for r in rows if r["t"] >= te] + return dict( + end=te, swing=sw, + first=max(win[0]["cmd"][1:]), + step=max(abs(b - a) for a, b in zip(seg, seg[1:])), + give=max(max(r["cmd"][1:]) for r in rows if r["t"] >= te), + gmin=min(r["grid"] for r in win), gmax=max(r["grid"] for r in win), + rows=rows) + + +def run(kind, n, house, **k): + return p.run(n, house, **dict(ENDS[kind], **k))[0] + + +TABLE = [] +# A settles from its drain: the tail this rule is about. A refusing battery +# already reads ~0 W, and a dropped one is out of the split (the meter has +# it), so their tails last at most a cycle. +SETTLE = ("Off", "complete") + + +def case(kind, n, house, **k): + now = measure(run(kind, n, house, **k), n, house) + old = measure(without(lambda: run(kind, n, house, **k)), n, house) + fresh = measure(run(kind, n, house, reg_s=1.0, **k), n, house) + TABLE.append((kind, n, house, now, old, fresh)) + return now, old, fresh + + +# ============================================================================ # +print("1. no swing on an old reading: every drain end, N=2 and N=3, 10 s " + "registers") +for kind in ENDS: + for n in (2, 3): + for house in (1200.0, 4000.0, -1500.0): + now, old, fresh = case(kind, n, house) + own = kind in SETTLE + check(f"{'⭐' if kind == 'Off' else ' '} {kind}, N={n}, house " + f"{house:.0f} W: old readings add at most one slew step " + f"to the others' swing in the 23 s after the end" + f"{', and it is at most one slew step' if own else ''} " + f"({now['swing']:.0f} W; 1 s registers " + f"{fresh['swing']:.0f}; release/1.0 {old['swing']:.0f})", + now["swing"] - fresh["swing"] <= SLEW + and (not own or now["swing"] <= SLEW)) +_by = {(k, n, h): (a, b) for k, n, h, a, b, f in TABLE} +_now, _old = _by[("Off", 2, 1200.0)] +_b = [r["cmd"][1] for r in _now["rows"] if 0 <= r["t"] - _now["end"] <= 23] +_bo = [r["cmd"][1] for r in _old["rows"] if 0 <= r["t"] - _old["end"] <= 23] +print(f" rig check 5 (N=2, 1200 W, A Off): B {_b}\n" + f" release/1.0: B {_bo}") +check("⭐ the rig case (SAFETY-95 check 5): release/1.0 swings B by more " + "than a slew step on A's old reading; now B holds within 50 W", + _old["swing"] > SLEW and max(_b) - min(_b) <= 50.0) +for n in (2, 3): + check(f"non-vacuous (N={n}): on release/1.0 old readings add more than " + f"a slew step in at least one of these cases", + any(b["swing"] - f["swing"] > SLEW + for k, nn, h, a, b, f in TABLE if nn == n)) +_s5 = [] +for lag in (5.0, 20.0): + for reg in (5.0, 10.0): + for n in (2, 3): + _s5.append(measure(run("Off", n, 1200.0, lag=lag, reg_s=reg), n, + 1200.0)["swing"]) +check(f"no plant constant is assumed: battery lag 5 s and 20 s, registers " + f"every 5 s and 10 s, N=2 and N=3: largest swing {max(_s5):.0f} W", + max(_s5) <= SLEW) +_ph = [measure(run("Off", 2, 1200.0, phase={"a": a, "b": b}), 2, 1200.0) + ["swing"] for a, b in ((0.0, 0.5), (0.9, 0.1), (0.3, 0.3))] +check(f"other register phases: largest swing {max(_ph):.0f} W", + max(_ph) <= SLEW) + +# ============================================================================ # +print("\n2. SAFETY-95/96's gains stay") +for kind in ENDS: + for n in (2, 3): + now, old = _by[(kind, n, 1200.0)] + check(f"{kind}, N={n}, 1200 W: no jump on the end cycle (others " + f"<= the house, {now['first']:.0f} W) and the site moves at " + f"most one slew step a cycle after ({now['step']:.0f} W)", + now["first"] <= 1200.0 - AIM + 50.0 and now["step"] <= SLEW) + now, old = _by[(kind, n, -1500.0)] + check(f"{kind}, N={n}, solar surplus: no battery gives into it " + f"(largest command after the end {now['give']:.0f} W" + + (f"; a lost link as release/1.0, {old['give']:.0f} W)" + if kind == "stale link" else ")"), + now["give"] <= (max(50.0, old["give"]) if kind == "stale link" + else 50.0)) +_now, _old = _by[("Off", 2, 1200.0)] +check(f"the grid in the window stays inside release/1.0's range " + f"([{_now['gmin']:.0f}, {_now['gmax']:.0f}] W, release/1.0 " + f"[{_old['gmin']:.0f}, {_old['gmax']:.0f}])", + _now["gmin"] >= _old["gmin"] - 50.0 and _now["gmax"] <= _old["gmax"]) + + +_ref = {} +for name, f in (("now", lambda: p.run(2, 1200.0, **ENDS["refusal"])), + ("old", lambda: without(lambda: p.run( + 2, 1200.0, **ENDS["refusal"])))): + rows, c = f() + _ref[name] = (c.inverters[0].refused, c.inverters[0].refused_at) +check(f"a refusal is still learned, at the same instant as release/1.0 " + f"({_ref['now']})", _ref["now"] == _ref["old"] + and _ref["now"][0] == 1) +_st = {} +for name, f in (("now", lambda: p.run(2, 1200.0, **ENDS["stale link"])), + ("old", lambda: without(lambda: p.run( + 2, 1200.0, **ENDS["stale link"])))): + rows, c = f() + _st[name] = sorted(c.live_units()) +check(f"a stale link is still dropped, as in release/1.0 " + f"(live: {_st['now']})", _st["now"] == _st["old"] + and len(_st["now"]) == 1) + +# ============================================================================ # +print("\n3. a house step inside the window is still followed") + + +def step(sign): + return lambda t: 1200.0 + (sign * 2000.0 if t >= T + 5.0 else 0.0) + + +for sign, what in ((1, "kettle +2000 W"), (-1, "solar -2000 W")): + for n in (2, 3): + rows = p.run(n, step(sign))[0] + rows_o = without(lambda: p.run(n, step(sign)))[0] + te = end_of(rows) + + def wh(rs): + return sum(abs(r["grid"] - AIM) for r in rs + if te + 5.0 <= r["t"] <= te + 35.0) / 3600.0 + sw = measure(rows, n, 1200.0)["swing"] + check(f"{what} at E+5 s, N={n}: the grid's distance from its aim " + f"over the next 30 s is within 1.5 Wh of release/1.0's " + f"({wh(rows):.1f} Wh, release/1.0 {wh(rows_o):.1f}); swing " + f"{sw:.0f} W", wh(rows) <= wh(rows_o) + 1.5 and sw <= SLEW) + +# ============================================================================ # +print("\n4. fresh readings and N=1: release/1.0's rows") + + +def digest(rows): + return hashlib.sha256(repr(rows).encode()).hexdigest() + + +_fresh = [(n, h) for n in (2, 3) for h in (1200.0, 4000.0, -1500.0)] +check("⭐ registers as fresh as the meter (1 s), N=2 and N=3, three houses: " + "every row byte-identical with release/1.0 (a reading that changed " + "this cycle is taken as it is)", + [digest(p.run(n, h, reg_s=1.0)[0]) for n, h in _fresh] + == [digest(without(lambda: p.run(n, h, reg_s=1.0))[0]) + for n, h in _fresh]) +_rf = [(n, h) for n in (2, 3) for h in (1200.0, -1500.0)] +check("⭐ a refusal end, 10 s registers, N=2 and N=3, 1200 W and a surplus: " + "every row byte-identical with release/1.0 (A reads ~0 W and will not " + "give, so its side is its reading)", + [digest(run("refusal", n, h)) for n, h in _rf] + == [digest(without(lambda: run("refusal", n, h))) for n, h in _rf]) +_n1 = [dict(), dict(before=None, soc=soc_done)] +check("⭐ N=1, A switched Off and A's drain completing, 10 s registers: " + "every row byte-identical with release/1.0", + [digest(p.run(1, 1200.0, **k)[0]) for k in _n1] + == [digest(without(lambda: p.run(1, 1200.0, **k))[0]) for k in _n1]) + +# ============================================================================ # +print("\n5. the rule's parts") +_c = p.run(2, 1200.0, dur=4.0)[1] +AB = {"inverter-a", "inverter-b"} +ia, ib = _c.inverters + + +def at(now, batts, cmds, changed): + """Set the clock, each battery's reading, its standing command and the + instant its reading last changed; then track.""" + _c._now_mono = now + for inv, b, cmd, ch in zip(_c.inverters, batts, cmds, changed): + inv.batt, inv.last_write = b, cmd + inv.batt_fresh.changed_mono = ch + _c._track_reads() + + +_c._read_span = {} +at(10.0, (2000.0, -100.0), (2000.0, 0.0), (10.0, 10.0)) +check("a reading that changed is the start of its range: (R, R) joined " + "with the command on its wire", + _c._read_span[A] == (2000.0, 2000.0, 10.0) + and _c._read_span["inverter-b"] == (-100.0, 0.0, 10.0)) +at(11.0, (2000.0, -100.0), (0.0, 1140.0), (10.0, 10.0)) +at(12.0, (2000.0, -100.0), (0.0, 800.0), (10.0, 10.0)) +check("every command since the reading widens the range (a lagging " + "battery moves toward each in turn): A [0, 2000], B [-100, 1140]", + _c._read_span[A][:2] == (0.0, 2000.0) + and _c._read_span["inverter-b"][:2] == (-100.0, 1140.0)) +at(13.0, (1241.0, -100.0), (0.0, 800.0), (13.0, 10.0)) +check("a new reading starts the range again: A [0, 1241]", + _c._read_span[A][:2] == (0.0, 1241.0)) +_c._read_span = {} +_c.inverters = _c.inverters[:1] +at(14.0, (2000.0,), (2000.0,), (14.0,)) +_c.inverters = [ia, ib] +check("N=1: nothing is tracked", _c._read_span == {}) + + +def tail(now, grid, target, held=None): + """`_drain_grid_w` with A settling: the error it hands the loop (grid + minus aim) and the house point it holds.""" + _c._now_mono, _c.grid, _c.target = now, grid, target + _c._drain_tail, _c._settle = {A}, {A: 0.0} + _c._drain_house = held + g = _c._drain_grid_w(AB) + return round(g - AIM, 6), _c._drain_house + + +# A: reading 1241 W from 13 s, range [0, 1241]; B: reading -100 W from 10 s, +# range [-100, 1140]. Grid -330 W: the house is in +# [-330 + 10 + 0 - 100, -330 + 10 + 1241 + 1140] = [-420, 2061]. +_c._read_span = {A: (0.0, 1241.0, 13.0), "inverter-b": (-100.0, 1140.0, 10.0)} +ia.batt, ib.batt = 1241.0, -100.0 +ia.batt_fresh.changed_mono, ib.batt_fresh.changed_mono = 13.0, 10.0 +check("⭐ held: the first window cycle holds the loop's start (1140 W) while " + "the meter allows it (error 0; release/1.0's would be -330 + 1241 - " + "100 + 10 - 1140 = -319 W)", tail(14.0, -330.0, 1140.0) == (0.0, 1140.0) + and round(_release(_c, AB) - AIM) == -319) +check("⭐ held, then kept: a later cycle keeps the held point, not the " + "loop's target", tail(15.0, -330.0, 1000.0, held=1140.0) + == (140.0, 1140.0)) +check("⭐ moved as far as the meter proves: a grid of +2000 W puts the " + "house at least at 2000 + 10 + 0 - 100 = 1910 W", + tail(15.0, 2000.0, 1140.0, held=1140.0) == (770.0, 1910.0)) +check("...and down: a grid of -2500 W puts it at most at -2500 + 10 + 1241 " + "+ 1140 = -109 W", tail(15.0, -2500.0, 1140.0, held=1140.0) + == (-1249.0, -109.0)) +ia.batt_fresh.changed_mono = 15.0 +check("⭐ a reading that changed this cycle is taken as it is: A at 1241 W, " + "not [0, 1241], so a grid of +2000 W puts the house at least at " + "2000 + 10 + 1241 - 100 = 3151 W", tail(15.0, 2000.0, 1140.0, + held=1140.0) == (2011.0, 3151.0)) +ia.batt_fresh.changed_mono = 13.0 +# A refuses to give (SAFETY-73): it reads 8 W, though it was asked 2000 W +# since that reading. Grid -1500 W: the house is at most -1500 + 10 + 8 + +# 1140 = -342 W, not -1500 + 10 + 2000 + 1140 = 1650 W. +_c._read_span[A] = (8.0, 2000.0, 13.0) +ia.batt = 8.0 +ia.refused = 1 +_ref1 = tail(15.0, -1500.0, 1140.0, held=1140.0) +ia.refused = -1 +_c._read_span[A] = (-2000.0, 8.0, 13.0) +_ref2 = tail(15.0, 1000.0, -500.0, held=-500.0) +ia.refused = 0 +check("⭐ a battery that refuses one way is taken at its reading on that " + "side: will not give, asked 2000 W, reads 8 W -> the house at most " + "-342 W; will not take, asked -2000 W -> at least 1000 + 10 + 8 - 100 " + "= 918 W", _ref1 == (-1482.0, -342.0) and _ref2 == (1418.0, 918.0)) +_c._read_span[A], ia.batt = (0.0, 1241.0, 10.0), 1241.0 +check("a range from an older reading than the one in hand (a cycle the " + "loop did not run) is not used: A at its reading, 1241 W", + tail(15.0, 2000.0, 1140.0, held=1140.0) == (2011.0, 3151.0)) +_c._read_span[A] = (0.0, 1241.0, 13.0) +_c._drain_tail, _c._settle, _c._drain_house = {A}, {}, 1140.0 +_c.grid = -330.0 +check("⭐ the tail's end lets the hold go: the meter again, and the next " + "tail starts from the loop's start", + _c._drain_grid_w(AB) == -330.0 and _c._drain_house is None) + +# ============================================================================ # +print("\n drain end | N | house W | swing W now/release/1 s registers | " + "end-cycle others W now/release | site step W now/release | grid in " + "window W now | release") +for k, n, h, a, b, f in TABLE: + print(f" {k} | {n} | {h:.0f} | {a['swing']:.0f}/{b['swing']:.0f}/" + f"{f['swing']:.0f} | " + f"{a['first']:.0f}/{b['first']:.0f} | {a['step']:.0f}/" + f"{b['step']:.0f} | [{a['gmin']:.0f}, {a['gmax']:.0f}] | " + f"[{b['gmin']:.0f}, {b['gmax']:.0f}]") +print() +if fails: + print(f"{len(fails)} FAILED: " + ", ".join(fails)) + sys.exit(1) +print("all SAFETY-103 checks passed") diff --git a/goodwe_controller/tools/safety103_plant.py b/goodwe_controller/tools/safety103_plant.py new file mode 100644 index 0000000..122f939 --- /dev/null +++ b/goodwe_controller/tools/safety103_plant.py @@ -0,0 +1,195 @@ +"""SAFETY-103's plant: test_safety95's lagging battery with the SAFETY-90 +register model, driven through the REAL Controller in simulated time. + +Each battery ramps at 800 W/s, then follows a first-order lag (`lag` s). Its +power register publishes every `reg_s` s at its own phase (the SAFETY-90/96 +rig: A at 0.6, B at 0.16 of the period), and reads `off` W from the truth +(B idles at -100 W). The meter reads every cycle (`tick` s). A drain on A +runs from the first cycle (the others finished theirs); `before(c, t)` ends +it. `run()` has no side effects on import, so tests and probes share it. +""" + +import asyncio +import math +import os +import random +import sys +import tempfile +from datetime import timedelta + +HERE = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +sys.path.insert(0, HERE) +sys.path.insert(0, os.path.join(HERE, "tools")) + +import opt11_identity as rig # noqa: E402 +from app import main as app_main, packmode # noqa: E402 +from app.main import Controller # noqa: E402 +from app.store import unit_key # noqa: E402 + +A = "inverter-a" +T = 300.0 # A's drain ends here +PHASE = {"a": 0.6, "b": 0.16, "c": 0.38} +OFF = {"b": -100.0} # the rig's B: idle reading -100 W +M = {"maintenance_enabled": True, "maintenance_start_hour": 11, + "battery_modes_enabled": True} + + +def options(n, extra=None): + o = rig._options(n, dict(M, **(extra or {}))) + if n > 1: + o["inverters"] = [ + {"name": f"inverter-{u}", "setpoint_entity": f"number.sp_{u}", + "soc_entity": f"sensor.soc_{u}", "batt_entity": f"sensor.batt_{u}", + "max_w": 2500, "bms_soc_floor": 10} for u in "abc"[:n]] + return o + + +def store(n): + """A due for its monthly cycle (it drains from the first cycle).""" + s = rig._Store() + for u in "bc"[:n - 1]: + s.data[unit_key(f"inverter-{u}", "last_completed")] = \ + rig.BASE.isoformat() + return s + + +def off(c, t): + """A switched Off at T (the SAFETY-95 check 5 end).""" + if t >= T: + c.store.data[unit_key(A, packmode.USE_KEY)] = False + + +def run(n, house, *, before=off, dur=T + 90.0, tick=1.0, reg_s=10.0, + lag=10.0, noise=50.0, seed=103, phase=None, offs=None, extra=None, + st=None, soc=None, dead=None, reject=None): + """Rows of one run. Each row is a dict: t, cmd (the add-on's commands), + target, grid (true), true (each battery's power), read (each register), + age (s since each register published), phase (maintenance phases). + `dead` (u, from s): u gives nothing from then on (board at 0 W). + `reject` {u: from s}: writes to u are refused from then on.""" + units = tuple("abc"[:n]) + phase = PHASE if phase is None else phase + offs = OFF if offs is None else offs + rnd = random.Random(seed) + soc = soc or (lambda u, t: 50.0) + + class Hass(rig._Hass): + def __init__(self, *a): + super().__init__(*a) + self.tgt = {u: 0.0 for u in units} + self.act = {u: 0.0 for u in units} + self.reg = {u: 0.0 for u in units} + self.pub_t = {u: 0.0 for u in units} + self.slot = {u: None for u in units} + self.tl = 0.0 + self.hn = 0.0 + + def advance(self, t): + for k in range(int(round((t - self.tl) / 0.1))): + now = self.tl + (k + 1) * 0.1 + for u in units: + c = self.cmd[u] + if dead is not None and u == dead[0] and now >= dead[1]: + c = 0.0 + d = c - self.tgt[u] + self.tgt[u] += math.copysign(min(abs(d), 80.0), d) + self.act[u] += (self.tgt[u] - self.act[u]) * ( + 1 - math.exp(-0.1 / lag)) + s = math.floor((now - phase.get(u, 0.0) * reg_s) / reg_s) + if s != self.slot[u]: + self.slot[u] = s + self.reg[u] = float(round(self.act[u] + offs.get(u, 0.0))) + self.pub_t[u] = now + self.tl = t + self.hn = rnd.uniform(-noise, noise) if noise else 0.0 + + def power(self, u): + return self.reg[u] + + def grid(self): + h = house(self.t) if callable(house) else house + return float(round(h + self.hn - sum(self.act.values()))) + + async def set_number(self, e, v): + u = e[-1] + if reject and u in reject and self.t >= reject[u]: + return False + self.cmd[u] = float(v) + return True + + opts = options(n, extra) + db = os.path.join(tempfile.mkdtemp(prefix="s103-"), "tariff.db") + + async def go(): + hass = Hass(units, soc, lambda u, t: False, lambda t: 0.0) + c = Controller(opts, hass, st or store(n), rig._Pub(), + stop=asyncio.Event(), peak_db_path=db) + for i in c.inverters: + i.dev_min, i.dev_max = -5000.0, 5000.0 + c.auto = True + real_dt, clock = app_main.datetime, [0.0] + + class _DT(real_dt): + @classmethod + def now(cls, tz=None): + return rig.BASE + timedelta(seconds=clock[0]) + app_main.datetime = _DT + rows = [] + try: + t = 0.0 + while t < dur: + hass.advance(t) + hass.t = clock[0] = t + for inv in c.inverters: + u = inv.name[-1] + if reject and u in reject and t >= reject[u] \ + and inv.rejected_since is None: + inv.rejected_since = rig.BASE + timedelta(seconds=t) + if before is not None: + r = before(c, t) + if asyncio.iscoroutine(r): + await r + await c.read_inputs(now_mono=t) + await c.cycle() + rows.append(dict( + t=t, cmd=tuple(i.target for i in c.inverters), + target=c.target, grid=hass.grid(), + true=tuple(round(hass.act[u]) for u in units), + read=tuple(hass.reg[u] for u in units), + age=tuple(round(t - hass.pub_t[u], 1) for u in units), + phase=tuple(m.phase for m in c.maints), + reason=c.loop_reason)) + t += tick + finally: + app_main.datetime = real_dt + return rows, c + return asyncio.run(go()) + + +def end_at(rows): + """The first cycle A's command left its drain (None if it never did).""" + return next((r["t"] for r in rows if r["t"] >= T + and r["cmd"][0] < 1990.0), None) + + +def window(rows, span=23.0): + """The rows from the end cycle to `span` s after it (the settle window, + PLANT_SETTLE_DOWN_S).""" + te = end_at(rows) + return [r for r in rows if te is not None and te <= r["t"] <= te + span] + + +def swing(rows, i=1, span=23.0): + """Battery i's command in the settle window after the end cycle, and + its largest swing there: a move that is later undone (up then down, or + down then up), the smaller of its two legs. A steady climb scores 0.""" + w = [r["cmd"][i] for r in window(rows, span)][1:] + best = 0.0 + for j in range(len(w)): + lo_b = min(w[:j + 1]) + hi_b = max(w[:j + 1]) + lo_a = min(w[j:]) + hi_a = max(w[j:]) + best = max(best, min(w[j] - lo_b, w[j] - lo_a), + min(hi_b - w[j], hi_a - w[j])) + return best, w -- 2.54.0 From 98dd110a4ff77ba0b6dea849bf4d65f1cc6226a3 Mon Sep 17 00:00:00 2001 From: admin Date: Thu, 8 Oct 2026 03:12:03 +0200 Subject: [PATCH 02/17] SAFETY-103: test a dropped battery is not counted; guard a read Mutation M13 (a battery out of the split counted in the house point) survived; a direct check now kills it. win[0] is read through a guard (test_named_absence). Co-Authored-By: Claude Opus 5.5 --- goodwe_controller/test_safety103.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/goodwe_controller/test_safety103.py b/goodwe_controller/test_safety103.py index f4729d3..1893445 100644 --- a/goodwe_controller/test_safety103.py +++ b/goodwe_controller/test_safety103.py @@ -118,7 +118,7 @@ def measure(rows, n, house): seg = [r["target"] for r in rows if r["t"] >= te] return dict( end=te, swing=sw, - first=max(win[0]["cmd"][1:]), + first=max(win[0]["cmd"][1:]) if win else math.nan, step=max(abs(b - a) for a, b in zip(seg, seg[1:])), give=max(max(r["cmd"][1:]) for r in rows if r["t"] >= te), gmin=min(r["grid"] for r in win), gmax=max(r["grid"] for r in win), @@ -319,13 +319,13 @@ _c.inverters = [ia, ib] check("N=1: nothing is tracked", _c._read_span == {}) -def tail(now, grid, target, held=None): +def tail(now, grid, target, held=None, live=AB): """`_drain_grid_w` with A settling: the error it hands the loop (grid minus aim) and the house point it holds.""" _c._now_mono, _c.grid, _c.target = now, grid, target _c._drain_tail, _c._settle = {A}, {A: 0.0} _c._drain_house = held - g = _c._drain_grid_w(AB) + g = _c._drain_grid_w(live) return round(g - AIM, 6), _c._drain_house @@ -374,6 +374,10 @@ check("a range from an older reading than the one in hand (a cycle the " "loop did not run) is not used: A at its reading, 1241 W", tail(15.0, 2000.0, 1140.0, held=1140.0) == (2011.0, 3151.0)) _c._read_span[A] = (0.0, 1241.0, 13.0) +check("⭐ a battery out of the split is not counted (the meter has its " + "power): B dropped, a grid of +2000 W puts the house at least at " + "2000 + 10 + 0 = 2010 W, not 1910 W", + tail(15.0, 2000.0, 1140.0, held=1140.0, live={A}) == (870.0, 2010.0)) _c._drain_tail, _c._settle, _c._drain_house = {A}, {}, 1140.0 _c.grid = -330.0 check("⭐ the tail's end lets the hold go: the meter again, and the next " -- 2.54.0 From c6f23a69d44d211c1643a09d0d3a8289bc527029 Mon Sep 17 00:00:00 2001 From: SAFETY-103 review Date: Thu, 8 Oct 2026 04:13:08 +0200 Subject: [PATCH 03/17] SAFETY-103 review: correct the swing after a refusal or lost link in DECISIONS The test table gives 1.5 kW at N=2 and 3.9 kW summed at N=3 under a surplus, and about 1 kW after a lost link without one; not 1.9 kW. Co-Authored-By: Claude Opus 5.5 --- .../DECISIONS/SAFETY-103-old-readings-after-a-drain.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/goodwe_controller/DECISIONS/SAFETY-103-old-readings-after-a-drain.md b/goodwe_controller/DECISIONS/SAFETY-103-old-readings-after-a-drain.md index 8f2adc7..eb271fc 100644 --- a/goodwe_controller/DECISIONS/SAFETY-103-old-readings-after-a-drain.md +++ b/goodwe_controller/DECISIONS/SAFETY-103-old-readings-after-a-drain.md @@ -70,8 +70,9 @@ still swing up to 130 W, a third of a slew step. the saturation freeze (SAFETY-96), unchanged. - A lost link or a refusal ends the tail at once (A is out of the split, or reads ~0 W). The swing after them is the loop's own on a lagging plant, - the same with 1 s registers and on release/1.0 (up to 1.9 kW at N=3 under - a surplus): not this card. + the same with 1 s registers and on release/1.0: under a surplus up to + 1.5 kW at N=2 and 3.9 kW summed at N=3; after a lost link about 1 kW + even without one. Not this card. - A battery that overshoots its command, or a reading offset beyond the range, can put the true power outside the range; the meter then moves the point, as before. -- 2.54.0 From 28eff568b56a0b381d9072e2a849493e14bb90a1 Mon Sep 17 00:00:00 2001 From: glenn schrooyen Date: Thu, 8 Oct 2026 09:36:06 +0200 Subject: [PATCH 04/17] MAINT-13: a full pack that stops at 98-99 % is not a failed charge A charge is complete at SoC >= target, or at >= target - 3 once the SoC has made no new high for 60 min of allowed charging, or when the charging time runs out at >= target - 3. The 36 h timeout counts only the minutes the price gate let the pack charge (plus a 7 day wall-clock backstop). A cycle that fails (stuck drain, charge below target - 3) backs off 7 days, tells the operator once (EN + NL), and no cycle starts on that pack in that time unless the operator forces one. State survives a restart; old state files load. No option added. DECISIONS/MAINT-13-full-pack-is-complete.md. Co-Authored-By: Claude Opus 5.5 --- .../MAINT-13-full-pack-is-complete.md | 98 ++++ goodwe_controller/DOCS.md | 23 + goodwe_controller/app/main.py | 27 +- goodwe_controller/app/maintenance.py | 126 ++++- goodwe_controller/app/messages.py | 30 +- goodwe_controller/app/store.py | 3 +- goodwe_controller/test_maint13.py | 468 ++++++++++++++++++ goodwe_controller/test_maintenance.py | 13 +- goodwe_controller/test_safety93.py | 4 + 9 files changed, 776 insertions(+), 16 deletions(-) create mode 100644 goodwe_controller/DECISIONS/MAINT-13-full-pack-is-complete.md create mode 100644 goodwe_controller/test_maint13.py diff --git a/goodwe_controller/DECISIONS/MAINT-13-full-pack-is-complete.md b/goodwe_controller/DECISIONS/MAINT-13-full-pack-is-complete.md new file mode 100644 index 0000000..e28140a --- /dev/null +++ b/goodwe_controller/DECISIONS/MAINT-13-full-pack-is-complete.md @@ -0,0 +1,98 @@ +# MAINT-13 - a full pack that stops at 98-99 % is not a failed charge + +**Status: for review.** Tests: `test_maint13.py`. No option added; every number +is a constant in `app/maintenance.py`. + +**What happened (live, B).** The drain ended 2026-10-04 18:24 UTC and the charge +began. It ran in pieces behind the price gate, reached 99 % at 2026-10-06 04:00, +fell back to 98 % at 04:15 while the loop still asked about -1370 W, and never +reached `maintenance_soc_target` 100. At 06:24 (2160 min = 36 h of wall clock +after the charge began) it aborted "charge stuck at 98 %". `last_completed` was +not set, so B was still due and would have drained again from 98 % to 11 % at the +next start hour, with no notice. Three faults: a pack may never report 100 % +while the BMS balances; a failed cycle came back the next day; and the 36 h +counted hours the price gate kept the pack waiting. + +## 1. The charge is complete when + +1. SoC >= target (as before), or +2. SoC >= target - 3 (`CHARGE_NEAR_FULL_PCT`) and the SoC has not beaten its + best for 60 min (`CHARGE_STALL_MIN`) of allowed charging, or +3. the charging time (`CHARGE_TIMEOUT_MIN`, 2160 min) runs out at SoC >= target - 3. + +Then the usual HOLD follows and `last_completed` is set after it. Each of 2 and +3 logs one plain line ("the battery takes no more charge at 98 %, so the charge +counts as done", "charge time is up at 98 %, close enough to full ..."). + +**Why "SoC has not beaten its best", and not asked/delivered watts.** SoC is the +one signal the live record gives us (99 at 04:00, 98 at 04:15); the asked power +was -1370 W but no delivered figure was kept, so a watt threshold (X, Y) would be +invented. SoC also covers the case where power flows but nothing is stored. +**Why 3 points.** The card's band; at target 100 it is 97, and the live pack sat +at 98-99. **Why 60 min.** A judgement, not a measurement: we have no per-minute record of +the pack taking charge near full. The one live fact is 2 h 24 min (04:00 to +06:24) without a new high, so 60 min is well inside it and long enough that a +pack still creeping up (one new high within the hour) keeps resetting the wait. +On the live run the rule fires after 60 gate-open minutes past the 99 %: 05:00 +UTC at the earliest, before 06:24 if the gate was open. If the pack turns out to +step up more slowly than that, raise the constant. Falling back by one point +does not reset the wait; only a new best does. + +## 2. A failed cycle backs off 7 days + +A drain stuck 12 h or a charge that times out below target - 3 is a failure +(`FAIL_BACKOFF_DAYS` = 7). The machine goes IDLE as before, `last_failed` is +stored, and `wants_start()` is false until 7 days later. The operator's forced +start ignores it; a cycle that completes after the failure ends it. An operator +abort, an interlock stop and a switched-off pack are not failures. + +**Why 7 days, not the 28 day interval.** A judgement: a failure may have a +short cause (several dull days with no cheap hours, a pack switched off), and +waiting the full interval leaves the coulomb counter uncalibrated for a month +more. A week is long enough that a bad pack costs one drain a week, with one +notice a week, not one a day. **Cost.** A truly faulty pack still drains once a week until someone looks; +the notice says so each time. + +**Notice, once.** The tick that fails carries `failed`; the Controller sends one +persistent notification "Battery cycle did not finish" (EN, with the Dutch +below it; per pack at N=2). It is not repeated, and Home Assistant restarts +wipe it, so the status page shows a "Maintenance paused" row with the date for +as long as the wait lasts. The operator may also see the log lines. + +## 3. The timeout counts only allowed time + +`Controller` passes `gate_open = self.cheap` (the same flag that lets the +cheap-window charge floor apply). Only those ticks add to `open_min`; one tick +adds at most 5 min, so a gap (add-on down) adds nothing. The stall wait pauses +while the gate is closed. The accounting is written to the state file at most +every 5 min (`charge_track`); a restart loses at most that. A state file from +before this change, or with no record for the current charge, starts from the +wall-clock minutes so far, which is the old behaviour. + +**Backstop.** A site whose gate never opens (no tariff, no sun) would never time +out, so a charge also ends after 7 days of wall clock (`CHARGE_WALL_TIMEOUT_MIN`): +complete at >= target - 3, a failure below. + +**MAINT-12.** This is the smallest change that makes the timeout honest: one +boolean in, one counter. MAINT-12's block plan can replace the boolean (the +minutes it plans are the allowed ones) without touching the rest. Two things it +must keep: the back-off is in `wants_start()`, so any new entry point (such as +`price_forecast.begin_phase`, which is not wired today) must go through it or +check `backoff_until()`; and `gate_open` must stay "the pack is allowed to +charge now". The peak guard pausing the charge shaping is not seen as a closed +gate: that time still counts. + +## 4. What does not change, and what this does not cover + +- The drain, the HOLD, the held-battery rules and `arbitrate()` are unchanged. A + backed-off pack does not want a start, so at N=2 the other pack may take the + slot. One pack at a time at N=1 and N=2. +- `last_failed` is a maintenance key and moves with the others when a second + inverter is added (`MAINT_UNIT_KEYS`). The charge record is not moved; it + restarts from the wall-clock minutes. +- It does not find out why a pack stopped. A pack that stops at 96 % and never + gets near is still a failure. +- The Dutch text is only in the notification. The status row and the log lines + are English, like the rest of the status page. +- It does not change the target: a site that wants a different stop point sets + `maintenance_soc_target`. diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md index 8771c2a..6523d3f 100644 --- a/goodwe_controller/DOCS.md +++ b/goodwe_controller/DOCS.md @@ -1667,6 +1667,20 @@ hour saved. If you set an earlier hour to get an evening start in UTC (for example 17 for 19:00 in summer), set it to 19. The add-on never changes a saved value. *For installers:* DECISIONS/MAINT-10-local-start-hour.md. +**When the charge counts as done.** A battery may stop at 98 or 99 % while it +balances, so it need not reach `maintenance_soc_target`. The charge counts as +done when the battery is within 3 points of the target and has taken no more +charge for 60 minutes of charging time, or when the charging time (36 hours) runs +out within 3 points of the target. Only the minutes the cheap-hours gate let the +battery charge count. Then the usual hold follows. + +**When a cycle fails.** If the battery does not empty in 12 hours, or does not +get within 3 points of the target in 36 hours of charging time, the cycle +stops, you get the notification *Battery cycle did not finish*, and it is not +tried again for 7 days. The status page shows *Maintenance paused* with the +date. Starting a cycle by hand ignores the wait. *For installers:* +DECISIONS/MAINT-13-full-pack-is-complete.md. + ### Tariff (all optional) | option | meaning | @@ -3250,6 +3264,14 @@ reading is stuck. Control keeps running and keeps following the reading, so a stuck reading can raise this month's peak. Check the meter. If it is stuck, stop control on the add-on page. +### Battery cycle did not finish + +The monthly battery cycle stopped because the battery did not empty, or did not +fill up, in time. The battery is back under normal control. The add-on waits 7 +days before it tries again, so it does not empty the battery every night. To +try sooner, start a cycle on the add-on page. If it keeps happening, check that +the battery follows its commands and that the cheap hours leave time to charge. + ### For installers: notifications This part is for installers. The texts live in `app/messages.py` and follow @@ -3269,6 +3291,7 @@ default `info` hides it. | Inverters working against each other | alarm | `goodwe_circulating_power` | SAFETY-31 | | *name* is not delivering | alarm | `goodwe_not_delivering_` | SAFETY-42 | | Meter reading may be stuck | alarm | `goodwe_meter_frozen` | SAFETY-18 | +| Battery cycle did not finish | warning | `goodwe_maint_failed_` | MAINT-13 | **The Details link.** Home Assistant 2026.2 and newer show add-on pages at `/config/app//documentation`; 2026.5 removed the old diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index c255a73..3b8a60a 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -77,7 +77,7 @@ from .inverter import (LINK_STALE_S, UNIT_A, build_roster, roster_entries, roster_names) from .maintenance import ( CHARGE, DRAIN, HOLD, IDLE, POWER_MOVING, MaintConfig, Maintenance, - MaintResult, arbitrate as maint_arbitrate, site_zone, + FAIL_BACKOFF_DAYS, MaintResult, arbitrate as maint_arbitrate, site_zone, ) from .mqtt import (MODE_CONFIGS_KEY, MODE_LABELS, USE_OFF, USE_ON, MqttPublisher, announced_units, mode_configs, @@ -3923,10 +3923,24 @@ class Controller: if blocked is None: maint_results.append(MaintResult(IDLE, False, None, False, [])) continue - res = machine.tick(now, inv.soc, blocked_by=blocked) + # MAINT-13: the charge timeout counts only the minutes the price + # gate (`self.cheap`) let the pack charge. + res = machine.tick(now, inv.soc, blocked_by=blocked, + gate_open=bool(self.cheap)) for msg in res.events: - self.log_event(msg, hint=messages.hint("maintenance", - phase=machine.phase)) + self.log_event(msg, hint=messages.hint( + "maint_failed" if res.failed else "maintenance", + phase=machine.phase)) + if res.failed: + # Said once: only the tick that failed carries `res.failed`. + multi = len(self.inverters) > 1 + slug = (unit_slugs(self.units)[self.inverters.index(inv)] + if multi else "battery") + await self.hass.call( + "persistent_notification", "create", + messages.maint_failed( + res.failed, FAIL_BACKOFF_DAYS, + unit=inv.name if multi else "", slug=slug).data()) maint_results.append(res) # ⚠️ CARRIED TO THE STATUS PAGE, NOT LEFT IN THE 40-LINE ACTION LOG. The # deferral is edge-logged (it lasts as long as the other pack's cycle), @@ -7100,6 +7114,7 @@ class Controller: # date, unchanged. _now = datetime.now(timezone.utc) _dues = [m.next_due(_now) for m in self.maints] + _waits = [u for u in (m.backoff_until(_now) for m in self.maints) if u] next_due = None if any(d is None for d in _dues) else min(_dues) rows = [ ("Grid", w(self.grid)), @@ -7147,6 +7162,10 @@ class Controller: *((("Maintenance waiting", "; ".join(f"{n}: {why}" for n, why in self._maint_deferred)),) if self._maint_deferred else ()), + # MAINT-13: only while a failed cycle waits, so a site with nothing + # failed carries exactly the rows it carried before. + *((("Maintenance paused", messages.row_maint_waits( + min(_waits).strftime("%Y-%m-%d"))),) if _waits else ()), ("Maintenance due", "now" if next_due is None else next_due.strftime("%Y-%m-%d")), ("Cheap window", "yes" if self.cheap else "no"), ("Tariff", self.tariff_words), diff --git a/goodwe_controller/app/maintenance.py b/goodwe_controller/app/maintenance.py index 8710775..6883331 100644 --- a/goodwe_controller/app/maintenance.py +++ b/goodwe_controller/app/maintenance.py @@ -65,10 +65,26 @@ POWER_MOVING = (DRAIN, CHARGE, HOLD) # 12 h means the battery is not following commands - a fault, not a slow day. The charge phase deliberately waits for sun and for cheap hours, so # it is expected to span a night; 36 h still bounds it. DRAIN_TIMEOUT_MIN = 720 +# MAINT-13: minutes the pack was ALLOWED to charge (the price gate open), not +# wall-clock minutes since the charge began. See CHARGE_WALL_TIMEOUT_MIN. CHARGE_TIMEOUT_MIN = 2160 # After an abort, do not immediately re-trigger in the same start hour. RESTART_COOLDOWN_MIN = 90 +# MAINT-13 (DECISIONS/MAINT-13-full-pack-is-complete.md). Constants, not options. +# A pack that reaches target - 3 and then takes no more charge for this many +# minutes of allowed charging counts as full. +CHARGE_NEAR_FULL_PCT = 3 +CHARGE_STALL_MIN = 60 +# Wall-clock backstop for a charge whose gate never opens (no cheap hours, no sun). +CHARGE_WALL_TIMEOUT_MIN = 10080 +# A failed cycle (stuck drain or charge) is not retried before this many days. +FAIL_BACKOFF_DAYS = 7 +# One tick counts at most this many minutes: a gap means the add-on was down. +CHARGE_TICK_MAX_MIN = 5 +# The charge accounting is written to the state file at most this often. +CHARGE_SAVE_MIN = 5 + # ⚠️ 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. # @@ -257,6 +273,9 @@ class MaintResult: # showing an idle machine and a due date in the past. Empty on every path # that is not deferring, so a roster of one never sees it. deferred: str = "" + # MAINT-13: "drain" or "charge" on the one tick where a cycle FAILED, so + # the Controller can tell the operator once. Empty on every other tick. + failed: str = "" class UnitHistory: @@ -344,6 +363,10 @@ class Maintenance: # per cycle would flush the very context that explains it. Same pattern # as `Controller._arb_action` and `_maint_shape_suspended`. self._deferred = "" + # MAINT-13: the charge accounting (see _charge_account), in memory and + # written to the state file every CHARGE_SAVE_MIN minutes. + self._trk = None + self._trk_saved = None # -- state --------------------------------------------------------------- @property @@ -351,6 +374,9 @@ class Maintenance: return self.store.get("phase", IDLE) def _enter(self, phase: str, now: datetime, events: list, msg: str) -> None: + self._trk = None + if self.store.get("charge_track") is not None: + self.store.set("charge_track", None) self.store.set("phase", phase) self.store.set_time("phase_started", now) _LOG.info("%s", msg) @@ -391,6 +417,21 @@ class Maintenance: return (last_try is None or (now - last_try) >= timedelta(minutes=RESTART_COOLDOWN_MIN)) + def backoff_until(self, now: datetime): + """MAINT-13: when a failed cycle may be tried again, or None. + + None when nothing failed, when the back-off is over, or when a cycle + completed after the failure (a forced start that worked ends it). + """ + failed = self.store.get_time("last_failed") + if failed is None: + return None + done = self.store.get_time("last_completed") + if done is not None and done >= failed: + return None + until = failed + timedelta(days=FAIL_BACKOFF_DAYS) + return until if now < until else None + def wants_start(self, now: datetime) -> bool: """Would this pack begin a cycle on this tick if the site let it? @@ -402,7 +443,8 @@ class Maintenance: return (self.phase == IDLE and self.cfg.enabled and in_start_slot(now, self.cfg.start_hour, self.tz) and self.due(now) - and self._cooled(now)) + and self._cooled(now) + and self.backoff_until(now) is None) # -- operations ---------------------------------------------------------- def force_start(self, now: datetime, blocked_by: str = "") -> list: @@ -430,6 +472,55 @@ 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: + """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 + gate-open minutes since the SoC last beat its best; a closed gate pauses + 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. + """ + since = self.store.get("phase_started") + trk = self._trk + if trk is None: + raw = self.store.get("charge_track") + if isinstance(raw, dict) and raw.get("since") == since: + trk = dict(raw) + else: + trk = {"since": since, "open_min": self._elapsed_min(now), + "flat_min": 0.0, "best": float(soc), "at": None} + self._trk = trk + at = trk.get("at") + try: + last = datetime.fromisoformat(at) if at else None + except (TypeError, ValueError): + last = None + 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"]: + trk["best"], trk["flat_min"] = float(soc), 0.0 + elif gate_open: + trk["flat_min"] += dt + if gate_open: + trk["open_min"] += dt + saved = self._trk_saved + if saved is None or (now - saved).total_seconds() >= CHARGE_SAVE_MIN * 60: + self.store.set("charge_track", dict(trk)) + self._trk_saved = now + return trk + + def _fail(self, now: datetime, kind: str, why: str) -> MaintResult: + """MAINT-13: stand down, remember the failure, and tell the Controller.""" + events = self.abort(now, why) + self.store.set_time("last_failed", now) + msg = (f"maintenance: the cycle did not finish. It waits " + f"{FAIL_BACKOFF_DAYS} days before it tries again.") + _LOG.warning("%s", msg) + events.append(msg) + return MaintResult(IDLE, True, 0.0, False, events, failed=kind) + def abort(self, now: datetime, why: str) -> list: events = [] self._enter(IDLE, now, events, f"maintenance: ABORT ({why})") @@ -457,13 +548,16 @@ class Maintenance: # -- the machine --------------------------------------------------------- def tick(self, now: datetime, soc: float | None, - blocked_by: str = "") -> MaintResult: + blocked_by: str = "", gate_open: bool = True) -> MaintResult: """One cycle of this pack's state machine. `blocked_by` is `arbitrate()`'s answer for THIS pack: empty when the site's one maintenance slot is available to it, otherwise the reason it is not. It gates two things and nothing else - a start, and a phase that is already moving power on a pack that does not hold the slot. + + `gate_open` (MAINT-13) is whether the pack may charge now (the price + gate); only those minutes count towards the charge timeout. """ events: list = [] phase = self.phase @@ -531,8 +625,8 @@ class Maintenance: f"after {elapsed:.0f} min -> charge") return MaintResult(CHARGE, False, None, True, events) if elapsed > DRAIN_TIMEOUT_MIN: - events += self.abort(now, f"drain stuck at {soc:.0f} % after {elapsed:.0f} min") - return MaintResult(IDLE, True, 0.0, False, events) + return self._fail(now, "drain", f"drain stuck at {soc:.0f} % " + f"after {elapsed:.0f} min") return MaintResult(DRAIN, True, self.cfg.discharge_w, False, events) if phase == CHARGE: @@ -541,9 +635,27 @@ class Maintenance: f"maintenance: charge complete at {soc:.0f} % after " f"{elapsed:.0f} min -> holding {self.cfg.hold_min} min to balance") return MaintResult(HOLD, True, 0.0, False, events) - if elapsed > CHARGE_TIMEOUT_MIN: - events += self.abort(now, f"charge stuck at {soc:.0f} % after {elapsed:.0f} min") - return MaintResult(IDLE, True, 0.0, False, events) + trk = self._charge_account(now, soc, gate_open) + near = soc >= self.cfg.soc_target - CHARGE_NEAR_FULL_PCT + # MAINT-13 rule 1: close to full and no longer taking charge. + if near and trk["flat_min"] >= CHARGE_STALL_MIN: + self._enter(HOLD, now, events, + f"maintenance: the battery takes no more charge at " + f"{soc:.0f} %, so the charge counts as done -> holding " + f"{self.cfg.hold_min} min to balance") + return MaintResult(HOLD, True, 0.0, False, events) + if (trk["open_min"] > CHARGE_TIMEOUT_MIN + or elapsed > CHARGE_WALL_TIMEOUT_MIN): + # MAINT-13 rule 2: out of time close to full is complete. + if near: + self._enter(HOLD, now, events, + f"maintenance: charge time is up at {soc:.0f} %, " + f"close enough to full, so the charge counts as " + f"done -> holding {self.cfg.hold_min} min to balance") + return MaintResult(HOLD, True, 0.0, False, events) + return self._fail(now, "charge", + f"charge stuck at {soc:.0f} % after " + f"{trk['open_min']:.0f} min of charging time") return MaintResult(CHARGE, False, None, True, events) if phase == HOLD: diff --git a/goodwe_controller/app/messages.py b/goodwe_controller/app/messages.py index 6ad9814..45e4a74 100644 --- a/goodwe_controller/app/messages.py +++ b/goodwe_controller/app/messages.py @@ -102,6 +102,7 @@ class Notice: title: str body: str section: str = "" # the DOCS.md heading, when it is not the title itself + body_nl: str = "" # MAINT-13: the same words in Dutch, shown under the English @property def docs(self) -> str: @@ -110,7 +111,9 @@ class Notice: def data(self) -> dict: """The `persistent_notification.create` payload.""" return {"notification_id": self.id, "title": self.title, - "message": f"{self.body}\n\n{details(self.docs)}"} + "message": f"{self.body}\n\n" + + (f"{self.body_nl}\n\n" if self.body_nl else "") + + details(self.docs)} # --- the catalogue ---------------------------------------------------------- @@ -124,6 +127,8 @@ T_CIRCULATING = "Inverters working against each other" T_NOT_DELIVERING = "{unit} is not delivering" S_NOT_DELIVERING = "An inverter is not delivering" T_METER_FROZEN = "Meter reading may be stuck" +T_MAINT_FAILED = "Battery cycle did not finish" +T_MAINT_FAILED_NL = "Batterijcyclus niet afgerond" ID_LOOP_BOUND = "goodwe_integrator_bound" ID_OUTPUT_CLAMP = "goodwe_output_clamp" @@ -132,6 +137,7 @@ ID_PEAK_RESERVE = "goodwe_peak_soc_floor" ID_CIRCULATING = "goodwe_circulating_power" ID_NOT_DELIVERING = "goodwe_not_delivering_{slug}" ID_METER_FROZEN = "goodwe_meter_frozen" +ID_MAINT_FAILED = "goodwe_maint_failed_{slug}" # Which limit cut a command (clamp.ClampResult.bound, plus the loop's own). # The setting is named by its label, "Inverter rating (W)". @@ -295,6 +301,24 @@ def meter_frozen(unchanged_s) -> Notice: f"stop control on the add-on page.") +def maint_failed(kind: str, days: int, unit: str = "", slug: str = "") -> Notice: + """MAINT-13: a monthly cycle stopped before it was done. Said once.""" + full = kind == "charge" + who = unit or "The battery" + who_nl = unit or "De batterij" + en = f"{who} did not {'fill up' if full else 'empty'}, so its cycle " \ + f"stopped and tries again in {days} days." + nl = f"{who_nl} raakte niet {'vol' if full else 'leeg'}, dus de cyclus is " \ + f"gestopt en probeert het over {days} dagen opnieuw." + return Notice(ID_MAINT_FAILED.format(slug=slug or "battery"), WARNING, + T_MAINT_FAILED, en, body_nl=f"{T_MAINT_FAILED_NL}. {nl}") + + +def row_maint_waits(until_text: str) -> str: + """MAINT-13: the status row while a failed cycle waits.""" + return f"Waits until {until_text} after a cycle that did not finish." + + # --- the status page (UX-08 phase 2b) ---------------------------------------- # The rows of the Commissioning list and the banner at the top of the add-on # page. `main.py` decides WHEN a row shows and its colour (ok / warn / bad); @@ -947,6 +971,7 @@ HINTS = { "not_delivering_cleared": "{unit}: not-delivering alarm cleared", "maintenance": "Battery maintenance: {phase}", "maint_paused": "Maintenance charging paused to stay under the peak limit", + "maint_failed": "The battery cycle did not finish, so it waits before trying again", "pack_on": "{unit}: battery switched on", "pack_off": "{unit}: battery switched off, it is asked for 0 W", "pack_mode": "{unit}: battery mode changed", @@ -1118,6 +1143,7 @@ def status_samples() -> list: ("row_battery_refuses", AMBER, row_battery_refuses("inverter-b", 1, True)), ("row_circulating_check", GREEN, row_circulating_check(0, 0, 60)), + ("row_maint_waits", AMBER, row_maint_waits("2026-10-13")), ] @@ -1148,6 +1174,8 @@ def samples() -> list: not_delivering("inverter-b", "inverter_b"), meter_frozen(600), meter_frozen(90), + maint_failed("charge", 7), + maint_failed("drain", 7, "inverter-b", "inverter_b"), ] diff --git a/goodwe_controller/app/store.py b/goodwe_controller/app/store.py index c716fd6..406975c 100644 --- a/goodwe_controller/app/store.py +++ b/goodwe_controller/app/store.py @@ -94,6 +94,7 @@ DEFAULTS = { "phase_started": None, "last_completed": None, "last_start_attempt": None, + "last_failed": None, "sign_tripped": False, "sign_reason": "", "circ_tripped": False, @@ -171,7 +172,7 @@ MAINT_SCHEMA_KEY = "maint_schema_version" # silently stop being migrated - the same property `UNIT_STORE_KEYS` gives the # SAFETY-02 pair. MAINT_UNIT_KEYS = ("phase", "phase_started", "last_completed", - "last_start_attempt") + "last_start_attempt", "last_failed") def unit_key(unit: str, key: str) -> str: diff --git a/goodwe_controller/test_maint13.py b/goodwe_controller/test_maint13.py new file mode 100644 index 0000000..0377498 --- /dev/null +++ b/goodwe_controller/test_maint13.py @@ -0,0 +1,468 @@ +"""MAINT-13: a full pack that stops at 98-99 % is not a failed charge. +`python3 test_maint13.py` + +Graded here: the stall rule, the near-full timeout rule, the 7 day back-off +after a failed cycle (and what ends it), the timeout counting only the minutes +the price gate was open, restart and old-state-file behaviour, the notification +(once), and the Controller wiring at one and two packs. +See DECISIONS/MAINT-13-full-pack-is-complete.md. + +Mutating what this file grades? Read MUTATION.md first (stale bytecode). +""" + +import asyncio +import json +import os +import re +import sys +import tempfile +from datetime import datetime, timedelta, timezone +from pathlib import Path + +from app import messages +from app.maintenance import ( + CHARGE, CHARGE_NEAR_FULL_PCT, CHARGE_STALL_MIN, CHARGE_TIMEOUT_MIN, + CHARGE_WALL_TIMEOUT_MIN, DRAIN, FAIL_BACKOFF_DAYS, HOLD, IDLE, + MaintConfig, Maintenance, arbitrate, +) +from app.main import Controller +from app.store import (DEFAULTS, MAINT_UNIT_KEYS, Store, migrate_maint_keys, + unit_key) + +HERE = Path(__file__).resolve().parent +UTC = timezone.utc +fails = [] + + +def check(name, cond): + print(f" {'ok ' if cond else 'FAIL'} {name}") + if not cond: + fails.append(name) + + +# The live site: target 100, shipped hold. Start hour 19 UTC for the schedule. +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) # B's drain ended (live) +MIN = timedelta(minutes=1) + + +class Mem: + """Store's surface, no disk; times kept as the real Store keeps them.""" + + 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_charge(store=None, cfg=CFG, at=T0): + """A pack that has just finished its drain and is charging at `at`.""" + m = Maintenance(cfg, store or Mem()) + m.force_start(at) + m.tick(at, 11.0) # drain complete -> charge + assert m.phase == CHARGE + return m + + +def run(m, start, minutes, soc, gate=True, step=1): + """Tick every `step` minutes for `minutes`; `soc` is a number or f(minute). + Returns (the last result, the time after the run, the first non-CHARGE result).""" + t, res, out = start, None, None + for i in range(0, minutes, step): + t = start + i * step * MIN + s = soc(i * step) if callable(soc) else soc + res = m.tick(t, s, gate_open=gate) + if out is None and res.phase != CHARGE: + out = (res, t) + break + return res, start + minutes * MIN, out + + +print("1. the stall rule: near full and no longer taking charge") +# Live record: 99 % at 04:00, back to 98 % at 04:15, never 100. +m = in_charge() +res, end, out = run(m, T0 + 60 * MIN, 400, lambda i: 99.0 if i < 15 else 98.0) +check("a pack that reaches 99 and falls back to 98 completes (HOLD) after " + f"{CHARGE_STALL_MIN} min without a new high", + out is not None and out[0].phase == HOLD) +check("...at 60 min after the last new high (the 99), not before", + out is not None and out[1] == T0 + 120 * MIN) +check("...with one plain log line", + out is not None and len(out[0].events) == 1 + and "takes no more charge at 98 %" in out[0].events[0]) +m = in_charge() +res, _, out = run(m, T0, 59, 98.0) +check("59 minutes flat is not enough", out is None and res.phase == CHARGE) +m = in_charge() +res, _, out = run(m, T0, 61, lambda i: 97.0) +check("target - 3 (97 of 100) counts as near full", out and out[0].phase == HOLD) +m = in_charge() +res, _, out = run(m, T0, 300, lambda i: 96.0) +check("target - 4 (96 of 100) does not complete however long it is flat", + out is None) +cfg99 = MaintConfig(enabled=True, start_hour=19, soc_target=99, hold_min=120) +m = in_charge(cfg=cfg99) +res, _, out = run(m, T0, 61, lambda i: 96.0) +check("the shipped target 99 works the same way (96 is near full)", + out and out[0].phase == HOLD) +m = in_charge() +res, _, out = run(m, T0, 400, lambda i: min(99.0, 94.0 + i // 30)) +check("a pack still rising 1 point per 30 min is not stalled: it completes 60 " + "min after the 99 at minute 150", out and out[1] == T0 + 210 * MIN) +m = in_charge() +res, _, out = run(m, T0, 130, lambda i: 97.0 if i < 50 else (98.0 if i < 100 else 98.0)) +check("a new high resets the wait: 97 flat for 50 min, 98 at 50 min -> " + "completes at 110, not 60", out and out[1] == T0 + 110 * MIN) +m = in_charge() +res, _, out = run(m, T0, 300, lambda i: 97.0, gate=False) +check("minutes with the gate closed do not count as 'no more charge'", + out is None) +m = in_charge() +_, t, _ = run(m, T0, 30, 98.0) +_, t, _ = run(m, t, 120, 98.0, gate=False) +res, _, out = run(m, t, 31, 98.0) +check("30 open + 120 closed + 31 open minutes complete (the closed ones " + "pause the count)", out and out[0].phase == HOLD) + +print("2. SoC at the target completes at once") +m = in_charge() +r = m.tick(T0 + MIN, 100.0) +check("100 of 100 -> HOLD on that tick", r.phase == HOLD) +check("...with the old log line", "charge complete at 100 %" in r.events[0]) +m = in_charge(cfg=cfg99) +check("99 of 99 -> HOLD", m.tick(T0 + MIN, 99.0).phase == HOLD) + +print("3. a charge that runs out of time") +m = in_charge() +res, _, out = run(m, T0, CHARGE_TIMEOUT_MIN + 10, + lambda i: 60.0 + i / 55.0, step=1) +check("still rising 1 point per 55 min (never 60 flat) and near full when the " + "time is up -> counts as complete", out and out[0].phase == HOLD) +check("...with one plain line saying so", + out and len(out[0].events) == 1 and "charge time is up" in out[0].events[0] + and "counts as done" in out[0].events[0]) +check("...and no failure is recorded", m.store.get_time("last_failed") is None) +m = in_charge() +res, _, out = run(m, T0, CHARGE_TIMEOUT_MIN + 10, 70.0) +check("a pack stuck at 70 % fails when the time is up", + out and out[0].phase == IDLE and out[0].failed == "charge") +check("...once: the failure is on one tick, and the pack is idle after", + m.phase == IDLE and m.tick(out[1] + MIN, 70.0).failed == "") +check("...with the old technical line and one plain line", + len(out[0].events) == 2 and "ABORT" in out[0].events[0] + and "did not finish" in out[0].events[1] + and f"{FAIL_BACKOFF_DAYS} days" in out[0].events[1]) +check("...and last_failed is set, last_completed is not", + m.store.get_time("last_failed") == out[1] + and m.store.get_time("last_completed") is None) + +print("4. the back-off after a failed cycle") +fail_at = out[1] +day = lambda n: fail_at.replace(hour=19, minute=0) + timedelta(days=n) +check("the next day at the start hour: no cycle (it would begin with a drain)", + not m.wants_start(day(1)) and m.tick(day(1), 70.0).phase == IDLE) +check("every day for 6 days: still no cycle", + all(not m.wants_start(day(n)) for n in range(1, 7))) +r = [m.tick(day(n), 70.0) for n in range(1, 7)] +check("...and tick() keeps the pack idle and silent", + all(x.phase == IDLE and not x.events and x.failed == "" for x in r)) +until = m.backoff_until(day(1)) +check("backoff_until() names the date, 7 days after the failure", + until == fail_at + timedelta(days=FAIL_BACKOFF_DAYS)) +check("...it holds one minute before and is over at that minute", + m.backoff_until(until - MIN) == until and m.backoff_until(until) is None) +check("at 7 days, in the start hour, the pack wants to start again", + m.backoff_until(until) is None + and m.wants_start(until.replace(hour=19, minute=5))) +check("a forced start ignores the back-off", + m.force_start(day(1)) and m.phase == DRAIN) +m2 = in_charge() +run(m2, T0, CHARGE_TIMEOUT_MIN + 10, 70.0) +m2.force_start(day(2)) +m2.tick(day(2) + MIN, 11.0) +res, _, out2 = run(m2, day(2) + 2 * MIN, 400, 100.0) +check("a forced cycle that completes ends the back-off", + out2 and out2[0].phase == HOLD) +m2.tick(out2[1] + 130 * MIN, 100.0) +check("...(the hold completes, last_completed is set)", + m2.store.get_time("last_completed") is not None + and m2.backoff_until(day(3)) is None) +m3 = in_charge() +m3.store.set_time("last_failed", T0) +m3.abort(T0 + MIN, "operator") +check("an operator abort is not a failure and starts no back-off", + m3.store.get_time("last_failed") == T0 + and in_charge().store.get_time("last_failed") is None) +m4 = Maintenance(CFG, Mem()) +m4.force_start(T0) +r = m4.tick(T0 + timedelta(minutes=721), 40.0) +check("a drain that times out is a failure too, with the same back-off", + r.failed == "drain" and m4.store.get_time("last_failed") is not None + and not m4.wants_start(day(1))) + +print("5. only the minutes the pack was allowed to charge count") +m = in_charge() +res, t, out = run(m, T0, CHARGE_TIMEOUT_MIN * 2, 70.0, gate=False) +check("72 h with the gate closed: not timed out", out is None and m.phase == CHARGE) +res, t, out = run(m, t, CHARGE_TIMEOUT_MIN - 5, 70.0) +check("...and 35 h 55 min open on top: still charging", out is None) +res, t, out = run(m, t, 10, 70.0) +check("...36 h of open time in all: the charge fails", out and out[0].failed == "charge") +m = in_charge() +res, t, out = run(m, T0, CHARGE_WALL_TIMEOUT_MIN + 5, 70.0, gate=False, step=5) +check("a gate that never opens still ends in a failure after 7 days", + out and out[0].failed == "charge") +m = in_charge() +res, t, out = run(m, T0, CHARGE_WALL_TIMEOUT_MIN + 5, 98.0, gate=False, step=5) +check("...or in a complete charge when the pack is near full", + out and out[0].phase == HOLD) +m = in_charge() +res, t, out = run(m, T0, 10, 70.0) +res = m.tick(t + 2 * timedelta(hours=1), 70.0) +trk = m._trk +check("a gap in the ticks (add-on down) counts for at most 5 minutes", + trk["open_min"] < 20, ) + +print("6. restarts and old state files") +with tempfile.TemporaryDirectory() as d: + path = os.path.join(d, "state.json") + st = Store(path) + m = Maintenance(CFG, st) + m.force_start(T0) + m.tick(T0, 11.0) + _, t, out = run(m, T0, 60, 70.0, gate=False) # 60 min waiting + _, t, out = run(m, t, 40, 70.0) # 40 min charging + open_before = m._trk["open_min"] + # a fresh process on the same file + m_b = Maintenance(CFG, Store(path)) + r = m_b.tick(t + MIN, 70.0) + check("a restart mid-charge keeps the open minutes (40 of 100 min; within " + "the 5 min save interval), not the 100 wall-clock minutes", + m_b.phase == CHARGE + and open_before - 6 <= m_b._trk["open_min"] <= open_before + 2 + and m_b._trk["open_min"] < 60) + _, t2, out = run(m_b, t + 2 * MIN, CHARGE_TIMEOUT_MIN - 30, 70.0) + check("...so the timeout comes after 36 h in all, not 36 h from the restart", + out is not None and out[0].failed == "charge") + m = Maintenance(CFG, Store(path)) + m.store.set_time("last_failed", T0) + m_c = Maintenance(CFG, Store(path)) + check("a restart mid back-off keeps it (last_failed is in the file)", + m_c.backoff_until(T0 + timedelta(days=3)) == T0 + timedelta(days=7) + and not m_c.wants_start((T0 + timedelta(days=3)).replace(hour=19))) + check("...and the state file is plain JSON", + isinstance(json.load(open(path)).get("last_failed"), str)) + old = {"phase": "charge", + "phase_started": (T0 - timedelta(hours=35)).isoformat(), + "last_completed": None, "last_start_attempt": None, + "auto": True} + path2 = os.path.join(d, "old.json") + json.dump(old, open(path2, "w")) + mo = Maintenance(CFG, Store(path2)) + check("a state file from before this change loads with no failure and no " + "charge record", mo.backoff_until(T0) is None + and mo.store.get("charge_track") is None and mo.phase == CHARGE) + r = mo.tick(T0, 70.0) + check("...a charge in progress counts the hours it already ran (35 h)", + r.phase == CHARGE and 34 * 60 < mo._trk["open_min"] < 36 * 60) + res, _, out = run(mo, T0 + MIN, 120, 70.0) + check("...so it fails when the 36 h are up, as before", + out and out[0].failed == "charge") + mi = Maintenance(CFG, Store(os.path.join(d, "none.json"))) + check("no state file: due, no back-off", mi.due(T0) and mi.backoff_until(T0) is None) + +print("7. two packs (N=2)") +a, b = Maintenance(CFG, Mem(), unit="inverter-a", name="inverter-a"), \ + Maintenance(CFG, Mem(), unit="inverter-b", name="inverter-b") +b.force_start(T0) +b.tick(T0, 11.0) +_, t, out = run(b, T0, CHARGE_TIMEOUT_MIN + 10, 70.0) +slot = datetime(2026, 10, 7, 19, 5, tzinfo=UTC) +check("B failed: B is backed off, A (due) is not", + out and out[0].failed == "charge" and not b.wants_start(slot) + and a.wants_start(slot)) +check("...so A takes the slot, and B is not named as waiting for anyone", + arbitrate([a, b], slot) == ["", ""]) +a.force_start(slot) +check("...and while A cycles, B is held as before", + "inverter-a" in arbitrate([a, b], slot + 3 * MIN)[1]) +check("B's failure is under B's own keys, not A's", + b.store.get_time("last_failed") is not None + and a.store.get_time("last_failed") is None) +mst = Mem({"last_failed": T0.isoformat(), "phase": "idle"}) +migrate_maint_keys(mst, "inverter-a", ["inverter-a", "inverter-b"]) +check("going from one pack to two moves the back-off to the first pack", + mst.data.get(unit_key("inverter-a", "last_failed")) == T0.isoformat() + and "last_failed" in MAINT_UNIT_KEYS) + + +print("8. the Controller: gate, notification, status row") + + +class Hass: + def __init__(self, numbers, limits): + self.numbers, self.limits_by, self.calls = dict(numbers), dict(limits), [] + + async def number(self, entity, invert=False): + v = self.numbers.get(entity) + return None if v is None else (-v if invert else v) + + async def limits(self, entity): + return self.limits_by.get(entity, (None, None, None)) + + async def set_number(self, entity, value): + return True + + async def call(self, domain, service, data): + self.calls.append((domain, service, data)) + return True + + +class Pub: + enabled = False + + def publish(self, *a, **k): + pass + + def close(self): + pass + + +OPTS1 = dict(setpoint_entity="number.sp_a", soc_entity="sensor.soc_a", + batt_entity="sensor.batt_a", meter_source="off", + meter_entity="sensor.grid", max_w=5000.0, gain=0.6, slew_w=6000, + deadband_w=15, target_grid_w=0, step_w=10, heartbeat_s=10, + stale_input_s=15, maintenance_discharge_w=2000, + maintenance_charge_w=2000, maintenance_enabled=True, + maintenance_soc_target=100, + price_now_entity="sensor.p_now", price_avg_entity="sensor.p_avg") +OPTS2 = {k: v for k, v in OPTS1.items() + if k not in ("setpoint_entity", "soc_entity", "batt_entity")} +OPTS2["inverters"] = [ + dict(name="inverter-a", setpoint_entity="number.sp_a", + soc_entity="sensor.soc_a", batt_entity="sensor.batt_a", max_w=5000.0), + dict(name="inverter-b", setpoint_entity="number.sp_b", + soc_entity="sensor.soc_b", batt_entity="sensor.batt_b", max_w=3000.0)] + + +def controller(opts, cheap, soc_a=70.0, soc_b=70.0): + p_now, p_avg = (0.05, 0.10) if cheap else (0.20, 0.10) + hass = Hass({"sensor.grid": 800.0, "sensor.soc_a": soc_a, + "sensor.batt_a": 0.0, "sensor.soc_b": soc_b, + "sensor.batt_b": 0.0, "sensor.p_now": p_now, + "sensor.p_avg": p_avg}, + {"number.sp_a": (-5000.0, 5000.0, 1.0), + "number.sp_b": (-3000.0, 3000.0, 1.0)}) + c = Controller(dict(opts), hass, Mem(), Pub(), stop=asyncio.Event()) + for inv in c.inverters: + inv.dev_min, inv.dev_max = -inv.max_w, inv.max_w + c.auto = True + return c, hass + + +def one_cycle(c): + async def go(): + await c.read_inputs() + await c.cycle() + asyncio.run(go()) + + +def put_in_charge(c, which, hours_ago, open_min): + now = datetime.now(UTC) + m = c.maints[which] + m.store.set("phase", CHARGE) + m.store.set_time("phase_started", now - timedelta(hours=hours_ago)) + m.store.set("charge_track", { + "since": m.store.get("phase_started"), "open_min": open_min, + "flat_min": 0.0, "best": 70.0, + "at": (now - timedelta(minutes=3)).isoformat()}) + return m + + +def notes(h): + return [x for x in h.calls if x[0] == "persistent_notification"] + + +for cheap in (True, False): + c, h = controller(OPTS1, cheap) + m = put_in_charge(c, 0, 5, 100.0) + one_cycle(c) + got = m._trk["open_min"] - 100.0 + check(f"N=1, price gate {'open' if cheap else 'closed'}: the Controller " + f"{'adds' if cheap else 'does not add'} the 3 minutes since the last " + f"tick to the charging time (+{got:.1f})", + (2.5 < got < 3.5) if cheap else abs(got) < 0.01) + +c, h = controller(OPTS1, True) +m = put_in_charge(c, 0, 40, CHARGE_TIMEOUT_MIN + 1) +one_cycle(c) +n = notes(h) +check("N=1: a charge past its time at 70 % ends in IDLE with ONE notification", + m.phase == IDLE and len(n) == 1 + and n[0][2]["notification_id"] == "goodwe_maint_failed_battery" + and n[0][2]["title"] == "Battery cycle did not finish") +one_cycle(c) +one_cycle(c) +check("...and later cycles send no second one, and start no drain", + len(notes(h)) == 1 and m.phase == IDLE) +check("...the status page says when it waits until", + any(k == "Maintenance paused" and "Waits until" in v + for k, v in c.status_payload()["rows"])) +c0, _h0 = controller(OPTS1, True) +check("...and has no such row when nothing failed", + not any(k == "Maintenance paused" for k, v in c0.status_payload()["rows"])) + +c, h = controller(OPTS2, True) +m = put_in_charge(c, 1, 40, CHARGE_TIMEOUT_MIN + 1) +one_cycle(c) +n = notes(h) +check("N=2: pack B fails, B alone is told, by name, under its own id", + c.maints[1].phase == IDLE and c.maints[0].phase == IDLE and len(n) == 1 + and n[0][2]["notification_id"] == "goodwe_maint_failed_inverter_b" + and "inverter-b" in n[0][2]["message"]) +check("...and its failure is on B's keys", + c.maints[1].store.get_time("last_failed") is not None + and c.maints[0].store.get_time("last_failed") is None) + +print("9. the words") +en, nl = messages.maint_failed("charge", 7), messages.maint_failed("drain", 7) +for n_ in (en, nl): + print(" " + n_.title + " | " + n_.body + " | " + n_.body_nl) +check("EN charge text", en.body == "The battery did not fill up, so its cycle " + "stopped and tries again in 7 days.") +check("EN drain text", nl.body == "The battery did not empty, so its cycle " + "stopped and tries again in 7 days.") +check("NL texts", en.body_nl == "Batterijcyclus niet afgerond. De batterij " + "raakte niet vol, dus de cyclus is gestopt en probeert het over 7 dagen " + "opnieuw." and nl.body_nl.endswith("raakte niet leeg, dus de cyclus is " + "gestopt en probeert het over 7 dagen opnieuw.")) +check("the notification carries EN, NL and the Details line, in that order", + re.fullmatch(r"The battery .*\n\nBatterijcyclus .*\n\nDetails: .*", + en.data()["message"], re.S) is not None) +check("the DOCS section and table row exist", + "### Battery cycle did not finish" in (HERE / "DOCS.md").read_text() + and "goodwe_maint_failed_" in (HERE / "DOCS.md").read_text()) +check("no new option: the numbers are constants", + (CHARGE_NEAR_FULL_PCT, CHARGE_STALL_MIN, FAIL_BACKOFF_DAYS) + == (3, 60, 7)) + +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 b090c36..f38a054 100644 --- a/goodwe_controller/test_maintenance.py +++ b/goodwe_controller/test_maintenance.py @@ -622,6 +622,8 @@ def flat_store(**over): "phase_started": "2026-09-03T22:00:00+00:00", "last_completed": "2026-08-07T12:00:00+00:00", "last_start_attempt": "2026-09-03T10:00:00+00:00", + # MAINT-13: a failed cycle on record, so that key's move is graded too. + "last_failed": "2026-09-01T10:00:00+00:00", }, **over)) @@ -929,10 +931,11 @@ print(f" ok={_rep8.ok} lost={len(_rep8.lost)} kept={len(_rep8.kept)} " f"misdirected={len(_rep8.misdirected)}; marker " f"{_m8.data.get(MAINT_SCHEMA_KEY, '<>')!r}") check("PRECONDITION: this fixture reaches the marker through the `lost` guard " - "and ONLY that one - two keys in `kept`, two in `lost`, `misdirected` " + "and ONLY that one - two keys in `kept`, three in `lost` (the three the " + "fixture records; MAINT-13 added `last_failed`), `misdirected` " "empty. If it ever stops having a non-empty `kept` the mutation below " "goes invisible again, so the shape is asserted rather than assumed", - _rep8.ran and len(_rep8.kept) == 2 and len(_rep8.lost) == 2 + _rep8.ran and len(_rep8.kept) == 2 and len(_rep8.lost) == 3 and not _rep8.misdirected and not _rep8.ok) check("⚠️ THE MARKER IS NOT STAMPED ON A MIGRATION THAT LOST A MONTH OF " "HISTORY, and the two keys that DID land do not buy it one. `rep.ok` is " @@ -1010,10 +1013,14 @@ check("...and nothing about a deferral, a slot or another pack appears in any " # pressure. _solo2 = Maintenance(cfg, MemStore()) _solo2.force_start(t0) +# MAINT-13: a drain that times out is a failed cycle, so one plain line follows +# the abort line (the abort line itself is unchanged). check("...and so is the abort, which is the string an operator reads under " "pressure", _solo2.tick(t0 + timedelta(hours=13), 60.0).events - == ["maintenance: ABORT (drain stuck at 60 % after 780 min)"]) + == ["maintenance: ABORT (drain stuck at 60 % after 780 min)", + "maintenance: the cycle did not finish. It waits 7 days before it " + "tries again."]) diff --git a/goodwe_controller/test_safety93.py b/goodwe_controller/test_safety93.py index f9599dd..fa0ddda 100644 --- a/goodwe_controller/test_safety93.py +++ b/goodwe_controller/test_safety93.py @@ -203,6 +203,10 @@ def timed_out(end): s.data[unit_key(A, "phase_started")] = ( rig.BASE + timedelta(seconds=end - CHARGE_TIMEOUT_MIN * 60 - 1) ).isoformat() + # MAINT-13: a timed-out charge now backs off 7 days instead of restarting + # at once, so a due B would start its own drain here. B has finished its + # cycle, as in b_done(), so only the charge end is graded. + s.data[unit_key("inverter-b", "last_completed")] = rig.BASE.isoformat() return s -- 2.54.0 From 6f05b2e4be0f6f462d1706909aca5fbdef50abc7 Mon Sep 17 00:00:00 2001 From: glenn schrooyen Date: Thu, 8 Oct 2026 10:29:46 +0200 Subject: [PATCH 05/17] MAINT-13 round 2: gate open when nothing holds the charge back; safety93 scenario restored - gate_open is true with no price gate set up, in a cheap window, or while the pack is taking charge; the stall rule and timeout now work on a no-tariff site. - reset the save timer on a phase change; exact 5 min gap-cap test. - test_safety93: original timeout scenario kept (B finished) plus a case where a due B takes the slot after A's failure. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC --- .../MAINT-13-full-pack-is-complete.md | 11 +++-- goodwe_controller/app/main.py | 26 ++++++++-- goodwe_controller/app/maintenance.py | 1 + goodwe_controller/test_maint13.py | 47 +++++++++++++++++-- goodwe_controller/test_safety93.py | 36 +++++++++++--- 5 files changed, 106 insertions(+), 15 deletions(-) diff --git a/goodwe_controller/DECISIONS/MAINT-13-full-pack-is-complete.md b/goodwe_controller/DECISIONS/MAINT-13-full-pack-is-complete.md index e28140a..b505b1e 100644 --- a/goodwe_controller/DECISIONS/MAINT-13-full-pack-is-complete.md +++ b/goodwe_controller/DECISIONS/MAINT-13-full-pack-is-complete.md @@ -61,15 +61,20 @@ as long as the wait lasts. The operator may also see the log lines. ## 3. The timeout counts only allowed time -`Controller` passes `gate_open = self.cheap` (the same flag that lets the -cheap-window charge floor apply). Only those ticks add to `open_min`; one tick +`Controller._charge_gate_open()` says whether the pack may charge now. It is +true when no price gate is set up (no price entities, no fixed tariff: nothing +holds the charge back, and `cheap` is always false there), in a cheap window, or +while the pack is really taking charge (battery power at or below -50 W, which is +sun outside cheap hours). It is false only when a gate exists, is shut, and the +pack is not charging; a pack asked for charge that delivers nothing then does not +count, which is the case the clock cannot judge. Only open ticks add to `open_min`; one tick adds at most 5 min, so a gap (add-on down) adds nothing. The stall wait pauses while the gate is closed. The accounting is written to the state file at most every 5 min (`charge_track`); a restart loses at most that. A state file from before this change, or with no record for the current charge, starts from the wall-clock minutes so far, which is the old behaviour. -**Backstop.** A site whose gate never opens (no tariff, no sun) would never time +**Backstop.** A gated site whose gate never opens (no cheap hours, no sun) would never time out, so a charge also ends after 7 days of wall clock (`CHARGE_WALL_TIMEOUT_MIN`): complete at >= target - 3, a failure below. diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index 3b8a60a..97985f1 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -3875,6 +3875,22 @@ class Controller: inv.refusal_note())] if n] + def _charge_gate_open(self, inv) -> bool: + """MAINT-13: may this pack charge now, for the maintenance timeout? + + True when no price gate is set up (no price entities, no fixed tariff: + nothing holds the charge back), in a cheap window, or while the pack is + really taking charge (sun outside cheap hours). Closed only when a gate + exists, it is shut, and the pack is not charging. + """ + o = self.o + gated = bool((o.get("price_now_entity") and o.get("price_avg_entity")) + or o.get("static_profile")) + if not gated or self.cheap: + return True + batt = inv.batt + return batt is not None and float(batt) <= -MAINT_CHARGING_W + async def cycle(self) -> None: """Gather claims, resolve precedence, write the winner. @@ -3923,10 +3939,10 @@ class Controller: if blocked is None: maint_results.append(MaintResult(IDLE, False, None, False, [])) continue - # MAINT-13: the charge timeout counts only the minutes the price - # gate (`self.cheap`) let the pack charge. + # MAINT-13: the charge timeout counts only the minutes the pack was + # allowed to charge (see `_charge_gate_open`). res = machine.tick(now, inv.soc, blocked_by=blocked, - gate_open=bool(self.cheap)) + gate_open=self._charge_gate_open(inv)) for msg in res.events: self.log_event(msg, hint=messages.hint( "maint_failed" if res.failed else "maintenance", @@ -7387,6 +7403,10 @@ class Controller: return None +# MAINT-13: a battery power reading at or below minus this many W is a pack +# that is taking charge. +MAINT_CHARGING_W = 50.0 + # SAFETY-61. Exit status when the control task dies; 70 is sysexits(3) # EX_SOFTWARE. See DECISIONS/SAFETY-61-control-task-death.md. EXIT_CONTROL_DIED = 70 diff --git a/goodwe_controller/app/maintenance.py b/goodwe_controller/app/maintenance.py index 6883331..4edc5c9 100644 --- a/goodwe_controller/app/maintenance.py +++ b/goodwe_controller/app/maintenance.py @@ -375,6 +375,7 @@ class Maintenance: def _enter(self, phase: str, now: datetime, events: list, msg: str) -> None: self._trk = None + self._trk_saved = None if self.store.get("charge_track") is not None: self.store.set("charge_track", None) self.store.set("phase", phase) diff --git a/goodwe_controller/test_maint13.py b/goodwe_controller/test_maint13.py index 0377498..7cad14d 100644 --- a/goodwe_controller/test_maint13.py +++ b/goodwe_controller/test_maint13.py @@ -231,10 +231,12 @@ check("...or in a complete charge when the pack is near full", out and out[0].phase == HOLD) m = in_charge() res, t, out = run(m, T0, 10, 70.0) +before = m._trk["open_min"] res = m.tick(t + 2 * timedelta(hours=1), 70.0) -trk = m._trk -check("a gap in the ticks (add-on down) counts for at most 5 minutes", - trk["open_min"] < 20, ) +check("a gap in the ticks (add-on down) counts for exactly 5 minutes (the cap)", + abs(m._trk["open_min"] - before - 5.0) < 1e-9) +m.tick(t + 2 * timedelta(hours=1) + 2 * MIN, 70.0) +check("...and a 2 minute step counts 2", abs(m._trk["open_min"] - before - 7.0) < 1e-9) print("6. restarts and old state files") with tempfile.TemporaryDirectory() as d: @@ -437,6 +439,45 @@ check("...and its failure is on B's keys", c.maints[1].store.get_time("last_failed") is not None and c.maints[0].store.get_time("last_failed") is None) +# a site with no price entities and no fixed tariff: nothing holds the charge back +NOTARIFF = {k: v for k, v in OPTS1.items() + if k not in ("price_now_entity", "price_avg_entity")} +c, h = controller(NOTARIFF, False) +m = put_in_charge(c, 0, 5, 100.0) +one_cycle(c) +got = m._trk["open_min"] - 100.0 +check("N=1, no tariff and no price entities: the gate counts as open, so the " + f"3 minutes count (+{got:.1f}) although `cheap` is false", + not c.cheap and 2.5 < got < 3.5) +c, h = controller(NOTARIFF, False) +m = put_in_charge(c, 0, 40, CHARGE_TIMEOUT_MIN + 1) +one_cycle(c) +check("...and a pack past its charging time there fails at once (not 7 days)", + m.phase == IDLE and len(notes(h)) == 1) +c, h = controller(OPTS1, False) +c.inverters[0].batt = -800.0 +check("price gate closed but the pack is taking 800 W (sun): gate open", + c._charge_gate_open(c.inverters[0]) is True) +c.inverters[0].batt = -10.0 +check("...closed and about idle: gate closed", + c._charge_gate_open(c.inverters[0]) is False) +c.inverters[0].batt = None +check("...no battery reading: gate closed", c._charge_gate_open(c.inverters[0]) is False) +c, h = controller(OPTS1, True) +c.cheap = True +check("price gate open: gate open", c._charge_gate_open(c.inverters[0]) is True) +c, h = controller(dict(NOTARIFF, static_profile="x"), False) +check("a fixed tariff set up counts as a gate (closed outside its cheap hours)", + c._charge_gate_open(c.inverters[0]) is False) + +# the save timer is reset on a phase change +mm = in_charge() +mm.tick(T0 + MIN, 70.0) +saved_before = mm._trk_saved +mm.abort(T0 + 2 * MIN, "operator") +check("leaving CHARGE resets the save timer along with the record", + saved_before is not None and mm._trk is None and mm._trk_saved is None) + print("9. the words") en, nl = messages.maint_failed("charge", 7), messages.maint_failed("drain", 7) for n_ in (en, nl): diff --git a/goodwe_controller/test_safety93.py b/goodwe_controller/test_safety93.py index fa0ddda..6809a8e 100644 --- a/goodwe_controller/test_safety93.py +++ b/goodwe_controller/test_safety93.py @@ -37,7 +37,7 @@ import safety79_identity as s79 # noqa: E402 import safety84_identity as s84 # noqa: E402 from app import packmode # noqa: E402 from app.main import Controller # noqa: E402 -from app.maintenance import (CHARGE_TIMEOUT_MIN, HOLD, IDLE, # noqa: E402 +from app.maintenance import (CHARGE_TIMEOUT_MIN, DRAIN, HOLD, IDLE, # noqa: E402 MaintResult) from app.store import unit_key # noqa: E402 from named_absence import first, present # noqa: E402 @@ -203,10 +203,6 @@ def timed_out(end): s.data[unit_key(A, "phase_started")] = ( rig.BASE + timedelta(seconds=end - CHARGE_TIMEOUT_MIN * 60 - 1) ).isoformat() - # MAINT-13: a timed-out charge now backs off 7 days instead of restarting - # at once, so a due B would start its own drain here. B has finished its - # cycle, as in b_done(), so only the charge end is graded. - s.data[unit_key("inverter-b", "last_completed")] = rig.BASE.isoformat() return s @@ -225,9 +221,16 @@ def switch_off(at): return before +def timed_out_b_done(end): + """timed_out(), with B's monthly cycle finished so it starts none.""" + s = timed_out(end) + s.data[unit_key("inverter-b", "last_completed")] = rig.BASE.isoformat() + return s + + ENDS = { "timeout": lambda: run(2, 6000.0, 9999.0, step=1760.0, dur=2080.0, - store=timed_out(1780.0)), + store=timed_out_b_done(1780.0)), "Off": lambda: run(2, 6000.0, 9999.0, step=1760.0, dur=2080.0, store=b_done(), before=switch_off(1780.0), extra={"battery_modes_enabled": True}), @@ -250,6 +253,27 @@ for k, fn in ENDS.items(): f"the grid", at(old, 1780.0, "b") == -1600.0 and charging_hi(old, 1780, 1798, hi1) >= 1) +# MAINT-13: A's timed-out charge is a failed cycle and backs off, so A no longer +# restarts at once; a due B takes the slot instead and starts ITS drain. That is +# a drain start (a step to the drain rate by design), not a charge end. +rows, c = run(2, 6000.0, 9999.0, step=1760.0, dur=2080.0, + store=timed_out(1780.0)) +print(f" timeout, B due: A {[cmd(r)['a'] for r in rows if 1776 <= r[0] <= 1790]}" + f" B {[cmd(r)['b'] for r in rows if 1776 <= r[0] <= 1790]}") +check("timeout with B due: the charge ends at 1780 s and A is not restarted", + phase_at(rows, 1778.0) == "charge" and phase_at(rows, 1780.0) == IDLE + and c.maints[0].phase == IDLE + and c.maints[0].backoff_until(rig.BASE + timedelta(seconds=1800)) + is not None) +check("⭐ timeout with B due: no battery charges from the grid after it", + charging_hi(rows, 1780, 1798, hi1) == 0 + and min(cmd(r)["b"] for r in rows if r[0] >= 1780) >= 0.0) +check("...B takes the slot and begins its own drain, which is the only large " + "step, to the drain rate and no higher", + c.maints[1].phase == DRAIN + and max(cmd(r)["b"] for r in rows if r[0] >= 1780) <= 2000.0 + and moves(rows, "a", 1780, 1798) <= SLEW) + # ============================================================================ # print("\n4. the charge ends while the guard has suspended it") # House 6000 W from 1500 s: the guard suspends the shaping, the loop has both -- 2.54.0 From f7b83cc75c1484edd1791d839fa1a0f63610affb Mon Sep 17 00:00:00 2001 From: glenn schrooyen Date: Thu, 8 Oct 2026 11:10:33 +0200 Subject: [PATCH 06/17] MAINT-13 round 2: read the price gate without a new main.py option read test_static_tariff forbids main.py reading `static_profile`; a small static_gated() in static_tariff.py answers it, and the Controller keeps the answer in `_price_gated` beside `cheap`. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC --- goodwe_controller/app/main.py | 10 +++++----- goodwe_controller/app/static_tariff.py | 10 ++++++++++ goodwe_controller/test_maint13.py | 22 ++++++++++++++++++---- 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index 97985f1..3eda31b 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -91,7 +91,7 @@ from .price import Absence, Uncertainty, brussels, price_option, provider_config from .price import required_spread as price_required_spread from .price import uncertainty_of as price_uncertainty_of from .price_history import PriceHistory -from .static_tariff import resolve_tariff, static_status +from .static_tariff import resolve_tariff, static_gated, static_status from .tariff import PeakStore, PeakTracker, next_quarter from .store import migrate_maint_keys, migrate_unit_keys from .validate import (FIRMWARE_INGEST_VETO_S, ConfigRefused, refuse, @@ -1188,6 +1188,7 @@ class Controller: self.grid = None self.peak_fc = None self.cheap = False + self._price_gated = False # MAINT-13: is a price gate set up # TARIFF-03. One line of prose for the status page saying WHICH tariff # window we are in and why it is or is not cheap - so "Cheap window: # no" is an evidenced answer rather than a bare no. @@ -3312,6 +3313,7 @@ class Controller: now_p = await self.hass.number(now_e) avg_p = await self.hass.number(avg_e) self.cheap = (now_p is not None and avg_p is not None and now_p <= avg_p) + self._price_gated = True self.tariff_words = "Prices from Home Assistant entities" else: # TARIFF-03. §9.2 makes a static day/night tariff a first-class @@ -3337,6 +3339,7 @@ class Controller: # one and the loser is silent. self.cheap, self.tariff_words = static_status( o, datetime.now(timezone.utc)) + self._price_gated = static_gated(o, datetime.now(timezone.utc)) # ⚠️ TARIFF-11. THE PROVIDER PLAN REACHES THE *Tariff* ROW HERE, AND # THIS IS THE ONLY PLACE THE TWO SOURCES MEET. Everything above is the @@ -3883,10 +3886,7 @@ class Controller: really taking charge (sun outside cheap hours). Closed only when a gate exists, it is shut, and the pack is not charging. """ - o = self.o - gated = bool((o.get("price_now_entity") and o.get("price_avg_entity")) - or o.get("static_profile")) - if not gated or self.cheap: + if not self._price_gated or self.cheap: return True batt = inv.batt return batt is not None and float(batt) <= -MAINT_CHARGING_W diff --git a/goodwe_controller/app/static_tariff.py b/goodwe_controller/app/static_tariff.py index fc6659b..aa838c2 100644 --- a/goodwe_controller/app/static_tariff.py +++ b/goodwe_controller/app/static_tariff.py @@ -1232,6 +1232,16 @@ def static_plan(opts: dict, since: datetime, until: datetime, *, profile, "fallback" if provisional else "static") +def static_gated(opts: dict, now: datetime) -> bool: + """MAINT-13: is a fixed tariff set up, so that a cheap window can hold a + charge back? False for no profile or one that cannot be used.""" + try: + return bool(static_plan(opts, now, now + timedelta(days=MEAN_WINDOW_DAYS), + now=now).has_prices) + except ProfileInvalid: + return False + + def static_status(opts: dict, now: datetime): """(cheap, words) for the status page and for main.py's cheap-window flag. diff --git a/goodwe_controller/test_maint13.py b/goodwe_controller/test_maint13.py index 7cad14d..505c13a 100644 --- a/goodwe_controller/test_maint13.py +++ b/goodwe_controller/test_maint13.py @@ -454,7 +454,12 @@ m = put_in_charge(c, 0, 40, CHARGE_TIMEOUT_MIN + 1) one_cycle(c) check("...and a pack past its charging time there fails at once (not 7 days)", m.phase == IDLE and len(notes(h)) == 1) +def read(c): + asyncio.run(c.read_inputs()) + + c, h = controller(OPTS1, False) +read(c) c.inverters[0].batt = -800.0 check("price gate closed but the pack is taking 800 W (sun): gate open", c._charge_gate_open(c.inverters[0]) is True) @@ -464,11 +469,20 @@ check("...closed and about idle: gate closed", c.inverters[0].batt = None check("...no battery reading: gate closed", c._charge_gate_open(c.inverters[0]) is False) c, h = controller(OPTS1, True) -c.cheap = True +read(c) check("price gate open: gate open", c._charge_gate_open(c.inverters[0]) is True) -c, h = controller(dict(NOTARIFF, static_profile="x"), False) -check("a fixed tariff set up counts as a gate (closed outside its cheap hours)", - c._charge_gate_open(c.inverters[0]) is False) +PROFILE = json.dumps({"periods": [ + {"name": "day", "from": "07:00", "to": "22:00", "days": "all", + "eur_per_kwh": 0.29}, + {"name": "night", "from": "22:00", "to": "07:00", "days": "all", + "eur_per_kwh": 0.19}]}) +c, h = controller(dict(NOTARIFF, price_provider="off", static_profile=PROFILE), + False) +read(c) +check("a fixed tariff set up counts as a gate: outside its cheap hours a pack " + "that is not charging is closed, inside them it is open", + c._price_gated is True + and c._charge_gate_open(c.inverters[0]) is c.cheap) # the save timer is reset on a phase change mm = in_charge() -- 2.54.0 From 6836f61bd401cb9235f730ab5d91dca37ed360c1 Mon Sep 17 00:00:00 2001 From: glenn schrooyen Date: Thu, 8 Oct 2026 12:01:17 +0200 Subject: [PATCH 07/17] MAINT-13 round 3: provider site, hourly fixed-tariff check, faster tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - A price-provider site with no fixed tariff is not a gate (its plan never sets `cheap`); DECISIONS §3 says so and a test pins it. - The fixed-tariff check is worked out once an hour, not every tick. - test_maint13: run() steps really are `step` minutes; the long loops use 5-minute steps (the per-tick cap); wall backstop checked "not before". - test_safety93: the drain bound is the rig's configured drain rate. Co-Authored-By: Claude Opus 5.5 --- .../MAINT-13-full-pack-is-complete.md | 8 ++- goodwe_controller/app/main.py | 17 +++++-- goodwe_controller/test_maint13.py | 50 +++++++++++++++---- goodwe_controller/test_safety93.py | 8 ++- 4 files changed, 67 insertions(+), 16 deletions(-) diff --git a/goodwe_controller/DECISIONS/MAINT-13-full-pack-is-complete.md b/goodwe_controller/DECISIONS/MAINT-13-full-pack-is-complete.md index b505b1e..af3aa27 100644 --- a/goodwe_controller/DECISIONS/MAINT-13-full-pack-is-complete.md +++ b/goodwe_controller/DECISIONS/MAINT-13-full-pack-is-complete.md @@ -67,7 +67,13 @@ holds the charge back, and `cheap` is always false there), in a cheap window, or while the pack is really taking charge (battery power at or below -50 W, which is sun outside cheap hours). It is false only when a gate exists, is shut, and the pack is not charging; a pack asked for charge that delivers nothing then does not -count, which is the case the clock cannot judge. Only open ticks add to `open_min`; one tick +count, which is the case the clock cannot judge. A site with a price provider +(TARIFF-11) and no fixed tariff counts as having no gate: the provider's plan +reaches only the Tariff row, never `cheap`, so no cheap window ever opens a grid +top-up there and only the sun charges the pack, as on a site with no tariff. Its +time counts in full. When a provider plan starts to drive `cheap`, +`_price_gated` must become true for it. Whether a fixed tariff is set up is +worked out once an hour, not on every tick. Only open ticks add to `open_min`; one tick adds at most 5 min, so a gap (add-on down) adds nothing. The stall wait pauses while the gate is closed. The accounting is written to the state file at most every 5 min (`charge_track`); a restart loses at most that. A state file from diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index 3eda31b..b7a3fef 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -1189,6 +1189,7 @@ class Controller: self.peak_fc = None self.cheap = False self._price_gated = False # MAINT-13: is a price gate set up + self._gated_key = None # MAINT-13: (options, hour) of the cache # TARIFF-03. One line of prose for the status page saying WHICH tariff # window we are in and why it is or is not cheap - so "Cheap window: # no" is an evidenced answer rather than a bare no. @@ -3337,9 +3338,9 @@ class Controller: # ⚠️ AND IT CANNOT BE BOTH. validate.py refuses static_profile # alongside the two Home Assistant entities, because this `if` picks # one and the loser is silent. - self.cheap, self.tariff_words = static_status( - o, datetime.now(timezone.utc)) - self._price_gated = static_gated(o, datetime.now(timezone.utc)) + _now = datetime.now(timezone.utc) + self.cheap, self.tariff_words = static_status(o, _now) + self._price_gated = self._static_gated(o, _now) # ⚠️ TARIFF-11. THE PROVIDER PLAN REACHES THE *Tariff* ROW HERE, AND # THIS IS THE ONLY PLACE THE TWO SOURCES MEET. Everything above is the @@ -3878,6 +3879,16 @@ class Controller: inv.refusal_note())] if n] + def _static_gated(self, o: dict, now: datetime) -> bool: + """`static_gated()`, worked out once an hour: it builds the same + month-long plan as `static_status()` (about 20 ms with a profile), and + whether a fixed tariff is set up does not change from tick to tick + (options change only with a restart).""" + key = (id(o), now.strftime("%Y-%m-%d %H")) + if key != self._gated_key: + self._gated_key, self._gated = key, static_gated(o, now) + return self._gated + def _charge_gate_open(self, inv) -> bool: """MAINT-13: may this pack charge now, for the maintenance timeout? diff --git a/goodwe_controller/test_maint13.py b/goodwe_controller/test_maint13.py index 505c13a..c2bf93b 100644 --- a/goodwe_controller/test_maint13.py +++ b/goodwe_controller/test_maint13.py @@ -82,8 +82,8 @@ def run(m, start, minutes, soc, gate=True, step=1): Returns (the last result, the time after the run, the first non-CHARGE result).""" t, res, out = start, None, None for i in range(0, minutes, step): - t = start + i * step * MIN - s = soc(i * step) if callable(soc) else soc + t = start + i * MIN + s = soc(i) if callable(soc) else soc res = m.tick(t, s, gate_open=gate) if out is None and res.phase != CHARGE: out = (res, t) @@ -215,20 +215,23 @@ check("a drain that times out is a failure too, with the same back-off", print("5. only the minutes the pack was allowed to charge count") m = in_charge() -res, t, out = run(m, T0, CHARGE_TIMEOUT_MIN * 2, 70.0, gate=False) +# 5 min steps: one tick counts at most 5 min, so this is the same count as +# 1 min steps in a fifth of the ticks. +res, t, out = run(m, T0, CHARGE_TIMEOUT_MIN * 2, 70.0, gate=False, step=5) check("72 h with the gate closed: not timed out", out is None and m.phase == CHARGE) -res, t, out = run(m, t, CHARGE_TIMEOUT_MIN - 5, 70.0) +res, t, out = run(m, t, CHARGE_TIMEOUT_MIN - 5, 70.0, step=5) check("...and 35 h 55 min open on top: still charging", out is None) res, t, out = run(m, t, 10, 70.0) check("...36 h of open time in all: the charge fails", out and out[0].failed == "charge") m = in_charge() -res, t, out = run(m, T0, CHARGE_WALL_TIMEOUT_MIN + 5, 70.0, gate=False, step=5) -check("a gate that never opens still ends in a failure after 7 days", - out and out[0].failed == "charge") +WALL = T0 + CHARGE_WALL_TIMEOUT_MIN * MIN +res, t, out = run(m, T0, CHARGE_WALL_TIMEOUT_MIN + 10, 70.0, gate=False, step=5) +check("a gate that never opens still ends in a failure after 7 days (not before)", + out and out[0].failed == "charge" and WALL < out[1] <= WALL + 5 * MIN) m = in_charge() -res, t, out = run(m, T0, CHARGE_WALL_TIMEOUT_MIN + 5, 98.0, gate=False, step=5) +res, t, out = run(m, T0, CHARGE_WALL_TIMEOUT_MIN + 10, 98.0, gate=False, step=5) check("...or in a complete charge when the pack is near full", - out and out[0].phase == HOLD) + out and out[0].phase == HOLD and WALL < out[1] <= WALL + 5 * MIN) m = in_charge() res, t, out = run(m, T0, 10, 70.0) before = m._trk["open_min"] @@ -256,7 +259,7 @@ with tempfile.TemporaryDirectory() as d: m_b.phase == CHARGE and open_before - 6 <= m_b._trk["open_min"] <= open_before + 2 and m_b._trk["open_min"] < 60) - _, t2, out = run(m_b, t + 2 * MIN, CHARGE_TIMEOUT_MIN - 30, 70.0) + _, t2, out = run(m_b, t + 2 * MIN, CHARGE_TIMEOUT_MIN - 30, 70.0, step=5) check("...so the timeout comes after 36 h in all, not 36 h from the restart", out is not None and out[0].failed == "charge") m = Maintenance(CFG, Store(path)) @@ -454,6 +457,19 @@ m = put_in_charge(c, 0, 40, CHARGE_TIMEOUT_MIN + 1) one_cycle(c) check("...and a pack past its charging time there fails at once (not 7 days)", m.phase == IDLE and len(notes(h)) == 1) +# a site with a price provider (TARIFF-11) and no fixed tariff: the provider's +# plan reaches only the Tariff row, never `cheap`, so no cheap window ever +# opens a grid top-up there. Like the site above, nothing gate-like holds the +# charge back, and the time counts. +PROVIDER = dict(NOTARIFF, price_provider="generic_dayahead", static_profile="") +c, h = controller(PROVIDER, False) +m = put_in_charge(c, 0, 5, 100.0) +one_cycle(c) +got = m._trk["open_min"] - 100.0 +check("N=1, price provider and no fixed tariff: not a gate (`cheap` is never " + f"set from the provider), so the 3 minutes count (+{got:.1f})", + c._price_gated is False and not c.cheap + and c._charge_gate_open(c.inverters[0]) is True and 2.5 < got < 3.5) def read(c): asyncio.run(c.read_inputs()) @@ -483,6 +499,20 @@ check("a fixed tariff set up counts as a gate: outside its cheap hours a pack " "that is not charging is closed, inside them it is open", c._price_gated is True and c._charge_gate_open(c.inverters[0]) is c.cheap) +import app.main as main_mod # noqa: E402 +_real_gated, _calls = main_mod.static_gated, [] +main_mod.static_gated = lambda o, now: _calls.append(now) or _real_gated(o, now) +try: + c._gated_key = None + read(c) + read(c) + read(c) + c._static_gated(c.o, datetime.now(UTC) + timedelta(hours=1)) +finally: + main_mod.static_gated = _real_gated +check("...worked out once an hour, not on every tick (3 reads in one hour = 1 " + f"build; a new hour = 1 more): {len(_calls)} builds", + len(_calls) == 2 and c._price_gated is True) # the save timer is reset on a phase change mm = in_charge() diff --git a/goodwe_controller/test_safety93.py b/goodwe_controller/test_safety93.py index 6809a8e..7e27348 100644 --- a/goodwe_controller/test_safety93.py +++ b/goodwe_controller/test_safety93.py @@ -265,13 +265,17 @@ check("timeout with B due: the charge ends at 1780 s and A is not restarted", and c.maints[0].phase == IDLE and c.maints[0].backoff_until(rig.BASE + timedelta(seconds=1800)) is not None) +# A command below 0 W is a charge, so B's commands must stay at or above 0; +# the drain rate is the one this rig's controller was configured with. +drain_w = c.maints[1].cfg.discharge_w +print(f" B drain rate configured on the rig: {drain_w:.0f} W") check("⭐ timeout with B due: no battery charges from the grid after it", charging_hi(rows, 1780, 1798, hi1) == 0 and min(cmd(r)["b"] for r in rows if r[0] >= 1780) >= 0.0) check("...B takes the slot and begins its own drain, which is the only large " - "step, to the drain rate and no higher", + "step, to the configured drain rate and no higher", c.maints[1].phase == DRAIN - and max(cmd(r)["b"] for r in rows if r[0] >= 1780) <= 2000.0 + and max(cmd(r)["b"] for r in rows if r[0] >= 1780) <= drain_w and moves(rows, "a", 1780, 1798) <= SLEW) # ============================================================================ # -- 2.54.0 From ea31289ec083ac779af903101b085b3fa1f2ba85 Mon Sep 17 00:00:00 2001 From: MAINT-13 review Date: Thu, 8 Oct 2026 13:08:40 +0200 Subject: [PATCH 08/17] MAINT-13 review: tests for a stale charge record and a clock step back; DOCS names the 7 day end Co-Authored-By: Claude Opus 5.5 --- goodwe_controller/DOCS.md | 3 ++- goodwe_controller/test_maint13.py | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md index 6523d3f..7c46df8 100644 --- a/goodwe_controller/DOCS.md +++ b/goodwe_controller/DOCS.md @@ -1672,7 +1672,8 @@ balances, so it need not reach `maintenance_soc_target`. The charge counts as done when the battery is within 3 points of the target and has taken no more charge for 60 minutes of charging time, or when the charging time (36 hours) runs out within 3 points of the target. Only the minutes the cheap-hours gate let the -battery charge count. Then the usual hold follows. +battery charge count, and the charge ends after 7 days at the +latest. Then the usual hold follows. **When a cycle fails.** If the battery does not empty in 12 hours, or does not get within 3 points of the target in 36 hours of charging time, the cycle diff --git a/goodwe_controller/test_maint13.py b/goodwe_controller/test_maint13.py index c2bf93b..07881ed 100644 --- a/goodwe_controller/test_maint13.py +++ b/goodwe_controller/test_maint13.py @@ -240,6 +240,10 @@ check("a gap in the ticks (add-on down) counts for exactly 5 minutes (the cap)", abs(m._trk["open_min"] - before - 5.0) < 1e-9) m.tick(t + 2 * timedelta(hours=1) + 2 * MIN, 70.0) check("...and a 2 minute step counts 2", abs(m._trk["open_min"] - before - 7.0) < 1e-9) +_t = t + 2 * timedelta(hours=1) + 2 * MIN +m.tick(_t - 10 * MIN, 70.0) # the clock steps back 10 min +check("...and a step back in time counts nothing (not minus)", + abs(m._trk["open_min"] - before - 7.0) < 1e-9) print("6. restarts and old state files") with tempfile.TemporaryDirectory() as d: @@ -286,6 +290,19 @@ with tempfile.TemporaryDirectory() as d: res, _, out = run(mo, T0 + MIN, 120, 70.0) check("...so it fails when the 36 h are up, as before", out and out[0].failed == "charge") + # A record left by another charge (an add-on rolled back and forward + # again does not clear it) must not be read as this charge's. + stale = {"phase": "charge", + "phase_started": (T0 - timedelta(hours=2)).isoformat(), + "charge_track": {"since": (T0 - timedelta(days=30)).isoformat(), + "open_min": 2000.0, "flat_min": 59.0, + "best": 99.0, "at": None}} + ms = Maintenance(CFG, Mem(stale)) + r = ms.tick(T0, 98.0) + check("a charge record from another charge is ignored: this charge counts " + "its own 2 h and its own best", + r.phase == CHARGE and 119 < ms._trk["open_min"] < 121 + and ms._trk["flat_min"] == 0.0 and ms._trk["best"] == 98.0) mi = Maintenance(CFG, Store(os.path.join(d, "none.json"))) check("no state file: due, no back-off", mi.due(T0) and mi.backoff_until(T0) is None) -- 2.54.0 From f39e52efd3a9f10383df5bd28d5842f6c9de7cee Mon Sep 17 00:00:00 2001 From: glenn schrooyen Date: Thu, 8 Oct 2026 20:37:01 +0200 Subject: [PATCH 09/17] 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") -- 2.54.0 From ef35f4b925e2db39a30e5f746b9f630ebc47e86c Mon Sep 17 00:00:00 2001 From: glenn schrooyen Date: Thu, 8 Oct 2026 21:25:35 +0200 Subject: [PATCH 10/17] 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") -- 2.54.0 From 61a61941bb5c65eeac25e749ff09b9cadabc0f93 Mon Sep 17 00:00:00 2001 From: MAINT-14 review Date: Thu, 8 Oct 2026 22:14:26 +0200 Subject: [PATCH 11/17] 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 " -- 2.54.0 From d12228d986332058635ad526703966799c222996 Mon Sep 17 00:00:00 2001 From: glenn schrooyen Date: Fri, 9 Oct 2026 03:03:06 +0200 Subject: [PATCH 12/17] SAFETY-107: a refusal held under another writer is learned once At one inverter SAFETY-52's "another writer" end tested target != loop_w. A held refusal makes the loop ask 0 W, so any binding claim (maintenance charge, peak guard, trade) ended it every cycle, and compute() learned it again in the same cycle: "BATTERY REFUSES" / "ENDED: another writer" each second on the rig. Now the held refusal is judged afresh against the writer's ask (compute() gets the same inputs as before); it ends only if compute() no longer finds it. Commands unchanged: 300-seed differential trace against release/1.0, N=1 and N=2, every command and refusal state identical. DECISIONS/SAFETY-107. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC --- ...SAFETY-107-refusal-under-another-writer.md | 93 ++++++ goodwe_controller/DOCS.md | 6 +- goodwe_controller/app/main.py | 38 ++- goodwe_controller/test_safety107.py | 296 ++++++++++++++++++ 4 files changed, 425 insertions(+), 8 deletions(-) create mode 100644 goodwe_controller/DECISIONS/SAFETY-107-refusal-under-another-writer.md create mode 100644 goodwe_controller/test_safety107.py diff --git a/goodwe_controller/DECISIONS/SAFETY-107-refusal-under-another-writer.md b/goodwe_controller/DECISIONS/SAFETY-107-refusal-under-another-writer.md new file mode 100644 index 0000000..40e427c --- /dev/null +++ b/goodwe_controller/DECISIONS/SAFETY-107-refusal-under-another-writer.md @@ -0,0 +1,93 @@ +# SAFETY-107 - a refusal held under another writer is learned once + +**Status: for review.** Tests: `test_safety107.py`. One inverter only; nothing +changes at N>1 (SAFETY-73's unit refusal has no "another writer" end). + +**The defect** (MAINT-13 F5, MAINT-14 "seen again"). A full pack under a +maintenance charge in a cheap window takes no charge. At N=1 the action log then +showed "BATTERY REFUSES asked -2500 W ..." and "BATTERY REFUSES ENDED: another +writer" every cycle: 548 + 548 lines in 15 min on the MAINT-13 rig head, +652 + 651 in 12 min on its base, 33 + 32 in 60 s in MAINT-14's c2b. The 40-line +log flooded and the hint under the buttons changed every second, so the +MAINT-13 notice was gone at once. + +**Cause** (`main.py`, the SAFETY-52 block before `compute()`). The end tested +`self.target != self.loop_w`: the wire differs from the loop's ask. A held +refusal makes the loop ask 0 W, so any claim that binds (the maintenance +charge's `hi`, the peak guard's `lo`, a trade's SET) differs from it on every +cycle. The refusal was ended against the same writer it had been learned +against. `sat_count` is not reset by that end, so `compute()` learned the same +refusal again in the same cycle. Nothing about the battery or the wire had +changed; only the log and the hint did. + +**Other writers.** The same flap comes from any writer that holds the wire +while the loop is refused, as long as the evidence lasts: a trade (a SET at +P_ARBITRAGE) into a full pack, and the peak guard over an empty pack: 335 +refusal lines in 15 min each at a 5 s meter (`test_safety107.py` part 2, run on +release/1.0). +A maintenance drain or hold does not flap: its SET resets `sat_count` each cycle. + +**The rule** (`_refusal_update()`, `rejudged`). When another writer held the +wire, the held refusal is judged afresh: `compute()` gets `refused=0` and no +probe, exactly as before the change. Then: + +1. it finds the same refusal against that writer's ask: the same episode + continues, with no line and no hint. The probe clock restarts, as it did on + the old end and start: a probe is the loop's ask, and it is not on the wire + while another writer is; +2. it does not: the refusal ends, logged "another writer", or "the battery + gives power" when the reading shows it (it was "another writer" before); +3. it finds the other direction: end, then a new start, as before. + +**Why this rule.** The end now means what SAFETY-52 meant by it: the refusal no +longer describes what is on the wire, judged by the same test that learns it. +`compute()` sees the same inputs as before and the state after each cycle is +the same (`batt_refused`, `batt_refused_at`), so every command is unchanged. +Keeping the held refusal into `compute()` instead (mutation M5) would also +stop the lines, but it holds the loop at 0 W where the old law let it ask past +a writer's limit, which changes commands; it also removes SAFETY-52's +owner-decided "another writer" exit. + +**Evidence.** +- Differential trace (`goodwe/scratch/SAFETY-107/probe/trace.py`, not in this + repo; the real Controller in the CI image, release/1.0 bf3ac79 against this + tree): 300 seeds, N=1 (153) and + N=2 (147), 1/2/5 s meter, full/empty/dead/healthy battery windows, a stepping + house, maintenance charge with the cheap window opening and closing, P1 and + the peak guard, trade windows, stop/start; 427 680 cycles, 294 seeds with a + refusal held. Every command, site target, integrator, `sat_count`, refusal + state and reason is identical. Refusal lines: 100 748 before, 2 188 after. +- The card's case: 1 line in 15 min at N=1 and N=2, 1/2/5 s meter, house + importing or exporting (release/1.0: 335-1 753 at N=1; N=2 was already 1). + +**Mutations** (`tools/mutation_run.sh` on purged copies; reds in +test_safety107 / test_refusal / test_refusal_units; trace = seeds of 300 whose +commands differ from release/1.0): + +| | mutation | reds | trace | +|---|---|---|---| +| M1 | `now == rejudged` -> `False` (every such cycle ends: the old flap) | 26 / 0 / 0 | 0 | +| M2 | `rejudged` always 0 (another writer never ends it) | 5 / 1 / 2 | 29 | +| M3 | the probe clock is not restarted on a continued episode | 1 / 0 / 0 | 2 | +| M4 | a probe may open while rejudged | 0 / 0 / 0 | 0 | +| M5 | the held refusal goes into `compute()` while rejudged | 2 / 1 / 2 | 27 | +| M6 | an end while rejudged always says "another writer" | 1 / 0 / 0 | 0 | + +M4 is equivalent: a probe matters only if `compute()` engages in that call, +which needs `SETTLE_CYCLES` saturated against the writer, and each of those +cycles restarted the clock. The line stays because it states the rule. + +**Cost.** None in power. A takeover by a different writer while the battery +still refuses is logged as an end and, ~12 cycles later, a new start against +that writer (3 lines for the episode, `test_safety107.py` part 3a). + +**Not covered.** +- A perfectly flat reading under a writer. The stale-reading gate trips, stops + the site and seeds the refusal (SAFETY-52 decision 3). After the stop the + writer's ask returns fresh, `compute()` has no saturation history against it, + so the refusal ends at the first cycle and the next trip seeds it again: 2 + lines per gate cycle (29 in 15 min, house exporting), the same as release/1.0. + The gate cycle itself (0 W, then the writer's ask) is the cause; stopping it + changes commands. None of the rig runs showed it: real readings move. +- At N=1 a refusal shapes only the loop. A writer above it still asks the + battery; at N>1 SAFETY-73's P_SAFETY claim binds every writer. diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md index 7c46df8..0d00d11 100644 --- a/goodwe_controller/DOCS.md +++ b/goodwe_controller/DOCS.md @@ -605,6 +605,10 @@ still moving towards the request, such as one turning from charging to giving (SAFETY-94). "Gives" is counted from what the battery's reading shows at rest, which the add-on learns after two quiet minutes and keeps: some inverters show about -100 W at rest (SAFETY-97). +While another task sets the power, such as a maintenance charge, the peak +guard or a trade, the refusal stays as long as the battery still refuses that +task's request. It is logged once, and the 200 W test waits until control sets +the power again (SAFETY-107). **With two or more inverters** the same rule works per inverter. An inverter whose battery gives less than 100 W for 12 cycles, and at least 22 s, while @@ -618,7 +622,7 @@ If the battery refuses to give power while its level reads above its “Battery cut-out level”, the log adds one warning ending in `REFUSES_ABOVE_CUT_OUT`: check that setting. A battery that refuses is not reported as *not delivering*. -*For installers:* SAFETY-52 and SAFETY-73. The event log reads +*For installers:* SAFETY-52, SAFETY-73 and SAFETY-107. The event log reads `BATTERY REFUSES …` and `BATTERY REFUSES ENDED …` (with the inverter's name at N>1), and the INFO line ends in `BATTERY_REFUSES`. `DECISIONS/SAFETY-52-a-battery-that-refuses.md` has the replay figures and diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index b7a3fef..a85adce 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -1279,9 +1279,26 @@ class Controller: _LOG.info("The battery refusal ends, control asks for power again: %s.", why) - def _refusal_update(self, decision, asked_w, batt_w, probe) -> None: - """Follow `compute()`'s carried refused direction, and say so.""" + def _refusal_update(self, decision, asked_w, batt_w, probe, + rejudged: int = 0) -> None: + """Follow `compute()`'s carried refused direction, and say so. + + `rejudged`: the refusal held while another writer had the wire, so + `compute()` judged it afresh against that writer's ask (SAFETY-107). + """ now = decision.refused or 0 + if rejudged: + if now == rejudged: + # Still refused against the ask on the wire: the same episode. + # The probe clock restarts, as it did when this was an end and + # a new start: a probe is the loop's ask, and it is not on the + # wire while another writer is. + self.batt_refused_at = self._now_mono + return + gave = (rejudged * (batt_w - self._site_idle_w()) + >= _ctl.PROBE_ANSWER_W) + self._refusal_end("the battery gives power" if gave + else "another writer") if now and not self.batt_refused: self._refusal_start(now, f"asked {asked_w:+.0f} W, the battery " f"gave {batt_w:+.0f} W for " @@ -4973,11 +4990,15 @@ class Controller: if trade_was and not mode_trade: self._reseed_run_edge(claims) # SAFETY-52: another writer overrode the loop, so a refusal learned - # against the loop's own ask no longer describes the wire. A gap + # against the loop's own ask may no longer describe the wire. A gap # (`loop_w` None: blind or stopped) keeps it: the stale-reading stop # is what seeds it, and dropping it there restarts the ~70 s cycle. + # SAFETY-107: it ends only if compute() no longer finds it against + # that writer's ask (`_refusal_update()`). A held refusal makes the + # loop ask 0 W, so any claim that binds differs from it every cycle. + rejudged = 0 if self.loop_w is not None and self.target != self.loop_w: - self._refusal_end("another writer") + rejudged = self.batt_refused # SAFETY-96: a battery that dropped out of the split leaves the # loop's start point (SAFETY-93's own if its charge ended too). self._reseed_drop(_live96) @@ -4986,7 +5007,8 @@ class Controller: # SAFETY-95: then a drain that ended, last (it reads the start # the others left). self._reseed_drain_end(maint_results, _live96) - probe = self._refusal_probe() + # No probe while it is judged afresh (the loop does not hold the wire). + probe = False if rejudged else self._refusal_probe() if len(self.inverters) > 1: probe = any(_probing.values()) # ⚠️ `actual_w` IS THE SITE SUM AND THAT IS DEV-02's OTHER HALF. @@ -5029,7 +5051,8 @@ class Controller: # limit already closes an empty battery's discharge end. # SAFETY-73: at N>1 the site law holds 0 W only while every # unit refuses the same way. Otherwise None, the old law. - refused=self.batt_refused if len(self.inverters) == 1 + refused=(0 if rejudged else self.batt_refused) + if len(self.inverters) == 1 else self._site_refused(), probe=probe, # SAFETY-94: at N=1 the engage also needs the reading to have @@ -5055,7 +5078,8 @@ class Controller: self.loop_w = decision.target_w self._loop_decision = decision if len(self.inverters) == 1: - self._refusal_update(decision, loop_prev, site_batt, probe) + self._refusal_update(decision, loop_prev, site_batt, probe, + rejudged) # --- TEL-10 ------------------------------------------------------ # ⚠️ THE INTEGRATOR HAS NEVER LEFT THIS PROCESS, AND THAT IS WHAT # THE CARD IS. `self.i_w` is carried between cycles and published diff --git a/goodwe_controller/test_safety107.py b/goodwe_controller/test_safety107.py new file mode 100644 index 0000000..8f202b9 --- /dev/null +++ b/goodwe_controller/test_safety107.py @@ -0,0 +1,296 @@ +"""Runnable check for SAFETY-107: a refusal held while another writer has the +wire is learned once. `python3 test_safety107.py` + +A full pack under a maintenance charge in a cheap window took no charge. At +one inverter SAFETY-52's "another writer" end then fired every cycle, and +compute() learned the refusal again in the same cycle: "BATTERY REFUSES" and +"BATTERY REFUSES ENDED: another writer" each second on the rig (MAINT-13 F5). +Now another writer ends a refusal only if compute(), judging it afresh +against that writer's ask, no longer finds it. DECISIONS/SAFETY-107. + +Drives the REAL Controller (read_inputs + cycle) on opt11_identity's rig. +1. The card's case: 15 min of a full pack under a maintenance charge, N=1 + and N=2, at a 1, 2 and 5 s meter: at most 2 refusal lines, held throughout. +2. Other writers: a trade and the peak guard over a battery that gives + nothing. One episode each, no flap. +3. A refusal still ends: a different writer really takes over, the battery + delivers again, a writer asks the other way, stop. The probe clock. + +4. Not covered, pinned: a perfectly flat reading under a writer. + +Mutations (MUTATION.md, purged copies) are listed in DECISIONS/SAFETY-107. +""" + +import functools +import logging +import os +import sys +import tempfile + +HERE = os.path.dirname(os.path.abspath(__file__)) +sys.path.insert(0, HERE) +sys.path.insert(0, os.path.join(HERE, "tools")) +logging.disable(logging.WARNING) + +import opt11_identity as rig # noqa: E402 +import safety79_identity as s79 # noqa: E402 +import app.main as app_main # noqa: E402 +from app import control # noqa: E402 +from app.arbiter import Claim, P_ARBITRAGE # noqa: E402 +from app.main import Controller # noqa: E402 +from named_absence import nth, present # noqa: E402 + +fails = [] +QUARTER = 900.0 + + +def check(name, cond): + print(f" {'ok ' if cond else 'FAIL'} {name}") + if not cond: + fails.append(name) + + +def run(n=1, *, plant, house, dur=QUARTER, tick=5.0, maint=True, + cheap=lambda c, t: True, trade=lambda t: None, p1=False, cap=2500, + auto=lambda t: True): + """Rows (t, commands, site target, loop reason, refused, refused_at, + unit refusals) and every action-log line as (t, line, hint). + + `plant(u, t, cmd)` is battery u's reading. `cheap(c, t)` opens the cheap + window. `trade(t)` is a SET claim at P_ARBITRAGE added to the claim list + (a writer above the loop), or None.""" + log, rows, now = [], [], [0.0] + real_power, real_tick = rig._Hass.power, rig.TICK + real_log, real_resolve, real_c = (Controller.log_event, app_main.resolve, + rig.Controller) + + def power(self, u): + return float(plant(u, self.t, self.cmd[u])) + + def log_event(self, msg, hint=None): + log.append((now[0], msg, hint)) + return real_log(self, msg, hint) + + def resolve(claims, **kw): + v = trade(now[0]) + if v is not None: + claims.append(Claim.set("arbitrage", P_ARBITRAGE, v, "trade")) + return real_resolve(claims, **kw) + + def before(c, t): + now[0] = t + if not getattr(c.hass, "_s107", False): + read = c.hass.number + + async def number(e, invert=False): + if e == "sensor.pnow": + return 0.05 if cheap(c, c.hass.t) else 0.30 + if e == "sensor.pavg": + return 0.20 + return await read(e, invert) + c.hass.number, c.hass._s107 = number, True + if p1: + s79._feed(c, t) + c.auto = auto(t) + + def on_cycle(c, t): + rows.append((t, tuple(c.hass.cmd[u] for u in "ab"[:n]), c.target, + c.loop_reason, c.batt_refused, c.batt_refused_at, + tuple(i.refused for i in c.inverters))) + + extra = {"price_now_entity": "sensor.pnow", + "price_avg_entity": "sensor.pavg"} + if maint: + extra.update({"maintenance_enabled": True, + "maintenance_start_hour": 11, + "maintenance_charge_w": 2000}) + if p1: + extra.update(s79.P1, peak_cap_w=cap) + db = os.path.join(tempfile.mkdtemp(prefix="s107-"), "tariff.db") + rig._Hass.power, rig.TICK = power, tick + Controller.log_event, app_main.resolve = log_event, resolve + rig.Controller = functools.partial(real_c, peak_db_path=db) + try: + _, c = rig.run(n, extra=extra, dur=dur, load=house, + soc=lambda u, t: 50.0, cut=lambda u, t: False, + before=before, on_cycle=on_cycle, + store=s79.store_charging(n) if maint else rig._Store()) + finally: + rig._Hass.power, rig.TICK = real_power, real_tick + Controller.log_event, app_main.resolve = real_log, real_resolve + rig.Controller = real_c + return rows, log, c + + +def full_a(u, t, cmd): + """A takes no charge (a full pack), its reading jittering by a few watts + as a real one does; B follows.""" + return -1.0 - int(t) % 3 if u == "a" and cmd < 0 else round(cmd) + + +def flat_a(u, t, cmd): + """`full_a` with a perfectly flat reading (the stale-reading gate trips).""" + return -1.0 if u == "a" and cmd < 0 else round(cmd) + + +def empty_a(u, t, cmd): + """A gives nothing (an empty pack), its reading jittering by a few watts + (SAFETY-73's jitter shape, so the stale-reading gate stays out of it); + B follows.""" + return -4.0 - int(t) % 3 if u == "a" and cmd >= -4.0 else round(cmd) + + +def lines(log, word="REFUSES"): + return [x for x in log if word in x[1]] + + +def ends(log): + return [x for x in log if "REFUSES ENDED" in x[1]] + + +def refused(r): + """Row r holds a refusal (site at N=1, any unit at N>1).""" + return bool(r[4]) or any(r[6]) + + +def held_from(rows): + """True when, once learned, the refusal is held on every later row.""" + first = next((k for k, r in enumerate(rows) if refused(r)), None) + return first is not None and all(refused(r) for r in rows[first:]) + + +# ============================================================================== +print("1. the card: a full pack under a maintenance charge, 15 min") +# ============================================================================== +for n, tick, house in ((n, k, h) for n in (1, 2) for k in (1.0, 2.0, 5.0) + for h in (800.0, -1500.0)): + rows, log, c = run(n, plant=full_a, house=lambda t: house, tick=tick) + ref = lines(log) + tag = f"N={n}, {tick:.0f} s meter, house {house:+.0f} W" + print(f" {tag}: {len(ref)} refusal line(s): " + + " / ".join(x[1][:60] for x in ref)) + check(f"{tag}: at most 2 refusal lines in 15 min (rig: 548 + 548)", + 1 <= len(ref) <= 2) + check(f"{tag}: no end line", not ends(log)) + check(f"{tag}: learned once and held to the end", held_from(rows)) + # N=1: the charge's ceiling stays on the wire. N=2: SAFETY-73 closes A + # in the refused direction, as before. + check(f"{tag}: still in its charge, the wire as before", + c.maints[0].phase == "charge" + and all(r[1][0] == (-2000.0 if n == 1 else 0.0) + for r in rows[-10:])) + # The hint under the buttons is set at most once for the refusal. + check(f"{tag}: the refusal hint is set once", + sum(1 for x in ref if x[2] is not None) == 1) + +# The probe clock: while the charge holds the wire no probe is tried (the +# loop's ask is not on the wire), so the action log stays quiet. +rows, log, _ = run(1, plant=full_a, house=lambda t: 800.0, dur=3600.0) +check("N=1, an hour: still one line, no probe tried under the charge", + len(lines(log)) == 1 + and not any(r[3] == control.PROBING for r in rows)) + +# ============================================================================== +print("\n2. other writers: a trade and the peak guard, no flap") +# ============================================================================== +# A trade (a SET above the loop) asks a full pack to charge for 15 min. +rows, log, _ = run(1, plant=full_a, house=lambda t: 800.0, maint=False, + trade=lambda t: -1500.0) +print(f" trade into a full pack: {len(lines(log))} line(s)") +check("a trade into a full pack: learned once, held, no end", + len(lines(log)) == 1 and not ends(log) and held_from(rows)) +# The peak guard asks an empty pack to give while the house is over the limit. +rows, log, c = run(1, plant=empty_a, house=lambda t: 4500.0, maint=False, + p1=True) +print(f" peak guard on an empty pack: {len(lines(log))} line(s), " + f"guard floor {c.peak_guard.floor_w if c.peak_guard else None}") +check("the peak guard on an empty pack: the guard holds the wire", + c.peak_guard is not None and c.peak_guard.engaged) +check("the peak guard on an empty pack: at most 2 lines, held to the end", + 1 <= len(lines(log)) <= 2 and held_from(rows)) + +# ============================================================================== +print("\n3. a refusal still ends") +# ============================================================================== +# a. Learned by the loop on an empty pack; at 300 s a trade really takes the +# wire and asks it to give 2000 W. The refusal was learned against the +# loop's ask: it ends, then is learned against the trade, once. +rows, log, _ = run(1, plant=empty_a, house=lambda t: 2000.0, maint=False, + trade=lambda t: 2000.0 if 300.0 <= t < 900.0 else None, + dur=1200.0) +e = ends(log) +print(" " + " / ".join(f"{x[0]:.0f} s {x[1][:45]}" for x in lines(log))) +check("a different writer takes over: the refusal ends, 'another writer'", + len(e) == 1 and "another writer" in e[0][1] and 300.0 < e[0][0] < 320.0) +check("...and is learned again against the trade, once, and held", + len(lines(log)) == 3 and held_from([r for r in rows if r[0] > 400.0])) +check("...and the loop keeps it when the trade leaves (no line)", + all(x[0] < 900.0 for x in lines(log)) and refused(nth(rows, -1))) +# The writer asks the other way, which the empty pack takes. +rows, log, _ = run(1, plant=empty_a, house=lambda t: 2000.0, maint=False, + trade=lambda t: -1500.0 if t >= 300.0 else None) +e = ends(log) +check("a writer that asks the other way ends it ('another writer')", + len(e) == 1 and "another writer" in e[0][1] and present(nth(rows, -1)) + and not refused(nth(rows, -1))) +# b. The full pack starts taking charge again under the same writer. +rows, log, _ = run(1, plant=lambda u, t, cmd: ( + round(cmd) if t >= 400.0 else full_a(u, t, cmd)), house=lambda t: 800.0) +e = ends(log) +print(" " + " / ".join(f"{x[0]:.0f} s {x[1][:45]}" for x in lines(log))) +check("the battery takes charge again: the refusal ends once, on its reading", + len(e) == 1 and "the battery gives power" in e[0][1] + and 400.0 <= e[0][0] < 430.0 and present(nth(rows, -1)) + and not refused(nth(rows, -1))) +# c. Stop and start still end it. +rows, log, _ = run(1, plant=full_a, house=lambda t: 800.0, + auto=lambda t: not 500.0 <= t < 520.0) +e = ends(log) +check("stopping control still ends it", any("control stopped" in x[1] + for x in e)) +# d. The probe clock. Learned under the charge; the cheap window closes when +# the refusal is 910 s old, and the loop holds the wire (a surplus, so it +# wants to charge). The first probe is a full PROBE_EVERY_S after the +# window closed, as before: never at once. +close = [None] + + +def window(c, t): + if close[0] is None and c.batt_refused: + close[0] = t + 910.0 + return close[0] is None or t < close[0] + + +rows, log, _ = run(1, plant=full_a, house=lambda t: -1500.0, cheap=window, + dur=3000.0) +probes = [r[0] for r in rows if r[3] == control.PROBING] +print(" " + " / ".join(f"{x[0]:.0f} s {x[1][:45]}" for x in lines(log))) +print(f" window closed at {close[0]} s, first probe at " + f"{probes[0] if probes else None} s") +check("the refusal carries over to the loop with no line", + len(lines(log)) == 1 and held_from(rows)) +check("the first probe after the writer left comes PROBE_EVERY_S later", + close[0] is not None and bool(probes) + and probes[0] >= close[0] + control.PROBE_EVERY_S - 10.0) + +# ============================================================================== +print("\n4. not covered: a perfectly flat reading under a writer") +# ============================================================================== +# The stale-reading gate trips on a flat reading while the command moves, +# stops the site and seeds the refusal (SAFETY-52 decision 3). After the stop +# the writer's ask comes back fresh, compute() has no saturation history +# against it yet, so the refusal ends at the first cycle and the next trip +# seeds it again: one pair per gate cycle, as on release/1.0 (the commands +# are the same; DECISIONS/SAFETY-107 "Not covered"). Pinned so a change shows. +rows, log, _ = run(1, plant=flat_a, house=lambda t: -1500.0) +trips = len(lines(log, "STALE inverter")) +print(f" flat reading, house -1500 W: {trips} gate trips, " + f"{len(lines(log))} refusal lines in 15 min") +check("flat reading: no per-cycle flap, at most one pair per gate trip", + trips >= 1 and len(lines(log)) <= 2 * trips) + +print() +if fails: + print(f"{len(fails)} FAILED: " + ", ".join(fails)) + sys.exit(1) +print("all SAFETY-107 checks passed") -- 2.54.0 From 848140c29f523f11269e2d10d793300723024341 Mon Sep 17 00:00:00 2001 From: goodwe-review Date: Fri, 9 Oct 2026 03:52:39 +0200 Subject: [PATCH 13/17] SAFETY-107 review: test a writer that turns round on a dead pack; say when rule 3 applies Co-Authored-By: Claude Opus 5.5 --- .../SAFETY-107-refusal-under-another-writer.md | 5 ++++- goodwe_controller/test_safety107.py | 11 +++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/goodwe_controller/DECISIONS/SAFETY-107-refusal-under-another-writer.md b/goodwe_controller/DECISIONS/SAFETY-107-refusal-under-another-writer.md index 40e427c..a3c1062 100644 --- a/goodwe_controller/DECISIONS/SAFETY-107-refusal-under-another-writer.md +++ b/goodwe_controller/DECISIONS/SAFETY-107-refusal-under-another-writer.md @@ -37,7 +37,10 @@ probe, exactly as before the change. Then: while another writer is; 2. it does not: the refusal ends, logged "another writer", or "the battery gives power" when the reading shows it (it was "another writer" before); -3. it finds the other direction: end, then a new start, as before. +3. it finds the other direction: end, then a new start, as before. In + practice 2 comes first: SAFETY-94's evidence restarts when the ask changes + sign, so a writer that turns round ends the refusal at once and the other + way is learned ~12 cycles later (`test_safety107.py` part 3e). **Why this rule.** The end now means what SAFETY-52 meant by it: the refusal no longer describes what is on the wire, judged by the same test that learns it. diff --git a/goodwe_controller/test_safety107.py b/goodwe_controller/test_safety107.py index 8f202b9..e3f7516 100644 --- a/goodwe_controller/test_safety107.py +++ b/goodwe_controller/test_safety107.py @@ -272,6 +272,17 @@ check("the refusal carries over to the loop with no line", check("the first probe after the writer left comes PROBE_EVERY_S later", close[0] is not None and bool(probes) and probes[0] >= close[0] + control.PROBE_EVERY_S - 10.0) +# e. A writer turns round on a dead pack with no gap. The evidence restarts +# on the new sign (SAFETY-94), so the old refusal ends at once and the +# other way is learned once, ~12 cycles later. +rows, log, _ = run(1, plant=lambda u, t, cmd: ( + -2.0 - int(t) % 3 if u == "a" else round(cmd)), house=lambda t: 2000.0, + maint=False, trade=lambda t: 2000.0 if t < 300.0 else -2000.0) +e = ends(log) +print(" " + " / ".join(f"{x[0]:.0f} s {x[1][:45]}" for x in lines(log))) +check("a writer turns round on a dead pack: one end, then the other way", + len(lines(log)) == 3 and len(e) == 1 and 300.0 < e[0][0] < 400.0 + and present(nth(rows, -1)) and nth(rows, -1)[4] == -1) # ============================================================================== print("\n4. not covered: a perfectly flat reading under a writer") -- 2.54.0 From cd1bf2a694d5368c88c74808e45f8d37a9c97747 Mon Sep 17 00:00:00 2001 From: admin Date: Fri, 9 Oct 2026 23:56:29 +0200 Subject: [PATCH 14/17] MAINT-15: the balancing hold ends on time with no SoC reading HOLD's time check moves into Maintenance._hold(), used by step() and _no_soc(). A hold that loses its SoC now ends at hold_min, writes last_completed and logs the same COMPLETE line. The 15 min HOLD line says the hold still ends at its normal time. Behaviour with a reading is unchanged (differential trace against db9e231). Co-Authored-By: Claude Opus 5.5 --- .../MAINT-14-no-soc-in-maintenance.md | 5 +- .../MAINT-15-hold-ends-without-soc.md | 43 +++ goodwe_controller/app/maintenance.py | 35 ++- goodwe_controller/test_maint14.py | 6 +- goodwe_controller/test_maint15.py | 255 ++++++++++++++++++ 5 files changed, 324 insertions(+), 20 deletions(-) create mode 100644 goodwe_controller/DECISIONS/MAINT-15-hold-ends-without-soc.md create mode 100644 goodwe_controller/test_maint15.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 index 0ffa494..71cd057 100644 --- a/goodwe_controller/DECISIONS/MAINT-14-no-soc-in-maintenance.md +++ b/goodwe_controller/DECISIONS/MAINT-14-no-soc-in-maintenance.md @@ -58,10 +58,7 @@ allows discharge. ## 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 is card MAINT-15. The "no battery level" line in - HOLD says only that the hold waits for the level. +- **HOLD with no SoC**: closed by MAINT-15 (`MAINT-15-hold-ends-without-soc.md`). - 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/DECISIONS/MAINT-15-hold-ends-without-soc.md b/goodwe_controller/DECISIONS/MAINT-15-hold-ends-without-soc.md new file mode 100644 index 0000000..2230603 --- /dev/null +++ b/goodwe_controller/DECISIONS/MAINT-15-hold-ends-without-soc.md @@ -0,0 +1,43 @@ +# MAINT-15 - the balancing hold ends on time with no SoC reading + +**Status: for review.** Tests: `test_maint15.py`. No option added. + +**The fault.** `step()` sent every tick with no SoC to `_no_soc()`, and for +HOLD that returned 0 W without the time check. A hold that lost its reading +never ended: `last_completed` was never written and the next due date never +moved. Harmless for power (0 W), but the cycle was stuck. This was left open +in MAINT-14. + +## Rule + +1. **HOLD ends at `hold_min` whether or not there is a reading.** One helper, + `Maintenance._hold()`, holds the time check and is used by both paths. When + the time is up it writes `last_completed`, goes IDLE and logs the same + "cycle COMPLETE" line. Before the time is up it commands 0 W as before. +2. **Text.** The 15-min "no battery level" line in HOLD now reads "The hold + still ends at its normal time." Still once per episode. + +## Why + +HOLD is reached only after the charge counted as done. Every entry into HOLD +is in `step()` or `_no_soc()` and follows a charge that reached the target, +stalled near full (MAINT-13) or timed out near full (MAINT-13/14). Nothing else +sets the phase to HOLD (`_enter(HOLD` has four call sites, all after a done +charge). The hold is a wait for the BMS to balance; it never needed the level. +So a hold that ends with no reading is a completed cycle. The "cycle COMPLETE" +line is true without a reading: it names minutes of hold, not a level. + +## Cost + +- If the level was lost during the hold, the cycle is recorded as complete + without anyone seeing the final level. The charge had counted as done, so this + is the same trust MAINT-14 already gives a charge that ends with no reading. + +## What this does not cover + +- It does not check that the pack is still full at the end of the hold. +- It does not say why the reading was lost. +- With a reading, nothing changes (proved by a differential trace against + release/1.0 in `test_maint15.py`). +- A state file written by an older version with `phase: hold` loads and ends on + time; the file is not changed in format. diff --git a/goodwe_controller/app/maintenance.py b/goodwe_controller/app/maintenance.py index fc7ecb5..d38b205 100644 --- a/goodwe_controller/app/maintenance.py +++ b/goodwe_controller/app/maintenance.py @@ -541,11 +541,29 @@ class Maintenance: events.append(msg) return MaintResult(IDLE, True, 0.0, False, events, failed=kind) + def _hold(self, now: datetime, events: list) -> MaintResult: + """HOLD for one tick, with or without a SoC reading (MAINT-15). + + HOLD needs no level to end: it is reached only after the charge counted + as done (MAINT-13/14), so its end is a time check and nothing else. + """ + elapsed = self._elapsed_min(now) + if elapsed >= self.cfg.hold_min: + self.store.set_time("last_completed", now) + self._enter(IDLE, now, events, + f"maintenance: cycle COMPLETE ({elapsed:.0f} min hold) " + "- handing back to the loop") + return MaintResult(IDLE, False, None, False, events) + # 0 W, not "do nothing": commanding zero is what stops the loop + # pulling the pack straight back down the moment it is full, which + # is exactly when the BMS needs it parked at the top. + return MaintResult(HOLD, True, 0.0, False, events) + 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 + HOLD stays at 0 W and ends at its normal time (MAINT-15). 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). @@ -563,7 +581,7 @@ 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." + tail = ("The hold still ends at its normal time." if phase == HOLD else f"The time limit of the {phase} still applies.") msg = (f"maintenance: no battery level for {NOSOC_NOTE_MIN} min. " @@ -571,7 +589,7 @@ class Maintenance: _LOG.warning("%s", msg) events.append(msg) if phase == HOLD: - return MaintResult(HOLD, True, 0.0, False, events) + return self._hold(now, events) elapsed = self._elapsed_min(now) if phase == DRAIN: if elapsed > DRAIN_TIMEOUT_MIN: @@ -736,16 +754,7 @@ class Maintenance: return MaintResult(CHARGE, False, None, True, events) if phase == HOLD: - if elapsed >= self.cfg.hold_min: - self.store.set_time("last_completed", now) - self._enter(IDLE, now, events, - f"maintenance: cycle COMPLETE ({elapsed:.0f} min hold) " - "- handing back to the loop") - return MaintResult(IDLE, False, None, False, events) - # 0 W, not "do nothing": commanding zero is what stops the loop - # pulling the pack straight back down the moment it is full, which - # is exactly when the BMS needs it parked at the top. - return MaintResult(HOLD, True, 0.0, False, events) + return self._hold(now, events) _LOG.error("maintenance: unknown phase %r, forcing idle", phase) events += self.abort(now, f"unknown phase {phase!r}") diff --git a/goodwe_controller/test_maint14.py b/goodwe_controller/test_maint14.py index 8296610..c957eb1 100644 --- a/goodwe_controller/test_maint14.py +++ b/goodwe_controller/test_maint14.py @@ -245,8 +245,8 @@ 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]) +check("HOLD: the line exists and says the hold ends at its normal time (MAINT-15)", + len(note) == 1 and "normal time" in note[0] and "waits" not 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)) @@ -268,7 +268,7 @@ _, _, 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) + and sum("normal time" in e for e in ev) == 1) print("6. restart while the SoC is missing") with tempfile.TemporaryDirectory() as d: diff --git a/goodwe_controller/test_maint15.py b/goodwe_controller/test_maint15.py new file mode 100644 index 0000000..ec5e8ee --- /dev/null +++ b/goodwe_controller/test_maint15.py @@ -0,0 +1,255 @@ +"""MAINT-15: the HOLD ends on time with no SoC reading. +`python3 test_maint15.py` + +Graded here: a HOLD that loses its SoC (at the start, mid-hold, just before +hold_min) ends on time, writes last_completed and says the COMPLETE line; SoC +back mid-hold changes nothing; HOLD stays 0 W; one pack's loss leaves the other +pack alone; a restart in HOLD with no SoC and an old state file. +See DECISIONS/MAINT-15-hold-ends-without-soc.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 + + + +HOLD_MIN = CFG.hold_min + + +def in_hold(store=None, unit=None, at=T0): + """A pack in HOLD that started at `at` + 1 min (charge counted as done).""" + m = in_charge(store, unit, at) + m.tick(at + MIN, 100.0) + assert m.phase == HOLD + return m + + +H0 = T0 + MIN # the hold's start +print("1. SoC lost in HOLD: the hold ends on time") +for label, lost_at in (("at the start", 0), ("mid-hold", 60), + ("just before hold_min", HOLD_MIN - 1)): + m = in_hold() + run(m, H0, lost_at, 100.0) if lost_at else None + res, _, out, ev = run(m, H0 + lost_at * MIN, HOLD_MIN + 5 - lost_at, None) + check(f"lost {label}: ends at hold_min, to IDLE, not failed", + out is not None and out[0].phase == IDLE and out[0].failed == "" + and out[1] == H0 + HOLD_MIN * MIN) + check("...last_completed is written at that time", + m.store.get_time("last_completed") == H0 + HOLD_MIN * MIN) + check("...the same COMPLETE line, once, and no last_failed", + sum("cycle COMPLETE (120 min hold)" in e for e in ev) == 1 + and m.store.get_time("last_failed") is None) + check("...the pack is idle and hands the setpoint back", + m.phase == IDLE and not out[0].owns_setpoint) + +m = in_hold() +res, _, out, _ = run(m, H0, HOLD_MIN - 1, None) +check("before hold_min: still HOLD at 0 W, owned, nothing written", + out is None and res.phase == HOLD and res.owns_setpoint + and res.setpoint_w == 0.0 and m.store.get_time("last_completed") is None) +check("...the pack is not due again (next due date moved after the end)", + (run(m, H0 + (HOLD_MIN - 1) * MIN, 3, None), m.due(H0 + 130 * MIN))[1] is False) + +print("2. the text") +m = in_hold() +_, _, out, ev = run(m, H0, HOLD_MIN + 5, None) +note = [e for e in ev if "no battery level for" in e] +check("one 15 min line, saying the hold ends at its normal time", + len(note) == 1 and "normal time" in note[0] and "waits" not in note[0] + and f"{NOSOC_NOTE_MIN} min" in note[0]) +check("...and the other events are the one COMPLETE line only", len(ev) == 2) +m = in_hold() +_, _, _, ev = run(m, H0, NOSOC_NOTE_MIN - 2, None) +check("under the threshold the line is not said", ev == []) + +print("3. SoC comes back mid-hold") +ends = {} +for label, soc in (("no SoC", lambda i: None if i < 40 else 100.0), + ("SoC the whole time", 100.0), + ("a low SoC after the gap", lambda i: None if i < 40 else 55.0)): + m = in_hold() + res, _, out, ev = run(m, H0, HOLD_MIN + 5, soc) + ends[label] = (out[1], m.store.get_time("last_completed")) if out else None + check(f"{label}: ends at hold_min", out is not None + and out[1] == H0 + HOLD_MIN * MIN) +check("...the end time is the same with or without the gap", + len(set(ends.values())) == 1) +m = in_hold() +run(m, H0, 40, None) +res, _, out, _ = run(m, H0 + 40 * MIN, 30, 100.0) +check("back mid-hold: still HOLD at 0 W until the time is up", + out is None and res.phase == HOLD and res.setpoint_w == 0.0) + +print("4. restart in HOLD with no SoC") +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) + m.tick(H0, 100.0) + run(m, H0, 50, None) + m2 = Maintenance(CFG, Store(path)) + r = m2.tick(H0 + 51 * MIN, None) + check("a fresh process reads HOLD and holds 0 W", m2.phase == HOLD + and r.phase == HOLD and r.owns_setpoint and r.setpoint_w == 0.0) + res, _, out, _ = run(m2, H0 + 52 * MIN, HOLD_MIN, None) + check("...and the hold ends at hold_min from the hold start, not the restart", + out and out[0].phase == IDLE and out[1] == H0 + HOLD_MIN * MIN + and m2.store.get_time("last_completed") == H0 + HOLD_MIN * MIN) + import json + old = {"phase": "hold", "phase_started": H0.isoformat(), + "last_completed": None, "last_start_attempt": None, "auto": True} + p2 = os.path.join(d, "old.json") + json.dump(old, open(p2, "w")) + mo = Maintenance(CFG, Store(p2)) + r = mo.tick(H0 + 10 * MIN, None) + check("an old state file in HOLD loads and holds 0 W", mo.phase == HOLD + and r.setpoint_w == 0.0) + res, _, out, _ = run(mo, H0 + 11 * MIN, HOLD_MIN, None) + check("...and ends on time, writing last_completed", + out and out[1] == H0 + HOLD_MIN * MIN + and mo.store.get_time("last_completed") == H0 + HOLD_MIN * MIN) + mr = Maintenance(CFG, Store(p2)) + check("...the written file reloads as a finished cycle", + mr.phase == IDLE and mr.store.get_time("last_completed") is not None) + +print("5. two packs") +a, b = (Maintenance(CFG, Mem(), unit="inverter-a", name="inverter-a"), + Maintenance(CFG, Mem(), unit="inverter-b", name="inverter-b")) +a_ok, b_ok = (Maintenance(CFG, Mem(), unit="inverter-a", name="inverter-a"), + Maintenance(CFG, Mem(), unit="inverter-b", name="inverter-b")) +for x in (a, a_ok): + x.force_start(T0) + x.tick(T0, 11.0) + x.tick(H0, 100.0) +# B is idle; A's hold loses its SoC (twin site: A keeps it) +same, a_end = True, None +for i in range(0, HOLD_MIN + 10): + t = H0 + i * MIN + ra = a.tick(t, None) + rb = b.tick(t, 60.0) + ra2 = a_ok.tick(t, 100.0) + rb2 = b_ok.tick(t, 60.0) + same = same and (rb.phase, rb.owns_setpoint, rb.setpoint_w) == \ + (rb2.phase, rb2.owns_setpoint, rb2.setpoint_w) and b.phase == b_ok.phase + if a_end is None and a.phase == IDLE: + a_end = t +check("A loses SoC in HOLD: A ends on time (as the twin with a reading)", + a_end == H0 + HOLD_MIN * MIN and a_ok.phase == IDLE + and a.store.get_time("last_completed") == a_ok.store.get_time("last_completed")) +check("...B is the same as in the twin site where A kept its SoC", same) +# B in its own HOLD with SoC, A in HOLD without +a, b = (Maintenance(CFG, Mem(), unit="inverter-a", name="inverter-a"), + Maintenance(CFG, Mem(), unit="inverter-b", name="inverter-b")) +a.force_start(T0); a.tick(T0, 11.0); a.tick(H0, 100.0) +b_start = H0 + 30 * MIN +b.force_start(b_start - MIN); b.tick(b_start - MIN, 11.0); b.tick(b_start, 100.0) +b_end = None +for i in range(0, HOLD_MIN + 40): + t = H0 + i * MIN + a.tick(t, None) + if t >= b_start: + b.tick(t, 100.0) + if b_end is None and b.phase == IDLE: + b_end = t +check("B (SoC present) ends at its own hold_min, A's loss changes nothing", + b_end == b_start + HOLD_MIN * MIN and a.phase == IDLE + and b.store.get_time("last_completed") == b_start + HOLD_MIN * MIN) +check("...A's end is not delayed by B", a.store.get_time("last_completed") + == H0 + HOLD_MIN * MIN) + +print("6. other phases unchanged") +m = in_drain() +res, _, out, _ = run(m, T0, 60, None) +check("DRAIN with no SoC still holds 0 W and does not end", out is None + and res.phase == DRAIN and res.setpoint_w == 0.0) +m = in_charge() +res, _, out, _ = run(m, T0, 600, None) +check("CHARGE with no SoC still keeps charging", out is None + and res.phase == CHARGE and res.charge_only) +m = in_hold() +res, _, out, _ = run(m, H0, 30, 100.0) +check("HOLD with a reading is unchanged: 0 W, ends at hold_min", out is None + and res.setpoint_w == 0.0) + +print() +if fails: + print(f"{len(fails)} FAILED:") + for f in fails: + print(" -", f) + sys.exit(1) +print("all ok") -- 2.54.0 From 35b2a699de816dde5b3b2aa8877107a7cb0cb206 Mon Sep 17 00:00:00 2001 From: admin Date: Sat, 10 Oct 2026 00:42:01 +0200 Subject: [PATCH 15/17] MAINT-15 round 2: in-repo differential check, full-hold with-reading check Co-Authored-By: Claude Opus 5.5 --- .../MAINT-15-hold-ends-without-soc.md | 8 ++- goodwe_controller/test_maint15.py | 56 +++++++++++++++++-- 2 files changed, 56 insertions(+), 8 deletions(-) diff --git a/goodwe_controller/DECISIONS/MAINT-15-hold-ends-without-soc.md b/goodwe_controller/DECISIONS/MAINT-15-hold-ends-without-soc.md index 2230603..3da9bc0 100644 --- a/goodwe_controller/DECISIONS/MAINT-15-hold-ends-without-soc.md +++ b/goodwe_controller/DECISIONS/MAINT-15-hold-ends-without-soc.md @@ -37,7 +37,11 @@ line is true without a reading: it names minutes of hold, not a level. - It does not check that the pack is still full at the end of the hold. - It does not say why the reading was lost. -- With a reading, nothing changes (proved by a differential trace against - release/1.0 in `test_maint15.py`). +- With a reading, nothing changes. `test_maint15.py` section 7 compares the new + HOLD tick by tick with a frozen copy of the release/1.0 HOLD branch (phase, + setpoint, owner, events, `last_completed`) at four `hold_min` values. A wider + trace of the whole cycle (800 seeded runs against db9e231) was run outside the + repo; its script and log are kept under `scratch/MAINT-15/` of the dev + workspace, not in this repository. - A state file written by an older version with `phase: hold` loads and ends on time; the file is not changed in format. diff --git a/goodwe_controller/test_maint15.py b/goodwe_controller/test_maint15.py index ec5e8ee..8e0985f 100644 --- a/goodwe_controller/test_maint15.py +++ b/goodwe_controller/test_maint15.py @@ -16,9 +16,7 @@ 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, + CHARGE, DRAIN, HOLD, IDLE, NOSOC_NOTE_MIN, MaintConfig, Maintenance, ) from app.store import DEFAULTS, Store @@ -241,10 +239,56 @@ m = in_charge() res, _, out, _ = run(m, T0, 600, None) check("CHARGE with no SoC still keeps charging", out is None and res.phase == CHARGE and res.charge_only) + + +def old_hold_branch(phase_started, last_completed, now, hold_min): + """FROZEN copy of release/1.0 (db9e231) HOLD branch of step(), with a + reading: returns (phase, owns_setpoint, setpoint_w, events, + last_completed). Kept as a reference for the differential check below.""" + elapsed = (now - phase_started).total_seconds() / 60.0 + if elapsed >= hold_min: + return (IDLE, False, None, + [f"maintenance: cycle COMPLETE ({elapsed:.0f} min hold) " + "- handing back to the loop"], now) + return (HOLD, True, 0.0, [], last_completed) + + +print("7. with a reading: the new HOLD against the frozen release/1.0 branch") +for hm in (0, 5, 60, 120): + cfg = MaintConfig(enabled=True, interval_days=28, start_hour=19, + discharge_w=2000, charge_w=2000, soc_floor=11, + soc_target=100, hold_min=hm) + m = Maintenance(cfg, Mem()) + m.force_start(T0) + m.tick(T0, 11.0) + m.tick(H0, 100.0) + started, last, same, end = H0, None, True, None + for i in range(0, hm + 15): + t = H0 + i * MIN + if m.phase != HOLD: + break + want = old_hold_branch(started, last, t, hm) + r = m.tick(t, 100.0 - (i % 3)) + got = (r.phase, r.owns_setpoint, r.setpoint_w, r.events, + m.store.get_time("last_completed")) + same = same and got == want + last = want[4] + if r.phase == IDLE: + end = t + check(f"hold_min {hm}: every tick identical (phase, owns, W, events, " + "last_completed)", same) + check("...ends at hold_min, last_completed = that tick, one COMPLETE line", + end == H0 + hm * MIN and m.store.get_time("last_completed") == end + and m.phase == IDLE) + m = in_hold() -res, _, out, _ = run(m, H0, 30, 100.0) -check("HOLD with a reading is unchanged: 0 W, ends at hold_min", out is None - and res.setpoint_w == 0.0) +res, _, out, ev = run(m, H0, HOLD_MIN + 5, 100.0) +check("HOLD with a reading: 0 W until the end, then IDLE at hold_min", + out is not None and out[1] == H0 + HOLD_MIN * MIN and out[0].phase == IDLE) +check("...last_completed is that tick and the COMPLETE line is said once", + m.store.get_time("last_completed") == H0 + HOLD_MIN * MIN + and ev == ["maintenance: cycle COMPLETE (120 min hold) " + "- handing back to the loop"]) print() if fails: -- 2.54.0 From 35432ce0659647b5700ce577920e94913eb6ec3a Mon Sep 17 00:00:00 2001 From: glenn schrooyen Date: Sat, 10 Oct 2026 23:29:13 +0200 Subject: [PATCH 16/17] Release 0.17.0 Bump version to 0.17.0 and add the closed 0.17.0 CHANGELOG section. No code change, no option change. Co-Authored-By: Claude Opus 5.5 --- goodwe_controller/CHANGELOG.md | 45 ++++++++++++++++++++++++++++++++++ goodwe_controller/config.yaml | 2 +- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/goodwe_controller/CHANGELOG.md b/goodwe_controller/CHANGELOG.md index 849271a..a3b4720 100644 --- a/goodwe_controller/CHANGELOG.md +++ b/goodwe_controller/CHANGELOG.md @@ -1,5 +1,50 @@ # Changelog +## 0.17.0 + +**A minor bump: changed behaviour, and no option changes.** No option was +added, removed or renamed, and no default changed (83 options). Options you +saved work as before. The changes below apply with one inverter or two, unless +a line says "Two inverters". + +**Maintenance with no battery level.** This release changes how battery +maintenance ends when the battery level is missing (MAINT-13, MAINT-14, +MAINT-15). It now ends on its own time limits instead of waiting for the level +to come back; you do not need to change any option. + +Contents of this release, in merge order since 0.16.0 (`451729c`): + +| | | +|---|---| +| `b527e99` | Two inverters: after a drain ends, the other battery no longer swings on old readings. In the test, it held steady for 23 s where 0.16.0 moved it by 720 W ([[SAFETY-103]]) | +| `8a17711` | A full battery that stops at 98 or 99 % no longer counts as a failed charge. After 60 min with no new high, the charge counts as done; a charge that truly fails now waits 7 days and shows one notice ([[MAINT-13]]) | +| `bf3ac79` | With no battery level, a maintenance charge still ends at its time limit and a drain waits at 0 W. The log no longer repeats a line every second ([[MAINT-14]]) | +| `db9e231` | One inverter: when maintenance charges a battery that is already full, the log no longer repeats "The battery refuses to give power" every second. In the test, 1 line in 16 min where 0.16.0 wrote about 1,700; the power does not change ([[SAFETY-107]]) | +| `580c200` | The balancing hold after a charge now ends on time when the battery level is missing. Before, it waited until the level came back ([[MAINT-15]]) | + +### Known issues + +- A false "Battery is not keeping up" can still show when the peak guard pauses a maintenance charge ([[SAFETY-92]]). +- A battery capped at a low limit with a perfectly flat reading can still trip the stale check ([[SAFETY-74]]). +- A battery whose reading at rest is beyond about ±170 W is still not caught when it refuses, as in 0.14.0 ([[SAFETY-97]]). +- The peak guard can still back off near the end of a 15-minute period when a meter reading is late, by less than 30 s ([[SAFETY-99]]). +- Two inverters: the maintenance charge can switch on and off while the house is above the peak aim ([[SAFETY-88]]). +- Two inverters: the "circulating" figure counts a battery's reading at rest ([[SAFETY-98]]). +- Two inverters: a drain that starts during a big step in house use can send the other battery to full power, and about 1 kW goes to the grid for about 10 s ([[SAFETY-104]]). +- Two inverters: when a battery comes back during a maintenance charge, the grid can step by several kW at once ([[SAFETY-101]]). +- Two inverters: when a battery drops out during a solar surplus, the others can swing by up to 1.5 kW ([[SAFETY-105]]). +- Two inverters: after a drain ends, a change in house use can go unanswered for about 9 s, because some readings are old ([[SAFETY-106]]). +- Two inverters: when one battery's level is missing, both batteries are held at 0 W, and for the first 15 s the last command stays on ([[SAFETY-108]]). +- Two inverters: if one battery's level stays missing for 12 h, the other battery's cycle can be marked failed, with a 7-day wait and a notice, though it is healthy ([[MAINT-17]]). +- One inverter: after a link drop, the battery's own safety stop to 0 W can be taken as a refusal, and the site stays at 0 W for up to 15 min ([[SAFETY-102]]). + +### If you are crossing from a version older than 0.10.0 + +`## 0.10.0` below can refuse an old config: `slew_w` above its limit, and the +withdrawn `price_provider` ids `engie_dynamic`, `totalenergies_dynamic` and +`eneco_eddy`. Read it before you update. Run `tools/predeploy_check.py` with the +bare options (`jq '.data.options'`) first. + ## 0.16.0 **A minor bump: changed behaviour, no option added or removed, and one default diff --git a/goodwe_controller/config.yaml b/goodwe_controller/config.yaml index 88ca8e8..3ccdcc1 100644 --- a/goodwe_controller/config.yaml +++ b/goodwe_controller/config.yaml @@ -1,5 +1,5 @@ name: GoodWe RS485 Controller -version: "0.16.0" +version: "0.17.0" slug: goodwe_controller description: >- Drives a GoodWe ES/BP battery inverter over RS485 by emulating its smart -- 2.54.0 From 808ec04dc47cb7da4e02c7e6322ceeca4ce88f1a Mon Sep 17 00:00:00 2001 From: goodwe-review Date: Sun, 11 Oct 2026 00:21:39 +0200 Subject: [PATCH 17/17] REL-0.17.0 review: CHANGELOG wording fixes SAFETY-107 row names the tested base; MAINT-17 says code reading only; SAFETY-105 figure is from a simulation; SAFETY-97 edge is 175 W; the maintenance note cites MAINT-14 and MAINT-15; SAFETY-108 "about 15 s". Co-Authored-By: Claude Opus 5.5 --- goodwe_controller/CHANGELOG.md | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/goodwe_controller/CHANGELOG.md b/goodwe_controller/CHANGELOG.md index a3b4720..f407c42 100644 --- a/goodwe_controller/CHANGELOG.md +++ b/goodwe_controller/CHANGELOG.md @@ -7,10 +7,9 @@ added, removed or renamed, and no default changed (83 options). Options you saved work as before. The changes below apply with one inverter or two, unless a line says "Two inverters". -**Maintenance with no battery level.** This release changes how battery -maintenance ends when the battery level is missing (MAINT-13, MAINT-14, -MAINT-15). It now ends on its own time limits instead of waiting for the level -to come back; you do not need to change any option. +**Maintenance with no battery level.** When the battery level is missing, +battery maintenance now ends on its own time limits instead of waiting for the +level to come back (MAINT-14, MAINT-15). You do not need to change any option. Contents of this release, in merge order since 0.16.0 (`451729c`): @@ -19,23 +18,23 @@ Contents of this release, in merge order since 0.16.0 (`451729c`): | `b527e99` | Two inverters: after a drain ends, the other battery no longer swings on old readings. In the test, it held steady for 23 s where 0.16.0 moved it by 720 W ([[SAFETY-103]]) | | `8a17711` | A full battery that stops at 98 or 99 % no longer counts as a failed charge. After 60 min with no new high, the charge counts as done; a charge that truly fails now waits 7 days and shows one notice ([[MAINT-13]]) | | `bf3ac79` | With no battery level, a maintenance charge still ends at its time limit and a drain waits at 0 W. The log no longer repeats a line every second ([[MAINT-14]]) | -| `db9e231` | One inverter: when maintenance charges a battery that is already full, the log no longer repeats "The battery refuses to give power" every second. In the test, 1 line in 16 min where 0.16.0 wrote about 1,700; the power does not change ([[SAFETY-107]]) | +| `db9e231` | One inverter: when maintenance charges a battery that is already full, the log no longer repeats "The battery refuses to give power" every second. In the test, 1 line in 16 min where the code before this fix wrote about 1,700 in 15 min; the power does not change ([[SAFETY-107]]) | | `580c200` | The balancing hold after a charge now ends on time when the battery level is missing. Before, it waited until the level came back ([[MAINT-15]]) | ### Known issues - A false "Battery is not keeping up" can still show when the peak guard pauses a maintenance charge ([[SAFETY-92]]). - A battery capped at a low limit with a perfectly flat reading can still trip the stale check ([[SAFETY-74]]). -- A battery whose reading at rest is beyond about ±170 W is still not caught when it refuses, as in 0.14.0 ([[SAFETY-97]]). +- A battery whose reading at rest is beyond about ±175 W is still not caught when it refuses ([[SAFETY-97]]). - The peak guard can still back off near the end of a 15-minute period when a meter reading is late, by less than 30 s ([[SAFETY-99]]). - Two inverters: the maintenance charge can switch on and off while the house is above the peak aim ([[SAFETY-88]]). - Two inverters: the "circulating" figure counts a battery's reading at rest ([[SAFETY-98]]). - Two inverters: a drain that starts during a big step in house use can send the other battery to full power, and about 1 kW goes to the grid for about 10 s ([[SAFETY-104]]). - Two inverters: when a battery comes back during a maintenance charge, the grid can step by several kW at once ([[SAFETY-101]]). -- Two inverters: when a battery drops out during a solar surplus, the others can swing by up to 1.5 kW ([[SAFETY-105]]). +- Two inverters: when a battery drops out during a solar surplus, the others can swing by up to 1.5 kW in a simulation ([[SAFETY-105]]). - Two inverters: after a drain ends, a change in house use can go unanswered for about 9 s, because some readings are old ([[SAFETY-106]]). -- Two inverters: when one battery's level is missing, both batteries are held at 0 W, and for the first 15 s the last command stays on ([[SAFETY-108]]). -- Two inverters: if one battery's level stays missing for 12 h, the other battery's cycle can be marked failed, with a 7-day wait and a notice, though it is healthy ([[MAINT-17]]). +- Two inverters: when one battery's level is missing, both batteries are held at 0 W, and for about 15 s the last command stays on ([[SAFETY-108]]). +- Two inverters: if one battery's level stays missing for 12 h, the other battery's cycle can be marked failed, with a 7-day wait and a notice, though it is healthy. This comes from reading the code; it has not been seen on a site ([[MAINT-17]]). - One inverter: after a link drop, the battery's own safety stop to 0 W can be taken as a refusal, and the site stays at 0 W for up to 15 min ([[SAFETY-102]]). ### If you are crossing from a version older than 0.10.0 -- 2.54.0