Compare commits
5
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
aae74d7571 | ||
|
|
8cc9438aec | ||
|
|
96a808a057 | ||
|
|
88cd97cc7a | ||
|
|
e2e331ea54 |
+10
-1
@@ -52,9 +52,18 @@ Run the checks before deploying. No framework needed:
|
|||||||
|
|
||||||
```bash
|
```bash
|
||||||
cd goodwe_controller
|
cd goodwe_controller
|
||||||
python3 test_control.py && python3 test_maintenance.py && python3 test_arbiter.py
|
python3 test_control.py && python3 test_maintenance.py && python3 test_arbiter.py \
|
||||||
|
&& python3 test_validate.py && python3 test_p1.py && python3 test_loop.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 —
|
||||||
|
an understated runtime is what makes a slow test look like a hung one, and a slow test
|
||||||
|
that looks hung gets deleted.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## 3. Known broken / unfinished
|
## 3. Known broken / unfinished
|
||||||
|
|||||||
@@ -67,6 +67,121 @@ 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,
|
configuration problem and nothing is broken, the numbered problems, no traceback,
|
||||||
and exit status 78 (`EX_CONFIG`).
|
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.
|
||||||
|
|
||||||
|
**It was worse than a latency limit: it was a safety claim that could not
|
||||||
|
fire.** `stale_input_s` and the `inputs missing -> 0 W` claim both live *inside*
|
||||||
|
`cycle()`. Pre-fix, if the meter value happened to be constant, `cycle()` was
|
||||||
|
never called again - so that claim had no way to run. Constant meter, SoC entity
|
||||||
|
dying at t=4 s, same binary:
|
||||||
|
|
||||||
|
```
|
||||||
|
pre-fix writes: [(0.0, 220.0), (0.0, 220.0), (10.1, 220.0)]
|
||||||
|
final target: 220.0 W reason: 'loop -> 220 W'
|
||||||
|
post-fix writes: [(0.0, 220.0), (0.0, 220.0), (3.0, 430.0), (9.1, 0.0), (10.1, 0.0)]
|
||||||
|
final target: 0.0 W reason: 'safety -> 0 W'
|
||||||
|
```
|
||||||
|
|
||||||
|
The pre-fix controller held 220 W through the entire input outage with its
|
||||||
|
status line still reading `loop -> 220 W`, and layer 1 does not cover that case
|
||||||
|
on a `meter_source: off` install: no P1 age sensor is published, so the firmware
|
||||||
|
has nothing to veto on, while the heartbeat keeps its watchdog fed. So the old
|
||||||
|
behaviour was not "holds a stale command safely" - it was a failsafe that a
|
||||||
|
repeating meter switched off. `test_loop.py` asserts both halves.
|
||||||
|
|
||||||
|
`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.
|
||||||
|
|
||||||
|
The reset is not the only thing holding that up, and the other argument is the
|
||||||
|
sturdier one: the combined cycle rate is `max(arrival rate, 1/floor)`, never the
|
||||||
|
sum. At 1/15 s the floor is strictly slower than the ~1/5 s arrival rate, so
|
||||||
|
even a hypothetical floor racing the arrivals could not exceed the nominal
|
||||||
|
healthy rate. The re-tune is unreachable by construction. ⚠️ That holds only
|
||||||
|
while `cycle_floor_s` stays above the meter's update interval, and the schema
|
||||||
|
minimum is 5 s - which is exactly the reference meter's interval. Nothing
|
||||||
|
enforces the relationship; `validate.py` is the obvious home for it and does not
|
||||||
|
have it yet.
|
||||||
|
|
||||||
|
Observed over six real minutes on the same rig, one binary, meter held at a
|
||||||
|
literally constant 350.0 W: **before, 1 cycle** (at startup) and then nothing,
|
||||||
|
the command stuck at 220 W against a 350 W import for the rest of the window.
|
||||||
|
**After, 24 cycles**, worst gap 15.14 s, and the ordinary PI ramp doing its job -
|
||||||
|
220, 430, 650, 860, 1080, 1300, 1510, 1730, 1940, 2000 W, one step per cycle,
|
||||||
|
rail at t=136 s and flat after. Three minutes with the meter parked at the
|
||||||
|
target instead: **1 cycle before, 12 after, and 19 writes either way - the same
|
||||||
|
19**. Three minutes on a normally-changing meter: **36 cycles and 49 writes on
|
||||||
|
both sides, the same values in the same order.** The heartbeat write count never
|
||||||
|
moves; the only writes the fix adds are the ten the loop was supposed to be
|
||||||
|
making all along.
|
||||||
|
|
||||||
|
15 s is bracketed, not round.
|
||||||
|
|
||||||
|
**Below it, ~11 s, and it arises more routinely than a dropped poll.**
|
||||||
|
`HomeWizardLocalSource` polls at 5.0 s against a meter that updates every 4.97 s
|
||||||
|
(NOTES.md:640). Two near-equal periods beat, so roughly once every ~14 minutes
|
||||||
|
the poller steps over an update entirely - a ~10 s gap between changed values on
|
||||||
|
a perfectly healthy meter, plus up to 1 s for the 1 s tick to notice. A failed
|
||||||
|
HTTP poll (retried on the next tick, no backoff) adds another 5 s. The floor
|
||||||
|
sits above the beat-skip so it does not fire on that alone; what it takes to
|
||||||
|
reach 15 s is a beat-skip *and* a failed poll coinciding.
|
||||||
|
|
||||||
|
**Above it, 30 s.** `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. ⚠️ The default is therefore not
|
||||||
|
independently justified at 15: if the firmware's veto ever moves, this moves
|
||||||
|
with it. Noted beside the constant in `main.py`.
|
||||||
|
|
||||||
|
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. In
|
||||||
|
P1 mode 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 there can only ever run on a reading
|
||||||
|
this system already calls fresh. ⚠️ That is transport-conditional: with
|
||||||
|
`meter_source: off`, `read_inputs()` takes the grid figure straight from
|
||||||
|
`hass.number()` with **no age gate at all**, so a Home Assistant entity that has
|
||||||
|
gone quiet reads as current on that path, with or without this change. Running
|
||||||
|
the full law is still right there - the alternative is a degraded second law
|
||||||
|
reasoning about an age nobody measured - but the floor does not make that path
|
||||||
|
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
|
**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
|
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.
|
unsigned registers and refuse negatives, so neither fits the meter installed here.
|
||||||
|
|||||||
+36
-14
@@ -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 |
|
| `target_grid_w` | -10 | What the meter should rest at. Negative = a slight export |
|
||||||
| `step_w` | 10 | Quantisation |
|
| `step_w` | 10 | Quantisation |
|
||||||
| `saturation_w` | 500 | Divergence that counts as "the inverter is at a limit" |
|
| `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 |
|
| `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 |
|
| `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" |
|
| `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) |
|
| `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
|
**cycles** instead, and that is a deliberate, accepted deviation rather than an
|
||||||
oversight - the acceptance criterion is not met as literally written.
|
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
|
A cycle here is one meter update **or `cycle_floor_s` elapsing, whichever comes
|
||||||
meter value differs from the previous poll. At the reference P1's ~5 s update rate the
|
first**. At the reference P1's ~5 s update rate the default of 3 cycles is usually
|
||||||
default of 3 cycles is usually around 15 s, but there is **no guaranteed wall-clock
|
around 15 s, and the worst case is bounded at `3 x cycle_floor_s` = 45 s. It is still
|
||||||
window** - a meter that repeats the same value stalls the counter for as long as it
|
not the 10 s the specification asks for, so the deviation stands - but it is now a
|
||||||
repeats.
|
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
|
The half that survives is the first one: the control law is still a pure function with
|
||||||
without hardware, and a seconds-based window would have to live in the controller;
|
no clock, and the wall-clock window still lives in `Controller`, which has one - see
|
||||||
- a stalled counter is a detection-latency limit and not a runaway risk. The condition
|
`run_control()`. What changed is that the controller now enforces a floor on the gap
|
||||||
that stalls it - an unchanging meter - stops the whole loop, so nothing accumulates
|
between cycles, so `saturation_cycles` finally means a bounded number of seconds.
|
||||||
while it is stalled.
|
|
||||||
|
|
||||||
If a guaranteed window matters on your site, raise `saturation_cycles` for a fast meter,
|
If a tighter window matters on your site, lower `cycle_floor_s` (not below your meter's
|
||||||
and treat the figure as "N meter updates" rather than "N seconds".
|
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
|
#### Why `target_grid_w` is not zero
|
||||||
|
|
||||||
|
|||||||
@@ -95,19 +95,23 @@ def compute(
|
|||||||
# lets slew be larger than saturation_w.
|
# lets slew be larger than saturation_w.
|
||||||
#
|
#
|
||||||
# The spec states this window twice and differently: "> 10 s" (§11.2) and
|
# 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
|
# "3 samples" (§10.3). This counts CYCLES, and a cycle is still not a unit
|
||||||
# time: run_control() calls cycle() only when the meter value CHANGES
|
# of time - but it is now a BOUNDED one. run_control() runs a cycle on a
|
||||||
# (`if self.grid != last_grid`), so three cycles is three distinct meter
|
# changed meter value OR on `cycle_floor_s` elapsing, whichever comes first
|
||||||
# readings and nothing more. At the reference P1's ~5 s update rate that is
|
# (SAFETY-12), so three cycles is at most 3 x cycle_floor_s = 45 s at the
|
||||||
# usually ~15 s, but there is no upper bound on it - a meter that repeats a
|
# defaults, and usually ~15 s at the reference P1's ~5 s update rate.
|
||||||
# value stalls the counter.
|
|
||||||
#
|
#
|
||||||
# That is a detection-latency limit, not a windup hazard: the same
|
# ⚠️ THIS COMMENT USED TO CLAIM there was no upper bound and that it did not
|
||||||
# condition that stalls the counter stalls the whole loop, so nothing
|
# matter, "because the same condition that stalls the counter stalls the
|
||||||
# accumulates in the meantime either. If a wall-clock window is ever
|
# whole loop, so nothing accumulates in the meantime". The first clause was
|
||||||
# required, it belongs in Controller (which has a clock) and not here.
|
# true and the second was the bug: a stalled loop does not stop commanding,
|
||||||
# ponytail: this function is worth keeping clockless; the ceiling is that
|
# it goes on emitting its last command through the heartbeat. Windup was
|
||||||
# saturation_cycles cannot express a guaranteed number of seconds.
|
# 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
|
saturated_now = abs(prev_w - actual_w) > tuning.saturation_w
|
||||||
sat_count = min(sat_count + 1, 10) if saturated_now else 0
|
sat_count = min(sat_count + 1, 10) if saturated_now else 0
|
||||||
frozen = sat_count >= tuning.saturation_cycles
|
frozen = sat_count >= tuning.saturation_cycles
|
||||||
|
|||||||
@@ -342,11 +342,85 @@ class Controller:
|
|||||||
last_grid = object()
|
last_grid = object()
|
||||||
heartbeat = float(self.o.get("heartbeat_s", 10))
|
heartbeat = float(self.o.get("heartbeat_s", 10))
|
||||||
last_beat = 0.0
|
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 - and therefore no
|
||||||
|
# change to the per-cycle gain the tuning was measured against.
|
||||||
|
#
|
||||||
|
# That is true twice over, and the second reason is the sturdier one:
|
||||||
|
# the combined cycle rate is max(arrival rate, 1/floor), never the sum.
|
||||||
|
# At the defaults 1/15 s is strictly slower than the ~1/5 s arrival
|
||||||
|
# rate, so even a floor that raced the arrivals could not push the loop
|
||||||
|
# past its nominal healthy rate. The re-tune is unreachable by
|
||||||
|
# construction, not merely by the reset. ⚠️ That argument holds only
|
||||||
|
# while floor > the meter's update interval - which is why the schema
|
||||||
|
# minimum of 5 s deserves the second look validate.py has not given it
|
||||||
|
# yet: 5 s IS the reference meter's interval, i.e. the one configurable
|
||||||
|
# value that would double the cycle rate.
|
||||||
|
#
|
||||||
|
# Why it has to exist, and it is not only about latency: `self.grid !=
|
||||||
|
# last_grid` alone means a meter that repeats a value runs NO control
|
||||||
|
# cycles at all. The heartbeat below keeps re-emitting the last command
|
||||||
|
# and the firmware keeps putting it on the wire, so every indicator
|
||||||
|
# reads healthy - and, worse, the `inputs missing -> 0 W` claim lives
|
||||||
|
# INSIDE cycle(), so on a repeating meter THAT SAFETY CLAIM CANNOT FIRE.
|
||||||
|
# Demonstrated in test_loop.py: constant meter, SoC entity dies at t=4 s,
|
||||||
|
# and the pre-fix controller holds 220 W through the whole outage with
|
||||||
|
# the status line still reading "loop -> 220 W". Layer 1 does not cover
|
||||||
|
# it on a `meter_source: off` install either - no P1 age sensor is
|
||||||
|
# published, so the firmware has nothing to veto on. 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".
|
||||||
|
#
|
||||||
|
# ⚠️ THE 15 s DEFAULT HAS A CEILING IT DOES NOT OWN. It is under
|
||||||
|
# validate.FIRMWARE_INGEST_VETO_S (30 s) and under meter_max_age_s on
|
||||||
|
# purpose, so the loop re-decides at least twice inside any window in
|
||||||
|
# which layer 1 might otherwise be the only thing deciding. If the
|
||||||
|
# firmware's veto ever moves, this default moves with it - it is not
|
||||||
|
# independently justified at 15.
|
||||||
|
floor = float(self.o.get("cycle_floor_s", 15))
|
||||||
|
last_cycle = None
|
||||||
while not self.stopping:
|
while not self.stopping:
|
||||||
await self.read_inputs()
|
await self.read_inputs()
|
||||||
# A cycle is one meter update, exactly as on the reference install.
|
now_mono = asyncio.get_running_loop().time()
|
||||||
if self.grid != last_grid:
|
# 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_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 in P1
|
||||||
|
# mode 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 cannot run the PI on a 60 s old
|
||||||
|
# figure, because at 60 s there is no figure.
|
||||||
|
#
|
||||||
|
# ⚠️ THAT GUARANTEE IS TRANSPORT-CONDITIONAL, and saying so
|
||||||
|
# matters more than the guarantee: with `meter_source: off`,
|
||||||
|
# read_inputs() takes self.grid straight from hass.number() with
|
||||||
|
# NO age gate at all, so a Home Assistant entity that has gone
|
||||||
|
# quiet reads as a current value on that path with or without
|
||||||
|
# this change. Running the full law is still the right choice
|
||||||
|
# there - the alternative is a degraded second law reasoning
|
||||||
|
# about an age nobody measured - but the floor does not make
|
||||||
|
# that path fresh, and nothing here should be read as claiming
|
||||||
|
# it does. 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()
|
await self.cycle()
|
||||||
# ⚠️ The heartbeat is not an optimisation. The ESP32 treats silence
|
# ⚠️ The heartbeat is not an optimisation. The ESP32 treats silence
|
||||||
# longer than ~30 s as "the controller is gone" and zeroes the
|
# longer than ~30 s as "the controller is gone" and zeroes the
|
||||||
|
|||||||
@@ -90,6 +90,7 @@ options:
|
|||||||
integrator_max_w: 0
|
integrator_max_w: 0
|
||||||
heartbeat_s: 10
|
heartbeat_s: 10
|
||||||
stale_input_s: 15
|
stale_input_s: 15
|
||||||
|
cycle_floor_s: 15
|
||||||
auto_start: false
|
auto_start: false
|
||||||
|
|
||||||
# --- maintenance -----------------------------------------------------------
|
# --- maintenance -----------------------------------------------------------
|
||||||
@@ -151,6 +152,7 @@ schema:
|
|||||||
integrator_max_w: int(0,15000)
|
integrator_max_w: int(0,15000)
|
||||||
heartbeat_s: int(2,25)
|
heartbeat_s: int(2,25)
|
||||||
stale_input_s: int(5,120)
|
stale_input_s: int(5,120)
|
||||||
|
cycle_floor_s: int(5,60)
|
||||||
auto_start: bool
|
auto_start: bool
|
||||||
|
|
||||||
maintenance_enabled: bool
|
maintenance_enabled: bool
|
||||||
|
|||||||
@@ -0,0 +1,213 @@
|
|||||||
|
"""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 ~95 SECONDS OF WALL CLOCK (measured 93 s), on purpose, and
|
||||||
|
that figure is the only thing standing between it and a future developer who
|
||||||
|
decides a slow test is a broken one. 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. Eight runs of the real loop is what the eight scenarios cost.
|
||||||
|
|
||||||
|
⚠️ If you shorten the windows, re-measure and update this number and the one in
|
||||||
|
HANDOVER.md. An understated runtime reads as a hang.
|
||||||
|
|
||||||
|
`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, soc_dies_at=None):
|
||||||
|
self.meter = meter
|
||||||
|
self.soc_dies_at = soc_dies_at
|
||||||
|
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):
|
||||||
|
now = time.monotonic() - self.t0
|
||||||
|
if entity == "sensor.grid":
|
||||||
|
return self.meter(now)
|
||||||
|
if entity == "sensor.batt":
|
||||||
|
return self.batt
|
||||||
|
if entity == "sensor.soc":
|
||||||
|
if self.soc_dies_at is not None and now >= self.soc_dies_at:
|
||||||
|
return None
|
||||||
|
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, soc_dies_at=None, **over):
|
||||||
|
"""Run the real control loop for `seconds`; return (cycle times, writes)."""
|
||||||
|
async def go():
|
||||||
|
hass = FakeHass(meter, soc_dies_at)
|
||||||
|
c = Controller(dict(BASE, cycle_floor_s=floor, **over), 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, c.target, c.reason
|
||||||
|
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("an input dies while the meter repeats - the safety claim must still fire")
|
||||||
|
# ⚠️ THE STRONGEST PROPERTY IN SAFETY-12, and the one that makes it a safety fix
|
||||||
|
# rather than a latency fix. The `inputs missing -> 0 W` claim lives INSIDE
|
||||||
|
# cycle() (main.py, the stale_input_s branch and the P_SAFETY claim below it).
|
||||||
|
# Pre-fix, on a meter whose value happened to repeat, cycle() was never called
|
||||||
|
# again - so that claim could not fire AT ALL. The controller held its last
|
||||||
|
# command through an input outage of unbounded length with the status line still
|
||||||
|
# reading "loop -> N W".
|
||||||
|
#
|
||||||
|
# Layer 1 does not cover this either on a `meter_source: off` install: no P1 age
|
||||||
|
# sensor is published (see Controller.publish), so the firmware has nothing to
|
||||||
|
# veto on, and the heartbeat keeps its watchdog fed the whole time.
|
||||||
|
#
|
||||||
|
# stale_input_s is shortened to 3 s here so the outage resolves inside a test
|
||||||
|
# rather than inside a quarter of a minute. The mechanism is the default one.
|
||||||
|
c2_stamps, c2_writes, pre_target, pre_reason = drive(
|
||||||
|
FLAT, NEVER, 10.0, soc_dies_at=4.0, stale_input_s=3)
|
||||||
|
check("pre-fix: a dead input during a constant meter never reaches the safety claim",
|
||||||
|
pre_target != 0.0 and "safety" not in pre_reason)
|
||||||
|
check(f"...so it holds its last command instead ({pre_target:g} W, {pre_reason!r})",
|
||||||
|
pre_target == 220.0)
|
||||||
|
|
||||||
|
d2_stamps, d2_writes, post_target, post_reason = drive(
|
||||||
|
FLAT, FLOOR, 10.0, soc_dies_at=4.0, stale_input_s=3)
|
||||||
|
check("post-fix: the same outage commands 0 W", post_target == 0.0)
|
||||||
|
check(f"...through the safety claim, not by accident ({post_reason!r})",
|
||||||
|
"safety" in post_reason)
|
||||||
|
check("...and the 0 W actually reached the device", 0.0 in d2_writes)
|
||||||
|
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
print()
|
||||||
|
if fails:
|
||||||
|
print(f"{len(fails)} FAILED: " + "; ".join(fails))
|
||||||
|
sys.exit(1)
|
||||||
|
print("all checks passed")
|
||||||
Reference in New Issue
Block a user