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")