Files
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
2026-08-28 20:05:10 +02:00

18 KiB
Raw Permalink Blame History

Changelog

0.4.0

CFG-01. The add-on now refuses to start on a configuration it cannot honour, instead of logging a line and carrying on. 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 only where continuing produces one of two outcomes: (a) a stated limit is untrue - the operator configured a bound and a lower layer silently enforces a different one - or (b) the add-on is inert and indistinguishable from healthy. (b) is not a special case of (a): with no setpoint_entity, layer 1 pinning the inverter at 0 W is the failsafe working. The system does not become unsafe, it becomes a convincing impression of a working controller.

Four things qualify:

  • max_w above (or below) the setpoint device's own rating. This is the one that changed behaviour. It used to warn the device wins and run anyway, with write_setpoint() silently clamping, so the configured ceiling was never the real one and the only evidence had scrolled off the log. The refusal names the configured value, the device's value, and which wins.
  • No setpoint_entity. A controller with no actuator: 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.
  • A meter_source that can never produce a reading - missing p1_host, missing p1_net_entity, a blank MQTT topic, a phase list disagreeing with meter_phases, an unknown source name, or a meter_poll_s at or past meter_max_age_s. These were already caught, but they only disabled ingestion, which leaves the grid reading None forever: permanent "inputs missing", 0 W, and a status page indistinguishable from a healthy meter nobody has sent anything to yet.
  • meter_max_age_s above the firmware's ingest veto. The schema accepts int(5,300); the ESP32's threshold is a compile-time max_age_s: "30" and it trips layer 1 at p1_age_s >= 30. Set 120 and the add-on treats a 119 s reading as fresh and keeps commanding, while the firmware has already vetoed and is holding the inverter at 0 W - healthy meter, running controller, nothing moving, and no line anywhere saying the number you set was overruled. Structurally the same fault as the max_w case. Only above the veto is refused: below it the add-on gives up first, which is fail-safe and visible on its own status page. Mirrored as validate.FIRMWARE_INGEST_VETO_S - it cannot be imported from a compile-time YAML substitution, so it carries a note about the one direction in which it can go stale.

max_w at or below zero is refused too. Schema-blocked on a real install, but validate_startup_config() is now also the simulation suite's entry point, and a non-positive max_w inverts the arbiter's supervised bounds into a contradiction that resolves to 0 W - i.e. it would read as a safety trip rather than as the typo it is.

The predicates for the meter group still live in p1.validate_meter_options(), which build_source() also calls - one definition, two callers, not two competing validators. Two transports gained a check they never had: ha_dsmr with only one of its two entities, and mqtt_p1 with a blank topic, both of which previously built a transport that could never read.

Deliberately not refused, because it is untidy rather than unsafe: integrator_max_w above max_w (the arbiter still bounds the wire to ±max_w, so the cost is unwind latency, not amperage - warned), a missing soc_entity / batt_entity (fail-safe, and already a red NOT READY banner naming it), a setpoint_entity that has not appeared on the Core API yet (ESPHome is routinely late; refusing would make a slow boot need a human - re-checked once the real rating arrives), and a thin-but-nonzero meter_poll_s margin.

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).

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.

TEL-05. meter_source: homewizard_local polls the meter's own local API directly. This exists because of something the HA paths cannot do: Home Assistant fires no state change for a repeated reading and exposes no arrival signal at all, so sensor.p1_sample_age_s on those transports measures time since the value changed, not since the meter reported. Real captured data has gaps of 42 s and 97 s between changes on a perfectly healthy meter - long enough for a 30 s staleness threshold to command 0 W for no reason. Every HTTP response to the local API is a genuine arrival, so the age means what it says.

Verified on the rig against a literally constant 350.0 W for 70 s - the exact state where the HA path recorded zero samples in 45 s - with 14 arrivals and the age never exceeding the poll cadence.

Set p1_host to the meter's address and meter_poll_s (default 5 s, matching the meter's own ~5 s update). Still defaults to off; nothing changes for an install that does not opt in.

Freeze detection is separate and deliberately not thresholded: a meter answering 200 OK with a stale number is arriving, so no arrival stamp can flag it. The energy registers carry that as unchanged_s, surfaced on the status line for a human to read rather than alarmed on - at a converged -10 W a 1 Wh register needs about six minutes to move, so any threshold would false-trip at the target operating point.

Unreleased

TEL-05. A fourth meter_source, homewizard_local, which polls a HomeWizard P1's own local API (GET /api/v1/data) instead of watching a Home Assistant entity. Set p1_host to the meter's address; meter_poll_s (default 5 s, the meter's own update rate) sets the cadence.

✅ This is the first transport whose sensor.p1_sample_age_s measures when the meter reported, and therefore the first one a firmware watchdog may threshold. Every HTTP response is an arrival: the meter answered, now, with its current reading, and whether the number moved is not consulted. Home Assistant cannot express that at all — a repeated reading emits no state_changed, advances last_reported on neither serialiser, and state_reported is not subscribable ("Event filter is required"). On ha_signed that made a healthy meter under a flat load indistinguishable from a dead one, and our own capture of this house's meter goes 42.2 s and 97.0 s between changes — both past the default meter_max_age_s of 30, i.e. a false trip to 0 W on a meter that is fine. If you have a HomeWizard P1, move to this mode.

Everything TEL-01 established is reused, not re-implemented: ingest timestamping, meter_max_age_s, the clock-recomputed age, the plausibility bounds and the §20 unsigned-decode rejection, and the same split_signed sign convention ha_signed uses. A failed or timed-out poll submits nothing, so it is a missing reading — never 0 W — and it does not reset the age.

Verified on the ENV-01 rig against sim/hwsim.py, steady and with --fault freeze injected. Still defaults to off.

⚠️ An arrival stamp still cannot see a frozen meter, and no arrival detector can: a meter answering 200 OK forever with a stale number is arriving. The local API does expose what the HA path never had — the total_power_*_kwh registers stop advancing — and the transport tracks it as unchanged_s, but that is deliberately not folded into the age and not thresholded: this controller regulates grid power toward ~0 W, and at a converged −10 W the export register needs six minutes to move by its 1 Wh resolution while the power figure legitimately repeats. Thresholding it at 30 s would rebuild the false-trip limit cycle at the exact operating point we aim for. Freeze detection needs the low-power case solved first, separately. It is shown on the status page's P1 line instead — leaving it unthresholded only holds up if a human can read it, so now they can.

TEL-04. A third meter_source, ha_signed, reading one signed Home Assistant entity: positive = import, negative = export. That is the shape a HomeWizard P1 publishes (sensor.p1_meter_active_power), and it is the meter actually fitted here - which neither TEL-01 transport can read, because ha_dsmr needs two unsigned registers and refuses a negative one, i.e. every exporting telegram. Set p1_net_entity, and p1_phase_net_entities for the per-phase capacity-tariff figures on a three-phase connection.

Everything TEL-01 established is inherited rather than re-implemented - the new transport is a subclass of the ha_dsmr one overriding only which entities it wants and how they become a sample. So ingest timestamping, meter_max_age_s, sensor.p1_sample_age_s recomputed against the clock and republished once a second, the plausibility bounds, and unavailable / unknown treated as a missing reading and never 0 W all behave identically across the three sources.

Still defaults to off; an existing install is unaffected until it opts in.

⚠️ sensor.p1_sample_age_s is published on ha_signed, but must not yet be thresholded by the ESP32 stale-input watchdog. On the HA WebSocket paths the age is stamped from state_changed, so it measures time since the value changed, not since the meter reported - and Home Assistant exposes no arrival signal for a repeated reading (no state_changed, no last_reported movement on either serialiser, and state_reported is not subscribable over the WebSocket). Measured on the ENV-01 rig against the real HomeWizard integration. ha_dsmr mostly escapes it because a telegram moves several entities at once; ha_signed has one, so a healthy meter under a flat load is indistinguishable from a dead one. Our own capture has the house meter going 42.2 s and 97.0 s between changes. Raising meter_max_age_s does not fix that, it only chooses which error you get; the fix is an arrival stamp from the meter itself and is a separate ticket. Full detail in DOCS.md.

0.3.0

SAFETY-04. The control law's integrator is now an explicit accumulator, bounded independently of the output clamp instead of inheriting whatever headroom the clamp happened to leave. It also freezes while the inverter is not tracking, rather than continuing to wind up against a command nothing is acting on. integrator_max_w (default 0) governs the bound; 0 means "follow max_w", which is the existing behaviour.

Behaviour is unchanged at the defaults - a 4,928-case equivalence sweep against the previous control law confirms it decides identically at integrator_max_w: 0.

TEL-01. P1 meter ingestion, so a Belgian P1's two unsigned registers (consumption, injection) no longer need a hand-written signed template sensor: the subtraction moves into the add-on, done once and tested. Two transports, chosen with the new meter_source option: ha_dsmr subscribes to the DSMR integration over the HA WebSocket, mqtt_p1 reads a topic. Defaults to off, which keeps the existing meter_entity path untouched - nothing changes for an install that does not opt in.

Enabling it publishes sensor.p1_sample_age_s: seconds since the newest accepted telegram, recomputed against the clock and republished roughly once a second rather than only when a telegram lands. That is deliberate - Home Assistant only pushes a state on change, so a meter sitting at a genuinely constant reading would otherwise look identical to a dead one. Watching the age instead means a frozen meter shows a climbing age, not a flat line. The firmware watchdog subscribes to this exact entity id.

Known limits, both already in DOCS.md: on mqtt_p1, a bridge stuck republishing its last telegram still "arrives", so the age cannot detect that particular failure - prefer ha_dsmr where both are available. And a dead P1 meter takes 45 s to reach 0 W commanded (30 s for meter_max_age_s to call the reading stale, then 15 s of stale_input_s on top), which is meter_max_age_s and stale_input_s stacking, not either one alone.

0.2.1

target_grid_w (default -10 W): what the meter should rest at. The deadband holds any resting point inside it indefinitely, and the meter bills import and export on separate registers, so resting at +14 W import costs 0.34 kWh/day with the loop behaving perfectly. Biasing the target slightly negative moves that residue onto the export register. Configurable in the Configuration tab; see DOCS.md for the trade-off table.

Behaviour is unchanged at target_grid_w: 0.

0.2.0

Precedence between strategies is now a first-class object instead of an if/else ladder, ahead of there being more than three of them.

Every strategy returns a CLAIM each cycle - set ("I want X") or limit ("the result must stay within these bounds") - and arbiter.py resolves them by one rule:

  1. Highest-priority set wins; no claim at all means 0 W.
  2. Then every limit whose priority is >= that set's priority applies, most restrictive first.
  3. Contradictory limits are a BUG: command 0 W and say so.

Clause 2 is why "money outranks maintenance" is now a consequence of the priorities rather than a special case in a Jinja template: the maintenance charge-only limit binds the loop, but will not bind a higher-priority peak shaving claim when one exists.

  • Maintenance shaping (charge-only, cheap-window floor) moved out of the control law. control.py is once again only a controller that tracks the meter.
  • The loop computes from the ARBITER's last output, not its own last wish. If something outranked it, that is what the hardware actually did, and tracking anything else makes it jump when it regains control.
  • Every decision is explainable: "loop -> 0 W, limited by maintenance (charge-only)" now appears in the UI and the log, instead of a bare number.
  • Safety limits (device rating, supervised max_w) bind every strategy including the highest, and are still enforced a second time at the point of writing.

0.1.6

Findings from installing this on a live system, replacing a working YAML implementation. Every one of these was silent - the add-on looked healthy while being completely unable to do its job.

  • run.sh must use #!/usr/bin/with-contenv sh. The HA base images run s6-overlay, which starts services with a SANITISED environment. With a plain shebang, SUPERVISOR_TOKEN is simply absent and every Home Assistant call returns 401 - while homeassistant_api: true makes the permissions look correctly granted. Startup now logs the token length and probes the Core API, so the next person sees it in one line.
  • Bump version: for every change. Supervisor keys the built image by version, so editing source and rebuilding silently reuses the old image. Two fixes appeared not to work because of this.
  • Dependencies come from apk, not pip. Alpine is musl and there are no musl wheels for aiohttp; pip would compile it on the client's Pi.
  • paho-mqtt 1.x and 2.x are both supported. Alpine ships 1.x, which has no CallbackAPIVersion; that raised and took the whole add-on down with it.
  • MQTT can no longer take down control. Publisher construction is wrapped - observability must never stop the controller.
  • MQTT discovery is published from on_connect. paho drops QoS-0 publishes issued before the CONNACK, so announcing straight after connect() published nothing at all while logging "MQTT connected".
  • Repeated failures log at most once a minute. The control loop retries every second; unthrottled warnings rolled the log buffer and destroyed the startup diagnostics needed to debug the 401 above.
  • auto_start works. The store's defaults supplied auto: False, so the fallback to the option could never fire.

Known issue: after deleting the MQTT entities from the registry during development, Home Assistant would not re-adopt them from retained discovery - not even after clearing the retained topics and reconnecting. The add-on publishes correct discovery and live state (verified on the broker); this is an HA-side adoption problem and affects status entities only, never control.

0.1.0

First packaged release. Ports the control loop and the monthly maintenance cycle from the reference Home Assistant implementation into an add-on.

  • Grid-following control: gain/slew/clamp/deadband with anti-windup, all tuned against measured hardware behaviour (see FIELD-GUIDE.md §14).
  • Saturation freeze with the duration term — three consecutive diverging cycles, not one. The instantaneous test fires on every large correction, because the plant itself needs 3-6 s to settle.
  • Monthly maintenance cycle as an ownership state machine: drain / charge / hold, with exactly one writer of the setpoint at any moment.
  • Capacity-tariff awareness: the maintenance charge is capped by quarter-hour peak headroom, and peak shaving outranks the maintenance schedule.
  • Failsafe behaviour: commands 0 W on missing inputs, on stop, and on shutdown. Never replays a stale setpoint - the reference implementation did, and the hardware watchdog cannot catch that.
  • Ingress UI with a commissioning checklist that names problems in words.
  • Optional MQTT discovery for status entities.

Known limits:

  • Home Assistant OS / Supervised only (add-ons cannot run on Container/Core).
  • The inverter protocol is reverse-engineered; no vendor contract.
  • Without the optional RS485 e-stop, nothing covers the host machine dying.