diff --git a/goodwe_controller/DECISIONS/SAFETY-40-the-integrator-bound-is-not-a-bug.md b/goodwe_controller/DECISIONS/SAFETY-40-the-integrator-bound-is-not-a-bug.md new file mode 100644 index 0000000..521c806 --- /dev/null +++ b/goodwe_controller/DECISIONS/SAFETY-40-the-integrator-bound-is-not-a-bug.md @@ -0,0 +1,28 @@ +# SAFETY-40: the integrator bound is not a bug + +**Decision.** `i_clamped` (the integrator reaching `integrator_max_w`, i.e. +anti-windup) keeps alarming through the same `ClampWatchdog`: it is counted, +notified on the same cadence, and escalates on a sustained run +(SAFETY-41). It gets its own text, a `WARNING` and an amber status row. Only +`clamped` (a command over `max_w`) and write-path clamps keep the BUG SIGNAL, +the `ERROR` and the red row. + +**Why it still notifies.** On a default install `clamped` is unreachable +(`control.py`, at the integrator bound), so the integrator bound is the only +loop-side signal of a runaway such as the 14 768 W one. Silencing it would +remove that. The card says the same: "the fix is to say the true thing, not to +say less." + +**Why a bug has its own budget.** A genuine over-rating must not wait up to +`REASSERT_S` behind anti-windup notices. `ClampWatchdog` tracks a separate +`last_bug_alarm`, and a bug's text is never overwritten by later anti-windup +text within an episode. Across episodes the WARNING uses its own +notification id (`goodwe_integrator_bound`), so it cannot replace a BUG SIGNAL +notice still on screen (review of #130). With only bug engagements, the cadence is the same as +before (`test_clamp.py` part 9). + +**Numbers.** They come from the `Decision` of the clamping tick (`i_bound_w`, +`requested_w`, `clamped_w`, `target_w`), not from `i_w` or `self.target`. + +**Open for the owner.** Whether scattered one-cycle anti-windup episodes should +notify at all, or only log and count. They notify now, as before. diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md index f0076f3..a4a7faf 100644 --- a/goodwe_controller/DOCS.md +++ b/goodwe_controller/DOCS.md @@ -1630,7 +1630,9 @@ share no reason code, no notification id, no status row and no counter. running - this is not the sign-disagreement failsafe, which commands 0 W. You get a persistent notification (`goodwe_output_clamp`, replaced rather than stacked), an `ERROR` in the log with the reason code `OUTPUT_CLAMP`, a row in -the event list, and a red status banner while it is engaged. Once it clears, a +the event list, and a red status banner while it is engaged. The message quotes +the command the loop computed and the value it was clamped to, both from the +cycle that clamped. Once it clears, a warning row keeps the engagement count so the evidence does not vanish with the condition. Consecutive engagements share one event row carrying the exact count, because the event list keeps only 40 rows and a row per cycle would flush every @@ -1639,9 +1641,11 @@ other guard's entries out of it. **There is nothing to configure**, deliberately. The bound is `max_w`, which you already set; giving the guard a limit of its own would be a second number that can disagree with the first, which is exactly the failure the startup validation -exists to prevent. The alarm's cadence - first engagement, then re-asserted every -30 s while it persists, cleared after 3 clean cycles - is fixed for the same -reason `RAIL_FRAC` is: it describes the notification bus, not the plant. +exists to prevent. The alarm's cadence - normally at most one notification per +150 s (an episode that has not yet notified does so once it has engaged 3 +cycles in a row, and an over-rating has its own 150 s budget), cleared after 3 +clean cycles - is fixed for the same reason `RAIL_FRAC` is: it describes the +notification bus, not the plant. WARNING: **on a default install the engagement you will see is the integrator's, not the output's.** The control law bounds the integrator to `max_w` and *then* @@ -1650,6 +1654,17 @@ rating unless `integrator_max_w` has been deliberately raised above `max_w`. Both are watched. A guard that watched only the emitted value would be silent on every standard configuration. +**The integrator bound is not a bug, and it is reported as what it is.** It +reads *"the loop's integrator reached its +5000 W anti-windup bound; the loop's +command this cycle was +2000 W"*, titled *"the control loop is at its integrator +bound"*, logged as a `WARNING`, with an amber (not red) status row and its own +notification (`goodwe_integrator_bound`), so it never replaces a BUG SIGNAL +notice still on screen. Brief +engagements after a large load or solar step are normal. If it stays engaged, +check that the battery is following its commands. Only a command over the rating +carries the BUG SIGNAL text, 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 frozen-meter alarm A meter that has died is easy — it stops answering, the sample age climbs past diff --git a/goodwe_controller/app/clamp.py b/goodwe_controller/app/clamp.py index d787108..e25a704 100644 --- a/goodwe_controller/app/clamp.py +++ b/goodwe_controller/app/clamp.py @@ -73,6 +73,7 @@ An alarm wired only to this module's `ClampResult.engaged` would therefore be silent on the exact configuration the whole ticket is about. The winding is visible one layer up, at the integrator bound, and that is why `Decision` now carries `i_clamped` / `clamped` BOOLEANS and the caller ORs them in here. +They alarm in different words: see `loop_clamp_note()` (SAFETY-40). AND WHY BOOLEANS RATHER THAN `Decision.reason`. `reason` is a single string that later stages overwrite: a cycle that clamps the integrator and is then slew @@ -200,9 +201,8 @@ REASON_CODE = "OUTPUT_CLAMP" # ceiling rests on one number a test author chose; if that number is ever # revisited, this constant moves with it. # -# ⚠️ THIS IS A STOP-GAP AND DOES NOT MAKE THE ALARM CORRECT. The message still -# calls the integrator bound a BUG SIGNAL and still pairs a request from one -# moment with a `self.target` read at another - SAFETY-40. Nothing here is +# ⚠️ THIS WAS A STOP-GAP; the wording and the numbers were SAFETY-40's, fixed +# in `loop_clamp_note()` below. Nothing here is # suppressed: every engagement is still counted, the action-log row is written on # every engaged cycle regardless, and the rising edge of every episode alarms # unconditionally, so no alarm is DROPPED - only delayed. @@ -234,6 +234,31 @@ CLEAR_CYCLES = 3 SUSTAINED_CYCLES = 3 +BUG_SIGNAL = ("This is a BUG SIGNAL, not a limit being used: something " + "upstream computed a setpoint the actuator cannot deliver.") + + +def loop_clamp_note(d) -> str: + """The loop-side alarm text for one `control.Decision`, or "". + + SAFETY-40: the integrator bound is anti-windup doing its job and says so; + only a command over the rating is a BUG SIGNAL. Every number is the + decision's own, i.e. from the tick that clamped. + """ + if d.clamped: + return (f"{REASON_CODE}: the loop computed {d.requested_w:+.0f} W, over " + f"the {abs(d.clamped_w):.0f} W rating, and it was clamped to " + f"{d.clamped_w:+.0f} W; the loop's command this cycle was " + f"{d.target_w:+.0f} W. {BUG_SIGNAL}") + if d.i_clamped: + return (f"{REASON_CODE}: the loop's integrator reached its " + f"{d.i_bound_w:+.0f} W anti-windup bound; the loop's command " + f"this cycle was {d.target_w:+.0f} W. Brief " + f"engagements after a large load or solar step are normal. If it " + f"persists, check that the battery is following its commands.") + return "" + + @dataclass(frozen=True) class ClampResult: """What the guard did, with the RAW value preserved. @@ -342,23 +367,51 @@ class ClampWatchdog: # spoken yet - the two the escalation clause needs. self.run_len = 0 self.spoken = False + # SAFETY-40. Did this episode include a genuine over-rating (`bug`), and + # has THAT spoken? A bug has its own budget, so anti-windup notices can + # never delay it; with bug=True throughout, cadence is as before. + self.bug = False + self.bug_spoken = False + self.last_bug_alarm: datetime | None = None - def note(self, engaged: bool, reason: str, now: datetime) -> bool: + def _due(self, last, now) -> bool: + return last is None or (now - last).total_seconds() >= REASSERT_S + + def _speak(self, now, bug) -> bool: + self.last_alarm = now + self.spoken = True + if bug: + self.last_bug_alarm = now + self.bug_spoken = True + return True + + def note(self, engaged: bool, reason: str, now: datetime, + bug: bool = True) -> bool: """Advance one cycle. True when the caller should raise the alarm. `now` is the caller's clock, the same discipline as - `SignWatchdog.evaluate()` and `FreezeWatchdog.evaluate()`. + `SignWatchdog.evaluate()` and `FreezeWatchdog.evaluate()`. `bug` False + is the integrator bound (SAFETY-40): counted and notified like the + rest, but it never replaces a bug's text within an episode. """ if engaged: self.engagements += 1 self.clean_cycles = 0 - self.reason = reason self.run_len += 1 - if not self.active: + rising = not self.active + if rising: self.active = True self.since = now self.episodes += 1 self.spoken = False + self.bug = False + self.bug_spoken = False + if bug or not self.bug: + self.reason = reason + self.bug = self.bug or bug + if bug and not self.bug_spoken and self._due(self.last_bug_alarm, now): + return self._speak(now, True) + if rising: # ⚠️ SAFETY-41. THE RISING EDGE IS RATE-LIMITED TOO, and it did # not used to be. It fired unconditionally, so a condition that # engages and clears repeatedly raised one notification PER @@ -384,11 +437,8 @@ class ClampWatchdog: # still written on every engaged cycle, and a genuine runaway - # which by definition keeps engaging - is picked up by the # re-assert path within REASSERT_S. Delay, not loss. - if (self.last_alarm is None - or (now - self.last_alarm).total_seconds() >= REASSERT_S): - self.last_alarm = now - self.spoken = True - return True + if self._due(self.last_alarm, now): + return self._speak(now, self.bug) return False # ⚠️ THE ESCALATION. An episode that has not yet spoken because the # budget was spent by an EARLIER, unrelated episode speaks anyway @@ -396,15 +446,10 @@ class ClampWatchdog: # engages every cycle and reaches it; the scattered one-cycle # railing that produced the live flood never does. if not self.spoken and self.run_len >= SUSTAINED_CYCLES: - self.last_alarm = now - self.spoken = True - return True + return self._speak(now, self.bug) # Still engaged: re-assert on the slow cadence, not every cycle. - if (self.last_alarm is None - or (now - self.last_alarm).total_seconds() >= REASSERT_S): - self.last_alarm = now - self.spoken = True - return True + if self._due(self.last_alarm, now): + return self._speak(now, self.bug) return False self.run_len = 0 @@ -420,5 +465,6 @@ class ClampWatchdog: # above. The episode ends; the notification budget does not reset # with it. self.reason = "" + self.bug = False self.clean_cycles = 0 return False diff --git a/goodwe_controller/app/control.py b/goodwe_controller/app/control.py index fa89464..9a6cfda 100644 --- a/goodwe_controller/app/control.py +++ b/goodwe_controller/app/control.py @@ -340,6 +340,12 @@ class Decision: # the comment at the bound below, and clamp.py. i_clamped: bool = False clamped: bool = False + # SAFETY-40. The alarm's numbers, taken at the clamp so they are one tick's: + # the bound the integrator reached, and the command before/after the + # rating clamp. `target_w` is this tick's emitted command. + i_bound_w: float = 0.0 + requested_w: float = 0.0 + clamped_w: float = 0.0 def slew_limit_w(tuning: Tuning) -> float: @@ -563,6 +569,7 @@ def compute( # winding, "clamped" is a command that came out over the rating anyway. reason = "i-clamped" i_w = bounded + i_bound_w = bounded if i_clamped else 0.0 want = i_w # ⚠️ Maintenance shaping (charge-only, cheap-window floor) used to live @@ -574,6 +581,7 @@ def compute( # An early draft applied a floor after the clamp and let demand escape it. target = max(-tuning.max_w, min(tuning.max_w, want)) clamped = target != want + clamped_w = target if clamped: reason = "clamped" @@ -740,7 +748,8 @@ def compute( target = round(target / step) * step return Decision(float(target), sat_count, frozen, reason, float(i_w), - i_clamped, clamped) + i_clamped, clamped, float(i_bound_w), float(want), + float(clamped_w)) def maintenance_charge_floor( diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index 6a19576..e8160d4 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -49,7 +49,8 @@ from .circulation import ( circulating_w, opposing, ) from .clamp import ( - REASON_CODE as CLAMP_REASON, ClampWatchdog, clamp_to_rating, is_commandable, + BUG_SIGNAL as CLAMP_BUG_SIGNAL, REASON_CODE as CLAMP_REASON, ClampWatchdog, + clamp_to_rating, is_commandable, loop_clamp_note, ) from . import freshness from .freeze import ( @@ -994,7 +995,8 @@ class Controller: # the counterpart to `clamp_last` moving onto the Inverter: the integrator # that wound is the SITE integrator, of which there is exactly one, while # the value a clamp bit is a value written to ONE device. - self.loop_clamped: tuple[bool, str] = (False, "") + # (engaged, note, over_rating) - SAFETY-40 splits the last one out. + self.loop_clamped: tuple[bool, str, bool] = (False, "", False) self.stopping = False # SAFETY-61. time.monotonic() at the end of the last run_control() pass. self.loop_pass_mono = None @@ -3271,11 +3273,12 @@ class Controller: # it - so a string test is silent on the two cases a winding loop # most often presents as. These flags are set where the clamp # happens and nothing downstream touches them. + # SAFETY-40: every number comes from THIS decision, at clamp time, + # never from `self.target` after slew/freeze/arbiter rewrote it. self.loop_clamped = ( decision.i_clamped or decision.clamped, - (f"the loop asked for {decision.i_w:+.0f} W" - + (" (integrator bound)" if decision.i_clamped else "") - + (" (output bound)" if decision.clamped else "")), + loop_clamp_note(decision), + decision.clamped, ) # SITE. compute() regulates `grid_w` - the P1 measurement - to # zero. Its subject is the meter, and §6.2's architecture is ONE @@ -3299,7 +3302,7 @@ class Controller: # A loop that is not running cannot be winding. Clearing this here # rather than letting the last value stand is what makes the alarm # de-assert when control is stopped. - self.loop_clamped = (False, "") + self.loop_clamped = (False, "", False) # 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 @@ -3716,20 +3719,17 @@ class Controller: clamped_units = [inv for inv in self.inverters if inv.clamp_last is not None and inv.clamp_last.engaged] engaged_write = bool(clamped_units) - engaged_loop, loop_note = self.loop_clamped + engaged_loop, loop_note, loop_bug = self.loop_clamped engaged = engaged_write or engaged_loop - if engaged: - where = (", ".join(f"{inv.name}: {inv.clamp_last.describe()}" - for inv in clamped_units) if engaged_write - else f"{CLAMP_REASON}: {loop_note}, above the " - f"{self.site_max_w:.0f} W rating, and " - f"was clamped to {self.target:+.0f} W") - reason = (f"{where}. This is a BUG SIGNAL, not a limit being used: " - f"something upstream computed a setpoint the actuator " - f"cannot deliver.") + # SAFETY-40: only an over-rating is a bug; the integrator bound is not. + bug = engaged_write or loop_bug + if engaged_write: + reason = (", ".join(f"{inv.name}: {inv.clamp_last.describe()}" + for inv in clamped_units) + ". " + CLAMP_BUG_SIGNAL) else: - reason = "" - fired = self.clamp_wd.note(engaged, reason, datetime.now(timezone.utc)) + reason = loop_note if engaged else "" + fired = self.clamp_wd.note(engaged, reason, datetime.now(timezone.utc), + bug=bug) # --- THE ACTION LOG, AND IT IS COALESCED PER EPISODE ------------------ # Every engagement is recorded, but consecutive ones are recorded by @@ -3760,6 +3760,21 @@ class Controller: self.log_event(row) if not fired: return + if not self.clamp_wd.bug: + # SAFETY-40: the integrator bound is anti-windup, not a bug - a + # WARNING and a plainly titled notice, same cadence. Its OWN id, so + # it can never replace a BUG SIGNAL notice still on screen. + _LOG.warning("SAFETY-03 - %s Engagements so far: %d.", + self.clamp_wd.reason, self.clamp_wd.engagements) + await self.hass.call("persistent_notification", "create", { + "notification_id": "goodwe_integrator_bound", + "title": "GoodWe: the control loop is at its integrator bound", + "message": (f"{self.clamp_wd.reason}\n\nNo command exceeded " + f"the rating and the controller is STILL RUNNING. " + f"See the add-on documentation, 'The winding-loop " + f"alarm'."), + }) + return _LOG.error( "SAFETY-03 - %s Silently clipping this is how a winding integrator " "reached 14 768 W on this project before: the clamp makes every " @@ -4893,16 +4908,21 @@ class Controller: out.append({"ok": False, "warn": "NOT COMMANDABLE" not in note, "text": note}) - # SAFETY-03. Red while a clamp is engaged, and the engagement COUNT is - # shown even after it clears - a loop that wound once and stopped is - # still a bug, and a row that vanishes with the condition takes the + # SAFETY-03. Red while an over-rating clamp is engaged (amber for the + # integrator bound, SAFETY-40), and the engagement COUNT is shown even + # after it clears - a row that vanishes with the condition takes the # evidence with it. Reported only when there is something to report: # unlike the freeze detector this guard has no "not covered" state, it # runs on every write on every transport. - if self.clamp_wd.active: + if self.clamp_wd.active and self.clamp_wd.bug: out.append({"ok": False, "warn": False, "text": "OUTPUT CLAMP ENGAGED - " + self.clamp_wd.reason + f" Engagements: {self.clamp_wd.engagements}."}) + elif self.clamp_wd.active: + # SAFETY-40: the integrator bound alone is amber, not red. + out.append({"ok": False, "warn": True, + "text": "LOOP AT INTEGRATOR BOUND - " + self.clamp_wd.reason + + f" Engagements: {self.clamp_wd.engagements}."}) elif self.clamp_wd.engagements: out.append({"ok": False, "warn": True, "text": f"output clamp: not engaged now, but it has " diff --git a/goodwe_controller/test_clamp.py b/goodwe_controller/test_clamp.py index dc033a2..d6dcdf6 100644 --- a/goodwe_controller/test_clamp.py +++ b/goodwe_controller/test_clamp.py @@ -38,6 +38,9 @@ acceptance criteria; part 6 is TEST-11's and part 7 is SAFETY-28's: the check that replaced a config-shape proxy: `ACTUATOR_WRITE_KEYS` graded how many option KEYS exist, which stays 1 while a per-inverter config adds a second real writer. + 9. SAFETY-40. The integrator bound (normal anti-windup) is reported in its + own words and never as a BUG SIGNAL; the over-rating keeps it; and every + number in either message is the clamp tick's own Decision. ------------------------------------------------------------------------------ WHY PART 4 ALSO ASSERTS ON `Decision`'s BOOLEANS AND NOT ON `reason` @@ -99,6 +102,9 @@ T0 = datetime(2026, 8, 30, 9, 0, 0, tzinfo=timezone.utc) # which is also the device rating, so this is the bound the alarm is written at. LIVE_MAX_W = 5000.0 CLAMP_ID = "goodwe_output_clamp" +# SAFETY-40: the integrator-bound WARNING has its own id, so it can never +# replace a BUG SIGNAL notice still on screen. +BOUND_ID = "goodwe_integrator_bound" def check(name, cond): @@ -458,7 +464,7 @@ def wired(grid, cycles=40, batt=0.0, track=True, **over): def clamp_notes(hass): return [d for dom, svc, d in hass.calls if (dom, svc) == ("persistent_notification", "create") - and d.get("notification_id") == CLAMP_ID] + and d.get("notification_id") in (CLAMP_ID, BOUND_ID)] # THE 10.3 RUNAWAY, replayed. "With the inverter switched off entirely, an @@ -497,12 +503,16 @@ print(f" the loop clamped on {ctl_r.clamp_wd.engagements} of {60} cycles, " f"notification(s)") check("AC 3: and it ALARMED - silently clipping this is the defect", ctl_r.clamp_wd.engagements > 0 and len(notes_r) >= 1) -check("...with its own notification_id, not shared with any other guard", - all(d["notification_id"] == CLAMP_ID for d in notes_r)) +check("...with its own notification_id, not shared with any other guard - " + "on a default install the integrator bound's (SAFETY-40)", + all(d["notification_id"] == BOUND_ID for d in notes_r)) check("...one episode, re-asserted rather than one per cycle", ctl_r.clamp_wd.episodes == 1 and len(notes_r) < ctl_r.clamp_wd.engagements) -check("...and it names itself a bug signal, not a limit in use", - bool(notes_r) and "out of range" in notes_r[0]["message"]) +# SAFETY-40: on a default install this runaway is the INTEGRATOR bound, so it +# says so, and does not call anti-windup a bug (part 8 grades the wording). +check("...and it names the integrator bound, not a BUG SIGNAL", + bool(notes_r) and "anti-windup bound" in notes_r[0]["message"] + and "BUG SIGNAL" not in notes_r[0]["message"]) rows_r = [e for e in ctl_r.events if REASON_CODE in e] check(f"the action log records every engagement, coalesced into one row per " f"episode ({len(rows_r)} row(s), {ctl_r.clamp_wd.engagements} " @@ -528,8 +538,9 @@ check("...and the count in the row is the counter's, so nothing is lost", f"{ctl_l.clamp_wd.engagements} engagement(s)" in nth(rows_l, -1)) # The RED row exists only while engaged, and the episode above is short, so it # is captured mid-run rather than asserted after the hysteresis has cleared it. -check("the status page goes red WHILE it is engaged", - any(not c["ok"] and not c["warn"] and "OUTPUT CLAMP ENGAGED" in c["text"] +check("the status page flags it WHILE it is engaged - amber, since it is the " + "integrator bound (SAFETY-40; red is part 9's over-rating case)", + any(not c["ok"] and c["warn"] and "LOOP AT INTEGRATOR BOUND" in c["text"] for c in ctl_r.red_snapshot)) check("...and once clear, a warn row still says how many times it engaged", any(c["warn"] and f"{ctl_r.clamp_wd.engagements} time(s)" in c["text"] @@ -806,11 +817,11 @@ class _SpyWatchdog(ClampWatchdog): self.seen = [] # clamp_last.requested_w at each consultation self.seen_write = [] # was the WRITE-path half engaged, per cycle - def note(self, engaged, reason, now): + def note(self, engaged, reason, now, **kw): cl = self.owner.clamp_last self.seen.append(None if cl is None else cl.requested_w) self.seen_write.append(bool(cl and cl.engaged)) - return super().note(engaged, reason, now) + return super().note(engaged, reason, now, **kw) def probed(cls=Controller, grid=9000.0, cycles=5, **over): @@ -986,7 +997,7 @@ check("PRECONDITION: the write really did clamp, on the DEVICE bound", and hass_w.writes[-1] == 2500.0) check("...and the loop-side half is SILENT here, so self.clamp_last is the " "only evidence that this happened at all", - ctl_w.loop_clamped == (False, "")) + ctl_w.loop_clamped == (False, "", False)) check("AC 3: the guard alarmed on write-path evidence alone", ctl_w.clamp_wd.engagements == 1 and len(notes_w) == 1) check("...and the operator is told WHICH bound bit, so a device-rating clamp " @@ -1858,6 +1869,178 @@ check("...EDGE-logged, not level-logged: 25 cycles did not flush the 40-row " len(_shadow_lines) < 25 and len(_shad_c.events) <= 40) +# ============================================================================== +print() +print("9. SAFETY-40: the integrator bound is not a BUG SIGNAL, and the numbers " + "are the clamp tick's own") +# Three defects, each graded on the wired Controller.cycle() with compute() +# spied, so every number is compared with the Decision of the SAME tick: +# 1. `i_clamped` (normal anti-windup) used the over-rating BUG SIGNAL text; +# 2. "the loop asked for" printed `Decision.i_w`, the integrator, as a request; +# 3. "was clamped to" printed `self.target`, read after slew/freeze rewrote it. +# The fixtures are chosen so the numbers the old text used DIFFER from the true +# ones (MUTATION.md habit 4); the PRECONDITION checks pin that. +import logging +import app.main as _main_mod + +_real_compute = _main_mod.compute + + +class _LogCap(logging.Handler): + def __init__(self): + super().__init__() + self.records = [] + + def emit(self, record): + self.records.append(record) + + +def s40_run(grid, cycles, track, **over): + """Per tick: (Decision, self.target, clamp_wd.reason, engaged) + hass + log.""" + ticks, decisions = [], [] + + def spy(*a, **k): + d = _real_compute(*a, **k) + decisions.append(d) + return d + + cap = _LogCap() + lg = logging.getLogger("goodwe") + lg.addHandler(cap) + _main_mod.compute = spy + + async def go(): + hass = _Hass(grid, track=track) + c = Controller(dict( + {"setpoint_entity": "number.sp", "meter_entity": "sensor.grid", + "soc_entity": "sensor.soc", "batt_entity": "sensor.batt", + "meter_source": "off", "max_w": LIVE_MAX_W, "gain": 0.6, + "slew_w": 1000, "deadband_w": 15, "target_grid_w": 0, + "step_w": 10, "heartbeat_s": 10, "stale_input_s": 15}, + **over), hass, _Store(), _Pub(), stop=asyncio.Event()) + c.auto = True + c.dev_min, c.dev_max = -LIVE_MAX_W, LIVE_MAX_W + red = [] + for _ in range(cycles): + n = len(decisions) + before = c.clamp_wd.engagements + await c.read_inputs() + await c.cycle() + d = decisions[-1] if len(decisions) > n else None + ticks.append((d, c.target, c.clamp_wd.reason, + c.clamp_wd.engagements > before)) + if c.clamp_wd.active and not red: + red = c.checks() + return hass, c, red + + try: + hass, c, red = asyncio.run(go()) + finally: + _main_mod.compute = _real_compute + lg.removeHandler(cap) + return ticks, hass, c, red, cap.records + + +def _f(d, name): + """A Decision field, NaN when absent - so an old tree goes red, not crashes.""" + return getattr(d, name, float("nan")) + + +# --- 9a. default install: only the INTEGRATOR bound can engage --------------- +ticks_i, hass_i, ctl_i, red_i, log_i = s40_run(6000.0, 20, track=False) +wind = [(d, t, r) for d, t, r, e in ticks_i + if e and d is not None and d.i_clamped and not d.clamped] +for d, t, r in wind: + print(f" tick: bound {_f(d, 'i_bound_w'):+.0f} i_w {d.i_w:+.0f} command " + f"{d.target_w:+.0f} self.target {t:+.0f}\n -> {r}") +check("PRECONDITION: the integrator bound engaged, and on a tick where the " + "carried integrator AND the emitted command both differ from the bound", + any(d.i_w != _f(d, 'i_bound_w') and d.target_w != _f(d, 'i_bound_w') + for d, t, r in wind)) +check("defect 1: the integrator bound is NOT reported as a BUG SIGNAL", + wind and all("BUG SIGNAL" not in r for d, t, r in wind)) +check("defect 1: ...it names the anti-windup bound, from the same tick", + wind and all(f"its {_f(d, 'i_bound_w'):+.0f} W anti-windup bound" in r + for d, t, r in wind)) +check("defect 2: the integrator accumulator is not printed as a request", + wind and all(f"{d.i_w:+.0f} W" not in r and "asked for" not in r + for d, t, r in wind)) +check("the command quoted is that tick's Decision, not a later read", + wind and all(f"command this cycle was {d.target_w:+.0f} W" in r + for d, t, r in wind)) +check("at the bound nothing is described as 'above' or 'over' the rating", + wind and all(" above " not in r and " over " not in r + for d, t, r in wind)) +notes_i = clamp_notes(hass_i) +check("it still NOTIFIES (not suppressed), under a title that claims no bug", + notes_i and all("integrator bound" in n["title"] + and "exceeded" not in n["title"] for n in notes_i)) +check("...under its OWN notification_id, so it cannot replace a BUG SIGNAL " + "notice still on screen", + notes_i and all(n["notification_id"] == BOUND_ID for n in notes_i)) +check("...logged as a WARNING, not an ERROR", + any(x.levelno == logging.WARNING and "anti-windup" in x.getMessage() + for x in log_i) + and not any(x.levelno >= logging.ERROR and "OUTPUT_CLAMP" in x.getMessage() + for x in log_i)) +check("...and the status row is amber, not red", + any(c["warn"] and "LOOP AT INTEGRATOR BOUND" in c["text"] for c in red_i) + and not any(not c["warn"] and "OUTPUT CLAMP ENGAGED" in c["text"] + for c in red_i)) + +# --- 9b. the genuine over-rating: integrator_max_w above max_w ---------------- +ticks_o, hass_o, ctl_o, red_o, log_o = s40_run(9000.0, 12, track=True, + integrator_max_w=9000) +over = [(d, t, r) for d, t, r, e in ticks_o + if e and d is not None and d.clamped] +for d, t, r in over[:2]: + print(f" tick: requested {_f(d, 'requested_w'):+.0f} clamped {_f(d, 'clamped_w'):+.0f}" + f" i_w {d.i_w:+.0f} self.target {t:+.0f}\n -> {r}") +check("PRECONDITION: the output clamp engaged on a tick where the slew limit " + "rewrote the command AND the integrator differs from the request", + any(t != _f(d, 'clamped_w') and d.i_w != _f(d, 'requested_w') for d, t, r in over)) +check("the over-rating KEEPS the BUG SIGNAL", + over and all("BUG SIGNAL" in r for d, t, r in over)) +check("defect 2: it quotes the requested COMMAND of that tick, not i_w", + over and all(f"computed {_f(d, 'requested_w'):+.0f} W" in r for d, t, r in over)) +check("defect 3: 'clamped to' is the clamp's own result, not self.target", + over and all(f"clamped to {_f(d, 'clamped_w'):+.0f} W" in r for d, t, r in over)) +check("...and the 14 768 W rationale is still in its ERROR", + any(x.levelno == logging.ERROR and "14 768 W" in x.getMessage() + for x in log_o)) +check("...under the 'exceeded the rating' title, with a red status row", + clamp_notes(hass_o) + and all("exceeded the rating" in n["title"] + and n["notification_id"] == CLAMP_ID for n in clamp_notes(hass_o)) + and any(not c["warn"] and "OUTPUT CLAMP ENGAGED" in c["text"] + for c in red_o)) + +# --- 9c. the text, directly, and the two budgets ------------------------------- +from app.clamp import loop_clamp_note # noqa: E402 + +d_at = compute(prev_w=1000.0, grid_w=9000.0, actual_w=1000.0, + tuning=Tuning(max_w=5000.0, slew_w=1000.0, gain=0.6), i_w=4500.0) +check(f"exactly at the rating ({d_at.i_bound_w:+.0f} W) is 'reached', never " + f"'above' - {loop_clamp_note(d_at)!r}", + d_at.i_clamped and not d_at.clamped and abs(d_at.i_bound_w) == 5000.0 + and "above" not in loop_clamp_note(d_at)) +check("a healthy decision produces no text", + loop_clamp_note(compute(0.0, 100.0, 0.0, Tuning(max_w=5000.0))) == "") + +# Anti-windup notices must never spend the budget a BUG SIGNAL needs. +wd = ClampWatchdog() +fired_w = wd.note(True, "wind", T0, bug=False) +fired_b = wd.note(True, "bug", T0 + timedelta(seconds=5), bug=True) +wd.note(True, "wind again", T0 + timedelta(seconds=10), bug=False) +check("an anti-windup notice does not delay a bug in the same episode, and " + "does not overwrite its text", + fired_w and fired_b and wd.bug and wd.reason == "bug") +wd2 = ClampWatchdog() +seq = [wd2.note(True, "b", T0 + timedelta(seconds=5 * i)) for i in range(40)] +check("bug-only cadence is unchanged: rising edge, then every REASSERT_S", + seq[0] and sum(seq) == 1 + int(5 * 39 // REASSERT_S)) + + print() if fails: print(f"{len(fails)} FAILED: {', '.join(fails)}") diff --git a/goodwe_controller/test_freeze.py b/goodwe_controller/test_freeze.py index a5e0f00..661113d 100644 --- a/goodwe_controller/test_freeze.py +++ b/goodwe_controller/test_freeze.py @@ -835,7 +835,8 @@ frozen_rows = [c for c in ctl_f.checks() if "METER MAY BE FROZEN" in c["text"]] print(f" frozen at +4000 W: command ran to {ctl_f.target:.0f} W, " f"{len(notes)} notification(s), {len(frozen_rows)} red status row(s)") # SURFACE-WIDE, deliberately, and the second guard has now landed. SAFETY-03 -# ships `goodwe_output_clamp`, and this scenario - a runaway loop winding into +# ships `goodwe_output_clamp` (and, for the integrator bound on a default +# install, `goodwe_integrator_bound` - SAFETY-40), and this scenario - a runaway loop winding into # the rail - raises it legitimately: the loop asks for more than the rating and # is clamped. So the answer to "is this scenario still meant to raise exactly # one alarm" is NO, it raises exactly two, and BOTH ARE NAMED here rather than @@ -846,7 +847,7 @@ _ids = sorted(d.get("notification_id") for d in all_notes) print(f" the whole notification surface this run: {_ids}") check("the whole notification surface carried exactly two alarms this run - " "SAFETY-18's and SAFETY-03's, both named", - _ids == ["goodwe_meter_frozen", "goodwe_output_clamp"]) + _ids == ["goodwe_integrator_bound", "goodwe_meter_frozen"]) # THIS GUARD'S, keyed on the id. check("...and the freeze detector is the one that raised it", len(notes) == 1) # ⚠️ Selected by TEXT, not by id - see notes_by_text_of(). One of the two