e2e331ea5465e667488ccf8548ec9c1a1f4552d3
12
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 glenn schrooyenandClaude Opus 5
|
547d796c5d |
CFG-01 review: widen the rule, refuse the firmware-veto case, fix the stop footgun
Review findings, all confirmed in code before changing anything. THE RULE DID NOT AUTHORISE ITS OWN BEST REFUSAL. The stated test was "makes a safety mechanism inoperative, or makes a stated limit untrue", and a blank setpoint_entity does neither - layer 1 pinning the inverter at 0 W is the failsafe WORKING, as validate.py's own text admitted. The real justification is a second, separate category: the add-on is inert and indistinguishable from healthy. Stated as (a) and (b), with each refusal tagged, and with both halves explicitly bounded - (a) is not "any two numbers that could disagree", (b) is not "anything degraded". Left implicit, the gap is where trivia-creep comes back in. meter_max_age_s ABOVE THE FIRMWARE'S VETO IS NOW REFUSED. The schema allows int(5,300); the firmware's threshold is a compile-time `max_age_s: "30"` and it trips layer 1 at `p1_age_s >= 30`. At 120 the add-on believes a 119 s reading is fresh and keeps commanding while the board has already vetoed and is holding at 0 W - healthy meter, running controller, nothing moving. Structurally identical to the max_w case, and unlike max_w it is settable on this install, so the headline behaviour finally has a live demonstration. That firmware file's own comment says a lower bound on this "belongs in CFG-01, not here". Only ABOVE is refused: below the veto the add-on gives up first, which is fail-safe and visible on its own status page. Scoped to P1 being enabled, because meter_max_age_s governs nothing otherwise. FIRMWARE_INGEST_VETO_S cannot be imported from a compile-time YAML substitution, so it carries a note on the single direction in which it can go stale - and a warning that TEL-06 is editing `ingest_stale`, so a matching integer may stop meaning a matching rule. stop= WAS A LIVE FOOTGUN. It defaulted to None, which made a Controller that cannot stop itself the DEFAULT one. On the late-refusal path that meant: log the banner, return, and leave the process alive forever with no control loop and no exit - the exact "inert but looks healthy" failure this module exists to refuse. Now required and keyword-only, so the broken shape does not construct; test_p1.py's five call sites pass an Event. That whole path had no test and no live demo. It has one now: it drives run_control() against a device that only reports a narrower range once the loop starts, and asserts it refuses, signals, returns, and writes NOTHING - plus the converse, that an agreeing device leads to real writes. This also covers the max_w composition that cannot be reached through the Supervisor on the live install. max_w <= 0 is refused. Schema-blocked in production, but validate_startup_config() is the simulation suite's entry point too, and a non-positive max_w inverts the arbiter's supervised bounds into a contradiction that reads as a safety trip rather than a typo. Verified live 2026-08-28 (backup goodwe_controller-cfg01r2-1787940076): meter_max_age_s 120 refused with a message naming 120, 30, and THE FIRMWARE WINS; no traceback; add-on restored byte-exact and running. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa |
||
|
glenn schrooyenandClaude Opus 5
|
fc9bd89dca |
CFG-01: refuse to start on a configuration that cannot be honoured
main.py used to read the setpoint device's own +-5000 W rating, notice a
configured max_w above it, log one WARNING, and run anyway with
write_setpoint() quietly clamping. The operator's stated ceiling was never
the real ceiling and the only evidence had scrolled off the log. specs
§1.5 is explicit that such a config is refused at startup, not survived at
runtime.
validate_startup_config() runs in amain() before the broker, the web
server or any task exists, so a refusal cannot leave half a controller
running behind a UI. It refuses exactly three things, all sharing one
property - continuing makes a safety mechanism inoperative or makes a
stated limit untrue:
1. max_w outside the setpoint device's own rating.
2. No setpoint_entity: every write is a no-op, the device rating is
never read, and the ESP32 heartbeat that holds off the layer-1
failsafe never happens. It ran, looked healthy, and moved nothing.
3. A meter_source that can never produce a reading. These were already
caught, but they only DISABLED ingestion, which leaves grid None
forever: permanent inputs-missing, 0 W, and a status page
indistinguishable from a healthy meter nobody has fed yet.
Those last predicates move into p1.validate_meter_options(), which
build_source() also calls - one definition, two callers, not two
competing validators. ha_dsmr with half its entities and mqtt_p1 with a
blank topic gained the check the other two transports already had.
Deliberately not refused, because untidy is not unsafe: integrator_max_w
above max_w (the arbiter still bounds the wire to +-max_w, so the cost is
unwind latency - warned), a missing soc/batt entity (fail-safe and
already a red NOT READY banner), a setpoint entity that has not appeared
on the Core API yet (ESPHome is routinely late; re-checked in
run_control() once the real rating arrives), and a thin meter_poll_s
margin.
A refusal is legible as one: a banner saying in words that this is
configuration and nothing is broken, numbered problems, no traceback, and
exit 78 (EX_CONFIG).
Verified on the live add-on 2026-08-28, not by reading the code: a blank
setpoint_entity and a meter_poll_s equal to meter_max_age_s each stopped
the add-on with the banner and no stack trace.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa
|
||
|
glenn schrooyenandClaude Opus 5
|
80ebb96e7d |
Two comments asserted an entity id that has never existed
mqtt.py said the P1 age entity "must be exactly sensor.p1_sample_age_s, because SAFETY-01's firmware watchdog subscribes to that literal id", and main.py:340 repeated it. Both are wrong, and wrong in the direction that disarms a safety layer. On HA 2026.8 object_id in a discovery payload does not determine entity_id. HA composes it from the device name plus the entity name, so this one is sensor.goodwe_p1_sample_age. That is what the firmware subscribes to, that is what exists on the live system (verified 2026-08-26), and the firmware's own comment records that sensor.p1_sample_age_s never existed on any HA. The hazard is not the stale name itself, it is that two shipped artefacts disagreed about a safety-critical id while sounding certain. A reader who trusted these comments would have "corrected" the firmware to a name that resolves to nothing - and the failure is silent, because has_state() simply stays false forever and the ingest guard never arms. The firmware calls that out as the shape that FAILS SILENTLY AND LOOKS CORRECT. The comment now says what actually determines the id, and states plainly that renaming the device or the entity's friendly name changes the composed id and disarms the guard. It also cross-references TEL-06, since the guard reads this sensor's value rather than its arrival and so cannot fire for a dead publisher. Found in a backlog audit, as the substance behind PKG-02's premise: the real coupling between firmware and add-on is names, not versions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa |
||
|
glenn schrooyenandClaude Opus 5
|
4d41b0a79e |
TEL-05 review follow-ups: show unchanged_s, name a timeout, keep the poll task alive
Four follow-ups on the reviewed and approved TEL-05 work. Additive; no shipped behaviour changes except the two failure paths below. 1. unchanged_s had no operator surface. DOCS.md told a reader "the transport tracks it as unchanged_s" and there was nowhere to look: main.py built the transport, scheduled run(), and never read the object again. The status page now shows it on the healthy P1 line. Still NOT thresholded and NOT folded into the age - that refusal was reviewed and upheld, because at the converged -10 W this controller aims for a 1 Wh register needs ~6 minutes to move, so any limit false-trips at the target operating point. The whole argument for leaving it to a human requires the human being able to see it. 2. The "equivalent mutant" note on the content_type guard was wrong, and the comment is downgraded to say so. web.Response(text=...) defaults to text/plain, so the fake meter CAN serve valid JSON under the wrong mimetype. Test added; shipped behaviour was already correct. 3. A timed-out poll logged an empty reason: str(asyncio.TimeoutError()) is "", so the status page read "last error:" and then nothing, on a hung meter, at the moment the battery had just gone to 0 W. Falls back to the class name. Note str(err), not `err or ...` - an exception object is always truthy. 4. submit() sat outside the try in poll_once() and run() had no except, so a raise would kill the poll task permanently and SILENTLY - safe (the age climbs, the controller commands 0 W) but indistinguishable from a dead meter. Both wrapped; poll_s is already the retry cadence, so no backoff. Also a comment at the parse_homewizard range(phases) slice: a 3-phase meter configured as 1-phase understates the capacity-tariff figure. Filed separately, not fixed here. 242 checks in test_p1.py (236 before, 6 new). test_control 55, test_arbiter 18, test_maintenance 21, all untouched and green. Each new check proved non-vacuous: six mutations, six named reds, no suite aborts, sources restored byte-identical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa |
||
|
glenn schrooyenandClaude Opus 5
|
4bd659c499 |
TEL-01 review fixes: stop the age sensor tripping installs that have no meter
T-1, and it was a fleet-wide trip to zero. publish() emitted p1_age unconditionally, and published_age_s counts from P1Ingest.__init__ when no sample has ever arrived. With meter_source defaulting to off, every existing install would have published sensor.p1_sample_age_s climbing without bound; the ESP32 does `has_state() && state >= max_age_s` and forces the layer-1 failsafe, so each of them would have pinned its inverter at 0 W within 30 s. Exactly the opposite of the zero-regression the off default was for. The key is now omitted from the payload AND from MQTT discovery when P1 is off, so the entity does not exist at all - which is the status quo, and what has_state() is testing for. The predicate is one function, is_enabled(), because the grid reading, the task start and the discovery announcement have to agree or this comes back. T-2, connect no longer manufactures a sample. get_states returns whatever HA currently holds, which after a Core restart is a RestoreEntity value of unknown age; stamping it with ingest_ts=now reset the age and reported a fresh meter that could have been dead for an hour. run()'s own docstring already said a reconnect must emit nothing - the code disagreed with it, and a test asserted the violation. The cache is still primed, so the first real state_changed builds a complete sample; the age just stays honest until one arrives. T-3, gaps are no longer filled with the last held value. The averager held a sample forward across any interval, so a meter dying at 5 kW and returning ten minutes later credited 5 kW x 600 s to the capacity-tariff accumulator - a fabricated peak on a permanent record. The hold is capped at max_age_s: past that the stretch is walked so block boundaries still land correctly, but nothing accumulates and elapsed does not grow, which is what finally makes the comment about a gap dragging the billed average down true. Same threshold for control and billing: a reading too old to steer by is too old to bill by. T-4, the out-of-order/duplicate guard is covered. It was untested, and the reason is worth recording: the obvious assertion passes without the guard, because the negative interval is separately refused by the covered > 0 test. What the guard prevents is the timestamp REWIND, which only shows up one sample later as a re-integrated window. The test now goes one sample later. T-6, DOCS was wrong about latency. meter_max_age_s and stale_input_s stack, so meter death to 0 W is 45 s and not 30. Documented as a table with both clocks. Also documented the T-5 asymmetry rather than papering over it: the age measures arrival, not change, so a stuck MQTT bridge republishing its last telegram still looks fresh. Correct on ha_dsmr, not detectable on mqtt_p1 without a change-detector. Written up as a known limit. Writing the T-1 test caught a second defect in the test itself: it recorded only MQTT topics, and object_id lives in the payload, so "the age sensor is not announced" had been passing for the wrong reason. test_p1.py: 99 -> 122 checks. 14 mutations run, all 14 red, files restored byte-identical - including one per fix above. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa |
||
|
glenn schrooyenandClaude Opus 5
|
f47f1f0129 |
TEL-01: P1 ingestion, with the derivation and the age the EMS owns
A Belgian P1 meter publishes two UNSIGNED registers, not one signed figure. Until now the add-on asked the installer to bridge that gap with a template sensor, which put the sign convention of the whole control loop in a text box. This moves it into the EMS: net = import - export, derived once, in one place, with a test that fails if anyone inverts it. Two transports behind one contract, chosen by `meter_source`: the HA WebSocket subscribing to the DSMR integration's entities, and MQTT on a configurable topic. Everything downstream reads P1Ingest, so switching is a config edit. `meter_source: off` is the default and keeps the existing meter_entity path, so no installed system changes until it opts in. The other half is the timestamp. Every accepted sample is stamped at ingest with a monotonic clock, `meter_max_age_s` is applied to it, and the age is published as sensor.p1_sample_age_s for the ESP32's stale-input watchdog. That entity is recomputed against the clock every second rather than only when a telegram lands, because HA pushes state only on change: a meter frozen at a constant reading emits nothing and looks, to anything watching the value, exactly like a meter that has died. The age tells them apart. Deliberately absent: any fallback to an inverter-side power figure. The inverter's own AC power correlates 0.998 with battery power and 0.09 with the real meter, so failing over to it means regulating against your own output. A gap stays a gap - a reconnect emits no synthetic sample, and a rejected telegram never resolves to 0 W or refreshes the timestamp. Quarter-hour averages are time-weighted over clock-aligned blocks rather than a mean of samples, so a cadence change cannot bias the capacity-tariff figure, and only offtake is accumulated so a quarter of pure export averages to 0 kW. Per-phase import is kept separately: on an unbalanced three-phase load the phase sum and the connection net are different numbers, and only one of them is billed. test_p1.py: 99 checks, runnable with a bare interpreter and no meter. Includes an end-to-end run of the HA transport against a fake Home Assistant websocket. Stacked on SAFETY-04; nothing here touches control.py. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa |
||
|
glenn schrooyenandClaude Opus 5
|
e46175559b |
SAFETY-04 review fixes: the freeze deadlocked, the bound was too loose
S-1. The frozen branch admitted a correction only if it shrank |i_w|. That is unsatisfiable for BOTH signs of error whenever |correction| > 2*|i_w|, i.e. whenever the integrator is near zero, so the loop stopped moving and the freeze could never clear - it clears when the inverter tracks, and not tracking is what saturation means. Measured: 0 W held into a 2 kW import indefinitely, where release/1.0 recovers on the next cycle. Re-encoded as the same asymmetric rule the output freeze has always used: may not wind further in the direction it is already pushing, may fall, cross zero or reverse. Same interpretation, an encoding that cannot deadlock. S-2. integrator_max_w defaulted to 1.5x max_w, which ADDED windup: in release/1.0 the accumulator was the post-clamp command and could never pass the rail. Default is now "follow max_w" (config 0 = unset). Measured on the 4000 W load-drop sim, first cycle after the drop: 1000 W at the new default, 1800 W at 3000. DOCS row inverted - the useful direction is below max_w, and the 14 768 W anecdote is a vendor controller, not evidence about this code. S-3. The claim that i_w=None preserved release/1.0 exactly was false, because the S-1 gate ran regardless of seeding. It is true again, and now asserted rather than asserted-about: 3024-case exhaustive comparison against a transcription of the old law, over both freeze states, both signs and either side of the deadband. Added the carried-i_w convergence/overshoot sim that the shipped configuration was missing. S-4. Cycles are distinct meter values, not seconds: cycle() runs only when the meter reading changes, so the window has no wall-clock bound. Comment and DOCS corrected; the stall is detection latency, not a windup hazard, because the same condition stalls the whole loop. test_control.py: 33 -> 41 checks, all passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa |
||
|
glenn schrooyenandClaude Opus 5
|
37bac79ad8 |
SAFETY-04: clamp the integrator, not only the output
The loop was in velocity form: the accumulator WAS the commanded power, so "clamp the integrator" and "clamp the output" were the same line of code and could not be set apart. This makes the accumulator an explicit carried value (`i_w`), bounds it with its own `integrator_max_w`, and keeps the output clamp where it was. Killing either mechanism now still leaves the other holding - which is the point of the ticket, and what the new regression test asserts. Freeze semantics: while saturated the integrator may unwind but not wind further. A strict freeze would strand the command at whatever it reached, because the condition that releases it is the inverter tracking again, and not tracking is exactly what saturation means. The integrator is re-seeded from the arbiter's actual output whenever the loop did not get what it asked for, so entering any failsafe (all of which resolve to 0 W) zeroes it, and the first cycle after release does not dump the stale period as power. test_control.py: 24 -> 33 checks, all passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa |
||
|
glenn schrooyenandClaude Opus 5
|
5184a2cfc2 |
Rest the meter just below zero, not at zero
The deadband is a one-way ratchet: any resting point inside it holds indefinitely. Import and export are separate registers on the meter, so a loop resting at +14 W bills 0.34 kWh/day while behaving perfectly. target_grid_w (default -10 W) moves that residue onto the export register. Worst billed rest point drops from 15 W to under 5 W. Behaviour is unchanged at target_grid_w: 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
 adminandClaude Opus 5
|
017b798fe6 |
Precedence arbiter: one rule instead of an if/else ladder
Two controllers writing one actuator is the failure this system exists to
avoid. "Exactly one writer" was true, but only as a convention held up by
careful reading - which does not survive an EV charger and a heat pump wanting
the same battery.
Strategies now return claims and arbiter.py resolves them:
highest-priority `set` wins (none at all means 0 W), then every `limit` whose
priority is >= that set's applies, most restrictive first; contradictory
limits command 0 W and are flagged as the bug they are.
The second clause is the whole point. "Money outranks maintenance" used to be a
hand-written exception inside a Jinja template; it is now a consequence of the
priorities - the charge-only limit binds the loop but cannot bind a
higher-priority peak claim.
Also: maintenance shaping moved out of control.py, which is a controller again
and not a policy engine; the loop now tracks the arbiter's actual output rather
than its own last wish, so it does not jump when it regains control; and every
decision explains itself ("loop -> 0 W, limited by maintenance(charge-only)")
in the UI and the log.
19 new assertions in test_arbiter.py, each one a precedence question someone
will eventually ask in the field. Deployed to the reference site as 0.2.0 and
holding grid within a few watts of zero.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016NckgXecasQb2eSsPYNSW6
|
||
|
adminandClaude Opus 5
|
594f8f9fc9 |
Six silent failures found by installing this on a live system
Migrated the reference site off the YAML packages and onto the add-on. Every bug below presented identically: the add-on starts, logs "started", serves its UI, and cannot do its job. - run.sh needs #!/usr/bin/with-contenv sh. s6-overlay sanitises the environment for services, so a plain shebang means SUPERVISOR_TOKEN is absent and every Core API call is 401 - while homeassistant_api: true makes permissions look granted. Startup now prints the token length and probes the API. - Supervisor keys the image by config.yaml `version`, so rebuilding without a bump reuses the old image. Two fixes appeared not to work because of it. - Alpine is musl and has no aiohttp wheel on PyPI; deps now come from apk so nothing compiles on a client's Pi. - Alpine ships paho-mqtt 1.x, which has no CallbackAPIVersion. That raised at construction and took the control loop down with it - so MQTT setup is now wrapped too. Observability must never be able to stop the controller. - MQTT discovery is published from on_connect: paho silently drops QoS-0 publishes issued before the CONNACK, so the previous code announced nothing while logging "MQTT connected". - Repeated failures now log once a minute. Six warnings a second rolled the log buffer and destroyed the startup diagnostics needed to find the 401. - auto_start could never fire, because the store's defaults always supplied auto: False for the fallback to find. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NckgXecasQb2eSsPYNSW6 |
||
|
adminandClaude Opus 5
|
0a1e61dbc9 |
Packaged as a Home Assistant add-on, with a field guide
Turns the reference RS485 controller into something a technician can install at a client site: a typed config form instead of YAML, an ingress UI that names misconfiguration in words, and persistent state that cannot be broken by a timezone. Why an add-on rather than YAML packages or blueprints: - Blueprints cannot create helpers, and the maintenance cycle is a state machine whose phase and completion date must survive restarts. - YAML packages need filesystem access, a configuration.yaml edit and a restart - none of which belong in a client install. - Add-ons authenticate with SUPERVISOR_TOKEN, so there is no long-lived token to generate, store or leak on someone else's machine. - Requires HA OS/Supervised. Container and Core installs cannot run add-ons at all, which is a market decision, not an oversight. The control law and the maintenance machine are pure functions with no Home Assistant imports, and both ship with runnable checks (22 and 22 assertions). Every assertion corresponds to a rule whose absence caused an observed failure on hardware - the saturation duration term, the clamp-before-slew ordering, the deadband, the sign convention. One behaviour deliberately differs from the implementation it replaces: when its inputs go missing this commands 0 W rather than replaying the last setpoint. The reference version kept replaying, which the hardware watchdog cannot catch - from the ESP32's side, Home Assistant is still talking to it. Includes the ESPHome firmware (now parameterised: node name, inverter rating, watchdog timeout) and the optional RS485 e-stop. FIELD-GUIDE.md carries the commissioning gates, all judged on the wire rather than on how Home Assistant looks, plus the written statement a site without an e-stop needs signed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NckgXecasQb2eSsPYNSW6 |