Skip to content

feat(nibe_local): opt-in Solar PV surplus write path (srcfl/ftw#537) - #46

Open
HuggeK wants to merge 4 commits into
srcfl:mainfrom
HuggeK:nibe-solar-pv-feed
Open

feat(nibe_local): opt-in Solar PV surplus write path (srcfl/ftw#537)#46
HuggeK wants to merge 4 commits into
srcfl:mainfrom
HuggeK:nibe-solar-pv-feed

Conversation

@HuggeK

@HuggeK HuggeK commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

What

nibe_local gains its first write path, and deliberately its only one: the
pump'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:

  1. host capabilities.http.allow_write (the new FTW-core write grant),
  2. driver write.solar_pv: true with a mandatory write.max_w clamp ceiling,
  3. the owner-side enable on the pump itself (register 2107).

Every clamp answers a quantified risk:

  • value clamped to [0, max_w] — a sign bug or telemetry spike must not tell the pump there are 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 (idempotent, rate-limited);
  • 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 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_patch

The write uses a new host verb, host.http_patch, added in a companion
srcfl/ftw PR (host-side gate capabilities.http.allow_write). On a core
without 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:

  • wrapped the read path's http_get and json_decode in pcall (matches the esphome-dsmr idiom);
  • lowercased the three mutable interval locals (setup_retry_ms, poll_interval_ms, full_refresh_ms) 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_registersresolve_pv_points: the _registers suffix made test_no_undefined_locals read a declared local function as an undeclared register guard.

baselines/ftw is 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

  • New fake-pump Lua harness test_nibe_local_ftw.lua: clamp, deadband, sign conversion, pump-side gates (2107 disabled, read-only 200 rejection), dead-man's switch, orphan clear, u16 sentinel skip, rate-limit decrease bypass, default-mode idempotence, invalid-max_w keeps clearing armed.
  • host_mock.lua gains an http_patch mock; spec/host-api-profile.json declares http_patch.
  • Full catalog suite green for nibe_local (convention + test_no_undefined_locals); make check validators (sync/validate/host-api/history/baseline/sandbox) pass.

🤖 Generated with Claude Code

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 <48095810+HuggeK@users.noreply.github.com>
Co-authored-by: HuggeK <48095810+HuggeK@users.noreply.github.com>

frahlg commented Jul 29, 2026

Copy link
Copy Markdown
Member

Automated maintenance pass: not merging this one, and flagging it for a maintainer rather than acting further.

Reasons, in order of weight:

  1. This is the repo's first write path on nibe_local, and AGENTS.md is explicit that new drivers start read-only and that control needs "a safe default mode, bounded leases, structured results and HIL acceptance for every target host." Whatever the quality of the fake-pump Lua harness, HIL acceptance on a real S-series unit is a human decision this session can't make or substitute for.
  2. It depends on an unlanded companion change — FTW core's host.http_patch verb (tracked in Opt-in writable NIBE driver: feed PV production to the Solar PV registers (2107/2108/2109), later cap the electric add-heat ftw#537) — so even a clean merge here doesn't reach working control until that lands and the driver is re-verified against it.
  3. It's from a non-collaborator contributor via a fork, so GitHub has held CI as action_required — nothing here has actually run against this repo's runners yet.
  4. It's marked draft by its author, which already keeps it out of the merge queue.

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

claude and others added 2 commits August 4, 2026 20:23
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 <48095810+HuggeK@users.noreply.github.com>
Signed-off-by: Hugo Karlsson <48095810+HuggeK@users.noreply.github.com>
Signed-off-by: Hugo Karlsson <48095810+HuggeK@users.noreply.github.com>
@HuggeK
HuggeK force-pushed the nibe-solar-pv-feed branch from fd30fef to 4a17db5 Compare August 4, 2026 18:25

frahlg commented Aug 5, 2026

Copy link
Copy Markdown
Member

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 action_required since it came from a fork).

What hasn't changed, and is why this still isn't a merge:

  1. This is nibe_local's first write path. AGENTS.md is explicit that new drivers start read-only and that control requires "a safe default mode, bounded leases, structured results and HIL acceptance for every target host." The Lua harness here (fake-pump test_nibe_local_ftw.lua) is thorough on the software side, but it isn't hardware-in-the-loop acceptance on a real S-series unit — that's a human/hardware step this session can't perform or waive.
  2. The write path is inert without FTW core's host.http_patch verb (tracked in Opt-in writable NIBE driver: feed PV production to the Solar PV registers (2107/2108/2109), later cap the electric add-heat ftw#537), which hasn't landed. The driver degrades to read-only without it, so nothing unsafe ships today — but "safe to merge" and "ready to actually control a pump" are different questions, and the second one still needs that companion change plus re-verification against it.

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

frahlg commented Aug 6, 2026

Copy link
Copy Markdown
Member

Automated maintenance pass: re-checked. Verdict unchanged — still not merging, still flagged for a maintainer — but one fact from the last pass needs correcting.

host.http_patch / capabilities.http.allow_write (srcfl/ftw#537) has landed and shipped, in ftw 1.14.0 (confirmed against ftw's CHANGELOG.md and go/internal/drivers/lua.go/host.go at current main). So the companion dependency is no longer a blocker — a host running 1.14.0+ would actually pick up the write path once merged, not just degrade to read-only.

That doesn't change the merge decision, though: this is still nibe_local's first write path, and AGENTS.md's requirement — "Control needs a safe default mode, bounded leases, structured results and HIL acceptance for every target host" — is a hardware-in-the-loop step nobody has performed yet. The fake-pump Lua harness is thorough but isn't HIL acceptance on a real S-series unit. dco is also still failing on this PR's head commit.

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

frahlg commented Aug 7, 2026

Copy link
Copy Markdown
Member

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 driver_command results consistent with the rest of the catalog.

Two gaps worth a human look before this lands:

  1. No hardware-in-the-loop evidence. Test coverage here is a new Lua harness against a mocked pump only — no real S-series capture or field data in the diff, which AGENTS.md calls for on any control change ("Control needs a safe default mode, bounded leases, structured results and HIL acceptance for every target host").
  2. The write-refusal-counting rule (docs/WRITING-A-DRIVER.md, "the same rule for writes") isn't applied here, and CI doesn't catch it: test_refused_write_settles.py only probes drivers with PROTOCOL == "modbus", and nibe_local is PROTOCOL = "http" (writes via host.http_patch), so it's silently skipped. In the current code, driver_default_mode has no bounded refusal counter — if the pump keeps refusing the clear write (e.g. left read-only on the pump side), it retries every 60s indefinitely instead of counting to N and reporting the state as held, the way sungrow 1.5.7 does. Might be worth a follow-up issue for the refusal-counting check itself, since this gap would apply to any future HTTP-protocol control driver, not just this one.

Also: dco is currently failing on the head commit, and the PR base (1.1.1) is behind current main (1.1.2), so there's a real merge conflict to resolve independent of the above.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants