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

SAFETY-18: a meter frozen at a wrong-but-fresh value now raises an alarm

Browse Source 
A meter that answers 200 OK with a stale document forever is ARRIVING on every
poll, so every freshness mechanism in the stack reads it as healthy while the
loop integrates a constant error and rails: 360/135/75 s at the live install's
max_w: 5000 for a meter frozen at 350/1000/>=3000 W. Fifteen minutes of a 5 kW
unattended import sets the month's billed capaciteitstarief peak.

The detector is a conjunction, not the bare unchanged_s threshold TEL-05
measured and rejected:

    unchanged_s >= meter_freeze_s   AND   |command| within 5 % of max_w

Both terms already existed and were already on the status page, and they fail
for independent reasons: at the converged point the command is nowhere near the
rail, and at the rail against a real load the kWh registers move in seconds. At
the shipped 300 s the relation is structural rather than a margin - the slowest
a healthy 1 Wh register can tick and still look unchanged is 12 W, which is
inside the 15 W deadband, so the command there is parked at 0 W.

Notification only, deliberately: an HA persistent notification with a fixed
notification_id, an ERROR log line, a UI event and a red status row. No claim,
no latch, no 0 W. A false trip parks the battery; a false notification costs an
alert. Only meter_source: homewizard_local is covered - the status page says
NOT COVERED elsewhere rather than pretending the guard is armed.

DOCS.md states the capacity-tariff exposure this does NOT close: peak_cap_w caps
maintenance charging and nothing caps the loop. That cap is SAFETY-07.

test_freeze.py grades both directions and was verified by breaking what it
watches (six mutations, all red): silent over the committed 40-minute converged
capture, silent over an hour of TEL-05's low-power case where the rejected bare
threshold is shown to fire, alarming on a genuinely frozen meter driven over
real HTTP through the real HomeWizardLocalSource (the rig's own hwsim freeze
fault when reachable), silent on the same railed command against a meter that is
still moving, and asserting on the service call that leaves Controller.cycle()
rather than on the watchdog's own flag.

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 02:39:54 +02:00
co-authored by Claude Opus 5
parent bb64f8fdd7
commit 7aa1bd052e
7 changed files with 1040 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
goodwe_controller/app/freeze.py
+261
View File
@@ -0,0 +1,261 @@
"""SAFETY-18 - the frozen-but-fresh meter detector.
WHAT IT CATCHES, in one sentence: a meter that keeps answering 200 OK with a
document that never changes, which every freshness mechanism in this stack reads
as perfectly healthy while the loop winds to the rail and stays there.
THE FAILURE NEEDS NO FAULT ANYWHERE ELSE. A frozen-but-talking meter is
*arriving* on every poll, so:
- `meter_max_age_s` is structurally blind. Every response resets the arrival
stamp (p1.py, `HomeWizardLocalSource`), so the age never climbs. Age catches
a meter that stops TALKING; this one stops CHANGING.
- firmware layer 1 is fed. Its watchdog keys on setpoint staleness, which the
heartbeat satisfies, and on `sensor.goodwe_p1_sample_age`, which a talking
meter satisfies.
- the arbiter contributes only the device rating and +-`max_w` - it bounds the
magnitude, which is the number this fault drives us to, not away from.
- there is no SoC guard on the grid-following loop at all. `soc_floor` belongs
to the maintenance cycle; the BMS is the sole energy backstop.
The loop reads a constant error and integrates it. Time from freeze to the rail,
replaying the real `control.compute()` at 15 s cycles:
meter frozen at to rail, max_w 2000 to rail, max_w 5000
350 W 150 s 360 s
1000 W 60 s 135 s
>= 3000 W 30 s 75 s
⚠️ THE LIVE INSTALL RUNS `max_w: 5000`, which is both the schema maximum and the
device rating, so the right-hand column is the one that describes this house.
------------------------------------------------------------------------------
THIS HAS A EURO FIGURE, AND IT IS THE REASON THE TICKET IS P1
`maintenance_charge_floor()` caps MAINTENANCE charging against `peak_cap_w`. The
grid-following loop is not capped by anything but `max_w`. A meter frozen
reading a large export drives a 5 kW unattended grid IMPORT, and >= 15 minutes
of that sets the month's billed capaciteitstarief peak: specs.md:583 records a
measured case where a single 4998 W quarter moved a monthly peak from 4832 W to
5013 W and billed the whole month at it.
⚠️ THIS CUT DOES NOT CAP THAT. It notifies. See DOCS.md, "Capacity-tariff
exposure of the grid-following loop", and SAFETY-07.
------------------------------------------------------------------------------
WHY IT IS A CONJUNCTION AND NOT AN `unchanged_s` THRESHOLD
`unchanged_s` (p1.py) already measures exactly the right thing: seconds since
the meter served a DIFFERENT measurement, fingerprinted over the power figure
and both energy registers. It is computed, it is on the status page, and TEL-05
deliberately refused to threshold it - correctly:
this controller regulates grid power toward ~0 W, and at a converged -10 W
the export register takes about six minutes to advance by its 1 Wh
resolution while the power figure legitimately repeats.
So a bare `unchanged_s > N` false-trips at the exact operating point we aim for,
and `p1.py` says as much in a note that ends "whoever wires a freeze detector
must handle the low-power case first". This is that handling, and it is not a
smaller number:
unchanged_s >= hold_s AND |commanded_w| is at the rail
The second term is what TEL-05's case cannot satisfy. At the converged point the
command is nowhere near +-`max_w`; on the 40 minutes of live converged behaviour
in `sim/scenarios/p1-age-2026-08-26.jsonl.gz` the meter never once leaves
+-500 W, so the loop never even approaches a 5000 W rail. And the reverse case -
a command genuinely pinned at the rail by a real 8 kW load - cannot satisfy the
FIRST term, because a real railed condition means kilowatts still crossing the
meter, and a kilowatt advances a 1 Wh register every 3.6 s.
The two terms fail for independent reasons. That is the whole design.
------------------------------------------------------------------------------
WHERE `hold_s` COMES FROM
Measured on `sim/scenarios/p1-age-2026-08-26.jsonl.gz` - 2399 s (40 min) of the
live house at the converged operating point, mean grid -14.2 W, 1194 polls:
polls that repeated the previous reading 735 of 1193 (61.6 %)
distinct-reading transitions 458
median gap between distinct readings 6.0 s
p95 gap 6.0 s
LARGEST `unchanged_s` any sample observes 8.0 s
longest interval serving no distinct reading 15.4 s
samples outside +-500 W 0
The default 300 s is 37x the largest `unchanged_s` this house has been recorded
producing while converged, and 19.5x the longest interval in which it served no
distinct reading at all. The two figures differ because the capture has an 11.4 s
hole in its OWN polling; the second is quoted because it is the pessimistic one.
300 s is not fitted to either margin - it is picked as "clearly past any
plausible register-tick interval, and clearly inside the 900 s quarter that bills
the capacity tariff". test_freeze.py re-measures both every run, so this
paragraph cannot quietly stop being true.
⚠️ BE PRECISE ABOUT WHAT THAT CAPTURE PROVES. It is POWER-ONLY - the shipped
fingerprint also covers both kWh registers, so a real `unchanged_s` on that data
can only be shorter, never longer, which makes it a conservative measurement of
the converged case. But it does NOT on its own refute a bare `unchanged_s`
threshold, and saying it did would be exactly the overstatement this project
keeps having to walk back: at 8 s worst, a bare 300 s threshold is silent on it
too. The case that refutes the bare threshold is the one below, which no capture
of THIS meter contains because this meter's power figure jitters.
The floor on the other side: at the rail, the smallest residual grid flow that
still ticks a 1 Wh register inside 300 s is 12 W. Any condition that genuinely
pins this loop at +-5000 W has three orders of magnitude more than that crossing
the meter.
⚠️ AND THOSE TWO NUMBERS DO NOT MERELY HAVE MARGIN - THEY CANNOT BOTH BE TRUE.
The worry the conjunction has to answer is "a HEALTHY meter whose register is
slower than `hold_s`". At 300 s that is |grid| < 12 W. The shipped `deadband_w`
is 15 W, so anywhere the register is that slow the control law is already inside
its deadband and the command is parked - the rail term is not merely unlikely to
be true there, it is 0 W. The two terms are mutually exclusive by construction
at the defaults, not just far apart, and the check re-derives that every run.
That relationship is the one to protect. `hold_s` may be lowered only while
3600/hold_s stays under `deadband_w`; at the schema floor of 60 s that is 60 W
against a 15 W deadband, and the mutual exclusion is gone. A 60 s `meter_freeze_s`
is a supported configuration that gives up this argument and keeps only the
margin - do not set one without a capture that says the margin is enough.
⚠️ WHAT `hold_s` DOES NOT BUY IS DETECTION SPEED. The alert lands at
max(hold_s, time-to-rail) after the freeze, so at the shipped default and
`max_w: 5000` that is 300-360 s. The rail is REACHED first, from 75 s. A
notification is not a brake; see the DOCS.md note.
------------------------------------------------------------------------------
THE RAIL TERM, PRECISELY
abs(commanded_w) >= RAIL_FRAC * max_w
`commanded_w` is the EMITTED setpoint - what the arbiter resolved and
`write_setpoint()` put on the wire - not the loop's raw wish, for the same
reason SAFETY-02 uses the emitted value: the wire carries the emitted number.
RAIL_FRAC is 0.95 and is a constant rather than an option. The command is
quantised to `step_w` (max 100) and clamped to `max_w` (min 100), so at the rail
the emitted value IS `max_w` on every schema-legal combination; the 5 % is slack
for a device rating that clips `max_w` slightly in `write_setpoint()`, not a
tuning knob. Making it configurable would offer a way to widen the one term that
carries the whole no-false-positive argument.
⚠️ NOT GATED ON `following`, and that is deliberate. A maintenance charge
commands against the meter on purpose and can sit at the rail for hours - but it
does so while pushing kilowatts through the meter, so the FIRST term is what
excludes it, and it excludes it on physics rather than on bookkeeping. Gating on
`following` as well would additionally blind the detector to a maintenance phase
running against a meter that really has frozen, which is the same fault with a
worse actuator behind it.
------------------------------------------------------------------------------
NOTIFY, DO NOT TRIP
First cut is an alarm and nothing else: an HA persistent notification, an ERROR
in the log, an event on the status page, and a failed check row. It does NOT
claim the actuator, does NOT file a P_SAFETY claim and does NOT latch.
That is a deliberate ordering of costs, not timidity. A false trip here parks
the battery - on a house that is running this loop precisely to avoid buying
power at the wrong moment - and the operator's response to a parked battery is
to switch the guard off. A false notification costs one alert. The trip is a
separate decision that has to be earned with observed data from a real install,
and it needs an answer to a question this cut does not have one for: what should
the loop do instead, given that 0 W is itself a grid import of whatever the
house is drawing.
⚠️ TRANSPORT-CONDITIONAL. `unchanged_s` exists ONLY on `homewizard_local`,
because it is the only transport that sees the meter's own document. Under
`ha_dsmr`, `mqtt_p1`, `ha_signed` and `meter_source: off` this detector is
silent and the exposure above is uncovered. The caller passes None and gets
False; there is no half-measure available on those paths, and pretending
otherwise would be worse than the gap.
"""
from dataclasses import dataclass
from datetime import datetime
# See "THE RAIL TERM, PRECISELY" above. A constant, on purpose.
RAIL_FRAC = 0.95
@dataclass(frozen=True)
class FreezeWatchdogConfig:
"""Default, not a constant - config.yaml carries `meter_freeze_s`."""
hold_s: float = 300.0
def rail_w(max_w) -> float:
"""The magnitude at or above which a command counts as railed."""
return abs(float(max_w)) * RAIL_FRAC
def looks_frozen(unchanged_s, commanded_w, max_w,
cfg: FreezeWatchdogConfig) -> bool:
"""The predicate, and the ONLY place the conjunction is written down.
Public for the same reason `watchdog.disagrees()` is: the checks that grade
it must assert on THE SHIPPED EXPRESSION. A safety predicate transcribed
into its own test is one copy plus a future disagreement about what the
check means, and the copy that drifts is always the one doing the grading.
`unchanged_s` seconds since the meter served a different measurement, or
None on a transport that cannot know (see the module note).
`commanded_w` the EMITTED setpoint standing on the wire.
`max_w` the configured output bound the rail is measured against.
"""
if unchanged_s is None or commanded_w is None or not max_w:
return False
return (float(unchanged_s) >= cfg.hold_s
and abs(float(commanded_w)) >= rail_w(max_w))
class FreezeWatchdog:
"""One instance per controller, evaluated once per control cycle.
Level-triggered state with an EDGE for the alarm: `evaluate()` returns True
on the cycle the conjunction first becomes true and False on every cycle
after, so the caller raises one notification per episode rather than one
every second. `active` is the level, for the status page.
No latch, deliberately - see "NOTIFY, DO NOT TRIP". A latch exists so a trip
cannot erase its own evidence by working; nothing here acts, so there is
nothing to erase. When the meter starts moving again the alert clears
itself, which is the honest report of the state.
"""
def __init__(self, cfg: FreezeWatchdogConfig | None = None):
self.cfg = cfg or FreezeWatchdogConfig()
self.active = False
self.since: datetime | None = None
self.reason = ""
self.episodes = 0
def evaluate(self, unchanged_s, commanded_w, max_w, now: datetime) -> bool:
"""Advance one cycle. Returns True ONLY on the rising edge.
`now` is the caller's clock so this stays testable and replayable - the
same discipline as `SignWatchdog.evaluate()`.
"""
if not looks_frozen(unchanged_s, commanded_w, max_w, self.cfg):
self.active = False
self.since = None
self.reason = ""
return False
self.reason = (
f"METER_FROZEN: the meter has answered {float(unchanged_s):.0f} s "
f"without its measurement changing while the command stands at "
f"{float(commanded_w):+.0f} W, at the {float(max_w):.0f} W rail "
f"(limits: unchanged over {self.cfg.hold_s:.0f} s, command over "
f"{rail_w(max_w):.0f} W)")
if self.active:
return False
self.active = True
self.since = now
self.episodes += 1
return True
goodwe_controller/app/main.py
+87
View File
@@ -36,6 +36,7 @@ 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 .freeze import FreezeWatchdog, FreezeWatchdogConfig, rail_w
from .hass import HomeAssistant
from .maintenance import IDLE, MaintConfig, Maintenance
from .mqtt import MqttPublisher
@@ -127,6 +128,18 @@ class Controller:
"once you know why the command opposed the meter.",
self.sign_wd.reason or "(reason not recorded)")
# SAFETY-18. The frozen-but-fresh meter detector - see freeze.py for why
# it is a conjunction and not an `unchanged_s` threshold, and for the
# measurements behind the default.
#
# ⚠️ NOT restored from the store, unlike SAFETY-02's latch, and the
# asymmetry is the point: this one does not latch because it does not
# act. A restart re-derives its state from the meter within one
# `meter_freeze_s` window, and until then the honest report is "not
# alarming yet" rather than an alarm nobody can clear.
self.freeze_wd = FreezeWatchdog(
FreezeWatchdogConfig(hold_s=float(opts.get("meter_freeze_s", 300))))
# 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.
@@ -393,6 +406,53 @@ class Controller:
self.reason = resolution.explain()
await self.write_setpoint(self.target)
# --- SAFETY-18, and it runs LAST on purpose --------------------------
# Both terms of the conjunction describe the state that now exists: the
# command that has just gone on the wire, and how long the meter that
# 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_meter_frozen()
async def check_meter_frozen(self) -> None:
"""SAFETY-18. Alarm on a meter that answers forever without changing.
Notification only - no claim, no latch, no 0 W. freeze.py carries the
reasoning and the euro figure; DOCS.md carries the exposure this does
NOT close.
⚠️ getattr, not an attribute access: `unchanged_s` exists only on
`homewizard_local`, and `p1_source` is None until amain() builds the
transport. Every other meter_source hands the predicate None and gets
False, which is the honest answer - see the transport note in freeze.py.
"""
unchanged = getattr(self.p1_source, "unchanged_s", None)
fired = self.freeze_wd.evaluate(unchanged, self.target,
float(self.o.get("max_w", 2000)),
datetime.now(timezone.utc))
if not fired:
return
self.log_event("METER FROZEN? " + self.freeze_wd.reason)
_LOG.error(
"SAFETY-18 - %s. The grid-following loop is regulating against a "
"number that has stopped moving, and nothing in this stack stops "
"it: the meter is still answering, so the age is fresh, the "
"firmware watchdog is fed and the arbiter only bounds the "
"magnitude. CHECK THE METER. Sustained for a quarter of an hour "
"this sets the month's billed capacity-tariff peak. This is an "
"ALARM ONLY - the loop is still running.", self.freeze_wd.reason)
# ⚠️ A fixed notification_id, so a second episode REPLACES the first
# rather than stacking. An operator who has to dismiss eleven identical
# notifications learns to dismiss the twelfth without reading it.
await self.hass.call("persistent_notification", "create", {
"notification_id": "goodwe_meter_frozen",
"title": "GoodWe: the P1 meter may be frozen",
"message": (
f"{self.freeze_wd.reason}.\n\nThe controller is STILL RUNNING "
f"and still following this reading. Check the meter; if it is "
f"stuck, stop control from the add-on UI."),
})
# -- tasks ---------------------------------------------------------------
async def run_control(self) -> None:
entity = self.o.get("setpoint_entity", "")
@@ -581,6 +641,33 @@ class Controller:
f"{self.p1.parse_errors} rejected"
+ (f", measurement unchanged for {unchanged:.0f} s"
if unchanged is not None else "")})
# SAFETY-18. Reported in both states, for the reason SAFETY-02 is:
# a detector nobody can see is armed is a detector nobody trusts
# when it fires. ok=False/warn=False so the banner goes red - this
# is an alarm, and the banner is the loudest surface we own. It
# changes NOTHING about control; see check_meter_frozen().
if self.freeze_wd.active:
out.append({"ok": False, "warn": False,
"text": "METER MAY BE FROZEN - " + self.freeze_wd.reason
+ ". The loop is STILL FOLLOWING it and is "
"still running. Check the meter. Held here "
"for a quarter of an hour this sets the "
"month's billed capacity peak."})
elif getattr(self.p1_source, "unchanged_s", None) is None:
# Not "armed". Every transport but homewizard_local is invisible
# to this check, and saying "armed" there would be a lie the
# status page tells on behalf of a guard that cannot run.
out.append({"ok": False, "warn": True,
"text": f"freeze detector: NOT COVERED on "
f"meter_source {o.get('meter_source')} - only "
f"homewizard_local sees whether the meter's own "
f"document changes"})
else:
out.append({"ok": True, "warn": False,
"text": f"freeze detector: armed (alerts after "
f"{self.freeze_wd.cfg.hold_s:.0f} s unchanged "
f"while the command is over "
f"{rail_w(self.o.get('max_w', 2000)):.0f} W)"})
rows = [("battery SoC", self.soc, o.get("soc_entity")),
("battery power", self.batt, o.get("batt_entity"))]
if not self.p1_enabled: