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

SAFETY-02 BLOCKER: persist the latch across an add-on restart #9

Merged
admin merged 2 commits from safety-02-latch-persistence into release/1.0 2026-08-28 23:09:28 +02:00
Conversation 1 Commits 2 Files Changed 8
+577 -72
8 changed files with 577 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files
HANDOVER.md
+19 -5
View File
@@ -57,11 +57,25 @@ python3 test_control.py && python3 test_maintenance.py && python3 test_arbiter.p
&& python3 test_loop.py
```
`test_watchdog.py` covers SAFETY-02's logic and its trip path. Its headline
validation is not here at all: `sim/replay_sign.py` in the infra repo drives the
same module over the two captured RS485 days and must fire on all four of the
2026-08-20 episodes and stay silent on 2026-08-23. That needs both checkouts —
`GOODWE_ADDON_REPO=/path/to/goodwe-addon python3 sim/replay_sign.py`.
`test_watchdog.py` covers SAFETY-02's logic, its arming gate (derived from the
real arbiter, not passed in), its trip path, and the fact that the latch survives
an add-on restart. Its headline validation is not here at all: `sim/replay_sign.py`
in the infra repo drives the same module over the two captured RS485 days and must
fire on all four of the 2026-08-20 episodes and stay silent on 2026-08-23. That
needs both checkouts:
```bash
GOODWE_ADDON_REPO=/path/to/goodwe-addon python3 sim/replay_sign.py
GOODWE_ADDON_REPO=/path/to/goodwe-addon python3 sim/replay_sign.py --stub
```
The second run checks the grader still *discriminates* rather than merely still
passing — a watchdog that never trips and one that always trips must both be
rejected.
⚠️ **If you are testing the failsafe by hand, note that stopping and starting the
add-on will not clear a trip.** That is the point of it; use the *Start / stop
control* button twice, or delete `sign_tripped` from `/data/state.json`.
⚠️ `test_loop.py` takes about **95 seconds** (measured 93 s) and that is not a bug. It
drives the real control loop, which sleeps 1 s a tick, eight times over, to prove a
goodwe_controller/CHANGELOG.md
+65 -1
View File
@@ -1,5 +1,69 @@
# Changelog
## 0.5.1
**SAFETY-02's latch now survives an add-on restart, which is the difference
between a latch and a pause.** `auto` was persisted and `sign_wd.tripped` was
not, so reconstructing the `Controller` against the same store — exactly what a
container restart does — brought it back **armed, into the same fault, with no
operator involved**: `tripped=False`, banner back to `NOT READY`, and it resumed
commanding (measured: −1000 W, −2000 W) before re-tripping 180 s later. Under a
Supervisor that restarts a crashed add-on that is a loop of three-minute
wrong-direction bursts — and "restart the add-on" is the first thing an operator
looking at a `FAILSAFE` banner and a 0 W inverter will try.
`sign_tripped` and `sign_reason` are now `Store` keys beside the maintenance
state, written the moment the trip happens (reason first, flag second, so a crash
between the two can never persist a latch with no explanation) and restored in
`Controller.__init__`. Cleared only by the operator's start. `DOCS.md` said "it
does not clear itself"; that was false across a restart and now is not.
Also in this release, all of it review follow-up rather than new behaviour:
- **The arming gate is tested against the real arbiter.** It was the decision the
whole ticket turns on and it was asserted in three comment blocks and verified
by nothing — every case passed `following` in as a literal. `test_watchdog.py`
now derives it from `resolve()` across twelve claim configurations, including
the redirect case `winner == "loop"` alone would have got wrong and the `lo`
limit that raises magnitude without flipping sign.
- **The MQTT `status: "failsafe"` value is tested.** The check that claimed to
cover it restated the assertion three lines above it and never touched the
payload.
- **A forward hazard written down where it will be read, and made to bite.**
`winner == "loop"` disarms the watchdog under any higher-priority SET claim.
Today only maintenance and the safety stops emit one and neither follows the
meter; a peak-shaving strategy at `P_PEAK` **does**, and would blind the
watchdog for every peak event.
The gate is now `watchdog.loop_is_following()`, called by both `main.py` and
the test, for the same reason `disagrees()` was extracted: the test held a
*transcription* of the shipped expression, so editing the shipped gate alone to
`winner in ("loop", "peak")` left every suite green while the watchdog went
blind. Two copies, and the one that drifts is the one doing the grading.
⚠️ Extracting it was not sufficient on its own. The realistic P_PEAK row —
peak shaving asking for *more* than the loop — still could not detect the
widened gate, because the magnitude term rejects it regardless and masks the
winner term. A second row with peak asking for *less* isolates `winner` as the
only deciding term, and that is the one that turns red.
- **The worked example described a configuration that does not ship.** At the
defaults `maintenance_charge_w` (2500) exceeds `max_w` (2000), so that claim is
an arbiter *contradiction* resolving to 0 W, not a −2500 W charge. The example
is now written at 1500 W. (The contradiction is a real pre-existing bug —
a default cheap-window maintenance charge silently never happens — filed
separately, deliberately not fixed here.)
- **`disagrees()` is public**, so `sim/replay_sign.py` stops carrying its own copy
of the predicate it grades.
- `held_s` takes the clock instead of reading one, restoring the pure-function
property the module advertises.
- **The `sign_reason`-before-`sign_tripped` write ordering is enforced, not just
documented.** Swapping the two lines used to pass every check. A store that
fails the second write now proves the property the ordering exists for: what
survives a crash mid-trip is a reason with no latch, never a latch with no
explanation.
- Measured figures corrected throughout: 93.4 % / 4212 of 17280 (was 92.9 % /
4127 of 16349), per-cluster 0.3 / 0.9 / 10.5 / 0.0 %.
## 0.5.0
**SAFETY-02 — the sign-disagreement watchdog.** The add-on now watches whether
@@ -23,7 +87,7 @@ module over the committed RS485 captures of both 2026-08-20 (the four episodes)
and 2026-08-23 (healthy): **18 trips on the fault day, every one inside a window
`NOTES.md` §4.7 accounts for, and 0 on the healthy day.** The four episodes do
share one signature rather than merely resembling each other — sign agreement
runs 0.3 / 0.9 / 10.5 / 0.0 % inside them against 92.9 % on the healthy day.
runs 0.3 / 0.9 / 10.5 / 0.0 % inside them against 93.4 % on the healthy day.
Three decisions worth knowing about:
goodwe_controller/DOCS.md
+15 -6
View File
@@ -294,11 +294,20 @@ there. The banner reads `FAILSAFE`, the commissioning list says which two
numbers disagreed and for how long, and the `Controller status` entity reports
`failsafe`.
**It does not clear itself.** Pressing *Start / stop control* twice clears it.
That is deliberate: commanding 0 W removes the command side of the comparison,
so the disagreement always vanishes the instant the trip works, and a watchdog
that took that as proof the fault was over would hand the inverter straight back
to the loop that was driving the wrong way.
**It does not clear itself, and restarting the add-on does not clear it either.**
Pressing *Start / stop control* twice — stop, then start — is the only thing that
does. That is deliberate on both counts:
- Commanding 0 W removes the command side of the comparison, so the disagreement
always vanishes the instant the trip works. A watchdog that took that as proof
the fault was over would hand the inverter straight back to the loop that was
driving the wrong way, three minutes later, forever.
- The trip is written to `/data/state.json` and restored on startup. Restarting
the add-on is the first thing most people try when they see a red banner, and
a latch that a restart clears would bring the controller back **armed, into the
same fault, unattended** — and under a Supervisor that restarts a crashed add-on
automatically, would do so in a loop. After a restart the banner, the 0 W and
the recorded reason are all still there.
**What it is for.** This is a loop-integrity check: it asks whether the loop's
output direction still matches the input state it was regulating. It catches a
@@ -326,7 +335,7 @@ must not be mistaken for a fault.
Both numbers are measured rather than chosen, and are exposed only because the
ticket requires defaults rather than constants. On the captured healthy day sign
agreement is 92.9 %; on the four captured fault episodes it is 0.3 %, 0.9 %,
agreement is 93.4 %; on the four captured fault clusters it is 0.3 %, 0.9 %,
10.5 % and 0.0 %. There is no reason to move either value without a capture that
says so.
goodwe_controller/app/main.py
+47 -26
View File
@@ -41,7 +41,7 @@ from .maintenance import IDLE, MaintConfig, Maintenance
from .mqtt import MqttPublisher
from .p1 import P1Ingest, build_source, is_enabled
from .validate import ConfigRefused, refuse, validate_startup_config
from .watchdog import SignWatchdog, SignWatchdogConfig
from .watchdog import SignWatchdog, SignWatchdogConfig, loop_is_following
from . import web
OPTIONS_PATH = "/data/options.json"
@@ -105,10 +105,27 @@ class Controller:
)
# SAFETY-02. One instance, one actuator, one cadence - see watchdog.py.
self.sign_wd = SignWatchdog(SignWatchdogConfig(
min_w=float(opts.get("sign_disagree_w", 300)),
hold_s=float(opts.get("sign_disagree_s", 180)),
))
#
# ⚠️ RESTORED FROM THE STORE, and that is the difference between a latch
# and a pause. A trip held only in memory is cleared by the one action an
# operator staring at a FAILSAFE banner is most likely to take - restart
# the add-on - and the controller then comes back armed, into the fault,
# with nobody involved. Restored here, the banner and the 0 W survive the
# restart and only `auto_toggle` clears them.
self.sign_wd = SignWatchdog(
SignWatchdogConfig(
min_w=float(opts.get("sign_disagree_w", 300)),
hold_s=float(opts.get("sign_disagree_s", 180)),
),
tripped=bool(store.data.get("sign_tripped", False)),
reason=str(store.data.get("sign_reason", "") or ""),
)
if self.sign_wd.tripped:
_LOG.error("SAFETY-02 was TRIPPED before this restart and stays "
"tripped: %s. The inverter is held at 0 W. Restarting the "
"add-on does not clear this - start control from the UI "
"once you know why the command opposed the meter.",
self.sign_wd.reason or "(reason not recorded)")
# P1 ingestion (TEL-01). `meter_source: off` keeps the original
# single-entity meter_entity path, so an existing install is unchanged
@@ -243,9 +260,18 @@ class Controller:
standing = self.last_write if self.last_write is not None else self.target
if self.sign_wd.evaluate(standing, self.grid, self.following, now) \
and not was_tripped:
# ⚠️ REASON FIRST, FLAG SECOND, and each `set` writes the file. The
# ordering is the whole atomicity story: a crash between the two
# persists a reason nothing reads, which is harmless, where the other
# order persists a latch with no explanation for the operator who has
# to decide whether to clear it.
self.store.set("sign_reason", self.sign_wd.reason)
self.store.set("sign_tripped", True)
self.log_event("FAILSAFE " + self.sign_wd.reason)
_LOG.error("SAFETY-02 TRIPPED - %s. Commanding 0 W and holding it "
"until control is started again.", self.sign_wd.reason)
"until control is started again. This survives an add-on "
"restart; restarting will not clear it.",
self.sign_wd.reason)
stale_after = float(self.o.get("stale_input_s", 15))
if not self.inputs_ok and not self.sign_wd.tripped:
@@ -352,25 +378,17 @@ class Controller:
# --- SAFETY-02's arming gate ------------------------------------------
# Was the number about to go on the wire chosen by the grid-following
# loop, or by a strategy that commands against the meter on purpose?
# Only the former is expected to agree in sign with P1. See gate 1 in
# watchdog.py - without this, every maintenance grid charge trips the
# failsafe within three minutes of starting.
# Only the former is expected to agree in sign with P1 - without this,
# every maintenance grid charge trips the failsafe within three minutes.
#
# ⚠️ "THE LOOP WON" IS NOT ENOUGH, and the difference is a whole class of
# false trip. A LIMIT claim can survive the loop winning and still
# replace its number: maintenance's charge-only shaping is
# `hi = -charge_w`, so the loop can ask for +500 W of discharge, win the
# arbitration, and have the emitted value come out at -2500 W of grid
# charge. Winner says "loop"; the wire carries maintenance's intent. So
# the test is on the NUMBER, not on the name: the loop is following only
# while the emitted value points the way the loop asked and is no larger
# than it asked - i.e. limits reduced it, they did not redirect it.
# Zero on either side falls out as not-following, which costs nothing:
# a 0 W command cannot be a wrong-direction command.
asked = self.loop_w
self.following = (resolution.winner == "loop" and asked is not None
and resolution.target_w * asked > 0
and abs(resolution.target_w) <= abs(asked))
# ⚠️ THE EXPRESSION LIVES IN watchdog.py, NOT HERE, and that is not
# tidiness. It was inline, test_watchdog.py held a transcription of it,
# and the P_PEAK case meant to force whoever lands peak shaving to come
# and change an expectation did not: editing this line alone to
# `winner in ("loop", "peak")` left every suite green while the watchdog
# went blind for every peak event. Read loop_is_following() before
# touching the gate - the forward hazard and the reasoning are there.
self.following = loop_is_following(resolution, self.loop_w)
self.target = resolution.target_w
self.reason = resolution.explain()
await self.write_setpoint(self.target)
@@ -600,9 +618,10 @@ class Controller:
"text": "SIGN-DISAGREEMENT FAILSAFE - " + self.sign_wd.reason
+ ". The inverter is held at 0 W. Find out why the "
"command opposed the meter before starting control "
"again; starting clears this."})
"again; starting clears this. Restarting the "
"add-on does NOT clear it."})
else:
held = self.sign_wd.held_s
held = self.sign_wd.held_s(datetime.now(timezone.utc))
out.append({"ok": True, "warn": False,
"text": f"sign watchdog: armed (trips after "
f"{self.sign_wd.cfg.hold_s:.0f} s opposed, both over "
@@ -672,6 +691,8 @@ class Controller:
# An operator pressing start has, at minimum, looked at the
# banner that says why.
self.sign_wd.reset()
self.store.set("sign_tripped", False)
self.store.set("sign_reason", "")
self.log_event("sign-disagreement failsafe cleared by operator")
if not self.auto:
self.target = 0.0
goodwe_controller/app/store.py
+23 -2
View File
@@ -22,11 +22,30 @@ _LOG = logging.getLogger("goodwe.store")
# `auto_start` option only when the key is absent - if a default supplied False,
# the fallback could never fire and auto_start would silently do nothing on
# every fresh install.
#
# ⚠️ `sign_tripped` IS in here, and the difference from `auto` is the whole
# reason both decisions are written down. `auto` has an option to fall back to
# (`auto_start`), so a stored default would mask a fresh install. A latched
# failsafe has no such fallback: absent means "never tripped", which is exactly
# what a default of False says.
#
# It is persisted for the reason this module's docstring gives for maintenance
# state, applied to something with a much shorter fuse. SAFETY-02's latch is the
# only thing standing between a controller that has been caught driving the wrong
# way and its own restart, and "restart the add-on" is the FIRST thing an operator
# looking at a FAILSAFE banner will try. Held only in memory it came back armed,
# into the fault, unattended - measured: after reconstructing Controller against
# the same store, `tripped` was False, the banner read NOT READY, and it resumed
# commanding -1000, -2000 W before re-tripping 180 s later. Under a supervisor
# that restarts a crashed add-on that is a loop, three minutes of wrong-direction
# power at a time, forever.
DEFAULTS = {
"phase": "idle",
"phase_started": None,
"last_completed": None,
"last_start_attempt": None,
"sign_tripped": False,
"sign_reason": "",
}
@@ -42,8 +61,10 @@ class Store:
loaded = json.load(fh)
if isinstance(loaded, dict):
self.data = {**DEFAULTS, **loaded}
_LOG.info("state restored: phase=%s last_completed=%s",
self.data.get("phase"), self.data.get("last_completed"))
_LOG.info("state restored: phase=%s last_completed=%s "
"sign_tripped=%s",
self.data.get("phase"), self.data.get("last_completed"),
self.data.get("sign_tripped"))
except FileNotFoundError:
_LOG.info("no saved state, starting fresh")
except (json.JSONDecodeError, OSError) as err:
goodwe_controller/app/watchdog.py
+162 -28
View File
@@ -18,15 +18,36 @@ module over both of them. Every number in this file was measured on those two
days, not chosen:
day samples gated in sign agreement
2026-08-23 4 127 / 16 349 92.9 % (healthy)
2026-08-20 6 892 / 15 598 55.6 % (the whole fault day)
episode A 15:26-16:43 0.3 %
episode B 17:11-18:28 0.8 %
episode C 20:01-20:31 11.0 %
episode D 20:55-21:11 0.0 %
2026-08-23 4 212 / 17 280 93.4 % (healthy)
2026-08-20 7 182 / 17 280 56.4 % (the whole fault day)
cluster A 15:26-16:43 0.3 %
cluster B 17:11-18:28 0.9 %
cluster C 20:01-20:31 10.5 %
cluster D 20:55-21:11 0.0 %
That is the signature, and it is the SAME signature in all four episodes: not a
noisier version of healthy, a near-total inversion.
⚠️ THOSE AGREEMENT FIGURES ARE THE WEAKEST EVIDENCE ON THIS PAGE, and an earlier
version of this comment leaned on them as though they were the strongest. The
clusters are found by looking for sustained opposed samples, so measuring "how
opposed is this cluster" over them is close to a tautology - and "the healthy day
contains no such cluster" is the same sentence as "the healthy day produces no
trip", not independent corroboration of it.
What actually carries the argument is two things neither the detector nor the
clustering assumes:
1. The RUN-LENGTH DISTRIBUTION. The longest unbroken opposed run on the healthy
day is 30 s. On the fault day there are 18 runs over 180 s and the longest is
1750 s. Nothing about that gap was chosen - it is what the two days measure.
2. The clusters land where a human independently wrote down onsets. NOTES.md
§4.7 records 15:01, 16:56, 19:57 and 20:23 from watching it happen, before
any of this existed, and cluster D coincides with the separately documented
inverter-off runaway of §4.7 item 2.
Be precise about (2), because the convenient overstatement is available and wrong:
the clustering does NOT reproduce four episodes. It yields THREE clusters over the
four documented onsets - C spans both 19:57 and 20:23 - plus a fourth for §4.7
item 2, which is not one of the four. Every trip does fall inside a documented
window, which is the claim worth making.
------------------------------------------------------------------------------
WHY IT LIVES HERE AND NOT IN THE FIRMWARE OR THE WITNESS
@@ -68,21 +89,31 @@ Four gates, and each one exists to exclude a specific thing that is NOT a fault:
1. `following`. ⚠️ THE GATE THAT IS EASIEST TO FORGET AND MOST EXPENSIVE TO
OMIT. Sign agreement is a property of a GRID-FOLLOWING loop. It is not
expected of a strategy that deliberately commands against the meter -
maintenance is exactly that. A maintenance grid charge of 2500 W into a house
drawing 500 W puts +3000 W on the meter against a -2500 W command: opposed,
maintenance is exactly that. A maintenance grid charge of 1500 W into a house
drawing 500 W puts +2000 W on the meter against a -1500 W command: opposed,
both far over any sane threshold, and sustained for HOURS. Armed
unconditionally this watchdog would trip on every single maintenance cycle,
which is not a tuning problem, it is the wrong question being asked. So the
caller says whether the grid-following loop actually won the arbitration this
cycle, and the timer only accumulates when it did.
⚠️ 1500 W, NOT THE DEFAULT 2500 W, AND THE REASON IS NOT COSMETIC. At the
shipped defaults `maintenance_charge_w` is 2500 and `max_w` is 2000, so the
cheap-window claim is `hi = -2500` against a supervised `lo = -2000`: lo > hi,
which arbiter.py resolves as a CONTRADICTION and 0 W, not as a -2500 W charge.
The example above is written at 1500 W because that is a combination the
schema and the arbiter both actually produce. (The contradiction at the
defaults is a real pre-existing bug - a default maintenance charge silently
never happens in a cheap window - but it is maintenance's, not this module's,
and it is filed separately. Do not "fix" it here.)
2. The strict `< 0` product. "Opposed" and "neither side is zero" in one
expression.
3. The 300 W magnitude gate ON BOTH SIDES. Near zero the sign is noise, and near
zero is where a healthy loop LIVES - the deadband holds the command still
while the meter wanders either side of `target_grid_w`. On the healthy day
only 4 127 of 16 349 samples (25 %) are even gated in; the other 75 % are the
only 4 212 of 17 280 samples (24 %) are even gated in; the other 76 % are the
converged band where sign carries no information. This is also why the
deadband cannot produce a false trip: inside it the meter is by definition
under 300 W.
@@ -109,6 +140,33 @@ a stuck reading of +800 W makes the loop command discharge and keep commanding
discharge. Command and meter converge on AGREEING. Freezes are SAFETY-12's and
SAFETY-01's problem, and this watchdog neither helps nor hinders there.
⚠️ ALL OF THAT DEPENDS ON ONE PROPERTY OF THE CONTROL LAW, so it is an invariant
and not a happy accident: the saturation freeze in control.py must keep letting
the command FALL, CROSS ZERO AND REVERSE - "may not wind further in the direction
it is already pushing", not "only corrections that shrink |i_w|". control.py
records the shrink-only encoding as tried and rejected because it pins the command
at its last value indefinitely; a pinned command against a meter that has moved on
is precisely a sustained sign disagreement, so under that encoding this watchdog
WOULD false-trip on a plain load step. If anyone ever revisits that freeze, this
module's no-false-positive argument is downstream of the answer.
------------------------------------------------------------------------------
WHAT IT DOES NOT CATCH
An inverter that ACTS opposite to what it was told. Tell it to discharge 2000 W
and have it charge instead, and the meter moves FURTHER the way it was already
going - so the command and the meter agree, all the way into the rail. This
watchdog compares the loop's output direction against the input state it was
regulating; it is blind to the plant lying about which way it went. That is the
saturation freeze's question (|command - the battery's actual power| > 500 W,
NOTES.md §4.7 row 4), and the two checks are complementary rather than
overlapping. Neither substitutes for the other.
Nor does it see a sign inversion introduced DOWNSTREAM of this add-on - in the
firmware, on the wire, or in a future driver. Our view stays self-consistent and
this stays silent. That is a wire-versus-intent problem and it belongs to the
witness, which is the only vantage point that sees both.
------------------------------------------------------------------------------
STATE, TRIP AND LATCH
@@ -123,6 +181,15 @@ hand the actuator straight back to the loop that was driving the wrong way -
three minutes later, forever. `tripped` therefore stays true until an operator
`reset()`s it by starting control again, which is a deliberate human act after
looking at why.
⚠️ AND THE LATCH HAS TO OUTLIVE THE PROCESS. Held only in memory it is not a
latch at all - a container restart clears it and the add-on comes back armed,
into the same fault, unattended. Worse than merely losing state: "restart the
add-on" is the first thing an operator confronted with a FAILSAFE banner and a
0 W inverter will try, and under a supervisor that restarts a crashed add-on it
is a loop of three-minute wrong-direction bursts. `tripped` and `reason` are
therefore constructor arguments, restored from `Store` in `Controller.__init__`
and written back the moment the trip happens. See store.py.
"""
from dataclasses import dataclass
@@ -137,6 +204,71 @@ class SignWatchdogConfig:
hold_s: float = 180.0
def disagrees(commanded_w, grid_w, following: bool,
cfg: SignWatchdogConfig) -> bool:
"""The predicate, and the ONLY place it is written down.
⚠️ Public because `sim/replay_sign.py` needs it too - to describe episodes, to
measure agreement, to sweep the clock-skew guard - and it had reimplemented it
inline. Two copies of a safety predicate is one copy plus a future
disagreement about what the check means, and the copy that drifts is always
the one in the harness that grades it.
"""
return (following
and commanded_w is not None and grid_w is not None
and commanded_w * grid_w < 0
and abs(commanded_w) > cfg.min_w
and abs(grid_w) > cfg.min_w)
def loop_is_following(resolution, loop_asked) -> bool:
"""Did the grid-following loop actually choose the number going on the wire?
Gate 1 of the predicate above, and it lives here rather than inline in
`cycle()` for the reason `disagrees()` does: the test suite has to be able to
assert on THE SHIPPED EXPRESSION, not on a transcription of it that drifts
apart from it in silence.
⚠️ IT WAS A TRANSCRIPTION, AND THAT WAS MEASURED, NOT FEARED. The P_PEAK case
in test_watchdog.py exists to make whoever lands peak shaving come here and
change an expectation. With the expression copied into the test it did not do
that: editing the shipped gate alone to `winner in ("loop", "peak")` left every
suite green while the watchdog went blind for the whole of every peak event.
One definition, two callers, and that test now bites.
`resolution` the arbiter's Resolution. Duck-typed on `.winner` and
`.target_w` so this module still imports nothing from arbiter.
`loop_asked` what the grid-following loop claimed this cycle, or None if it
did not claim at all.
"The loop won" is NOT the question, and the difference is a whole class of
false trip. A LIMIT claim can survive the loop winning and still replace its
number: maintenance's cheap-window shaping is `hi = -maintenance_charge_w`, so
at 1500 W the loop can ask for +500 W of discharge, win the arbitration, and
have the emitted value come out at -1500 W of grid charge. Winner says "loop";
the wire carries maintenance's intent.
So the test is on the NUMBER: the loop is following only while the emitted
value points the way the loop asked AND is no larger than it asked - limits
reduced it, they did not redirect it. The magnitude half is load-bearing on
its own, because a `lo` limit can raise magnitude without flipping sign, and
that is not the loop following either. Zero on either side falls out as
not-following, which costs nothing: a 0 W command cannot be a
wrong-direction command.
⚠️ FORWARD HAZARD, and the claim block in cycle() already anticipates it.
`winner == "loop"` disarms the watchdog under ANY higher-priority SET claim.
Today only maintenance and the safety stops emit one and neither follows the
meter, so the gate costs nothing. A peak-shaving strategy claiming SET at
P_PEAK **is** grid-following in sign, and the moment it lands this goes blind
for the duration of every peak event. Widening it means asking "does the
winning strategy follow the meter", not adding another name to a comparison.
"""
return (resolution.winner == "loop" and loop_asked is not None
and resolution.target_w * loop_asked > 0
and abs(resolution.target_w) <= abs(loop_asked))
class SignWatchdog:
"""One instance per actuator, evaluated once per control cycle.
@@ -147,11 +279,16 @@ class SignWatchdog:
arbiter applies to the actuator, applied to the detector's own state.
"""
def __init__(self, cfg: SignWatchdogConfig | None = None):
def __init__(self, cfg: SignWatchdogConfig | None = None, *,
tripped: bool = False, reason: str = ""):
self.cfg = cfg or SignWatchdogConfig()
self.disagree_since: datetime | None = None
self.tripped = False
self.reason = ""
# ⚠️ RESTORED FROM THE STORE, NOT DEFAULTED. A latch that lives only in
# memory is not a latch: a container restart clears it, and the add-on
# comes back armed, into the same fault, with no operator involved. See
# store.py's note on `sign_tripped` for the measured behaviour.
self.tripped = bool(tripped)
self.reason = reason if self.tripped else ""
# -- the whole check, and it is a pure function of its arguments ----------
def evaluate(self, commanded_w, grid_w, following: bool,
@@ -178,15 +315,7 @@ class SignWatchdog:
return True
cfg = self.cfg
disagree = (
following
and commanded_w is not None and grid_w is not None
and commanded_w * grid_w < 0
and abs(commanded_w) > cfg.min_w
and abs(grid_w) > cfg.min_w
)
if not disagree:
if not disagrees(commanded_w, grid_w, following, cfg):
self.disagree_since = None
return False
@@ -206,13 +335,18 @@ class SignWatchdog:
)
return True
@property
def held_s(self) -> float:
"""How long the current disagreement has been standing, for the UI."""
def held_s(self, now: datetime) -> float:
"""How long the current disagreement has been standing, for the UI.
⚠️ Takes the clock rather than reading one. It was a property calling
`datetime.now()`, which quietly broke the property this module's header
advertises - every other observable here is a pure function of what it
was handed - and an unclocked function is one nothing can test, which is
exactly what happened to it.
"""
if self.disagree_since is None:
return 0.0
return (datetime.now(self.disagree_since.tzinfo)
- self.disagree_since).total_seconds()
return (now - self.disagree_since).total_seconds()
def reset(self) -> None:
self.disagree_since = None
goodwe_controller/config.yaml
+2 -2
View File
@@ -1,5 +1,5 @@
name: GoodWe RS485 Controller
version: "0.5.0"
version: "0.5.1"
slug: goodwe_controller
description: >-
Drives a GoodWe ES/BP battery inverter over RS485 by emulating its smart
@@ -101,7 +101,7 @@ options:
# Both defaults are measured, not chosen. On the captured healthy day
# (2026-08-23) the longest legitimate opposed run - a load step the loop has
# not caught up with - is 30 s, and sign agreement over the gated samples is
# 92.9 %. On the captured fault day (2026-08-20) the four episodes run 0.0-11 %
# 93.4 %. On the captured fault day (2026-08-20) the clusters run 0.0-10.5 %
# agreement with opposed runs up to 1752 s. 300 W / 180 s sits in the middle of
# a very wide gap: at 60 s the healthy day still trips nothing, at 300 s the
# fault day still trips 13 times. There is no reason to move either number
goodwe_controller/test_watchdog.py
+244 -2
View File
@@ -13,7 +13,9 @@ table-driven half: boundaries, timers, the latch, and the arming gate.
import sys
from datetime import datetime, timedelta, timezone
from app.watchdog import SignWatchdog, SignWatchdogConfig
from app.watchdog import (
SignWatchdog, SignWatchdogConfig, loop_is_following,
)
fails = []
T0 = datetime(2026, 8, 20, 15, 0, 0, tzinfo=timezone.utc)
@@ -150,6 +152,101 @@ p = validate_startup_config({**base, "max_w": 2000, "sign_disagree_w": 1999},
-5000, 5000)
check("one watt under the ceiling is reachable, so accepted", p == [])
print()
print("the arming gate, derived from the REAL arbiter")
# ⚠️ THIS IS THE DECISION THE WHOLE TICKET TURNS ON, and until now it was
# asserted in three comment blocks and verified by nothing: every other case in
# this file passes `following` in as a literal. What has to hold is that the
# expression in main.py - loop won AND the emitted value points the way the loop
# asked AND is no larger - is never True for a resolution the loop did not
# actually get. So run real claims through the real resolve() and derive it.
from app.arbiter import ( # noqa: E402
P_LOOP, P_MAINT_SHAPE, P_MAINTENANCE, P_PEAK, P_SAFETY, Claim, resolve,
)
def following_from(claims, asked):
"""THE SHIPPED gate, not a copy of it.
⚠️ This used to transcribe main.py's expression with a comment saying
"keep the two in step", which is exactly the arrangement that does not keep
two things in step. Measured: editing the shipped gate alone to
`winner in ("loop", "peak")` left this suite - including the P_PEAK row below,
whose entire job is to catch that edit - completely green, while the watchdog
went blind for the duration of every peak event. Import it or the P_PEAK
assertion is decoration.
"""
r = resolve(claims)
return r, loop_is_following(r, asked)
MAXW = Claim.limit("supervised", P_SAFETY, -2000, 2000, "max_w")
DEV = Claim.limit("device", P_SAFETY, -5000, 5000, "rating")
# name, claims, what the loop asked, expected `following`
TABLE = [
("the loop alone, unopposed",
[Claim.set("loop", P_LOOP, 900)], 900, True),
("the loop bound by max_w - reduced, not redirected",
[Claim.set("loop", P_LOOP, 4000), MAXW], 4000, True),
("charging, bound by max_w on the other side",
[Claim.set("loop", P_LOOP, -4000), MAXW], -4000, True),
("maintenance owns the setpoint outright",
[Claim.set("loop", P_LOOP, 900),
Claim.set("maintenance", P_MAINTENANCE, -2500, "charge"), MAXW], 900, False),
("maintenance charge-only clamps the loop to 0 W",
[Claim.set("loop", P_LOOP, 900),
Claim.limit("maintenance", P_MAINT_SHAPE, hi=0.0, reason="charge-only"),
MAXW], 900, False),
# ⚠️ THE CASE `winner == \"loop\"` ALONE WOULD HAVE GOT WRONG. 1500 W, not
# the default 2500 W: at max_w 2000 a -2500 hi against a -2000 lo is an
# arbiter contradiction, not a charge - see main.py.
("cheap-window charge REDIRECTS the loop while it still 'wins'",
[Claim.set("loop", P_LOOP, 500),
Claim.limit("maintenance", P_MAINT_SHAPE, hi=-1500, reason="cheap-window"),
MAXW], 500, False),
("...and the contradiction at the shipped defaults is not following either",
[Claim.set("loop", P_LOOP, 500),
Claim.limit("maintenance", P_MAINT_SHAPE, hi=-2500, reason="cheap-window"),
MAXW], 500, False),
("a lo limit RAISES the magnitude without flipping the sign",
[Claim.set("loop", P_LOOP, 200),
Claim.limit("floor", P_SAFETY, lo=800), MAXW], 200, False),
("a safety stop wins outright",
[Claim.set("loop", P_LOOP, 900),
Claim.set("safety", P_SAFETY, 0.0, "stopped"), MAXW], 900, False),
("no claims at all - the arbiter's own failsafe",
[MAXW, DEV], None, False),
("the loop asked for zero",
[Claim.set("loop", P_LOOP, 0.0), MAXW], 0.0, False),
# The forward hazard, asserted as it stands TODAY rather than as it should
# be. When peak shaving lands, these expectations are what have to change.
("peak shaving at P_PEAK disarms the gate (loop_is_following's forward hazard)",
[Claim.set("loop", P_LOOP, 900),
Claim.set("peak", P_PEAK, 1800, "shaving"), MAXW], 900, False),
# ⚠️ AND THE ROW ABOVE CANNOT DETECT A WIDENED GATE ON ITS OWN, which was
# found by trying it: with the shipped gate edited to
# `winner in ("loop", "peak")` that case STILL comes out False, because peak
# shaving asked for MORE than the loop and the magnitude term rejects it
# anyway. The winner term is masked. This row isolates it - a peak claim
# smaller than the loop's ask and pointing the same way, so `winner` is the
# only term that can decide - and it is what actually turns red if the gate
# is widened without anyone revisiting the hazard.
("...and with peak BELOW the loop's ask, only the winner term separates them",
[Claim.set("loop", P_LOOP, 1800),
Claim.set("peak", P_PEAK, 900, "shaving"), MAXW], 1800, False),
]
for name, claims, asked, expect in TABLE:
r, got = following_from(claims, asked)
check(f"{name} -> following={expect}", got is expect)
if got is not expect:
print(f" resolution was {r.explain()!r}")
# And the gate is never True when the loop did not win, whatever else is going on.
check("no configuration above arms the gate without the loop winning",
all(not following_from(c, a)[1]
for _, c, a, _ in TABLE if following_from(c, a)[0].winner != "loop"))
print()
print("end to end: the trip is COMMAND ZERO, not alarm-only")
# ⚠️ THE FAULT INJECTED HERE IS A NEGATIVE GAIN, and that is not a contrivance -
@@ -237,10 +334,155 @@ check("the status page says so in words",
any("SIGN-DISAGREEMENT FAILSAFE" in c["text"] for c in ctrl.checks()))
check("the banner leads with it",
"FAILSAFE" in ctrl.status_payload()["banner"])
check("MQTT reports the failsafe state", ctrl.sign_wd.tripped)
check("it stays at 0 W on later cycles too",
hass.writes[-1] == 0.0 and hass.writes.count(0.0) >= 1)
# ⚠️ THIS USED TO BE `check("MQTT reports the failsafe state", ctrl.sign_wd.tripped)`
# which restates the assertion three lines above and never touches the payload.
# The `status: "failsafe"` value was shipped untested. Capture what publish()
# actually hands the broker.
published = {}
ctrl.mqtt = type("P", (), {"enabled": True,
"publish": lambda _s, v: published.update(v),
"close": lambda _s: None})()
ctrl.publish()
check("MQTT's controller-status sensor reports 'failsafe'",
published.get("status") == "failsafe")
ctrl.sign_wd.reset()
ctrl.publish()
check("...and goes back to 'running' once cleared",
published.get("status") == "running")
print()
print("the latch survives a restart - it is the only thing between a tripped")
print("controller and the first thing an operator will try")
# ⚠️ THE BLOCKER THIS FILE MISSED FIRST TIME ROUND. `auto` is persisted and
# `tripped` was not, so reconstructing a Controller against the same Store -
# which is exactly what a container restart does - came back armed, into the
# fault, with no operator involved: tripped=False, banner NOT READY, and it
# resumed commanding -1000 then -2000 W before re-tripping 180 s later.
class _MemStore:
"""A Store that keeps its dict across "restarts", like the real file does."""
def __init__(self):
self.data = {"phase": "idle", "sign_tripped": False, "sign_reason": ""}
def set(self, k, v):
self.data[k] = v
def get_time(self, *a):
return None
async def _restart():
shared = _MemStore()
hass1 = _Hass()
opts = {"setpoint_entity": "number.sp", "meter_entity": "sensor.grid",
"soc_entity": "sensor.soc", "batt_entity": "sensor.batt",
"meter_source": "off", "max_w": 2000, "gain": -0.6,
"sign_disagree_w": 300, "sign_disagree_s": 1, "auto_start": True}
a = Controller(opts, hass1, shared, _P(), stop=asyncio.Event())
a.auto = True
a.dev_min, a.dev_max = -5000.0, 5000.0
for _ in range(12):
await a.read_inputs()
await a.cycle()
await asyncio.sleep(0.25)
# The restart. Same options, same store, brand new Controller and Hass.
hass2 = _Hass()
b = Controller(opts, hass2, shared, _P(), stop=asyncio.Event())
b.auto = True
b.dev_min, b.dev_max = -5000.0, 5000.0
for _ in range(6):
await b.read_inputs()
await b.cycle()
await asyncio.sleep(0.05)
return a, b, shared, hass2
pre, post, store, hass2 = asyncio.run(_restart())
check("it tripped before the restart", pre.sign_wd.tripped)
check("the trip reached the store", store.data["sign_tripped"] is True)
check("...with the reason beside it, so the operator is told why",
"SIGN_DISAGREEMENT" in store.data["sign_reason"])
check("the new Controller comes back TRIPPED", post.sign_wd.tripped)
check("...and says why, restored rather than re-detected",
"SIGN_DISAGREEMENT" in post.sign_wd.reason)
check("...commands 0 W from its very first cycle", post.target == 0.0)
check("...and never commanded anything else after the restart",
hass2.writes != [] and set(hass2.writes) == {0.0})
check("the banner still leads with the failsafe",
"FAILSAFE" in post.status_payload()["banner"])
# ⚠️ TWO PRESSES, and the first one must NOT clear it. `auto` is True while
# tripped, so the first press stops control; only the transition back to True
# clears. That is the documented behaviour and it is worth an assertion, because
# a one-press clear would be reachable by an operator who never read the banner.
asyncio.run(post.handle_action("auto_toggle")) # -> stopped
check("pressing stop does not clear the failsafe",
post.sign_wd.tripped and store.data["sign_tripped"] is True)
asyncio.run(post.handle_action("auto_toggle")) # -> started
check("pressing start DOES clear it, and clears the store with it",
not post.sign_wd.tripped and store.data["sign_tripped"] is False
and store.data["sign_reason"] == "")
# One more restart from the cleared store, to prove the clear persisted too.
fresh = Controller({"setpoint_entity": "number.sp", "meter_source": "off"},
_Hass(), store, _P(), stop=asyncio.Event())
check("...so the next restart comes back armed and untripped, as it should",
not fresh.sign_wd.tripped)
# ⚠️ THE WRITE ORDERING IS AN INVARIANT, SO IT GETS AN ASSERTION. main.py writes
# `sign_reason` and THEN `sign_tripped`, each one its own `save()`, so that a
# process that dies between the two leaves a reason nothing reads - harmless -
# rather than a latch with no explanation for the operator who has to decide
# whether to clear it. Documented reasoning is not enforcement: swapping those two
# lines used to pass every check in this file.
#
# The death is modelled as the second write raising. In the field it is a power
# cut or a container kill (Store.save() swallows OSError rather than raising), so
# what is asserted here is the PROPERTY - what survives on disk - not the
# mechanism.
class _CrashOnFlag(_MemStore):
def set(self, k, v):
if k == "sign_tripped" and v:
raise RuntimeError("power cut, mid-trip")
super().set(k, v)
async def _crash_mid_write():
store = _CrashOnFlag()
hass = _Hass()
opts = {"setpoint_entity": "number.sp", "meter_entity": "sensor.grid",
"soc_entity": "sensor.soc", "batt_entity": "sensor.batt",
"meter_source": "off", "max_w": 2000, "gain": -0.6,
"sign_disagree_w": 300, "sign_disagree_s": 1, "auto_start": True}
c = Controller(opts, hass, store, _P(), stop=asyncio.Event())
c.auto = True
c.dev_min, c.dev_max = -5000.0, 5000.0
died = False
for _ in range(12):
try:
await c.read_inputs()
await c.cycle()
except RuntimeError:
died = True
break
await asyncio.sleep(0.25)
return store, died
crashed_store, died = asyncio.run(_crash_mid_write())
check("the second write is the one that sets the flag", died)
check("a crash between the two writes leaves the REASON on disk",
"SIGN_DISAGREEMENT" in crashed_store.data.get("sign_reason", ""))
check("...and never a latch with nothing to explain it",
not crashed_store.data.get("sign_tripped"))
print()
if fails:
print(f"{len(fails)} FAILED: {', '.join(fails)}")