glenn schrooyenandClaude Opus 5
acbdf41248
Two survivors from TARIFF-01's review, closed in test_tariff.py only. app/ is
untouched and no shipped behaviour changes. Two further items on the card were
already closed in shipped code and are struck rather than re-implemented.
ITEM 1 - the per-phase reduction's PLUMBING was unobservable.
Mutating PeakStore.record_block()'s `billed_w(block, policy)` to
`billed_w(block, POLICY_NET)` (+4 B) leaves all 17 suites green. Measured
three times now: by TARIFF-01's reviewer, re-run through TEST-15's purge
harness this session, and re-run again here on a clean uncontended tree.
The product code is correct - tariff.py:363 really is
`def record_block(self, block, policy: str = POLICY_NET)` and it really does
forward the argument. What was missing is any check that could tell that
apart from a hardcoded call, and the reason is the third instance of one
shape: A STIMULUS EVALUATED WHERE THE QUANTITY IT GRADES IS DEGENERATE, after
WITNESS-02's span normalisation and TARIFF-01's own DST window.
billed_w() short-circuits to block.offtake_avg_w whenever
per_phase_offtake_avg_w is absent. Every block reaching a store anywhere in
the suite was single-phase or built as _CB(..., None) - so NET, SUM and MAX
collapsed onto one number and the argument could not be observed at the call
site. Section 7 grades the FUNCTION on an unbalanced fixture and says nothing
about its CALLER.
So section 7's own (3000, 500, -200) W block - whose three reductions are
3300 / 3500 / 3000 W, asserted distinct three lines above - is now
round-tripped THROUGH the store under each policy and read back from both
tables (peak() and recompute_month(), so both writes in that single
transaction are graded). record_block() with no policy is checked to bank the
NET figure. An anti-degeneracy pin is stated over the three BANKED numbers,
not over vals, because it is the banked ones this triple is made of.
Direction, which is why this is worth plumbing-testing: NET is the SMALLEST
of the three here, because the exporting phase nets off. A store that quietly
billed NET while asked for MAX would UNDER-record the month's peak, and an
under-recorded peak is maximum headroom for SAFETY-07 - the same
under-guarding direction as the getattr(..., 0.0) default SAFETY-07 already
removed from project().
ITEM 2 - the SAFETY-20 import grader claimed more than it checked.
It collected `m.module for m in walk(tree) if isinstance(m, ImportFrom)` and
matched a literal list of six spellings. That is blind to two of Python's
three import forms:
* `from . import arbiter as _ab` is an ImportFrom whose .module is None -
the dots are in .level and the module name is in .names - so it collected
"" and matched nothing;
* `import app.arbiter` is an ast.Import node the walk filtered out entirely.
Both were added to app/tariff.py and the shipped suite stayed GREEN, with the
_names half not firing either (an alias import creates no Name node). Low
severity: any USE of either alias still produces an Attribute the _names half
catches. But the printed sentence "it imports nothing from arbiter, clamp or
control" was false for two forms out of three, and a check that overclaims is
worse than one that claims less, because the next reader stops looking.
_imported_modules() now walks Import as well as ImportFrom and joins
`from . import X` onto its base. _forbidden_imports() matches per dotted
SEGMENT, not by substring: `app.arbiter` must not hide behind a package
prefix, and `clamps` / `arbiters` / `controller` must not be falsely flagged -
`controller` is not hypothetical, this package is goodwe_controller. Five
evasion spellings and eight innocent ones grade the grader itself, and both
the name half and the import half are shown non-vacuous against main.py,
which carries all three import forms and really does import all three modules.
ITEMS 3 AND 4 - ALREADY CLOSED. STRUCK, NOT RE-IMPLEMENTED.
Item 3: app/tariff.py:266-267 is `def is_observed(self): return self.state ==
OBSERVED`, already false for NO_RECORD. Nothing to do.
Item 4: app/tariff.py:672-680 reads `held = max(averager.held_net_w, 0.0)`
with NO getattr default, and carries a comment naming TEST-13 item 4. Fixed
by SAFETY-07 in 616b62d; held_net_w is a documented public property on
QuarterAverager (p1.py:290), so a rename raises AttributeError on the first
cycle instead of crediting 0 W held. A projection crediting 0 W held reads
LOW, which is the under-guarding direction for SAFETY-07's floor. Restoring
a default would re-open a money guard.
And the recorded NON-finding stays a non-finding: removing clear_partial()
is still green and that is a PASS. quarter_start is the PRIMARY KEY and the
upsert carries `ON CONFLICT ... WHERE excluded.offtake_avg_w >
quarter_blocks.offtake_avg_w`, so a replay is a structural no-op. That
demonstrates TARIFF-01's "double counting is answered structurally, not by
care" claim rather than contradicting it.
EVIDENCE - all through tools/mutation_run.sh (recursive __pycache__ purge,
python3 -B, tree re-read from disk, mutated line grepped off disk, byte delta
recorded), in the goodwe-addon-ci image against a :ro mount.
baseline, 17 suites, before any edit ............................ 17 PASS
baseline, 17 suites, final tree ................................. 17 PASS
A1 billed_w(block, policy) -> billed_w(block, POLICY_NET) +4 B 5 RED
the 'sum' and 'max' round-trips, both tiers, and the distinctness pin
A2 billed_w(block, policy) -> billed_w(block, POLICY_MAX) +4 B 6 RED
the 'net' and 'sum' round-trips, both tiers, the default, and the pin
B1 add `from . import arbiter as _ab` to app/tariff.py +29 B 1 RED
the import sentence. SHIPPED suite on the same tree: GREEN - the hole.
B2 add `import app.arbiter` to app/tariff.py +19 B 1 RED
the import sentence. SHIPPED suite on the same tree: GREEN - the hole.
B3 add `from .control import compute` (the form the OLD grader DID
catch, as a regression control) 1 RED
C1 delete the ast.Import branch from _imported_modules() 1 RED
names the two spellings it MISSED, so the grader-grader is live
C2 segment match -> substring match (`any(f in m for f in ...)`) 1 RED
names clamps / arbiters / controller as falsely flagged
CONTROL, and the point of the whole ticket: A1 graded by the SHIPPED
test_tariff.py is green, and A1 graded by all 17 SHIPPED suites on a clean
uncontended run is 17 PASS, rc=0. The survivor is real and it is now closed.
ONE THING THE REVIEWER SHOULD KNOW. During the first (contended - two docker
containers at once) run of that 17-suite survivor sweep, test_peakguard.py
reported FAIL once. It has passed 8 times since on the identical tree,
including three concurrent runs and one clean uncontended full sweep, and its
tail was lost to a grep so I cannot say which check it was. I believe it is a
timing-sensitive check under CPU contention and NOT related to this change -
this commit touches no product code and no peakguard fixture - but I could not
reproduce it, so I am recording it rather than asserting it away.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
GoodWe RS485 Controller — Home Assistant add-on repository
Replaces a GoodWe battery inverter's vendor controller with a local one: holds net grid exchange at zero by emulating the inverter's smart meter over RS485, and runs the monthly battery maintenance cycle the vendor box was doing.
repository.yaml add-on repository metadata
goodwe_controller/ the add-on
config.yaml manifest + options schema (the per-site config form)
app/ the controller
control.py the control law — pure functions, no I/O
maintenance.py the monthly cycle state machine
hass.py Supervisor/Core API access
store.py persistent state in /data
mqtt.py optional status entities
web.py ingress UI
main.py orchestration, heartbeat, failsafe behaviour
test_control.py runnable checks — no framework needed
test_maintenance.py runnable checks — walks a full cycle in fake time
DOCS.md the add-on's documentation tab
MUTATION.md how to run a mutation so the result means something
tools/mutation_run.sh the mutation harness - purges bytecode, then grades
firmware/goodwe-master.yaml ESPHome config for the T-CAN485
estop/rs485_log.py optional RS485 e-stop / bus witness
docs/GoodWe-RS485-Field-Guide.pdf the guide techs carry (10pp, A4)
docs/build_guide.py its single source - rebuilds the HTML and PDF
FIELD-GUIDE.md how to edit and rebuild the guide
Picking this up after a break? Read HANDOVER.md first — current state,
what is known broken, and what to do next.
Install
Settings → Add-ons → Add-on store → ⋮ → Repositories → add
https://gittea.kammenstraatha.duckdns.org/admin/goodwe-addon
then install GoodWe RS485 Controller.
⚠️ The repository must be PUBLIC for this to work. Home Assistant clones it
anonymously; against a private repo the store shows nothing and gives no useful
error. If it must stay private, install locally instead: copy
goodwe_controller/ into the machine's /addons/ folder, then
ha store reload — it appears as a Local add-on.
⚠️ Bump version: in goodwe_controller/config.yaml for every change.
Supervisor keys the built image by that version, so without a bump it silently
reuses the old image and your fix appears not to work — on your bench and on
every client's machine.
Requires Home Assistant OS or Supervised. Add-ons cannot be installed on HA Container or Core.
Read this first
The inverter holds its last command forever. It has no meter-timeout of its own — a controller that dies mid-command leaves the battery running until a human intervenes. Measured on real hardware: 5 kW of discharge held for 113 seconds after a controller went silent.
Three layers exist because of that, and only the third covers the Home Assistant machine itself dying:
- ESP32 watchdog — no fresh setpoint for ~30 s → command 0 W, and keep commanding it.
- Wind-down before firmware updates — 0 W written before the update starts.
- RS485 e-stop (optional) — writes 0 W after 30 s of total bus silence.
Sites sold without the e-stop must have the acknowledgement in FIELD-GUIDE.md
§10 signed.
Development
The control law and the maintenance machine are pure Python with no Home Assistant imports, so they run anywhere:
cd goodwe_controller
python3 test_control.py
python3 test_maintenance.py
Both must pass before shipping any change. To prove a check bites - the way
almost every claim in this repository was established - mutate the module and
run tools/mutation_run.sh, which clears __pycache__ first. It is not
optional: a same-size edit reverted inside one second leaves CPython reusing the
old bytecode, and the run grades code that is not there. goodwe_controller/MUTATION.md
has the mechanism, the measurement, and what the flags do not do. They are not unit-test theatre —
each assertion corresponds to a rule whose absence produced an observed failure
on real hardware, and the comments in control.py say which.
Compatibility
Check the serial number, not the model name. Characters 6-8 of the serial are the platform tag:
| tag | platform | verdict |
|---|---|---|
ESU EMU ESA BPS BPU EMJ IJL |
105 — ES/EM/BP, AA55-era meter bus | supported |
SPB SPN |
745 — SBP G2 (GW…-SBP-20) |
no |
ESN ESC |
745 — ES G2 (GW…-ES-20) |
no |
ETU EHU BTU … |
205/745/753 — ET/EH/BT hybrids | no |
SDT DST MSU NSU … |
PV-only inverters — no battery to control | no |
A "-20" or "G2" suffix means a different platform with a different meter
protocol. The tags come from the goodwe library's PLATFORM_105_MODELS, which
is also what Home Assistant's own integration uses to pick a protocol.
- GoodWe ES / BP family inverters (AA55 / RS485 meter-bus generation)
- The protocol is reverse-engineered. There is no vendor contract, and a firmware change on GoodWe's side could break every installation at once. Say so when you sell it.