SAFETY-03: alarm when a computed setpoint exceeds the rating, do not just clip it
The clamp was never the missing half. specs.md 10.3 records an unclamped integrator climbing ~130 W per 4 s past 10 kW and reporting 14 768 W pinned at the 5 kW rail; clipped silently that is a flat, legal-looking 5000 W hold and nothing says the loop asked for three times it. write_setpoint() - the one path every write takes - now clamps to the VALIDATED max_w and reports whether the raw value differed; check_output_clamp() raises OUTPUT_CLAMP as a notification, an ERROR, an event row and a red status banner. Not a trip: the clamped value is written and the loop keeps running. The rating is max_w and nothing is min()ed at runtime - CFG-01 already refuses a max_w outside the device range at startup. The device range is applied second as defence in depth; it cannot bite after a successful validation, so if it ever does it alarms too and names the bound. No new config key. Decision grew i_clamped / clamped booleans, and that is the load-bearing part: `reason` is one string that slew-limiting and the saturation freeze overwrite, so an alarm keyed on it is silent on exactly the cycles a winding loop produces. Mutating main.py back onto the string drops the 10.3 replay from 2 engagements to 0. Distinct from saturation by construction - no shared reason code, notification id, status row or counter - and checked in the negative, after first asserting the saturation freeze really did engage. Two things found by running it rather than by reading it: a row per engagement flushed the 40-row event log and evicted SAFETY-18's own METER FROZEN? row (now coalesced per episode with an exact count), and a naive clamp turns a nan into a command at the full rating because every comparison against nan is False. test_clamp.py is new: assert-based, no framework. Six author mutations reproduced red, including "clamp kept, alarm deleted". Two of the check's own hypotheses about the freeze/clamp interaction were refused by its sweep and rewritten to what it measured. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC
This commit is contained in:
glenn schrooyen
co-authored by
Claude Opus 5
Claude Opus 5
parent
0cdaa3bd3e
commit
c040aac4b8
@@ -36,6 +36,10 @@ from .arbiter import (
|
||||
P_LOOP, P_MAINT_SHAPE, P_MAINTENANCE, P_SAFETY, Claim, resolve,
|
||||
)
|
||||
from .control import Tuning, compute, maintenance_charge_floor, peak_at_risk
|
||||
from .clamp import (
|
||||
REASON_CODE as CLAMP_REASON, ClampResult, ClampWatchdog, clamp_to_rating,
|
||||
is_commandable,
|
||||
)
|
||||
from .freeze import FreezeWatchdog, FreezeWatchdogConfig, rail_w
|
||||
from .hass import HomeAssistant
|
||||
from .maintenance import IDLE, MaintConfig, Maintenance
|
||||
@@ -140,6 +144,19 @@ class Controller:
|
||||
self.freeze_wd = FreezeWatchdog(
|
||||
FreezeWatchdogConfig(hold_s=float(opts.get("meter_freeze_s", 300))))
|
||||
|
||||
# SAFETY-03. The output clamp's alarm - see clamp.py for why the clamp
|
||||
# itself is the cheap half.
|
||||
#
|
||||
# ⚠️ NO CONFIG OF ITS OWN, on purpose. The bound it clamps to is `max_w`,
|
||||
# which is already an option, already validated against the device's
|
||||
# rating at startup, and already used by Tuning and the arbiter. Giving
|
||||
# this a second key would be a fourth place a limit has to be kept in
|
||||
# step (CFG-03 already tracks three) and, worse, would recreate exactly
|
||||
# the two-numbers-that-disagree failure the §14 warning box describes.
|
||||
# The anti-flood cadence is a module constant for the same reason
|
||||
# freeze.RAIL_FRAC is: it describes the event bus, not the plant.
|
||||
self.clamp_wd = ClampWatchdog()
|
||||
|
||||
# P1 ingestion (TEL-01). `meter_source: off` keeps the original
|
||||
# single-entity meter_entity path, so an existing install is unchanged
|
||||
# until it opts in.
|
||||
@@ -171,6 +188,11 @@ class Controller:
|
||||
self.last_write_ok = None
|
||||
self.inputs_bad_since = None
|
||||
self.dev_min = self.dev_max = None
|
||||
# SAFETY-03. What the last write's clamp did, and what the loop's own
|
||||
# bounds did last cycle. `loop_clamped` is a (engaged, note) pair so the
|
||||
# alarm can say WHICH bound bit without re-deriving it from a string.
|
||||
self.clamp_last: ClampResult | None = None
|
||||
self.loop_clamped: tuple[bool, str] = (False, "")
|
||||
self.events: list[str] = []
|
||||
self.stopping = False
|
||||
|
||||
@@ -229,11 +251,37 @@ class Controller:
|
||||
entity = self.o.get("setpoint_entity", "")
|
||||
if not entity:
|
||||
return
|
||||
# Never send a value the device will reject outright. A rejected write is
|
||||
# silent from the controller's point of view, and the inverter then keeps
|
||||
# doing whatever it was already doing.
|
||||
if self.dev_min is not None and self.dev_max is not None:
|
||||
value = max(self.dev_min, min(self.dev_max, value))
|
||||
|
||||
# --- SAFETY-03: THE CLAMP, AND IT IS ON THE ONLY PATH TO THE WIRE -----
|
||||
# Every write in this add-on comes through here - the cycle, the
|
||||
# heartbeat, the operator's stop button and shutdown() - so no optimizer,
|
||||
# maintenance or manual path can route around it. That placement is the
|
||||
# first acceptance criterion, and it is why the guard is not in
|
||||
# control.py: a guard the control law owns is a guard every other claimer
|
||||
# bypasses.
|
||||
#
|
||||
# ⚠️ A NON-FINITE VALUE IS LOSS OF INPUT, NOT AN OUT-OF-RANGE ONE. §11.1
|
||||
# rule 1: command zero and keep commanding. It must not reach the clamp
|
||||
# arithmetic - see is_commandable() for what a nan does to it - and it
|
||||
# must not be counted as a clamp engagement, or a transport bug lands in
|
||||
# the winding-loop counter.
|
||||
if not is_commandable(value):
|
||||
self.log_event(f"non-finite setpoint {value!r} - commanding 0 W")
|
||||
_LOG.error("refusing to write a non-finite setpoint %r - commanding "
|
||||
"0 W and continuing to command. This is a bug upstream, "
|
||||
"not a limit.", value)
|
||||
value = 0.0
|
||||
result = clamp_to_rating(value, float(self.o.get("max_w", 2000)),
|
||||
self.dev_min, self.dev_max)
|
||||
self.clamp_last = result
|
||||
if result.engaged:
|
||||
# ⚠️ EVERY engagement, never deduplicated: this is the action log
|
||||
# (§15), and it is a different surface from the notification, which
|
||||
# IS deduplicated. The count is what says how hard the loop pushed;
|
||||
# the notification only says that it did.
|
||||
self.log_event(result.describe())
|
||||
value = result.applied_w
|
||||
|
||||
changed = (self.last_write is None or value != self.last_write)
|
||||
if not (changed or force):
|
||||
return
|
||||
@@ -379,11 +427,29 @@ class Controller:
|
||||
self.sat_count = decision.sat_count
|
||||
self.i_w = decision.i_w
|
||||
self.loop_w = decision.target_w
|
||||
# --- SAFETY-03 ---------------------------------------------------
|
||||
# ⚠️ READ THE BOOLEANS, NOT `decision.reason`. On a default install
|
||||
# the integrator bound is what bites and the output clamp is
|
||||
# unreachable (control.py says so), and `reason` is then overwritten
|
||||
# by whichever of slew-limiting or the saturation freeze ran after
|
||||
# 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.
|
||||
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 "")),
|
||||
)
|
||||
claims.append(Claim.set("loop", P_LOOP, decision.target_w, decision.reason))
|
||||
else:
|
||||
# Stopped or blind: no accumulation may survive the outage.
|
||||
self.i_w = 0.0
|
||||
self.loop_w = None
|
||||
# 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, "")
|
||||
|
||||
resolution = resolve(claims)
|
||||
if resolution.contradiction:
|
||||
@@ -412,8 +478,92 @@ class Controller:
|
||||
# chose it has been standing still. Evaluated before write_setpoint()
|
||||
# it would be reasoning about the PREVIOUS command against THIS cycle's
|
||||
# meter - the one combination that is true of neither moment.
|
||||
await self.check_output_clamp()
|
||||
await self.check_meter_frozen()
|
||||
|
||||
async def check_output_clamp(self) -> None:
|
||||
"""SAFETY-03. Alarm whenever a clamp actually engaged this cycle.
|
||||
|
||||
Runs AFTER write_setpoint() for the same reason SAFETY-18 does: the
|
||||
clamp is arithmetic done inside the write, so before the write there is
|
||||
nothing yet to report.
|
||||
|
||||
⚠️ PUBLISHING IS NOT IN THE WRITE PATH. The clamp itself is synchronous
|
||||
inside write_setpoint(); this - the awaits, the notification, anything
|
||||
that can block on Home Assistant - is out here. A stalled supervisor must
|
||||
never be able to delay or drop a frame.
|
||||
|
||||
⚠️ THIS IS NOT A TRIP. No claim is filed, nothing is latched, 0 W is not
|
||||
commanded and `publish()` still reports "running". §11.2's action for
|
||||
this row is "clamp and alarm", and the clamped value is written and the
|
||||
loop keeps running. Turning this into a trip means coming here and
|
||||
changing that deliberately, not discovering it in the field.
|
||||
"""
|
||||
engaged_write = bool(self.clamp_last and self.clamp_last.engaged)
|
||||
engaged_loop, loop_note = self.loop_clamped
|
||||
engaged = engaged_write or engaged_loop
|
||||
if engaged:
|
||||
where = (self.clamp_last.describe() if engaged_write
|
||||
else f"{CLAMP_REASON}: {loop_note}, above the "
|
||||
f"{float(self.o.get('max_w', 2000)):.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.")
|
||||
else:
|
||||
reason = ""
|
||||
fired = self.clamp_wd.note(engaged, reason, datetime.now(timezone.utc))
|
||||
|
||||
# --- THE ACTION LOG, AND IT IS COALESCED PER EPISODE ------------------
|
||||
# Every engagement is recorded, but consecutive ones are recorded by
|
||||
# UPDATING one row's count rather than inserting a row each.
|
||||
#
|
||||
# ⚠️ THAT IS NOT COSMETIC, AND IT WAS FOUND THE HARD WAY. `log_event()`
|
||||
# keeps 40 rows; a winding loop clamps on every cycle, so a row per
|
||||
# engagement flushes the whole ring within 40 cycles - and the first
|
||||
# thing it evicted, in test_freeze.py, was SAFETY-18's "METER FROZEN?"
|
||||
# row. A guard whose log drowns out the other guards' logs has moved the
|
||||
# flood rather than removed it. The count here is `clamp_wd.engagements`,
|
||||
# so nothing is lost: the row says exactly how many, and the number is
|
||||
# read off the counter rather than off the number of rows.
|
||||
#
|
||||
# This lives here rather than only in write_setpoint() because on a
|
||||
# default install the actuator-side clamp is unreachable - compute() has
|
||||
# already bounded the integrator - so the only engagements that exist are
|
||||
# loop-side ones. Measured: 60 cycles of the §10.3 runaway produce 0
|
||||
# write-path engagements and 2 loop-side ones.
|
||||
if engaged and not engaged_write:
|
||||
tag = f"{CLAMP_REASON} episode {self.clamp_wd.episodes}"
|
||||
row = (f"{tag}: {self.clamp_wd.engagements} engagement(s) - "
|
||||
f"{reason}")
|
||||
if self.events and tag in self.events[0]:
|
||||
stamp = self.events[0].split(" ", 1)[0]
|
||||
self.events[0] = f"{stamp} {row}"
|
||||
else:
|
||||
self.log_event(row)
|
||||
if not fired:
|
||||
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 "
|
||||
"dashboard read a legal number while the thing producing it goes on "
|
||||
"climbing. The command IS clamped and the loop is STILL RUNNING - "
|
||||
"this is an alarm, not a trip. Engagements so far: %d.",
|
||||
self.clamp_wd.reason, self.clamp_wd.engagements)
|
||||
# A fixed notification_id, for the reason check_meter_frozen() gives -
|
||||
# and a DIFFERENT one from every other alarm this add-on raises, because
|
||||
# a clamp is not a frozen meter and neither is a saturation freeze.
|
||||
await self.hass.call("persistent_notification", "create", {
|
||||
"notification_id": "goodwe_output_clamp",
|
||||
"title": "GoodWe: a computed setpoint exceeded the rating",
|
||||
"message": (
|
||||
f"{self.clamp_wd.reason}\n\nThe command has been clamped and "
|
||||
f"the controller is STILL RUNNING. This is NOT the inverter "
|
||||
f"failing to follow a legal command (that is the saturation "
|
||||
f"freeze, which is normal and does not alarm) - the value asked "
|
||||
f"for was itself out of range."),
|
||||
})
|
||||
|
||||
async def check_meter_frozen(self) -> None:
|
||||
"""SAFETY-18. Alarm on a meter that answers forever without changing.
|
||||
|
||||
@@ -694,6 +844,23 @@ class Controller:
|
||||
out.append({"ok": True, "warn": False,
|
||||
"text": f"setpoint {sp} (range {self.dev_min:g}…{self.dev_max:g} W)"})
|
||||
|
||||
# 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
|
||||
# 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:
|
||||
out.append({"ok": False, "warn": False,
|
||||
"text": "OUTPUT CLAMP ENGAGED - " + 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 "
|
||||
f"engaged {self.clamp_wd.engagements} time(s) in "
|
||||
f"{self.clamp_wd.episodes} episode(s) since "
|
||||
f"start - see the event log"})
|
||||
|
||||
if self.last_write_ok is False:
|
||||
out.append({"ok": False, "warn": False,
|
||||
"text": "last write to the inverter was REJECTED - check the log"})
|
||||
|
||||
Reference in New Issue
Block a user