build: make the bundled drivers a snapshot of device-drivers, not a second copy#673
Merged
Conversation
…econd copy drivers/*.lua exists because startup is deliberately local: a gateway boots and runs offline, and a remote refresh must never block it. But the files were kept by hand, which made this repository a second source of truth for driver code. It had already drifted. A Sungrow fix landed in device-drivers -- stop reading a battery block on inverters that have no battery, which is what took a customer's SG12RT offline -- while the bundled copy here kept the bug. Two of 37 files differed and nothing would have said so. So the tree becomes generated. drivers/BUNDLED_SOURCE.json pins the device-drivers commit and lists which drivers FTW carries for recovery; scripts/sync-bundled-drivers.sh writes them, and --check fails if they drift. CI runs the check, and the path filter now covers the pin and the script so moving either cannot skip it. Both drifted files are corrected by this commit. The snapshot comes from that repository's source, not from its signed channel: channel artifacts are deliberately write-inert, and a gateway that falls back to this recovery copy still has to be able to control a battery. That distinction is why bundling and the channel are not alternatives to each other. Which drivers are bundled stays FTW's decision. device-drivers publishes 80; recovery needs 37. A .lua here that the pin does not list is reported, since an unlisted file is one nothing regenerates -- which is how the second source grew. Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.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: 65a42d8c37
ℹ️ 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".
Comment on lines
+310
to
+311
| if model_family == nil and detect_tries < DETECT_ATTEMPTS then | ||
| model_family = detect_model_family() |
There was a problem hiding this comment.
When register 4999 returns the documented sentinel value, this branch leaves model_family unset, so hybrid_block_worth_reading() is false and the poll skips the running-state and fault registers. This breaks the existing hybrid fault contract: go test ./internal/drivers now fails in TestSungrowPollSurfacesAndClearsThermalDeviceFault because the configured 0x0100 fault is never surfaced. Either retain hybrid probing while classification is pending or update the fixture/contract as part of this generated driver update so the required core test job passes.
AGENTS.md reference: AGENTS.md:L71-L72
Useful? React with 👍 / 👎.
… works Syncing the bundled snapshot brought in the Sungrow model gating, and one test started failing: the fault channel no longer fired. The mock never set register 4999, so it read back 0 -- the documented "not present" sentinel. The driver deliberately holds off reading the 13xxx block while it cannot tell which family it is talking to, because an SG string inverter cannot afford a burst of failed reads when the host fails the whole poll on any failure. So the running state was never read and no fault was raised. The test simulates an SH hybrid, and a real one answers 4999, so it now does too. That exposed a path worth pinning rather than assuming. A hybrid whose 4999 is genuinely unreadable falls back to probing after three tries, and if that fallback did not work such an inverter would silently lose its battery and its fault channel for the whole session. TestSungrowProbesTheHybridBlockWhenThe DeviceTypeNeverAnswers polls until the fault appears and fails if it never does. Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com>
frahlg
added a commit
that referenced
this pull request
Jul 26, 2026
directManifestRuntimePolicy refused any device-drivers artifact that was not read-only, and the channel marked every one of them read-only. Meanwhile the bundled copy -- a pinned snapshot of the same repository since #673 -- carries no policy at all and drives batteries fine. One source file, two behaviours, decided by where the file came from. A driver the channel publishes with control_enabled now gets no runtime policy, which is exactly what the bundled copy of that same source already gets. A driver that declares itself read-only still gets a read-only policy bound to its signed identity and permissions. read_only and control_enabled are two spellings of one fact, so a manifest where they agree with each other -- both true or both false -- is refused: a driver that may control while claiming to be read-only reads as safe to anything that checks only one of them. Pairs with srcfl/device-drivers#31, which stops making artifacts write-inert. This side has to reach the field first, or a gateway on an older build refuses to load an updated driver from the channel. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
frahlg
added a commit
that referenced
this pull request
Jul 26, 2026
…rol (#676) directManifestRuntimePolicy refused any device-drivers artifact that was not read-only, and the channel marked every one of them read-only. Meanwhile the bundled copy -- a pinned snapshot of the same repository since #673 -- carries no policy at all and drives batteries fine. One source file, two behaviours, decided by where the file came from. A driver the channel publishes with control_enabled now gets no runtime policy, which is exactly what the bundled copy of that same source already gets. A driver that declares itself read-only still gets a read-only policy bound to its signed identity and permissions. read_only and control_enabled are two spellings of one fact, so a manifest where they agree with each other -- both true or both false -- is refused: a driver that may control while claiming to be read-only reads as safe to anything that checks only one of them. Pairs with srcfl/device-drivers#31, which stops making artifacts write-inert. This side has to reach the field first, or a gateway on an older build refuses to load an updated driver from the channel. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Answers the question directly: is device-drivers the source for FTW's drivers? Until this PR, not quite — and the gap had already cost something.
The drift was real
drivers/*.luaexists because startup is deliberately local: a gateway boots and runs offline, and a remote refresh must never block it (main.go:353). But the files were kept by hand, which made this repository a second source of truth for driver code.It had already diverged. The Sungrow fix — stop reading a battery block on inverters that have no battery, which is what took a customer's SG12RT offline — landed in device-drivers while the bundled copy here kept the bug. Two of 37 files differed and nothing would have said so.
Resolution order is
local override → activated channel artifact → bundled recovery. So a gateway that had not activated anything from the channel was still running the broken driver.Both files are corrected by this commit.
Bundling and the channel are not alternatives
Worth stating because it looked like a blocker earlier: the snapshot comes from device-drivers' source, not from its signed channel.
Channel artifacts are deliberately write-inert. A gateway that falls back to the recovery copy still has to be able to control a battery, so recovery cannot be built from artifacts that cannot write. That is why the two mechanisms coexist rather than one replacing the other.
How it works
drivers/BUNDLED_SOURCE.jsonscripts/sync-bundled-drivers.sh--checkfails on drift--checkin thedriversjobThe path filter now covers the pin file and the script as well as
drivers/*.lua, so moving either cannot skip the check —.jsonwould not have triggered it before.Which drivers are bundled stays FTW's decision. device-drivers publishes 80; recovery needs 37. A
.luahere that the pin does not list is reported, since an unlisted file is one nothing regenerates — which is exactly how the second source grew.To change a driver from now on
Fix it in device-drivers, move the pin, run the script, commit the result. Editing a
.luahere fails CI with that instruction. Documented inREADME.mdandAGENTS.md.make verifyclean. The script isbash -nchecked alongside the others, and avoidsmapfileso it runs on the bash that ships with macOS.🤖 Generated with Claude Code