diff --git a/goodwe_controller/app/clamp.py b/goodwe_controller/app/clamp.py index 0cc4af8..b20e3e6 100644 --- a/goodwe_controller/app/clamp.py +++ b/goodwe_controller/app/clamp.py @@ -157,10 +157,20 @@ REASON_CODE = "OUTPUT_CLAMP" # and at a 30 s re-assert the owner received a persistent notification about it # all day. # -# ⚠️ THE FIGURE ABOVE IS AN ENGAGEMENT COUNT READ OFF THE LIVE LOG, NOT A -# MEASUREMENT IN THIS REPO, and it is not the alarm count. Do not compare it -# with the 2-in-60 above without noting that one is engagements and the other is -# engagements too, but over a runaway rather than ordinary running. +# ⚠️ BOTH FIGURES ARE ENGAGEMENT COUNTS; only the regime differs - 2-in-60 was +# measured over a §10.3 RUNAWAY, 48-in-305 was ordinary running on the live +# install. The 48 is read off the live log, not measured in this repo. +# +# ⚠️ AND RAISING THIS CONSTANT ALONE DID NOT FIX THAT FLOOD - review found it +# and the first version of this change was wrong. At ~5 s cycles a 1-in-6 +# engagement leaves ~25 s of clean cycles between engagements, past +# CLEAR_CYCLES x 5 s = 15 s, so the alarm DE-ASSERTED between almost every one +# and came back as a fresh rising edge. The rising edge used to fire +# unconditionally, so those notifications never consulted this constant at all: +# 48 notifications at 30 s, and still 48 at 150 s. The fix was to rate-limit the +# RISING EDGE too and let `last_alarm` survive a de-assert, so this interval +# bounds the notification rate across episodes rather than only within one. +# Measured on that same window afterwards: 51 episodes, 11 notifications. # # WHY the loop reaches its integrator bound so much more often than the guard's # author saw: `max_w` on the live install is 5000 W, while the loop's slew was @@ -320,8 +330,36 @@ class ClampWatchdog: self.active = True self.since = now self.episodes += 1 - self.last_alarm = now - return True + # ⚠️ SAFETY-41. THE RISING EDGE IS RATE-LIMITED TOO, and it did + # not used to be. It fired unconditionally, so a condition that + # engages and clears repeatedly raised one notification PER + # EPISODE no matter how fast the episodes came - and + # `REASSERT_S` never entered into it, because it only governs + # the still-engaged path below. + # + # That is the shape the live install actually produced: 48 + # engagements in 25 minutes, scattered roughly one cycle in six. + # At a ~5 s cycle the gaps are ~25 s of clean cycles, which + # exceeds CLEAR_CYCLES x 5 s = 15 s, so the alarm DE-ASSERTED + # between almost every engagement and each one came back as a + # fresh rising edge. Measured on this class over that window: + # 48 episodes and 48 notifications at REASSERT_S = 30 - and + # still 48 at REASSERT_S = 150. Raising the re-assert removed + # NONE of them, because none of them came from the re-assert. + # + # So the interval now bounds the NOTIFICATION RATE rather than + # the repeat rate within one episode: `last_alarm` survives a + # de-assert, and a new episode speaks only if the interval has + # elapsed. ⚠️ NOTHING IS DROPPED. Every engagement is still + # counted, every episode still increments, the action-log row is + # still written on every engaged cycle, and a genuine runaway - + # which by definition keeps engaging - is picked up by the + # re-assert path within REASSERT_S. Delay, not loss. + if (self.last_alarm is None + or (now - self.last_alarm).total_seconds() >= REASSERT_S): + self.last_alarm = now + return True + return False # Still engaged: re-assert on the slow cadence, not every cycle. if (self.last_alarm is None or (now - self.last_alarm).total_seconds() >= REASSERT_S): @@ -335,7 +373,11 @@ class ClampWatchdog: if self.clean_cycles >= CLEAR_CYCLES: self.active = False self.since = None - self.last_alarm = None + # ⚠️ `last_alarm` DELIBERATELY SURVIVES THE DE-ASSERT. Clearing it + # here is what made the next rising edge speak immediately however + # soon it arrived, which is the flood described at the rising edge + # above. The episode ends; the notification budget does not reset + # with it. self.reason = "" self.clean_cycles = 0 return False diff --git a/goodwe_controller/test_clamp.py b/goodwe_controller/test_clamp.py index 49f0022..8caf7eb 100644 --- a/goodwe_controller/test_clamp.py +++ b/goodwe_controller/test_clamp.py @@ -259,6 +259,51 @@ check(f"...and on no more than {MAX_FRACTION:.0%} of cycles (it was " check("...while every single cycle is still counted", pinned.engagements == int(QUARTER_S)) +# --- THE SCATTERED REGIME, WHICH IS THE ONE THE OWNER ACTUALLY HIT ----------- +# ⚠️ REVIEW-FOUND, SAFETY-41, and it falsified the first version of this card. +# The rising edge used to fire UNCONDITIONALLY, so REASSERT_S governed only the +# still-engaged path. A condition that engages and clears repeatedly therefore +# raised one notification PER EPISODE at any value of the constant. +# +# That is what the live install produced: 48 engagements in ~25 minutes, +# scattered about one cycle in six. At the measured ~5 s cycle the gaps are ~25 s +# of clean cycles, past CLEAR_CYCLES x 5 s = 15 s, so the alarm de-asserted +# between almost every engagement. Raising REASSERT_S 30 -> 150 removed NONE of +# them. The interval now bounds the notification RATE across episodes. +# +# The stimulus is written in wall-clock seconds and in the shape the field +# actually produced, so it does not move when the constant does. +SCATTER_CYCLE_S = 5.0 +SCATTER_ONE_IN = 6 +SCATTER_CYCLES = 305 + +scat = ClampWatchdog() +scat_alarms = sum(1 for i in range(SCATTER_CYCLES) + if scat.note(i % SCATTER_ONE_IN == 0, "scattered", + T0 + timedelta(seconds=SCATTER_CYCLE_S * i))) +_scat_span = SCATTER_CYCLE_S * SCATTER_CYCLES +print(f" scattered 1-in-{SCATTER_ONE_IN} over {_scat_span:.0f} s at " + f"{SCATTER_CYCLE_S:.0f} s cycles: {scat.episodes} episodes, " + f"{scat.engagements} engagements, {scat_alarms} alarm(s)") +check(f"a condition that de-asserts between engagements is rate-limited ACROSS " + f"episodes, not once per episode ({scat_alarms} alarms for " + f"{scat.episodes} episodes)", + scat_alarms < scat.episodes) +check(f"...and the rate honours the same interval the pinned case does: at most " + f"ceil({_scat_span:.0f}/{REASSERT_S:.0f}) + 1 = " + f"{int(_scat_span // REASSERT_S) + 1}", + scat_alarms <= int(_scat_span // REASSERT_S) + 1) +check("...while EVERY episode and EVERY engagement is still counted - the " + "budget is on notifications, never on evidence", + scat.episodes == scat.engagements == SCATTER_CYCLES // SCATTER_ONE_IN + 1) +# ⚠️ THE COUNTERFACTUAL. A runaway that begins from quiet must NOT be delayed by +# a budget spent on an earlier, unrelated episode - so a cold watchdog speaks at +# once, and this is the half that would go red if the rate limit were applied +# without regard to how long it has been silent. +check("...and a rising edge from a COLD watchdog still fires immediately, so a " + "runaway beginning from quiet is never held back", + ClampWatchdog().note(True, "cold runaway", T0) is True) + # --- THE HYSTERESIS, PINNED AGAINST THE CHATTER DIRECTION -------------------- # ⚠️ REVIEW-FOUND. This used to issue `range(CLEAR_CYCLES - 1)` clean cycles, # which at CLEAR_CYCLES = 1 issues ZERO of them and passes while printing