feat(zap): opt-in read of PV and battery from Zap - #974
Conversation
Keep Zap as the P1/HAN meter by default. Settings → Devices can turn on read-only PV or battery ingest when Zap is the only reader. The driver never writes. Bundled pin moves to srcfl/device-drivers@60021a0 (srcfl/device-drivers#100). 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: 5e7ac82aaa
ℹ️ 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".
| '<label class="drv-read-pv" style="margin-top:8px;display:flex;align-items:center;gap:6px;font-weight:normal">' + | ||
| '<input type="checkbox" data-checkbox-path="drivers.' + idx + '.config.read_pv"' + | ||
| (lcfg.read_pv ? ' checked' : '') + '>' + | ||
| 'Read PV from devices on this Zap ' + | ||
| help('Pull generation from inverters Zap already talks to. Leave this off when a native inverter driver owns PV, so Combined does not count the same array twice.') + | ||
| '</label>' + | ||
| '<label class="drv-read-battery" style="margin-top:8px;display:flex;align-items:center;gap:6px;font-weight:normal">' + | ||
| '<input type="checkbox" data-checkbox-path="drivers.' + idx + '.config.read_battery"' + |
There was a problem hiding this comment.
When an existing site still resolves drivers/zap.lua to a managed or local pre-3.1 driver, these filename-only controls remain visible even though that driver does not implement read_pv or read_battery. Saving either switch therefore succeeds but produces no telemetry, misleading the operator into believing ingest is enabled. The repository deliberately gives managed/local drivers precedence over the newly pinned bundled copy, so render these controls only when the active catalog entry/version declares the corresponding capability.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
srcfl/device-drivers#100 landed as 19dd64cc. Point the recovery snapshot at that commit instead of the PR head. Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ce74bdf. Configure here.
| ], | ||
| "repository": "srcfl/device-drivers", | ||
| "commit": "042e0efe8827946de8178845746948f8555b3622", | ||
| "commit": "19dd64ccc563d4b1bd102793b066e3f75a83687d", |
There was a problem hiding this comment.
Bundled pin reverts intended commit
High Severity
The bundled pin moves from 60021a0 to 19dd64c, but this PR states the recovery snapshot should use 60021a0 — current device-drivers main plus the Zap 3.1.0 change (and the nibe_local / foxess_h3_smart / myuplink fixes already on main). Restoring the earlier pin can drop that Zap ingest support and those other driver fixes from the image that falls back to this snapshot.
Reviewed by Cursor Bugbot for commit ce74bdf. Configure here.
1 participant
Summary
Zap is still the P1/HAN site meter by default. Sites that cannot talk to the inverter from FTW — closed SolarEdge Modbus, no SetApp, or Zap already owning the RS-485 bus — can turn on a read-only ingest of PV and/or battery under Settings → Devices.
That is the Discord case: Zap already had the PV reading, native Modbus did not. Dual-Modbus sites keep the 3.0 path because both flags default off.
The driver never writes. Chargers stay out.
Depends on srcfl/device-drivers#100 (
zap3.1.0). Bundled pin moves to60021a0. That pin is currentdevice-driversmain plus the Zap change, so the recovery snapshot also picks upnibe_local1.1.3,foxess_h3_smart0.9.5 and a smallmyuplinkfix already on main. After #100 merges, move the pin to the merge commit if GitHub squashes.Operator path
Settings → Devices → Zap:
YAML equivalent:
config.read_pv: true/config.read_battery: true.Test plan
go test ./internal/drivers -run 'Zap|zap|CatalogSourceful'go test ./internal/drivers ./cmd/ftwnode --test web/setup.test.mjsmake verify(pre-commit)Note
Medium Risk
Changes how site PV/battery telemetry can be sourced (double-count risk if opted in alongside a native driver). Reads stay opt-in and the driver remains write-inert.
Overview
Sourceful Zap stays the P1/HAN site meter by default, but operators can opt in to read-only PV and/or battery ingest from devices Zap already talks to (
read_pv/read_batteryin YAML, or Settings → Devices). Use this when FTW cannot talk to the inverter natively (closed Modbus, Zap owns RS-485). Chargers are still not ingested; the driver never writes.Catalog capabilities now include
pvandbatteryso the UI can offer those switches. Zap is excluded from the generic disable-PV/disable-battery overlap controls so Combined is not double-counted unless the operator turns a Zap read on.Docs, example config, setup copy, and tests cover mixed sites, inverter-only Zap, and aggregation of multiple listed inverters. Bundled driver pin moves with the Zap 3.1.0 snapshot.
Reviewed by Cursor Bugbot for commit ce74bdf. Bugbot is set up for automated code reviews on this repo. Configure here.