diff --git a/goodwe_controller/CHANGELOG.md b/goodwe_controller/CHANGELOG.md index c1fb4c0..612931c 100644 --- a/goodwe_controller/CHANGELOG.md +++ b/goodwe_controller/CHANGELOG.md @@ -2,6 +2,96 @@ ## Unreleased +**An operator can declare a second inverter, and the shape was measured against +the Supervisor rather than assumed (CFG-04).** DEV-02 made rating, SoC, +watchdogs, heartbeat and store keys per-instance, so the code no longer assumed +one inverter — but the options schema still described exactly one, and +`build_roster()` returned a list of length one and said so in its own docstring. +The roster could only ever be a roster of one. **Version bumped to 0.6.0, +because the Supervisor keys the built image by `version:` and a schema change +without a bump is a schema nobody reads.** + +* **`inverters:` — a list of objects, defaulting to `[]`.** Empty is not + "unconfigured", it is **the single-inverter configuration**: the flat + `setpoint_entity` / `soc_entity` / `batt_entity` / `batt_invert` / `max_w` + keys describe the one unit exactly as they always did. An install that + upgrades reads identically — same entities, same rating, same behaviour, no + operator action, and the whole existing check suite runs against it unchanged. +* **⚠️ The shape was ESTABLISHED, not assumed, and the empty default is the + load-bearing part.** There was no list-of-objects anywhere in `config.yaml` + and `static_profile` carries its structure as a JSON string in a `str?` + precisely because a nested schema was doubted. So Supervisor **2026.08.0**'s + own `AppOptions` validator — the version this site runs — was run against this + repository's `config.yaml` out of the release image + (`tools/supervisor_shape_probe.py`, committed; the transcript and the + reasoning are in `DECISIONS/CFG-04-inverter-roster.md`). It accepts the list, + enforces per-entry types, refuses a missing `name` / `setpoint_entity` / + `max_w` **by name**, and renders the option in the UI as a repeatable section. + And the shipped `options:` block validates as it stands while the **same + block with only `inverters` removed** is answered with **`Missing option + 'inverters' in root`** — one key varied, so the refusal is about that key — + which is every options blob written before this version, since the key did not + exist when they were written. `App.options` is + the shipped `options:` block deep-merged with the operator's stored one, so + `inverters: []` in `config.yaml` is the difference between an upgrade nobody + notices and an add-on that cannot start with a schema pinned to the version + that would fix it. That is `docs/deploy-rollback.md` §4's trap in the forward + direction, found before it was sprung rather than after. +* **Nothing is inherited by an entry that omits a key, and that is DEV-02 + restated as a config rule.** `name`, `setpoint_entity` and `max_w` are + required of every entry; a missing `soc_entity` means *this unit has no SoC + sensor*, never *use the other one's*. A three-line fallback to the flat keys + would have rebuilt the sibling-grading defect DEV-02 spent a card removing, in + YAML. Defaults still live in exactly one place (CFG-03): there is no + per-entry default anywhere, because there is no optional per-entry number. +* **SAFETY-28's `actuator_writers()` refuses two inverters naming one + `setpoint_entity` — the first configuration in this add-on's life in which + that was expressible.** The census was already structural and case-folding, so + it needed no change; what it needed was a config that could express the + hazard, and now there is one. Its sibling refusal is new: a flat + `setpoint_entity` left set beside a declared roster names an actuator nothing + will write, and is refused rather than ignored. +* **⚠️ Two PERMISSIVE READERS in shipped code, found and fixed.** Both failed in + the direction that hides a defect. (1) `validate.py` rules 5 and 6 compared + `sign_disagree_w` and `maintenance_charge_w` against the **flat** `max_w`; at + a roster of two 2000 W units with `max_w: 5000` left over from a + single-inverter install, `sign_disagree_w: 3000` passed rule 5 while + SAFETY-02's watchdog could fire on **neither** unit — rule 5's own failure + mode, reached through rule 5. They now read the roster's caps + (`unit_caps()`), which at one inverter is `[max_w]` exactly. (2) + `run_control()` re-ran the device-rating check behind `if + self.o.get("setpoint_entity")` — the flat key, which a roster config leaves + blank — so on a two-inverter site **no** inverter's `max_w` was ever graded + against the device it is written to. `validate_unit_ranges()` grades each unit + against its own ESPHome and runs unconditionally. +* **⚠️ Three config.yaml READERS IN THE CHECKS took the first match on a + stripped line, and one of them was already known.** `test_peakguard.py`'s + `read_option()` and `_schema_bound()` and `test_inverter.py`'s + `read_schema_max()` threw indentation away, so the nested per-inverter + `max_w:` began outranking the flat one — **silently, and correctly**, because + the two bounds are the same `int(100,5000)` today. A reader that is right by + coincidence is the failure mode this project is named after. All three are + anchored at exactly two spaces now, the halves of the file disambiguate + `peak_cap_w` instead of first-match, and an ambiguous key raises rather than + picking one. `test_tariff.py`'s copy of the idiom went with them. +* **The audit is a check, not a promise.** `test_inverter.py` part 11 finds + every `config.yaml`-reading function in the suite — transitively, so a reader + that never names the file because it delegates is still found — **executes** + each one against a synthetic two-inverter `config.yaml` whose per-inverter + numbers differ from its flat ones, and fails if any returns the per-inverter + value. The table is not maintained by hand: the signature decides, so the + eleventh reader is named on its first run. `config.yaml` declares `inverters:` + **before** `max_w:` on purpose, and part 11 asserts that ordering, so the trap + stays baited rather than sorted out by an ordering nobody would maintain. +* **Ten mutations run and observed red** (`tools/mutation_run.sh`), listed in + `test_inverter.py` part 12 with the check that bites each. **Two survived the + first run and the checks were changed, not the table:** the inheritance check + had blank flat keys, so "inherited" and "not inherited" were the same empty + string; and the `run_control()` check asserted a **source line** was present, + which re-indenting it does not remove. The second is now driven through the + shipped `run_control()` under a deadline, because that mutation's real symptom + is a hang rather than a red. + **Circulating power - two inverters pushing against each other - becomes a measured quantity, a bounded one, and an alarm that survives a restart (SAFETY-31).** Inverter A charging at 3 kW while inverter B discharges at 3 kW diff --git a/goodwe_controller/DECISIONS/CFG-04-inverter-roster.md b/goodwe_controller/DECISIONS/CFG-04-inverter-roster.md new file mode 100644 index 0000000..097f632 --- /dev/null +++ b/goodwe_controller/DECISIONS/CFG-04-inverter-roster.md @@ -0,0 +1,213 @@ +# CFG-04 — how a second inverter is declared, and why that shape was measured + +**Status: DECIDED 2026-09-04 — a list of objects, `inverters:`, with `[]` as +the shipped default.** The alternative on the table was the `static_profile` +precedent: a structured value carried as a JSON **string** in a `str?` option. +Both work. The list wins on three counts that were measured rather than argued, +and the empty default is not cosmetic — without it the upgrade breaks. + +Filed with CFG-04. Card 6a99fdf9d381935eddce4467. + +--- + +## 1. The question, and why it could not be answered from the documentation + +`config.yaml` used `list(off|ha_dsmr|…)` for enums and `str?` for free text, and +**there was no list-of-objects anywhere in the file**. `static_profile` +(`options:` `""`, `schema:` `str?`) carries a whole tariff structure as one JSON +line, and its own comment says why: *"a JSON string rather than a nested schema, +deliberately … it survives config.yaml's flat options block"*. That comment is +evidence that a nested schema was **doubted**, not that it was tried. + +So the question was not "is this expressible in YAML" but **"does the Supervisor +this site actually runs accept it"** — and the Supervisor pins the options schema +to the *installed* version (`docs/pickup.md`), so guessing wrong is not a +compile error, it is an operator whose add-on will not start after an update +(`docs/deploy-rollback.md` §4, in the forward direction). + +## 2. How it was answered + +`tools/supervisor_shape_probe.py`, committed beside this record, runs +**Supervisor's own `AppOptions` validator and its own `SCHEMA_APP_CONFIG`** out +of the release image — `ghcr.io/home-assistant/amd64-hassio-supervisor:2026.08.0` +— against **this repository's `config.yaml`**. 2026.08.0 is the version the live +install reports (`supervisor_version` in its own backup metadata, 2026-09-03, +alongside Home Assistant 2026.8.3). + +Nothing was re-implemented: if the Supervisor changes its rules, the script +changes its answer. Nothing was touched, either — it reads the file and runs a +validator in a throwaway container. It does **not** talk to the live Supervisor +and does not POST options, because the schema conflict is the one thing you do +not want to discover on a running site. + +### What it printed, 2026-09-04 + +⚠️ **This transcript was RE-RUN after review, against the same image tag, and +it is not the one this record first carried.** The first §B validated the +reconstructed live blob — a blob missing many required keys that have nothing +to do with this card — and quoted its refusal as evidence about `inverters`. +It never was: that refusal names whichever unrelated key the validator reaches +first, and it has printed `meter_freeze_s`, `p1_phase_export_entities` and +`sign_disagree_w` on three different runs. §B is now the isolated +counterfactual — the shipped `options:` block, and the same block with +`inverters` and nothing else removed — and it asserts on the KEY NAME in the +refusal. The mis-cited half is kept, relabelled, as §B2. See §4. + +``` +App 'GoodWe RS485 Controller' uses deprecated 'arch' values: ['armv7'] +Option 'inverters' does not exist in the schema for GoodWe RS485 Controller (local_goodwe_controller) +== A. the whole config.yaml through SCHEMA_APP_CONFIG == + PASS Supervisor accepts the add-on manifest, version 0.6.0 + schema declares 66 keys; inverters -> [{"name": "str", "setpoint_entity": "str", "max_w": "int(100,5000)", "soc_entity": "str?", "batt_entity": "str?", "batt_invert": "bool?"}] + options ship inverters -> [] + +== B. THE LOAD-BEARING ONE: an options blob with no `inverters` key == + PASS the shipped `options:` block validates AS IT STANDS - so anything refused on the next line is refused for the one key that was removed + PASS ...and the SAME block with ONLY `inverters` removed is REFUSED, BY NAME - which is every options blob written before 0.6.0, since the key did not exist when they were written + Missing option 'inverters' in root in GoodWe RS485 Controller (local_goodwe_controller) + +== B2. AC3: a reconstructed pre-0.6.0 blob upgrades into this version == + PASS the reconstructed raw blob is REFUSED - for an unrelated missing key, because the reconstruction is incomplete; see the refusal printed below and §B for the `inverters` measurement + Missing option 'sign_disagree_w' in root in GoodWe RS485 Controller (local_goodwe_controller) + PASS the same blob MERGED with these defaults - what App.options actually is + inverters=[] setpoint_entity='number.kelder_goodwe_master_stage1_goodwe_setpoint_w' max_w=5000 + +== C. AC2: a two-inverter blob an operator would POST == + PASS two inverters + [{"name": "inverter-a", "setpoint_entity": "number.goodwe_a_setpoint_w", "soc_entity": "sensor.goodwe_a_soc", "batt_entity": "sensor.goodwe_a_battery_power", "max_w": 5000}, {"name": "inverter-b", "setpoint_entity": "number.goodwe_b_setpoint_w", "soc_entity": "sensor.goodwe_b_soc", "batt_entity": "s + +== D. what the schema itself refuses, per entry == + PASS an entry with no name is refused + Missing option 'name' in inverters in GoodWe RS485 Controller (local_goodwe_controller) + PASS an entry with no setpoint_entity is refused + Missing option 'setpoint_entity' in inverters in GoodWe RS485 Controller (local_goodwe_controller) + PASS an entry with no max_w is refused + Missing option 'max_w' in inverters in GoodWe RS485 Controller (local_goodwe_controller) + PASS an entry with max_w over the range is refused + value must be at most 5000.0 + PASS a JSON STRING where the list belongs is refused + Invalid list for option 'inverters' in GoodWe RS485 Controller (local_goodwe_controller) + +== E. the ROLLBACK direction, and the operator's UI == + PASS a schema WITHOUT `inverters` drops the key with a warning rather than refusing - so a downgrade is not the §B trap in reverse + ['arbitrage_enabled', 'arbitrage_soc_ceiling', 'arbitrage_soc_floor', 'auto_start', 'batt_entity', 'batt_invert'] + PASS Supervisor's UI renderer turns it into a repeatable section rather than choking - an operator can add the second inverter in the Configuration tab, which the JSON-string alternative could not offer + [{"name": "inverters", "type": "schema", "optional": true, "multiple": true, "schema": [{"name": "name", "required": true, "type": "string"}, {"name": "setpoint_entity", "required": true, "type": "string"}, {"name": "max_w", "lengthMin": 100.0, "lengthMax": 5000.0, "required": true, "type": "integer + +== all cases passed == +``` + +*(`arch: armv7` deprecation is pre-existing and unrelated.)* + +## 3. What that establishes, item by item + +| Claim | Evidence | +|---|---| +| A list of objects is a legal `schema:` element | §A — the whole manifest passes `SCHEMA_APP_CONFIG`, whose `SCHEMA_ELEMENT` explicitly permits `[{str: Self}]` | +| Per-entry types are enforced | §D — `max_w: 9999` refused with *"value must be at most 5000.0"* | +| A missing required key is refused **by name** | §D — *"Missing option 'name' in inverters"* | +| A two-inverter blob validates | §C | +| The operator can edit it in the UI | §E — `UiOptions` renders `{"type": "schema", "multiple": true}` | +| The `inverters: []` default is what makes a pre-0.6.0 blob validate | §B — the shipped defaults **minus only that key** are refused with *"Missing option 'inverters' in root"*; unmodified they pass | +| A downgrade drops the key rather than refusing | §E — a schema without `inverters` **drops the key with a warning**; it is not the §B trap in reverse | + +## 4. ⚠️ The one that decided the default: `inverters: []` + +§B is the load-bearing measurement, and it is an **isolated counterfactual**: +exactly one key differs between its two runs. `supervisor/apps/options.py`'s +`_check_missing_options()` treats a nested list as **required** unless its +element is an optional scalar — a list whose element is a dict is never +optional. So the shipped `options:` block validates as it stands, and the same +block with **only** `inverters` removed is answered with: + +``` +Missing option 'inverters' in root in GoodWe RS485 Controller (local_goodwe_controller) +``` + +The live install's stored options have no such key, because it did not exist +when they were written. If `config.yaml` declared `inverters:` in `schema:` and +**not** in `options:`, then on update `App.test_update_schema()` would log *"new +schema is not compatible"* and the operator would be left with an add-on that +will not start — and the schema they would need to fix it with is pinned to the +version they cannot install. + +What makes it safe is that `App.options` is +`deepmerge(config.yaml's options:, the operator's persisted options)`, so a +shipped `[]` fills the gap before validation ever runs. §B shows the halves that +carry the claim — the defaults **minus this one key refused, by this key's +name**, and the unmodified defaults **accepted** — and §B2 shows the merge doing +its job on a reconstructed pre-0.6.0 blob. + +⚠️ **What §B2 does NOT show, and what this record used to claim it did.** §B2's +`LIVE` dict is an incomplete reconstruction: it carries the keys `HANDOVER.md` +and `docs/deploy-rollback.md` record and no others, so it is missing many +required keys unrelated to this card. Its RAW half is therefore refused for +whichever of *those* the validator reaches first — three separate runs have +printed `meter_freeze_s`, `p1_phase_export_entities` and `sign_disagree_w` — +and the earlier version of this section quoted that refusal as if it named +`inverters`. It never did. The string above was produced by §B, where the +`inverters` key is the only thing that varies. + +The probe's assertion on that line is on the **key name in the refusal**, not on +"a refusal happened". With the weaker assertion §B would have gone on printing +PASS after someone deleted `inverters: []` from `config.yaml` — blind to the one +regression it is the committed evidence for. It is now checked both ways: with +the default deleted the probe prints `2 CASE(S) FAILED` and exits 1; restored, +it prints `all cases passed` and exits 0. + +The merge strategy was checked too, because it is the other way this could have +gone wrong: `_OPTIONS_MERGER` is `type_strategies=[(dict, ["merge"])]` with an +`override` fallback, so **lists override rather than concatenate**. A shipped +one-element default plus an operator's two-element list would otherwise have +produced three entries — with inverter A declared twice, aimed at one actuator, +which is exactly the hazard SAFETY-28 refuses. `[]` makes that unreachable and +the override rule means even a non-empty default could not have grown a roster. + +## 5. Why the list, and not the `str?` + JSON precedent + +The JSON string works — §C of the earlier draft of this probe validated the same +roster as a `str?` — and if the list had failed it is what would have shipped. +It loses on three things: + +1. **SAFETY-28's writer census stops working.** `actuator_writers()` walks the + options **tree** looking for `setpoint_entity` at any depth, and its own + docstring anticipates this card: *"Add CFG-04's per-inverter config — a list + of inverters, each with its own `setpoint_entity` — and there are two real + writers"*. Inside a JSON **string** those entities are invisible to it, so + the §1.5 two-writers guarantee would have had to be re-implemented against a + parsed copy — a second place for the census to be wrong. +2. **No per-field validation.** With `str?` the Supervisor sees one opaque + string: a typo'd rating, a missing name and a malformed entity id all arrive + at the add-on identically, and every error message has to be written here. + §D shows the Supervisor doing that work, in its own words, before the add-on + starts. +3. **No UI.** §E renders a repeatable section; a `str?` renders a text box into + which an operator hand-types JSON. `static_profile` accepts that cost because + §14 gives it exactly one key and its structure is a tariff document; a roster + is a list of devices an operator adds one at a time. + +## 6. What is deliberately NOT in the shape + +- **No per-entry defaults.** `name`, `setpoint_entity` and `max_w` are + **required**; the sensors are optional and their absence means *this unit has + no such sensor*, never *use the other one's*. CFG-03 keeps defaults in exactly + one place, and a per-entry default would be a second one — worse, it would be + a per-entry default whose value is another inverter's measurement, which is + the sibling-grading defect DEV-02 spent a card removing. +- **No `gain`, `slew_w`, `integrator_max_w` per entry.** §6.2 and ALLOC-01 are + ONE integrator on P1 above the split. `app/inverter.py`'s module docstring + carries that argument; the config must not make a second one expressible. +- **No `inverters` entry for the flat keys.** They ARE the single-inverter + shape. Declaring both is refused (rule 9e) rather than merged, because a + config that silently becomes a different config is worse than one that + refuses to load. + +## 7. Revisit triggers + +- **A Supervisor upgrade that changes `AppOptions`.** Re-run the probe with the + new version tag. The whole point of committing it is that this costs a minute. +- **SPIKE-04 measuring inverter B.** It supplies a real `max_w` for the second + entry. It does not change the shape. +- **A second `Maintenance` FSM.** `validate.py` rule 6 grades + `maintenance_charge_w` against the first entry of `unit_caps_indexed()` + because `Controller.maint_unit` is `inverters[0]`. When that stops being true, that line moves with it. diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md index 5ac4ac9..99547f3 100644 --- a/goodwe_controller/DOCS.md +++ b/goodwe_controller/DOCS.md @@ -55,6 +55,82 @@ Use the ESP32's readings rather than the inverter's cloud or dongle sensors: those serve cached values, and a stale reading here ends the maintenance charge phase having charged nothing. +### More than one inverter (`inverters`) + +The six keys above describe **one** inverter, and if that is your site there is +nothing to do here — `inverters` ships empty and empty means *"the flat keys +above describe the one inverter"*. **An install that upgrades into 0.6.0 reads +identically to before: same entities, same rating, same behaviour, no operator +action.** + +To run two, declare them. Each entry names its own actuator, its own rating and +its own sensors: + +```yaml +setpoint_entity: "" # ⚠️ must be cleared — see below +inverters: + - name: inverter-a + setpoint_entity: number.goodwe_a_setpoint_w + soc_entity: sensor.goodwe_a_soc + batt_entity: sensor.goodwe_a_battery_power + max_w: 5000 + - name: inverter-b + setpoint_entity: number.goodwe_b_setpoint_w + soc_entity: sensor.goodwe_b_soc + batt_entity: sensor.goodwe_b_battery_power + max_w: 3000 +``` + +| per-entry key | required | meaning | +|---|---|---| +| `name` | yes | Short identifier. It namespaces this unit's SAFETY-02 latch, heartbeat and write bookkeeping in the add-on's state file, so **two units may not share one** | +| `setpoint_entity` | yes | This inverter's own ESPHome `number.*_goodwe_setpoint_w` | +| `max_w` | yes | The supervised cap on what may be written to **this** inverter. The site's ceiling is the **sum** | +| `soc_entity` | | This pack's state of charge | +| `batt_entity` | | This unit's battery power, `+` = discharging | +| `batt_invert` | | Flip if needed | + +⚠️ **Nothing is inherited.** An entry that leaves out `soc_entity` has **no SoC +sensor** — it does *not* fall back to the flat one. That is deliberate: every +per-device value became per-instance in 0.5.x precisely so that no inverter is +ever graded against a sibling's numbers, and a silent fallback would rebuild +that in YAML. `name`, `setpoint_entity` and `max_w` are required of every entry +for the same reason: there is no rating to borrow. + +⚠️ **Clear the flat `setpoint_entity` when you declare a roster,** or the add-on +refuses to start and says so. It is the single-inverter form and it is not read +once a roster exists, so leaving it set means one of two things: it names an +entity an entry also names — two writers on one actuator, which specs §1.5 makes +a non-goal and the add-on refuses outright — or it names a different one, which +nothing would ever write, leaving a site running quietly smaller than its +configuration reads. `soc_entity`, `batt_entity`, `batt_invert` and `max_w` +cannot be blanked (the schema gives them no empty value); they are simply not +read, and the add-on writes one line into its action log at every start saying +so. + +⚠️ **Two inverters naming the same `setpoint_entity` are refused,** whichever +keys they are declared under and whatever the capitalisation. Two independent +setpoints on one inverter is the arbitration problem this add-on exists not to +have. + +⚠️ **Update the add-on first, then add the roster.** Home Assistant validates the +options you save against the schema of the version that is *installed*, so +`inverters` cannot be set before 0.6.0 is installed. The order is: update, then +configure. + +⚠️ **Going the other way — downgrading with a roster configured — is recoverable +but NOT uneventful, and the add-on will not start until you refill a field.** +The *schema* side is fine: the older version's schema has no `inverters` key, so +it drops the key with a warning rather than refusing the whole blob (measured — +`DECISIONS/CFG-04-inverter-roster.md` §E). The *add-on* side is not. Rule 9(e) +requires the flat `setpoint_entity` be **cleared** when a roster is declared, so +a config that downgrades cleanly is by construction a config whose flat +actuator is blank — and the older version has nothing else to write to. It stops +at startup validation with a refusal naming `setpoint_entity`, which is loud and +in the log rather than silent, but it is an outage until someone puts the +single-inverter entity back in that field. **Before downgrading, write down the +`setpoint_entity` of the unit you want to keep running.** + ### P1 meter ingestion `meter_entity` above expects one signed sensor, which usually means a template @@ -272,7 +348,7 @@ behind it would trip the firmware watchdog on a system that is working fine. | option | default | meaning | |---|---|---| -| `max_w` | 2000 | Hard limit on what may be commanded. Start low, raise after commissioning | +| `max_w` | 2000 | Hard limit on what may be commanded. Start low, raise after commissioning. **Not read when `inverters` is declared** — each entry carries its own, and the site ceiling is their sum | | `gain` | 0.6 | Correction per cycle. **At the limit — do not raise** | | `slew_w` | 1000 | Maximum change per cycle | | `deadband_w` | 15 | Ignore errors smaller than this | diff --git a/goodwe_controller/MUTATION.md b/goodwe_controller/MUTATION.md index 9a11757..13dad66 100644 --- a/goodwe_controller/MUTATION.md +++ b/goodwe_controller/MUTATION.md @@ -135,8 +135,11 @@ manufactures them from a **fixture**: the check runs, the assertion is evaluated, the sentence above it is true — and the numbers it compares are arranged so that the failure it describes could not have changed the answer. -It is not hypothetical and it is not rare. Three instances in one week, all -found by review rather than by a red line: +It is not hypothetical and it is not rare. Six instances in two weeks; three +were found by review, two - CFG-04's fixture pair - by an author's own mutation +battery, which is the cheaper place, and the sixth by review again, in the one +place nobody had thought to point the rule at: not a check in the suite, but a +COMMITTED EVIDENCE ARTIFACT four documents cite. * **ARB-03** shipped a call-site scan asserting that every `claims.append` in `cycle()` routes through `actuator_of()`. Its own parse **saw 3 of the 8 @@ -150,6 +153,40 @@ found by review rather than by a red line: B's rating to A's, so it compared `(-5000, 5000)` against `(-5000, 5000)` — **a number against itself**. Filing both per-unit bounds from `self.inverters[0]` left **all 19 files green**. +* **CFG-04** wrote *"an entry that omits `soc_entity` has NO SoC sensor and does + not borrow the flat one"* against a fixture whose **flat `soc_entity` was + blank**. Inherited and not-inherited were both `""`, so the mutation that + makes `build_roster()` fall back to the flat key survived. The fixture now + sets a real entity there, and the check asserts that the flat value is present + as well as that the entry's is empty. +* **CFG-04** also wrote *"main.py runs the per-unit range check + UNCONDITIONALLY"* as a **substring test over `app/main.py`'s source**. The + mutation that defeats it puts the call back inside `if entity:` — one level of + indentation — and the substring is still there. That is habit 4 below in a new + costume: the check asserted against the TEXT of the thing under test rather + than against its behaviour. It now drives the shipped `run_control()` on a + two-inverter config, **under a deadline**, because the mutation's real symptom + is that the controller enters its loop and never returns. +* **CFG-04's `tools/supervisor_shape_probe.py` §B** — and this one is the rule + reaching outside the suite. The probe is the committed evidence that + `config.yaml`'s `inverters: []` default is load-bearing, and four documents + quoted it as answering *"Missing option 'inverters' in root"*. §B validated + the reconstructed LIVE options blob, which is missing many required keys + unrelated to this card, so its refusal named whichever of THOSE the validator + reached first — `meter_freeze_s`, `p1_phase_export_entities` and + `sign_disagree_w` on three different runs. The quoted string was never + produced by the instrument. And the assertion was a bare **`not ok`**: delete + `inverters: []` from `config.yaml` and §B went on printing **PASS**, blind to + the exact regression it was cited for. §B is now an isolated counterfactual — + the shipped defaults, and the same defaults minus that one key — asserting on + the **key name in the refusal**; the mis-cited blob survives, relabelled, as + §B2. Re-run with the default deleted it prints `2 CASE(S) FAILED` and exits 1. + + ⚠️ **A committed transcript is a check, and it decays like one.** Nothing in + this document had been pointed at `tools/`, because the artifacts there are + filed as *measurements* rather than as *checks* — and a measurement whose + assertion cannot fail is exactly the thing this section is about. Two habits + follow, and both are habit 5 and 6 below. Note what these have in common with a masked rung: the mutation *runs*, and the suite is *honestly* green. Nothing in a purge, a fingerprint or a disassembly @@ -162,7 +199,7 @@ check was handed. > **Any check asserting "X differs from Y" must be run once with X and Y > actually made different.** -Three cheap habits fall out of it, and all three are now used in this repository: +Cheap habits fall out of it, and all of them are now used in this repository: 1. **Write the PRECONDITION as its own check.** `test_peakguard.py` 10g asserts that A and B have different ratings *before* it asserts that B's bound is @@ -177,6 +214,34 @@ Three cheap habits fall out of it, and all three are now used in this repository actuator: {'inverter-a': (-5000, 5000), 'inverter-b': (-5000, 5000)}` is self-evidently not evidence of a difference. +4. **Assert on the BEHAVIOUR, not on the source text of the thing under test.** + A substring test over a module is a check on how the code is spelled; + whitespace, an alias or a moved line defeats it while the property it names + is gone. `test_inverter.py` part 10 has one of each — a source pin for a + claim that genuinely IS about the code's shape, and a driven `run_control()` + for a claim about behaviour — and the second replaced the first after the + mutation battery walked straight through it. +5. **A check whose failure mode is a HANG needs a deadline.** Otherwise the + mutation that defeats it produces a CI timeout with no name attached, which + reads like flaky infrastructure and gets retried rather than read. +6. **Assert on the CONTENT of a refusal, not on the fact that one happened.** + `not ok` is satisfied by any error at all, including one about a different + key on a different blob. `"Missing option 'inverters' in root" in out` goes + red the moment the refusal stops being about the thing it is cited for. This + is habit 2 in the negative direction: a refusal, like a difference, has to be + asserted against a literal. +7. **Vary ONE thing in a counterfactual, and vary it from an ACCEPTED + baseline.** `supervisor_shape_probe.py` §B validates the shipped defaults + first, precisely so the refusal on the next line has nowhere else to come + from. Without the accepted baseline the second line is a refusal of unknown + provenance, which is what §B used to print. +8. **A committed transcript is a check.** Anything under `tools/` that a + document quotes is subject to every rule above, and to the mutation battery: + break the property, re-run the instrument, and confirm the transcript goes + red. `inverters: []` was deleted from `config.yaml` and the probe re-run + against `ghcr.io/home-assistant/amd64-hassio-supervisor:2026.08.0` for + exactly that reason. + And the mutation-table consequence: **a variant is not covered because a near-miss of it is.** DEV-02's table listed the *actuator* variant of the per-unit claim defect (`device`/`supervised` filed without `inv`), which the diff --git a/goodwe_controller/app/inverter.py b/goodwe_controller/app/inverter.py index 589b97d..1d0babe 100644 --- a/goodwe_controller/app/inverter.py +++ b/goodwe_controller/app/inverter.py @@ -134,10 +134,17 @@ from .watchdog import SignWatchdog, SignWatchdogConfig # constant the firmware and validate.py already share. LINK_STALE_S = 30.0 -# The one inverter this add-on drives today. ⚠️ A NAME, NOT A CONFIGURATION - -# it exists so that a per-inverter claim has an actuator to be filed ON, which -# is what makes the site/unit split something a check can grade rather than a -# comment. It lives here rather than in main.py now that the roster does. +# The name of the inverter a SINGLE-INVERTER config describes. ⚠️ A NAME, NOT A +# CONFIGURATION - it exists so that a per-inverter claim has an actuator to be +# filed ON, which is what makes the site/unit split something a check can grade +# rather than a comment. It lives here rather than in main.py now that the +# roster does. +# +# ⚠️ CFG-04 DID NOT MAKE THIS "THE FIRST ONE". It is the name the flat-keys path +# gives its one unit, and nothing else; a declared `inverters:` roster names its +# own entries and this constant is not consulted. The store migration +# (`migrate_unit_keys`) still targets it, because the flat pre-DEV-02 keys can +# only ever have belonged to a single-inverter install. UNIT_A = "inverter-a" # The per-unit keys this object keeps in the Store, namespaced by unit name. @@ -308,27 +315,104 @@ class Inverter: f"max_w={self.max_w:.0f} dev=({self.dev_min}, {self.dev_max})>") -def build_roster(opts: dict, store, sign_cfg=None) -> list: - """The site's inverters, in roster order. TODAY THAT IS EXACTLY ONE. +def roster_entries(opts: dict) -> list: + """The `inverters:` option, normalised to a list of dicts. Never None. - ⚠️ INVERTER B DOES NOT EXIST YET (~2 weeks out) AND THIS FUNCTION MUST NOT - INVENT IT. There is no `inverters:` list in config.yaml, no second - `setpoint_entity`, and no measured rating, ramp or lag for a second device - - SPIKE-04 measures those and CFG-04 shapes the option that carries them. What - this card delivers is that the CODE no longer assumes there is one; adding - the second entry is a config change to THIS FUNCTION and to nothing else, - which is the point. - - So: unit A is built from the flat options, exactly as the shipped scalars - were, and the flat options keep their names. An install that upgrades reads - identically - same entities, same rating, same behaviour - which is AC 8. + Separate from `build_roster()` because validate.py needs to grade the SAME + list this builds from, and a second transcription of "what counts as an + entry" is a second answer to it. A non-list, or a list with non-dict + members, comes back as it was so the validator can name what it found - + dropping the junk here would hand the validator a clean list and a config + nobody refused. """ - return [Inverter( - UNIT_A, - setpoint_entity=opts.get("setpoint_entity", ""), - soc_entity=opts.get("soc_entity", ""), - batt_entity=opts.get("batt_entity", ""), - batt_invert=bool(opts.get("batt_invert")), - max_w=float(opts.get("max_w", 2000)), - store=store, sign_cfg=sign_cfg)] + entries = opts.get("inverters") + if entries is None or entries == "": + return [] + if not isinstance(entries, (list, tuple)): + return [entries] + return list(entries) + +def build_roster(opts: dict, store, sign_cfg=None) -> list: + """The site's inverters, in roster order. + + TWO SHAPES, AND WHICH ONE IS IN FORCE IS DECIDED BY ONE TEST: is + `inverters:` empty? + + EMPTY the five flat keys - `setpoint_entity`, `soc_entity`, + `batt_entity`, `batt_invert`, `max_w` - describe the ONE inverter, + exactly as the shipped scalars did. This is what every install + that upgrades into this version reads, without an operator + touching anything, and it is byte-identical to what DEV-02 + shipped. config.yaml's default is `[]` for precisely that reason, + and Supervisor 2026.08.0 refuses the whole options blob without + it - see the options-block note on `inverters:`. + + NON-EMPTY each entry describes one inverter and the flat keys describe + none of them. ⚠️ NOTHING IS INHERITED. An entry that omits + `soc_entity` has no SoC sensor; it does NOT borrow the flat one, + because "this unit's number came from somewhere else" is the + defect DEV-02 spent a card removing and a fallback here would + rebuild it in three lines. `name`, `setpoint_entity` and `max_w` + are required of every entry - the schema enforces it on the + Supervisor path, `validate.py` rule 9 enforces it for sim/inject.py + and the checks, which reach this function with any dict at all. + + ⚠️ THIS FUNCTION REFUSES NOTHING, AND THAT IS ON PURPOSE. A refusal here + happens inside `Controller.__init__`, before the startup gate has run and + before anything can render the reason to an operator; validate.py's rule 9 + is the place that says no, with a message, through `refuse()`. So this + builds the most honest roster it can out of whatever it was handed - a + missing name becomes a positional one, a missing rating becomes 0.0 W - and + every one of those states is something rule 9 has already refused by the + time a control loop exists. Keep the two in step: a shape this tolerates + that rule 9 does not name is a shape that runs. + """ + entries = roster_entries(opts) + if not entries: + # ⚠️ THE UNCHANGED PATH. Same name, same entities, same rating, same + # store keys as before CFG-04 - an install that upgrades reads + # identically, which is AC 3. + return [Inverter( + UNIT_A, + setpoint_entity=opts.get("setpoint_entity", ""), + soc_entity=opts.get("soc_entity", ""), + batt_entity=opts.get("batt_entity", ""), + batt_invert=bool(opts.get("batt_invert")), + max_w=float(opts.get("max_w", 2000)), + store=store, sign_cfg=sign_cfg)] + + roster = [] + for i, entry in enumerate(entries): + entry = entry if isinstance(entry, dict) else {} + # ⚠️ THE FALLBACK NAME IS POSITIONAL, NOT `UNIT_A`. Two nameless entries + # both called `inverter-a` would share one Store namespace, so one + # unit's SAFETY-02 latch would be the other's - a fault on B latching A + # is the exact failure DEV-02's namespaced keys removed. Rule 9 refuses + # a nameless entry outright; this only decides what the refusal is + # allowed to call it in the meantime. + name = str(entry.get("name") or "").strip() or f"inverter-{i + 1}" + roster.append(Inverter( + name, + setpoint_entity=entry.get("setpoint_entity", ""), + soc_entity=entry.get("soc_entity", ""), + batt_entity=entry.get("batt_entity", ""), + batt_invert=bool(entry.get("batt_invert")), + # ⚠️ NOT `opts["max_w"]`. See the docstring: a rating an entry did + # not state is not the flat one, it is a rating rule 9 refuses. + max_w=_entry_max_w(entry), + store=store, sign_cfg=sign_cfg)) + return roster + + +def _entry_max_w(entry: dict) -> float: + """One entry's supervised cap, or 0.0 when it did not state a usable one. + + 0.0 rather than a borrowed number, and rather than an exception: rule 9 + refuses `max_w` missing or unparseable by name, and a roster that cannot be + built at all would refuse it with a traceback instead. + """ + try: + return float(entry.get("max_w")) + except (TypeError, ValueError): + return 0.0 diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index a9e90d8..334af98 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -52,7 +52,8 @@ from .hass import HomeAssistant # ⚠️ `LINK_STALE_S` IS RE-EXPORTED DELIBERATELY. `live_units()` reasons about it # and two checks read `app.main.LINK_STALE_S` to build a stale link, so the # window an operator would look up here is the one the roster actually uses. -from .inverter import LINK_STALE_S, UNIT_A, build_roster +from .inverter import (LINK_STALE_S, UNIT_A, build_roster, + roster_entries) from .maintenance import IDLE, MaintConfig, Maintenance from .mqtt import MqttPublisher from . import peakguard @@ -61,7 +62,8 @@ from .price import Uncertainty, price_option from .static_tariff import resolve_tariff, static_status from .tariff import PeakStore, PeakTracker from .store import migrate_unit_keys -from .validate import ConfigRefused, refuse, validate_startup_config +from .validate import (ConfigRefused, refuse, validate_startup_config, + validate_unit_ranges) from .watchdog import SignWatchdogConfig, loop_is_following from . import web @@ -177,6 +179,24 @@ class Controller: ) self.store_migration = migrate_unit_keys(store, UNIT_A) self.inverters = build_roster(opts, store, sign_cfg) + # ⚠️ CFG-04. SAY WHICH SHAPE IS IN FORCE, BECAUSE THE OTHER ONE'S KEYS + # ARE STILL SITTING IN THE UI. A declared `inverters:` roster reads none + # of `soc_entity`, `batt_entity`, `batt_invert` or `max_w`, and an + # operator looking at the Configuration page sees all four with values + # in them. validate.py rule 9(e) refuses the one that is genuinely + # dangerous - a flat `setpoint_entity` naming an actuator nothing will + # write - and the rest cannot be refused (the schema gives them no blank + # value), so they are DISCLOSED instead: one line in §15's action log, + # at every start, naming the roster it built and the keys it ignored. + # Silence here is how "the site is running at half the rating I typed" + # takes a week to notice. + if roster_entries(opts): + self.log_event( + "roster: %d inverters from `inverters:` - %s. The flat " + "soc_entity / batt_entity / batt_invert / max_w keys are NOT " + "read in this shape; each entry carries its own." + % (len(self.inverters), + ", ".join(f"{i.name} {i.max_w:.0f} W" for i in self.inverters))) self.tuning = Tuning( gain=float(opts.get("gain", 0.6)), @@ -2386,37 +2406,47 @@ class Controller: inv.setpoint_entity) await asyncio.gather(*(_limits(inv) for inv in self.inverters)) entity = self.o.get("setpoint_entity", "") + # ⚠️ THIS USED TO WARN AND CARRY ON, and that was the bug CFG-01 + # exists to remove: a max_w above the device's rating meant + # write_setpoint() silently clamped and the configured ceiling was + # never the real one. + # + # amain() already ran this gate before anything started. It runs + # AGAIN here for one case the gate cannot cover: ESPHome is often + # not up yet when the add-on boots, so hass.limits() returned + # (None, None) and the range checks were skipped rather than + # guessed at. This is the first moment the device's real rating is + # known. If it conflicts, refuse now - stopping deterministically + # through the normal shutdown, which commands 0 W on the way out. + # + # ⚠️⚠️ AND THE `if entity:` THAT USED TO WRAP THE WHOLE BLOCK WAS A + # READER THAT FAILED PERMISSIVE - CFG-04's own hazard, in this file. + # `entity` is the FLAT `setpoint_entity`, which a per-inverter config + # leaves blank (validate rule 9(e) requires it), so on the very config + # this add-on now supports the entire re-check was SKIPPED: no + # inverter's `max_w` was ever graded against the device it is written + # to, and the skip was silent. The flat gate still guards the flat + # rules, because they read flat options and one device's range; the + # per-unit rule runs unconditionally, off the roster, and grades each + # unit against ITS OWN ESPHome. At one inverter `validate_unit_ranges()` + # skips the unit the flat call already graded, so nothing is reported + # twice and the N=1 path is unchanged. + problems = [] if entity: - # ⚠️ THIS USED TO WARN AND CARRY ON, and that was the bug CFG-01 - # exists to remove: a max_w above the device's rating meant - # write_setpoint() silently clamped and the configured ceiling was - # never the real one. - # - # amain() already ran this gate before anything started. It runs - # AGAIN here for one case the gate cannot cover: ESPHome is often - # not up yet when the add-on boots, so hass.limits() returned - # (None, None) and the range checks were skipped rather than - # guessed at. This is the first moment the device's real rating is - # known. If it conflicts, refuse now - stopping deterministically - # through the normal shutdown, which commands 0 W on the way out. - # ⚠️ GRADED AGAINST THE FIRST UNIT'S RANGE, because `validate.py` - # grades the FLAT options - `max_w`, `setpoint_entity` - which are - # unit A's. A per-unit validator arrives with CFG-04's per-unit - # options; inventing one here would be this file asserting a schema - # for a config that does not exist yet. _first = self.inverters[0] - problems = validate_startup_config(self.o, _first.dev_min, - _first.dev_max) - if problems: - refuse(problems) - self.refused = problems - self.log_event("REFUSED: " + problems[0]) - # Signal, then return WITHOUT entering the loop. amain() wakes - # on this, runs the normal wind-down (which commands 0 W) and - # re-raises ConfigRefused afterwards, so the exit code is 78 and - # the inverter is left at zero rather than latched. - self.stop_event.set() - return + problems += validate_startup_config(self.o, _first.dev_min, + _first.dev_max) + problems += validate_unit_ranges(self.inverters, skip_entity=entity) + if problems: + refuse(problems) + self.refused = problems + self.log_event("REFUSED: " + problems[0]) + # Signal, then return WITHOUT entering the loop. amain() wakes + # on this, runs the normal wind-down (which commands 0 W) and + # re-raises ConfigRefused afterwards, so the exit code is 78 and + # the inverter is left at zero rather than latched. + self.stop_event.set() + return last_grid = object() heartbeat = float(self.o.get("heartbeat_s", 10)) diff --git a/goodwe_controller/app/validate.py b/goodwe_controller/app/validate.py index f76a819..13d2e9a 100644 --- a/goodwe_controller/app/validate.py +++ b/goodwe_controller/app/validate.py @@ -150,6 +150,32 @@ NOT READY banner already names the problem, (b) does not apply. permits equality because equality there leaves one reachable point that works; here it leaves none. + 9. An `inverters:` roster that cannot mean what it says. [(a) and (b)] + CFG-04's five cases are enumerated on `_roster_problems()`. Three of them + are (b) - an entry with no `setpoint_entity`, or no rating, is a unit that + takes a share of the site target every cycle and delivers none of it, on a + status page that reports a site running at full strength. Two are (a): two + entries sharing a name share one Store namespace, so one unit's SAFETY-02 + latch IS the other's and an operator clearing one clears both; and a flat + `setpoint_entity` left set beside a declared roster is an actuator the + configuration names and nothing writes. + + ⚠️ RULES 5 AND 6 CHANGED SHAPE FOR THIS, and that is the part worth + reading. Both compared a threshold against the flat `max_w`, which was + right while there was exactly one inverter and is a PERMISSIVE READER the + moment there are two: `max_w: 5000` left over from a single-inverter + install, beside a roster of two 2000 W units, passed rule 5 at + `sign_disagree_w: 3000` while the watchdog could fire on NEITHER unit - + rule 5's own failure mode, reached through rule 5. They now read + `unit_caps()`, which is the roster's caps and, at one inverter, is + `[max_w]` exactly. + + What rule 9 deliberately does NOT do is grade a rating against a device. + `dev_min`/`dev_max` here are one device's range and there are now N of + them, none of which has answered at startup. `validate_unit_ranges()` is + the per-unit form and main.py runs it from `run_control()`, once every + unit's own ESPHome has replied. + WHAT IS DELIBERATELY *NOT* REFUSED, because it is untidy rather than unsafe: - `integrator_max_w` above `max_w`. DOCS.md is right that the only thing the @@ -202,6 +228,7 @@ WHAT IS DELIBERATELY *NOT* REFUSED, because it is untidy rather than unsafe: import logging from .arbitrage import enabled as arbitrage_on +from .inverter import roster_entries from .maintenance import MAINT_OPTION_DEFAULTS from .p1 import is_enabled, validate_meter_options from .peakguard import PEAK_OPTION_DEFAULTS @@ -307,6 +334,229 @@ def actuator_writers(opts: dict) -> dict: return census +def unit_caps(opts: dict) -> list[float]: + """Each configured inverter's supervised cap, in roster order. + + ⚠️ WHY THIS EXISTS AT ALL, AND IT IS THE PERMISSIVE-READER PROBLEM IN THIS + FILE. Rules 5 and 6 both compare a threshold against "max_w", and both of + them are really comparing it against WHAT ONE UNIT MAY BE COMMANDED - the + arbiter files `supervised` per inverter, from `inv.max_w`, and there is no + such thing as a site rating on the wire. At one inverter those are the same + number and reading the flat option was right. At two they are not: a config + with `max_w: 5000` left over from a single-inverter install and a roster of + two 2000 W units would pass rule 5 at `sign_disagree_w: 3000` while the + watchdog could not fire on EITHER unit - the disarmed-guard-reporting-armed + state those rules exist to refuse, reached through the rule itself. + + So the caps come from the roster the add-on will actually build + (`inverter.roster_entries()`, not a second reading of the option), and fall + back to the flat `max_w` only where that roster is the flat one too. + """ + return [cap for _, cap in unit_caps_indexed(opts)] + + +def unit_caps_indexed(opts: dict) -> list[tuple[int, float]]: + """`unit_caps()`, but each cap carries the ROSTER INDEX it came from. + + ⚠️ WHY THE INDEX HAS TO TRAVEL WITH THE NUMBER. This function SKIPS entries + with no usable `max_w`, so position i in the caps list is not position i in + the roster the moment one entry is malformed. Rules 5 and 6 used to feed a + caps-list index straight into `_unit_label()`, which indexes `entries` - so + on a roster whose first entry has a junk `max_w`, the refusal named the + WRONG INVERTER. An operator told to lower a number on `inverter-a` when the + number is on `inverter-b` is being sent to the wrong device by the very + message that exists to stop that. Rule 9(d) already refuses such a config so + the two lists cannot desynchronise in shipped use - but "unreachable" is a + claim about today's rule ordering, not a property of this function, and + `validate_startup_config()` is also `sim/inject.py`'s entry point. + """ + entries = roster_entries(opts) + if not entries: + cap = _number(opts, "max_w", 2000.0) + return [] if cap is None else [(0, cap)] + caps: list[tuple[int, float]] = [] + for index, entry in enumerate(entries): + if not isinstance(entry, dict): + continue + cap = _number(entry, "max_w", None) + if cap is not None: + caps.append((index, cap)) + return caps + + +def _unit_label(opts: dict, index: int) -> str: + """"" at a single-inverter config, " (inverter-b)" at a roster. + + The messages below have to read identically on the install that upgrades + into this version - it is the same configuration, refused for the same + reason - and have to NAME THE UNIT the moment there is more than one, or an + operator with three inverters is told a number is wrong and not which one. + """ + entries = roster_entries(opts) + if len(entries) <= 1: + return "" + entry = entries[index] if index < len(entries) else {} + name = "" + if isinstance(entry, dict): + name = str(entry.get("name") or "").strip() + return f" ({name or f'inverters[{index}]'})" + + +def _roster_problems(opts: dict) -> list[str]: + """Rule 9 (CFG-04): the `inverters:` roster, refused where it cannot mean + what it says. + + ⚠️ THE SUPERVISOR ALREADY ENFORCES MOST OF THIS AND THAT IS NOT A REASON TO + DROP IT. `config.yaml`'s schema makes `name`, `setpoint_entity` and `max_w` + required per entry, and Supervisor 2026.08.0 refuses a blob that omits one + by name - measured, not assumed. But `validate_startup_config()` is also + `sim/inject.py`'s entry point and every check's, and those reach it with a + dict the Supervisor never saw; the same argument `price_provider` makes two + rules up. A rule the Supervisor duplicates costs nothing and covers the + callers it does not. + + Five things are refused, and each is a config that would otherwise RUN: + + a. `inverters` is not a list. `roster_entries()` wraps a scalar so this + can name it; without the rule the roster would be one inverter built + out of a string's `.get`, i.e. an immediate AttributeError. + b. An entry with no usable `name`. Store keys are namespaced BY NAME + (`store.unit_key`), so two nameless entries would share one namespace + and one unit's SAFETY-02 latch would silently be the other's. + c. Two entries with the SAME name - (b) reached the long way round. + d. An entry with no `setpoint_entity`, or no usable `max_w`. The first is + a unit this controller cannot command; the second is a unit whose + supervised claim would be `+-0 W`, which is a unit pinned at zero by a + missing key rather than by a decision. + e. A flat `setpoint_entity` alongside a declared roster. The flat keys are + the SINGLE-inverter shape; leaving one set while a roster is declared + means one of two things and neither is safe. If it names an entity an + entry also names, SAFETY-28's census below already refuses it as two + writers on one actuator. If it names a DIFFERENT one, nothing writes + it, the site runs quietly smaller than the configuration reads, and the + one place it would show up is a status page nobody compares against the + config. Refused rather than ignored, for §1.5's reason and for this + card's: a config that silently becomes a different config is worse than + one that refuses to load. + """ + problems: list[str] = [] + raw = opts.get("inverters") + if raw is None or raw == "" or raw == []: + return problems + if not isinstance(raw, (list, tuple)): + return [f"inverters is {type(raw).__name__} {raw!r}, and it must be a " + "LIST of inverter objects - one per inverter, each naming its " + "own name, setpoint_entity and max_w. A single inverter needs " + "no list at all: leave `inverters` empty and the flat " + "setpoint_entity/soc_entity/batt_entity/max_w keys describe it."] + + entries = roster_entries(opts) + seen: dict = {} + for i, entry in enumerate(entries): + where = f"inverters[{i}]" + if not isinstance(entry, dict): + problems.append( + f"{where} is {type(entry).__name__} {entry!r}, and every entry " + "must be an object with at least name, setpoint_entity and " + "max_w.") + continue + name = str(entry.get("name") or "").strip() + if not name: + problems.append( + f"{where} has no name. Every inverter needs one: the Store " + "namespaces this unit's SAFETY-02 latch, its heartbeat and its " + "write bookkeeping under it, so two unnamed units would share " + "one namespace and a trip caused by one would latch the other. " + "Any short identifier works - inverter-a, inverter-b, garage.") + elif name in seen: + problems.append( + f"{where} and {seen[name]} are both called {name!r}. The Store " + "keys every per-unit latch under the unit's name, so two units " + "with one name share one SAFETY-02 latch and one heartbeat " + "deadline - a trip caused by either would stop both, and an " + "operator clearing it would clear both. Give them different " + "names.") + else: + seen[name] = where + who = f"{where} ({name})" if name else where + if not str(entry.get("setpoint_entity") or "").strip(): + problems.append( + f"{who} has no setpoint_entity, so this add-on cannot command " + "it. It would sit in the roster, take a share of the site " + "target every cycle and deliver none of it, while the status " + "page showed a site running at full strength. Give it the " + "ESPHome number entity that carries ITS setpoint, or remove " + "the entry.") + if _number(entry, "max_w", None) is None or (_number(entry, "max_w", None) or 0) <= 0: + problems.append( + f"{who} has no usable max_w (got {entry.get('max_w')!r}). It is " + "the supervised cap on what may be written to THIS inverter, it " + "is not shared with any other unit and it is deliberately not " + "inherited from the flat max_w - a rating borrowed from a " + "sibling is exactly what DEV-02 removed. State this unit's own " + "rating in watts.") + + flat_sp = str(opts.get("setpoint_entity") or "").strip() + if entries and flat_sp: + problems.append( + f"setpoint_entity is set to {flat_sp} AND an inverters list of " + f"{len(entries)} is declared. The flat setpoint_entity/soc_entity/" + "batt_entity/max_w keys are the SINGLE-inverter shape and are not " + "read at all once a roster exists, so this config declares an " + "actuator that nothing will ever write - a site running quietly " + "smaller than its own configuration. Move that inverter into the " + "list as its own entry and clear setpoint_entity.") + return problems + + +def validate_unit_ranges(units, *, skip_entity: str = "") -> list[str]: + """Rule 2, per inverter, against each unit's OWN reported device range. + + ⚠️ THE READER THIS REPLACES FAILED PERMISSIVE, WHICH IS THE DIRECTION THAT + HIDES A DEFECT. `run_control()` re-ran rule 2 behind `if + self.o.get("setpoint_entity")` - the FLAT key - because the flat options + were the only per-device options there were. Declare a roster and that key + is blank by rule 9(e), so the whole re-check was skipped and NO inverter's + `max_w` was ever graded against the device it is written to. That is rule + 2's own failure mode restored by a config shape: the operator's stated + ceiling is not the ceiling and nothing says which won. + + `units` is the roster (anything with `.name`, `.setpoint_entity`, `.max_w`, + `.dev_min`, `.dev_max`). A unit whose device did not answer is SKIPPED, not + guessed at, for the reason in this module's docstring: ESPHome is often not + up when the add-on boots and (None, None) means "not read", not "zero". + + `skip_entity` is the flat `setpoint_entity` when the caller has already + graded it through `validate_startup_config(opts, dev_min, dev_max)`. At a + single-inverter config that is the one unit, and grading it twice would + report the same conflict twice under two wordings. + """ + problems: list[str] = [] + skip = str(skip_entity or "").strip().lower() + for unit in units: + entity = str(getattr(unit, "setpoint_entity", "") or "").strip() + if not entity or entity.lower() == skip: + continue + cap = float(getattr(unit, "max_w", 0.0) or 0.0) + dev_min = getattr(unit, "dev_min", None) + dev_max = getattr(unit, "dev_max", None) + name = getattr(unit, "name", entity) + if dev_max is not None and cap > float(dev_max): + problems.append( + f"{name}: max_w is {cap:.0f} W but {entity} accepts at most " + f"{float(dev_max):.0f} W - THE DEVICE WINS, so the {cap:.0f} W " + f"configured for this inverter would silently never happen. " + f"Lower this entry's max_w to {float(dev_max):.0f} W or less.") + elif dev_min is not None and -cap < float(dev_min): + problems.append( + f"{name}: max_w is {cap:.0f} W, so this controller may command " + f"{-cap:.0f} W, but {entity} accepts no less than " + f"{float(dev_min):.0f} W - THE DEVICE WINS on the charge side. " + f"Lower this entry's max_w to {abs(float(dev_min)):.0f} W or " + "less.") + return problems + + class ConfigRefused(Exception): """The configuration is wrong. Not "something broke" - see main.py's handler. @@ -365,7 +615,21 @@ def validate_startup_config(opts: dict, dev_min=None, dev_max=None) -> list[str] "each actuator exactly one writer, or point the second one at " "its own entity.") + # --- 9. CFG-04's roster, graded BEFORE anything reads a per-unit number -- + # ⚠️ FIRST, THOUGH IT IS THE NEWEST RULE. Rules 5 and 6 below now ask the + # roster what one unit may be commanded, so a roster that is nonsense has to + # have been named as nonsense before they read it - otherwise an entry with + # no `max_w` silently contributes no cap, `min()` sees a shorter list, and + # the operator is told about a sign watchdog when the real problem is a + # missing rating one line up. + problems.extend(_roster_problems(opts)) + # --- 2. the configured ceiling against the device's own rating ----------- + # ⚠️ THE FLAT `max_w`, AND ONLY THE FLAT ONE. `dev_min`/`dev_max` are ONE + # device's range - the flat `setpoint_entity`'s - so this rule can only ever + # speak for the single-inverter shape. The per-unit form of it is + # `validate_unit_ranges()`, which main.py runs once every unit's own device + # has answered; it is not called from here because at startup no device has. max_w = _number(opts, "max_w", 2000.0) if max_w is None: problems.append(f"max_w {opts.get('max_w')!r} is not a number") @@ -444,18 +708,37 @@ def validate_startup_config(opts: dict, dev_min=None, dev_max=None) -> list[str] # below max_w, so threshold == max_w is already unsatisfiable. Anything # BELOW max_w is reachable and therefore fine - this refuses the disarmed # case only, not "close to the rail". + # + # ⚠️ AND IT IS GRADED AGAINST THE SMALLEST UNIT'S CAP, NOT AGAINST THE FLAT + # `max_w`. There is one SignWatchdog PER INVERTER (DEV-02) and each grades + # its own unit's standing command, which the arbiter bounds by THAT UNIT's + # `supervised` claim - so the watchdog that can never fire is the one on the + # smallest inverter, and a threshold under the site's largest cap says + # nothing about it. At one inverter `unit_caps()` is `[max_w]` and this is + # the identical comparison it always was. sign_w = _number(opts, "sign_disagree_w", 300.0) + # ⚠️ (index, cap) PAIRS, NOT BARE CAPS. `unit_caps()` skips entries with no + # usable `max_w`, so `caps.index(weakest)` was an index into the CAPS list + # being handed to `_unit_label()`, which indexes the ROSTER - and the message + # named the wrong inverter as soon as the two lengths differed. The index + # travels with the number now, so the name is the name of the unit the + # number came off. + caps_ix = unit_caps_indexed(opts) + caps = [cap for _, cap in caps_ix] + weakest = min(caps) if caps else None if sign_w is None: problems.append(f"sign_disagree_w {opts.get('sign_disagree_w')!r} is not a number") - elif max_w is not None and max_w > 0 and sign_w >= max_w: + elif weakest is not None and weakest > 0 and sign_w >= weakest: + where = _unit_label(opts, min(caps_ix, key=lambda p: p[1])[0]) problems.append( - f"sign_disagree_w is {sign_w:.0f} W but max_w is {max_w:.0f} W, so " + f"sign_disagree_w is {sign_w:.0f} W but max_w{where} is " + f"{weakest:.0f} W, so " "the SAFETY-02 sign-disagreement watchdog can never fire: the " "arbiter never lets a command exceed max_w, and the watchdog needs " f"a command over {sign_w:.0f} W. It would sit there reporting " "'armed' on the status page while being incapable of tripping - the " f"one runaway failure mode this system has actually observed. Lower " - f"sign_disagree_w below {max_w:.0f} W (300 W is the measured " + f"sign_disagree_w below {weakest:.0f} W (300 W is the measured " "default), or raise max_w.") # --- 6. a maintenance charge the ceiling can actually accommodate -------- @@ -484,22 +767,40 @@ def validate_startup_config(opts: dict, dev_min=None, dev_max=None) -> list[str] # the one in-code mirror of config.yaml for this family - the same move # rule 8 below already makes with `PEAK_OPTION_DEFAULTS` - and # test_maintenance.py pins it by reading the shipped file. + # + # ⚠️ AND IT IS GRADED AGAINST THE FIRST UNIT'S CAP, NOT THE SMALLEST AND NOT + # THE FLAT ONE. Rule 5 takes the weakest unit because there is a watchdog on + # every one of them; there is exactly ONE `Maintenance` in this add-on and + # `Controller.maint_unit` names the pack it recalibrates - the first in the + # roster, because the cycle's history lives under un-namespaced Store keys. + # The shaping claim is filed on THAT actuator and on no other, so the cap + # that can contradict it is THAT unit's. Taking `min()` here would refuse a + # perfectly workable site - a 5000 W pack being recalibrated beside a 2000 W + # sibling the cycle never touches - which is the over-refusal this file's + # docstring says gets a validator switched off. When `maint_unit` stops + # being `inverters[0]`, this line moves with it. charge_w = _number(opts, "maintenance_charge_w", float(MAINT_OPTION_DEFAULTS["maintenance_charge_w"])) + # ⚠️ THE SAME PAIRING AS RULE 5, FOR THE SAME REASON. `caps[0]` is the first + # unit WITH A USABLE CAP, which is `inverters[0]` only while every earlier + # entry has one; `_unit_label(opts, 0)` was hard-wired to `inverters[0]` + # regardless. Both come off the one pair now. + maint_index, maint_cap = caps_ix[0] if caps_ix else (0, None) + maint_where = _unit_label(opts, maint_index) if charge_w is None: problems.append( f"maintenance_charge_w {opts.get('maintenance_charge_w')!r} is not a number") - elif max_w is not None and max_w > 0 and charge_w > max_w: + elif maint_cap is not None and maint_cap > 0 and charge_w > maint_cap: problems.append( - f"maintenance_charge_w is {charge_w:.0f} W but max_w is " - f"{max_w:.0f} W, so a maintenance charge in a cheap window can " + f"maintenance_charge_w is {charge_w:.0f} W but max_w{maint_where} is " + f"{maint_cap:.0f} W, so a maintenance charge in a cheap window can " "never happen: the shaping asks the arbiter for at most " f"{-charge_w:.0f} W while the supervised limit allows no less than " - f"{-max_w:.0f} W, which is a contradiction it resolves to 0 W - " + f"{-maint_cap:.0f} W, which is a contradiction it resolves to 0 W - " "THE CEILING WINS, and what you would see is an ARBITER " "CONTRADICTION line that reads like a bug rather than like these " - f"two numbers. Lower maintenance_charge_w to {max_w:.0f} W or less, " - f"or raise max_w to at least {charge_w:.0f} W.") + f"two numbers. Lower maintenance_charge_w to {maint_cap:.0f} W or " + f"less, or raise max_w to at least {charge_w:.0f} W.") # --- 7. two price sources on one site ------------------------------------ problems.extend(_price_problems(opts)) @@ -559,18 +860,23 @@ def validate_startup_config(opts: dict, dev_min=None, dev_max=None) -> list[str] # three-way divergence (config.yaml 2000, MaintConfig 2500) and the card # named only the charge side. Closing one key and leaving its twin is how a # single-source fix gets reproduced as a single-source bug one line down. + # ⚠️ AND AGAINST THE MAINTENANCE UNIT'S CAP, for rule 6's reason: it is the + # same one actuator being clamped, so grading the warning against the flat + # `max_w` would warn about a site number while the clamp is a unit number. disch_w = _number(opts, "maintenance_discharge_w", float(MAINT_OPTION_DEFAULTS["maintenance_discharge_w"])) - if disch_w is not None and max_w is not None and max_w > 0 and disch_w > max_w: + if (disch_w is not None and maint_cap is not None and maint_cap > 0 + and disch_w > maint_cap): _LOG.warning( - "maintenance_discharge_w %.0f W is above max_w %.0f W. Not refused - " - "the drain phase SETS the setpoint and the arbiter clamps it to " - "%.0f W and names the clamp ('limited by supervised(max_w)'), so " - "nothing is silent and the cycle still completes. But it drains at " - "%.0f W, not the %.0f W you configured, and the drain phase has a " - "720 min timeout. Lower it to %.0f W or raise max_w to remove the " + "maintenance_discharge_w %.0f W is above max_w%s %.0f W. Not " + "refused - the drain phase SETS the setpoint and the arbiter clamps " + "it to %.0f W and names the clamp ('limited by supervised(max_w)'), " + "so nothing is silent and the cycle still completes. But it drains " + "at %.0f W, not the %.0f W you configured, and the drain phase has " + "a 720 min timeout. Lower it to %.0f W or raise max_w to remove the " "surprise.", - disch_w, max_w, max_w, max_w, disch_w, max_w) + disch_w, maint_where, maint_cap, maint_cap, maint_cap, disch_w, + maint_cap) return problems diff --git a/goodwe_controller/config.yaml b/goodwe_controller/config.yaml index a30c3a4..ca05e76 100644 --- a/goodwe_controller/config.yaml +++ b/goodwe_controller/config.yaml @@ -1,5 +1,5 @@ name: GoodWe RS485 Controller -version: "0.5.2" +version: "0.6.0" slug: goodwe_controller description: >- Drives a GoodWe ES/BP battery inverter over RS485 by emulating its smart @@ -39,6 +39,59 @@ options: batt_invert: false setpoint_entity: "" + # --- ARCH-01 / CFG-04: the inverter roster --------------------------------- + # ⚠️ EMPTY IS NOT "UNCONFIGURED", IT IS THE SINGLE-INVERTER CONFIGURATION, and + # it is what every install that upgrades into this version lands on. An empty + # list means THE FIVE FLAT KEYS ABOVE DESCRIBE THE ONE INVERTER - same + # entities, same rating, same behaviour, no operator action - which is the + # only migration story this key was allowed to have. `build_roster()` reads + # this and nothing else decides the shape. + # + # Declared, it REPLACES those five keys rather than extending them. Each entry + # names its own actuator, its own rating and its own sensors: + # + # inverters: + # - name: inverter-a + # setpoint_entity: number.goodwe_a_setpoint_w + # soc_entity: sensor.goodwe_a_soc + # batt_entity: sensor.goodwe_a_battery_power + # max_w: 5000 + # - name: inverter-b + # setpoint_entity: number.goodwe_b_setpoint_w + # soc_entity: sensor.goodwe_b_soc + # batt_entity: sensor.goodwe_b_battery_power + # max_w: 3000 + # + # ⚠️ NOTHING IS INHERITED FROM THE FLAT KEYS BY AN ENTRY THAT OMITS IT, and + # that is the whole of DEV-02 restated as a config rule. `soc`, `batt` and the + # rating became per-instance precisely so that no inverter is ever graded + # against a sibling's numbers; an entry silently inheriting `soc_entity` would + # rebuild that defect in YAML. `name`, `setpoint_entity` and `max_w` are + # therefore REQUIRED of every entry (the schema below enforces it, and + # validate.py rule 9 enforces it again for the callers the Supervisor never + # sees). An omitted `soc_entity`/`batt_entity` means THIS UNIT HAS NO SUCH + # SENSOR - `inputs_ok` already stops the site for that - and never means + # "use the other one's". + # + # ⚠️ AND THE FLAT `setpoint_entity` MUST THEN BE BLANK. Leaving it set while a + # roster is declared is two config sites aimed at actuators, one of which + # nothing writes; if it names an entity an entry also names, SAFETY-28's + # writer census refuses it as two writers on one actuator, and if it names a + # different one the add-on would run a site quietly smaller than the config + # reads. Rule 9 refuses both rather than picking one. + # + # ⚠️ THE SHAPE WAS MEASURED, NOT ASSUMED. A list of objects is not used + # anywhere else in this file, and whether Supervisor's options validator + # accepts one was an open question - `static_profile` carries its structure as + # a JSON string in a `str?` for exactly that doubt. It was settled by running + # Supervisor 2026.08.0's OWN `AppOptions` validator (the version this site + # runs) against both candidates: tools/supervisor_shape_probe.py is the + # script, DECISIONS/CFG-04-inverter-roster.md is what it printed and why the + # list won. The default `[]` is load-bearing in that record: WITHOUT it the + # validator answers `Missing option 'inverters' in root` for every existing + # options blob, and the upgrade leaves an add-on that cannot start. + inverters: [] + # --- P1 meter ingestion (specs §5.2 / §14 `meter:`) ------------------------- # `off` keeps the original single meter_entity path above, so an existing # install is untouched until it opts in. ha_dsmr subscribes to the DSMR @@ -369,6 +422,32 @@ schema: batt_invert: bool setpoint_entity: str + # CFG-04. THE ONE LIST-OF-OBJECTS IN THIS FILE, and see the options block for + # the measurement that says Supervisor 2026.08.0 accepts it: nested types are + # enforced per entry, a missing required key names the key, a scalar where the + # list belongs is refused, and the UI renders it as a repeatable section. + # + # ⚠️ THREE KEYS ARE REQUIRED PER ENTRY AND THAT IS DELIBERATE. An entry with + # no `name` has no Store namespace and would share another unit's SAFETY-02 + # latch; one with no `setpoint_entity` is a unit nothing can command; one with + # no `max_w` would have to borrow a rating, which is the sibling-grading + # defect DEV-02 removed. The optional pair is the sensors, whose absence + # honestly means "this unit reports none". + # + # ⚠️ THIS BLOCK IS DECLARED BEFORE `max_w:` ON PURPOSE. `max_w` now appears + # TWICE under `schema:` - once nested here, once flat below - and a reader + # that matches a stripped line takes the nested one FIRST. That is the + # first-match trap this card had to fix in three checks; leaving the nesting + # first keeps the trap baited, so the fix stays graded rather than assumed. + # test_inverter.py part 12 asserts the ordering is still hostile. + inverters: + - name: str + setpoint_entity: str + max_w: int(100,5000) + soc_entity: str? + batt_entity: str? + batt_invert: bool? + meter_source: list(off|ha_dsmr|mqtt_p1|ha_signed|homewizard_local) # ⚠️ 2 is accepted by this range but is not a real Belgian connection. A # telegram whose phase count disagrees is rejected at ingest and logged, so a diff --git a/goodwe_controller/test_inverter.py b/goodwe_controller/test_inverter.py index 474026b..4f6f125 100644 --- a/goodwe_controller/test_inverter.py +++ b/goodwe_controller/test_inverter.py @@ -4,7 +4,9 @@ THE DEFECT, in one sentence: every piece of per-device state in the Controller was a SCALAR, and several of those scalars are safety inputs - so a second inverter would have been graded against the first one's numbers. -Nine parts, and what each one is deliberately NOT allowed to be: +Twelve parts, and what each one is deliberately NOT allowed to be. Parts 1-8 +and 12 are DEV-02's; parts 9-11 are CFG-04's, which is the card that gave an +OPERATOR a way to declare the second inverter DEV-02 made the code ready for. 1. AC 1. THE GRADER, over `app/main.py` itself. Read, not run: the claim is "no per-device value remains a Controller scalar", which is a statement @@ -36,7 +38,21 @@ Nine parts, and what each one is deliberately NOT allowed to be: evidence for that and they run unmodified; what is here is the part they cannot see - that the compatibility views REFUSE at N>=2 rather than answering for the first inverter. - 9. MUTATION. Which check bites for each way of defeating this card. + 9. CFG-04. A TWO-INVERTER CONFIG, through the shipped `build_roster()`, into + a roster the shipped `live_units()` answers for - and the other half of it, + that an EMPTY `inverters:` is byte-identical to what shipped before. + 10. CFG-04. RULE 9, and SAFETY-28's two-writers hazard, which is expressible + for the first time in this add-on's life. Also the two PERMISSIVE READERS + this card found in shipped code - rule 5 grading against the flat `max_w`, + and run_control() skipping the whole range re-check when the flat + `setpoint_entity` is blank - both run BOTH WAYS, because a refusal that + fires on everything passes a check that only looks for the refusal. + 11. CFG-04. EVERY config.yaml READER IN THE SUITE, FED A TWO-INVERTER CONFIG. + Not a lint over their source: the functions are extracted and EXECUTED + against a synthetic file whose per-inverter numbers differ from its flat + ones, because on the shipped file a first-match reader is wrong and right + at the same time. + 12. MUTATION. Which check bites for each way of defeating this card. ⚠️ INVERTER B DOES NOT EXIST YET (~2 weeks out; SPIKE-04 measures it). Every second unit below is built from parameters passed IN, and where a number is @@ -83,11 +99,21 @@ def check(name, cond): def read_schema_max(key): - """The schema's upper bound for `key` out of the SHIPPED config.yaml.""" + """The schema's upper bound for `key` out of the SHIPPED config.yaml. + + ⚠️ ANCHORED AT EXACTLY TWO SPACES, WHICH IS THE WHOLE OF CFG-04's FIX HERE. + This used to match `line.strip()`, and CFG-04's `inverters:` put a NESTED + `max_w: int(100,5000)` six spaces in and earlier in the file - so the + stripped version answered with the per-inverter bound and stayed green, + because the two bounds are the same number today. Part 12 below runs this + against a config where they are NOT the same number. + """ body = io.open(HERE / "config.yaml", encoding="utf-8").read() for line in body[body.index("\nschema:"):].splitlines(): + if not line.startswith(f" {key}:") or line[2:3].isspace(): + continue st = line.strip() - if st.startswith(f"{key}:") and "(" in st and ")" in st: + if "(" in st and ")" in st: return float(st[st.index("(") + 1:st.rindex(")")].split(",")[1]) return None @@ -1280,9 +1306,660 @@ check("...and `units` has no setter at all, so a site cannot claim two " and _c8.units == ("inverter-a", "inverter-b")) + # ============================================================================== print() -print("9. mutation: what defeats this card, and which check bites") +print("9. CFG-04: a two-inverter CONFIG becomes a two-unit roster") + +# ⚠️ EVERY SECOND UNIT ABOVE WAS CONSTRUCTED BY THIS FILE. That was the whole of +# DEV-02's honesty: the code stopped assuming one inverter, and there was still +# no way for an OPERATOR to declare a second - `build_roster()` returned a list +# of exactly one and said so in its own docstring. This part grades the config +# path: an options dict, through the shipped `build_roster()`, into a roster the +# shipped `live_units()` answers for. +_TWO_OPTS = dict(BASE_OPTS) +_TWO_OPTS.pop("setpoint_entity") +_TWO_OPTS.pop("soc_entity") +_TWO_OPTS.pop("batt_entity") +_TWO_OPTS["inverters"] = [ + dict(name="inverter-a", setpoint_entity="number.sp_a", + soc_entity="sensor.soc_a", batt_entity="sensor.batt_a", + max_w=A_RATING), + # ⚠️ 3000 W IS A HYPOTHESIS AND IS MARKED AS ONE WHEREVER IT APPEARS. It is + # here because it DIFFERS from A's 5000 W, which is the only property any + # check below uses. SPIKE-04 measures the real device. + dict(name="inverter-b", setpoint_entity="number.sp_b", + soc_entity="sensor.soc_b", batt_entity="sensor.batt_b", + max_w=B_RATING_HYPOTHESIS), +] +_c9, _hass9 = built(_TWO_OPTS) +print(f" the roster built from `inverters:` -> {[repr(i) for i in _c9.inverters]}") +check("a two-inverter config produces a TWO-unit roster, each with its own " + "actuator, its own sensors and its own rating - which is the thing this " + "card exists to make possible at all", + [i.name for i in _c9.inverters] == ["inverter-a", "inverter-b"] + and [i.setpoint_entity for i in _c9.inverters] == ["number.sp_a", + "number.sp_b"] + and [i.max_w for i in _c9.inverters] == [A_RATING, B_RATING_HYPOTHESIS] + and [i.soc_entity for i in _c9.inverters] == ["sensor.soc_a", + "sensor.soc_b"]) +check("...and `live_units()` - the method the allocator asks, and the one that " + "decides what may be commanded this tick - lists BOTH of them", + _c9.live_units() == ("inverter-a", "inverter-b")) +check("...and the site bound the loop runs on is the SUM, not one unit's " + f"rating: {A_RATING:.0f} + {B_RATING_HYPOTHESIS:.0f} W", + _c9.site_max_w == A_RATING + B_RATING_HYPOTHESIS + and _c9.tuning.max_w == A_RATING + B_RATING_HYPOTHESIS) +check("...and each unit keeps its OWN Store namespace, so a SAFETY-02 trip on " + "one cannot latch the other - DEV-02's property, now reachable from a " + "config file instead of from a test fixture", + _c9.inverters[0].key("sign_tripped") != _c9.inverters[1].key("sign_tripped")) + +# ⚠️ AND THE SHAPE IS ANNOUNCED, because the other shape's keys are still +# sitting in the Configuration UI with values in them. +check("...and the action log says which shape is in force and what it is NOT " + "reading, at every start - an operator whose site runs at half the " + "rating they typed has one line that says why", + any("roster:" in e and "inverter-b" in e and "NOT" in e + for e in _c9.events)) + +# --- the other half, and it is the half an upgrade depends on ---------------- +_c9a, _ = built() # BASE_OPTS: the flat keys, no list +_c9b, _ = built({"inverters": []}) # what config.yaml actually ships +check("⚠️ AC 3: an EMPTY `inverters:` is not 'unconfigured', it is the " + "single-inverter configuration - the flat keys describe the one unit, " + "under the same name, with the same entities and the same rating as " + "before this card. That is what an install that upgrades reads, with " + "nobody touching anything", + [(i.name, i.setpoint_entity, i.soc_entity, i.batt_entity, i.max_w) + for i in _c9b.inverters] + == [(i.name, i.setpoint_entity, i.soc_entity, i.batt_entity, i.max_w) + for i in _c9a.inverters] + == [(UNIT_A, "number.sp_a", "sensor.soc_a", "sensor.batt_a", A_RATING)]) +check("...and the key ABSENT and the key EMPTY are the same configuration, not " + "two code paths - config.yaml ships `[]` and an options dict written " + "before this version has neither", + _c9b.units == _c9a.units == (UNIT_A,)) +# ⚠️ AND THE SHIPPED DEFAULT IS NOT COSMETIC - IT IS WHAT MAKES THE UPGRADE +# SURVIVABLE, AND IT WAS MEASURED. Supervisor 2026.08.0's own AppOptions +# validator - the version this site runs, run against this very file by +# tools/supervisor_shape_probe.py section B - answers `Missing option +# 'inverters' in root` for the shipped `options:` block with ONLY this key +# removed (one key varied, so the refusal is about this key; section B2's +# reconstructed blob is refused for unrelated missing keys and is NOT this +# measurement), and `App.test_update_schema()` +# turns that into "new schema is not compatible" at UPDATE time. The Supervisor +# merges `options:` into the operator's stored blob before validating, so this +# one line is the difference between an upgrade nobody notices and an add-on +# that cannot start. See DECISIONS/CFG-04-inverter-roster.md; deploy-rollback.md +# §4 is the same trap in the other direction. +_cfg_txt_9 = io.open(HERE / "config.yaml", encoding="utf-8").read() +check("⚠️ config.yaml SHIPS `inverters: []` in its options block. Without it " + "Supervisor refuses every options blob written before this version - " + "measured on 2026.08.0, not assumed - and the operator is left with an " + "add-on that will not start", + "\n inverters: []\n" in _cfg_txt_9[:_cfg_txt_9.index("\nschema:")]) +_sch9 = _cfg_txt_9[_cfg_txt_9.index("\nschema:"):] +_entry9 = _sch9[_sch9.index("\n inverters:"):] +_entry9 = _entry9[:_entry9.index("\n\n")] +_decl9 = dict(ln.strip().lstrip("- ").split(": ", 1) + for ln in _entry9.splitlines()[1:] if ": " in ln) +print(f" the shipped per-entry schema: {_decl9}") +check("...and the schema half declares it as a LIST OF OBJECTS whose `name`, " + "`setpoint_entity` and `max_w` are REQUIRED - no trailing `?` - so the " + "Supervisor refuses a half-written entry before the add-on sees it, and " + "an entry can never fall back on a sibling's rating because there is no " + "entry without one", + set(_decl9) == {"name", "setpoint_entity", "max_w", "soc_entity", + "batt_entity", "batt_invert"} + and not any(_decl9[k].endswith("?") + for k in ("name", "setpoint_entity", "max_w")) + and all(_decl9[k].endswith("?") + for k in ("soc_entity", "batt_entity", "batt_invert"))) + +# ⚠️ NOTHING IS INHERITED, AND THIS IS DEV-02 RESTATED AS A CONFIG RULE. +# ⚠️ THE FLAT KEYS ARE SET TO REAL ENTITIES HERE, AND THAT IS THE WHOLE CHECK. +# The first draft left them blank, so "inherited" and "not inherited" produced +# the SAME empty string and the check passed against a `build_roster()` that +# DID inherit - a mutation this file ran and watched survive. An assertion that +# X is not Y has to be run with X and Y actually different (MUTATION.md). +_c9c, _ = built({"inverters": [dict(name="inverter-a", + setpoint_entity="number.sp_a", + max_w=A_RATING)], + "soc_entity": "sensor.soc_a", "batt_entity": "sensor.batt_a"}) +check("an entry that omits `soc_entity` has NO SoC sensor and does not borrow " + "the flat one - which is set to a real entity right here, so inheriting " + "and not inheriting are visibly different answers. A unit silently " + "graded against a sibling's measurement is the exact defect DEV-02 spent " + "a card removing, and a three-line fallback here would rebuild it in YAML", + _c9c.inverters[0].soc_entity == "" and _c9c.inverters[0].batt_entity == "" + and _c9c.o["soc_entity"] == "sensor.soc_a") + +_c9d, _ = built({"inverters": [ + dict(name="inverter-a", setpoint_entity="number.sp_a", max_w=A_RATING), + dict(name="inverter-b", setpoint_entity="number.sp_b")]}) +check("...and an entry that omits `max_w` does not inherit the flat rating " + "either - it gets 0 W and validate.py rule 9 refuses it by name, which " + "is a refusal an operator can act on rather than a site quietly running " + "at somebody else's number", + _c9d.inverters[1].max_w == 0.0) + + +# ============================================================================== +print() +print("10. CFG-04: rule 9, and SAFETY-28's hazard now expressible for the " + "first time") + +from app.validate import ( # noqa: E402 + actuator_writers, unit_caps, unit_caps_indexed, validate_startup_config, + validate_unit_ranges, _unit_label, +) + + +def _why(problems, needle): + return any(needle in p for p in problems) + + +def _opts(**kw): + """A complete, ACCEPTED options dict, so a refusal below is the new one.""" + o = dict(BASE_OPTS, meter_source="off", sign_disagree_w=300, + sign_disagree_s=180, maintenance_charge_w=2000, + maintenance_discharge_w=2000, peak_soc_floor=20, bms_soc_floor=10) + o.update(kw) + return o + + +check("the fixture this part refuses things out of is itself ACCEPTED, or " + "every check below would pass on a config that was already broken", + validate_startup_config(_opts(), -A_RATING, A_RATING) == []) + +# --- AC 5: the two-writers hazard -------------------------------------------- +_same = _opts(setpoint_entity="", inverters=[ + dict(name="inverter-a", setpoint_entity="number.sp_a", max_w=A_RATING), + dict(name="inverter-b", setpoint_entity="NUMBER.SP_A", + max_w=B_RATING_HYPOTHESIS)]) +_p_same = validate_startup_config(_same) +print(f" two inverters, one entity -> {len(_p_same)} refusal(s)") +check("⚠️ AC 5: two inverters naming the SAME setpoint_entity are REFUSED. " + "specs §1.5's non-goal has been enforced 'by construction' for the whole " + "life of this add-on because the schema had exactly one such key; this " + "is the first configuration in which it is expressible, and it is the " + "first time that guarantee has had to be a check", + _why(_p_same, "one actuator") and _why(_p_same, "number.sp_a")) +check("...and case-folded, so the hazard cannot be smuggled past on " + "NUMBER.SP_A versus number.sp_a - which is exactly how it would be " + "written by someone copying an entity id out of the HA UI", + _why(_p_same, "inverters[0].setpoint_entity") + and _why(_p_same, "inverters[1].setpoint_entity")) +check("...and two inverters on two DIFFERENT entities are accepted, because " + "that is the configuration this card exists for and a validator that " + "fires on the legitimate case gets switched off", + validate_startup_config(_opts(setpoint_entity="", inverters=[ + dict(name="inverter-a", setpoint_entity="number.sp_a", max_w=A_RATING), + dict(name="inverter-b", setpoint_entity="number.sp_b", + max_w=B_RATING_HYPOTHESIS)])) == []) + +# --- the rest of rule 9 ------------------------------------------------------- +_p_flat = validate_startup_config(_opts(inverters=[ + dict(name="inverter-b", setpoint_entity="number.sp_b", + max_w=B_RATING_HYPOTHESIS)])) +check("a flat setpoint_entity left set BESIDE a declared roster is refused. It " + "names an actuator the roster does not, so nothing would ever write it - " + "a site running quietly smaller than its own configuration, which is the " + "'silently a different config' outcome this card ranks below a refusal", + _why(_p_flat, "AND an inverters list")) +_p_dup = validate_startup_config(_opts(setpoint_entity="", inverters=[ + dict(name="twin", setpoint_entity="number.sp_a", max_w=A_RATING), + dict(name="twin", setpoint_entity="number.sp_b", + max_w=B_RATING_HYPOTHESIS)])) +check("two entries with the SAME name are refused - the Store namespaces every " + "per-unit latch by name, so one name is one SAFETY-02 latch for two " + "units and an operator clearing it clears both", + _why(_p_dup, "both called") and _why(_p_dup, "twin")) +for _what, _bad, _needle in ( + ("with no name at all", + dict(setpoint_entity="number.sp_b", max_w=3000), "has no name"), + ("this controller cannot command", + dict(name="inverter-b", max_w=3000), "has no setpoint_entity"), + ("with no rating", + dict(name="inverter-b", setpoint_entity="number.sp_b"), + "no usable max_w"), + ("whose rating is not a number", + dict(name="inverter-b", setpoint_entity="number.sp_b", max_w="lots"), + "no usable max_w")): + check(f"an entry {_what} is refused, and the refusal says which entry and " + f"which key ({_needle!r}) rather than that something is wrong", + _why(validate_startup_config(_opts(setpoint_entity="", + inverters=[_bad])), _needle)) +check("`inverters` that is not a list at all is refused with the shape it " + "should have been, rather than crashing inside build_roster()", + _why(validate_startup_config(_opts(inverters="number.sp_b")), + "must be a\nLIST".replace("\n", " "))) + +# --- the permissive readers INSIDE validate.py -------------------------------- +# ⚠️ RUN BOTH WAYS, WHICH IS MUTATION.md's RULE. A check that only asserts the +# refusal happens would also pass if the rule fired on everything; the +# counterfactual is the flat reader's own answer on the same config. +_weak = _opts(max_w=A_RATING, sign_disagree_w=3000, setpoint_entity="", + inverters=[dict(name="inverter-a", setpoint_entity="number.sp_a", + max_w=2000), + dict(name="inverter-b", setpoint_entity="number.sp_b", + max_w=2000)]) +print(f" flat max_w {_weak['max_w']:.0f} W, roster caps {unit_caps(_weak)}, " + f"sign_disagree_w {_weak['sign_disagree_w']:.0f} W") +check("⚠️ A PERMISSIVE READER INSIDE THE VALIDATOR, FIXED AND GRADED. Rule 5 " + "compared sign_disagree_w against the FLAT max_w. With 5000 left over " + "from a single-inverter install and a roster of two 2000 W units, 3000 W " + "passed that comparison while the SAFETY-02 watchdog could fire on " + "NEITHER unit - rule 5's own failure mode, reached through rule 5. It " + "now reads the roster's caps and refuses", + _why(validate_startup_config(_weak), "sign_disagree_w")) +check("...and the counterfactual: the number the old reader used, on the same " + "config, does not trip it. Without this line the check above would pass " + "for a rule that refused everything", + 3000.0 < float(_weak["max_w"]) and min(unit_caps(_weak)) == 2000.0) +check("...and at one inverter `unit_caps()` IS `[max_w]`, so the rule is the " + "identical comparison it always was and no existing install changes " + "verdict", unit_caps(_opts()) == [A_RATING]) + +# --- rules 5 and 6 named the WRONG INVERTER when the two lists desynchronised - +# ⚠️ WHAT THE DEFECT WAS. `unit_caps()` SKIPS an entry with no usable `max_w`, +# so caps position i stopped being roster position i the moment one entry was +# malformed. Rule 5 did `_unit_label(opts, caps.index(weakest))` and rule 6 did +# `caps[0]` beside a hard-wired `_unit_label(opts, 0)` - both feeding a CAPS +# index to a function that indexes the ROSTER. The number in the message was +# right and the NAME beside it was another inverter's: an operator sent to the +# wrong device by the message that exists to stop exactly that. +# +# It is latent, not live - rule 9(d) refuses a junk `max_w` in the same pass, so +# the operator sees both refusals. But "unreachable" was a property of today's +# rule ordering rather than of the code, and `validate_startup_config()` is also +# `sim/inject.py`'s entry point. The index travels with the cap now. +_skew = _opts(setpoint_entity="", sign_disagree_w=1500, maintenance_charge_w=3000, + maintenance_discharge_w=1000, inverters=[ + dict(name="inverter-a", setpoint_entity="number.sp_a", + max_w="junk"), # SKIPPED by unit_caps() + dict(name="inverter-b", setpoint_entity="number.sp_b", max_w=2000), + dict(name="inverter-c", setpoint_entity="number.sp_c", max_w=1000)]) +_p_skew = validate_startup_config(_skew) +print(f" caps {unit_caps(_skew)} but pairs {unit_caps_indexed(_skew)} - " + f"roster is {len(_skew['inverters'])} long") +check("the fixture actually desynchronises the two lists, or the two checks " + "below prove nothing: one entry is skipped, so caps is SHORTER than the " + "roster and every caps index is off by one", + unit_caps(_skew) == [2000.0, 1000.0] + and unit_caps_indexed(_skew) == [(1, 2000.0), (2, 1000.0)]) +check("⚠️ RULE 5 NAMES THE UNIT THE NUMBER CAME OFF. The weakest cap is " + "inverter-c's 1000 W; it sits at caps index 1, and `_unit_label(opts, 1)` " + "is inverter-B. The refusal says inverter-c", + _why(_p_skew, "max_w (inverter-c) is 1000 W") + and not _why(_p_skew, "max_w (inverter-b) is 1000 W")) +check("⚠️ RULE 6 TOO. `caps[0]` is the first entry WITH a usable cap - " + "inverter-b - while `_unit_label(opts, 0)` was hard-wired to inverter-a, " + "so the maintenance refusal quoted B's rating under A's name", + _why(_p_skew, "max_w (inverter-b) is 2000 W") + and not _why(_p_skew, "max_w (inverter-a) is 2000 W")) +check("...and the counterfactual, so this is not a check that would pass on a " + "validator that named nothing: the OLD expressions still evaluate, on " + "this same config, to the wrong names", + _unit_label(_skew, unit_caps(_skew).index(1000.0)) == " (inverter-b)" + and _unit_label(_skew, 0) == " (inverter-a)") +check("...and rule 9(d) refuses this config anyway - which is WHY the defect " + "was latent, and is recorded rather than used as a reason not to fix it", + _why(_p_skew, "inverter-a")) + +# --- validate_unit_ranges: rule 2, per unit, against ITS OWN device ----------- +_c10, _ = built(_TWO_OPTS) +_c10.inverters[0].dev_min, _c10.inverters[0].dev_max = -A_RATING, A_RATING +# B's ESPHome reports a 2000 W device while the config claims 3000 W for it. +_c10.inverters[1].dev_min, _c10.inverters[1].dev_max = -2000.0, 2000.0 +_p_rng = validate_unit_ranges(_c10.inverters) +print(f" per-unit range check -> {_p_rng}") +check("⚠️ A READER THAT SILENTLY PINNED INVERTER #1, IN main.py. run_control() " + "re-ran rule 2 behind `if self.o.get('setpoint_entity')` - the FLAT key, " + "which a roster config leaves blank - so on a two-inverter site NO " + "unit's max_w was graded against the device it is written to, and the " + "skip was silent. The per-unit form catches B against B's own ESPHome", + len(_p_rng) == 1 and _why(_p_rng, "inverter-b") + and _why(_p_rng, "THE DEVICE WINS")) +check("...and it says INVERTER-B, not 'the setpoint device' - a site with " + "three inverters and one bad rating needs to be told which one", + _why(_p_rng, "number.sp_b") and not _why(_p_rng, "inverter-a")) +check("...and a unit whose ESPHome has not answered is SKIPPED rather than " + "guessed at, which is the same (None, None) rule the flat check has " + "always had - refusing there would refuse every cold boot", + validate_unit_ranges([_c9.inverters[1]]) == []) +check("...and at N=1 the unit the flat call already graded is skipped, so one " + "conflict is reported once and not twice under two wordings", + validate_unit_ranges(_c9a.inverters, skip_entity="number.sp_a") == []) +# ⚠️ AND THE ADD-ON IS DRIVEN, NOT GREPPED. The first draft asserted the call's +# SOURCE LINE was in main.py, and the mutation that puts it back inside `if +# entity:` - which is one level of indentation - left that string intact and the +# check green. So `run_control()` is run, on a two-inverter config with no flat +# `setpoint_entity` at all, against an ESPHome that reports a device narrower +# than what B's entry claims. +_hass_rc = Hass({"sensor.grid": 0.0, "sensor.soc_a": 50.0, "sensor.batt_a": 0.0, + "sensor.soc_b": 50.0, "sensor.batt_b": 0.0}, + {"number.sp_a": (-A_RATING, A_RATING, 1.0), + # B's real device is 2000 W; its roster entry claims 3000 W. + "number.sp_b": (-2000.0, 2000.0, 1.0)}) +_rc_opts = {k: v for k, v in _TWO_OPTS.items() if k != "setpoint_entity"} +_c_rc = Controller(dict({k: v for k, v in BASE_OPTS.items() + if k not in ("setpoint_entity", "soc_entity", + "batt_entity")}, + **_rc_opts), + _hass_rc, MemStore(), Pub(), stop=asyncio.Event()) +check("the fixture really does have NO flat setpoint_entity, which is the " + "condition the old re-check silently skipped on", + not _c_rc.o.get("setpoint_entity") and len(_c_rc.inverters) == 2) +# ⚠️ WITH A DEADLINE, BECAUSE THE FAILURE MODE IS NOT AN EXCEPTION. A +# `run_control()` that does NOT refuse enters the control loop and never +# returns, so the mutation this check exists to catch turns a red into a HANG - +# and a check that hangs is worse than one that passes wrongly, because CI +# times out with no name attached. The timeout makes "did not refuse" a +# failure with a sentence on it. +async def _drive_rc(): + try: + await asyncio.wait_for(_c_rc.run_control(), timeout=10.0) + return "" + except asyncio.TimeoutError: + return ("run_control() entered the control loop instead of refusing - " + "the per-unit range check did not run") + + +_rc_why = asyncio.get_event_loop_policy().new_event_loop().run_until_complete( + _drive_rc()) +print(f" run_control() on a two-inverter config -> refused={_c_rc.refused}" + + (f" [{_rc_why}]" if _rc_why else "")) +check("⚠️ RUN, NOT GREPPED: the shipped `run_control()` REFUSES this config and " + "never enters the loop. Before this card the whole re-check hung off the " + "flat `setpoint_entity`, so a roster config skipped it and B's 3000 W " + "against a 2000 W device was never noticed by anything", + not _rc_why and _c_rc.refused + and any("inverter-b" in p for p in _c_rc.refused) + and _c_rc.stop_event.is_set()) +check("...and it wrote nothing on the way out - a refusal is a config that " + "never ran, not one that ran once", + not _hass_rc.writes) + + + +# ============================================================================== +print() +print("11. CFG-04: every config.yaml reader in the suite, FED a two-inverter " + "config") + +# ⚠️ THE HAZARD THIS PART EXISTS FOR, AND IT IS THE ONE THE PM PROOFREAD NAMED. +# CFG-03 made config.yaml the single source of truth for defaults, so nine +# checks across this suite now READ it rather than transcribing it. Every one of +# those readers was written when a key appeared at most twice - once under +# `options:`, once under `schema:` - and several disambiguated by taking the +# FIRST match on a `line.strip()`, which throws the indentation away. +# +# CFG-04's `inverters:` is a list of objects and its entries carry a `max_w:` of +# their own. So `max_w` is now declared at TWO nesting levels, and a stripped +# first-match reader answers with the PER-INVERTER one. On the shipped file that +# is harmless by luck - the two bounds are the same `int(100,5000)` - which is +# precisely what makes it dangerous: the reader silently returns inverter #1's +# number and every check that consumes it stays green. This project's rule is +# that a check which stays green after you break the thing it watches was never +# watching, and a reader that is right by coincidence is the same defect one +# layer down. +# +# So: a SYNTHETIC config.yaml is built from the shipped one with the nested +# per-inverter numbers made DIFFERENT from the flat ones (MUTATION.md's rule - +# an assertion that "X is not Y" must be run once with X and Y actually made +# different), and every reader in the suite is executed against it. +import re as _re # noqa: E402 +import tempfile as _tempfile # noqa: E402 + +_SHIPPED_CFG = io.open(HERE / "config.yaml", encoding="utf-8").read() + +# The per-inverter numbers, chosen only because they differ from BOTH flat +# answers (options `max_w: 2000`, schema `int(100,5000)`). Nothing is measured +# here and inverter B still does not exist. +_NESTED_W = "3141" + +_two_cfg = _SHIPPED_CFG.replace( + " inverters: []\n", + " inverters:\n" + " - name: inverter-a\n" + " setpoint_entity: number.sp_a\n" + f" max_w: {_NESTED_W}\n" + " - name: inverter-b\n" + " setpoint_entity: number.sp_b\n" + f" max_w: {_NESTED_W}\n", 1) +_two_cfg = _two_cfg.replace(" max_w: int(100,5000)\n", + f" max_w: int(100,{_NESTED_W})\n", 1) +check("the synthetic two-inverter config really did get built - it differs " + "from the shipped file, and the per-inverter numbers really are " + "different from the flat ones, or every reader below would 'pass' by " + "answering the same number either way", + _two_cfg != _SHIPPED_CFG + and _two_cfg.count(f"max_w: {_NESTED_W}") == 2 + and f"max_w: int(100,{_NESTED_W})" in _two_cfg + and _NESTED_W not in _SHIPPED_CFG) + +# ⚠️ AND THE SHIPPED FILE ITSELF STILL BAITS THE TRAP. If somebody moves +# `inverters:` below `max_w:` in the schema block, the stripped-first-match bug +# stops being reachable on the shipped file and this whole part quietly becomes +# a check about a synthetic fixture. Assert the ordering rather than trusting a +# comment to preserve it. +_schema_half = _SHIPPED_CFG[_SHIPPED_CFG.index("\nschema:"):] +check("⚠️ the shipped config.yaml still declares the NESTED per-inverter " + "`max_w:` BEFORE the flat one under `schema:`, so a stripped-line reader " + "would genuinely take the wrong one. Reorder that and the readers below " + "become right by accident and this part stops watching anything", + _schema_half.index(" max_w: int(") < _schema_half.index("\n max_w: int(")) + +_TMPCFG = Path(_tempfile.mkdtemp(prefix="cfg04-")) +io.open(_TMPCFG / "config.yaml", "w", encoding="utf-8").write(_two_cfg) + +# --- who the readers ARE, found rather than listed --------------------------- +# ⚠️ THE TABLE IS AUDITED, THE DISCOVERY IS MECHANICAL, AND BOTH HALVES MATTER. +# A hand-written list of readers is a list that goes stale the moment somebody +# adds the tenth one - and the tenth one is exactly the one that gets a +# stripped-line idiom typed for it. So every function in every test file whose +# body touches config.yaml is DISCOVERED by AST, and a discovered function that +# is not in the table below fails this part rather than being skipped. +_AUDITED = { + ("test_peakguard.py", "read_option"), + ("test_peakguard.py", "read_schema_max"), + ("test_peakguard.py", "read_schema_min"), + ("test_peakguard.py", "_schema_bound"), + ("test_inverter.py", "read_schema_max"), + ("test_maintenance.py", "shipped_default"), + ("test_maintenance.py", "schema_range"), + ("test_freeze.py", "_cfg_default"), + ("test_price.py", "shipped"), + ("test_tariff.py", "shipped_option"), +} +# Helpers the audited readers call. Not called with a key themselves, so there +# is nothing to feed them; they are exec'd so their callers work. +_HELPERS = {("test_peakguard.py", "_cfg_halves"), ("test_peakguard.py", "_top_level")} +_CFG_GLOBALS = ("_CFG_TXT", "_CFG_TEXT", "_cfg_lines", "_cfg_txt", "_cfg_src", + "_SCHEMA_AT", "_txt") +# ⚠️ AND THE INDIRECT ONES, WHICH IS WHERE THE FIRST DRAFT OF THIS AUDIT WENT +# WRONG. `read_schema_max()` and `_schema_bound()` in test_peakguard.py do not +# contain the string "config.yaml" at all - they go through `_cfg_halves()` - +# so a predicate that only looks for the filename finds nine readers and misses +# the two that CFG-04 actually had to change. Naming a helper counts as reading. +_HELPER_NAMES = tuple(n for _f, n in _HELPERS) + + +def _reads_cfg(src): + return ("config.yaml" in src + or any(g in src for g in _CFG_GLOBALS) + or any(f"{h}(" in src for h in _HELPER_NAMES)) + + +_discovered, _sources, _all_fns = set(), {}, {} +for _path in sorted(HERE.glob("test_*.py")): + _body = io.open(_path, encoding="utf-8").read() + if "config.yaml" not in _body: + continue + for _node in ast.walk(ast.parse(_body)): + if not isinstance(_node, ast.FunctionDef): + continue + _seg = ast.get_source_segment(_body, _node) or "" + _all_fns[(_path.name, _node.name)] = _seg + if _reads_cfg(_seg): + _discovered.add((_path.name, _node.name)) +_sources.update({k: _all_fns[k] for k in _discovered}) +# ⚠️ TO A FIXPOINT, because reading is TRANSITIVE and the first draft of this +# audit stopped at one hop. `read_schema_max()` calls `_schema_bound()` which +# calls `_cfg_halves()`; only the last of those three names the file, and the +# first two are among the readers CFG-04 had to change. A one-hop predicate +# found nine readers, missed those two, and would have reported "all clear" for +# the exact functions this part was written about. +while True: + _grew = False + for (_f, _n), _seg in _all_fns.items(): + if (_f, _n) in _discovered: + continue + if any(_df == _f and f"{_dn}(" in _seg for _df, _dn in _discovered): + _discovered.add((_f, _n)) + _sources[(_f, _n)] = _seg + _grew = True + if not _grew: + break + +# ⚠️ WHAT MAKES A DISCOVERED FUNCTION AUDITABLE IS ITS SIGNATURE, NOT A LIST +# SOMEBODY MAINTAINS. A reader is a thing you hand a key and get a value from, +# so `def f(key)` / `def f(key, which)` is the shape, and EVERY function of that +# shape in the closure above has to be in the table - which makes the table +# self-maintaining: write the eleventh reader and this check names it on the +# first run. The rest of the closure are CONSUMERS (a `replay()` that takes a +# number `_cfg_default()` produced), and there is no key to feed them; what they +# are held to instead is that they do not do their own key matching, one check +# further down. +def _key_shaped(src): + _sig = src.split("(", 1)[1].split(")", 1)[0] if "(" in src else "" + _args = [a.split("=")[0].split(":")[0].strip() + for a in _sig.split(",") if a.strip()] + return _args[:1] == ["key"] + + +_key_takers = {k for k in _discovered if _key_shaped(_sources[k])} +_consumers = _discovered - _key_takers - _HELPERS +print(f" {len(_discovered)} functions in the config.yaml read-closure; " + f"{len(_key_takers)} take a key and are fed the two-inverter config, " + f"{len(_HELPERS)} are helpers, {len(_consumers)} are consumers") +check("every key-taking reader in the suite is in the audited table - the " + "table is not maintained by hand, the SIGNATURE decides, so the eleventh " + f"reader cannot arrive unwatched. Unaudited: {sorted(_key_takers - _AUDITED)}", + _key_takers == _AUDITED) +check("...and the closure is transitive, so a reader that never names the file " + "because it goes through a helper is still in it - which is where the " + "first draft of this audit failed, on the two functions CFG-04 changed", + ("test_peakguard.py", "read_schema_max") in _discovered + and "config.yaml" not in _sources[("test_peakguard.py", "read_schema_max")]) + +# --- and now RUN each of them against the two-inverter config ----------------- +_answers, _broken = {}, [] +for _key in sorted(_AUDITED): + _file, _fn = _key + _ns = {"io": io, "re": _re, "ast": ast, "Path": Path, "HERE": _TMPCFG, + "_CFG_TXT": _two_cfg, "_CFG_TEXT": _two_cfg, "_txt": _two_cfg, + "_cfg_txt": _two_cfg, "_cfg_src": _two_cfg, + "_cfg_lines": _two_cfg.splitlines(), + "_SCHEMA_AT": _two_cfg.index("\nschema:")} + # Every function of that file that is in the read-closure, so a reader + # that delegates (read_schema_max -> _schema_bound -> _cfg_halves) resolves. + for _dep in sorted(_HELPERS | _key_takers): + if _dep[0] == _file and _dep != _key: + exec(_sources[_dep], _ns) # noqa: S102 + exec(_sources[_key], _ns) # noqa: S102 + _f = _ns[_fn] + try: + _answers[_key] = _f("max_w", 1) if _fn == "_schema_bound" else _f("max_w") + except Exception as _err: # noqa: BLE001 + # ⚠️ AN EXCEPTION IS A PASS HERE. "Handle the per-inverter shape or FAIL + # LOUDLY on it" - a reader that refuses to answer an ambiguous question + # is doing the second, and it is the answer that quietly names inverter + # #1 that this part is hunting. + _answers[_key] = f"raised {type(_err).__name__}" +for _key, _got in sorted(_answers.items()): + print(f" {_key[0]}:{_key[1]}('max_w') -> {_got!r}") + if _NESTED_W in str(_got): + _broken.append(f"{_key[0]}:{_key[1]}") + elif _got is None: + _broken.append(f"{_key[0]}:{_key[1]} (None - answered nothing at all)") + +check("⚠️ AC 4/AC 6: NOT ONE reader answers with the PER-INVERTER number when " + "fed a two-inverter config. A reader that silently returns inverter #1's " + f"value fails right here. Offenders: {_broken}", not _broken) +check("...and every one of them still answers SOMETHING - a reader that went " + "None on the new shape would take its whole check with it, which is " + "fail-safe but is not the same as handling it", + all(v is not None for v in _answers.values())) +check("...and the answers are the SITE numbers: the options half says the " + "shipped default 2000, the schema half says the shipped ceiling 5000", + _answers[("test_peakguard.py", "read_option")] == 2000.0 + and _answers[("test_peakguard.py", "read_schema_max")] == 5000.0 + and _answers[("test_inverter.py", "read_schema_max")] == 5000.0 + and _answers[("test_tariff.py", "shipped_option")] == 2000.0 + and _answers[("test_freeze.py", "_cfg_default")] == 2000.0 + and _answers[("test_price.py", "shipped")] == 2000 + and _answers[("test_maintenance.py", "shipped_default")] == "2000" + and _answers[("test_maintenance.py", "schema_range")] == (100.0, 5000.0)) + +# --- the reads that are NOT functions ---------------------------------------- +# ⚠️ SAID PLAINLY: THIS ONE IS A TEXT BACKSTOP, NOT A BEHAVIOURAL CHECK, and it +# is here because six files read config.yaml at MODULE level (test_validate's +# `_shipped` regex, test_watchdog's `circulation_*` scrape, test_arbitrage's and +# test_price_forecast's `\n key:` counts) where there is no function to call. +# Every one of those is already anchored at exactly two spaces and none of them +# changed verdict on the two-inverter file - but the way to keep that true is to +# ban the idiom that was not: a stripped line, matched by prefix, first hit +# wins. If a module-level read ever needs to become auditable, make it a +# function and it lands in the table above automatically. +# ⚠️ THE NEEDLE IS ASSEMBLED AT RUNTIME so that this check's own source does +# not match it, and it is scoped to lines within ten of a config.yaml +# reference: the same three tokens over a plugin DOCSTRING, looking for a +# SOURCES: line, is an entirely different thing (test_price.py part 4) and a +# blanket grep fails on it. +_NEEDLE = ".strip()" + ".startswith(" +_NEAR = ("config.yaml", "_CFG", "_cfg", "_txt") +_permissive = [] +for _path in sorted(HERE.glob("test_*.py")): + _body = io.open(_path, encoding="utf-8").read() + if "config.yaml" not in _body: + continue + _lines = _body.splitlines() + for _n, _line in enumerate(_lines, 1): + if _NEEDLE not in _line: + continue + _window = "\n".join(_lines[max(0, _n - 11):_n]) + if any(_w in _window for _w in _NEAR): + _permissive.append(f"{_path.name}:{_n}") +check("no check in this suite matches a config.yaml key on a STRIPPED line any " + "more - the idiom that made indentation invisible, and with it the " + f"difference between the site's max_w and one inverter's. Found: {_permissive}", + not _permissive) + +# ⚠️ AND THE ADD-ON ITSELF READS NONE OF THIS. The Dockerfile does not copy +# config.yaml into the image, so every reader above is a CHECK reading the +# shipped file, and app/ code cannot have this class of bug at all. Asserted, +# because "the running add-on cannot read it" is load-bearing for CFG-03's whole +# single-source argument and is exactly the kind of fact that stops being true. +# ⚠️ A STRING CONSTANT, NOT A MENTION. Half of app/ discusses config.yaml in +# its comments - that is CFG-03 working - and grepping the text would fail on +# the documentation rather than on a read. +_app_opens = [] +for _f in sorted((HERE / "app").glob("*.py")): + for _node in ast.walk(ast.parse(io.open(_f, encoding="utf-8").read())): + if isinstance(_node, ast.Constant) and _node.value == "config.yaml": + _app_opens.append(_f.name) +check("no module under app/ so much as names config.yaml as a PATH, so the " + "first-match hazard is confined to the checks - CFG-03's in-code mirrors " + f"exist precisely because the running add-on cannot open this file. " + f"Found: {sorted(set(_app_opens))}", not _app_opens) + + +# ============================================================================== +print() +print("12. mutation: what defeats this card, and which check bites") # Each row is a one-line defeat of something above, with the check that catches # it. ⚠️ THESE WERE RUN, NOT REASONED: every one was applied to the shipped @@ -1372,9 +2049,61 @@ MUTATIONS = [ "test_inverter.py part 6: 'the next heartbeat does NOT put the old shares " "back' - a hazard THIS CARD created, because the heartbeat re-emits a " "share now and not the site number"), + # --- CFG-04's ten, run the same way (tools/mutation_run.sh) ------------- + ("test_inverter.py's `read_schema_max()` back to matching a STRIPPED line", + "part 11: 'NOT ONE reader answers with the PER-INVERTER number' names it " + "as an offender. ⚠️ THE SHIPPED FILE CANNOT CATCH THIS - the two `max_w` " + "bounds are the same int(100,5000) - which is why part 11 executes the " + "readers against a synthetic config where they differ"), + ("test_peakguard.py's `read_option()` back to whole-file first match", + "part 11, same check, plus the text backstop naming test_peakguard.py:234"), + ("test_peakguard.py's `_schema_bound()` back to a stripped line", + "part 11, which names BOTH it and `read_schema_max()` - the caller " + "delegates, so one edit breaks two readers and the transitive closure " + "finds both"), + ("`build_roster()` inheriting the flat `soc_entity` for an entry that " + "omits it", + "part 9: 'an entry that omits soc_entity has NO SoC sensor'. ⚠️ THIS ONE " + "SURVIVED THE FIRST RUN and the FIXTURE was changed, not the rule: the " + "flat keys were blank, so inherited and not-inherited were the same empty " + "string. MUTATION.md's own failure mode, found in this file"), + ("`build_roster()` inheriting the flat `max_w` for an entry that omits it", + "part 9: '...and an entry that omits max_w does not inherit the flat " + "rating either'"), + ("validate.py rule 9(e) dropped - a flat `setpoint_entity` beside a " + "declared roster silently ignored", + "part 10: 'a flat setpoint_entity left set BESIDE a declared roster is " + "refused'"), + ("validate.py rule 5 back to grading `sign_disagree_w` against the FLAT " + "`max_w`", + "part 10: 'A PERMISSIVE READER INSIDE THE VALIDATOR', with the " + "counterfactual beside it so a rule that refused everything would not pass"), + ("main.py's per-unit range check put back inside `if entity:`", + "part 10: 'RUN, NOT GREPPED'. ⚠️ THIS ONE SURVIVED THE FIRST RUN TOO - the " + "check asserted the call's source line was present, and re-indenting it " + "leaves the line present. It drives `run_control()` now, under a deadline, " + "because the mutation's real symptom is a HANG and not a red"), + ("config.yaml no longer shipping the `inverters: []` default", + "part 9: 'config.yaml SHIPS inverters: []'. The consequence is not in " + "this suite at all - Supervisor answers `Missing option 'inverters' in " + "root` and the add-on will not start after the upgrade - which is why the " + "check is on the file and the measurement is in " + "DECISIONS/CFG-04-inverter-roster.md §B. ⚠️ THE PROBE ITSELF WAS " + "REGRESSION-BLIND TO THIS MUTATION until the review: its section B " + "asserted `not ok` on a DIFFERENT blob, so deleting the default left it " + "printing PASS. Section B now varies one key and asserts on the KEY NAME " + "in the refusal, and the mutation was re-run against the real image: " + "`2 CASE(S) FAILED`, exit 1"), + ("validate.py allowing two roster entries with the SAME name", + "part 10: 'two entries with the SAME name are refused' - one Store " + "namespace for two units is DEV-02's defect rebuilt in YAML"), ] -# ⚠️ TWO OF THESE SURVIVED THE FIRST TIME AND THE CHECKS WERE CHANGED, NOT THE -# TABLE. The serialised dispatch and the shared `last_beat` both passed while +# ⚠️ FOUR OF THESE SURVIVED THE FIRST TIME AND THE CHECKS WERE CHANGED, NOT THE +# TABLE - two in DEV-02, two more in CFG-04, and the CFG-04 pair are recorded in +# their own rows above because both are the same mistake in different clothes: a +# check that observed the TEST FIXTURE (blank flat keys) or the SOURCE TEXT (a +# line that survives re-indentation) rather than the behaviour it was named +# after. The serialised dispatch and the shared `last_beat` both passed while # part 6 gathered its OWN `write_setpoint()` calls and computed its OWN `due` # list - a check on this file rather than on the add-on. `send_heartbeats()` was # extracted from `run_control()` so that the shipped dispatch is reachable by a @@ -1386,7 +2115,7 @@ for what, caught in MUTATIONS: check(f"all {len(MUTATIONS)} defeats of this card are caught by a named check " f"above. A guard whose checks survive its own defeat is not a guard, and " f"this repository has three separate records of exactly that", - len(MUTATIONS) == 24) + len(MUTATIONS) == 34) print() diff --git a/goodwe_controller/test_peakguard.py b/goodwe_controller/test_peakguard.py index 44e27a6..0faf418 100644 --- a/goodwe_controller/test_peakguard.py +++ b/goodwe_controller/test_peakguard.py @@ -178,20 +178,65 @@ def fresh_store(name: str) -> PeakStore: return PeakStore(str(TMP / f"{name}.db")) -def read_option(key: str): - """The shipped default for `key`, out of config.yaml. +# ⚠️⚠️ THESE THREE READERS WERE CHANGED BY CFG-04, AND THE OLD ONES WERE THE +# PERMISSIVE KIND. Read this before touching them. +# +# WHAT THEY USED TO DO. `read_option()` walked the whole file and took the FIRST +# line whose STRIPPED text started with `:`; `_schema_bound()` did the same +# from `schema:` onwards. The docstring justified first-match by naming the one +# collision that existed - `peak_cap_w:` appears once under `options:` and once +# under `schema:` - and it was right about that one. +# +# WHY THAT STOPPED BEING SAFE. CFG-04's `inverters:` is a LIST OF OBJECTS, and +# its entries carry a `max_w:` of their own, indented six spaces under +# `schema:`. `line.strip()` throws the indentation away, so `read_schema_max +# ("max_w")` began answering with the PER-INVERTER bound instead of the site +# one - silently, and in this file's case correctly, because the two bounds +# happen to be the same `int(100,5000)` today. A reader that returns inverter +# #1's number and keeps passing is the failure mode this repository is named +# after: green, and not watching. config.yaml declares `inverters:` BEFORE +# `max_w:` deliberately so that the trap stays baited rather than sorted out by +# an ordering nobody would maintain. +# +# WHAT THEY DO NOW. A top-level option and a top-level schema key are at EXACTLY +# two spaces; anything nested is at four or more. So the anchor is the +# indentation, the file's two halves disambiguate `peak_cap_w` (the options half +# for a default, the schema half for a bound) and first-match is not relied on +# at all. And where a key really is ambiguous - the same key twice at top level +# in the same half - these RAISE instead of picking one, because a reader that +# guesses is how this got here. +# +# test_inverter.py part 12 runs all of them against a synthetic TWO-INVERTER +# config.yaml whose nested numbers differ from its flat ones, which is the only +# way to tell a reader that is anchored from one that is lucky. +def _cfg_halves(): + body = (HERE / "config.yaml").read_text(encoding="utf-8") + marker = body.index("\nschema:") + return body[:marker], body[marker:] - ⚠️ `peak_cap_w:` appears TWICE - once under `options:`, once under `schema:`. - test_tariff.py's part 9 documents the trap; this takes the FIRST match, which - is the options block, i.e. the number an install actually lands on. - """ - for line in (HERE / "config.yaml").read_text(encoding="utf-8").splitlines(): - if line.strip().startswith(f"{key}:"): - try: - return float(line.split(":", 1)[1].strip()) - except ValueError: - continue - return None + +def _top_level(half: str, key: str) -> list: + """Every ` key: ...` line at EXACTLY two spaces. Raises if there are two.""" + hits = [ln for ln in half.splitlines() + if ln.startswith(f" {key}:") and not ln[2:3].isspace()] + if len(hits) > 1: + raise AssertionError( + f"config.yaml declares {key!r} {len(hits)} times at the top level " + f"of one block: {hits!r}. This reader will not pick one for you - " + "an option with two defaults is a question for whoever wrote the " + "second, not a tie for a check to break.") + return hits + + +def read_option(key: str): + """The shipped default for `key`, out of config.yaml's `options:` block.""" + hits = _top_level(_cfg_halves()[0], key) + if not hits: + return None + try: + return float(hits[0].split(":", 1)[1].strip()) + except ValueError: + return None def read_schema_max(key: str): @@ -210,11 +255,9 @@ def read_schema_min(key: str): def _schema_bound(key: str, which: int): - body = (HERE / "config.yaml").read_text(encoding="utf-8") - marker = body.index("\nschema:") - for line in body[marker:].splitlines(): + for line in _top_level(_cfg_halves()[1], key): st = line.strip() - if st.startswith(f"{key}:") and "(" in st and ")" in st: + if "(" in st and ")" in st: return float(st[st.index("(") + 1:st.rindex(")")].split(",")[which]) return None diff --git a/goodwe_controller/test_tariff.py b/goodwe_controller/test_tariff.py index a071163..35f0ca4 100644 --- a/goodwe_controller/test_tariff.py +++ b/goodwe_controller/test_tariff.py @@ -791,17 +791,29 @@ check("the committed replay fixtures are present - a regression fixture CI " "cannot see is not a regression fixture", len(REPLAY) >= 2) # ⚠️ `peak_cap_w:` appears TWICE in config.yaml - once under `options:` with the -# default, once under `schema:` with `int(500,15000)`. Taking the last match -# would blow up on the schema line, and taking any match that parses would be -# luck; the FIRST is the options block, which is the number an install lands on. -SHIPPED_CAP_W = None -for line in (HERE / "config.yaml").read_text(encoding="utf-8").splitlines(): - if line.strip().startswith("peak_cap_w:"): - try: - SHIPPED_CAP_W = float(line.split(":", 1)[1].strip()) - except ValueError: - continue - break +# default, once under `schema:` with `int(500,15000)`. This used to take the +# FIRST match on a STRIPPED line and call that the options block; CFG-04 made +# stripped-line-first-match unsafe in this file's neighbours (a `max_w:` nested +# six spaces under `inverters:` in the schema block began outranking the +# top-level one), so it is anchored the same way they now are: the options half +# of the file, at exactly two spaces of indentation. `peak_cap_w` is not a +# per-inverter key today and this reader was never wrong - it is the IDIOM that +# was, and leaving one copy of it here would be leaving the next per-inverter +# key a place to land. test_inverter.py part 12 runs this function against a +# two-inverter config. +def shipped_option(key): + """The `options:` default for `key`, anchored at two spaces. None if absent.""" + body = (HERE / "config.yaml").read_text(encoding="utf-8") + for line in body[:body.index("\nschema:")].splitlines(): + if line.startswith(f" {key}:") and not line[2:3].isspace(): + try: + return float(line.split(":", 1)[1].strip()) + except ValueError: + return None + return None + + +SHIPPED_CAP_W = shipped_option("peak_cap_w") check("config.yaml's shipped peak_cap_w was read out of the file rather than " "typed here", SHIPPED_CAP_W is not None) diff --git a/goodwe_controller/test_validate.py b/goodwe_controller/test_validate.py index 0391d2b..535aaa6 100644 --- a/goodwe_controller/test_validate.py +++ b/goodwe_controller/test_validate.py @@ -190,10 +190,26 @@ check("...and the refusal names the actuator and every writer aimed at it, so " # inconveniences, and then none of this is enforced. Two inverters with two # entities are two writers on TWO actuators, which is not the non-goal - it is # the whole point of the per-inverter roster. +# +# ⚠️ THE ENTRIES BELOW GAINED `name` AND `max_w` WHEN CFG-04 LANDED, AND THE +# HAZARD FIXTURE ABOVE DELIBERATELY DID NOT. SAFETY-28 wrote both against a +# shape that did not exist yet and guessed `id`; CFG-04 chose `name`, and made +# `name`/`setpoint_entity`/`max_w` required of every entry (rule 9). The hazard +# fixture is left in the OLD guessed shape on purpose - the claim it makes is +# that `actuator_writers()` finds writers STRUCTURALLY, under a container and a +# key spelling this file was never told, and rewriting it into today's shape +# would quietly turn it into a check that the census knows about `inverters`. +# This one is the opposite claim - that a real, valid, two-inverter config is +# ACCEPTED - so it has to be a config the shipped validator actually accepts. _PER_INVERTER_OK = {k: v for k, v in base().items() if k != "setpoint_entity"} _PER_INVERTER_OK["inverters"] = [ - {"id": "inverter-a", "setpoint_entity": "number.goodwe_setpoint"}, - {"id": "inverter-b", "setpoint_entity": "number.goodwe_setpoint_b"}] + {"name": "inverter-a", "setpoint_entity": "number.goodwe_setpoint", + "max_w": 2000}, + # ⚠️ INVERTER B DOES NOT EXIST. 2000 W here is the SHIPPED default repeated, + # not a rating measured off a second device - SPIKE-04 measures that when + # there is one to measure. Nothing in this file depends on the number. + {"name": "inverter-b", "setpoint_entity": "number.goodwe_setpoint_b", + "max_w": 2000}] check("two inverters with their own entities are ACCEPTED - one writer each, " "no shared actuator, and that is CFG-04's whole purpose", validate_startup_config(_PER_INVERTER_OK, -5000, 5000) == []) @@ -219,11 +235,28 @@ check("...and entity ids are folded to lower case, so a second writer cannot " why(validate_startup_config(base(inverters=[{"setpoint_entity": "NUMBER.GOODWE_SETPOINT"}]), -5000, 5000), "one actuator")) +# ⚠️ THIS CHECK CHANGED SHAPE IN CFG-04, AND THE REASON IS A DECISION RATHER +# THAN AN ADJUSTMENT. It used to assert that a blank per-inverter entity left +# the whole config ACCEPTED - "blank is how an operator leaves a slot +# unconfigured". CFG-04 decided that is not true of a ROSTER ENTRY: an entry +# with no `setpoint_entity` is a unit that `allocator.plan()` still gives a +# share of the site target to and that nothing can command, on a status page +# reporting a site at full strength. Rule 9 refuses it by name. The claim about +# the CENSUS is untouched and is now asserted directly on the census, where it +# always belonged - a blank contributes no writer, so it cannot manufacture a +# two-writer refusal out of two unconfigured slots. check("a BLANK per-inverter entity is not a writer - the same rule the scalar " - "key gets, since write_setpoint() returns immediately on a blank entity " - "and blank is how an operator leaves a slot unconfigured", - validate_startup_config(base(inverters=[{"setpoint_entity": " "}]), - -5000, 5000) == []) + "key gets, since write_setpoint() returns immediately on a blank entity", + actuator_writers(base(inverters=[{"setpoint_entity": " "}, + {"setpoint_entity": ""}])) + == {"number.goodwe_setpoint": ["setpoint_entity"]}) +check("...but the ENTRY is refused all the same, by rule 9 and not by the " + "census: a roster slot nothing can write still takes a share of the site " + "target every cycle and delivers none of it", + why(validate_startup_config(base(inverters=[{"name": "inverter-b", + "max_w": 2000, + "setpoint_entity": " "}]), + -5000, 5000), "has no setpoint_entity")) # --------------------------------------------------------------------------- # print("3. a meter that can never read") diff --git a/goodwe_controller/tools/supervisor_shape_probe.py b/goodwe_controller/tools/supervisor_shape_probe.py new file mode 100644 index 0000000..49d7356 --- /dev/null +++ b/goodwe_controller/tools/supervisor_shape_probe.py @@ -0,0 +1,210 @@ +"""CFG-04: does Supervisor accept a list-of-objects option? MEASURED, not assumed. + +⚠️ WHY THIS SCRIPT EXISTS AND WHY IT IS COMMITTED. config.yaml had no +list-of-objects option anywhere before CFG-04, and `static_profile` carries a +structured value as a JSON STRING in a `str?` precisely because nobody knew +whether the Supervisor's options validator supported a nested one. That is a +question about a specific version of a specific piece of somebody else's +software, and the honest way to answer it is to run that software. + +WHAT IT RUNS. Supervisor's OWN `AppOptions` validator and its OWN +`SCHEMA_APP_CONFIG`, out of the release image, against THIS repository's +config.yaml. Nothing here is a re-implementation of the validation rules; if +Supervisor changes them, this script changes its answer. + +HOW TO RUN IT (the version is the one `ha info` reports on the target install - +2026.08.0 is what 192.168.2.158 ran when CFG-04 landed): + + docker run --rm \ + -v "$PWD:/w:ro" -v "$PWD/tools:/probe:ro" -v /tmp/out:/out \ + --entrypoint python3 \ + ghcr.io/home-assistant/amd64-hassio-supervisor:2026.08.0 \ + /probe/supervisor_shape_probe.py + +⚠️ IT TOUCHES NOTHING. It reads config.yaml and runs a validator in a throwaway +container. It does not talk to the live Supervisor, does not POST options and +does not install anything - the schema conflict in docs/deploy-rollback.md §4 is +exactly what you do NOT want to discover on a running site. + +WHAT IT ANSWERED, 2026-09-04, on 2026.08.0 - the full output is in +DECISIONS/CFG-04-inverter-roster.md: + + A. The manifest, list-of-objects schema and all, is ACCEPTED. + B. The shipped `options:` block validates as it stands, and the SAME block + with ONLY `inverters` removed is refused BY NAME - `Missing option + 'inverters' in root`. That is the one measurement the `inverters: []` + default rests on, and it is an ISOLATED counterfactual: exactly one key + varies between the two runs, so the refusal cannot be about anything else. + B2. A reconstructed pre-0.6.0 options blob, merged with those defaults, + validates. Its RAW form is refused - but for whichever required key the + reconstruction happens to lack FIRST, which is an artefact of the + reconstruction and NOT evidence about `inverters`. B is that evidence. + C. A two-inverter blob validates, per-entry types enforced. + D. An entry missing `name`, `setpoint_entity` or `max_w` is refused BY NAME, + and a JSON string where the list belongs is refused outright. +""" +import sys, json, copy +sys.path.insert(0, "/usr/src/supervisor") +import voluptuous as vol +from voluptuous.humanize import humanize_error +from deepmerge import Merger +from supervisor.apps.validate import SCHEMA_APP_CONFIG +from supervisor.apps.options import AppOptions +from pathlib import Path +from supervisor.utils.yaml import read_yaml_file + +MERGER = Merger(type_strategies=[(dict, ["merge"])], + fallback_strategies=["override"], type_conflict_strategies=["override"]) + +raw = read_yaml_file(Path("/w/config.yaml")) +print("== A. the whole config.yaml through SCHEMA_APP_CONFIG ==") +try: + cfg = SCHEMA_APP_CONFIG(raw) + print(" PASS Supervisor accepts the add-on manifest, version", cfg["version"]) +except vol.Invalid as e: + print(" FAIL ", humanize_error(raw, e)); sys.exit(1) + +schema, defaults = cfg["schema"], cfg["options"] +print(" schema declares", len(schema), "keys; inverters ->", json.dumps(schema["inverters"])) +# .get, not [], deliberately: if someone deletes the `inverters: []` default +# this line must not crash the probe before section B gets to REFUSE it. +print(" options ship inverters ->", json.dumps(defaults.get("inverters"))) + +def validate(options): + v = vol.Schema(vol.All(dict, AppOptions(None, schema, cfg["name"], "local_goodwe_controller"))) + try: + return True, v(copy.deepcopy(options)) + except vol.Invalid as e: + return False, str(e) + +FAILED = [] + +def show(t, ok, d=""): + print(f" {'PASS' if ok else 'FAIL'} {t}") + if d: print(" " + str(d)[:300]) + if not ok: FAILED.append(t) + +# The live install's persisted options, reconstructed from HANDOVER.md §1 and +# docs/deploy-rollback.md §0/§B.5. No `inverters` key: it did not exist. +LIVE = { + "meter_entity": "sensor.p1_meter_active_power", "meter_invert": False, + "soc_entity": "sensor.goodwe_battery_soc", "batt_entity": "sensor.goodwe_battery_power", + "batt_invert": False, + "setpoint_entity": "number.kelder_goodwe_master_stage1_goodwe_setpoint_w", + "meter_source": "homewizard_local", "meter_phases": 1, "meter_max_age_s": 30, + "meter_poll_s": 5, "p1_host": "192.168.2.250", + "max_w": 5000, "gain": 0.6, "slew_w": 1000, "deadband_w": 15, "target_grid_w": -10, + "step_w": 10, "saturation_w": 500, "saturation_cycles": 3, "integrator_max_w": 0, + "heartbeat_s": 10, "stale_input_s": 15, "cycle_floor_s": 15, "auto_start": False, + "maintenance_enabled": False, "estop_fitted": True, "log_level": "info", +} + +print("\n== B. THE LOAD-BEARING ONE: an options blob with no `inverters` key ==") +# ⚠️ THIS IS AN ISOLATED COUNTERFACTUAL AND IT HAS TO STAY ISOLATED. The claim +# the `inverters: []` default rests on is about ONE key, so exactly ONE key +# varies between these two runs: the full shipped `options:` block, and the same +# block with `inverters` removed and nothing else touched. Feeding the validator +# some other, INCOMPLETE blob would also produce a refusal - but about whichever +# required key that blob lacks first, which is a different measurement wearing +# this one's label. §B2 below is that other blob, and it is labelled as such. +ok, out = validate(dict(defaults)) +show("the shipped `options:` block validates AS IT STANDS - so anything refused " + "on the next line is refused for the one key that was removed", ok, + "" if ok else out) +_no_inv = {k: v for k, v in defaults.items() if k != "inverters"} +ok_ni, out_ni = validate(_no_inv) +# ⚠️ THE ASSERTION IS ON THE KEY NAME, NOT ON `not ok`, AND THAT IS THE POINT. +# A refusal is only evidence for THIS key if it NAMES this key. Asserted as a +# bare `not ok`, this line would go on printing PASS after someone deleted +# `inverters: []` from config.yaml - blind to the exact regression it is the +# committed evidence for. Asserted on the string, it goes red: with the default +# gone the line above fails too, because then nothing distinguishes the two runs. +_names_it = (not ok_ni) and "Missing option 'inverters' in root" in str(out_ni) +show("...and the SAME block with ONLY `inverters` removed is REFUSED, BY NAME - " + "which is every options blob written before 0.6.0, since the key did not " + "exist when they were written", _names_it, out_ni) + +print("\n== B2. AC3: a reconstructed pre-0.6.0 blob upgrades into this version ==") +# ⚠️ WHAT THIS SECTION IS AND IS NOT. LIVE is an INCOMPLETE reconstruction - it +# carries the keys HANDOVER.md and deploy-rollback.md record and no others, so +# it is missing many required keys that have nothing to do with this card. Its +# raw refusal is therefore about whichever of THOSE the validator reaches first +# (it has printed `meter_freeze_s` and `p1_phase_export_entities` on different +# runs), NOT about `inverters`. It is kept because the MERGED half is a real +# demonstration that `deepmerge(options:, stored)` is what App.options is; the +# raw half is printed only so the merge has something to be contrasted with. +# The `inverters` measurement is §B, where one key and only one key varies. +ok, out = validate(LIVE) +show("the reconstructed raw blob is REFUSED - for an unrelated missing key, " + "because the reconstruction is incomplete; see the refusal printed below " + "and §B for the `inverters` measurement", not ok, out) +merged = MERGER.merge(copy.deepcopy(defaults), copy.deepcopy(LIVE)) +ok, out = validate(merged) +show("the same blob MERGED with these defaults - what App.options actually is", ok, + f"inverters={out.get('inverters')!r} setpoint_entity={out.get('setpoint_entity')!r} " + f"max_w={out.get('max_w')!r}" if ok else out) + +print("\n== C. AC2: a two-inverter blob an operator would POST ==") +TWO = dict(LIVE, setpoint_entity="", inverters=[ + {"name": "inverter-a", "setpoint_entity": "number.goodwe_a_setpoint_w", + "soc_entity": "sensor.goodwe_a_soc", "batt_entity": "sensor.goodwe_a_battery_power", + "max_w": 5000}, + {"name": "inverter-b", "setpoint_entity": "number.goodwe_b_setpoint_w", + "soc_entity": "sensor.goodwe_b_soc", "batt_entity": "sensor.goodwe_b_battery_power", + "max_w": 3000}]) +ok, out = validate(MERGER.merge(copy.deepcopy(defaults), copy.deepcopy(TWO))) +show("two inverters", ok, json.dumps(out.get("inverters")) if ok else out) +# The validated blob, for anyone who wants to POST it. Optional: the probe's +# whole job is the verdicts above, and a missing /out must not fail it. +try: + json.dump(out if ok else {}, open("/out/two_inverter_options.json", "w"), indent=1) +except OSError: + pass + +print("\n== D. what the schema itself refuses, per entry ==") +for title, entry in ( + ("no name", {"setpoint_entity": "number.b", "max_w": 3000}), + ("no setpoint_entity", {"name": "inverter-b", "max_w": 3000}), + ("no max_w", {"name": "inverter-b", "setpoint_entity": "number.b"}), + ("max_w over the range", {"name": "inverter-b", "setpoint_entity": "number.b", "max_w": 9999})): + ok, out = validate(dict(TWO, inverters=[entry])) + show(f"an entry with {title} is refused", not ok, out) +ok, out = validate(dict(TWO, inverters="[]")) +show("a JSON STRING where the list belongs is refused", not ok, out) + + +print("\n== E. the ROLLBACK direction, and the operator's UI ==") +# The old (flat) schema, i.e. what an operator would be running after a +# downgrade, meeting options that carry an `inverters` list. +_flat_schema = {k: v for k, v in schema.items() if k != "inverters"} +_v = vol.Schema(vol.All(dict, AppOptions(None, _flat_schema, cfg["name"], + "local_goodwe_controller"))) +try: + _rolled = _v(MERGER.merge(copy.deepcopy(defaults), copy.deepcopy(TWO))) + show("a schema WITHOUT `inverters` drops the key with a warning rather " + "than refusing - so a downgrade is not the §B trap in reverse", + "inverters" not in _rolled, sorted(_rolled)[:6]) +except vol.Invalid as e: + show("a schema WITHOUT `inverters` drops the key", False, str(e)) + +from supervisor.apps.options import UiOptions # noqa: E402 +try: + _ui = [e for e in UiOptions(None)(schema) if e.get("name") == "inverters"] + show("Supervisor's UI renderer turns it into a repeatable section rather " + "than choking - an operator can add the second inverter in the " + "Configuration tab, which the JSON-string alternative could not offer", + bool(_ui) and _ui[0].get("multiple") is True, json.dumps(_ui)) +except Exception as e: # noqa: BLE001 + show("Supervisor's UI renderer survives the shape", False, f"{type(e).__name__}: {e}") + + +# ⚠️ THE EXIT CODE IS PART OF THE INSTRUMENT. Without it the probe prints FAIL +# and still succeeds, and a transcript is only evidence if a wrong answer is +# distinguishable from a right one by something other than reading it. +print() +if FAILED: + print(f"== {len(FAILED)} CASE(S) FAILED ==") + for t in FAILED: + print(" - " + t) + sys.exit(1) +print("== all cases passed ==")