From 4b81387908751a970dce4023acfc62177010fd66 Mon Sep 17 00:00:00 2001 From: glenn schrooyen Date: Thu, 1 Oct 2026 19:02:51 +0200 Subject: [PATCH 1/2] SAFETY-85: no false 'not keeping up' while maintenance or the peak guard sets the battery The loop's integrator bound counts only on a cycle the loop decided. A battery another owner set (maintenance, the peak guard, a safety limit) alarms only when it gives more than saturation_w less than told for saturation_cycles cycles. At N>=2 the loop is bounded while maintenance holds a battery, so it does not wind and control comes back one slew step a cycle. The status row counts episodes, not cycles. Digests re-pinned where behaviour changed on purpose; DECISIONS/SAFETY-85-no-false-not-keeping-up.md has the list. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019DBUfvndgHwaV339Fvd2gr --- .../SAFETY-85-no-false-not-keeping-up.md | 80 +++++ goodwe_controller/DOCS.md | 14 +- goodwe_controller/app/clamp.py | 26 ++ goodwe_controller/app/control.py | 8 +- goodwe_controller/app/main.py | 60 +++- goodwe_controller/test_clamp.py | 12 +- goodwe_controller/test_modes.py | 26 +- goodwe_controller/test_peak_margin.py | 4 +- goodwe_controller/test_safety78.py | 7 +- goodwe_controller/test_safety84.py | 19 +- goodwe_controller/test_safety85.py | 303 ++++++++++++++++++ goodwe_controller/test_safety86.py | 30 +- 12 files changed, 554 insertions(+), 35 deletions(-) create mode 100644 goodwe_controller/DECISIONS/SAFETY-85-no-false-not-keeping-up.md create mode 100644 goodwe_controller/test_safety85.py diff --git a/goodwe_controller/DECISIONS/SAFETY-85-no-false-not-keeping-up.md b/goodwe_controller/DECISIONS/SAFETY-85-no-false-not-keeping-up.md new file mode 100644 index 0000000..09cf889 --- /dev/null +++ b/goodwe_controller/DECISIONS/SAFETY-85-no-false-not-keeping-up.md @@ -0,0 +1,80 @@ +# SAFETY-85 - no false "not keeping up" while another owner sets the battery + +**Status: implemented 2026-10-01, for review.** Tests: `test_safety85.py`. + +**The defect.** "Battery is not keeping up" (SAFETY-40, `goodwe_integrator_bound`) +fired about every 2.5 min during maintenance (drain, charge, HOLD) and under the +peak guard, while the battery gave exactly what it was told. Each warning +re-created the HA notification. Rig counts: 24 in an hour (SAFETY-83 F1), 43 in +7 episodes (SAFETY-86), 35 in 16 (SAFETY-87), 12 in 6 (SAFETY-84). The page +counter counted cycles (861), not episodes. + +**Cause, measured on the real Controller.** +- N=1: when a higher claim decides, `cycle()` re-seeds the integrator from the + target each cycle, then `compute()` adds `gain * error` for a grid error the + loop does not own. Target + kick crosses the rail and sets `i_clamped`. The + carried integrator is not wound (SAFETY-48 caps the lead at one slew step), + so control comes back one slew step at a time. +- N>=2: a maintenance pin is a unit claim. The loop stays the site winner and + winds to its rail for watts the pinned battery may not give. When the pin + lifts, the site jumps: 2500 W in one cycle when a HOLD ends under a 4500 W + house, 1390 W when a charge pin lifts (`s84 n2_charge_dear`, t=1800 s). + +**Decision.** Three rules; the alarm, its text and its id are unchanged. +1. **The loop's clamp counts only on a cycle the loop decided.** The winner is + the loop. Its number is on the wire, or only a rating (`device`, + `supervised`) cut it. No bound held it, and nothing was unmet. + (`Controller._loop_alarm_input`.) +2. **A battery another owner set alarms when it gives less than it was told.** + This covers every battery on a cycle the loop did not decide, and a battery + with its own pin otherwise. The test is more than `saturation_w` short in + the told direction, for `saturation_cycles` cycles in a row, against what + stands on the wire (`clamp.delivery_short`). It covers a real shortfall + during maintenance. Before, that shortfall was silent unless the kick + happened to reach the rail (house 1000 W, drain 2000 W, battery gives + nothing: 0 warnings before, 4 in 10 min now). +3. **At N>=2, while maintenance holds a battery, the loop is bounded** by + OPT-11's `loop_bound()` (what the batteries may place). It does not wind, + and control comes back one slew step a cycle. Why maintenance only: that is + where the jump was measured and where the owner order puts maintenance + above the loop. Other N>=2 limits (pack cut-out, SAFETY-73 refusal) keep + the old law. Rule 1 still silences them (the target is unmet), so they no + longer warn either. + +Fixing the windup was chosen over only ignoring the wound part, because at +N>=2 the wound integrator moves real watts at hand-back. At N=1 there was +nothing to fix: re-seeding already prevents windup. + +**What SAFETY-03/40 protect is kept.** With the loop in charge, the integrator +bound still warns (a runaway, a house above the rating), and a saturated +inverter under the loop's own command still does not alarm (SAFETY-03 AC 2): +rule 2 grades only batteries another owner set. + +**The counter.** `ClampWatchdog.bound_episodes` counts episodes that had a +bound engagement. The status row shows it ("N times since start"). +`bound` (cycles) stays for the log and the tests. + +**Re-pinned digests.** Every change is intended; nothing at N=1 moves a watt. + +| digest | what changed | +|---|---| +| s78 `n1_drain_owner` | the false clamp rows are gone from the log; commands identical | +| s78 `n2_drain`, s84 `n2_charge`, `n2_charge_banked`, s79 `n2_maint` (test_peak_margin) | loop reason (`mode-bound`), integrator and loop claim while bounded; commands identical | +| opt11 `n2_maint` (test_modes) | A drops out at t=1655 s: B follows the house at once, 8 cycles differ (max 920 W) | +| s84 `n2_charge_dear` | A's pin lifts at t=1800 s: 400 W a cycle, not a 1390 W jump; the surplus step after it; 98 cycles | + +test_safety86's "without the ceiling the log floods" now needs the ceiling +and the bound both removed: either one alone keeps the loop inside what the +batteries can give. The `alarmed()` helpers in test_safety84/86 also read +SAFETY-81's quarter stamp, because ALLOCATOR rows can push its row out of +the 40-row log. + +**Not changed, open for the owner.** +- With the loop in charge and the house above the batteries' total rating, + it still warns (SAFETY-40). On the rig's 2 x 2000 W N=2, most of SAFETY-84's + count came from houses of 4500 and 6500 W. +- A battery that tapers near full during a charge, more than `saturation_w` + under its command for 15 s, now warns (rule 2). The text already says + "check it is not empty or full". +- TEL-10's `loop_reason` reads `mode-bound` while the maintenance bound holds + the loop, with modes off. diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md index a03194f..d246895 100644 --- a/goodwe_controller/DOCS.md +++ b/goodwe_controller/DOCS.md @@ -1953,6 +1953,13 @@ over the rating is logged as a software fault, at `ERROR`, and it has its own notification budget. Why: [`DECISIONS/SAFETY-40-the-integrator-bound-is-not-a-bug.md`](DECISIONS/SAFETY-40-the-integrator-bound-is-not-a-bug.md). +The integrator bound counts only on a cycle the loop decided. While +maintenance, the peak guard or a safety limit sets a battery, the same alarm +fires only when that battery gives more than `saturation_w` less than it was +told, for `saturation_cycles` cycles. At two or more inverters the loop does not +wind while maintenance holds a battery. +Why: [`DECISIONS/SAFETY-85-no-false-not-keeping-up.md`](DECISIONS/SAFETY-85-no-false-not-keeping-up.md). + ### The frozen-meter alarm A meter that has died is easy — it stops answering, the sample age climbs past @@ -3122,7 +3129,9 @@ The battery gives less power than the add-on asks for. This is normal for a short time, for example just after a large load switches on or the sun comes out. It is not a fault, and control keeps running. If it keeps coming back, check that the battery is switched on, and that it is not empty (when it should -give power) or full (when it should charge). +give power) or full (when it should charge). During the monthly maintenance or +while the peak limit is guarded, it only shows when the battery gives less than +it was told. The status page counts how many times it happened. ### Power request too high @@ -3227,7 +3236,8 @@ section without a link. The documentation tab adds no heading anchors, so the link cannot jump to the section. **Battery is not keeping up.** The grid-following loop's integrator reached its -anti-windup bound (`integrator_max_w`, SAFETY-40). No command went over the +anti-windup bound (`integrator_max_w`, SAFETY-40), or a battery that another +owner set gave less than it was told (SAFETY-85). No command went over the rating. The log line is a WARNING with reason code `OUTPUT_CLAMP`. See *The winding-loop alarm*. diff --git a/goodwe_controller/app/clamp.py b/goodwe_controller/app/clamp.py index 31aeb80..e690faf 100644 --- a/goodwe_controller/app/clamp.py +++ b/goodwe_controller/app/clamp.py @@ -260,6 +260,24 @@ def loop_clamp_note(d) -> str: return "" +def shortfall_note(named) -> str: + """SAFETY-85: the alarm text while another owner sets the batteries. + `named` is `[(inverter name or None, asked W, given W)]`, one per battery + that gave less than asked.""" + return f"{REASON_CODE}: " + ", ".join( + f"{name or 'the battery'} gives {given:+.0f} W of the {asked:+.0f} W " + f"asked" for name, asked, given in named) + + +def delivery_short(asked_w, given_w, margin_w) -> bool: + """SAFETY-85: did a battery give more than `margin_w` less than asked, in + the asked direction? Giving more than asked, or 0 W asked, is not short.""" + asked, given = float(asked_w), float(given_w) + if asked == 0.0: + return False + return (asked - given) * (1.0 if asked > 0 else -1.0) > abs(float(margin_w)) + + def write_clamp_note(named) -> str: """The write-side alarm text: `named` is `[(inverter name or None, ClampResult)]` for every inverter whose write was cut back this cycle. @@ -382,6 +400,10 @@ class ClampWatchdog: self.over_rating = 0 # bug=True: "Power request too high" self.bound = 0 # bug=False: "Battery is not keeping up" self.episodes = 0 # rising edges + # SAFETY-85: episodes with a bug=False engagement. The status row + # counts these; counting cycles read 861 on one rig run. + self.bound_episodes = 0 + self._bound_counted = False self.since: datetime | None = None self.last_alarm: datetime | None = None self.reason = "" @@ -436,6 +458,10 @@ class ClampWatchdog: self.spoken = False self.bug = False self.bug_spoken = False + self._bound_counted = False + if not bug and not self._bound_counted: + self._bound_counted = True + self.bound_episodes += 1 if bug or not self.bug: self.reason = reason self.facts = tuple(facts) diff --git a/goodwe_controller/app/control.py b/goodwe_controller/app/control.py index 8e01509..1f2713a 100644 --- a/goodwe_controller/app/control.py +++ b/goodwe_controller/app/control.py @@ -382,6 +382,9 @@ class Decision: # SAFETY-52. The refused direction to carry back in (+1 discharge, -1 # charge, 0 none), or None when the caller does not carry it. refused: int | None = None + # SAFETY-85. The `bound` held the integrator or the command this cycle, + # so the batteries' limits, not the loop, decided it. + held: bool = False def slew_limit_w(tuning: Tuning) -> float: @@ -614,11 +617,13 @@ def compute( # OPT-11: the integrator does not wind past what the modes let the # batteries place. Only a side tighter than the rail is left in `bound` # (above), so this never takes SAFETY-03's `i_clamped` away from the rail. + held_by_bound = False if bound is not None: held = max(float(bound[0]), min(float(bound[1]), i_w)) if held != i_w: i_w = held reason = MODE_BOUND + held_by_bound = True # ⚠️ Applied EVERY cycle, frozen or not: the freeze is conditional, this # bound is not. It is what makes the worst-case unwind time finite and @@ -839,6 +844,7 @@ def compute( if inside != target: target = inside reason = MODE_BOUND + held_by_bound = True # SAFETY-48's lead cap above may pull the integrator back toward the # slewed command; what is CARRIED stays inside the bound too. i_w = max(float(bound[0]), min(float(bound[1]), i_w)) @@ -860,7 +866,7 @@ def compute( return Decision(float(target), sat_count, frozen, reason, float(i_w), i_clamped, clamped, float(i_bound_w), float(want), - float(clamped_w), refused) + float(clamped_w), refused, held_by_bound) def maintenance_charge_floor( diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index 2e257b0..ec4617f 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -59,7 +59,8 @@ from .circulation import ( ) from .clamp import ( REASON_CODE as CLAMP_REASON, ClampWatchdog, - clamp_to_rating, is_commandable, loop_clamp_note, write_clamp_note, + clamp_to_rating, delivery_short, is_commandable, loop_clamp_note, + shortfall_note, write_clamp_note, ) from . import freshness from . import homeview @@ -1179,6 +1180,10 @@ class Controller: self.loop_clamped: tuple[bool, str, bool] = (False, "", False) # UX-08: the numbers of a loop-side over-rating, for the notification. self.loop_clamp_facts: tuple = () + # SAFETY-85: this cycle's loop Decision, and how many cycles in a row + # a battery set by another owner gave less than asked. + self._loop_decision = None + self._short_run = 0 self.stopping = False # SAFETY-61. time.monotonic() at the end of the last run_control() pass. self.loop_pass_mono = None @@ -4547,9 +4552,13 @@ class Controller: refused=self.batt_refused if len(self.inverters) == 1 else self._site_refused(), probe=probe, - # OPT-11: None unless battery modes are on, which is the law - # exactly as it was. - bound=self.loop_bound(claims) if self.modes_on else None, + # OPT-11: None unless battery modes are on. SAFETY-85: or while + # maintenance holds a battery at N>=2, so the loop does not + # wind past what the others can give and jump when it ends. + bound=(self.loop_bound(claims) if self.modes_on or ( + len(self.inverters) > 1 + and any(r.phase != IDLE for r in maint_results)) + else None), ) if decision.frozen and self.sat_count < self.tuning.saturation_cycles: self.log_event(f"saturation freeze ({self.target:.0f} W vs " @@ -4558,6 +4567,7 @@ class Controller: self.sat_count = decision.sat_count self.i_w = decision.i_w self.loop_w = decision.target_w + self._loop_decision = decision if len(self.inverters) == 1: self._refusal_update(decision, loop_prev, site_batt, probe) # --- TEL-10 ------------------------------------------------------ @@ -4637,6 +4647,7 @@ class Controller: # de-assert when control is stopped. self.loop_clamped = (False, "", False) self.loop_clamp_facts = () + self._loop_decision = None # SAFETY-28. A new resolution is a new tick, and the previous record is # stale from this line onwards. Incremented BEFORE resolve() so that the @@ -4856,6 +4867,8 @@ class Controller: # That is the dropped-link case: the inverter holds its last command # FOREVER, so "not in the split" must mean zero on the wire, not # silence. The write is the same clamped, budgeted path as every other. + # SAFETY-85, before the writes: the wire still holds last cycle's. + self._loop_alarm_input(resolution) writes = [] for inv in self.inverters: inv.target = float(resolution.shares.get(inv.name, 0.0)) @@ -5069,6 +5082,38 @@ class Controller: for inv in self.inverters: inv.target = shadow.WIRE_W + def _loop_alarm_input(self, resolution) -> None: + """SAFETY-85. The loop's clamp counts only on a cycle the loop decided + (a rating may cut it). A battery another owner set counts when it gave + less than it was told for `saturation_cycles` cycles in a row. + DECISIONS/SAFETY-85-no-false-not-keeping-up.md.""" + d = self._loop_decision + if d is None: + self._short_run = 0 + return + unmet = self.allocation.unmet_w if self.allocation is not None else 0 + cut = (resolution.bound_by or "").split("(")[0] + owns = (resolution.winner == "loop" and not d.held and not unmet + and (resolution.target_w == d.target_w + or cut in ("device", "supervised"))) + # Graded against what stands on the wire (0 W in shadow mode). + others = [inv for inv in self.inverters if not owns + or allocator.pin_w(resolution.unit_bounds.get(inv.name)) + is not None] + short = [(inv, inv.standing_w(), inv.batt) for inv in others + if inv.batt is not None + and delivery_short(inv.standing_w(), inv.batt, + self.tuning.saturation_w)] + self._short_run = self._short_run + 1 if short else 0 + if owns and self.loop_clamped[0]: + return + engaged = self._short_run >= self.tuning.saturation_cycles + named = len(self.inverters) > 1 + self.loop_clamped = (engaged, shortfall_note( + [(inv.name if named else None, a, g) for inv, a, g in short]) + if engaged else "", False) + self.loop_clamp_facts = () + async def check_output_clamp(self) -> None: """SAFETY-03. Alarm whenever a clamp actually engaged this cycle. @@ -6291,7 +6336,8 @@ class Controller: elif _cw.active: # SAFETY-40: the integrator bound alone is amber, not red. out.append({"ok": False, "warn": True, "id": "not_keeping_up", - "text": messages.row_not_keeping_up(_cw.bound)}) + "text": messages.row_not_keeping_up( + _cw.bound_episodes)}) # No over-rating in THIS episode (`bug` would be set), so any count # is from an earlier one: keep that software-fault evidence shown. if _cw.over_rating: @@ -6303,10 +6349,10 @@ class Controller: out.append({"ok": False, "warn": True, "id": "power_too_high", "text": messages.row_before( messages.T_OUTPUT_CLAMP, _cw.over_rating)}) - if _cw.bound: + if _cw.bound_episodes: out.append({"ok": False, "warn": True, "id": "not_keeping_up", "text": messages.row_before( - messages.T_LOOP_BOUND, _cw.bound)}) + messages.T_LOOP_BOUND, _cw.bound_episodes)}) for inv in self.inverters: if inv.last_write_ok is False: diff --git a/goodwe_controller/test_clamp.py b/goodwe_controller/test_clamp.py index 98e9a52..b3cc1b2 100644 --- a/goodwe_controller/test_clamp.py +++ b/goodwe_controller/test_clamp.py @@ -553,12 +553,14 @@ check("the status page flags it WHILE it is engaged - amber, since it is the " any(not c["ok"] and c["warn"] and c.get("id") == "not_keeping_up" and c["text"].startswith(messages.T_LOOP_BOUND) for c in ctl_r.red_snapshot)) -check("...and once clear, a warn row still says how many times it engaged, " - "under the integrator bound's own title", +# SAFETY-85: the row counts episodes, not cycles. +check("...and once clear, a warn row still says how many times it happened " + "(episodes, not cycles), under the integrator bound's own title", any(c["warn"] and c.get("id") == "not_keeping_up" - and c["text"].startswith(messages.T_LOOP_BOUND) - and messages.times(ctl_r.clamp_wd.bound) in c["text"] - and "fine now" in c["text"] for c in ctl_r.checks())) + and c["text"] == messages.row_before( + messages.T_LOOP_BOUND, ctl_r.clamp_wd.bound_episodes) + for c in ctl_r.checks()) + and 0 < ctl_r.clamp_wd.bound_episodes < ctl_r.clamp_wd.bound) # UX-07 item 1: the after-episode row used to say "output clamp ... engaged N # time(s)" for integrator-bound cycles. Each count now sits under its own name. check("UX-07 item 1: bound-only engagements are counted as the bound, and no " diff --git a/goodwe_controller/test_modes.py b/goodwe_controller/test_modes.py index a4e4fc8..6e38ed3 100644 --- a/goodwe_controller/test_modes.py +++ b/goodwe_controller/test_modes.py @@ -108,15 +108,27 @@ A5DDDAD = { _claims, _undo_c = spy_claims() _bounds, _undo_b = spy_bounds() try: - _now = rig.digests() + _now = {k: rig.digest(rig.run(**v)[0]) + for k, v in rig.SCENARIOS.items() if k != "n2_maint"} + _now["resolve"] = rig.digest(rig.resolve_rows()) +finally: + _undo_b() +# SAFETY-85: n2_maint (maintenance at N=2) is the one run with a bound. +_bounds_m, _undo_b = spy_bounds() +try: + _now["n2_maint"] = rig.digest(rig.run(**rig.SCENARIOS["n2_maint"])[0]) finally: _undo_c() _undo_b() # SAFETY-78 changes n2_maint ON PURPOSE: on a5dddad and 5a4cdc2 its N=2 drain # was refused on 392 of 540 cycles (site at 0 W); now none is. Printed on the # SAFETY-78 tree the same way. Every other digest is unchanged. +# SAFETY-85 changes it again ON PURPOSE: while A's drain pin stands the loop +# is bounded by what the batteries can place, so when A drops out at t=1655 B +# follows the house down at once instead of unwinding 920 W too high. Was +# cd4d9cb5b7e29ff4ab4fe65dab5106683d8a599f09323fbdee4018ad9b28fcd1. SAFETY78_N2_MAINT = \ - "cd4d9cb5b7e29ff4ab4fe65dab5106683d8a599f09323fbdee4018ad9b28fcd1" + "1afcd46b1b987868b110d2b0f4a7555848c9aa4728a836d4a2ce2a2107268497" for k in A5DDDAD: print(f" {k:7} {'same' if _now[k] == A5DDDAD[k] else 'DIFFERS'} " f"{_now[k][:16]}") @@ -129,12 +141,14 @@ check("⭐ at N=1 and N=2, with the option absent and with it false, every " {k: v for k, v in _now.items() if k != "n2_maint"} == {k: v for k, v in A5DDDAD.items() if k != "n2_maint"}) check("...and n2_maint is the SAFETY-78 digest: the N=2 drain that a5dddad " - "refused on 392 of 540 cycles now runs (test_safety78.py)", + "refused on 392 of 540 cycles now runs (test_safety78.py), bounded by " + "SAFETY-85", _now["n2_maint"] == SAFETY78_N2_MAINT != A5DDDAD["n2_maint"]) -check("...and no cycle in those runs filed a `mode` claim or handed " - "`compute()` a bound", +check("...and no cycle in those runs filed a `mode` claim, and only " + "n2_maint's (SAFETY-85: maintenance at N=2) handed `compute()` a bound", _claims and not any(c.source == "mode" for cl in _claims for c in cl) - and _bounds and set(_bounds) == {None}) + and _bounds and set(_bounds) == {None} + and any(b is not None for b in _bounds_m)) # ============================================================================ # diff --git a/goodwe_controller/test_peak_margin.py b/goodwe_controller/test_peak_margin.py index f8da15c..19f09d9 100644 --- a/goodwe_controller/test_peak_margin.py +++ b/goodwe_controller/test_peak_margin.py @@ -67,8 +67,10 @@ for k in ED739D4: # SAFETY-84 changes n2_maint ON PURPOSE: on 9d5b7d1 its N=2 cheap-window # charge was refused on 214 of 540 cycles (site at 0 W); now none is. # Printed on the SAFETY-84 tree the same way (margin 0). +# SAFETY-85: the loop is bounded while A charges (reasons change, commands +# do not). Was 95722b262f1bcf504493d7b5babc3895799ac56cc15f5d2ccdbe7c9f04adfcbb. SAFETY84_N2_MAINT = \ - "95722b262f1bcf504493d7b5babc3895799ac56cc15f5d2ccdbe7c9f04adfcbb" + "ef97ae2e13668b3e0b2d22f3f5f37835ea46cd306c2cc475c97e284f9dcc6545" check("⭐ at N=1 and N=2, with and without battery modes and with a " "maintenance charge, margin 0 is " "byte-identical to ed739d4 (n2_maint apart, below)", diff --git a/goodwe_controller/test_safety78.py b/goodwe_controller/test_safety78.py index 2757003..9b3907f 100644 --- a/goodwe_controller/test_safety78.py +++ b/goodwe_controller/test_safety78.py @@ -332,15 +332,18 @@ print("\n5. the N=1 and N=2 digests") # newest action-log line hashed. N=2: the same on release/1.0 35cdffe. N=1: # re-pinned by SAFETY-83, whose `--diff` against 35cdffe changes only cycles # where the peak guard asked for more than the drain (test_safety83.py). +# SAFETY-85 re-pinned two: n1_drain_owner loses its false 'not keeping up' +# rows (commands unchanged); n2_drain's loop is bounded while A drains +# (reasons and the loop claim change, commands unchanged). PINNED = { "n1_drain": "b0f9ef44149163bc0acece537fe047871c00f0d9ea213e7a7fc8efc4ad3aa500", "n1_drain_modes": "95ab82426e23bce308db3edebe18b1ab37d956952e9f521adce37150ce87ee8a", "n1_drain_owner": - "10f7f42a6a952e899d739eb10c674874215c440f41ead7a9f2408bc9b4cb9ea4", + "607c2583c7ae4bdafb90d780b3414c62d76c430ca67798cd117d1a46f3bb40ae", "n2_drain": - "055d83de0552d3a014dc73d87fdd697dde8b5933b8032f9b3a464ff0d08795fb", + "5806a6f457b7baf8dfb44daddced502f1939c114e15b84029dbb2659be8c8b0f", "n2_drain_owner": "05750347f1829c249f99154613f068f442a59ef59f409093907f6d0add78dd4c", } diff --git a/goodwe_controller/test_safety84.py b/goodwe_controller/test_safety84.py index fcc0d4e..9fc4502 100644 --- a/goodwe_controller/test_safety84.py +++ b/goodwe_controller/test_safety84.py @@ -83,8 +83,11 @@ def banked(c): def alarmed(c): - """Did SAFETY-81's 'Load too high' alarm fire on this run?""" - return any("PEAK_GUARD_SATURATED" in e for e in c.events) + """Did SAFETY-81's 'Load too high' alarm fire on this run? Its quarter + stamp too, since ALLOCATOR rows can push its row out of the 40-row log + (SAFETY-85).""" + return c._peak_alarm_block is not None or any( + "PEAK_GUARD_SATURATED" in e for e in c.events) def run(house, n=2, **kw): @@ -465,6 +468,10 @@ print("\n8. the N=1 and N=2 digests") # 2026-09-30, 480 cycles each, every claim hashed. N=1 and n2_charge_dear: # the same on release/1.0 9d5b7d1 (the tool copied in). N=2 in a cheap # window: new, the charge that 9d5b7d1 refused on every cycle. +# SAFETY-85 re-pinned the three N=2 ones: the loop is bounded while A's +# charge pin stands. n2_charge and _banked change only the loop's reason and +# claim on 2 cycles; n2_charge_dear changes commands on 98 cycles, where A's +# pin lifts (t=1800) and B no longer jumps 1390 W, and after the surplus step. PINNED = { "n1_charge": "0ed25c856456b97acb07a39299a8cb7b1a762c4d0005b88a0823c03057a80be5", @@ -473,11 +480,11 @@ PINNED = { "n1_charge_banked": "2619acdd06b04e56c000b168d64e1d044346c6ba3f929c94258d6ad8189ec609", "n2_charge": - "748abe785a79084bc738c82d0d6f90b951e1cb78e9629195c0c01d3bb67b55e2", + "b16d692128f14bcce3d87b142d290e0d6b13724289680e44a05215c9eaa24a7c", "n2_charge_dear": - "c34f08b3b5b6256e68cf05030fb0c29522916dc8f57c351008f48384657966c0", + "00a197d89a5dd1e8b3ae6a5d1af66329a4dc4850476569c5931c125fa4d9eaee", "n2_charge_banked": - "fb600ee1dd3202910715c97c5809584d50c6b8b57b55aaa4dc83bcaaa97627ef", + "a6973faea916e4c88ca1770047f94478346eeb1efa4889b154fad0747f91267f", } _now = s84.digests() for k in PINNED: @@ -485,7 +492,7 @@ for k in PINNED: f"{_now[k][:16]}") check("⭐ at N=1 a charge in and out of a cheap window and under a banked " "month peak is byte-identical to 9d5b7d1 (commands, reasons and every " - "claim), and so is N=2 outside a cheap window", + "claim), and N=2 outside a cheap window matches its SAFETY-85 pin", all(_now[k] == PINNED[k] for k in ("n1_charge", "n1_charge_dear", "n1_charge_banked", "n2_charge_dear"))) diff --git a/goodwe_controller/test_safety85.py b/goodwe_controller/test_safety85.py new file mode 100644 index 0000000..28af008 --- /dev/null +++ b/goodwe_controller/test_safety85.py @@ -0,0 +1,303 @@ +"""Runnable check for SAFETY-85: no false 'not keeping up' while another owner +sets the battery. `python3 test_safety85.py` + +The loop's integrator bound fired every ~2.5 min during maintenance and under +the peak guard, though the battery gave what it was told: the loop's own ask, +re-seeded and pushed by a grid error it did not own, crossed its rail. Now the +loop's clamp counts only on a cycle the loop decided, a battery another owner +set alarms only when it gives less than told, and at N>=2 the loop is bounded +while maintenance holds a battery. Drives the REAL Controller +(tools/safety84_identity.py's harness). DECISIONS/SAFETY-85-no-false-not-keeping-up.md. + +1. N=1 and N=2: a drain, a charge and a HOLD with the house above what the + battery is set to, an hour each: no warning; without the fix they warn. +2. A battery that gives less than told during a drain does alarm (N=1, N=2); + a two-cycle dip does not. +3. Control coming back from maintenance does not jump (N=2 hold end, N=2 + charge pin lifting, N=1 hold end). +4. The loop's own alarm stays (house over the rating); a safety limit at N=2 + (a battery at its cut-out) does not warn. +5. The status row counts episodes, not cycles. +6. The rules at their boundaries. + +Mutations (MUTATION.md, purged copies) are listed in the pull request. +""" + +import logging +import os +import sys +from datetime import datetime, timedelta, timezone + +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 safety84_identity as s84 # noqa: E402 +import app.main as app_main # noqa: E402 +from app import clamp, messages # noqa: E402 +from app.main import Controller # noqa: E402 +from app.store import unit_key # noqa: E402 + +fails = [] +A = "inverter-a" +HOUR = 3600.0 +SLEW = 400.0 # the rig's slew_w +OPT0 = dict(s84.OPT) + + +def check(name, cond): + print(f" {'ok ' if cond else 'FAIL'} {name}") + if not cond: + fails.append(name) + + +async def _call(self, *a): + """The rig's Home Assistant, recording each notification id it is sent.""" + if len(a) >= 3 and a[1] == "create" and isinstance(a[2], dict): + self.__dict__.setdefault("notes", []).append( + a[2].get("notification_id")) + return True + + +rig._Hass.call = _call + + +def store(n, phase): + """Battery A in `phase` from the first cycle ('drain': due at 11:00; + 'idle': only with `maint=False`).""" + if phase == "drain": + return rig._Store() + if phase == "charge": + return s79.store_charging(n) + s = rig._Store() + if phase == "hold": + for k, v in (("phase", "hold"), ("phase_started", + rig.BASE.isoformat())): + s.data[k if n == 1 else unit_key(A, k)] = v + return s + + +def run(n, phase, house, cap, *, dur=HOUR, soc=None, cut=None, extra=None, + maint=True): + """Rows and controller; `cap` is the peak limit (20000: never binds).""" + s84.OPT = dict(OPT0, peak_cap_w=cap, maintenance_enabled=maint) + if soc is None: + def soc(u, t): + return 99.0 if phase == "hold" and u == "a" else 60.0 + try: + return s84.run(n, load=lambda t: house, cheap=(phase == "charge"), + store=store(n, phase), soc=soc, dur=dur, cut=cut, + extra=dict({"maintenance_hold_min": 600, + "maintenance_discharge_w": 2000}, + **(extra or {}))) + finally: + s84.OPT = OPT0 + + +def warned(c): + """'Battery is not keeping up': notifications, cycles, episodes.""" + return (getattr(c.hass, "notes", []).count(messages.ID_LOOP_BOUND), + c.clamp_wd.bound, c.clamp_wd.bound_episodes) + + +def patched(obj, name, value): + """Run `fn` with `obj.name` replaced (the 'without the fix' runs).""" + def wrap(fn): + real = getattr(obj, name) + setattr(obj, name, value) + try: + return fn() + finally: + setattr(obj, name, real) + return wrap + + +no_gate = patched(Controller, "_loop_alarm_input", lambda self, r: None) +no_bound = patched(Controller, "loop_bound", lambda self, claims: None) + + +def cmd(r): + return dict(r[2]) + + +def steps(rows, lo, hi): + """The largest change in the total command between cycles in [lo, hi].""" + tot = [(r[0], sum(cmd(r).values())) for r in rows] + return max(abs(b - a) for (t0, a), (t1, b) in zip(tot, tot[1:]) + if lo <= t1 <= hi) + + +# ============================================================================ # +print("1. the house above what the battery is set to, an hour each") +CASES = [ + # (n, phase, house, cap): the peak guard raises the drain / the HOLD at a + # 3500 W limit; the rest are the phase alone under a house that pushes the + # loop's re-seeded ask past its rail. + (1, "drain", 6500.0, 3500), (1, "drain", 7500.0, 20000), + (1, "charge", 10000.0, 20000), + (1, "hold", 6500.0, 3500), (1, "hold", 9000.0, 20000), + (2, "drain", 6500.0, 3500), (2, "charge", 10000.0, 20000), + (2, "hold", 4500.0, 20000), (2, "hold", 6500.0, 3500), +] +NEW, OLD = {}, {} +for k in CASES: + NEW[k] = run(*k) + OLD[k] = no_gate(lambda: no_bound(lambda: run(*k))) + print(f" N={k[0]} {k[1]:6} house {k[2]:5.0f} limit {k[3]:5}: phases " + f"{sorted({p for r in NEW[k][0] for p in r[-1][3]})}, warnings " + f"(notes, cycles, episodes) {warned(NEW[k][1])}, without the fix " + f"{warned(OLD[k][1])}") +check("⭐ no 'Battery is not keeping up' notification, cycle or episode in any " + "of the nine hours, at N=1 and N=2", + all(warned(c) == (0, 0, 0) for _r, c in NEW.values())) +check("...the phase held all hour (the battery was set by maintenance, or by " + "the guard over it, throughout)", + all(all(r[-1][3][0] == k[1] for r in NEW[k][0]) for k in CASES)) +check("non-vacuous: without the fix every one of the nine warns, the " + "notification re-created on the re-assert cadence", + all(warned(c)[0] >= 1 and warned(c)[1] > 0 for _r, c in OLD.values()) + and sum(warned(c)[0] for _r, c in OLD.values()) >= 9 * 5) + +# ============================================================================ # +print("\n2. a battery that really gives less than told still alarms") + + +def gone(t0, t1): + """Battery A gives nothing from t0 to t1.""" + return lambda u, t: u == "a" and t0 <= t < t1 + + +SHORT = {n: run(n, "drain", 1000.0, 20000, dur=1200.0, cut=gone(600, 1200)) + for n in (1, 2)} +for n, (rows, c) in SHORT.items(): + print(f" N={n}: {warned(c)}, log " + f"{[e for e in c.events if 'OUTPUT_CLAMP' in e][:1]}") +_r1, _c1 = SHORT[1] +check("⭐ N=1, a drain of 2000 W that gives nothing: it warns, and the log " + "says what was asked and what came", + warned(_c1)[0] >= 1 and any( + "the battery gives -4 W of the +2000 W asked" in e + for e in _c1.events)) +_r2, _c2 = SHORT[2] +check("⭐ N=2, the same on battery A while B follows the house: it warns and " + "names inverter-a", + warned(_c2)[0] >= 1 and any( + "inverter-a gives -4 W of the +2000 W asked" in e + for e in _c2.events)) +_hb = run(2, "hold", 4500.0, 20000, dur=1200.0, + cut=lambda u, t: u == "b" and t >= 600) +print(f" N=2 HOLD on A, B gives nothing: {warned(_hb[1])}") +check("⭐ N=2, A in its HOLD and B, which the loop drives, gives nothing: the " + "loop stands at what B may give, and it warns naming inverter-b", + warned(_hb[1])[0] >= 1 and any( + "inverter-b gives -4 W of the +2500 W asked" in e + for e in _hb[1].events)) +check("...it starts no earlier than the saturation cycles allow (3 x 5 s)", + all(e.split(" ", 1)[0] >= "11:10:10" for c in (_c1, _c2) + for e in c.events if "OUTPUT_CLAMP" in e)) +_quiet = patched(app_main, "delivery_short", lambda *a: False)( + lambda: run(1, "drain", 1000.0, 20000, dur=1200.0, cut=gone(600, 1200))) +check("non-vacuous: without the delivery rule the N=1 drain that gives " + "nothing is silent (the loop's clamp never reaches its rail here)", + warned(_quiet[1]) == (0, 0, 0)) +_dip = run(1, "drain", 1000.0, 20000, dur=900.0, cut=gone(600, 610)) +check("a dip of two cycles is not an alarm", warned(_dip[1]) == (0, 0, 0)) + +# ============================================================================ # +print("\n3. control coming back from maintenance does not jump") +END = {"maintenance_hold_min": 10} +_h2 = run(2, "hold", 4500.0, 20000, dur=900.0, extra=END) +_h2_old = no_bound(lambda: run(2, "hold", 4500.0, 20000, dur=900.0, + extra=END)) +_h1 = run(1, "hold", 4500.0, 20000, dur=900.0, extra=END) +s84.OPT = dict(OPT0) +_cd = s84.run(2, cheap=False) +_cd_old = no_bound(lambda: s84.run(2, cheap=False)) +print(f" N=2 hold end: largest step {steps(_h2[0], 560, 700):.0f} W " + f"(without the bound {steps(_h2_old[0], 560, 700):.0f} W); N=1 " + f"{steps(_h1[0], 560, 700):.0f} W; N=2 charge pin lifts at 1800 s: " + f"{steps(_cd[0], 1780, 1840):.0f} W (without " + f"{steps(_cd_old[0], 1780, 1840):.0f} W)") +check("⭐ N=2, the HOLD ends under a 4500 W house: the batteries move at most " + "one slew step a cycle", + _h2[1].maints[0].phase != "hold" and steps(_h2[0], 560, 700) <= SLEW) +check("⭐ N=2, A's charge pin lifts at a quarter's start: at most one slew " + "step a cycle", steps(_cd[0], 1780, 1840) <= SLEW) +check("...and N=1 already moved one slew step at a time (re-seeded each " + "cycle), unchanged", steps(_h1[0], 560, 700) <= SLEW) +check("non-vacuous: without the bound the N=2 loop stands wound at its rail " + "and both jump (2500 W at the hold's end)", + steps(_h2_old[0], 560, 700) >= 2000.0 + and steps(_cd_old[0], 1780, 1840) >= 1000.0) + +# ============================================================================ # +print("\n4. what still warns, and what a safety limit does") +_rail = run(1, "drain", 6500.0, 20000, dur=600.0, maint=False) +print(f" N=1 no maintenance, house 6500 over a 5000 W rating: " + f"{warned(_rail[1])}") +check("⭐ the loop in charge at its rail (house over the rating) still warns, " + "as SAFETY-40 decided", warned(_rail[1])[0] >= 1) + + +def at_cutout(u, t): + return 9.0 if u == "a" else 60.0 + + +_cut = run(2, "idle", 6000.0, 20000, dur=900.0, soc=at_cutout, maint=False) +_cut_old = no_gate(lambda: run(2, "idle", 6000.0, 20000, dur=900.0, + soc=at_cutout, maint=False)) +print(f" N=2, A at its cut-out, house 6000: {warned(_cut[1])}, without " + f"the fix {warned(_cut_old[1])}") +check("⭐ N=2, A at its cut-out level (a safety limit sets it to 0 W) and B " + "at its rating: no warning, each battery gives what it is told", + warned(_cut[1]) == (0, 0, 0) and cmd(_cut[0][-1]) == {"a": 0.0, + "b": 2500.0}) +check("non-vacuous: without the fix it warns", + warned(_cut_old[1])[0] >= 1) + +# ============================================================================ # +print("\n5. the status row counts episodes") +_wd = clamp.ClampWatchdog() +_t = datetime(2026, 9, 20, 11, 0, tzinfo=timezone.utc) +for ep in range(3): + for i in range(10): + _wd.note(True, "x", _t, bug=False) + _t += timedelta(seconds=5) + for i in range(clamp.CLEAR_CYCLES): + _wd.note(False, "", _t) + _t += timedelta(seconds=5) +_wd.note(True, "y", _t, bug=True) +check("three bound episodes of ten cycles and one over-rating: 3 episodes, " + "30 cycles", (_wd.bound_episodes, _wd.bound, _wd.episodes) == (3, 30, 4)) +_rows = [x for x in _c1.checks() if x.get("id") == "not_keeping_up"] +check("⭐ the page row reads the episodes, not the cycles " + f"({_c1.clamp_wd.bound_episodes} vs {_c1.clamp_wd.bound})", + _rows and _c1.clamp_wd.bound > _c1.clamp_wd.bound_episodes >= 1 + and _rows[0]["text"].startswith(messages.T_LOOP_BOUND + ": " + + messages.times( + _c1.clamp_wd.bound_episodes))) + +# ============================================================================ # +print("\n6. the rules at their boundaries") +DS = [((2000, 1499, 500), True), ((2000, 1500, 500), False), + ((2000, 2600, 500), False), ((-2000, -1499, 500), True), + ((-2000, -2600, 500), False), ((0, -900, 500), False), + ((0, 900, 500), False), + ((2000, -4, 500), True), ((-2000, 600, 500), True)] +check("delivery_short: more than the margin short in the asked direction; " + "more than asked, or 0 W asked, is never short", + all(clamp.delivery_short(*a) is want for a, want in DS)) +check("the note names each battery, asked before given", + clamp.shortfall_note([("inverter-a", 2000.0, -4.0)]) + == "OUTPUT_CLAMP: inverter-a gives -4 W of the +2000 W asked" + and clamp.shortfall_note([(None, -2000.0, 0.0)]) + == "OUTPUT_CLAMP: the battery gives +0 W of the -2000 W asked") + +print() +if fails: + print(f"{len(fails)} FAILED: " + ", ".join(fails)) + sys.exit(1) +print("all SAFETY-85 checks passed") diff --git a/goodwe_controller/test_safety86.py b/goodwe_controller/test_safety86.py index 35a4d4a..253d6b1 100644 --- a/goodwe_controller/test_safety86.py +++ b/goodwe_controller/test_safety86.py @@ -102,8 +102,11 @@ def on_aim(c, aim=AIM): def alarmed(c): - """Did SAFETY-81's 'Load too high' alarm fire on this run?""" - return any("PEAK_GUARD_SATURATED" in e for e in c.events) + """Did SAFETY-81's 'Load too high' alarm fire on this run? Its quarter + stamp too, since ALLOCATOR rows can push its row out of the 40-row log + (SAFETY-85).""" + return c._peak_alarm_block is not None or any( + "PEAK_GUARD_SATURATED" in e for e in c.events) def refused(rows): @@ -231,8 +234,21 @@ def rows_logged(c, word): return sum(1 for e in c.events if word in e) +def no_bound(fn): + """Run without SAFETY-85's loop bound (modes are off here).""" + real = Controller.loop_bound + Controller.loop_bound = lambda self, claims: None + try: + return fn() + finally: + Controller.loop_bound = real + + _nz_rows, _nz = run(noisy, 2) -_nz_old = no_ceiling(lambda: run(noisy, 2))[1] +# SAFETY-85's loop bound also keeps the loop inside what the batteries can +# give, so the flood needs both the ceiling and the bound taken out. +_nz_old = no_bound(lambda: no_ceiling(lambda: run(noisy, 2)))[1] +_nz_nc = no_ceiling(lambda: run(noisy, 2))[1] print(f" noisy house: ALLOCATOR rows {rows_logged(_nz, 'ALLOCATOR')}, " f"clamp rows {rows_logged(_nz, 'OUTPUT_CLAMP')} (without the site " f"ceiling {rows_logged(_nz_old, 'ALLOCATOR')} and " @@ -244,9 +260,13 @@ check("⭐ while A gives, the site target is the guard's floor (the `hold` " if cmd(r)["a"] > 0.0) and rows_logged(_nz, "ALLOCATOR") == 0 and rows_logged(_nz, "OUTPUT_CLAMP") <= 1) -check("non-vacuous: without the ceiling the loop asks 5000 W and the " - "ALLOCATOR and clamp rows fill the 40-row log", +check("non-vacuous: without the ceiling and SAFETY-85's bound the loop asks " + "5000 W and the ALLOCATOR and clamp rows fill the 40-row log", rows_logged(_nz_old, "ALLOCATOR") >= 10 and len(_nz_old.events) == 40) +check("...and either one alone keeps it clean (without the ceiling: " + f"{rows_logged(_nz_nc, 'ALLOCATOR')} ALLOCATOR rows)", + on_aim(_nz_nc) and rows_logged(_nz_nc, "ALLOCATOR") == 0 + and rows_logged(_nz_nc, "OUTPUT_CLAMP") <= 1) # ============================================================================ # print("\n4. a month peak banked above the limit") -- 2.54.0 From 7b723a6cd5d83a989457aaeaba811e0a211c0425 Mon Sep 17 00:00:00 2001 From: SAFETY-85 review Date: Thu, 1 Oct 2026 20:25:41 +0200 Subject: [PATCH 2/2] SAFETY-85 review: grade discharge only, over the plant's settle time Rule 2 graded each battery against last cycle's command and graded charges too. On plantsim's battery (10 s lag) at the 2 s meter it warned at every drain or charge start and 15 times an hour under the peak guard at N=2; a charge tapering near full warned 11 times in 50 min (SAFETY-42 section 7). Now a discharge is graded against the least it stood at over SHORT_CYCLES / SHORT_SPAN_S. DOCS lists `mode-bound` under Loop reason. test_safety85 part 7: peak guard with a dead battery, lagging plant, taper; part 3: the bound ends with maintenance. Co-Authored-By: Claude Opus 5.5 --- .../SAFETY-85-no-false-not-keeping-up.md | 37 ++-- goodwe_controller/DOCS.md | 22 ++- goodwe_controller/app/clamp.py | 8 +- goodwe_controller/app/main.py | 34 +++- goodwe_controller/test_safety85.py | 165 +++++++++++++++++- 5 files changed, 222 insertions(+), 44 deletions(-) diff --git a/goodwe_controller/DECISIONS/SAFETY-85-no-false-not-keeping-up.md b/goodwe_controller/DECISIONS/SAFETY-85-no-false-not-keeping-up.md index 09cf889..778e326 100644 --- a/goodwe_controller/DECISIONS/SAFETY-85-no-false-not-keeping-up.md +++ b/goodwe_controller/DECISIONS/SAFETY-85-no-false-not-keeping-up.md @@ -25,14 +25,16 @@ counter counted cycles (861), not episodes. the loop. Its number is on the wire, or only a rating (`device`, `supervised`) cut it. No bound held it, and nothing was unmet. (`Controller._loop_alarm_input`.) -2. **A battery another owner set alarms when it gives less than it was told.** - This covers every battery on a cycle the loop did not decide, and a battery - with its own pin otherwise. The test is more than `saturation_w` short in - the told direction, for `saturation_cycles` cycles in a row, against what - stands on the wire (`clamp.delivery_short`). It covers a real shortfall - during maintenance. Before, that shortfall was silent unless the kick - happened to reach the rail (house 1000 W, drain 2000 W, battery gives - nothing: 0 warnings before, 4 in 10 min now). +2. **A battery another owner set to discharge alarms when it gives less than + it was told.** This covers every battery on a cycle the loop did not + decide, and a battery with its own pin otherwise. The test is more than + `saturation_w` short of the least it stood at over `SHORT_CYCLES` cycles + and `SHORT_SPAN_S` (the plant's measured settle, SAFETY-13/73), for + `saturation_cycles` cycles in a row (`clamp.delivery_short`). It covers a + real shortfall during maintenance or under the peak guard. Before, that + shortfall was silent unless the kick happened to reach the rail (house + 1000 W, drain 2000 W, battery gives nothing: 0 warnings before, 4 in 10 min + now). 3. **At N>=2, while maintenance holds a battery, the loop is bounded** by OPT-11's `loop_bound()` (what the batteries may place). It does not wind, and control comes back one slew step a cycle. Why maintenance only: that is @@ -69,12 +71,21 @@ batteries can give. The `alarmed()` helpers in test_safety84/86 also read SAFETY-81's quarter stamp, because ALLOCATOR rows can push its row out of the 40-row log. +**Review (PR #163).** The first draft graded each battery against last +cycle's command and graded charges too. On plantsim's battery (10 s lag) at +the live 2 s meter, every drain or charge start warned once, and an N=2 drain +or HOLD under the peak guard with a stepping house warned 15 times an hour. A +maintenance charge tapering near full warned 11 times in 50 min, which +SAFETY-42 section 7 had ruled out. With the settle window and discharge only: +0 in all of those cases. A battery that gives nothing still warns, lagging +or not. At a hypothetical 20 s lag, it warns at most twice an hour. The cost: +a battery that does not take a charge another owner set is not named by this +alarm (the same gap SAFETY-42 states; the charge has its 36 h timeout). +`test_safety85.py` part 7. + **Not changed, open for the owner.** - With the loop in charge and the house above the batteries' total rating, it still warns (SAFETY-40). On the rig's 2 x 2000 W N=2, most of SAFETY-84's count came from houses of 4500 and 6500 W. -- A battery that tapers near full during a charge, more than `saturation_w` - under its command for 15 s, now warns (rule 2). The text already says - "check it is not empty or full". -- TEL-10's `loop_reason` reads `mode-bound` while the maintenance bound holds - the loop, with modes off. +- TEL-10's `Loop reason` reads `mode-bound` while the maintenance bound holds + the loop, with modes off. DOCS lists the value with both meanings. diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md index d246895..2d0964b 100644 --- a/goodwe_controller/DOCS.md +++ b/goodwe_controller/DOCS.md @@ -577,7 +577,7 @@ accumulator is doing: | `Loop integrator lead` | W | that value minus **the command it was computed against**, i.e. the command the previous cycle left standing | | `Loop reason` | — | the law's own word for this cycle | -`Loop reason` takes one of ten values: +`Loop reason` takes one of eleven values: | value | what happened | |---|---| @@ -588,6 +588,7 @@ accumulator is doing: | `slew-limited` | the command could not move as far as the integrator asked for; `slew_w` is what stopped it | | `saturated-freeze` | the inverter is not following, so the command's magnitude may fall but not rise | | `freeze-released` | the freeze had run out of plant-shaped explanations and the command was let move again | +| `mode-bound` | the batteries' limits set the command, not the meter: their battery modes, or, with two or more inverters, a battery in its monthly maintenance | | `not-running` | control is stopped **and** shadow mode is off, so the loop did not run | | `battery refuses` | the battery gave less than 100 W for 12 cycles (a minute at a 5-second meter) while asked for more than 500 W, so the add-on asks 0 W in that direction (see below) | | `battery probe` | the add-on asks 200 W to test whether a refusing battery gives power again | @@ -1954,10 +1955,12 @@ notification budget. Why: [`DECISIONS/SAFETY-40-the-integrator-bound-is-not-a-bug.md`](DECISIONS/SAFETY-40-the-integrator-bound-is-not-a-bug.md). The integrator bound counts only on a cycle the loop decided. While -maintenance, the peak guard or a safety limit sets a battery, the same alarm -fires only when that battery gives more than `saturation_w` less than it was -told, for `saturation_cycles` cycles. At two or more inverters the loop does not -wind while maintenance holds a battery. +maintenance, the peak guard or a safety limit sets a battery to discharge, the +same alarm fires only when that battery gives more than `saturation_w` less +than the least it was told over the plant's settle time (at least 23 s), for +`saturation_cycles` cycles. A charge is not graded: a full battery takes less +as it tops up. At two or more inverters the loop does not wind while +maintenance holds a battery. Why: [`DECISIONS/SAFETY-85-no-false-not-keeping-up.md`](DECISIONS/SAFETY-85-no-false-not-keeping-up.md). ### The frozen-meter alarm @@ -3130,8 +3133,9 @@ short time, for example just after a large load switches on or the sun comes out. It is not a fault, and control keeps running. If it keeps coming back, check that the battery is switched on, and that it is not empty (when it should give power) or full (when it should charge). During the monthly maintenance or -while the peak limit is guarded, it only shows when the battery gives less than -it was told. The status page counts how many times it happened. +while the peak limit is guarded, it only shows when the battery gives +clearly less than it was told. The status page counts how many times it +happened. ### Power request too high @@ -3237,8 +3241,8 @@ link cannot jump to the section. **Battery is not keeping up.** The grid-following loop's integrator reached its anti-windup bound (`integrator_max_w`, SAFETY-40), or a battery that another -owner set gave less than it was told (SAFETY-85). No command went over the -rating. The log line is a WARNING with reason code `OUTPUT_CLAMP`. See *The +owner set to discharge gave less than it was told (SAFETY-85). No command went +over the rating. The log line is a WARNING with reason code `OUTPUT_CLAMP`. See *The winding-loop alarm*. **Power request too high.** A computed command above a limit was cut before it diff --git a/goodwe_controller/app/clamp.py b/goodwe_controller/app/clamp.py index e690faf..16816a3 100644 --- a/goodwe_controller/app/clamp.py +++ b/goodwe_controller/app/clamp.py @@ -270,12 +270,10 @@ def shortfall_note(named) -> str: def delivery_short(asked_w, given_w, margin_w) -> bool: - """SAFETY-85: did a battery give more than `margin_w` less than asked, in - the asked direction? Giving more than asked, or 0 W asked, is not short.""" + """SAFETY-85: did a battery asked to discharge give more than `margin_w` + less? A charge is never short: a full pack tapers (SAFETY-42 section 7).""" asked, given = float(asked_w), float(given_w) - if asked == 0.0: - return False - return (asked - given) * (1.0 if asked > 0 else -1.0) > abs(float(margin_w)) + return asked > 0.0 and asked - given > abs(float(margin_w)) def write_clamp_note(named) -> str: diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index ec4617f..a673fe3 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -281,6 +281,11 @@ def pack_energy_of(inv): # ordinary load steps. At 2 s and slower this changes nothing. REFUSED_SPAN_S = (_ctl.SETTLE_CYCLES - 1) * _ctl.LOOP_TICK_S +# SAFETY-85 review: a battery another owner set is graded against the least it +# 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 + def _refused_interval(sign: int) -> tuple: """SAFETY-73: the (lo, hi) a refusing unit may be asked: 0 W that way.""" @@ -1180,9 +1185,11 @@ class Controller: self.loop_clamped: tuple[bool, str, bool] = (False, "", False) # UX-08: the numbers of a loop-side over-rating, for the notification. self.loop_clamp_facts: tuple = () - # SAFETY-85: this cycle's loop Decision, and how many cycles in a row - # a battery set by another owner gave less than asked. + # SAFETY-85: this cycle's loop Decision, each battery's recent + # standing commands, and how many cycles in a row a battery set by + # another owner gave less than asked. self._loop_decision = None + self._asked_hist: dict = {} self._short_run = 0 self.stopping = False # SAFETY-61. time.monotonic() at the end of the last run_control() pass. @@ -5084,25 +5091,36 @@ class Controller: def _loop_alarm_input(self, resolution) -> None: """SAFETY-85. The loop's clamp counts only on a cycle the loop decided - (a rating may cut it). A battery another owner set counts when it gave - less than it was told for `saturation_cycles` cycles in a row. - DECISIONS/SAFETY-85-no-false-not-keeping-up.md.""" + (a rating may cut it). A battery another owner set to discharge counts + when it gave less than it was told for `saturation_cycles` cycles in a + row. DECISIONS/SAFETY-85-no-false-not-keeping-up.md.""" d = self._loop_decision if d is None: self._short_run = 0 + self._asked_hist = {} return unmet = self.allocation.unmet_w if self.allocation is not None else 0 cut = (resolution.bound_by or "").split("(")[0] owns = (resolution.winner == "loop" and not d.held and not unmet and (resolution.target_w == d.target_w or cut in ("device", "supervised"))) - # Graded against what stands on the wire (0 W in shadow mode). + # Graded against the least that stood on the wire (0 W in shadow + # mode) over SHORT_CYCLES cycles and SHORT_SPAN_S seconds. + now, asked = self._now_mono, {} + for inv in self.inverters: + h = self._asked_hist.setdefault(inv.name, []) + h.append((now, inv.standing_w())) + while len(h) > SHORT_CYCLES and ( + now is None or h[0][0] is None + or now - h[0][0] > SHORT_SPAN_S): + h.pop(0) + asked[inv.name] = min(w for _t, w in h) others = [inv for inv in self.inverters if not owns or allocator.pin_w(resolution.unit_bounds.get(inv.name)) is not None] - short = [(inv, inv.standing_w(), inv.batt) for inv in others + short = [(inv, asked[inv.name], inv.batt) for inv in others if inv.batt is not None - and delivery_short(inv.standing_w(), inv.batt, + and delivery_short(asked[inv.name], inv.batt, self.tuning.saturation_w)] self._short_run = self._short_run + 1 if short else 0 if owns and self.loop_clamped[0]: diff --git a/goodwe_controller/test_safety85.py b/goodwe_controller/test_safety85.py index 28af008..ed70653 100644 --- a/goodwe_controller/test_safety85.py +++ b/goodwe_controller/test_safety85.py @@ -19,6 +19,8 @@ while maintenance holds a battery. Drives the REAL Controller (a battery at its cut-out) does not warn. 5. The status row counts episodes, not cycles. 6. The rules at their boundaries. +7. Review: a battery the peak guard sets that gives nothing warns; a battery + that lags (10 s) and a charge that tapers do not. Mutations (MUTATION.md, purged copies) are listed in the pull request. """ @@ -82,13 +84,15 @@ def store(n, phase): def run(n, phase, house, cap, *, dur=HOUR, soc=None, cut=None, extra=None, maint=True): - """Rows and controller; `cap` is the peak limit (20000: never binds).""" + """Rows and controller; `cap` is the peak limit (20000: never binds); + `house` is watts or a function of time.""" s84.OPT = dict(OPT0, peak_cap_w=cap, maintenance_enabled=maint) if soc is None: def soc(u, t): return 99.0 if phase == "hold" and u == "a" else 60.0 try: - return s84.run(n, load=lambda t: house, cheap=(phase == "charge"), + load = house if callable(house) else (lambda t: house) + return s84.run(n, load=load, cheap=(phase == "charge"), store=store(n, phase), soc=soc, dur=dur, cut=cut, extra=dict({"maintenance_hold_min": 600, "maintenance_discharge_w": 2000}, @@ -209,7 +213,20 @@ check("a dip of two cycles is not an alarm", warned(_dip[1]) == (0, 0, 0)) # ============================================================================ # print("\n3. control coming back from maintenance does not jump") END = {"maintenance_hold_min": 10} -_h2 = run(2, "hold", 4500.0, 20000, dur=900.0, extra=END) +_bounds = [] +_real_compute = app_main.compute + + +def _spy_compute(*a, **kw): + _bounds.append(kw.get("bound")) + return _real_compute(*a, **kw) + + +app_main.compute = _spy_compute +try: + _h2 = run(2, "hold", 4500.0, 20000, dur=900.0, extra=END) +finally: + app_main.compute = _real_compute _h2_old = no_bound(lambda: run(2, "hold", 4500.0, 20000, dur=900.0, extra=END)) _h1 = run(1, "hold", 4500.0, 20000, dur=900.0, extra=END) @@ -224,6 +241,14 @@ print(f" N=2 hold end: largest step {steps(_h2[0], 560, 700):.0f} W " check("⭐ N=2, the HOLD ends under a 4500 W house: the batteries move at most " "one slew step a cycle", _h2[1].maints[0].phase != "hold" and steps(_h2[0], 560, 700) <= SLEW) +_held = [any(p != "idle" for p in r[-1][3]) for r in _h2[0]] +print(f" bound per cycle: {len(_bounds)} calls, {len(_held)} rows, " + f"{sum(_held)} in maintenance, " + f"{sum(b is not None for b in _bounds)} bounded") +check("...and the bound is on while A holds and off from the cycle the HOLD " + "ends (review)", + len(_bounds) == len(_held) and any(not h for h in _held) + and all((b is not None) == h for b, h in zip(_bounds, _held))) check("⭐ N=2, A's charge pin lifts at a quarter's start: at most one slew " "step a cycle", steps(_cd[0], 1780, 1840) <= SLEW) check("...and N=1 already moved one slew step at a time (re-seeded each " @@ -283,12 +308,11 @@ check("⭐ the page row reads the episodes, not the cycles " # ============================================================================ # print("\n6. the rules at their boundaries") DS = [((2000, 1499, 500), True), ((2000, 1500, 500), False), - ((2000, 2600, 500), False), ((-2000, -1499, 500), True), - ((-2000, -2600, 500), False), ((0, -900, 500), False), - ((0, 900, 500), False), - ((2000, -4, 500), True), ((-2000, 600, 500), True)] -check("delivery_short: more than the margin short in the asked direction; " - "more than asked, or 0 W asked, is never short", + ((2000, 2600, 500), False), ((2000, -4, 500), True), + ((0, -900, 500), False), ((0, 900, 500), False), + ((-2000, -1499, 500), False), ((-2000, 600, 500), False)] +check("delivery_short: a discharge more than the margin short; more than " + "asked, 0 W asked, or a charge (a full pack tapers) is never short", all(clamp.delivery_short(*a) is want for a, want in DS)) check("the note names each battery, asked before given", clamp.shortfall_note([("inverter-a", 2000.0, -4.0)]) @@ -296,6 +320,129 @@ check("the note names each battery, asked before given", and clamp.shortfall_note([(None, -2000.0, 0.0)]) == "OUTPUT_CLAMP: the battery gives +0 W of the -2000 W asked") +# ============================================================================ # +print("\n7. review: the peak guard, a battery that lags, a charge that tapers") +import math # noqa: E402 + + +def under_guard(n, cut): + """A HOLD the peak guard raises (house 6500, limit 3500), 20 min.""" + return run(n, "hold", 6500.0, 3500, dur=1200.0, cut=cut) + + +_g1 = under_guard(1, lambda u, t: t >= 600) +_g2 = under_guard(2, lambda u, t: u == "a" and t >= 600) +print(f" under the guard, the battery gives nothing: N=1 {warned(_g1[1])}, " + f"N=2 {warned(_g2[1])}") +check("⭐ the peak guard sets the battery and it gives nothing: it warns at " + "N=1, and at N=2 (A, raised for the peak, gives nothing; B gives all " + "it is told)", + warned(_g1[1])[0] >= 1 and warned(_g2[1])[0] >= 1 + and all("peak" in r[6] for rows, _c in (_g1, _g2) for r in rows + if 600 <= r[0] <= 620)) +check("...and it gives what it is told until then (no warning before 600 s)", + all(e.split(" ", 1)[0] >= "11:10:00" for c in (_g1[1], _g2[1]) + for e in c.events if "OUTPUT_CLAMP" in e)) + + +def lagging(fn, lag=10.0): + """Run `fn` on plantsim's battery (800 W/s ramp, `lag` s first-order lag) + at the live 2 s meter tick.""" + real_p, real_t = rig._Hass.power, rig.TICK + + def power(self, u): + st = self.__dict__.setdefault("_plant", {}) + a, tgt, tl = st.get(u, (0.0, 0.0, self.t)) + for _ in range(int(round((self.t - tl) / 0.1))): + d = self.cmd[u] - tgt + tgt += math.copysign(min(abs(d), 80.0), d) + a += (tgt - a) * (1 - math.exp(-0.1 / lag)) + st[u] = (a, tgt, self.t) + if self.cut(u, self.t) and self.cmd[u] >= -4.0: + return -4.0 + return float(round(a)) + rig._Hass.power, rig.TICK = power, 2.0 + try: + return fn() + finally: + rig._Hass.power, rig.TICK = real_p, real_t + + +def kettle(t): + """800 W, plus 2 kW 3 min in 10, plus 3 kW 1 min in 4.""" + return (800.0 + (2000.0 if t % 600 < 180 else 0.0) + + (3000.0 if t % 240 < 60 else 0.0)) + + +def lag_runs(): + return [run(n, ph, kettle, 3500, dur=1800.0) + for n, ph in ((1, "drain"), (2, "drain"), (2, "hold"))] + + +_lag = lagging(lag_runs) +_lag_old = patched(app_main, "SHORT_CYCLES", 1)( + lambda: patched(app_main, "SHORT_SPAN_S", 0.0)( + lambda: lagging(lag_runs))) +_lag_dead = lagging(lambda: run(1, "drain", 1000.0, 20000, dur=900.0, + cut=gone(600, 900))) +print(f" lagging battery, house steps under the guard: " + f"{[warned(c) for _r, c in _lag]}, graded on last cycle's command " + f"{[warned(c) for _r, c in _lag_old]}; gives nothing " + f"{warned(_lag_dead[1])}") +check("⭐ a battery that lags (10 s) behind maintenance and peak-guard steps " + "at a 2 s meter does not warn (N=1 drain, N=2 drain and HOLD, 30 min)", + all(warned(c) == (0, 0, 0) for _r, c in _lag)) +check("non-vacuous: graded on last cycle's command alone, it warns", + sum(warned(c)[0] for _r, c in _lag_old) >= 3) +check("...and a lagging battery that gives nothing still warns", + warned(_lag_dead[1])[0] >= 1) + + +def taper(fn): + """Battery A takes the full charge until 600 s, then its acceptance falls + to 150 W over 30 min (constant-voltage taper).""" + real_p = rig._Hass.power + + def power(self, u): + c = self.cmd[u] + if c < 0 and u == "a": + c = max(c, -max(150.0, 2500.0 - max(0.0, self.t - 600) / 1800 + * 2350.0)) + return float(round(c)) + rig._Hass.power = power + try: + return fn() + finally: + rig._Hass.power = real_p + + +def charge_runs(): + s84.OPT = dict(OPT0, peak_cap_w=20000) + try: + return [s84.run(n, load=lambda t: 800.0, cheap=True, + store=s79.store_charging(n), soc=lambda u, t: 60.0, + dur=2400.0) for n in (1, 2)] + finally: + s84.OPT = OPT0 + + +def both_ways(asked, given, margin): + asked, given = float(asked), float(given) + return asked != 0.0 and (asked - given) * math.copysign(1.0, asked) > margin + + +_tp = taper(charge_runs) +_tp_old = patched(app_main, "delivery_short", both_ways)( + lambda: taper(charge_runs)) +print(f" a charge that tapers: {[warned(c) for _r, c in _tp]}, grading " + f"charges too {[warned(c) for _r, c in _tp_old]}") +check("⭐ a maintenance charge that tapers near full does not warn (N=1, N=2)", + all(warned(c) == (0, 0, 0) for _r, c in _tp) + and all("charge" in {p for r in rows for p in r[-1][3]} + for rows, _c in _tp)) +check("non-vacuous: grading charges too, the taper warns", + all(warned(c)[0] >= 3 for _r, c in _tp_old)) + print() if fails: print(f"{len(fails)} FAILED: " + ", ".join(fails)) -- 2.54.0