Give every driver one id and one version - #132
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: b4f0e66d0e
ℹ️ 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".
|
|
||
| DRIVER = { host_api_min = 1, host_api_max = 1, | ||
| id = "esphome-dsmr", | ||
| id = "esphome_dsmr", |
There was a problem hiding this comment.
When the ESPHome package is built, packages/v1/esphome-dsmr/package-source.json still reads its source and artifact input from drivers/lua/esphome-dsmr.lua; this commit deletes that file while establishing drivers/lua/esphome_dsmr.lua as the current driver. build_package therefore fails while reading the input, and the recipe also continues to advertise version 1.0.2 rather than 1.0.4. Update or remove the recipe atomically with the rename.
AGENTS.md reference: AGENTS.md:L86-L88
Useful? React with 👍 / 👎.
| name = "Eastron SDM630 meter", | ||
| manufacturer = "Eastron", | ||
| version = "1.1.1", | ||
| version = "2.0.2", |
There was a problem hiding this comment.
When either sdm630 or foxess_h3_smart is built for its still-declared blixt-l1 target, _validate_lua_source_for_target requires a DRIVER_MANIFEST table. This commit removes that table from both package inputs without changing their recipes, so make package-driver ID=sdm630 TARGET=blixt-l1 and the FoxESS equivalent raise Lua source must declare DRIVER_MANIFEST. Keep the tables or remove the Blixt targets in the same change.
AGENTS.md reference: AGENTS.md:L86-L86
Useful? React with 👍 / 👎.
| name = "FoxESS H3-Smart / 1K5", | ||
| manufacturer = "Fox ESS", | ||
| version = "0.9.5", | ||
| version = "0.9.6", |
There was a problem hiding this comment.
The FoxESS package recipe remains at version 0.9.5 while this source and its catalog manifest move to 0.9.6. Because the recipe packages this exact Lua file, a generated signed package advertises 0.9.5 in its package metadata while the runtime DRIVER table identifies itself as 0.9.6, recreating the identity split this change is intended to eliminate. Update the recipe version with the source bump.
AGENTS.md reference: AGENTS.md:L87-L88
Useful? React with 👍 / 👎.
This is FTW's driver repository. The package pilot gave five drivers a third version line beside the catalog DRIVER version and the manifest version, under package ids that cannot contain "_" where catalog ids do. Nothing published or consumed it: the release workflow never called the packager, CI built unsigned candidates only, and no release carries a package or index asset. Removed: - packages/v1 (recipes for esphome-dsmr, foxess_h3_smart, pixii, sdm630 and sungrow, the separate pixii and sungrow target files, and the sungrow PILOT.md); - tools/driver_package.py, tools/build_candidate.py, tools/derive_sungrow_ftw_observe.py, make package-driver and the packages CI job; - the package-v1 and migration specs, the driver inventory and Blixt reference specs, and every sourceful.driver-* JSON schema, with the tests that validated them; - package fields and the pilot's own override entries from the support status, the always-empty "Stable package" column from the catalog page, and the now unused jsonschema dependency; - package and Device Support text from AGENTS.md, README.md, CONTRIBUTING.md, SECURITY.md, SOURCE_IMPORT.md and the specs. spec/control-runtime-v2.md keeps only FTW Core's v2 command interface and spec/signing.md only the FTW channel. Tests that held a rule against both a catalog driver and its package target now hold it against the catalog driver. No driver Lua, manifest or version changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MuerPFZFG88kgu8sWVHeq7 Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com>
|
Automated maintenance pass (device-driver repo review): not merging, not actionable yet. CI is red on Left a status comment on #133 as well — recommending Fredrik merge that one directly given its blast radius (spec/schema removals with stated downstream couplings) and that Once #133 lands and this rebases cleanly: content otherwise looks well-scoped (one id/version per driver, 18 patch bumps, no behavior change beyond identity/version alignment, one CHANGELOG entry per driver version) and worth a re-check then. Also note the stated conflict with #60/#66 (heishamon version line) — those are still correctly held open pending hardware confirmation of Generated by Claude Code |
The channel build accepted a DRIVER table whose id was spelled differently from the catalog (sungrow-shx for sungrow, easee-cloud for easee_cloud) and whose version counted FTW's releases instead of this repository's. FTW compares its bundled copy with the signed channel by id and version, so the two spellings kept it from matching them: "Use bundled" failed on a real box and a first install did not reach the running driver. - The channel build now requires the DRIVER id and version to equal the catalog's; tests/test_driver_truth.py checks the id beside the version. - The exemption for drivers byte-identical to baselines/ftw is gone. None is identical any more, so it no longer did anything. - Checks that were stricter than FTW's host now match it: FTW keeps os.time and os.date for MQTT timestamps (go/internal/drivers/lua.go), a guarded "if messages then" loop handles an empty poll, text inside a string is not a module call, and the V2X field list includes what FTW's API reads (freq_hz, vehicle_soc, connected, status, control_mode, protocol). Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com>
b4f0e66 to
7eb77e6
Compare
- 18 drivers: the DRIVER id is now the catalog id (ctek, easee_cloud, sungrow, zap, ...). Patch bump each. - heishamon, sdm630: the DRIVER version now matches the catalog. - ambibox_v2x, ferroamp_dc2_v2x: id and version match; ferroamp_dc2_v2x decodes JSON through pcall. Control logic unchanged; HIL still pending before stable. - esphome_dsmr: the source is the ESPHome DSMR driver itself, not a generated channel artifact committed as source that the channel wrapped twice (the published file reported id esphome-dsmr 1.0.2). The separate catalog entry esphome-dsmr stays: sites installed it from the channel, and folding it in needs a migration of its own. - 8 drivers: drop the Blixt L1 DRIVER_MANIFEST table. FTW never read it and this repository builds nothing for other products. One CHANGELOG entry per driver version, "## <id> <version>". driver-history.json is left to the release, which records a version once it is published. Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com>
7eb77e6 to
740bff8
Compare
…#1429) * fix(drivers): one id per driver, from the bundled copy to the channel Moves the pin to srcfl/device-drivers#132, where every driver's DRIVER table states the catalog id and version. FTW's identity rule for the old spellings (IdentifiesSameDriver) goes: a bundled copy and a channel artifact are the same driver when their ids are equal. - Catalog tests look drivers up by the catalog ids. - The recovery-snapshot provenance test for esphome_dsmr is gone: the bundled file is now the driver's source, and sync-bundled-drivers.sh --check already binds every bundled file to the pinned commit. - setup.js recognises the Zap driver by its id `zap`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MuerPFZFG88kgu8sWVHeq7 * docs(release): promote the driver channel as the last stable step drivers-stable only serves installs that read the channel (1.x-3.x) and the stable rows of the Versions list; 0.x runs the drivers its release bundles. Promoting it is one existing command after a stable Core release, not new automation. The proposal to promote it with Core goes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MuerPFZFG88kgu8sWVHeq7 * fix(drivers): pin the merged one-id device-drivers commit Moves the pin to bcf4683, srcfl/device-drivers#132 as merged on main, instead of the branch commit it was tested against before its rebase. The changeset no longer says esphome-dsmr is gone: #132 keeps that catalog entry for sites that installed it. The stable promotion step says what to do when main has moved past drivers-beta. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MuerPFZFG88kgu8sWVHeq7 * docs(agents): a driver selection survives updates and rollbacks Matches #1428: a newer release driver runs over an early selection without discarding it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MuerPFZFG88kgu8sWVHeq7 * fix(drivers): pin device-drivers 43710c8 (esphome_dsmr 1.0.5) srcfl/device-drivers#134 puts esphome_dsmr's host API fields on their own lines; FTW reads DRIVER fields line by line and its publication check refused the 1.0.4 file. The bundled set now publishes cleanly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MuerPFZFG88kgu8sWVHeq7 --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 participant
Why
Owner decision (2026-09-25): a driver has one id and one version everywhere: the source
DRIVERtable, the catalog, the signed channel and FTW's bundled copy. There are no special identity rules. FTW compares its bundled copy with the signed channel by id and version. On the owner's box, the two spellings (easee-cloud/easee_cloud) made "Use bundled" fail, kept a first install from reaching the running driver, and left the Versions list empty for 21 bundled drivers.An audit of 489c937 found the signed channel already consistent: id = manifest name and version = manifest version for all 88 entries, with beta equal to stable. The splits were only in source
DRIVERtables and in the package pilot.Changes
Rule (first commit)
DRIVERid == catalog id andDRIVERversion == catalog version._identifies_same_driveris gone.tests/test_driver_truth.pychecks the id beside the version.holds_for_ftw_drivers,bump_driver.is_ftw_baseline, the version-test skip) is removed. No driver is byte-identical tobaselines/ftwany more, so it did nothing.os.time/os.datefor MQTT timestamps (go/internal/drivers/lua.go).if messages thenhandles an empty poll."sid-os.local").freq_hz,vehicle_soc,connected,status,control_mode,protocol(FTWgo/internal/api/api.go).Drivers (second commit)
pcall. Control logic is unchanged; HIL is still pending before stable.esphome-dsmr1.0.2); the manifest sayscontrol: false. The separate entryesphome-dsmrstays: its channel artifacts have real downloads (38 beta, 11 stable), and folding it in needs a migration of its own.DRIVER_MANIFESTtable: sdm630, foxess_h3_smart, acuvim, deye-svk, konja-261-svk and the three 50-125k-svk. FTW never read it, and per the owner this repository builds nothing for other products.## <id> <version>, so FTW can later show it beside each version.Review
Codex review found premature
driver-history.jsonentries (the release records a version once it is published) and the stranding of existingesphome-dsmrinstalls; both are fixed in this revision.make check: 4644 passed.Order and conflicts
make checkfails only intests/test_driver_package.py: 7 tests of the pilot that PR deletes. On this branch 4629 tests pass. I will rebase once it lands.FTW side (separate PR in srcfl/ftw)
IdentifiesSameDriver, since plain equality suffices.sungrowinstead ofsungrow-shx), as the owner approved.