From e76c70ecb0f3f3829dba853a03ac4f31cd2d3bc8 Mon Sep 17 00:00:00 2001 From: glenn schrooyen Date: Tue, 20 Oct 2026 00:52:24 +0200 Subject: [PATCH] SAFETY-119 round 2: one settle window per run of hand-backs A hand-back used to restart the window, so a guard and a charge taking turns hid the loop's integrator bound for good (0 warnings in 2 h; 28 before the fix). A run of hand-backs (the wire away less than a settle between them) now hides the bound for at most 11 bound cycles and 22 s in all, then it counts. The quarter-edge kick stays hidden. No command or text changes. Tests for the two reviewer mutants that only unit checks caught (a restart counted as a hand-back; span only), DECISIONS gives the delay at 2 s, 5 s and 60 s. test_safety87.py:543 shortened. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC --- .../SAFETY-119-bound-after-hand-back.md | 41 +++- goodwe_controller/app/main.py | 51 +++-- goodwe_controller/test_safety119.py | 216 ++++++++++++++++-- goodwe_controller/test_safety87.py | 5 +- 4 files changed, 269 insertions(+), 44 deletions(-) diff --git a/goodwe_controller/DECISIONS/SAFETY-119-bound-after-hand-back.md b/goodwe_controller/DECISIONS/SAFETY-119-bound-after-hand-back.md index 49f3a5c..87399f7 100644 --- a/goodwe_controller/DECISIONS/SAFETY-119-bound-after-hand-back.md +++ b/goodwe_controller/DECISIONS/SAFETY-119-bound-after-hand-back.md @@ -17,21 +17,42 @@ until the plant's settle has passed (`SHORT_CYCLES` cycles and counts as before. An over-rating (`clamped`, a bug) on a cycle the loop decided always counts. No command changes, and no text was added or changed. +**One window per run of hand-backs (round 2).** A run is hand-backs with the +wire away less than a settle (12 cycles and 22 s) between them. It ends when +the loop holds the wire, or the wire is away, a whole settle in a row. Each +hand-back restarts the settle, but a run hides the bound for at most 11 bound +cycles and 22 s in all. Round 1 restarted the window at every hand-back, so +a guard and a charge taking turns hid the bound for good (0 warnings in 2 h +where 28 were raised before). Now a bound that stays warns after at most 11 +of its cycles, however often the wire changes hands. + +**Delay.** The first counted bound cycle, from the hand-back (worst case): + +| meter | one hand-back | repeated hand-backs | +|---|---|---| +| 2 s | 22 s | 264 s | +| 5 s (default) | 55 s | 660 s | +| 60 s | 660 s | 7920 s | + +One hand-back: 11 cycles, never less than 22 s. Repeated: the worst case is +one bound cycle per turn with the wire away 11 cycles between (22 at 1 s), +so 11 turns. It grows with the meter; it stays under a quarter (900 s) up +to a 5 s meter. +Measured: N=2, 5 s meter, cheap-window charge, no usable forecast, house 50 W +over the 5000 W rating: 14 warnings in 2 h (two in each quarter after the first), 0 with +round 1, 28 without any rule. The cap held. + **Kept.** The loop in charge at its rail warns on the same cycle as before. -A loop still at its rail after a hand-back warns 11 cycles later (at least -22 s): 22 s at a 2 s meter, 55 s at 5 s, 11 min at a 60 s cycle. The start -and a restart after a stop are not hand-backs. +The start and a restart after a stop are not hand-backs. The quarter-edge +kick of `test_safety119.py` part 1 stays hidden (0 warnings in 2 h). **Re-pinned.** test_safety84 (all six) and test_safety87 (`n1_crossing`, `n1_above`, `n1_charge_low`): only OUTPUT_CLAMP log rows differ; commands and -claims are the same (`test_safety119.py` part 5). +claims are the same (`test_safety119.py` part 6). -**Open.** Each hand-back restarts the window, so a loop that loses the wire -again within 11 cycles is never counted. Seen in review: N=2 (2 x 2500 W), a -5 s meter, a cheap-window charge with no usable forecast, a house 50-100 W -over the 5000 W rating. The guard and the charge took the wire back at least -every 11 cycles. 0 notifications in 2 h, 28 without this rule. The cap held. -From 200 W over the rating it warns at least once a quarter. +**Not hidden.** A battery that gives less than another owner told it still +alarms through SAFETY-85 rule 2. "Always counts" for an over-rating holds on +cycles the loop decided; on others SAFETY-85 rule 1 already dropped it. **Not changed.** The kick itself is control behaviour: for a few seconds at a quarter's edge the loop asks up to its rail. In `test_safety119.py` part 1 diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index 98a7fc4..c4a2daa 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -299,6 +299,10 @@ REFUSED_PROGRESS_W = _ctl.PROBE_ANSWER_W / 2 # was told over the same span as above (the plant's measured settle time), so # a healthy battery still settling on a new command is not short. SHORT_CYCLES, SHORT_SPAN_S = _ctl.SETTLE_CYCLES, REFUSED_SPAN_S +# SAFETY-119: a run of hand-backs (the wire away less than a settle between +# them) may hide the loop's bound for at most this many cycles and +# SHORT_SPAN_S, in all. +RUN_BOUND_CYCLES = SHORT_CYCLES - 1 def _refused_interval(sign: int) -> tuple: @@ -1272,10 +1276,13 @@ class Controller: self._loop_decision = None self._asked_hist: dict = {} self._short_run = 0 - # SAFETY-119: did another owner set the wire last cycle, and since - # when (monotonic s, owned cycles) has the loop had it back. + # SAFETY-119: did another owner set the wire last cycle; the open run + # (monotonic s and count of the loop's cycles in a row, bound cycles + # hidden, monotonic s of the first); the wire's cycles away in a row + # (first s, count). self._wire_lost = False self._handback = None + self._away = None self.stopping = False # SAFETY-61. time.monotonic() at the end of the last run_control() pass. self.loop_pass_mono = None @@ -5818,6 +5825,7 @@ class Controller: self._short_run = 0 self._asked_hist = {} self._wire_lost, self._handback = False, None + self._away = None return unmet = self.allocation.unmet_w if self.allocation is not None else 0 cut = (resolution.bound_by or "").split("(")[0] @@ -5855,25 +5863,42 @@ class Controller: self.loop_clamp_facts = () def _settling(self, owns, now) -> bool: - """SAFETY-119: is the loop within the plant's settle (SHORT_CYCLES - cycles and SHORT_SPAN_S) of taking the wire back from another owner? - Its integrator is re-seeded from that owner's command and kicked by a - meter that does not show it yet, so its bound there is not counted. - DECISIONS/SAFETY-119-bound-after-hand-back.md.""" + """SAFETY-119: does the loop's bound on this cycle count? Not within + a run of hand-backs from another owner: its integrator is re-seeded + from that owner's command and kicked by a meter that does not show it + yet. A run opens at a hand-back and ends when the loop held the wire, + or the wire was away, a whole settle in a row (SHORT_CYCLES cycles + and SHORT_SPAN_S). Each hand-back restarts the settle, but the run + hides the bound for RUN_BOUND_CYCLES cycles and SHORT_SPAN_S in all, + so a bound that stays is counted after that, however often the wire + changes hands. DECISIONS/SAFETY-119-bound-after-hand-back.md.""" + def aged(t0, span): + return now is None or t0 is None or now - t0 >= span + if not owns: - self._wire_lost, self._handback = True, None + self._wire_lost = True + t0, k = self._away or (now, 0) + self._away = (t0, k + 1) + if k + 1 >= SHORT_CYCLES and aged(t0, SHORT_SPAN_S): + self._handback = None return False + self._away = None if self._wire_lost: - self._wire_lost, self._handback = False, (now, 0) + self._wire_lost = False + self._handback = (now, 0) + (self._handback or (0,) * 4)[2:] if self._handback is None: return False - t0, n = self._handback + tc, n, hid, tb = self._handback n += 1 - self._handback = (t0, n) - if n >= SHORT_CYCLES and (now is None or t0 is None - or now - t0 >= SHORT_SPAN_S): + self._handback = (tc, n, hid, tb) + if aged(tc, SHORT_SPAN_S) and n >= SHORT_CYCLES: self._handback = None return False + if self.loop_clamped[0] and not self.loop_clamped[2]: + if hid >= RUN_BOUND_CYCLES and aged(tb, SHORT_SPAN_S): + return False + tb = tb if hid else now + self._handback = (tc, n, hid + 1, tb) return True async def check_output_clamp(self) -> None: diff --git a/goodwe_controller/test_safety119.py b/goodwe_controller/test_safety119.py index f0b8e62..0836f04 100644 --- a/goodwe_controller/test_safety119.py +++ b/goodwe_controller/test_safety119.py @@ -9,8 +9,10 @@ It crossed its rail for one or two cycles, the loop's ask beat the one-sided limit, so the cycle counted as the loop's own and raised `goodwe_integrator_bound`. The battery gave what it was told. Now the loop's bound does not count until the plant's settle (SHORT_CYCLES cycles and -SHORT_SPAN_S) has passed since the hand-back. -DECISIONS/SAFETY-119-bound-after-hand-back.md. +SHORT_SPAN_S) has passed since the hand-back. Round 2: one such window per +run of hand-backs, hiding at most RUN_BOUND_CYCLES bound cycles and +SHORT_SPAN_S in all, so a bound that stays warns when the guard and the +charge keep taking turns. DECISIONS/SAFETY-119-bound-after-hand-back.md. 1. The rig's shape (1 s meter that lags the battery, 1 s battery lag, house 800 W, rating 2500 W, limit 2500 W, margin 50, charge 2000 W, no @@ -18,11 +20,14 @@ DECISIONS/SAFETY-119-bound-after-hand-back.md. the same as without the fix; without the fix it warns. 2. A genuine integrator bound still warns: the loop in charge with the house over the rating (at once, as before), and the loop at its rail after a - hand-back (after the settle, not lost). + hand-back (after the settle). 3. With a usable forecast: the same commands and the same alarms as without the fix. -4. The rule at its boundaries (`Controller._settling`). -5. test_safety84's and test_safety87's re-pinned digests: the same commands +4. Repeated hand-backs (N=2, 5 s meter, the guard and the charge take turns): + it warns at least once per quarter; a start or a blind cycle is not a + hand-back; a slow meter delays by cycles, not seconds. +5. The rule at its boundaries (`Controller._settling`). +6. test_safety84's and test_safety87's re-pinned digests: the same commands and claims; only OUTPUT_CLAMP log rows differ. Mutations (MUTATION.md, purged copies) are listed in the pull request. @@ -126,11 +131,12 @@ def house(seed, base=800.0, step=None): return load -def run(load, *, dur=7200.0, delay=1, extra=None, cheap=True, cut=None, - charging=True): - """One N=1 run; `charging=False`: no maintenance, the loop in charge.""" +def run(load, *, n=1, dur=7200.0, delay=1, extra=None, cheap=True, cut=None, + charging=True, blind=0.0): + """One run of `n` inverters of 2500 W; `charging=False`: no maintenance, + the loop in charge; `blind`: no meter reading until that second.""" extra = dict(extra or {}) - s84.OPT = dict(OPT0, peak_cap_w=int(CAP), peak_margin_w=50, + s84.OPT = dict(OPT0, peak_cap_w=int(CAP * n), peak_margin_w=50, maintenance_charge_w=2000) if not charging: extra.update(maintenance_enabled=False, peak_cap_w=20000) @@ -138,10 +144,16 @@ def run(load, *, dur=7200.0, delay=1, extra=None, cheap=True, cut=None, s84.OPT[k] = extra.pop(k) DELAY[0] = delay try: - return s84.run(1, load=load, cheap=cheap, - store=(s79.store_charging(1) if charging + feed = s84.feed(cheap) + + def before(c, t): + if t >= blind: # before that: no meter reading + return feed(c, t) + return s84.run(n, load=load, cheap=cheap, + store=(s79.store_charging(n) if charging else rig._Store()), soc=lambda u, t: 60.0, dur=dur, cut=cut, + before=before if blind else None, extra=dict({"max_w": 2500, "slew_w": 0}, **extra)) finally: s84.OPT = OPT0 @@ -166,12 +178,13 @@ def notes(c): if i == messages.ID_LOOP_BOUND] -def quarters(c): +def quarters(c, tick=1.0): """Each whole quarter's true average import (W).""" q = {} for t, g in c.hass.grid_true: q.setdefault(int(t // QUARTER), []).append(g) - return [sum(v) / len(v) for v in q.values() if len(v) == QUARTER] + return [sum(v) / len(v) for v in q.values() + if len(v) == QUARTER / tick] def commands(rows): @@ -272,18 +285,100 @@ check("...the forecast was used (the charge is shaped under -2000 W)", -2000.0 < min(r[1] for r in _fc[0][:300]) < 0.0) # ============================================================================ # -print("\n4. the rule at its boundaries") +print("\n4. repeated hand-backs: the guard and the charge take turns") + + +def round1(self, owns, now): + """Round 1's rule, kept here to show what it hid: every hand-back + restarts the window, so a loss within the settle never lets it end.""" + d = self.__dict__ + if not owns: + d["_r1"], d["_r1_n"] = True, None + return False + if d.get("_r1"): + d["_r1"], d["_r1_n"] = False, (now, 0) + if d.get("_r1_n") is None: + return False + t0, n = d["_r1_n"] + d["_r1_n"] = (t0, n + 1) + if n + 1 >= app_main.SHORT_CYCLES and ( + now is None or t0 is None or now - t0 >= app_main.SHORT_SPAN_S): + d["_r1_n"] = None + return False + return True + + +# N=2 (2 x 2500 W), a 5 s meter, a cheap-window charge, no usable forecast, +# the house 50 W over the 5000 W rating from 300 s: the loop never held the +# wire for more than 10 cycles. Round 1: no notification in 2 h. +rig.TICK = 5.0 +_nv = run(house(1, step=(300.0, 5050.0)), n=2) +_nv_old = no_fix(lambda: run(house(1, step=(300.0, 5050.0)), n=2)) +_nv_r1 = patched(Controller, "_settling", round1)( + lambda: run(house(1, step=(300.0, 5050.0)), n=2)) +rig.TICK = 1.0 +_per_q = [sum(1 for t in notes(_nv[1]) if int(t // QUARTER) == q) + for q in range(8)] +print(f" N=2, 5 s meter, house 5050 W: notifications {len(notes(_nv[1]))} " + f"(per quarter {_per_q}); round 1's rule {len(notes(_nv_r1[1]))}; " + f"without the fix {len(notes(_nv_old[1]))}; highest quarter " + f"{max(quarters(_nv[1], 5.0)):.0f} W") +check("⭐ it warns at least once in every quarter after the first warning " + "(the guard and the charge keep taking turns)", + len(notes(_nv[1])) >= 7 and all(_per_q[q] >= 1 for q in range(1, 8))) +check("non-vacuous: round 1's rule (a window per hand-back) never warned " + "here, and the fix still hides some of what was raised without it", + notes(_nv_r1[1]) == [] + and _nv[1].clamp_wd.bound < _nv_old[1].clamp_wd.bound) +check("...the cap held and every command is the same as without the fix", + len(quarters(_nv[1], 5.0)) == 8 + and max(quarters(_nv[1], 5.0)) < 5000.0 + and commands(_nv[0]) == commands(_nv_old[0])) + +# A start, or a blind cycle, is not a hand-back: the loop at its rail after +# 20 s of not running warns at once (R2: a restart counted as a hand-back). +_bl = run(house(1, base=4000.0), dur=120.0, charging=False, blind=20.0) +_bl_old = no_fix(lambda: run(house(1, base=4000.0), dur=120.0, + charging=False, blind=20.0)) +print(f" the loop not running until 20 s, then at its rail: " + f"{notes(_bl[1])}, without the fix {notes(_bl_old[1])}") +check("⭐ a start after a blind spell is not a hand-back: it warns at the " + "same cycle as without the fix", + notes(_bl[1])[:1] and 20.0 <= notes(_bl[1])[0] <= 26.0 + and notes(_bl[1]) == notes(_bl_old[1])) + +# The settle is SHORT_CYCLES cycles, not only SHORT_SPAN_S (R4): at a 5 s +# meter the loop at its rail after a hand-back warns 11 cycles (55 s) later. +rig.TICK = 5.0 +_sl = run(house(1, step=(600.0, -6000.0)), dur=900.0, + extra={"peak_cap_w": 20000}) +rig.TICK = 1.0 +_sl1 = [t for t in notes(_sl[1]) if t >= 600.0][:1] +print(f" 5 s meter, hand-back to a loop at its rail at 600 s: first " + f"notification {_sl1}") +check("⭐ at a 5 s meter the settle is 11 cycles: the first warning comes " + "55 s after the hand-back, not 22 s", + _sl1 and 600.0 + 11 * 5.0 <= _sl1[0] <= 600.0 + 11 * 5.0 + 10.0) + +# ============================================================================ # +print("\n5. the rule at its boundaries") class _C: _settling = Controller._settling def __init__(self): - self._wire_lost, self._handback = False, None + self._wire_lost, self._handback, self._away = False, None, None + self.loop_clamped = (False, "", False) def seq(c, steps): - return [c._settling(o, t) for o, t in steps] + """(owned, time[, the loop's bound that cycle]) -> does it count not.""" + out = [] + for st in steps: + c.loop_clamped = (len(st) > 2 and st[2], "", False) + out.append(c._settling(st[0], st[1])) + return out N, SPAN = app_main.SHORT_CYCLES, app_main.SHORT_SPAN_S @@ -303,14 +398,97 @@ check(f"at a 0.5 s meter the span decides: settling until {SPAN:.0f} s", _c = _C() _out = seq(_c, [(False, 0.0), (True, 2.0), (True, 4.0), (False, 6.0), (True, 8.0), (True, 10.0)]) -check("losing the wire again restarts the window", +check("losing the wire again restarts the settle (but not the run)", _out == [False, True, True, False, True, True] - and _c._handback == (8.0, 2)) + and _c._handback == (8.0, 2, 0, 0)) _c = _C() check("no monotonic clock: the cycle count alone decides", seq(_c, [(False, None)] + [(True, None)] * (N + 1)) == [False] + [True] * (N - 1) + [False] * 2) +# One run of hand-backs hides RUN_BOUND_CYCLES bound cycles and SHORT_SPAN_S, +# however often the wire changes hands. T = the loop's bound that cycle. +M = app_main.RUN_BOUND_CYCLES + + +def turns(c, tick, owned, lost, periods, first=0.0): + """A hand-back, then `periods` x (`owned` bound cycles, `lost` cycles + away). Returns (time, hidden) of each owned bound cycle.""" + t, out = first, [] + for _ in range(lost): + c._settling(False, t) + t += tick + for _ in range(periods): + for _ in range(owned): + c.loop_clamped = (True, "", False) + out.append((t, c._settling(True, t))) + t += tick + for _ in range(lost): + c._settling(False, t) + t += tick + return out, t + + +_c = _C() +_o, _ = turns(_c, 5.0, 1, 3, 20) +_hid = [h for _t, h in _o] +check(f"at 5 s, one bound cycle per turn: {M} are hidden, the {M + 1}th and " + "every later one counts", + _hid == [True] * M + [False] * (20 - M)) +_c = _C() +_o, _ = turns(_c, 1.0, 2, 1, 12) +_first = next(t for t, h in _o if not h) +check(f"at 1 s the span holds too: not counted before {SPAN:.0f} s after " + f"the first hidden one (counted at {_first:.0f} s)", + _o[0][0] + SPAN <= _first <= _o[0][0] + SPAN + 3.0 + and sum(h for _t, h in _o) > M) +_c = _C() +_t = 0.0 +for _ in range(20): # an over-rating: always counts + for _ in range(3): + _c._settling(False, _t) + _t += 5.0 + _c.loop_clamped = (True, "", True) + _c._settling(True, _t) + _t += 5.0 +for _ in range(3): + _c._settling(False, _t) + _t += 5.0 +_c.loop_clamped = (True, "", False) +check("an over-rating does not use up what the run may hide", + _c._settling(True, _t) is True) +_c = _C() +_o1, _t = turns(_c, 2.0, 1, 3, 20) +for _ in range(N): # the wire away a whole settle + _c._settling(False, _t) + _t += 2.0 +_o2 = [] +for _ in range(20): + _c.loop_clamped = (True, "", False) + _o2.append(_c._settling(True, _t)) + _c._settling(False, _t + 2.0) + _t += 4.0 +check("a settle with the wire away ends the run: the next hand-back hides " + f"{M} again", + [h for _t2, h in _o1] == [True] * M + [False] * (20 - M) + and _o2 == [True] * M + [False] * (20 - M)) +_c = _C() +_o1, _t = turns(_c, 2.0, 1, 3, 20) +for _ in range(N - 1 - 3): # 3 already: N - 1 away + _c._settling(False, _t) + _t += 2.0 +_c.loop_clamped = (True, "", False) +check("...but not a shorter time away: the same run, nothing more hidden", + _c._settling(True, _t) is False) +_c = _C() +_o1, _t = turns(_c, 2.0, 1, 3, 20) +_ = [_c._settling(True, _t + 2.0 * i) for i in range(N + 1)] +_c._settling(False, _t + 40.0) +_c.loop_clamped = (True, "", False) +check("the loop holding the wire a whole settle ends the run too: the next " + "hand-back hides again", + _c._settling(True, _t + 42.0) is True) + # An over-rating (a command over `max_w`) is a bug, never settled away. _ctl = _hb[1] _ctl.loop_clamped = (True, "x", True) @@ -342,7 +520,7 @@ check("a stop clears it: the next start is not a hand-back", (_ctl._wire_lost, _ctl._handback) == (False, None)) # ============================================================================ # -print("\n5. the pinned digests of test_safety84 and test_safety87: only the " +print("\n6. the pinned digests of test_safety84 and test_safety87: only the " "log changes") # Those harnesses run at the 5 s tick with a battery that follows at once. rig._Hass.power, rig.TICK, s79._feed = _real_power, 5.0, _real_feed diff --git a/goodwe_controller/test_safety87.py b/goodwe_controller/test_safety87.py index 1de2ebd..0670f62 100644 --- a/goodwe_controller/test_safety87.py +++ b/goodwe_controller/test_safety87.py @@ -540,8 +540,9 @@ _now = s87.digests() for k in PINNED: print(f" {k} {'same' if _now[k] == PINNED[k] else 'DIFFERS'}") check("⭐ every N=1 digest (claims, commands, the guard's decision and the " - "log) equals b8ddfd7's (the log as re-pinned by SAFETY-119), under, crossing and above the reserve, in a " - "charge and a HOLD, and under a banked peak", + "log) equals b8ddfd7's (the log as re-pinned by SAFETY-119), under, " + "crossing and above the reserve, in a charge and a HOLD, and under a " + "banked peak", _now == PINNED) # ============================================================================ #