chore(drivers): move the bundled pin to device-drivers main - #704
Conversation
![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: e9b721fe83
ℹ️ 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".
| if sf_present[addr] == nil then | ||
| sf_present[addr] = false |
There was a problem hiding this comment.
When the gateway polls while the Pixii is offline—or any scale-factor request has a transient transport failure—this marks the register absent for the lifetime of the Lua driver. After connectivity recovers, read_sf skips that register and permanently substitutes exponent 0, so battery and site-meter values can be wrong by powers of ten while Core accepts them as fresh telemetry. Distinguish the known illegal-address response from transient errors, or retry before caching absence, in device-drivers before resyncing this snapshot.
AGENTS.md reference: AGENTS.md:L12-L15
Useful? React with 👍 / 👎.
| elseif mppt_present == nil then | ||
| mppt_present = false |
There was a problem hiding this comment.
If Model 103 succeeds but the first MPPT request times out or otherwise fails transiently, this permanently classifies the block as absent. Every later poll then emits zero MPPT voltage/current without retrying, even after communication recovers, until the driver is restarted. Only cache a definitive illegal-address response—or require repeated failures—in device-drivers before resyncing the snapshot.
AGENTS.md reference: AGENTS.md:L12-L15
Useful? React with 👍 / 👎.
|
Note for whoever reviews:
Worth a look on its own: a suite that hangs for ten minutes and then passes is going to fail somebody's unrelated PR again. |
The gate diffs changed .lua files between two Git revisions of this repository. That works only while the drivers are committed here, and it was never quite the right question: what reaches a gateway is decided by where drivers/BUNDLED_SOURCE.json points, not by what this history shows. check-versions gains -old-dir/-new-dir, comparing two materialised snapshots instead of two revisions. ValidateDriverVersionChange is untouched -- it always took bytes rather than revisions, so the rule being enforced is identical. scripts/check-driver-versions.sh reads the pin at a base revision and the pin now, and stays quiet when they match: a pull request that does not move the pin cannot have moved any driver's bytes. When it has moved, both snapshots are fetched and every bundled driver compared. The Git-based mode stays for now. It still works while the files are committed, and removing it belongs with the deletion. Tested where it counts -- the failure case. Six table-driven cases in main_test.go: changed bytes under an unchanged version fail and name the driver, a bump passes, untouched files pass, a newly bundled driver has nothing to compare, a dropped one is a coverage decision rather than a version one, and every offender is reported rather than just the first. Also run against the real pin move in #704 (1f67078 -> 3f62a00), where pixii, solaredge_legacy and sungrow all changed with bumps: verified across 37 bundled drivers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com>
The gate diffs changed .lua files between two Git revisions of this repository. That works only while the drivers are committed here, and it was never quite the right question: what reaches a gateway is decided by where drivers/BUNDLED_SOURCE.json points, not by what this history shows. check-versions gains -old-dir/-new-dir, comparing two materialised snapshots instead of two revisions. ValidateDriverVersionChange is untouched -- it always took bytes rather than revisions, so the rule being enforced is identical. scripts/check-driver-versions.sh reads the pin at a base revision and the pin now, and stays quiet when they match: a pull request that does not move the pin cannot have moved any driver's bytes. When it has moved, both snapshots are fetched and every bundled driver compared. The Git-based mode stays for now. It still works while the files are committed, and removing it belongs with the deletion. Tested where it counts -- the failure case. Six table-driven cases in main_test.go: changed bytes under an unchanged version fail and name the driver, a bump passes, untouched files pass, a newly bundled driver has nothing to compare, a dropped one is a coverage decision rather than a version one, and every offender is reported rather than just the first. Also run against the real pin move in #704 (1f67078 -> 3f62a00), where pixii, solaredge_legacy and sungrow all changed with bumps: verified across 37 bundled drivers. Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
e9b721f to
29e821f
Compare
frahlg
changed the title
frahlg
changed the title
Brings the whole absent-register campaign into the recovery snapshot: every bundled driver now gives up on a register the device does not answer instead of paying a failed read for it on every poll. 497 addresses across 49 drivers, plus the control-path fixes that landed alongside. Pin moves 1f67078 -> edb32ff. Generated by scripts/sync-bundled-drivers.sh; nothing edited by hand. The SemVer gate added in #709 verified the move across all 37 bundled drivers. go test ./... passes, including the Sungrow zero-power case that held this back, and the full-stack e2e is green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com>
c630e1e to
d951951
Compare
|
Heads up on the pin: it still points at
The Go test change on this branch is compatible with all three — verified by dropping each driver version into Suggest holding the pin move until #43 lands, then re-running |
|
Correction to the comment above: I said the test passes under #44's driver. It does not — I had not run that combination when I wrote it. Measured, dropping each version into
#44 kept #42's exemption, so 1.5.5 fails for the same reason 1.5.4 does. The rest stands: hold the pin move until #43 lands, then re-run |
1 participant
Brings the whole absent-register campaign into the recovery snapshot. Every bundled driver now gives up on a register the device does not answer instead of paying a failed read for it on every poll — 497 addresses across 49 drivers — plus the control-path fixes that landed alongside.
Pin moves
1f67078→edb32ff. Generated byscripts/sync-bundled-drivers.sh; nothing edited by hand.Verified
check-driver-versions.sh(the gate from #709)sync-bundled-drivers.sh --checkgo test ./...What unblocked it
This PR was held because
TestSungrowZeroBatteryCommandForcesIdlefailed: the driver refused a battery command issued before the first poll, and the test expects a zero-power command to write idle.Settled in device-drivers#42 — zero always goes through. It is not a dispatch but the host handing the device back to itself, it arrives from the lifecycle rather than the planner so it lands before anything is confirmed, and refusing to write stop is a different risk from refusing to write charge. Non-zero setpoints still require evidence, so the hole closed in device-drivers#40 stays shut.
That work also found the Lua mock raising where this host returns
nilplus a message — which is what made a fixture bug look like a driver bug.🤖 Generated with Claude Code