SAFETY-34: re-assert latched alarms after a Core-only restart (#128)

goodwe_controller/app/main.py

@@ -113,6 +113,12 @@ __all__ = ["UNIT_A", "Controller", "amain", "main"]
 # the re-issue happens by itself) than hold the control loop for ten seconds.
 WRITE_BUDGET_S = 4.0
 
+# SAFETY-34: a Core-only restart wipes persistent_notification and does NOT
+# restart this add-on (measured on TEST-23), so a latched alarm is re-asserted
+# this often. Why: goodwe docs/DECISIONS.md, SAFETY-34.
+REANNOUNCE_S = 300.0
+_mono = time.monotonic          # patched by the checks; asyncio keeps the real one
+
 # --------------------------------------------------------------------------- #
 # OPT-04: THE BACKGROUND TARIFF REFRESH, AND WHY IT IS NOT OPTIONAL
 #
@@ -691,6 +697,9 @@ class Controller:
         # does, so the latch alone is not the alarm - re-raising it is.
         # `run_control()` does that, once, before the first cycle.
         self._circ_announced = False
+        # SAFETY-34: when each latched alarm was last put in front of HA, by
+        # notification_id - see REANNOUNCE_S.
+        self._announced_mono = {}
         if self.circ_wd.tripped:
             _LOG.error(
                 "SAFETY-31 was TRIPPED before this restart and stays tripped: "
@@ -3943,15 +3952,17 @@ class Controller:
         The latch says "this site is tripped"; the flag says "this process has
         put that in front of a human". A restart clears the flag and keeps the
         latch, so the notification is RE-RAISED on the first cycle after every
-        add-on start - which is also the first cycle after a Home Assistant
-        restart, because the add-on is restarted with it.
+        add-on start. A Core-only restart does NOT restart the add-on, so it is
+        also re-asserted every REANNOUNCE_S (SAFETY-34).
 
         ⚠️ ONE CODE PATH FOR BOTH, ON PURPOSE. A fresh trip and a restart into
         an existing trip raise the alarm through these same lines. Two paths
         would be two chances for the restart one to rot unnoticed, and the
         restart one is the case nobody exercises by hand.
         """
-        if not self.circ_wd.tripped or self._circ_announced:
+        if not self.circ_wd.tripped:
+            return
+        if self._circ_announced and not self._reannounce_due("goodwe_circulating_power"):
             return
         # Set BEFORE the await. `hass.call()` can raise or hang, and a retry
         # every cycle for a broker that is down would be an alarm storm on the
@@ -3959,6 +3970,7 @@ class Controller:
         # already durable and the log line above already fired; this surface is
         # best-effort by design.
         self._circ_announced = True
+        self._announced_mono["goodwe_circulating_power"] = _mono()
         names = self.units
         pos, neg = opposing([inv.batt for inv in self.inverters],
                             min_w=self.circ_wd.cfg.min_w / 2.0)
@@ -3993,6 +4005,11 @@ class Controller:
                 f"know why two inverters were opposing."),
         })
 
+    def _reannounce_due(self, notification_id: str) -> bool:
+        """SAFETY-34: has this latched alarm gone REANNOUNCE_S without being re-asserted?"""
+        t = self._announced_mono.get(notification_id)
+        return t is None or _mono() - t >= REANNOUNCE_S
+
     async def check_not_delivering(self) -> None:
         """SAFETY-42 + SAFETY-34. The alarm, and it must survive a restart.
 
@@ -4012,8 +4029,8 @@ class Controller:
         not persisted. The latch says "this unit is tripped"; membership says
         "this process has put that in front of a human". A restart empties the
         set and keeps the latch, so the notification is RE-RAISED on the first
-        cycle after every add-on start - which is also the first cycle after a
-        Home Assistant restart, because the add-on is restarted with it.
+        cycle after every add-on start, and re-asserted every REANNOUNCE_S
+        because a Core-only restart does not restart the add-on (SAFETY-34).
 
         ⚠️ ONE CODE PATH FOR BOTH, ON PURPOSE - `check_circulating()`'s
         argument. A fresh trip and a restart into an existing trip raise the
@@ -4031,7 +4048,10 @@ class Controller:
         names = self.units
         slugs = unit_slugs(names)
         for inv, slug in zip(self.inverters, slugs):
-            if not inv.delivery_wd.tripped or inv.name in self._delivery_announced:
+            nid = f"goodwe_not_delivering_{slug}"
+            if not inv.delivery_wd.tripped:
+                continue
+            if inv.name in self._delivery_announced and not self._reannounce_due(nid):
                 continue
             # Added BEFORE the await, for `check_circulating()`'s reason:
             # `hass.call()` can raise or hang, and a retry every cycle for a
@@ -4039,9 +4059,10 @@ class Controller:
             # latch is already durable and the log line already fired; this
             # surface is best-effort by design.
             self._delivery_announced.add(inv.name)
+            self._announced_mono[nid] = _mono()
             peers = ", ".join(n for n in names if n != inv.name) or "no sibling"
             await self.hass.call("persistent_notification", "create", {
-                "notification_id": f"goodwe_not_delivering_{slug}",
+                "notification_id": nid,
                 "title": f"GoodWe: {inv.name} is not delivering its share",
                 "message": (
                     f"{inv.delivery_wd.reason}\n\nWHY NOTHING ELSE SAID SO. A "

goodwe_controller/test_delivery.py

@@ -1195,8 +1195,8 @@ async def _restart():
     c1 = _ctrl(h1, store=shared)
     await _drive(c1)
     # A NEW process, a NEW HomeAssistant object (so the in-memory notification
-    # is gone with it), the SAME durable file. That is what a Home Assistant
-    # restart is from this add-on's point of view: the add-on goes with it.
+    # is gone with it), the SAME durable file: an ADD-ON restart. A Core-only
+    # restart keeps the process; graded in part 8b (SAFETY-34).
     h2 = _DeliveryHass(b_soc=50.0)
     c2 = _ctrl(h2, store=shared)
     await c2.check_not_delivering()
@@ -1250,6 +1250,40 @@ check("the operator's START press is the only way out, and it clears the "
       and _rs.data.get(unit_key("inverter-b", "delivery_reason")) == ""
       and _r2._delivery_announced == set())
 
+print()
+print("8b. SAFETY-34: a Core-only restart - same add-on process, HA's memory wiped")
+import app.main as _main_mod  # noqa: E402
+
+
+async def _core_restart():
+    shared = _MemStore()
+    c = _ctrl(_DeliveryHass(b_soc=50.0), store=shared)
+    await _drive(c)
+    return c
+
+
+_c = asyncio.run(_core_restart())
+_real_mono, _clock = _main_mod._mono, [_main_mod._mono()]
+_main_mod._mono = lambda: _clock[0]
+try:
+    _c.hass = _DeliveryHass(b_soc=50.0)
+    asyncio.run(_c.check_not_delivering())
+    _early = [x for x in _c.hass.calls
+              if "not_delivering" in str(x[2].get("notification_id", ""))]
+    _clock[0] += _main_mod.REANNOUNCE_S
+    asyncio.run(_c.check_not_delivering())
+    _late = [x for x in _c.hass.calls
+             if "not_delivering" in str(x[2].get("notification_id", ""))]
+finally:
+    _main_mod._mono = _real_mono
+check("SAFETY-34: the latch tripped in this process",
+      _c.inverters[1].delivery_wd.tripped)
+check("SAFETY-34: inside REANNOUNCE_S the same process does not re-raise",
+      len(_early) == 0)
+check("SAFETY-34: after REANNOUNCE_S the SAME process re-asserts the card, "
+      "because a Core-only restart wiped it and did not restart the add-on",
+      len(_late) == 1 and "inverter_b" in _late[0][2]["notification_id"])
+
 
 print()
 if fails:

goodwe_controller/test_watchdog.py

@@ -1055,8 +1055,8 @@ async def _circ_restart():
         await a.read_inputs()
         await a.cycle()
         await asyncio.sleep(0.25)
-    # THE RESTART. Same store, brand new Controller and Hass - which is exactly
-    # what a Home Assistant restart does to this add-on. The house has stopped
+    # THE RESTART. Same store, brand new Controller and Hass - an ADD-ON
+    # restart. A Core-only restart keeps the process; graded below (SAFETY-34). The house has stopped
     # circulating in the meantime, so nothing in the new process's own
     # measurements could re-derive the trip.
     h2 = _CircHass(a_w=0.0, b_w=0.0)
@@ -1116,6 +1116,28 @@ check("...and it does not raise the same card on every cycle - a per-PROCESS "
       "this' a different question from 'is this site tripped'",
       len(_notes2) == 1)
 
+# SAFETY-34: a Core-ONLY restart. The add-on process lives on (measured on
+# TEST-23), HA's memory is wiped, so the SAME process must re-assert the card.
+import app.main as _main_mod  # noqa: E402
+_real_mono, _clock = _main_mod._mono, [_main_mod._mono()]
+_main_mod._mono = lambda: _clock[0]
+try:
+    _h3 = _CircHass(a_w=0.0, b_w=0.0)
+    _b.hass = _h3
+    asyncio.run(_b.check_circulating())
+    _early = [c for c in _h3.calls if c[0] == "persistent_notification"]
+    _clock[0] += _main_mod.REANNOUNCE_S
+    asyncio.run(_b.check_circulating())
+    _late = [c for c in _h3.calls if c[0] == "persistent_notification"]
+finally:
+    _main_mod._mono = _real_mono
+check("SAFETY-34: inside REANNOUNCE_S the same process does not re-raise - no "
+      "storm on the event bus", len(_early) == 0)
+check("SAFETY-34: a Core-only restart wipes the card and the add-on process "
+      "survives it, so after REANNOUNCE_S the SAME process re-asserts it",
+      len(_late) == 1
+      and _late[0][2]["notification_id"] == "goodwe_circulating_power")
+
 
 print()
 print("6a. AC 4: THE ALARM NAMES WHICH PACKS, AND WHICH WAY ROUND")