feat(nibe_local): opt-in Solar PV surplus write path (srcfl/ftw#537) - #46
feat(nibe_local): opt-in Solar PV surplus write path (srcfl/ftw#537)#46HuggeK wants to merge 5 commits into
Conversation
The NIBE local driver gains its first write path, and deliberately its only one: the pump's native Solar PV surplus feed. The S-series was built to take a live "available solar power" number from NIBE's Modbus accessory (registers 2107 enable / 2109 available power) and soak the surplus into heating and hot water using owner-tuned offsets. FTW now acts as that accessory. Control by hint — the pump's firmware decides what to do with the number, so a wrong value degrades to wasted comfort, never to unsafe operation. Off by default and triple-gated: the host `capabilities.http.allow_write` grant, the driver's `write.solar_pv: true` with a mandatory `write.max_w` clamp ceiling, and the owner-side enable on the pump itself (register 2107). Local-API variableIds are resolved from each point's modbusRegisterID at poll time, so 2107/2109 are never hard-coded. Safety posture (each clamp answers a quantified risk): - value clamped to [0, max_w] so a sign bug or telemetry spike cannot tell the pump there is 100 kW of surplus; - deadband + at-most-one-write-per-interval against register churn; a decrease past the deadband is written immediately (surplus collapsed = safety direction); - dead-man's switch clears the feed when commands stop — the pump's timeout for a silently stopped feed is undocumented, so the driver does not lean on it; - driver_default_mode clears the feed on watchdog trip / stale site meter / driver stop, rate-limited and idempotent; - a startup sweep clears a non-zero feed a crashed run left standing; - the clearing machinery stays armed even when a config mistake (missing max_w) refuses NEW writes, so a mistake never strands a standing feed. The Local REST API's most dangerous habit is covered: it rejects a write *inside* an HTTP 200 with a per-point "error: read only value" result. The driver judges success on the body, never the status code, and surfaces a read-only pump as an actionable error naming the installer menu (7.5.15). Requires `host.http_patch` from the FTW core; on older cores the driver says so and stays read-only. The header documents the decommission step (turn 2107 off, or set the API read-only) for the day FTW is no longer there to clear the feed. Editing this promoted driver drops its byte-identity exemption from baselines/ftw, so the full catalog convention suite now applies to it. Brought it into compliance without changing behaviour: - wrapped the read path's http_get and json_decode in pcall; - lowercased the three mutable interval locals so driver_poll's returns match the poll-interval convention (they are reassigned from config, so lowercase is the honest style; genuine constants stay UPPERCASE); - renamed resolve_pv_registers -> resolve_pv_points: the `_registers` suffix made test_no_undefined_locals read a declared local function as an undeclared register guard. authors is set to "Claude Code (with the help of HuggeK)" per request. Tests: a fake-pump Lua harness (test_nibe_local_ftw.lua) exercising the clamp, deadband, sign conversion, pump-side gates, dead-man's switch, orphan clear, sentinel skip and default-mode idempotence; host_mock gains an http_patch mock; spec/host-api-profile.json declares http_patch. Signed-off-by: Hugo Karlsson <[email protected]> Co-authored-by: HuggeK <[email protected]>
|
Automated maintenance pass: not merging this one, and flagging it for a maintainer rather than acting further. Reasons, in order of weight:
The safety reasoning in the PR body (register risk classification, triple-gating, dead-man's switch) reads carefully thought through, and the write-path design looks sound on review. Flagging for a maintainer to: (a) approve CI so it actually runs, (b) decide when/whether to pursue HIL acceptance on real hardware, and (c) coordinate the FTW-side dependency before this leaves draft. Generated by Claude Code |
The write path existed and had no way to be turned on except by hand-editing two keys in config.yaml on the gateway — config.write.solar_pv here, and capabilities.http.allow_write on the host. FTW's Settings screen now renders a switch for a write path, but only for a driver that states it has one (srcfl/ftw#769), which is what keeps that switch off every read-only driver in the catalog. write_capabilities is metadata, not a grant: declaring the path is what makes it offerable, and the feed still needs the operator to turn it on, a max_w ceiling, and the pump's own Solar PV input enabled. Co-authored-by: HuggeK <[email protected]> Signed-off-by: Hugo Karlsson <[email protected]>
Signed-off-by: Hugo Karlsson <[email protected]>
fd30fef to
4a17db5
Compare
|
Automated maintenance pass: re-checked, still not merging — status update rather than a new verdict. What changed since the last pass: this PR is no longer draft, and CI now runs and is green on this repo's own runners (previously held as What hasn't changed, and is why this still isn't a merge:
Flagging for a maintainer to decide when/whether to run HIL acceptance on real hardware and to coordinate the FTW-side dependency. The design itself (register risk classification, triple-gating, dead-man's switch) continues to read as careful work. Generated by Claude Code |
|
Automated maintenance pass: re-checked. Verdict unchanged — still not merging, still flagged for a maintainer — but one fact from the last pass needs correcting.
That doesn't change the merge decision, though: this is still Flagging for a maintainer to decide when/whether to run HIL acceptance on a real pump; the design itself continues to read as careful, safety-conscious work. Generated by Claude Code |
|
Automated repo maintenance pass (device-driver PR review). This is nibe_local's first write path on a live heat pump, and the design covers most of AGENTS.md's control bar well: a safe default mode that clears the feed on watchdog/stale-meter/stop, a bounded 300s dead-man's-switch lease plus a startup orphan sweep, and structured Two gaps worth a human look before this lands:
Also: Generated by Claude Code |
|
Branch updated with main; the conflicts were the catalog-wide 1.1.2 patch bumps from d128510 landing under this PR's 1.2.0 (kept — 1.2.0 was minted from 1.1.1 and the merged driver carries the verification-claim metadata), plus both sides' CHANGELOG bullets (both kept). Contract tests pass locally on the merged tree. 🤖 Generated with Claude Code |
# Conflicts: # CHANGELOG.md Signed-off-by: Hugo Karlsson <[email protected]>
Conflicts were the nibe_local version (branch 1.2.0 supersedes main's catalog-wide 1.1.2 patch bump from the verification-claim change) and both sides' new CHANGELOG bullets, all kept. Co-authored-by: HuggeK <[email protected]> Signed-off-by: Hugo Karlsson <[email protected]>
059dfce to
de17764
Compare
miravoss26
left a comment
There was a problem hiding this comment.
First write path for nibe_local — the pump's native Solar PV surplus feed (control-by-hint, so a wrong value degrades to wasted comfort, never unsafe operation). This is unusually careful work for an actuation change and I read it closely because of that.
Traced the deadband/rate-limit logic against the test suite — it's correct: the unconditional max_w clamp applies before any rate gate, a genuine decrease beyond the deadband bypasses the rate limit and writes immediately (safety direction), small jitter in either direction is swallowed, and an explicit zero always bypasses both the rate limit and the pump-side enable gate. The startup orphan-sweep correctly distinguishes a standing feed from the u16 not-connected sentinel (65535) so it doesn't turn "no accessory" into "accessory reporting zero."
Safety layering checks out: host allow_write + driver write.solar_pv+mandatory max_w + pump-side register 2107 enable, dead-man's switch for the undocumented pump-side timeout, default-mode clear gated on requested (not the validated flag) so a bad max_w config can't disarm the clearing machinery — a nice detail, and it's exercised by its own test.
Two minor, non-blocking notes:
write_pv_surplusjudges success via substring match (find(resp, "error")) rather than real JSON parsing of the per-point status. Matches the pump's fixed vocabulary and is tested, but a response shape with "error" appearing in an unrelated field would false-positive. Low risk given the closed vocabulary.serial(from the device's own registration response) is concatenated raw into the PATCH URL path — same pattern as the read-only driver this builds on (#7), not new here, and still same-host/pinned-cert scoped.
Test coverage is genuinely comprehensive — every safety branch has a corresponding assertion. verification_status stays beta, which matches a first write path that hasn't seen extended field hours yet. Safe to merge from my read.
What
nibe_localgains its first write path, and deliberately its only one: thepump's native Solar PV surplus feed (registers 2107 enable / 2109
available power). The NIBE S-series was built to take a live "available solar
power" number from NIBE's own Modbus accessory and soak the surplus into
heating and hot water using owner-tuned offsets. FTW now acts as that
accessory — control by hint: the pump's firmware decides what to do with
the number, so a wrong value degrades to wasted comfort, never to unsafe
operation.
Tracks srcfl/ftw#537.
Safety posture
Off by default and triple-gated:
capabilities.http.allow_write(the new FTW-core write grant),write.solar_pv: truewith a mandatorywrite.max_wclamp ceiling,Every clamp answers a quantified risk:
[0, max_w]— a sign bug or telemetry spike must not tell the pump there are 100 kW of surplus;driver_default_modeclears the feed on watchdog trip / stale site meter / driver stop (idempotent, rate-limited);max_w) refuses new writes, so a mistake never strands a standing feed.The Local REST API's most dangerous habit is covered: it rejects a write
inside an HTTP 200 with a per-point
error: read only valueresult. Thedriver judges success on the body, never the status code, and surfaces a
read-only pump as an actionable error naming installer menu 7.5.15. The
driver header documents the decommission step (turn 2107 off, or set the API
read-only) for the day FTW is no longer there to clear the feed.
Requires FTW core
host.http_patchThe write uses a new host verb,
host.http_patch, added in a companionsrcfl/ftw PR (host-side gate
capabilities.http.allow_write). On a corewithout it the driver detects the absence, says so, and stays read-only —
so this driver is safe to publish ahead of the core change.
Catalog-convention compliance (why the driver body changed beyond the write path)
Editing this promoted driver drops its byte-identity exemption from
baselines/ftw, so the full catalog convention suite now applies to it.Brought it into compliance without changing behaviour:
http_getandjson_decodeinpcall(matches theesphome-dsmridiom);setup_retry_ms,poll_interval_ms,full_refresh_ms) sodriver_poll's returns match the poll-interval convention — they are reassigned from config, so lowercase is the honest style; genuine constants stayUPPERCASE;resolve_pv_registers→resolve_pv_points: the_registerssuffix madetest_no_undefined_localsread a declared local function as an undeclared register guard.baselines/ftwis intentionally left untouched — the baseline means "field-proven, Go-tested", which this new write path is not yet.Why 2107/2109 specifically (register risk classification)
The Solar PV feed is the safest writable register class on the S-series:
it is control-by-hint and self-clearing in intent — the whole design
point is that FTW keeps feeding a number the pump interprets, and stopping the
feed (or the dead-man's clear) returns the pump to its own behaviour. Contrast
with persistent-state registers (degree minutes → short-cycling, operating
mode, DHW/legionella, heat curve) which latch and can harm the pump or comfort,
and forbidden ones (alarm/fault reset, sensor spoofing, defrost) which the
driver never exposes. 2108 ("include own consumption") and the offset
aggressiveness stay owner-tuned on the pump and are never written.
Tests
test_nibe_local_ftw.lua: clamp, deadband, sign conversion, pump-side gates (2107 disabled, read-only 200 rejection), dead-man's switch, orphan clear,u16sentinel skip, rate-limit decrease bypass, default-mode idempotence, invalid-max_wkeeps clearing armed.host_mock.luagains anhttp_patchmock;spec/host-api-profile.jsondeclareshttp_patch.nibe_local(convention +test_no_undefined_locals);make checkvalidators (sync/validate/host-api/history/baseline/sandbox) pass.🤖 Generated with Claude Code