goodwe-addon/goodwe_controller/DECISIONS/SAFETY-32-per-pack-bms-floor.md

SAFETY-32 — bms_soc_floor per pack, and the two judgement calls it left open

Status: decided and implemented on branch SAFETY-32. Card: SAFETY-32 — "One bms_soc_floor for two packs grades the shallower pack against the deeper one's cut-out", scoped by the card's own PM PROOFREAD to its third bullet only. Bullets 1 (aggregate anti-windup) and 2 (per-instance achievable_w) were verified as already shipped by DEV-02 and CFG-04 and were not touched — main.py's actual_w=self.site_batt_w comment is the record of why the first of those must stay site-level, and it still reads as written.

---

1. The defect, stated in the terms the code uses

bms_soc_floor describes a battery: it is the percent that pack's own BMS cuts out at, register 0x0560 on the reference GW5000S-BP, read as 10. It was a flat option because there was one pack.

peak_soc_floor describes the site's policy: the depth a money-saving discharge may never reach. Rule 8 refuses peak_soc_floor <= bms_soc_floor, because such a floor can never bind — the BMS wins and the status page reports a backstop that does not exist.

At two packs of different models the two cut-outs differ, and one number for both is wrong in the direction that empties a pack: with cut-outs of 10 % and 30 % and peak_soc_floor: 20, the add-on started, and the guard would command a forced discharge that pack B's BMS — not the operator's floor — is the thing that stops. That is rule 8's own failure mode, reached through rule 8.

---

2. DECISION 1 — required per entry, not defaulted from PEAK_OPTION_DEFAULTS

The card left this open and asked for the reason to be recorded.

Decided: REQUIRED of every roster entry, in config.yaml's schema (bms_soc_floor: int(0,50), no trailing ?) and in validate.py rule 8, which refuses a missing one by name.

The reason is that a default here is not a convenience, it is this card's own defect wearing a different hat. A default is one number applied to every pack, which is exactly what the flat key was. An add-on that answers "10 %" for a pack nobody has told it about has invented a property of somebody's battery, and the failure mode of the invention is the one this card exists to remove — the shallower pack protected by a floor that belongs to the deeper one. CFG-03's rule ("defaults live in exactly one place") points the same way: the one mirror PEAK_OPTION_DEFAULTS["bms_soc_floor"] is the default for the flat key, i.e. for the single-inverter shape, and reusing it per entry would make one dict the default in two shapes with two meanings.

It is also the treatment max_w already gets, for the same reason and by the same argument (inverter.py's build_roster() docstring): the per-unit numbers that describe hardware are required, and the per-unit sensors are optional because an absent sensor is a real state.

The cost is stated rather than hidden. This is a schema change: a roster config written before this branch is refused by the Supervisor until each entry gains the key, and every roster fixture in the suite needed it. Nothing in production carries a roster today — inverters: ships [] and the second inverter is ~8 days out — so the migration is paid by fixtures and by whoever writes the first real roster, at a startup refusal that names the entry.

What is NOT inherited. An entry that omits the key does not borrow the flat one. Inverter.bms_soc_floor is then None, peakguard.pack_floor_pct() answers None, and discharge_permitted() already treats an unknown floor as no permission — so even in the window before rule 8 runs (e.g. sim/inject.py, which does not run the startup gate) the absence costs a shave and never a pack. 0.0 was rejected explicitly: it is the most permissive value the key has.

3. DECISION 2 — the site verdict is the MAXIMUM of the packs' own floors

Controller.pack_floor_soc — the minimum measured SoC, the emptiest pack — is unchanged, exactly as the card instructed. What changed is the floor it is graded against. peakguard.binding_soc_floor() reduces the roster's per-pack floors, each of them max(peak_soc_floor, that pack's own cut-out), with a max(). The two reductions pair: a minimum SoC against a maximum floor is the conservative combination, and both move the verdict towards refusing a shave and away from spending a pack. min(soc) <= max(floor) is implied by "some pack is at or below its own floor", so the site verdict is never more permissive than grading each pack separately would be. The converse is not true and is not claimed — a site can be refused while every pack individually has headroom, which costs a shave.

3a. ⚠️ FIX ROUND — the reduction was written and argued, and NOT GRADED

PR #48 was rejected because the argument in §4 below justified writing the reduction and was then not carried into a check. Neither of the two mutations below appears anywhere in §6's twelve-row battery: it graded "the reduction dropped" and "the bare option handed over", and never asked what happens when the reduction is the wrong one, or when the roster handed to it is sliced. Both were applied to app/ by the reviewer and all 19 files stayed green:

mutation	Δ	why it survived
binding_soc_floor(): max(floors) → min(floors)	0 bytes	every roster in the suite was rule-8-valid or one pack long, so floors held identical numbers and max/min/first/last were four spellings of one answer
the decide() call site: self.packs → self.packs[:1]	+4 bytes	part 12g asserted "self.packs" in <the unparsed expression> — and self.packs[:1] contains that substring

The zero-byte one was proved to have actually run rather than assumed: dis.get_instructions showed the LOAD_GLOBALs as [float, pack_floor_pct, any, min], and binding_soc_floor([Pack("a",50,10), Pack("b",50,35)], 20.0) returned 20.0 where the shipped code returns 35.0.

The common cause is this repository's recurring defect — a check whose DATA cannot reach the failure it names. The one check that used a cut-out above the EMS floor (12c's _n1_deep) ran on a roster of ONE, where every possible reduction agrees. So no check in the suite ever evaluated binding_soc_floor() on two packs whose floors differ.

And the rule-8 degeneracy is not a defence, by §4's own words. If "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" is enough to write the reduction, it is enough to grade it. Through sim/inject.py, which does not run rule 8 at all, min(floors) lets the guard command a forced discharge to 20 % on a pack whose BMS cuts out at 35 %.

What was added — no shipped behaviour changed, only what observes it:

• test_peakguard.py part 12c2 evaluates binding_soc_floor() on a roster whose per-pack floors all differ. Four simulated cut-outs — 30, 25, 40, 35 %, every one strictly above peak_soc_floor: 20, so pack_floor_pct() selects the cut-out for each and floors is that tuple in that order. The four candidate reductions are then four different numbers:

  max   = 40   <- the shipped answer
  min   = 25
  first = 30   <- "grade the site against pack A", inside the function
  last  = 35
  mean  = 32.5
  

  The extremes sit at interior roster positions (index 2 is the max, index 1 the min). A two-pack roster structurally cannot separate these — there max and last coincide and so do min and first — and neither can three. Four is the smallest roster that separates all four. The precondition is its own check (habit 1), one pack is then raised past the old maximum (habit 7: the last pack, which was neither extreme, so the line can only move if the reduction read it), and peak_soc_floor is lifted above every cut-out to grade the other side of the conjunction.

• Part 12g is behaviour now, not source text (MUTATION.md habit 4). Real Controller.cycle()s on a two-pack roster, both packs held at the same SoC and everything else identical, with the packs' own cut-outs as the only thing varied:

  cut-outs A 10 / B 18 % -> decide() got soc_floor=20 %, floor_limited=False
  cut-outs A 10 / B 35 % -> decide() got soc_floor=35 %, floor_limited=True
  

  The observable is PeakDecision.soc_floor — the argument, echoed back by the shipped decide() — so the whole path from Controller.inverters to the guard's argument is under test, and the site's verdict flips as well as the number. self.packs[:1] answers 20 % both times. Which option key the floor comes from stays pinned on the source in part 4, because that claim genuinely is about the code's shape; that the option still governs is now graded behaviourally too.

• ⚠️ A THIRD SURVIVOR, FOUND WHILE FIXING THE FIRST TWO, AND RECORDED RATHER THAN QUIETLY REPAIRED. The first cut of part 12g varied only pack B's cut-out — so self.packs[-1:], grade the site against the last pack, agreed with the full roster on both runs and stayed green across all 19 files (+5 bytes). A two-run fixture that only ever moves the second pack pins [:1] and not [-1:]; it is the same defect one step along, and it was in the fix for this defect. Part 12g now runs the same pair of cut-outs in both orders:

  cut-outs A 10 / B 18 % -> soc_floor=20 %, floor_limited=False
  cut-outs A 10 / B 35 % -> soc_floor=35 %, floor_limited=True
  ...the SAME PAIR SWAPPED, A 35 / B 18 % -> soc_floor=35 %, floor_limited=True
  

  [:1] answers 20 % on the second run, [-1:] answers 20 % on the third, and the roster answers 35 % on both. No single-position slice passes all three. This is the mirror 12h already ran on the SoC axis, which is where the shape was borrowed from.

• ⚠️ AND A FOURTH, from the same battery: the EMPTY-roster branch. Rewriting binding_soc_floor([])'s float(soc_floor) to return None (−43 bytes) was green across all 19 files. It is defensive code — Controller.inverters is never empty on a started controller, and inputs_ok refuses the cycle before the guard is reached if it were — so nothing in the suite went near it. It is pinned rather than argued, in both directions (the option when there is one, None when there is not), because a branch with no check is a branch that can be rewritten under a green suite. The check also records why it cannot permit anything either way: pack_floor_soc is None at an empty roster, and discharge_permitted() refuses on a None SoC whatever the floor is.

• wired_over_limit() gained a prepare(ctl) hook, inside its retry loop so every attempt grades the same site, which is the only window in which a second inverter can join the roster before the telegrams are fed.

• In-process mutation rows 22, 22b and 23 record all four survivors, so a later edit that re-degenerates any of these fixtures goes red on a row rather than on nothing.

4. ⚠️ THE HONEST LIMITATION, and it is the reason this record exists

Under any configuration rule 8 accepts, binding_soc_floor() answers peak_soc_floor for every pack. Rule 8 refuses peak_soc_floor <= bms_soc_floor on every unit, so max(peak_soc_floor, bms_i) == peak_soc_floor everywhere the add-on will actually start. The per-pack floor is therefore non-degenerate only against a config rule 8 refuses.

This is not a defect and it is not hidden. It is the same argument validate.unit_caps_indexed() makes about itself, in that function's own words:

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

CFG-04's index desynchronisation is what happened the last time that reasoning was left implicit. So the reduction is written, and it is graded on both sides: test_peakguard.py part 12c sweeps 1775 rule-8-valid (peak_soc_floor, bms_soc_floor) pairs and asserts the answer is the option in every one of them (that is AC 5, measured over the whole option space rather than at the shipped point), and then asserts that on the pair rule 8 refuses the answer is the cut-out — because without that second line the sweep would be satisfied by soc_floor typed straight through, and the mutation that deletes the max() could never bite. It does bite: M2 on the PR.

What IS live at a rule-8-valid roster is the rest of the card: rule 8 now refuses the two-model config that used to start; the permission is answered per pack, so the alarm names the pack that stopped the site rather than "the battery"; and an entry with no stated cut-out is withheld instead of defaulted.

5. ⚠️ THE RESIDUAL — a per-pack EMS floor is NOT this card

peak_soc_floor is still one number for the site, so the card's own "or the deeper pack is needlessly withheld" half is not closed and this record does not pretend it is. With cut-outs of 10 % and 18 %, rule 8 obliges the operator to set the EMS floor above 18, and the 10 % pack is then held above its own need. Closing that means peak_soc_floor becoming a per-entry key too, which is a second option migration and a product decision about what the site-wide number then means. The card scoped itself to bms_soc_floor and explicitly told this developer not to widen it, so it is filed here as the open question it is rather than smuggled in.

Also still not in this card, and named by the shipped code already: the charge ceiling is the mirror image — pack_floor_soc is the wrong reduction for it, because a ceiling is the fullest pack and arbitrage's soc_ceiling takes one number. main.py:932-936 says so, and it belongs to CFG-04/SPIKE-04's per-pack bound. Nothing here touched it.

6. What was changed, and one thing that was found on the way

• config.yaml — bms_soc_floor: int(0,50) required per inverters: entry; the flat key and its default are untouched and are documented as the single-inverter shape's own cut-out.
• app/inverter.py — Inverter.bms_soc_floor; _entry_bms_floor() (None, not a borrowed number and not 0.0); _flat_bms_floor().
• app/peakguard.py — Pack, pack_floor_pct(), pack_permissions(), binding_soc_floor(). discharge_permitted() is unchanged and is still the one place the question is answered.
• app/validate.py — unit_bms_floors_indexed(); rule 8 grades every unit and names it via the roster index the value came off.
• app/main.py — Controller.packs; the decide(soc_floor=…) reduction; the floor-limited notification names the pack(s) at or below their own floor, at a roster of two or more only.
• tools/supervisor_shape_probe.py — re-run, and repaired. MUTATION.md habit 8: a committed transcript is a check and it decays. Making the key required broke §C outright and, worse, left §D printing PASS on a refusal about the wrong key — its "no max_w" case was answered Missing option 'bms_soc_floor'. §D is now an isolated counterfactual over a merged, accepted baseline with exactly one key varied per case, asserting the key's name in the refusal, and it carries a bms_soc_floor case in both directions. Re-run against ghcr.io/home-assistant/amd64-hassio-supervisor:2026.08.0: all cases passed.
• app/inverter.py, found by the mutation battery — _flat_bms_floor() was written with the default in two places (the .get() fallback and the blank test). Deleting either one survived, because each covered for the other: MUTATION.md's masking section in three lines of option reading. It has one defaulting line now, and M12 takes six checks red.

⚠️ No measurement of inverter B appears anywhere in this change. It does not physically exist. Every second-pack number in a fixture, in the probe and in config.yaml's commented example is simulated and labelled as such; only the 10 % on inverter A is read off hardware.

---

7. The probe transcript, re-run 2026-09-04 on 2026.08.0

Command (unchanged, in tools/supervisor_shape_probe.py's own docstring):

    docker run --rm -v "$PWD:/w:ro" -v "$PWD/tools:/probe:ro" \
      --entrypoint python3 \
      ghcr.io/home-assistant/amd64-hassio-supervisor:2026.08.0 \
      /probe/supervisor_shape_probe.py

It reads config.yaml and runs Supervisor's own AppOptions validator in a throwaway container. It touches no live Supervisor, POSTs nothing and installs nothing.

== 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, "bms_soc_floor": 10}, {"name": "inverter-b", "setpoint_entity": "number.goodwe_b_setpoint_w", "soc_entity": "sensor.goodwe_b_so

== D. what the schema itself refuses, per entry ==
  PASS  the per-entry BASELINE is ACCEPTED, so every refusal below has nowhere else to come from
        [{"name": "inverter-b", "setpoint_entity": "number.b", "max_w": 3000, "bms_soc_floor": 10}]
  PASS  an entry with no name is refused, and the refusal NAMES it
        Missing option 'name' in inverters in GoodWe RS485 Controller (local_goodwe_controller)
  PASS  an entry with no setpoint_entity is refused, and the refusal NAMES it
        Missing option 'setpoint_entity' in inverters in GoodWe RS485 Controller (local_goodwe_controller)
  PASS  an entry with no max_w is refused, and the refusal NAMES it
        Missing option 'max_w' in inverters in GoodWe RS485 Controller (local_goodwe_controller)
  PASS  an entry with no bms_soc_floor (SAFETY-32) is refused, and the refusal NAMES it
        Missing option 'bms_soc_floor' in inverters in GoodWe RS485 Controller (local_goodwe_controller)
  PASS  an entry with max_w over the range is refused, and the refusal NAMES it
        value must be at most 5000.0
  PASS  an entry with bms_soc_floor over the range (SAFETY-32) is refused, and the refusal NAMES it
        value must be at most 50.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 ==