From 7cadc437fd99102ac57975e5377762da27bad2a4 Mon Sep 17 00:00:00 2001 From: Claude Opus 5 Date: Fri, 31 Jul 2026 13:01:38 +0200 Subject: [PATCH] fix(catalog): stop badging five drivers Control when they refuse every command myuplink, nibe_local, pixii_pv, solis_string and tibber declared control: true. All five have a driver_command that refuses unconditionally, and four say so in a comment on the line above the return false -- "Read-only: no actuation", "Tibber Pulse is a read-only meter; no control surface". The Control badge is the first thing an owner reads when deciding whether a driver can run their heat pump or their battery, and for these five it promised something the code refuses. They are now control: false, which is what their sources have said all along. No Lua changed and no artifact bytes moved: control is not among the fields the channel copies into the signed artifact, so no version had to move. Verified by rebuilding the channel from main and comparing all 80 digests. tests/test_control_claims.py compares the claim against the code, which nothing did before. A driver_command that calls nothing cannot reach a write however it is invoked. Reverting any of the five fails it by name. It also asserts at least 20 drivers still read as controlling, because a guard that stops recognising anything passes exactly as loudly as one that works -- which its own unit test caught while it was being written, when the call regex counted `function driver_command(` and so found every driver busy. Signed-off-by: HuggeK <48095810+HuggeK@users.noreply.github.com> Co-authored-by: HuggeK <48095810+HuggeK@users.noreply.github.com> --- CHANGELOG.md | 2 + devices.yaml | 10 +-- index.yaml | 10 +-- manifests/myuplink.yaml | 2 +- manifests/nibe_local.yaml | 2 +- manifests/pixii_pv.yaml | 2 +- manifests/solis_string.yaml | 2 +- manifests/tibber.yaml | 2 +- tests/test_control_claims.py | 123 +++++++++++++++++++++++++++++++++++ 9 files changed, 140 insertions(+), 15 deletions(-) create mode 100644 tests/test_control_claims.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 628f22e..c42e4c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,8 @@ Driver versions follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html - **The release's version rule now runs on every pull request, where it can still be acted on.** The signed channel refuses to republish changed artifact bytes under a version it has already published — the rule that makes a version name a set of bytes an operator can pin. Asking it needed the published channel, so it only ever ran from the release job on `main`, *after* the merge: #53 merged with five green checks and took the release down, and the repair (#55) had to be a second pull request against a `main` that was already broken. Nothing about the question needs the signing key, though — the published manifest is a release asset and its signature verifies with the public key — so `ftw_repository.py check-versions` builds the artifacts in memory, signs nothing, and answers exactly what the release would. The new `signed channel accepts this tree` job runs it on every pull request, including from forks, which need no secret to read a public asset. It reports **every** colliding driver rather than the first, each with the `make bump-driver` command that fixes it. `_version_collisions` is the single statement of the rule that both the release and this check read, and `test_check_versions_answers_what_the_release_would_answer` holds them to the same verdict on the same tree — a rule stated twice is a rule that drifts, and a check that drifts from the release is worse than no check. `test_check_versions_catches_a_manifest_only_edit` is #53 in the shape it arrived in: a manifest field corrected, no Lua touched, and the bytes moved anyway ### Fixed +- **Five drivers were badged **Control** on the catalog page while being incapable of it.** `myuplink`, `nibe_local`, `pixii_pv`, `solis_string` and `tibber` all declared `control: true`, and all five have a `driver_command` that refuses unconditionally — four of them saying so in a comment on the line above the `return false`: *"Read-only: no actuation"*, *"Tibber Pulse is a read-only meter; no control surface"*. The page's **Control** badge is the first thing an owner reads when deciding whether a driver can run their heat pump or their battery, and for these five it promised something the code refuses. All five are now `control: false`, which is what their sources have said all along. No Lua changed and no artifact bytes moved — `control` is not among the fields the channel copies into the signed artifact — so no driver version had to move either +- `tests/test_control_claims.py` compares the claim against the code, which nothing did before. A `driver_command` that calls nothing cannot reach a write however it is invoked, so a manifest declaring control over one is making a claim its own entrypoint refuses. Reverting any of the five to `control: true` fails it by name, with the two ways out stated. It also asserts that at least 20 drivers still read as controlling, because a guard that quietly stops recognising anything passes exactly as loudly as one that works — a bug the guard's own unit test caught while it was being written, when the regex counted `function driver_command(` as a call and so found every driver busy - **`nibe_local` was filed as a meter.** Its manifest declared `ders: [meter]`; it reads a NIBE S-series heat pump. The driver emits nothing but metrics — `hp_power_w`, `hp_used_power_w`, the temperatures and the lifetime energy counters, all through `host.emit_metric` and never through `host.emit("meter", …)` — which its header states is deliberate, so that a pump's electrical draw cannot double-count against the site's real grid meter. The catalog was advertising a DER the driver does not provide, and the published page (#51) badges and filters on exactly that field: the pump sat under **Meter** and was absent from **Heat pump**, where an owner looking for it would start. Now `ders: [heatpump]`, matching `heishamon` — the other metric-only heat pump, which has been filed that way since it landed. The Lua source and its `sha256` are untouched, so the driver a host runs is byte-for-byte the one it ran before - **`nibe_local`** 1.1.0 → 1.1.1 — **the version has to move anyway, because the signed artifact carries this metadata.** `ders` is not catalog-only decoration: `_load_channel` copies it into the artifact's `capabilities` and the channel publishes source and metadata as one signed file, so correcting the field changed the published bytes. The channel then refused the release — `error: nibe_local: changed artifact needs a higher version than 1.1.0` — and it was right to, since that immutability is what lets an operator name the bytes a site is running. A patch rather than a minor: no register, field or emitted metric changed, only the DER the catalog states. The `DRIVER` table is deliberately left alone, keeping the driver byte-identical to its FTW baseline the way #29 established - **`myuplink` was filed under no device type at all.** Its manifest declared `ders: []`, so the one MyUplink cloud heat-pump driver was absent from **Heat pump** and from every other filter on the catalog page — unreachable by device type, which is how an owner looks for their pump. It is now `ders: [heatpump]`, matching the other two heat pumps. This revisits a deliberate choice: when #27 widened the DER vocabulary it set `myuplink` to none, reading `ders` as the emit stream a driver produces rather than the device it covers. #53 then settled the question the other way for `nibe_local`, citing `heishamon` — and the three drivers are the same case. All three report a pump through `host.emit_metric` and never call `host.emit`, `myuplink` and `heishamon` emit the same `hp_*` metric names, and two of the three already declared `heatpump`. `myuplink` was the one left behind, not the one holding the line diff --git a/devices.yaml b/devices.yaml index 9946885..a2dfcfa 100644 --- a/devices.yaml +++ b/devices.yaml @@ -542,7 +542,7 @@ manufacturers: driver: "solis_string" version: "1.1.1" ders: [pv] - control: true + control: false firmware_versions: "" notes: "Solis non-hybrid PV inverters via Modbus TCP." - name: "Growatt" @@ -965,7 +965,7 @@ manufacturers: driver: "myuplink" version: "1.1.1" ders: [heatpump] - control: true + control: false firmware_versions: "" notes: "Read-only heat-pump telemetry via MyUplink Cloud REST API v2: compressor power + hot-water/indoor/outdoor temperatures. Observe-only — no control. OAuth: authorization-code + refresh-token (connect in Settings → Devices)." - name: "NES" @@ -991,7 +991,7 @@ manufacturers: driver: "nibe_local" version: "1.1.1" ders: [heatpump] - control: true + control: false firmware_versions: "" notes: "Read-only NIBE S-series heat-pump telemetry over the on-prem Local REST API (HTTPS + Basic auth, self-signed cert pinned via tls_pin_sha256). Emits compressor/used power, lifetime energy meters, and the full ~980-point register map. Observe-only — no control." - name: "OpenEVSE" @@ -1030,7 +1030,7 @@ manufacturers: driver: "pixii_pv" version: "0.3.0" ders: [pv, meter] - control: true + control: false firmware_versions: "" notes: "Read-only Pixii telemetry: PV (external CTs) + grid meter (meter_w). Battery / SoC comes from the Modbus driver, not this one." - name: "PowerShaper" @@ -1794,7 +1794,7 @@ manufacturers: driver: "tibber" version: "1.1.0" ders: [meter] - control: true + control: false firmware_versions: "" notes: "Tibber Pulse grid meter via GraphQL-transport-ws liveMeasurement stream." - name: "VARTA" diff --git a/index.yaml b/index.yaml index 45aefd1..72ee92c 100644 --- a/index.yaml +++ b/index.yaml @@ -375,7 +375,7 @@ drivers: connectivity: cloud setup: [vendor_portal] ders: [heatpump] - control: true + control: false size_bytes: 15278 sha256: "d754fd4af96c2047f9d9e458a776ad8f10510569836618f4631dd70e2510f404" - name: "nibe_local" @@ -385,7 +385,7 @@ drivers: connectivity: local setup: [device_screen] ders: [heatpump] - control: true + control: false size_bytes: 18227 sha256: "b89177004a2eff5a5b60e2f275f584f079acf02876c049b231ba3170bb35ac50" - name: "opendtu" @@ -468,7 +468,7 @@ drivers: protocol: mqtt connectivity: local ders: [pv, meter] - control: true + control: false size_bytes: 6692 sha256: "d0901b6478e05b5cdf46fc2a9675700ba6008be2feb370be0ff7e87d2f1de883" - name: "saj" @@ -631,7 +631,7 @@ drivers: protocol: modbus connectivity: local ders: [pv] - control: true + control: false size_bytes: 5784 sha256: "3cfcf53653680dede7caf6173cee251efa3cf2c9ec46fdc16a89b9b526dceb7e" - name: "sonnen" @@ -670,7 +670,7 @@ drivers: connectivity: cloud setup: [vendor_portal] ders: [meter] - control: true + control: false size_bytes: 12410 sha256: "845ca59a775a3a4a4c27962454f14ba654fba0d1b905f24647c80db5c26a62ae" - name: "varta" diff --git a/manifests/myuplink.yaml b/manifests/myuplink.yaml index 03a17c5..c07bedd 100644 --- a/manifests/myuplink.yaml +++ b/manifests/myuplink.yaml @@ -6,7 +6,7 @@ protocol: http connectivity: cloud setup: [vendor_portal] ders: [heatpump] -control: true +control: false tested_devices: - manufacturer: "MyUplink (NIBE, Bosch, Atlantic, Daikin, ...)" model_family: "MyUplink Heat Pump (telemetry)" diff --git a/manifests/nibe_local.yaml b/manifests/nibe_local.yaml index 960b9ad..80c1c79 100644 --- a/manifests/nibe_local.yaml +++ b/manifests/nibe_local.yaml @@ -6,7 +6,7 @@ protocol: http connectivity: local setup: [device_screen] ders: [heatpump] -control: true +control: false tested_devices: - manufacturer: "NIBE" model_family: "NIBE REST API S-series" diff --git a/manifests/pixii_pv.yaml b/manifests/pixii_pv.yaml index 26967cf..0e699f4 100644 --- a/manifests/pixii_pv.yaml +++ b/manifests/pixii_pv.yaml @@ -5,7 +5,7 @@ author: "Sourceful Labs AB" protocol: mqtt connectivity: local ders: [pv, meter] -control: true +control: false tested_devices: - manufacturer: "Pixii" model_family: "Pixii PowerShaper (PV + meter)" diff --git a/manifests/solis_string.yaml b/manifests/solis_string.yaml index 8b0ba0c..133da9a 100644 --- a/manifests/solis_string.yaml +++ b/manifests/solis_string.yaml @@ -5,7 +5,7 @@ author: "Sourceful Labs AB" protocol: modbus connectivity: local ders: [pv] -control: true +control: false tested_devices: - manufacturer: "Ginlong Solis" model_family: "Solis string inverter" diff --git a/manifests/tibber.yaml b/manifests/tibber.yaml index 7ed6da5..ecd2136 100644 --- a/manifests/tibber.yaml +++ b/manifests/tibber.yaml @@ -6,7 +6,7 @@ protocol: http connectivity: cloud setup: [vendor_portal] ders: [meter] -control: true +control: false tested_devices: - manufacturer: "Tibber" model_family: "Tibber Pulse" diff --git a/tests/test_control_claims.py b/tests/test_control_claims.py new file mode 100644 index 0000000..c15e7a0 --- /dev/null +++ b/tests/test_control_claims.py @@ -0,0 +1,123 @@ +"""A driver badged "Control" must be able to control something. + +`control` is the manifest field the catalog page turns into a **Control** +badge, and it is the first thing an owner reads when deciding whether a driver +can run their battery or their heat pump. Five drivers carried `control: true` +while their `driver_command` was an unconditional refusal -- three of them +saying so in a comment on the line above the `return false`. + +Nothing caught it, because nothing compared the claim against the code. The +signed channel does not settle it either: it infers control from the mere +presence of a `driver_command` entrypoint, so a stub that always refuses reads +to it as a control path. + +This is the narrow, checkable half of that question: a command entrypoint that +cannot call anything cannot actuate anything, whatever the manifest says. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] + +# Keywords that open a Lua block, for balancing against `end`. +OPENERS = re.compile(r"\b(function|if|for|while|do)\b") +CLOSERS = re.compile(r"\bend\b") +ELSEIF = re.compile(r"\belseif\b") +CALL = re.compile(r"\b([A-Za-z_][A-Za-z_0-9.]*)\s*\(") +NOT_A_CALL = {"function", "if", "while", "for", "return", "and", "or", "not"} + + +def control_flag(manifest: Path) -> str: + for line in manifest.read_text(encoding="utf-8").splitlines(): + if line.startswith("control:"): + return line.split(":", 1)[1].strip() + return "" + + +def function_source(text: str, name: str) -> str | None: + """The source of a top-level `function name(...) ... end`, or None.""" + start = re.search(rf"^function\s+{re.escape(name)}\s*\(", text, re.M) + if not start: + return None + depth = 0 + collected: list[str] = [] + for line in text[start.start():].split("\n"): + collected.append(line) + code = re.sub(r"--.*$", "", line) + depth += len(OPENERS.findall(code)) + depth -= len(CLOSERS.findall(code)) + depth -= len(ELSEIF.findall(code)) + if len(collected) > 1 and depth <= 0: + break + return "\n".join(collected) + + +def is_inert(body: str) -> bool: + """Whether a function body can do nothing at all. + + Inert means it calls nothing: no host call, no helper, no dispatch. Such a + body can only fall through to its `return`, so it cannot reach a write no + matter what it is handed. + """ + # Drop the declaration line: `function driver_command(...)` is the + # signature, not a call, and counting it made every body look busy. + lines = re.sub(r"--.*$", "", body, flags=re.M).split("\n")[1:] + code = "\n".join(line for line in lines if line.strip()) + calls = [c for c in CALL.findall(code) if c not in NOT_A_CALL] + return not calls + + +def test_a_control_driver_has_a_command_that_can_do_something() -> None: + inert = [] + for manifest in sorted((ROOT / "manifests").glob("*.yaml")): + if control_flag(manifest) != "true": + continue + source = ROOT / "drivers" / "lua" / f"{manifest.stem}.lua" + body = function_source(source.read_text(encoding="utf-8"), "driver_command") + if body is not None and is_inert(body): + inert.append(manifest.stem) + + assert not inert, ( + "these manifests claim control: true, but their driver_command calls " + f"nothing and so can never actuate: {inert}. Either the driver has a " + "control path its command entrypoint does not reach, or the manifest " + "should say control: false.") + + +def test_the_check_can_still_see_a_real_control_path() -> None: + """A guard that flags nothing would pass just as quietly if it broke.""" + controlling = [] + for manifest in sorted((ROOT / "manifests").glob("*.yaml")): + if control_flag(manifest) != "true": + continue + source = ROOT / "drivers" / "lua" / f"{manifest.stem}.lua" + body = function_source(source.read_text(encoding="utf-8"), "driver_command") + if body is not None and not is_inert(body): + controlling.append(manifest.stem) + + assert len(controlling) >= 20, ( + "almost no driver reads as controlling, so this check has stopped " + f"measuring anything: {controlling}") + + +def test_a_refusal_stub_is_recognised_as_inert() -> None: + """The shape the five carried, so the guard is pinned to it directly.""" + assert is_inert( + "function driver_command(_action, _power_w, _cmd)\n" + " -- Read-only: no actuation.\n" + " return false\n" + "end") + # Accepting the lifecycle actions without acting on them is still inert. + assert is_inert( + "function driver_command(action)\n" + ' if action == "init" or action == "deinit" then return true end\n' + " return false\n" + "end") + # Reaching any call at all is not. + assert not is_inert( + "function driver_command(action, value)\n" + " return host.modbus_write(40001, value)\n" + "end")