admin/goodwe-addon
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge release/1.0 (SAFETY-03) into TEST-06
checks / unit (pull_request) Canceled after 0s
Details

Browse Source 
Two conflicts, both predicted, both mine as the later merger.

app/main.py - #12 inserted `await self.check_output_clamp()` on the line
immediately above `await self.check_meter_frozen()`, so its hunk abuts my
comment correction. Resolved so EACH CALL CARRIES ITS OWN BOUNDARY,
because the two guards do not share one:

  - check_output_clamp() reads `self.clamp_last`, which is None until
    write_setpoint() has run, so its boundary IS the write;
  - check_meter_frozen() grades `self.target`, assigned further up, so
    its boundary is the assignment.

Letting my correction generalise across both would have been worse than
the vague comment it replaces, because it would read as deliberate. The
AST of app/main.py is identical to merged release/1.0 (+1251 bytes, all
comment).

test_freeze.py - #12 independently added `frozen_notes()` keyed on the
same id. `notes_of()` subsumes it: same id, plus the title fallback that
keeps the id's own check independently falsifiable, plus `all_notes_of()`
for claims about the whole surface.

THE THREE SURFACE-WIDE ASSERTIONS, DECIDED ONE AT A TIME. Measured on the
merged tree:

  scenario                              surface  ids             freeze
  frozen meter, railed command             2     frozen + clamp     1
  TEL-05 converged, -14.2 W                0     -                  0
  moving meter, a real 8 kW load           1     clamp              0
  uncovered transport, no unchanged_s      1     clamp              0

  - the wiring scenario STAYS SURFACE-WIDE and now asserts exactly TWO
    alarms with BOTH IDS NAMED. The answer to "is this still meant to
    raise one alarm" is no, it raises two, and a third guard has to come
    back to this line. That is the one assertion in the file that
    accounts for the controller's entire alarm output;
  - the CONVERGED case stays surface-wide and stays silent: at the
    operating point the whole add-on should have nothing to say, that is
    still true, and narrowing it would give away a real property;
  - the other two NARROW to notes_of(). The clamp alarm there is correct
    and is SAFETY-03's subject: a real 8 kW load does drive the loop past
    the rating, and an uncovered transport does not stop it. Asserting
    whole-surface silence would make this file fail whenever another
    guard correctly does its job. Each still owns the load-bearing half -
    THE FREEZE DETECTOR SAID NOTHING - and the surface is PRINTED so a
    reader sees what else fired without this file grading it.

Re-verified on the merged tree: M1 RED 4, M2 RED 5, M3 RED 3 + RED 2,
N6 RED 1, N3 green (correctly), N8 RED 4, R1 RED 2, R2 RED 1. All nine
suites green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC
This commit is contained in:
glenn schrooyen
2026-08-30 17:45:09 +02:00
co-authored by Claude Opus 5
parent d31a12b7c6 6aabb832fc
commit 0942228d65
8 changed files with 1268 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files
goodwe_controller/app/main.py
+192 -16
View File
@@ -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:
@@ -406,24 +472,117 @@ class Controller:
self.reason = resolution.explain()
await self.write_setpoint(self.target)
# --- SAFETY-18, and it runs AFTER `self.target` is assigned ----------
# ⚠️ THE BOUNDARY IS THE ASSIGNMENT ABOVE, NOT write_setpoint(). This
# comment used to say "the command that has just gone on the wire",
# which is wrong twice over and was corrected by TEST-06's reviewer:
# `check_meter_frozen()` grades `self.target` - the arbiter's resolved
# intent - and `write_setpoint()` clamps only a local `value` and can
# skip the write entirely, so that number may never have gone anywhere.
# --- SAFETY-03, and its boundary is the WRITE ------------------------
# ⚠️ NOT THE SAME BOUNDARY AS SAFETY-18's BELOW, THOUGH THE CALLS ABUT.
# This one reads `self.clamp_last`, which is None until write_setpoint()
# has actually run, so it has to follow the write. The one below grades
# `self.target`, which is assigned further up - see check_output_clamp()
# and check_meter_frozen() for each. Two guards, two orderings, adjacent
# lines: do not "tidy" them into one rule.
await self.check_output_clamp()
# --- SAFETY-18, and its boundary is the ASSIGNMENT --------------------
# ⚠️ THE BOUNDARY IS `self.target = resolution.target_w` ABOVE, NOT
# write_setpoint(). This comment used to say "the command that has just
# gone on the wire", which is wrong twice over and was corrected by
# TEST-06's reviewer: `check_meter_frozen()` grades `self.target` - the
# arbiter's resolved intent - and `write_setpoint()` clamps only a local
# `value` and can skip the write entirely, so that number may never have
# gone anywhere.
#
# What both terms of the conjunction must describe is the SAME moment:
# the command this cycle resolved, against how long the meter that
# chose it has been standing still. Evaluated before
# the command this cycle resolved, against how long the meter that chose
# it has been standing still. Evaluated before
# `self.target = resolution.target_w` it would grade the PREVIOUS
# cycle's command against THIS cycle's meter - the one combination that
# was true of neither moment. Moving it between the assignment and
# write_setpoint() is therefore harmless; moving it above the
# assignment is not, and test_freeze.py part 5 fails on exactly that.
# write_setpoint() is therefore harmless; moving it above the assignment
# is not, and test_freeze.py part 5 fails on exactly that.
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.
@@ -704,6 +863,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"})