glenn schrooyenandClaude
fd08090360
The add-on is wired to the inverter's RS485 port labelled EMS. Reword README, DOCS, config description, field guide source and HTML. The HA manufacturer string is left unchanged: test_mqtt pins it byte for byte. Co-Authored-By: Claude <noreply@anthropic.com>
201 lines
10 KiB
Markdown
201 lines
10 KiB
Markdown
# Handover — 2026-08-23
|
||
|
||
Start here. `README.md` is what the thing is; this is where it stands, what is
|
||
known to be broken, and what to do next.
|
||
|
||
---
|
||
|
||
## 1. State of the world
|
||
|
||
The add-on **is controlling a real battery right now** — the author's own house,
|
||
a GoodWe GW5000S-BP.
|
||
|
||
| | |
|
||
|---|---|
|
||
| running | `local_goodwe_controller` **0.4.0**, `boot: auto`, holding grid at zero |
|
||
| installed as | a **Local** add-on: `/addons/goodwe_controller` on HA OS at 192.168.2.158 |
|
||
| repo | Gitea `admin/goodwe-addon` (ssh://git@192.168.2.147:2222) — **private** |
|
||
| tuning | gain 0.6, `max_w` 5000, slew 1000, deadband 15, step 10, saturation 500/3, heartbeat 10 s |
|
||
| maintenance | **disabled**, and with **no completion history** — first cycle is "due" the moment it is enabled |
|
||
| e-stop | fitted at this site, Pi tap armed with `--panic` |
|
||
|
||
The thing it replaced — a working Home Assistant YAML implementation — is parked
|
||
at `/config/packages_disabled_addon_migration/` on that machine. **Rollback** is:
|
||
stop the add-on, move those four files back into `/config/packages/`, reload
|
||
automations and helpers, switch `input_boolean.goodwe_auto` on.
|
||
|
||
⚠️ **Never run both.** They write the same setpoint. Two controllers on one
|
||
actuator is the failure the whole design exists to prevent.
|
||
|
||
The research record — protocol, register map, why every tuning value is what it
|
||
is, and a long "ruled out, do not re-investigate" list — lives in the *other*
|
||
repo, `Documents/goodwe`, in `NOTES.md`. Read it before changing a constant.
|
||
|
||
---
|
||
|
||
## 2. Deploying a change
|
||
|
||
```bash
|
||
# 1. edit under goodwe_controller/
|
||
# 2. BUMP version: in goodwe_controller/config.yaml <-- not optional
|
||
scp -r goodwe_controller root@192.168.2.158:/addons/
|
||
ssh root@192.168.2.158 "ha store reload && ha addons update local_goodwe_controller"
|
||
ssh root@192.168.2.158 "ha addons logs local_goodwe_controller | tail -20"
|
||
```
|
||
|
||
⚠️ **Supervisor keys the built image by `version:`.** Without a bump it silently
|
||
reuses the old image and your change appears not to work. This cost two rounds of
|
||
"the fix doesn't work" in one evening. It applies to client updates too — a
|
||
client only gets a change when the version increments.
|
||
|
||
Run the checks before deploying. No framework needed:
|
||
|
||
```bash
|
||
cd goodwe_controller
|
||
python3 test_control.py && python3 test_maintenance.py && python3 test_arbiter.py \
|
||
&& python3 test_validate.py && python3 test_p1.py && python3 test_watchdog.py \
|
||
&& python3 test_loop.py
|
||
```
|
||
|
||
⚠️ **Mutating a module to prove a check bites? Read `goodwe_controller/
|
||
MUTATION.md` first, and run `goodwe_controller/tools/mutation_run.sh`.** A
|
||
same-size edit reverted inside one second changes neither the file size nor the
|
||
whole-second mtime, so CPython reuses the bytecode it already compiled and the
|
||
suite grades code that is not there - once already, reported as a survivor and
|
||
disproved by disassembly (TEST-15). `python3 -B` does **not** save you: it stops
|
||
bytecode being written, not a stale `.pyc` being read. The purge is the fix.
|
||
|
||
`test_watchdog.py` covers SAFETY-02's logic, its arming gate (derived from the
|
||
real arbiter, not passed in), its trip path, and the fact that the latch survives
|
||
an add-on restart. Its headline validation is not here at all: `sim/replay_sign.py`
|
||
in the infra repo drives the same module over the two captured RS485 days and must
|
||
fire on all four of the 2026-08-20 episodes and stay silent on 2026-08-23. That
|
||
needs both checkouts:
|
||
|
||
```bash
|
||
GOODWE_ADDON_REPO=/path/to/goodwe-addon python3 sim/replay_sign.py
|
||
GOODWE_ADDON_REPO=/path/to/goodwe-addon python3 sim/replay_sign.py --stub
|
||
```
|
||
|
||
The second run checks the grader still *discriminates* rather than merely still
|
||
passing — a watchdog that never trips and one that always trips must both be
|
||
rejected.
|
||
|
||
⚠️ **If you are testing the failsafe by hand, note that stopping and starting the
|
||
add-on will not clear a trip.** That is the point of it; use the *Start / stop
|
||
control* button twice, or delete the trip from `/data/state.json`.
|
||
|
||
⚠️ **DEV-02 renamed that key. There is one `SignWatchdog` per inverter, so the
|
||
LIVE latch lives under `unit/<inverter>/sign_tripped`** —
|
||
`unit/inverter-a/sign_tripped` on a single-inverter site. **That is the one to
|
||
delete if you are clearing a trip by hand**; deleting the flat one alone clears
|
||
nothing.
|
||
|
||
⚠️ **The flat `sign_tripped` is still maintained, though — it is the ROLLBACK
|
||
VIEW, not a fossil.** It is written alongside the namespaced key on a trip and
|
||
cleared alongside it when you press *Start / stop control*, so an add-on you
|
||
**downgrade** past this version still finds the latch it should. It is also read
|
||
on **every** start, and the one disagreement it can produce is acted on: *the
|
||
flat key says tripped and no inverter came up latched* means the latch was
|
||
written by an older add-on while this install was rolled back — or the migration
|
||
lost it — and the controller **re-arms the latch and shouts in the log** rather
|
||
than coming up disarmed with a green banner. So if you edit `/data/state.json`
|
||
by hand, clear **both** spellings or the next start will put the trip back.
|
||
|
||
⚠️ `test_loop.py` takes about **95 seconds** (measured 93 s) and that is not a bug. It
|
||
drives the real control loop, which sleeps 1 s a tick, eight times over, to prove a
|
||
wall-clock property about it (SAFETY-12); mocking the clock there would leave the
|
||
assertion testing the mock. The other six suites together are ~440 checks and finish in
|
||
seconds — `test_watchdog.py` spends ~3 s of that driving a real `Controller` through a
|
||
trip. If you shorten `test_loop.py`'s windows, re-measure and correct this figure —
|
||
an understated runtime is what makes a slow test look like a hung one, and a slow test
|
||
that looks hung gets deleted.
|
||
|
||
---
|
||
|
||
## 3. Known broken / unfinished
|
||
|
||
1. **MQTT status entities are not adopted by Home Assistant.** The add-on
|
||
publishes correct discovery and live state — verified on the broker itself
|
||
(`goodwe_ctl/setpoint => 940.0`). After the entities were deleted from the
|
||
registry during development, HA would not re-adopt them, even after clearing
|
||
the retained topics and reconnecting. Affects **observability only**; control
|
||
is untouched. Try again after an HA restart, which is the cheapest next test.
|
||
2. **The repository is private**, so it cannot be used as an add-on repository
|
||
URL — HA clones anonymously. Either make it public, or keep installing
|
||
locally as at this site. Decide before the first client.
|
||
3. **No real maintenance cycle has ever run.** The mechanism is proven
|
||
end-to-end in simulated time and partially on hardware, but a genuine
|
||
low→full→hold cycle takes hours and has not happened. Do not decommission a
|
||
client's vendor controller before one has.
|
||
4. **The HA-side offline alarm is unproven.** The Pi's 30 s panic write is the
|
||
real cover; treat the HA alert as "within a few minutes".
|
||
5. **Only platform 105 is supported** — see §5.
|
||
|
||
---
|
||
|
||
## 4. What to do next, in order
|
||
|
||
1. **Peak shaving as a real strategy.** This is why the arbiter exists, and it is
|
||
now purely additive: a claim producer emitting `Claim.set("peak", P_PEAK, …)`
|
||
from the quarter-hour forecast, plus a priority. Nothing existing needs to
|
||
change — the maintenance charge-only limit already cannot bind it. Highest
|
||
value per unit of work in a capacity-tariff market.
|
||
2. **Run a full maintenance cycle** at the reference site and record what it
|
||
actually costs in energy and time.
|
||
3. **Fleet health before more features.** This system's signature failure is
|
||
*silent*: an entity-id prefix killed two safety alarms for a day, a shebang
|
||
made every call 401 while looking healthy. One site with the author watching
|
||
found those. Twenty sites will not.
|
||
4. **Decide the repository question** (§3.2).
|
||
|
||
---
|
||
|
||
## 5. The commercial question nobody has answered
|
||
|
||
**A GoodWe "-20" / G2 inverter cannot be driven by this.**
|
||
|
||
Platform comes from characters 6–8 of the serial number. Only
|
||
`ESU EMU ESA BPS BPU EMJ IJL` — platform 105, the AA55-era generation — are
|
||
supported. SBP G2 (`SPB`/`SPN`) and ES G2 (`ESN`/`ESC`) are platform 745, a
|
||
different protocol. The reference unit is `BPS`.
|
||
|
||
That matters more than it first sounds: **the SBP G2 is the model GoodWe
|
||
currently sells to replace a GW5000S-BP.** So today's product serves the
|
||
installed base, not the base being installed. Everything above the firmware —
|
||
control law, arbiter, maintenance, add-on, field guide — is protocol-agnostic
|
||
and would carry over; only the register map is platform-specific.
|
||
|
||
Answering it is an afternoon: put the Pi tap passively on a G2 unit's meter bus
|
||
with its vendor controller running, and capture. That either hands over the new
|
||
register map or proves the approach does not transfer. **Do this before building
|
||
more features on top of a shrinking base.**
|
||
|
||
---
|
||
|
||
## 6. Things that will bite you
|
||
|
||
Each of these was found the expensive way. They are in the code comments too.
|
||
|
||
- **`run.sh` must start `#!/usr/bin/with-contenv sh`.** s6-overlay sanitises the
|
||
environment, so a plain shebang means no `SUPERVISOR_TOKEN` and every HA call
|
||
returns 401 — while `homeassistant_api: true` makes permissions look granted.
|
||
- **A wrong entity id is not an error anywhere in HA.** It just never produces a
|
||
value, and the automation's own state still reads `on`.
|
||
- **HA composes entity ids from the device area and device name.** The same
|
||
firmware yields `sensor.goodwe_master_…` at one site and
|
||
`sensor.cellar_goodwe_master_…` at another.
|
||
- **`input_datetime` states parse timezone-naive**; subtracting `now()` raises
|
||
and kills the whole automation before it records itself as triggered. Use the
|
||
`timestamp` attribute.
|
||
- **Alpine ships paho-mqtt 1.x**, which has no `CallbackAPIVersion`, and musl has
|
||
no aiohttp wheel — install from apk, not pip.
|
||
- **A mutation run can grade code that is not there.** CPython validates a
|
||
`.pyc` on (source size, source mtime in whole seconds), so a same-size edit
|
||
reverted within one second is invisible to it and the old bytecode runs. It
|
||
manufactures *survivors*, never reds. Purge `__pycache__` -
|
||
`tools/mutation_run.sh` does - and see `goodwe_controller/MUTATION.md`.
|
||
- **The inverter holds its last command forever.** It has no meter-timeout. Every
|
||
failsafe here exists because of that one fact, and anything in doubt commands
|
||
**0 W** rather than holding.
|