SAFETY-12: the loop thinks on a clock, not only when the number moves

run_control() ran cycle() only when the meter value differed from the previous
poll. A meter that repeats a value therefore ran no control cycles at all -
while the heartbeat kept re-emitting the last command and the firmware kept
putting it on the wire every 2 s. Layer 1 stayed fed and every indicator read
healthy, so a command computed from an old reading went on being executed
through whatever the load did next. TEL-05 measured it: 70 s at a literally
constant 350.0 W, 14 arrivals, zero cycles.

cycle_floor_s (default 15 s) is a deadline on the GAP between cycles, reset by
every cycle whichever trigger fired - not a second scheduler. On a healthy meter
it never elapses, so the cycle rate and the commanded sequence are unchanged;
test_loop.py asserts that by driving the real loop twice on one binary (a huge
cycle_floor_s reproduces the old behaviour) and diffing the writes.

15 s is bracketed: above the ~11 s worst gap on a merely-unlucky healthy meter
(~5 s arrivals, 1 s detection, one dropped poll retried on the next tick), below
meter_max_age_s and the firmware's 30 s ingest veto, and equal to stale_input_s.

A floor cycle runs the full law. Freshness is already decided upstream - past
meter_max_age_s the reading is None, which is a P_SAFETY 0 W claim - and the
integrator advances per cycle, not per second, so bounding the gap bounds the
accumulation.

Not a freeze detector: nothing here decides whether a repeated reading is true.
DOCS.md's "accepted deviation" passage is qualified rather than deleted - the
window is now bounded at 3 x cycle_floor_s instead of open-ended, and its
"nothing accumulates while it is stalled" argument is called out as having
covered windup and not staleness. Same correction in control.py's comment.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa
This commit is contained in:
glenn schrooyen
2026-08-28 20:29:16 +02:00
co-authored by Claude Opus 5
parent e2e331ea54
commit 88cd97cc7a
6 changed files with 327 additions and 28 deletions
+56
View File
@@ -67,6 +67,62 @@ A refusal is legible as a refusal: a banner that says in words that this is a
configuration problem and nothing is broken, the numbered problems, no traceback,
and exit status 78 (`EX_CONFIG`).
**SAFETY-12.** The control loop now evaluates on a clock as well as on a change.
`run_control()` ran `cycle()` only when the meter value differed from the
previous poll, so a meter that repeated a value ran **no control cycles at
all** - while the heartbeat went on re-emitting the last command and the
firmware went on putting it on the wire every 2 s. Layer 1 stayed fed, every
indicator read healthy, and a command computed from an old reading kept being
executed through whatever the load did next. Measured on the rig during TEL-05:
70 s at a literally constant 350.0 W with 14 arrivals and zero cycles.
`cycle_floor_s` (default **15 s**) is a deadline on the gap between cycles, not
a second scheduler: every cycle resets it, whichever trigger fired. On a healthy
meter it never elapses, so the cycle rate, the write sequence and therefore the
per-cycle gain the tuning was measured against are unchanged - asserted in
`test_loop.py` by driving the real loop twice on one binary (a huge
`cycle_floor_s` reproduces the old behaviour exactly) and comparing the
commanded values.
15 s is bracketed, not round. Below it: arrivals are ~5 s (NOTES.md measures
4.97 s), detected up to 1 s late by the 1 s poll, and one dropped poll is
retried on the next tick - about 11 s worst case on a meter that is merely
unlucky, which the floor must not fire on. Above it: `meter_max_age_s` is 30 s,
past which there is no reading to evaluate, and the firmware's ingest veto is
also 30 s - so the loop is guaranteed to have re-decided at least twice inside
any window in which layer 1 might otherwise be the only thing deciding. It is
also exactly `stale_input_s`, this system's existing answer to "how long may we
hold without re-deciding".
A floor cycle runs the **full** law on the same inputs, integrator included. The
freshness question is already answered upstream and answered harder - past
`meter_max_age_s` the reading is None, which is "inputs missing", which is a
`P_SAFETY` 0 W claim - so a floor cycle can only ever run on a reading this
system already calls fresh. The integrator advances per *cycle*, not per second,
so bounding the gap bounds the accumulation; `integrator_max_w` bounds it after
that. A gentler second law for floor cycles would be a second set of dynamics
nobody can test on this hardware.
⚠️ **This is not a freeze detector**, and TEL-05 established one cannot be built
here: at the converged -10 W target a 1 Wh register needs ~6 minutes to move
while the power figure legitimately repeats, so any threshold on `unchanged_s`
false-trips at the exact operating point we run at. Nothing in this change
decides whether a repeated reading is *true*. It stops "the number is the same"
from meaning "do not think". The consequence, stated plainly: against a meter
genuinely frozen at a wrong value the loop will now correct toward that wrong
value rather than hold a stale command, bounded by `max_w`, the arbiter and the
device rating. That was already what happened to a lying meter whose value
merely jittered; bit-exact repetition was never a safety property.
`DOCS.md`'s "Saturation is counted in cycles, not seconds" section documented
the old behaviour as an accepted deviation. It is **qualified, not deleted**:
the deviation survives (the window is still counted in cycles, and 3 x 15 s is
still not the specification's 10 s) but it is now bounded at 45 s instead of
open-ended, and the passage's second justification - "nothing accumulates while
it is stalled" - is called out as having covered integrator windup and not
staleness, which was the actual hazard. The same wrong argument in
`control.py`'s saturation comment is corrected too.
**TEL-04.** `meter_source: ha_signed` reads a single *signed* power entity, which
is the shape a HomeWizard P1 publishes. Both transports 0.3.0 shipped require two
unsigned registers and refuse negatives, so neither fits the meter installed here.
+36 -14
View File
@@ -272,9 +272,10 @@ behind it would trip the firmware watchdog on a system that is working fine.
| `target_grid_w` | -10 | What the meter should rest at. Negative = a slight export |
| `step_w` | 10 | Quantisation |
| `saturation_w` | 500 | Divergence that counts as "the inverter is at a limit" |
| `saturation_cycles` | 3 | How many consecutive cycles before freezing. A cycle is one *changed* meter reading, not a fixed period - see the note below. **Do not set to 1** |
| `saturation_cycles` | 3 | How many consecutive cycles before freezing. A cycle is one meter update or `cycle_floor_s`, whichever comes first - see the note below. **Do not set to 1** |
| `integrator_max_w` | 0 | Bound on the loop's accumulator, and 0 means "same as `max_w`". Caps how much stale error can be waiting to unwind when the sign flips. **Do not raise it above `max_w`** - the output clamp already bounds what is commanded, so the only thing extra headroom buys is more cycles of wrong-direction power after every saturation event. Lowering it below `max_w` is the useful direction |
| `heartbeat_s` | 10 | Refresh interval; must stay well under the firmware watchdog |
| `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) |
@@ -284,22 +285,43 @@ The specification states the saturation window as **"> 10 s"**. This add-on coun
**cycles** instead, and that is a deliberate, accepted deviation rather than an
oversight - the acceptance criterion is not met as literally written.
A cycle here is one *changed* meter reading: the controller only runs the loop when the
meter value differs from the previous poll. At the reference P1's ~5 s update rate the
default of 3 cycles is usually around 15 s, but there is **no guaranteed wall-clock
window** - a meter that repeats the same value stalls the counter for as long as it
repeats.
A cycle here is one meter update **or `cycle_floor_s` elapsing, whichever comes
first**. At the reference P1's ~5 s update rate the default of 3 cycles is usually
around 15 s, and the worst case is bounded at `3 x cycle_floor_s` = 45 s. It is still
not the 10 s the specification asks for, so the deviation stands - but it is now a
bounded number rather than an open one.
Two reasons that is acceptable:
⚠️ **This passage used to say something different, and the second half of it was
wrong.** Until SAFETY-12 the loop ran only when the meter value *changed*, so a meter
that repeated a value stalled the counter for as long as it repeated, with no upper
bound at all. That was defended here on the grounds that "a stalled counter is a
detection-latency limit and not a runaway risk - the condition that stalls it stops the
whole loop, so nothing accumulates while it is stalled". That argument covers
**integrator windup and nothing else**. The hazard it misses is **staleness**: while the
loop was stopped the heartbeat kept re-emitting the last command and the firmware kept
putting it on the wire every 2 s, so a command computed from an old reading went on
being executed through a real load change, with every indicator reading healthy.
Measured on the rig during TEL-05: 70 s at a literally constant 350.0 W, 14 arrivals,
zero control cycles.
- the control law is a pure function with no clock, which is what makes it testable
without hardware, and a seconds-based window would have to live in the controller;
- a stalled counter is a detection-latency limit and not a runaway risk. The condition
that stalls it - an unchanging meter - stops the whole loop, so nothing accumulates
while it is stalled.
The half that survives is the first one: the control law is still a pure function with
no clock, and the wall-clock window still lives in `Controller`, which has one - see
`run_control()`. What changed is that the controller now enforces a floor on the gap
between cycles, so `saturation_cycles` finally means a bounded number of seconds.
If a guaranteed window matters on your site, raise `saturation_cycles` for a fast meter,
and treat the figure as "N meter updates" rather than "N seconds".
If a tighter window matters on your site, lower `cycle_floor_s` (not below your meter's
update interval, or the floor starts firing on a healthy meter) or raise
`saturation_cycles` for a fast meter.
**This is not a freeze detector, and it deliberately cannot be one.** Nothing here
decides whether a repeated reading is true — see `unchanged_s` in the meter section for
why no threshold on that is safe at the converged operating point. The floor only stops
"the number is the same" from meaning "do not think". The consequence is worth stating
plainly: against a meter that is genuinely frozen at a wrong value, the loop will now
correct toward that wrong value instead of holding a stale command. It is bounded by
`max_w`, the arbiter and the device rating, and it is the same thing that already
happened to a lying meter whose value merely jittered — bit-exact repetition was never a
safety property.
#### Why `target_grid_w` is not zero
+16 -12
View File
@@ -95,19 +95,23 @@ def compute(
# lets slew be larger than saturation_w.
#
# The spec states this window twice and differently: "> 10 s" (§11.2) and
# "3 samples" (§10.3). This counts CYCLES, and a cycle is not a unit of
# time: run_control() calls cycle() only when the meter value CHANGES
# (`if self.grid != last_grid`), so three cycles is three distinct meter
# readings and nothing more. At the reference P1's ~5 s update rate that is
# usually ~15 s, but there is no upper bound on it - a meter that repeats a
# value stalls the counter.
# "3 samples" (§10.3). This counts CYCLES, and a cycle is still not a unit
# of time - but it is now a BOUNDED one. run_control() runs a cycle on a
# changed meter value OR on `cycle_floor_s` elapsing, whichever comes first
# (SAFETY-12), so three cycles is at most 3 x cycle_floor_s = 45 s at the
# defaults, and usually ~15 s at the reference P1's ~5 s update rate.
#
# That is a detection-latency limit, not a windup hazard: the same
# condition that stalls the counter stalls the whole loop, so nothing
# accumulates in the meantime either. If a wall-clock window is ever
# required, it belongs in Controller (which has a clock) and not here.
# ponytail: this function is worth keeping clockless; the ceiling is that
# saturation_cycles cannot express a guaranteed number of seconds.
# ⚠️ THIS COMMENT USED TO CLAIM there was no upper bound and that it did not
# matter, "because the same condition that stalls the counter stalls the
# whole loop, so nothing accumulates in the meantime". The first clause was
# true and the second was the bug: a stalled loop does not stop commanding,
# it goes on emitting its last command through the heartbeat. Windup was
# never the hazard; staleness was.
#
# The wall-clock window lives in Controller, which has a clock, exactly as
# this comment always said it should. ponytail: this function stays
# clockless; the remaining ceiling is that saturation_cycles expresses a
# bound in seconds rather than an exact number of them.
saturated_now = abs(prev_w - actual_w) > tuning.saturation_w
sat_count = min(sat_count + 1, 10) if saturated_now else 0
frozen = sat_count >= tuning.saturation_cycles
+43 -2
View File
@@ -342,11 +342,52 @@ class Controller:
last_grid = object()
heartbeat = float(self.o.get("heartbeat_s", 10))
last_beat = 0.0
# ⚠️ SAFETY-12. The floor is a DEADLINE ON THE GAP BETWEEN CYCLES, not a
# second scheduler running alongside the arrival trigger. Every cycle
# resets it, whichever trigger fired, so on a healthy changing meter it
# never elapses and not one existing cycle moves - no beat frequency
# between the two triggers, no doubled cycle rate, and therefore no
# change to the per-cycle gain the tuning was measured against.
#
# Why it has to exist: `self.grid != last_grid` alone means a meter that
# repeats a value runs NO control cycles at all, while the heartbeat
# below keeps re-emitting the last command and the firmware keeps
# putting it on the wire - so every indicator reads healthy while the
# loop has not evaluated and the load may have moved underneath it.
# Measured on the rig during TEL-05: 70 s at a literally constant
# 350.0 W, 14 arrivals, zero cycles.
#
# This is NOT a freeze detector, and deliberately so - see
# HomeWizardLocalSource.unchanged_s for why one cannot be built here (at
# the converged -10 W target a 1 Wh register needs ~6 minutes to move,
# so any freeze threshold false-trips at the exact operating point we
# run at). Nothing here decides whether a repeated reading is true. It
# only stops "the number is the same" from meaning "do not think".
floor = float(self.o.get("cycle_floor_s", 15))
last_cycle = None
while not self.stopping:
await self.read_inputs()
# A cycle is one meter update, exactly as on the reference install.
if self.grid != last_grid:
now_mono = asyncio.get_running_loop().time()
# A cycle is one meter update, exactly as on the reference install -
# or the floor elapsing, whichever comes first.
overdue = last_cycle is None or (now_mono - last_cycle) >= floor
if self.grid != last_grid or overdue:
last_grid = self.grid
last_cycle = now_mono
# ⚠️ The FULL cycle, not a degraded one. A floor cycle runs the
# same law on the same inputs as an arrival cycle, because the
# freshness question is already answered upstream and answered
# harder: past `meter_max_age_s` (30 s) p1.net_w is None, which
# is "inputs missing", which becomes a P_SAFETY 0 W claim. So a
# floor cycle can only ever run on a reading this system already
# calls fresh - it cannot run the PI on a 60 s old figure,
# because at 60 s there is no figure. And the integrator
# advances per CYCLE, not per second (control.py), so bounding
# the gap bounds the accumulation: at most one extra
# gain x error per floor interval, under the same
# integrator_max_w as every other cycle. A second, gentler
# control law for floor cycles would be a second set of
# dynamics nobody can test on this hardware.
await self.cycle()
# ⚠️ The heartbeat is not an optimisation. The ESP32 treats silence
# longer than ~30 s as "the controller is gone" and zeroes the
+2
View File
@@ -90,6 +90,7 @@ options:
integrator_max_w: 0
heartbeat_s: 10
stale_input_s: 15
cycle_floor_s: 15
auto_start: false
# --- maintenance -----------------------------------------------------------
@@ -151,6 +152,7 @@ schema:
integrator_max_w: int(0,15000)
heartbeat_s: int(2,25)
stale_input_s: int(5,120)
cycle_floor_s: int(5,60)
auto_start: bool
maintenance_enabled: bool
+174
View File
@@ -0,0 +1,174 @@
"""Runnable check for the control loop's cadence. `python3 test_loop.py`
SAFETY-12: the loop must evaluate on a clock as well as on a change. Before the
fix, `run_control()` ran `cycle()` only when the meter value differed from the
previous poll, so a meter that repeats a value ran NO control cycles while the
heartbeat kept re-emitting the last command - measured on the rig during TEL-05
as 70 s at a literally constant 350.0 W with 14 arrivals and zero cycles.
⚠️ THIS TEST TAKES ~35 SECONDS OF WALL CLOCK, on purpose. The thing under test
is a wall-clock floor inside a loop that sleeps 1 s; mocking the clock would
leave the assertion testing the mock. It drives the real `run_control()` and
counts what actually happened.
`cycle_floor_s` is read from options with no lower bound in code, so a huge
value reproduces the pre-fix behaviour exactly. Every claim below is therefore
an A/B on one binary, not a comparison against a remembered old one.
"""
import asyncio
import sys
import time
from app.main import Controller
fails = []
NEVER = 1e9 # cycle_floor_s so large the floor can never elapse: pre-fix
def check(name, cond):
print(f" {'ok ' if cond else 'FAIL'} {name}")
if not cond:
fails.append(name)
class FakeHass:
"""A scripted meter and a plant that tracks the command instantly."""
def __init__(self, meter):
self.meter = meter
self.batt = 0.0
self.writes = []
self.t0 = time.monotonic()
async def limits(self, entity):
return (-5000.0, 5000.0, 1.0)
async def number(self, entity, invert=False):
if entity == "sensor.grid":
return self.meter(time.monotonic() - self.t0)
if entity == "sensor.batt":
return self.batt
if entity == "sensor.soc":
return 50.0
return None
async def set_number(self, entity, value):
self.writes.append(float(value))
self.batt = float(value)
return True
class _Store:
data = {}
def set(self, *a):
pass
class _Pub:
enabled = False
def publish(self, *a, **k):
pass
def close(self):
pass
BASE = {"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, "slew_w": 1000,
"deadband_w": 15, "target_grid_w": -10, "step_w": 10,
"heartbeat_s": 10, "stale_input_s": 15}
def drive(meter, floor, seconds):
"""Run the real control loop for `seconds`; return (cycle times, writes)."""
async def go():
hass = FakeHass(meter)
c = Controller(dict(BASE, cycle_floor_s=floor), hass, _Store(), _Pub(),
stop=asyncio.Event())
c.auto = True
stamps = []
real = c.cycle
async def counted():
stamps.append(time.monotonic() - hass.t0)
await real()
c.cycle = counted
task = asyncio.create_task(c.run_control())
await asyncio.sleep(seconds)
c.stopping = True
await asyncio.wait_for(task, timeout=10)
return stamps, hass.writes
return asyncio.run(go())
def worst_gap(stamps, seconds):
"""Longest stretch with no cycle, including the tail after the last one."""
edges = [0.0] + list(stamps) + [seconds]
return max(b - a for a, b in zip(edges, edges[1:]))
# --------------------------------------------------------------------------- #
print("a meter that repeats the same value")
FLAT = lambda t: 350.0 # noqa: E731
FLOOR, SECONDS = 3.0, 12.0
pre_stamps, pre_writes = drive(FLAT, NEVER, SECONDS)
# The defect itself, demonstrated rather than described: one cycle at startup
# and then nothing for the rest of the window, while writes keep happening.
check("pre-fix: a constant meter runs exactly one cycle, at startup",
len(pre_stamps) == 1)
check("...while the heartbeat keeps writing anyway, so nothing looks wrong",
len(pre_writes) >= 2)
post_stamps, post_writes = drive(FLAT, FLOOR, SECONDS)
check("post-fix: the same constant meter runs cycles throughout",
len(post_stamps) >= int(SECONDS / FLOOR) - 1)
# The actual acceptance criterion: a BOUND on the gap, not a count.
gap = worst_gap(post_stamps, SECONDS)
check(f"...with no gap longer than the floor ({gap:.1f}s <= {FLOOR + 1.5:.1f}s)",
gap <= FLOOR + 1.5)
check("...and the loop actually corrected, instead of holding a stale command",
len(post_writes) > len(pre_writes))
# --------------------------------------------------------------------------- #
print("a meter that changes normally - nothing may move")
# ⚠️ THE REGRESSION GUARD, and the more important half. The floor is a deadline
# on the gap between cycles, reset by every cycle whichever trigger fired - not
# a second scheduler. On a healthy meter it must never elapse, so the cycle
# count, the write count and the commanded values must be IDENTICAL either side
# of the fix. Anything else is a new write cadence, which SAFETY-12 forbids:
# the deadband, step_w and the 10 s force-write decide what reaches the wire,
# and this change is only allowed to alter when the loop thinks.
STEPS = [0.0, 300.0, 850.0, -400.0, 120.0, -900.0, 60.0]
LIVE = lambda t: STEPS[int(t // 2) % len(STEPS)] # noqa: E731
a_stamps, a_writes = drive(LIVE, NEVER, 12.0)
b_stamps, b_writes = drive(LIVE, FLOOR, 12.0)
check("a changing meter runs the same number of cycles as before the fix",
len(a_stamps) == len(b_stamps))
check("...commanding exactly the same sequence of values", a_writes == b_writes)
# --------------------------------------------------------------------------- #
print("a meter parked at the target - the converged operating point")
# The case TEL-05 warned about: at the converged target the power figure
# legitimately repeats for minutes. The loop must still run, and must still
# write nothing but the heartbeat, because the deadband - not the arrival
# trigger - is what holds the command still.
AT_TARGET = lambda t: -10.0 # noqa: E731
c_stamps, c_writes = drive(AT_TARGET, NEVER, 12.0)
d_stamps, d_writes = drive(AT_TARGET, FLOOR, 12.0)
check("the loop evaluates at the converged point too", len(d_stamps) > len(c_stamps))
check("...and the deadband still holds the command, so writes are unchanged",
d_writes == c_writes)
# --------------------------------------------------------------------------- #
print()
if fails:
print(f"{len(fails)} FAILED: " + "; ".join(fails))
sys.exit(1)
print("all checks passed")