SAFETY-81 review: 10 s hold, ERROR line per episode, more scenarios

- The alarm must hold for peakguard.ALARM_HOLD_S (10 s) on the projection's
  clock. A load that tops cap + battery for ~5 s every 30 s alarmed at every
  quarter start while the quarter landed under the aim (sine4500_fast).
- A cycle with no battery reading leaves the hold alone.
- Notified once per quarter, but the ERROR line is written per episode.
- New sim scenarios (sine4500_fast, twice7000, step6500_gap) with 5a4cdc2
  digests; control still byte-identical. Unit checks for export and the
  hold across a missing reading. test_peakguard's saturated fixture moves
  the hold start back instead of waiting it out.
- DECISIONS and DOCS updated: costs of the hold, tolerance reasoning, the
  double count in control left to a follow-up.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

goodwe_controller/DECISIONS/SAFETY-81-load-too-high.md

@@ -1,6 +1,7 @@
 # SAFETY-81 — no false "Load too high for battery" alarms
 
-**Status: 2026-09-29, owner go for the card. The 10 W tolerance is this PR's choice, for review.**
+**Status: 2026-09-29, owner go for the card. Review kept the 10 W tolerance
+as a constant and added the 10 s hold.**
 
 ## Defect
 
@@ -23,20 +24,31 @@ The alarm needs `saturated` and `peakguard.load_too_high()`: with the battery
 at full power from now on, the quarter still ends more than `ALARM_MISS_W`
 (10 W) over the real limit (`ceiling_w`). That is the miss itself, without the
 gain. The battery reading it uses is the lower of this cycle's and the last
-one's. At most one alarm per quarter, because near the threshold the test
+one's. It must hold for `ALARM_HOLD_S` (10 s). At most one notification per
-flickers with the meter.
+quarter, because near the threshold the test flickers with the meter; the
+ERROR log line is written for every episode.
 
 **Why 10 W:** at margin 0 the guard itself lands up to +5.7 W over (SAFETY-79
 test, check 5), and those misses are real but tiny; the alarm would fire in
 the last second of most guarded quarters. The margin (default 50) is the fix
-for them. Cost: a real miss of 10 W or less is not alarmed.
+for them. Cost: a real miss of 10 W or less is not alarmed; at a capacity
+tariff of the order of EUR 50 per kW a year, 10 W in one month is cents.
+Review measured +13 W on a 10 kW two-unit site at margin 0, not saturated, so
+no alarm: the tolerance only matters when the battery is at its rating. A
+constant, not an option: `peak_margin_w` is the owner's knob.
 
-**Cost:** a load that climbs slowly through the band between the aim and the
+**Why the hold (review):** the test assumes the load now lasts the rest of
-cap alarms when the quarter will end over the cap, not at the aim (10 s later
+the quarter. A load over cap + battery for ~5 s every 30 s (`sine4500_fast`,
-in `ramp6500`). A step overload alarms in the same cycle as before; a real
+a cycling hob or heater) alarmed at each quarter start while the quarter
-overload shows on the meter before the battery moves, so the lower reading
+landed under the aim. The hold stops that.
-costs it nothing. A quarter already over that the battery cannot win back
+
-still alarms (test_peakguard.py's saturated fixture).
+**Cost:** a real alarm comes 10-12 s later than on 5a4cdc2 (`step6500` at
+1211 s, not 1201 s); a slow ramp 20 s later (`ramp6500`), as it waits for the
+real cap. While the battery refuses or its reading is lost, the alarm waits
+until the quarter is lost even at full battery (27-60 s later in review runs);
+those faults have their own alarms. A load over cap + battery for more than
+10 s still alarms even if the quarter later lands under. `ALARM_HOLD_S = 0`
+removes the hold.
 
 ## Not changed: the 1 s spike itself
 
@@ -45,3 +57,7 @@ sample still sees 0 s left, files no claim, and an Off battery drops to 0 W for
 one cycle. That is the same edge tick as OPT-15 F1, so it is left to OPT-15.
 Control is unchanged here: every command in `tools/safety81_sim.py` is
 byte-identical to 5a4cdc2 (test_safety81.py part 1).
+
+The double count also moves control, not only the alarm: on a late sample the
+guard asks for the full rating for one cycle (edge4600 at 902 s: 2000 W where
+1150 W holds the aim). Left to a follow-up card.

goodwe_controller/DOCS.md

@@ -3212,9 +3212,10 @@ reason code `OUTPUT_CLAMP`.
 **Load too high for battery.** The capacity-peak guard (SAFETY-07) asks for
 everything the inverter can discharge and the running 15-minute average is
 still projected over the peak limit, so the month's billed peak
-(capaciteitstarief) goes up. It fires only when, with the battery at full
+(capaciteitstarief) goes up. It fires only when, for 10 s, the period would
-power, the period will still end more than 10 W over the real limit (not the
+still end more than 10 W over the real limit (not the margin's aim) with the
-margin's aim), and at most once per period (SAFETY-81). In shadow mode the
+battery at full power. It notifies at most once per period; later overloads
+in the same period go to the log (SAFETY-81). In shadow mode the
 guard's projection is made on a plant that did not respond (SAFETY-47), and
 the text says so. See *The capacity-peak guard*.
 

goodwe_controller/app/main.py

@@ -926,6 +926,7 @@ class Controller:
         self._peak_proj = None
         self._peak_batt_prev = None
         self._peak_alarm_block = None
+        self._peak_high_since = None
         self._peak_floor_limited = False
         self._maint_shape_suspended = False
         # OPT-11. Battery modes, off by default so an install that has not
@@ -4825,38 +4826,53 @@ class Controller:
         SAFETY-81: `g.saturated` alone is not enough. It is graded against the
         aim, and a 1 s meter spike, noise or the last seconds of a quarter set
         it while the quarter lands on target; `peakguard.load_too_high()` must
-        agree too. The battery reading it gets is the lower of this cycle's and
+        agree too, for `ALARM_HOLD_S`. The battery reading it gets is the lower
-        the last one's - see there.
+        of this cycle's and the last one's - see there.
 
         Edge-triggered, and de-asserting arms it again - the same shape as
         FreezeWatchdog's `evaluate()`, and for the same reason: one notification
-        per episode, not one per cycle. SAFETY-81: and at most one per quarter.
+        per episode, not one per cycle. SAFETY-81: the notification and action
+        log line at most once per quarter; the ERROR line once per episode.
         """
         g = self.peak_guard
         batt, prev = self.site_batt_w, self._peak_batt_prev
+        if batt is None:
+            # Unknown, not "fine": the hold below neither grows nor restarts.
+            return
         self._peak_batt_prev = batt
-        if batt is not None and prev is not None:
+        if prev is not None:
             batt = min(float(batt), float(prev))
-        saturated = bool(
+        p = self._peak_proj
-            g is not None and g.saturated and self._peak_proj is not None
+        high = bool(
+            g is not None and g.saturated and p is not None
+            and p.block_start is not None
             and peakguard.load_too_high(
-                self._peak_proj, batt_w=batt, ceiling_w=g.ceiling_w,
+                p, batt_w=batt, ceiling_w=g.ceiling_w,
                 achievable_w=self.site_rating_w))
+        # SAFETY-81: and for ALARM_HOLD_S, on the projection's clock.
+        saturated = False
+        if high:
+            t = p.block_start + timedelta(seconds=float(p.elapsed_s))
+            if self._peak_high_since is None:
+                self._peak_high_since = t
+            saturated = ((t - self._peak_high_since).total_seconds()
+                         >= peakguard.ALARM_HOLD_S)
+        else:
+            self._peak_high_since = None
         was, self._peak_saturated = self._peak_saturated, saturated
         if not saturated or was:
             return
-        # SAFETY-81: at most once per quarter. Near its threshold the test
-        # above flickers with the meter, and each flicker would re-alarm.
-        if self._peak_proj.block_start == self._peak_alarm_block:
-            return
-        self._peak_alarm_block = self._peak_proj.block_start
-        self.log_event("PEAK_GUARD_SATURATED - " + g.reason,
-                       hint=messages.hint("peak_saturated"))
         _LOG.error(
             "Switch off large loads, the battery alone cannot keep this "
             "15-minute period under the peak limit: %s.",
             messages.reason_code(g.reason))
         _LOG.debug("Evidence: %s.", g.reason)
+        # SAFETY-81: notified once per quarter; a later episode is logged only.
+        if p.block_start == self._peak_alarm_block:
+            return
+        self._peak_alarm_block = p.block_start
+        self.log_event("PEAK_GUARD_SATURATED - " + g.reason,
+                       hint=messages.hint("peak_saturated"))
         # A fixed notification_id, and a DIFFERENT one from every other alarm
         # here - check_meter_frozen() gives the reasoning. A saturated peak guard
         # is not a frozen meter and is not an out-of-range setpoint.

goodwe_controller/app/peakguard.py

@@ -870,6 +870,9 @@ def decide(projection, *, batt_w, cap_w, margin_w, reading, achievable_w, soc,
 # "Load too high for battery" fires. Above the guard's own landing error at
 # margin 0 (up to +5.7 W, SAFETY-79 test check 5), which margin 0 accepts.
 ALARM_MISS_W = 10.0
+# ...and for this long without a break, so a load that peaks for a few seconds
+# (a hob or heater cycling) is not read as the rest of the quarter.
+ALARM_HOLD_S = 10.0
 
 
 def load_too_high(projection, *, batt_w, ceiling_w, achievable_w) -> bool:

goodwe_controller/test_peakguard.py

@@ -1259,6 +1259,13 @@ check(f"the saturating fixture ({NET2:.0f} W) is a load the real ingest "
       f"accepts, not one it rejects as implausible", NET2 < PLAUSIBLE_MAX_W)
 print(f"    saturated case: {c2.peak_guard.explain()}")
 check("a load larger than the inverter saturates the guard", c2.peak_guard.saturated)
+# SAFETY-81 review: the alarm must hold ALARM_HOLD_S first. The fixture runs on
+# the wall clock, so the start of the hold is moved back instead of waited out.
+check("...but not on its first cycle: it must hold peakguard.ALARM_HOLD_S "
+      "(SAFETY-81)", hass2.notifications.count("goodwe_peak_saturated") == 0
+      and c2._peak_high_since is not None)
+c2._peak_high_since -= timedelta(seconds=peakguard.ALARM_HOLD_S)
+asyncio.get_event_loop_policy().new_event_loop().run_until_complete(c2.cycle())
 check("...and THAT raises exactly one notification, with its own id - the one "
       "state where the money will be spent and a human is the only remaining "
       "actuator", hass2.notifications.count("goodwe_peak_saturated") == 1)

goodwe_controller/test_safety81.py

@@ -8,20 +8,24 @@ DECISIONS/SAFETY-81-load-too-high.md has the record.
 1. Every command is byte-identical to release/1.0 5a4cdc2.
 2. No alarm on the quarter-edge spike, the last seconds, noise (margin 50
    and 0) or a burst the quarter absorbs.
-3. Real overloads still alarm, as fast as on 5a4cdc2.
+3. Real overloads still alarm, ALARM_HOLD_S after 5a4cdc2.
 4. peakguard.load_too_high() on its own.
+5. The hold across a cycle with no battery reading (review).
 
 Mutations (MUTATION.md, purged copies) are listed in the pull request.
 """
 
+import asyncio
 import os
 import sys
+from datetime import datetime, timedelta, timezone
 from types import SimpleNamespace
 
 HERE = os.path.dirname(os.path.abspath(__file__))
 sys.path.insert(0, os.path.join(HERE, "tools"))
 
 import safety81_sim as sim                                    # noqa: E402
+from app.main import Controller                               # noqa: E402
 from app.peakguard import load_too_high                       # noqa: E402
 
 fails = []
@@ -48,8 +52,14 @@ BASE = {
     "miss5560": ([1201.0, 1801.0], "cd991cfe1685750598236701a5a2f2f0342d66645609c536d42ab7ba07216e6c"),
     "ramp6500": ([1296.0, 1801.0], "8db7675535f1a01abf38bd6bfb996a6fe57711d3ecca53541bb1c0870748c46b"),
     "step7500_reserve": ([], "21eb9c07a4582f47f347084d1c75a28a5d767bc068bc3f74f32433d3239e9ccb"),
+    # Added in review, printed the same way.
+    "sine4500_fast": (["112, about one per 30 s"], "710008fd8f2d33a0e2cbb9a626bedafd6d87bc23712bbc5b74a4af9b3ec21897"),
+    "twice7000": ([1001.0, 1502.0, 2699.0], "1cddda8cf35859efb2ad503f1683f744cb812a6325a390c4fa3df19d717af043"),
+    "step6500_gap": ([1201.0, 1801.0], "95e5c90e4a6814526cb4f3bea76286063b449d86a420f888c014531fb6e913a3"),
 }
-RUN = {k: sim.run(**v)[:2] for k, v in sim.SCENARIOS.items()}
+OUT = {k: sim.run(**v) for k, v in sim.SCENARIOS.items()}
+RUN = {k: v[:2] for k, v in OUT.items()}
+ERR = {k: v[4] for k, v in OUT.items()}
 
 # ============================================================================ #
 print("1. every command byte-identical to release/1.0 5a4cdc2")
@@ -60,23 +70,33 @@ for k, (alarms, digest) in RUN.items():
 # ============================================================================ #
 print("2. no false alarm (5a4cdc2's are listed)")
 for k in ("edge4600", "edge4600_m0", "const4500", "const4500_m0", "sine4500",
-          "noise5000_m50", "noise5000_m0", "burst_end"):
+          "noise5000_m50", "noise5000_m0", "burst_end", "sine4500_fast"):
     check(f"{k}: none (5a4cdc2: {BASE[k][0]})", RUN[k][0] == [])
 
 # ============================================================================ #
-print("3. real overloads still alarm, as fast as 5a4cdc2")
+print("3. real overloads still alarm, ALARM_HOLD_S after 5a4cdc2")
-for k in ("step6500", "miss5560"):
+# ALARM_HOLD_S (10 s) plus up to 2 s of late meter samples; at 1800 s the
+# quarter-edge tick restarts the hold.
+for k, want in (("step6500", [1211.0, 1813.0]),
+                ("miss5560", [1213.0, 1813.0])):
     print(f"    {k}: load steps up at 1200 s; alarms at {RUN[k][0]}, "
           f"5a4cdc2 {BASE[k][0]}")
-    check(f"{k}: the same second as 5a4cdc2, and again only at the next "
+    check(f"{k}: after the hold, and again only at the next quarter",
-          f"quarter (at most once per quarter)", RUN[k][0] == BASE[k][0])
+          RUN[k][0] == want)
-# By design: until 1306 s this quarter would end at most ALARM_MISS_W over the
+check("step6500_gap: cycles stopped by a lost battery reading do not "
-# real 3500 W cap; 5a4cdc2 alarmed when it crossed the aim (cap - 50).
+      "restart the hold",
+      RUN["step6500_gap"][0] == [1215.0, 1813.0])
+check("twice7000: notified once in the quarter (5a4cdc2: "
+      f"{BASE['twice7000'][0]})", RUN["twice7000"][0] == [1011.0])
+check("twice7000: the ERROR line for both overloads",
+      ERR["twice7000"] == [1011.0, 1512.0])
+# By design: until 1306 s (then the hold) this quarter would end at most
+# ALARM_MISS_W over the real 3500 W cap; 5a4cdc2 alarmed at the aim (cap - 50).
 print(f"    ramp6500: alarms at {RUN['ramp6500'][0]}, "
       f"5a4cdc2 {BASE['ramp6500'][0]}")
 check("ramp6500: alarms once the quarter will end over the real cap by "
       "more than ALARM_MISS_W",
-      RUN["ramp6500"][0] == [1306.0, 1801.0])
+      RUN["ramp6500"][0] == [1316.0, 1813.0])
 check("step7500 under the peak reserve: not this alarm (the guard is not "
       "allowed to discharge)", RUN["step7500_reserve"][0] == [])
 
@@ -109,6 +129,40 @@ check("over cap + battery, but the quarter lands under the cap: no",
 check("...and when it does not: yes", ltl(proj(700, 3450, 4000), 2000))
 check("no battery reading: no", not ltl(proj(300, 3450, 5000), None))
 check("no seconds left: no", not ltl(proj(900, 4000, 5000), 2000))
+check("a quarter already over is not won back by export: yes",
+      ltl(proj(800, 3950, 1000), 0))
+
+# ============================================================================ #
+print("5. the hold across a cycle with no battery reading")
+# A reading the stale detector drops (STALE_READING) reaches the alarm as None
+# while the cycle still runs. Unknown is not "fine": it must not restart the
+# hold. Driven on Controller.check_peak_saturated() itself, with stubs.
+Q0 = datetime(2026, 9, 20, 11, 0, tzinfo=timezone.utc)
+notes = []
+
+
+async def _call(domain, service, data):
+    notes.append(data.get("notification_id"))
+
+
+stub = SimpleNamespace(
+    peak_guard=SimpleNamespace(saturated=True, ceiling_w=3500.0,
+                               reason="PEAK_GUARD: fixture"),
+    site_rating_w=2000.0, shadow=False, events=[],
+    _peak_batt_prev=None, _peak_alarm_block=None, _peak_high_since=None,
+    _peak_saturated=False, hass=SimpleNamespace(call=_call),
+    log_event=lambda msg, hint=None: None)
+first = None
+for sec in range(0, 30):
+    stub.site_batt_w = None if 4 <= sec < 8 else 2000.0
+    p = proj(300 + sec, 3450, 6000)
+    p.block_start = Q0
+    stub._peak_proj = p
+    asyncio.run(Controller.check_peak_saturated(stub))
+    if notes and first is None:
+        first = sec
+check(f"a 4 s gap in the battery reading: alarm at {first} s, not 18 s",
+      first == 10)
 
 print()
 if fails:

goodwe_controller/tools/safety81_sim.py

@@ -18,6 +18,7 @@ changed and what did not.
 
 import asyncio
 import hashlib
+import logging
 import os
 import random
 import sys
@@ -75,8 +76,9 @@ def ramp(before, after, at, secs):
 class _Plant:
     """One battery that reaches its command RESPONSE_S after the write."""
 
-    def __init__(self, load, rating, soc):
+    def __init__(self, load, rating, soc, batt_gap=()):
         self.load, self.rating, self.soc = load, rating, soc
+        self.batt_gap = batt_gap      # (from, to): the battery reading is lost
         self.t = 0.0
         self.hist = [(-1e9, 0.0)]     # (time the command takes effect, W)
         self.notes = []
@@ -106,6 +108,8 @@ class _Plant:
         if e.startswith("sensor.soc"):
             return self.soc
         if e.startswith("sensor.batt"):
+            if self.batt_gap and self.batt_gap[0] <= self.t < self.batt_gap[1]:
+                return None
             return self.batt(self.t)
         return None
 
@@ -115,10 +119,11 @@ class _Plant:
 
 
 def run(load, *, margin=50, dur=2700.0, rating=2000.0, cap=3500.0,
-        late_at=(), soc=60.0):
+        late_at=(), soc=60.0, batt_gap=()):
-    """Alarm times (s from the start of the first quarter) and a digest of
+    """Alarm times (s from the start of the first quarter), a digest of every
-    every cycle's target and written command. A sample is also late at each
+    cycle's target and written command, the Controller, the rows, and the
-    `t % 900` in `late_at`."""
+    times of the ERROR log line. A sample is also late at each `t % 900` in
+    `late_at`."""
     opts = rig._options(1, {
         "meter_source": "ha_signed", "p1_net_entity": "sensor.net",
         "meter_phases": 1, "peak_cap_w": cap, "peak_margin_w": margin,
@@ -129,7 +134,7 @@ def run(load, *, margin=50, dur=2700.0, rating=2000.0, cap=3500.0,
     db = os.path.join(tempfile.mkdtemp(prefix="s81-"), "tariff.db")
 
     async def go():
-        plant = _Plant(load, rating, soc)
+        plant = _Plant(load, rating, soc, batt_gap)
         c = Controller(opts, plant, store, rig._Pub(), stop=asyncio.Event(),
                        peak_db_path=db)
         for i in c.inverters:
@@ -143,6 +148,14 @@ def run(load, *, margin=50, dur=2700.0, rating=2000.0, cap=3500.0,
                 return rig.BASE + timedelta(seconds=clock[0])
         m.datetime = _DT
         rows, late = [], random.Random(8100)
+        errors = []
+
+        class _Err(logging.Handler):
+            def emit(self, r):
+                if "battery alone cannot keep" in r.getMessage():
+                    errors.append(plant.t)
+        h = _Err()
+        logging.getLogger("goodwe").addHandler(h)
         try:
             t = 0.0
             while t < dur:
@@ -161,10 +174,12 @@ def run(load, *, margin=50, dur=2700.0, rating=2000.0, cap=3500.0,
                 t += TICK
         finally:
             m.datetime = real_dt
-        return plant, c, rows
+            logging.getLogger("goodwe").removeHandler(h)
-    plant, c, rows = asyncio.run(go())
+        return plant, c, rows, errors
+    plant, c, rows, errors = asyncio.run(go())
     alarms = [t for t, nid in plant.notes if nid == ALARM]
-    return alarms, hashlib.sha256(repr(rows).encode()).hexdigest(), c, rows
+    return (alarms, hashlib.sha256(repr(rows).encode()).hexdigest(), c, rows,
+            errors)
 
 
 # Three quarters each (2700 s); a new quarter starts at 900 s and 1800 s.
@@ -194,10 +209,21 @@ SCENARIOS = {
     # The pack under its peak reserve (20 %), and a load far over cap +
     # battery: that is "Battery stopped limiting the peak", not this alarm.
     "step7500_reserve": dict(load=step(4500.0, 7500.0, 1200.0), soc=15.0),
+    # Review: a load swinging 4500 +/- 1200 W every 30 s (a hob or heater
+    # cycling). It tops cap + battery for about 5 s a swing, and the quarter
+    # still lands under the aim.
+    "sine4500_fast": dict(load=sine(4500.0, 1200.0, period=30.0, noise=200.0)),
+    # Review: two real overloads in one quarter, 1000-1100 s and 1500-1700 s.
+    "twice7000": dict(load=lambda t: 7000.0 if (1000 <= t < 1100
+                                                or 1500 <= t < 1700)
+                      else 4500.0),
+    # Review: step6500, and the battery reading is lost 1203-1215 s.
+    "step6500_gap": dict(load=step(4500.0, 6500.0, 1200.0),
+                         batt_gap=(1203.0, 1215.0)),
 }
 
 
 if __name__ == "__main__":
     for k, v in SCENARIOS.items():
-        a, d, _, _ = run(**v)
+        a, d, _, _, _ = run(**v)
         print(f"{k:16s} alarms at {a} digest {d}")