From 7d06aa674d39e32cb9ade36a0b1ba85c26d315d2 Mon Sep 17 00:00:00 2001 From: SAFETY-97 review Date: Sun, 4 Oct 2026 16:57:16 +0200 Subject: [PATCH] SAFETY-97: centre the band on the idle reading clamped to 75 W Owner decision 2026-10-04 (review R1). A wrong idle reading more than about 100 W off hid every refusal on that battery. Clamped under the band, the band keeps 0 W +-25 W, so a battery reading about 0 W while refusing is caught whatever was learned or restored. Samples are still taken to 250 W. Beyond about +-170 W of idle a battery behaves as in 0.14.0. Co-Authored-By: Claude Opus 5.5 --- .../DECISIONS/SAFETY-97-idle-offset.md | 43 +++++++---- goodwe_controller/app/idle.py | 20 +++--- goodwe_controller/test_safety97.py | 71 ++++++++++++++++--- 3 files changed, 103 insertions(+), 31 deletions(-) diff --git a/goodwe_controller/DECISIONS/SAFETY-97-idle-offset.md b/goodwe_controller/DECISIONS/SAFETY-97-idle-offset.md index 491e97f..e572314 100644 --- a/goodwe_controller/DECISIONS/SAFETY-97-idle-offset.md +++ b/goodwe_controller/DECISIONS/SAFETY-97-idle-offset.md @@ -1,8 +1,9 @@ # SAFETY-97 - "gives nothing" is graded against each battery's idle reading **Status: implemented 2026-10-04, folded into PR #171 (SAFETY-94), for -review.** Tests: `test_safety97.py` (rig: `tools/safety94_rig.py`). Closes -SAFETY-94 review finding F1. +review.** Owner decision 2026-10-04: the band's centre is clamped to ++-75 W (see Bounds). Tests: `test_safety97.py` (rig: `tools/safety94_rig.py`). +Closes SAFETY-94 review finding F1. ## The defect @@ -27,10 +28,11 @@ B sat on the band's edge and never latched: than 50 W, and while refused it does not learn, so its refusal cannot be learned as idle. A capped battery under a big ask is not sampled either. - **How:** the first value is the mean of 120 s of samples; then it follows - drift with a 30 min time constant. A sample beyond 250 W (`IDLE_MAX_W`) - is dropped (a register held at a giving value); the value is clamped to it. + drift with a 30 min time constant. A sample beyond 250 W (`IDLE_SAMPLE_W`) + is dropped (a register held at a giving value). The band is centred on + the value clamped to 75 W (`IDLE_MAX_W`; owner decision below). - **Restart:** saved per unit (`unit//idle_w`, on first learn and each - 5 W move) and restored, clamped. Without it a battery refusing from the + 5 W move) and restored, clamped to 250 W. Without it a battery refusing from the start is never quiet, so never learned, so never caught. - **Until learned the idle reading is 0 W**: the rule as it was. @@ -70,16 +72,29 @@ Every test of "gives nothing / gives / takes" now uses `reading - idle`: | SAFETY-30 per-unit sign | unchanged: opposition needs both over 250 W; B's 100 W idle cannot make or hide one alone | | SAFETY-42 shortfall | unchanged: B reads ~100 W short at every power, which errs towards reporting (report only) | -## Bounds +## Bounds: owner decision 2026-10-04 (review R1) -- Band: `|reading - idle| < 100 W`, `|idle| <= 250 W`. -- A correct idle reading: a refusing B behaves as a refusing A. -- A wrong one (worst, 250 W off): a refusing battery reading outside the band - is not refused by this rule; a frozen register is still caught by the stale - gate at 35 s of asking. Learning only at a ~0 W command for 2 min keeps a - wrong value unlikely. Once learned, a 10 min transient moves it by 28 %; - the first value (a 2 min mean) is the exposed one. -- An idle reading beyond 250 W is never learned: such a unit keeps 0 W. +Band: `|reading - centre| < 100 W`, centre = idle clamped to +-75 W. + +- **Why a clamp under the band.** A wrong idle reading (taught during a + rest while the reading shows something else, e.g. a grid top-up of a unit + held at 0 W, or restored onto a swapped inverter) more than about 100 W off + hid every refusal on that battery, at N=1 and N=2; with the first +-250 W + clamp the rig missed a refusing A whenever the error was 110-250 W. At + +-75 W the band always keeps 0 W +-25 W, so a battery reading about 0 W + while refusing is caught whatever was learned or restored, and a wrong idle + cannot refuse a healthy 0 W battery (its ask is over 200 W). +- **The trade-off.** Options: keep +-250 W (covers idle readings to about + +-350 W, any 100 W mis-learn hides refusals); clamp to +-75 W (chosen); + "old band or centred band" (rejected: a correct B would need to give 200 W + to answer its 200 W probe). +- **Cost.** A battery whose idle reading is beyond about +-170 W is centred + at most 75 W out, so its refusal falls outside the band: it behaves as in + 0.14.0 (not caught by this rule). B (-95 to -118 W) reads 20-43 W from its + clamped centre. B is caught across -150..+90 W of error in its idle + reading; learned 110 W or more too high it is missed, as in 0.14.0 at N=2. +- A frozen register is still caught by the stale gate at 35 s of asking. +- Recovery from a wrong value needs quiet time: tau 30 min. ## Catch times (rig, refusing from 200 s after a 400 s rest, +-4 W) diff --git a/goodwe_controller/app/idle.py b/goodwe_controller/app/idle.py index a0b682c..513d210 100644 --- a/goodwe_controller/app/idle.py +++ b/goodwe_controller/app/idle.py @@ -9,16 +9,18 @@ Learned per unit, never configured: - only while control runs, the unit is not refused, stale or tripped, and its command has been within IDLE_ASK_W for IDLE_QUIET_S (a refusing battery is asked more than that, so it cannot teach its refusal as idle); -- a reading beyond IDLE_MAX_W is not a sample, and the value is clamped to it; +- a reading beyond IDLE_SAMPLE_W is not a sample; - first value: the mean of IDLE_FIRST_S of samples; then a slow follow (IDLE_TAU_S); -- persisted per unit and restored, clamped, at start. -Until learned the idle reading is 0 W, the rule as it was. +- persisted per unit and restored, clamped to IDLE_SAMPLE_W, at start. +The band is centred on it clamped to IDLE_MAX_W (owner, 2026-10-04). Until +learned the idle reading is 0 W, the rule as it was. """ import math -IDLE_MAX_W = 250.0 # |idle| bound: a band centred further out hides too much +IDLE_MAX_W = 75.0 # |idle| used: under the 100 W band, so it keeps 0 W +-25 W +IDLE_SAMPLE_W = 250.0 # a reading beyond this is not a sample IDLE_ASK_W = 50.0 # a command this small counts as "asked ~0" IDLE_QUIET_S = 120.0 # 6 lags of 20 s: a 2500 W step has decayed to 6 W IDLE_FIRST_S = 120.0 # seconds of samples averaged for the first value @@ -41,7 +43,7 @@ class IdleReading: def __init__(self, saved=None): v = _num(saved) self.value = (None if v is None - else max(-IDLE_MAX_W, min(IDLE_MAX_W, v))) + else max(-IDLE_SAMPLE_W, min(IDLE_SAMPLE_W, v))) self.saved = self.value self.quiet_since = None self.last = None @@ -50,7 +52,10 @@ class IdleReading: @property def w(self) -> float: - return 0.0 if self.value is None else self.value + """The centre used: the learned value clamped to IDLE_MAX_W.""" + if self.value is None: + return 0.0 + return max(-IDLE_MAX_W, min(IDLE_MAX_W, self.value)) def observe(self, reading, standing_w, now_mono, may_learn) -> bool: """One read. Returns True when the value should be persisted.""" @@ -66,7 +71,7 @@ class IdleReading: self.quiet_since = now b = _num(reading) if (now - self.quiet_since < IDLE_QUIET_S or b is None - or abs(b) > IDLE_MAX_W or dt <= 0.0): + or abs(b) > IDLE_SAMPLE_W or dt <= 0.0): return False if self.value is None: self.acc_w += b * dt @@ -76,7 +81,6 @@ class IdleReading: self.value = self.acc_w / self.acc_s else: self.value += (b - self.value) * min(1.0, dt / IDLE_TAU_S) - self.value = max(-IDLE_MAX_W, min(IDLE_MAX_W, self.value)) if self.saved is None or abs(self.value - self.saved) >= IDLE_SAVE_W: self.saved = self.value return True diff --git a/goodwe_controller/test_safety97.py b/goodwe_controller/test_safety97.py index 003b3d8..084c6b3 100644 --- a/goodwe_controller/test_safety97.py +++ b/goodwe_controller/test_safety97.py @@ -169,7 +169,7 @@ def small_ask(): b, b_old = small_ask(), without_fix(small_ask) check("SAFETY-42: B giving nothing at a small ask, idle -100 W, is parked " "(not latched as not delivering in 700 s; with idle 0 W it is)", - not b.delivery_wd.tripped and b.idle_w == -100.0 + not b.delivery_wd.tripped and b.idle.value == -100.0 and b_old.delivery_wd.tripped) # ============================================================================ # @@ -213,8 +213,11 @@ check("⭐ a charge reversing into a discharge (lag 10 and 20 s, 1 s meter) " print("\n3. learning the idle reading") _t, r = caught(2, "b", 1.0) c = r["c"] -check("learned during the rest: A about 0 W, B about -100 W, and saved", - abs(c.inverters[0].idle_w) < 5 and abs(c.inverters[1].idle_w + 100) < 5 +check("learned during the rest: A about 0 W, B about -100 W (used: -75 W), " + "and saved", + abs(c.inverters[0].idle_w) < 5 + and abs(c.inverters[1].idle.value + 100) < 5 + and c.inverters[1].idle_w == -idle.IDLE_MAX_W and abs(c.store.data.get(unit_key("inverter-b", "idle_w"), 0) + 100) < 5) @@ -250,10 +253,11 @@ try: finally: rig.Plant = real_plant b = r["c"].inverters[1] -print(f" refusing B reading -160 W: refused at {_t} s, idle {b.idle_w:.1f}") +print(f" refusing B reading -160 W: refused at {_t} s, idle " + f"{b.idle.value:.1f}") check("⭐ not learned while refused: 800 s held at 0 W reading -160 W leaves " "B's idle reading at -100 W", - _t is not None and b.refused and abs(b.idle_w + 100.0) < 1.0) + _t is not None and b.refused and abs(b.idle.value + 100.0) < 1.0) # Not learned while control is stopped (another owner may drive the battery) # or while the unit's sign watchdog has latched. @@ -319,12 +323,58 @@ for t in range(0, 600): check("a reading beyond 250 W is not a sample (-300 W: never learned; a " "register held at 2000 W then -240 W: -240 W)", o.value is None and p.value is not None and abs(p.value + 240.0) < 1e-6) -check("a saved value is clamped to 250 W, and garbage is ignored", - idle.IdleReading(900).w == idle.IDLE_MAX_W +check("a saved value is clamped to 250 W, the band's centre to 75 W, and " + "garbage is ignored", + idle.IdleReading(900).value == idle.IDLE_SAMPLE_W + and idle.IdleReading(900).w == idle.IDLE_MAX_W and idle.IdleReading(-900.0).w == -idle.IDLE_MAX_W + and idle.IdleReading(-60.0).w == -60.0 and idle.IdleReading("x").value is None and idle.IdleReading(float("nan")).value is None) +# ============================================================================ # +print("\n3b. a wrong idle reading (owner, 2026-10-04: centre clamped to 75 W)") + + +def wrong_idle(n, unit, err, restore): + """Seconds from the cut to the refusal when the idle reading is `err` W + off: taught wrong during the rest, or restored wrong.""" + cu = "a" if n == 1 else unit + true = -100.0 if unit == "b" else 0.0 + if restore: + return caught(n, unit, 1.0, store={ + unit_key(f"inverter-{cu}", "idle_w"): true + err})[0] + + class _W(rig.Plant): + def advance(self, t): + rig.OFFSET[cu] = true + (err if t < REST else 0.0) + super().advance(t) + saved, real = dict(rig.OFFSET), rig.Plant + rig.Plant = _W + try: + return caught(n, unit, 1.0)[0] + finally: + rig.Plant = real + rig.OFFSET.clear() + rig.OFFSET.update(saved) + + +A_WRONG = {(n, e, rs): wrong_idle(n, "a", e, rs) for n in (1, 2) + for e, rs in ((-250.0, False), (-150.0, False), (150.0, False), + (250.0, False), (-250.0, True), (200.0, True), + (250.0, True))} +print(f" A (reads about 0 W) refusing, idle off by E: {A_WRONG}") +check("⭐ a battery reading about 0 W while refusing is caught whatever idle " + "reading was learned or restored (A, E -250..+250 W, N=1 and N=2)", + all(v is not None and v <= 32.0 for v in A_WRONG.values())) +check("⭐ ...including a wrong saved idle_w of +200 W on A", + all(A_WRONG[(n, 200.0, True)] is not None for n in (1, 2))) +B_WRONG = {(n, e): wrong_idle(n, "b", e, False) for n in (1, 2) + for e in (-150.0, -50.0, 90.0)} +print(f" B refusing, idle taught wrong by E: {B_WRONG}") +check("B is caught across -150..+90 W of error in its idle reading", + all(v is not None and v <= 32.0 for v in B_WRONG.values())) + # ============================================================================ # print("\n4. the rules at their edges") @@ -378,8 +428,11 @@ check("compute(): a charge refusal ends when the battery takes 100 W more " and control.compute(actual_w=-205.0, idle_w=-100.0, **args).refused == 0 and control.compute(actual_w=-110.0, **args).refused == 0) -check("the idle bound is under the band's reach of a real delivery: 250 W", - idle.IDLE_MAX_W == 250.0 and idle.IDLE_ASK_W < control.PROBE_W) +check("the band's centre stays within 75 W of 0 W, so the band keeps 0 W " + "+-25 W; samples to 250 W", + idle.IDLE_MAX_W == 75.0 and idle.IDLE_SAMPLE_W == 250.0 + and control.REFUSED_BELOW_W - idle.IDLE_MAX_W >= 25.0 + and idle.IDLE_ASK_W < control.PROBE_W) print() if fails: