SAFETY-40 review: integrator-bound WARNING gets its own notification id
checks / unit (pull_request) Successful in 8m46s
infra-pin / fresh (pull_request) Successful in 1s

A later anti-windup notice could replace a write-path BUG SIGNAL notice
under the shared goodwe_output_clamp id (escalation ignores the budget).
The WARNING now uses goodwe_integrator_bound and points to DOCS; the
cadence sentence in DOCS is corrected. Tests pin both ids (red on fd14569).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
glenn schrooyen
2026-09-26 23:08:44 +02:00
co-authored by Claude Opus 5.5
parent fd14569a71
commit 55a6d5dcdb
5 changed files with 33 additions and 17 deletions
@@ -2,7 +2,7 @@
**Decision.** `i_clamped` (the integrator reaching `integrator_max_w`, i.e. **Decision.** `i_clamped` (the integrator reaching `integrator_max_w`, i.e.
anti-windup) keeps alarming through the same `ClampWatchdog`: it is counted, anti-windup) keeps alarming through the same `ClampWatchdog`: it is counted,
notified on the same id and cadence, and escalates on a sustained run 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 (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, `clamped` (a command over `max_w`) and write-path clamps keep the BUG SIGNAL,
the `ERROR` and the red row. the `ERROR` and the red row.
@@ -16,7 +16,9 @@ say less."
**Why a bug has its own budget.** A genuine over-rating must not wait up to **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 `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 `last_bug_alarm`, and a bug's text is never overwritten by later anti-windup
text within an episode. With only bug engagements, the cadence is the same as 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). before (`test_clamp.py` part 9).
**Numbers.** They come from the `Decision` of the clamping tick (`i_bound_w`, **Numbers.** They come from the `Decision` of the clamping tick (`i_bound_w`,
+8 -6
View File
@@ -1641,10 +1641,10 @@ other guard's entries out of it.
**There is nothing to configure**, deliberately. The bound is `max_w`, which you **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 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 can disagree with the first, which is exactly the failure the startup validation
exists to prevent. The alarm's cadence - at most one notification per 150 s exists to prevent. The alarm's cadence - normally at most one notification per
(an episode that has not yet notified does so once it has engaged 3 cycles in 150 s (an episode that has not yet notified does so once it has engaged 3
a row), cleared after 3 clean cycles in a row, and an over-rating has its own 150 s budget), cleared after 3
cycles - is fixed for the same reason `RAIL_FRAC` is: it describes the clean cycles - is fixed for the same reason `RAIL_FRAC` is: it describes the
notification bus, not the plant. notification bus, not the plant.
WARNING: **on a default install the engagement you will see is the integrator's, WARNING: **on a default install the engagement you will see is the integrator's,
@@ -1657,10 +1657,12 @@ every standard configuration.
**The integrator bound is not a bug, and it is reported as what it is.** It **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 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 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. Brief 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, 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 check that the battery is following its commands. Only a command over the rating
carries the BUG SIGNAL text, and it never waits behind integrator-bound notices. 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). 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 ### The frozen-meter alarm
+6 -3
View File
@@ -3762,14 +3762,17 @@ class Controller:
return return
if not self.clamp_wd.bug: if not self.clamp_wd.bug:
# SAFETY-40: the integrator bound is anti-windup, not a bug - a # SAFETY-40: the integrator bound is anti-windup, not a bug - a
# WARNING and a plainly titled notice, same id and cadence. # 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.", _LOG.warning("SAFETY-03 - %s Engagements so far: %d.",
self.clamp_wd.reason, self.clamp_wd.engagements) self.clamp_wd.reason, self.clamp_wd.engagements)
await self.hass.call("persistent_notification", "create", { await self.hass.call("persistent_notification", "create", {
"notification_id": "goodwe_output_clamp", "notification_id": "goodwe_integrator_bound",
"title": "GoodWe: the control loop is at its integrator bound", "title": "GoodWe: the control loop is at its integrator bound",
"message": (f"{self.clamp_wd.reason}\n\nNo command exceeded " "message": (f"{self.clamp_wd.reason}\n\nNo command exceeded "
f"the rating and the controller is STILL RUNNING."), f"the rating and the controller is STILL RUNNING. "
f"See the add-on documentation, 'The winding-loop "
f"alarm'."),
}) })
return return
_LOG.error( _LOG.error(
+12 -4
View File
@@ -102,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. # which is also the device rating, so this is the bound the alarm is written at.
LIVE_MAX_W = 5000.0 LIVE_MAX_W = 5000.0
CLAMP_ID = "goodwe_output_clamp" 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): def check(name, cond):
@@ -461,7 +464,7 @@ def wired(grid, cycles=40, batt=0.0, track=True, **over):
def clamp_notes(hass): def clamp_notes(hass):
return [d for dom, svc, d in hass.calls return [d for dom, svc, d in hass.calls
if (dom, svc) == ("persistent_notification", "create") 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 # THE 10.3 RUNAWAY, replayed. "With the inverter switched off entirely, an
@@ -500,8 +503,9 @@ print(f" the loop clamped on {ctl_r.clamp_wd.engagements} of {60} cycles, "
f"notification(s)") f"notification(s)")
check("AC 3: and it ALARMED - silently clipping this is the defect", check("AC 3: and it ALARMED - silently clipping this is the defect",
ctl_r.clamp_wd.engagements > 0 and len(notes_r) >= 1) ctl_r.clamp_wd.engagements > 0 and len(notes_r) >= 1)
check("...with its own notification_id, not shared with any other guard", check("...with its own notification_id, not shared with any other guard - "
all(d["notification_id"] == CLAMP_ID for d in notes_r)) "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", 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) ctl_r.clamp_wd.episodes == 1 and len(notes_r) < ctl_r.clamp_wd.engagements)
# SAFETY-40: on a default install this runaway is the INTEGRATOR bound, so it # SAFETY-40: on a default install this runaway is the INTEGRATOR bound, so it
@@ -1971,6 +1975,9 @@ notes_i = clamp_notes(hass_i)
check("it still NOTIFIES (not suppressed), under a title that claims no bug", check("it still NOTIFIES (not suppressed), under a title that claims no bug",
notes_i and all("integrator bound" in n["title"] notes_i and all("integrator bound" in n["title"]
and "exceeded" not in n["title"] for n in notes_i)) 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", check("...logged as a WARNING, not an ERROR",
any(x.levelno == logging.WARNING and "anti-windup" in x.getMessage() any(x.levelno == logging.WARNING and "anti-windup" in x.getMessage()
for x in log_i) for x in log_i)
@@ -2003,7 +2010,8 @@ check("...and the 14 768 W rationale is still in its ERROR",
for x in log_o)) for x in log_o))
check("...under the 'exceeded the rating' title, with a red status row", check("...under the 'exceeded the rating' title, with a red status row",
clamp_notes(hass_o) clamp_notes(hass_o)
and all("exceeded the rating" in n["title"] for n in 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"] and any(not c["warn"] and "OUTPUT CLAMP ENGAGED" in c["text"]
for c in red_o)) for c in red_o))
+3 -2
View File
@@ -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, " 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)") f"{len(notes)} notification(s), {len(frozen_rows)} red status row(s)")
# SURFACE-WIDE, deliberately, and the second guard has now landed. SAFETY-03 # 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 # 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 # 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 # 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}") print(f" the whole notification surface this run: {_ids}")
check("the whole notification surface carried exactly two alarms this run - " check("the whole notification surface carried exactly two alarms this run - "
"SAFETY-18's and SAFETY-03's, both named", "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. # THIS GUARD'S, keyed on the id.
check("...and the freeze detector is the one that raised it", len(notes) == 1) 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 # ⚠️ Selected by TEXT, not by id - see notes_by_text_of(). One of the two