test: measure the refused-write flap across the catalog - #50
Conversation
sungrow 1.5.7 fixed a driver that reissued a refused write on every watchdog tick, forever, after a customer's SG12RT logged "self-consumption reset write failed: modbus exception function=0x06" once per tick indefinitely. This measures whether that shape is one driver or many. It is eleven. driver_default_mode runs on a timer -- lease expiry, the telemetry watchdog, the stale-meter standdown, shutdown -- and nothing reaching it can say no, so a driver that writes whatever the device answers keeps writing for the life of the session: atmoce deye ferroamp_modbus huawei pixii sigenergy solaredge solaredge_legacy solaredge_pv solinteg solis All eleven write 1 or 2 registers per call and never settle. sungrow is the only one that stops, and only because the field report forced it. refused_write_probe.lua refuses every write, calls driver_default_mode eight times and reports the per-call counts. test_refused_write_settles.py ratchets against refused-write-baseline.json the way #36 did for reads: a count that rises fails, a driver not listed must be clean, and a count that falls fails until the file is updated. Verified by putting the sungrow flap back -- it fails. This is the write-side twin of test_absent_register_settles.py, and the two failures are not equally severe. A failed read fails the whole poll and takes the site offline; a refused write costs bus traffic and a log line per tick. So this measures the flap rather than treating every occurrence as an outage -- but that log line is the one an operator reads to find the real fault, and on the SG12RT it drowned it. Also fixes host_mock.lua, which had no host.sleep. It is in spec/host-api.md and the real host provides it; solis and deye call it to space out writes. Lua turns a missing field into "attempt to call a nil value", so any test reaching their write-retry path died there rather than measuring it, and both looked like they crashed inside driver_default_mode when they do nothing of the kind. The mock now advances its clock instead of sleeping. Second time the mock has been caught lying, after #42. No driver changes. Paying the debt down is follow-up work, in batches, the way #36 did it. make check -- 3195 passed, 1361 skipped. Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e40e237f5d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| local writes, returned = {}, {} | ||
| for _ = 1, CALLS do | ||
| local before = #host._calls | ||
| local ok, result = pcall(driver_default_mode) |
There was a problem hiding this comment.
For drivers that release control only after an accepted command, invoking driver_default_mode directly tests only their no-op state. sma_pv is already a false negative: after a successful driver_command("curtail", ...), lines 472–478 leave curtail_active set whenever the release write is refused, so repeated default calls produce 1,1,1,1,1,1,1,1; this probe instead reports it clean because it never enters that state. Prime each supported control path before refusing writes so the safe-default behavior is actually exercised.
AGENTS.md reference: AGENTS.md:L15-L18
Useful? React with 👍 / 👎.
| f"({skip}). Remove it from {BASELINE.name}.") | ||
| return | ||
|
|
||
| found = 1 if writes else 0 |
There was a problem hiding this comment.
For a driver already listed in the baseline, found is always 1 regardless of how many writes each call attempts, so the promised “count goes up” ratchet cannot detect worsening debt. For example, atmoce currently attempts two writes per call but has baseline value 1; changing it to three or one hundred refused writes per tick would still pass. Compare a real write-count metric against the baseline rather than collapsing every non-settling result to a boolean.
Useful? React with 👍 / 👎.
| ROOT = Path(__file__).resolve().parents[2] | ||
| LUA = ROOT / "lua55" | ||
| PROBE = ROOT / "drivers" / "tests" / "lua_harness" / "refused_write_probe.lua" | ||
| DRIVERS = ROOT / "drivers" / "lua" |
There was a problem hiding this comment.
Restricting discovery to drivers/lua leaves the separate package implementations entirely outside this ratchet, including their driver_default_mode_v2 entrypoints. This is already observable in packages/v1/pixii/targets/ftw.lua:364-366 and packages/v1/sungrow/targets/ftw.lua:449-452, which unconditionally retry a refused default write on every invocation but never appear in this baseline; scan package targets and invoke the v2 entrypoint as well.
AGENTS.md reference: AGENTS.md:L22-L30
Useful? React with 👍 / 👎.
1 participant
#49 fixed a driver that reissued a refused write on every watchdog tick, forever, after a customer's SG12RT logged
self-consumption reset write failed: modbus exception function=0x06once per tick indefinitely.This measures whether that shape is one driver or many. It is eleven.
The rule
driver_default_modeis the one control path that runs on a timer — lease expiry, the telemetry watchdog, the stale-meter standdown, shutdown. Nothing reaching it can say no, so a driver that writes whatever the device answers keeps writing for the life of the session.2,2,2,2,2,2,2,22,2,2,2,2,2,2,21,1,1,1,1,1,1,13,3,3,0,0,0,0,0sungrow is the only one that settles, and only because the field report forced it.
What lands
drivers/tests/lua_harness/refused_write_probe.lua— refuses every write, callsdriver_default_modeeight times, reports the per-call countsdrivers/tests/test_refused_write_settles.py+refused-write-baseline.json— ratchets exactly as fix: pay off the absent-register debt in full #36 did for reads: a count that rises fails, a driver not listed must be clean, a count that falls fails until the baseline is updated. Verified by putting the sungrow flap back — it failsmake refused-write-report ID=<id>docs/WRITING-A-DRIVER.md, beside the read oneNo driver changes. Paying the debt down is follow-up work, in batches, the way #36 did it.
Severity, stated honestly
This is the write-side twin of
test_absent_register_settles.py, and the two failures are not equally severe. A failed read fails the whole poll and takes the site offline. A refused write costs bus traffic and a log line per tick. That is why this measures the flap rather than treating every occurrence as an outage — but that log line is the one an operator reads to find the real fault, and on the SG12RT it drowned it.Whether the other ten have models that refuse these registers is not established here. sungrow's SG/SH split is the case we have field evidence for; the rest is the same shape awaiting the same evidence.
A mock that was lying
host_mock.luahad nohost.sleep. It is inspec/host-api.mdand the real host provides it; solis and deye call it to space out writes. Lua turns a missing field intoattempt to call a nil value, so any test reaching their write-retry path died there rather than measuring it — and both looked like they crashed insidedriver_default_modewhen they do nothing of the kind. The mock now advances its clock instead of sleeping.Second time the mock has been caught lying, after #42.
Checks
make check— 3195 passed, 1361 skipped.🤖 Generated with Claude Code