SAFETY-40 review: integrator-bound WARNING gets its own notification id
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:
co-authored by
Claude Opus 5.5
parent
fd14569a71
commit
55a6d5dcdb
@@ -2,7 +2,7 @@
|
||||
|
||||
**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 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
|
||||
`clamped` (a command over `max_w`) and write-path clamps keep the BUG SIGNAL,
|
||||
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
|
||||
`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. 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).
|
||||
|
||||
**Numbers.** They come from the `Decision` of the clamping tick (`i_bound_w`,
|
||||
|
||||
@@ -1641,10 +1641,10 @@ 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 - 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), cleared after 3 clean
|
||||
cycles - is fixed for the same reason `RAIL_FRAC` is: it describes the
|
||||
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,
|
||||
@@ -1657,10 +1657,12 @@ 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. 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,
|
||||
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).
|
||||
|
||||
### The frozen-meter alarm
|
||||
|
||||
@@ -3762,14 +3762,17 @@ class Controller:
|
||||
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 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.",
|
||||
self.clamp_wd.reason, self.clamp_wd.engagements)
|
||||
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",
|
||||
"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
|
||||
_LOG.error(
|
||||
|
||||
@@ -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.
|
||||
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):
|
||||
@@ -461,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
|
||||
@@ -500,8 +503,9 @@ 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)
|
||||
# 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",
|
||||
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)
|
||||
@@ -2003,7 +2010,8 @@ check("...and the 14 768 W rationale is still in its ERROR",
|
||||
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"] 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"]
|
||||
for c in red_o))
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user