diff --git a/HANDOVER.md b/HANDOVER.md index dd4846b..2e6bca1 100644 --- a/HANDOVER.md +++ b/HANDOVER.md @@ -53,14 +53,22 @@ Run the checks before deploying. No framework needed: ```bash cd goodwe_controller python3 test_control.py && python3 test_maintenance.py && python3 test_arbiter.py \ - && python3 test_validate.py && python3 test_p1.py && python3 test_loop.py + && python3 test_validate.py && python3 test_p1.py && python3 test_watchdog.py \ + && 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_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 wall-clock property about it (SAFETY-12); mocking the clock there would leave the -assertion testing the mock. The other five suites together are ~395 checks and finish in -seconds. If you shorten `test_loop.py`'s windows, re-measure and correct this figure — +assertion testing the mock. The other six suites together are ~440 checks and finish in +seconds — `test_watchdog.py` spends ~3 s of that driving a real `Controller` through a +trip. If you shorten `test_loop.py`'s windows, re-measure and correct this figure — an understated runtime is what makes a slow test look like a hung one, and a slow test that looks hung gets deleted. diff --git a/goodwe_controller/CHANGELOG.md b/goodwe_controller/CHANGELOG.md index 1f8b237..c3203cf 100644 --- a/goodwe_controller/CHANGELOG.md +++ b/goodwe_controller/CHANGELOG.md @@ -1,5 +1,58 @@ # Changelog +## 0.5.0 + +**SAFETY-02 — the sign-disagreement watchdog.** The add-on now watches whether +the setpoint it is emitting and the grid figure it is regulating on point the +same way. If they oppose each other, both over `sign_disagree_w` (300 W), for +`sign_disagree_s` (180 s) **continuously**, it files a failsafe claim at +`P_SAFETY`, commands **0 W**, and latches there until an operator starts control +again. New module `app/watchdog.py`; new options `sign_disagree_w` and +`sign_disagree_s`. + +This is the gap the existing failsafes leave open. The stale-input watchdog asks +whether the meter is still talking, the saturation freeze asks whether the +inverter can follow, the output clamp asks whether the number is too big — a +loop can pass all three while charging the battery out of the grid, because the +only thing wrong is the sign. On 2026-08-20 a commercial controller did exactly +that four times in one day, for up to 77 minutes at a stretch, and reached a +commanded −14 768 W against a ±5000 W inverter. + +Validated against that day. `sim/replay_sign.py` in the infra repo drives this +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. + +Three decisions worth knowing about: + +- **It is disarmed whenever the grid-following loop is not what is driving.** + Sign agreement is a property of a loop that follows the meter. A maintenance + grid charge opposes the meter *by design* — 2500 W of charge into a house + drawing 500 W is +3000 W on the meter against a −2500 W command, for hours — + and armed unconditionally this would trip on every maintenance cycle. The gate + tests the emitted number, not the winning claim's name: a limit claim can let + the loop "win" and still replace its value with one pointing the other way. +- **The trip latches.** Commanding 0 W removes the command side of the + comparison, so the disagreement always clears the moment the trip works. + Un-latched, this would trip, clear, and hand the actuator back to the loop that + was driving the wrong way, every three minutes forever. +- **`sign_disagree_w` at or above `max_w` is refused at startup** (validator rule + 5). The arbiter never lets a command exceed `max_w`, so such a threshold makes + the watchdog incapable of firing while the status page still reports it armed. + A disarmed guard that advertises itself as armed is the failure mode this + project keeps refusing to ship. + +What it does **not** catch, stated so nobody assumes otherwise: an inverter that +acts opposite to what it was told moves the meter *further* the same way, so +command and meter agree while everything runs away. That is the saturation +freeze's job, which compares the command against the battery's actual power. + +`test_validate.py`'s "the smallest sane max_w still starts" case now names a +reachable sign threshold, because a 1 W ceiling genuinely does disarm the 300 W +default and is now correctly refused. + ## 0.4.0 **CFG-01.** The add-on now refuses to start on a configuration it cannot honour, diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md index 82c3731..1717980 100644 --- a/goodwe_controller/DOCS.md +++ b/goodwe_controller/DOCS.md @@ -24,6 +24,9 @@ The inverter **holds its last command forever** — it has no meter-timeout. So: commanding it. - This add-on commands 0 W when its inputs go missing, when you stop control, and when it shuts down. +- This add-on commands 0 W **and latches there** if what it is commanding and + what the meter reads point opposite ways for three minutes — see "The + sign-disagreement failsafe" below. - The **optional RS485 e-stop** is the only thing that covers this machine dying. Without it, a failed host leaves the battery latched at its last command until someone intervenes. @@ -278,6 +281,54 @@ behind it would trip the firmware watchdog on a system that is working fine. | `cycle_floor_s` | 15 | The longest the loop may go without evaluating. A deadline on the gap between cycles, reset by every cycle — on a healthy meter it never elapses and changes nothing. Raise it only if your meter is slower than ~10 s; it must stay under `meter_max_age_s`, past which there is no reading to evaluate anyway | | `stale_input_s` | 15 | How long inputs may be missing before commanding 0 W. In P1 mode this clock starts only *after* `meter_max_age_s` has already expired — the two stack, see "How long a dead meter takes to reach 0 W" | | `auto_start` | false | Start controlling on boot (only after commissioning) | +| `sign_disagree_w` | 300 | Both the command and the meter must exceed this before a sign difference counts. Must stay **below `max_w`** or the watchdog can never fire, and the add-on refuses to start | +| `sign_disagree_s` | 180 | How long they must stay opposed, continuously, before the failsafe trips | + +#### The sign-disagreement failsafe + +If the setpoint this add-on is emitting and the grid figure it is regulating on +point **opposite ways** — charging the battery while the house imports, or +discharging it while the house exports — with both over `sign_disagree_w`, for +`sign_disagree_s` **continuously**, the add-on commands **0 W** and holds it +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. + +**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 +polarity error anywhere between the meter and the wire, and an integrator so far +wound up that unwinding takes minutes. It is the check that would have caught all +four episodes of 2026-08-20, when a commercial controller charged the battery out +of the grid, four times, for up to 77 minutes at a time. + +**What it is not for.** It does not detect an inverter that acts opposite to what +it was told — that moves the meter *further* the same way, so command and meter +agree while everything runs away. That is the saturation freeze's job +(`saturation_w` / `saturation_cycles`), which compares the command against the +battery's actual power. The two checks are complementary and neither replaces the +other. + +**Why it will not false-trip on ordinary operation.** A battery that refuses a +command (empty, full, standby) leaves the meter where it was, so command and +meter agree. A frozen meter drives the loop into agreeing with it. Around the +resting point both signals are inside the 300 W gate, so the deadband can never +produce one. A load step does flip the meter's sign before the loop catches up — +measured on a full captured day, the longest such crossing is 30 s, against a +180 s window. And the watchdog is **disarmed whenever the grid-following loop is +not what is driving**: a maintenance grid charge opposes the meter by design and +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 %, +10.5 % and 0.0 %. There is no reason to move either value without a capture that +says so. #### Saturation is counted in cycles, not seconds diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index 5865baa..f34a9de 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -41,6 +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 . import web OPTIONS_PATH = "/data/options.json" @@ -103,6 +104,12 @@ class Controller: store, ) + # 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)), + )) + # 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. @@ -120,6 +127,11 @@ class Controller: self.sat_count = 0 self.i_w = 0.0 # the loop's integrator, carried between cycles self.loop_w = None # what the loop asked for last cycle, or None + # Did the grid-following loop actually win the LAST arbitration? That is + # the question SAFETY-02 has to ask about the command standing on the + # wire right now, so it is recorded when the arbitration happens rather + # than re-derived a cycle later. Starts False: nothing is on the wire. + self.following = False self.reason = "starting" self.grid = self.soc = self.batt = None self.peak_fc = None @@ -214,8 +226,29 @@ class Controller: for msg in result.events: self.log_event(msg) + # --- SAFETY-02, and it runs BEFORE the early return below ------------- + # ⚠️ ORDER MATTERS HERE. If this were evaluated after the "inputs missing + # but not yet stale" hold, a run of held cycles would leave + # `disagree_since` untouched across the gap and the timer would resume + # from a start time that predates it. It is evaluated every cycle, + # unconditionally, so a missing meter CLEARS the window - see watchdog.py. + # + # The value compared is the command STANDING ON THE WIRE, which is what + # this cycle's fresh meter reading is a response to. `last_write` is that + # value after the device clamp and confirmed accepted; `target` covers + # the case where nothing has been written yet (and, deliberately, the + # case where the last write was REJECTED - the inverter is then still + # latched on `last_write`, which is exactly what `last_write` holds). + was_tripped = self.sign_wd.tripped + 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: + 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) + stale_after = float(self.o.get("stale_input_s", 15)) - if not self.inputs_ok: + if not self.inputs_ok and not self.sign_wd.tripped: bad_for = (now - self.inputs_bad_since).total_seconds() if self.inputs_bad_since else 0.0 if bad_for < stale_after: # A single missed poll is not a fault. Hold, keep the heartbeat @@ -235,6 +268,16 @@ class Controller: claims.append(Claim.limit("supervised", P_SAFETY, -max_w, max_w, "max_w")) # --- safety stops ---------------------------------------------------- + # ⚠️ SAFETY-02 is claimed FIRST and unconditionally while latched. It is + # a SET at P_SAFETY, so it outranks maintenance, peak shaving and the + # loop alike - a controller pushing power the wrong way is wrong + # whatever it thinks it is doing. Re-filed EVERY cycle: claims are not + # sticky, and one filed once would lapse the next cycle and hand the + # actuator silently back to the loop that was driving the wrong way. + if self.sign_wd.tripped: + claims.append(Claim.set("failsafe", P_SAFETY, 0.0, + "sign disagreement")) + if not self.auto: claims.append(Claim.set("safety", P_SAFETY, 0.0, "stopped")) elif not self.inputs_ok: @@ -306,6 +349,28 @@ class Controller: resolution = resolve(claims) if resolution.contradiction: self.log_event("ARBITER CONTRADICTION - commanding 0 W, see the log") + # --- 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. + # + # ⚠️ "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)) self.target = resolution.target_w self.reason = resolution.explain() await self.write_setpoint(self.target) @@ -439,7 +504,14 @@ class Controller: "battery": self.batt, "soc": self.soc, "phase": self.maint.phase, - "status": "running" if self.auto else "stopped", + # ⚠️ SAFETY-02's alarm surface, and it is deliberately an existing + # one. IF-01/IF-02 own `ems/event/failsafe` and the `failsafe active` + # binary sensor and neither is built yet; inventing a bespoke topic + # here would be a surface to migrate later. This sensor is already + # discovered, already on every dashboard, and "failsafe" is + # additive to its vocabulary rather than a change to it. + "status": ("failsafe" if self.sign_wd.tripped + else "running" if self.auto else "stopped"), } # ⚠️ ONLY when P1 ingestion is actually running. The ESP32's stale-input # watchdog subscribes to sensor.goodwe_p1_sample_age - see the entity-id @@ -521,6 +593,22 @@ class Controller: out.append({"ok": False, "warn": False, "text": "last write to the inverter was REJECTED - check the log"}) + # SAFETY-02. Reported in both states: a watchdog nobody can see is armed + # is a watchdog nobody trusts when it fires. + if self.sign_wd.tripped: + out.append({"ok": False, "warn": False, + "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."}) + else: + held = self.sign_wd.held_s + 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 " + f"{self.sign_wd.cfg.min_w:.0f} W)" + + (f" - opposed for {held:.0f} s now" if held else "")}) + if not o.get("estop_fitted", False): out.append({"ok": False, "warn": True, "text": "no e-stop fitted: if this host dies the battery stays " @@ -538,7 +626,11 @@ class Controller: def status_payload(self) -> dict: checks = self.checks() bad = [c for c in checks if not c["ok"] and not c["warn"]] - if bad: + if self.sign_wd.tripped: + # Outranks "NOT READY": nothing is missing, something went wrong. + level, banner = "bad", ("FAILSAFE — the command opposed the meter. " + "Inverter held at 0 W.") + elif bad: level, banner = "bad", f"NOT READY — {bad[0]['text']}" elif not self.auto: level, banner = "warn", "Stopped — inverter commanded to 0 W" @@ -572,6 +664,15 @@ class Controller: self.auto = not self.auto self.store.set("auto", self.auto) self.log_event("control started" if self.auto else "control stopped") + if self.auto and self.sign_wd.tripped: + # ⚠️ THE ONLY WAY OUT OF A SIGN-DISAGREEMENT TRIP, on purpose. + # It clears on a deliberate human start, not on the condition + # going away - the condition ALWAYS goes away, because the trip + # commands 0 W and a 0 W command cannot disagree with anything. + # An operator pressing start has, at minimum, looked at the + # banner that says why. + self.sign_wd.reset() + self.log_event("sign-disagreement failsafe cleared by operator") if not self.auto: self.target = 0.0 await self.write_setpoint(0.0, force=True) diff --git a/goodwe_controller/app/validate.py b/goodwe_controller/app/validate.py index a1e8a22..d5be62c 100644 --- a/goodwe_controller/app/validate.py +++ b/goodwe_controller/app/validate.py @@ -74,6 +74,17 @@ NOT READY banner already names the problem, (b) does not apply. was overruled. That firmware file's own comment says a lower bound on this "belongs in CFG-01, not here" - so here it is. + 5. `sign_disagree_w` at or above `max_w`. [(a)] + The arbiter's `supervised` limit bounds every command to +-max_w, and + SAFETY-02's predicate needs `abs(command) > sign_disagree_w`. Set the + threshold at or above the ceiling and the command side of the comparison + is unsatisfiable BY CONSTRUCTION: the watchdog can never fire, on any + input, ever. Same shape as 1 and 4 - the operator set a number, a layer + they cannot see makes it mean something else, and nothing says so. Worse + here, because what the status page reports in that state is "sign + watchdog: armed". A disarmed guard that advertises itself as armed is the + one thing this project has decided repeatedly it will not ship. + WHAT IS DELIBERATELY *NOT* REFUSED, because it is untidy rather than unsafe: - `integrator_max_w` above `max_w`. DOCS.md is right that the only thing the @@ -238,6 +249,26 @@ def validate_startup_config(opts: dict, dev_min=None, dev_max=None) -> list[str] "or rebuild the firmware with a matching max_age_s and update " "validate.FIRMWARE_INGEST_VETO_S.") + # --- 5. a sign watchdog that can actually fire --------------------------- + # ⚠️ Strictly `>=`, matching the predicate exactly: watchdog.py tests + # `abs(commanded_w) > min_w`, and the arbiter holds `abs(commanded_w)` at or + # below max_w, so threshold == max_w is already unsatisfiable. Anything + # BELOW max_w is reachable and therefore fine - this refuses the disarmed + # case only, not "close to the rail". + sign_w = _number(opts, "sign_disagree_w", 300.0) + if sign_w is None: + problems.append(f"sign_disagree_w {opts.get('sign_disagree_w')!r} is not a number") + elif max_w is not None and max_w > 0 and sign_w >= max_w: + problems.append( + f"sign_disagree_w is {sign_w:.0f} W but max_w is {max_w:.0f} W, so " + "the SAFETY-02 sign-disagreement watchdog can never fire: the " + "arbiter never lets a command exceed max_w, and the watchdog needs " + f"a command over {sign_w:.0f} W. It would sit there reporting " + "'armed' on the status page while being incapable of tripping - the " + f"one runaway failure mode this system has actually observed. Lower " + f"sign_disagree_w below {max_w:.0f} W (300 W is the measured " + "default), or raise max_w.") + # --- warnings: real, but not worth refusing over ------------------------- integ = opts.get("integrator_max_w") integ_w = None if not integ else _number(opts, "integrator_max_w", None) diff --git a/goodwe_controller/app/watchdog.py b/goodwe_controller/app/watchdog.py new file mode 100644 index 0000000..5a1d8d9 --- /dev/null +++ b/goodwe_controller/app/watchdog.py @@ -0,0 +1,220 @@ +"""SAFETY-02 - the sign-disagreement watchdog. + +WHAT IT CATCHES, in one sentence: a grid-following loop that is pushing power in +the wrong direction, which every other guard in this system is blind to. + +The stale-input watchdog (SAFETY-01) asks "is the meter still talking?". The +saturation freeze asks "can the inverter follow?". The output clamp asks "is the +number too big?". A loop can pass all three and still be charging the battery +out of the grid while the house imports - the meter is fresh, the inverter is +tracking beautifully, and the magnitude is inside the rating. The only thing +that separates that from healthy operation is the SIGN. + +⚠️ THIS IS NOT A HYPOTHETICAL. On 2026-08-20 the vendor controller (Illusmart) +did exactly this four times in one day (NOTES.md §4.7). Both captures of that +bus are committed - `sim/scenarios/bus-2026-08-20.log.gz` and the healthy +`bus-2026-08-23.log.gz` in the infra repo - and `sim/replay_sign.py` drives this +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 % + +That is the signature, and it is the SAME signature in all four episodes: not a +noisier version of healthy, a near-total inversion. + +------------------------------------------------------------------------------ +WHY IT LIVES HERE AND NOT IN THE FIRMWARE OR THE WITNESS + +Three vantage points see three different things. The firmware knows what it +WROTE, the witness Pi knows what was ON THE WIRE and whether the inverter ACKed +it, and the add-on knows what it INTENDED and holds the authoritative meter. + +This check is not command-versus-wire; it is command-versus-physics. It needs +the emitted setpoint and the P1 net figure in the same place, and it needs the +authority to command zero when they fight: + + - the witness is read-only by charter and cannot command anything, so it could + raise the alarm but never take the trip action the AC requires; + - the firmware has the wire but not the meter. §5.1 makes P1 the only + authoritative grid figure and the board only ever receives its AGE + (`sensor.goodwe_p1_sample_age`), never its value. Putting the check there + would mean shipping the meter to the ESP32 for no other reason, and it would + be C++ per install rather than one generic guard; + - the add-on has both halves already, plus an arbiter that can outrank every + strategy with a P_SAFETY claim. + +The cost of standing here is real and worth stating: this watchdog compares what +we MEANT to put on the wire against the meter. If the firmware or the driver +inverts the sign on the way out, the add-on's own view still agrees with itself +and this check stays silent. That gap is a wire-versus-intent problem and it +belongs to the witness, not here. + +------------------------------------------------------------------------------ +WHAT "DISAGREEMENT" MEANS, PRECISELY + + disagree = following + and commanded_w * grid_w < 0 + and abs(commanded_w) > min_w + and abs(grid_w) > min_w + +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, + 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. + +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 + 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. + +4. The 180 s continuity requirement. This is what excludes the legitimate + transient. A load step reverses the meter's sign instantly while the command + is still where the last cycle left it; slew limiting then needs a few cycles + to cross over. Measured on the healthy day, the LONGEST such crossing is 30 s + - and that is with a vendor controller far cruder than ours. 180 s is six + times that. It is not a fitted threshold: at 60 s the healthy day still + produces zero trips, and at 300 s the fault day still produces 13. + +⚠️ A BATTERY THAT REFUSES A COMMAND CANNOT TRIP THIS, and that is structural +rather than lucky. At SoC 10 % in Standby, a 1350 W discharge command into a +1350 W import is DECLINED: the battery does nothing, so the meter stays at ++1350 W and the command stays at +1350 W. Same sign. Agreement. A refusal leaves +the meter where it was; only the inverter ACTING moves it, and acting in the +commanded direction moves it toward zero, not through it. The failure mode this +check keys on is the opposite of a refusal - it is the inverter obediently doing +a wrong thing. + +⚠️ A FROZEN METER cannot trip it either, for the same reason read the other way: +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. + +------------------------------------------------------------------------------ +STATE, TRIP AND LATCH + +One timestamp, not a sample counter. The cycle rate is not fixed - a meter +arrival, or `cycle_floor_s` elapsing, whichever comes first - so N samples is a +different wall-clock window on every path, and the AC is written in minutes. + +The latch is not optional and not a nicety. Commanding 0 W removes the command +side of the comparison (|0| is not > 300), so the disagreement disappears the +instant the trip works. Un-latched, this watchdog would trip, clear itself, and +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. +""" + +from dataclasses import dataclass +from datetime import datetime + + +@dataclass(frozen=True) +class SignWatchdogConfig: + """Defaults, not constants - config.yaml carries both (SAFETY-02 AC 5).""" + + min_w: float = 300.0 + hold_s: float = 180.0 + + +class SignWatchdog: + """One instance per actuator, evaluated once per control cycle. + + ⚠️ NOT SHARED ACROSS CADENCES. `disagree_since` is a wall-clock start time, + so driving one instance from two loops at two rates would have each one + resetting the other's window. If a fast on-device path ever wants this + check, it gets its own instance - the same exactly-one-writer discipline the + arbiter applies to the actuator, applied to the detector's own state. + """ + + def __init__(self, cfg: SignWatchdogConfig | None = None): + self.cfg = cfg or SignWatchdogConfig() + self.disagree_since: datetime | None = None + self.tripped = False + self.reason = "" + + # -- the whole check, and it is a pure function of its arguments ---------- + def evaluate(self, commanded_w, grid_w, following: bool, + now: datetime) -> bool: + """Advance the detector one cycle. Returns True while tripped. + + `commanded_w` the EMITTED setpoint - what is standing on the wire now, + after deadband, slew, quantisation and the arbiter. Not + the optimizer's raw wish: the wire carries the emitted + value, so anything else can disagree with the meter for a + reason that is not a fault. + `grid_w` net P1, + = importing. None when the meter is stale or + missing, which CLEARS the timer rather than accumulating + - a frozen figure held against a moving command would + otherwise manufacture a disagreement in about three + minutes, on top of SAFETY-01 having already commanded + zero for the same cause. + `following` did the grid-following loop actually win this cycle? See + gate 1 in the module docstring. + `now` the caller's clock, so this stays testable and replayable. + """ + if self.tripped: + # Latched. Do not re-evaluate: the trip removed the evidence. + 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: + self.disagree_since = None + return False + + if self.disagree_since is None: + self.disagree_since = now + return False + + held = (now - self.disagree_since).total_seconds() + if held < cfg.hold_s: + return False + + self.tripped = True + self.reason = ( + f"SIGN_DISAGREEMENT: commanded {commanded_w:+.0f} W against a meter " + f"reading {grid_w:+.0f} W for {held:.0f} s continuously " + f"(limits: both over {cfg.min_w:.0f} W, held over {cfg.hold_s:.0f} s)" + ) + return True + + @property + def held_s(self) -> float: + """How long the current disagreement has been standing, for the UI.""" + if self.disagree_since is None: + return 0.0 + return (datetime.now(self.disagree_since.tzinfo) + - self.disagree_since).total_seconds() + + def reset(self) -> None: + self.disagree_since = None + self.tripped = False + self.reason = "" diff --git a/goodwe_controller/config.yaml b/goodwe_controller/config.yaml index a30ac79..5ff30a0 100644 --- a/goodwe_controller/config.yaml +++ b/goodwe_controller/config.yaml @@ -1,5 +1,5 @@ name: GoodWe RS485 Controller -version: "0.4.0" +version: "0.5.0" slug: goodwe_controller description: >- Drives a GoodWe ES/BP battery inverter over RS485 by emulating its smart @@ -93,6 +93,27 @@ options: cycle_floor_s: 15 auto_start: false + # --- SAFETY-02 sign-disagreement watchdog ---------------------------------- + # Trips when the emitted setpoint and the P1 net figure point OPPOSITE ways, + # both over sign_disagree_w, continuously for sign_disagree_s. Action: 0 W and + # a failsafe that latches until an operator starts control again. + # + # 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 % + # 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 + # without a capture that says so. They are options because the AC says + # thresholds are defaults rather than constants - not because they are knobs + # anyone is expected to turn. + # + # ⚠️ sign_disagree_w must stay BELOW max_w or the watchdog can never fire (the + # arbiter bounds the command to +-max_w). validate.py refuses that config. + sign_disagree_w: 300 + sign_disagree_s: 180 + # --- maintenance ----------------------------------------------------------- maintenance_enabled: false maintenance_interval_days: 28 @@ -155,6 +176,11 @@ schema: cycle_floor_s: int(5,60) auto_start: bool + # No "off" value, deliberately: this is a P0 safety check and the schema is + # not the place to make disarming it a supported configuration. + sign_disagree_w: int(50,5000) + sign_disagree_s: int(30,1800) + maintenance_enabled: bool maintenance_interval_days: int(1,90) maintenance_start_hour: int(0,23) diff --git a/goodwe_controller/test_validate.py b/goodwe_controller/test_validate.py index 2c1848d..a46de58 100644 --- a/goodwe_controller/test_validate.py +++ b/goodwe_controller/test_validate.py @@ -160,8 +160,17 @@ for bad in (0, -1, -2000): check(f"max_w {bad} is refused", len(p) == 1 and why(p, "must be positive")) check("...and the message explains it would read as a trip, not a mistake", why(validate_startup_config(base(max_w=0), -5000, 5000), "contradiction")) +# ⚠️ SAFETY-02 changed what this case has to say, and the change is real rather +# than bookkeeping: with a 1 W ceiling the default 300 W sign-disagreement +# threshold is unreachable, so rule 5 refuses - correctly, because the watchdog +# would report itself armed while being incapable of firing. What this check is +# FOR is "a positive max_w is not itself a refusal", so it now names a sign +# threshold the ceiling can actually reach, and the interaction gets its own +# check below. check("the smallest sane max_w still starts", - validate_startup_config(base(max_w=1), -5000, 5000) == []) + validate_startup_config(base(max_w=1, sign_disagree_w=0.5), -5000, 5000) == []) +check("...but that same 1 W ceiling with the DEFAULT sign threshold is refused", + why(validate_startup_config(base(max_w=1), -5000, 5000), "never fire")) # --------------------------------------------------------------------------- # print("what must NOT be refused - a validator that fires on trivia gets deleted") diff --git a/goodwe_controller/test_watchdog.py b/goodwe_controller/test_watchdog.py new file mode 100644 index 0000000..a55b944 --- /dev/null +++ b/goodwe_controller/test_watchdog.py @@ -0,0 +1,248 @@ +"""Runnable check for the sign-disagreement watchdog. `python3 test_watchdog.py` + +Every case here is either a fault this must catch or a piece of NORMAL behaviour +it must not mistake for one. The second list is the longer one on purpose: an +alarm that fires on ordinary operation is worse than no alarm, because the next +person switches it off. + +The headline validation is not here - it is `sim/replay_sign.py` in the infra +repo, which drives this module over the two captured bus days. This file is the +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 + +fails = [] +T0 = datetime(2026, 8, 20, 15, 0, 0, tzinfo=timezone.utc) + + +def check(name, cond): + print(f" {'ok ' if cond else 'FAIL'} {name}") + if not cond: + fails.append(name) + + +def drive(samples, cfg=None, following=True): + """Feed (offset_s, commanded_w, grid_w) tuples. Returns (wd, trip_offset).""" + wd = SignWatchdog(cfg) + trip_at = None + for item in samples: + offset, cmd, grid = item[0], item[1], item[2] + f = item[3] if len(item) > 3 else following + if wd.evaluate(cmd, grid, f, T0 + timedelta(seconds=offset)) and trip_at is None: + trip_at = offset + return wd, trip_at + + +def opposed(duration_s, step=5, cmd=-1000, grid=1500, following=True, start=0): + return [(start + t, cmd, grid, following) + for t in range(0, int(duration_s) + 1, step)] + + +print("the fault it exists to catch") +wd, at = drive(opposed(300)) +check("charging into an importing house trips", wd.tripped) +check("and trips at 180 s, not before", at == 180) +check("the reason names both signals", "-1000" in wd.reason and "+1500" in wd.reason) +print(f" e.g. {wd.reason!r}") + +wd, at = drive(opposed(300, cmd=2000, grid=-2000)) +check("discharging into an exporting house trips too", wd.tripped and at == 180) + +print() +print("the boundaries") +wd, _ = drive(opposed(600, cmd=-300, grid=1500)) +check("300 W exactly does NOT qualify (strictly greater)", not wd.tripped) +wd, _ = drive(opposed(600, cmd=-301, grid=1500)) +check("301 W does", wd.tripped) +wd, _ = drive(opposed(600, cmd=-1000, grid=300)) +check("a 300 W meter does not qualify either", not wd.tripped) +wd, _ = drive(opposed(175)) +check("175 s of disagreement does not trip", not wd.tripped) +wd, _ = drive(opposed(180)) +check("180 s does (>= hold_s)", wd.tripped) + +print() +print("continuity - the rule that produced zero false positives on real data") +broken = opposed(175) + [(180, -1000, -1500)] + opposed(175, start=185) +wd, _ = drive(broken) +check("179 s, one agreeing sample, then 175 s more: no trip", not wd.tripped) +check("and the window really was reset", wd.disagree_since is not None) +long_enough = broken + opposed(200, start=365) +wd, _ = drive(long_enough) +check("but a full 180 s after the reset does trip", wd.tripped) + +print() +print("things that are NOT faults") +# The load-step transient. Measured on the healthy capture: longest 30 s. +wd, _ = drive(opposed(30) + [(35 + t, 800, 900) for t in range(0, 300, 5)]) +check("a 30 s load-step crossover does not trip", not wd.tripped) + +# SoC 10 %, Standby, declining a 1350 W discharge. The battery does nothing, so +# the meter stays where it was and the command agrees with it. +wd, _ = drive([(t, 1350, 1350) for t in range(0, 900, 5)]) +check("a battery refusing a discharge command cannot trip it", not wd.tripped) + +# A frozen meter drives command and reading into agreement, not disagreement. +wd, _ = drive([(t, 800, 800) for t in range(0, 900, 5)]) +check("a frozen meter cannot trip it", not wd.tripped) + +# Maintenance grid charge: 2500 W of charge against an importing house, for +# hours. Opposed, both huge - and entirely correct. +wd, _ = drive(opposed(7200, cmd=-2500, grid=3000, following=False)) +check("two hours of maintenance grid charge does not trip", not wd.tripped) +check("and the timer never even started", wd.disagree_since is None) + +wd, _ = drive([(t, -1000, None) for t in range(0, 900, 5)]) +check("a missing meter reading does not trip", not wd.tripped) +wd, _ = drive(opposed(175) + [(180, -1000, None)] + opposed(600, start=185)) +check("a missing reading mid-window RESETS it rather than accumulating", + wd.tripped and wd.disagree_since is not None) +wd, at = drive(opposed(175) + [(180, -1000, None)] + opposed(600, start=185)) +check("...so the trip lands 180 s after the gap, not after 180 s total", + at == 365) + +wd, _ = drive([(t, 0.0, 3000) for t in range(0, 900, 5)]) +check("a 0 W command is not a wrong-direction command", not wd.tripped) + +print() +print("the latch") +wd, _ = drive(opposed(200)) +check("tripped after the fault", wd.tripped) +# The trip commands 0 W, which removes the command side of the comparison. +for t in range(200, 900, 5): + wd.evaluate(0.0, 3000, True, T0 + timedelta(seconds=t)) +check("stays tripped once the command is zeroed", wd.tripped) +for t in range(900, 1500, 5): + wd.evaluate(1000, 1000, True, T0 + timedelta(seconds=t)) +check("stays tripped even once everything looks healthy again", wd.tripped) +wd.reset() +check("reset() is the only way out", not wd.tripped and wd.disagree_since is None) +check("and the reason is cleared with it", wd.reason == "") + +print() +print("the thresholds are configuration, not constants") +cfg = SignWatchdogConfig(min_w=800.0, hold_s=60.0) +wd, at = drive(opposed(300, cmd=-1000, grid=1500), cfg=cfg) +check("a 60 s hold trips at 60 s", wd.tripped and at == 60) +wd, _ = drive(opposed(600, cmd=-700, grid=1500), cfg=cfg) +check("an 800 W gate ignores a 700 W command", not wd.tripped) + +print() +print("the configuration that would disarm it is refused") +from app.validate import validate_startup_config # noqa: E402 + +base = {"setpoint_entity": "number.sp", "meter_source": "off", + "meter_phases": 1, "meter_max_age_s": 30} +p = validate_startup_config({**base, "max_w": 2000, "sign_disagree_w": 300}, + -5000, 5000) +check("300 W under a 2000 W ceiling is accepted", p == []) +p = validate_startup_config({**base, "max_w": 2000, "sign_disagree_w": 2000}, + -5000, 5000) +check("a threshold AT max_w is refused", len(p) == 1 and "never fire" in p[0]) +p = validate_startup_config({**base, "max_w": 500, "sign_disagree_w": 3000}, + -5000, 5000) +check("a threshold above max_w is refused", len(p) == 1 and "never fire" in p[0]) +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("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 - +# it is the minimal faithful model of what this watchdog is for. The check +# compares the loop's OUTPUT DIRECTION against the INPUT STATE it was regulating, +# so what it detects is a loop whose two ends have become decoupled in sign: a +# gain, a claim or a strategy with its polarity inverted, or an integrator so far +# wound up that unwinding takes minutes. (Illusmart's was the last one: it +# reached -14 768 W against a +-5000 W rail while writing once every 20-60 s, and +# that arithmetic alone is 5-15 minutes of wrong-direction command.) +# +# `gain` is float(0.05,1.0) in the schema, so this is not a reachable +# configuration - it is a stand-in for a code regression, which is the case a +# unit test can actually hold the line on. +import asyncio # noqa: E402 + +from app.main import Controller # noqa: E402 + + +class _Hass: + def __init__(self): + self.writes = [] + + async def limits(self, entity): + return (-5000.0, 5000.0, 1.0) + + async def number(self, entity, invert=False): + # A house importing 2000 W, steadily, whatever the battery does. + return {"sensor.grid": 2000.0, "sensor.soc": 50.0, + "sensor.batt": 0.0}.get(entity) + + async def set_number(self, entity, value): + self.writes.append(float(value)) + return True + + +class _S: + data = {} + + def set(self, *a): + pass + + def get_time(self, *a): + return None + + +class _P: + enabled = False + + def publish(self, *a, **k): + pass + + def close(self): + pass + + +async def _e2e(): + 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, _S(), _P(), stop=asyncio.Event()) + c.auto = True + c.dev_min, c.dev_max = -5000.0, 5000.0 + seen_wrong_way = False + for _ in range(12): + await c.read_inputs() + await c.cycle() + if c.target < -300 and not c.sign_wd.tripped: + seen_wrong_way = True + await asyncio.sleep(0.25) + return c, hass, seen_wrong_way + + +ctrl, hass, wrong_way = asyncio.run(_e2e()) +check("the broken loop really did command charge into an import first", wrong_way) +check("the watchdog tripped", ctrl.sign_wd.tripped) +check("the commanded value is 0 W", ctrl.target == 0.0) +check("0 W actually reached the device", 0.0 in hass.writes) +check("and it got there through the failsafe claim, not by accident", + "failsafe" in ctrl.reason) +print(f" status line: {ctrl.reason!r}") +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) + +print() +if fails: + print(f"{len(fails)} FAILED: {', '.join(fails)}") + sys.exit(1) +print("all checks passed")