diff --git a/CHANGELOG.md b/CHANGELOG.md index fcfeb47..f424bac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ Driver versions follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html ## [Unreleased] +### Changed +- **`myuplink` 1.2.0** — wraps cloud HTTP and JSON calls in `pcall`, so a failed request or malformed response returns the driver's retry path. The HTTP catalog rules now use `connectivity`: local drivers keep the config host, port and `http://` scheme, while cloud drivers use their vendor endpoint. The driver contract also accepts `host.emit_metric` for metrics-only drivers. + ### Added - **A driver-authoring rule for the hybrid inverter that has no battery** — *A hybrid inverter may have no battery* in `docs/WRITING-A-DRIVER.md`, and rule 8 in `drivers/lua/GUIDELINES.md`. The existing rules cover a read that **failed**; this is the case where nothing failed — the device is healthy, every register answered, and the battery still is not there. Nearly every hybrid inverter is sold both with storage and without it under one model number and one register map, so a PV-only site is not an edge case, it is half the product line. The SG12RT already cited at the top of that document is this same fact arriving as an outage rather than as a wrong number. The rule: fill each battery field only from a register that answered, emit the DER only when at least one did, and **detect** it rather than reading it off the model number or asking the operator to declare it — the site nobody told the driver about is exactly the one that reports wrong. Records how the absence actually arrives, which is vendor-specific and cannot be assumed from one example: registers that go silent (Sigenergy), that answer a plain zero, that answer `0xFFFF`/`0x7FFFFFFF`/NaN (`sma` and `solis` already carry sentinel helpers), or that fault - The rule is stated with the catalog measured rather than asserted: **24 drivers emit both `pv` and `battery`, and 20 of them emit the battery DER with no guard on whether any battery register answered.** Two of the 20 (`ferroamp`, `zap`) gate it on configuration or on API discovery instead — better than nothing, and still not detection, which is why the rule names the difference. `sigenergy` 1.1.3 below is the worked example of the fix. The other 19 are not touched here: each is its own driver, its own register map and its own version, and a sweep that changes nineteen drivers at once is not reviewable diff --git a/SUPPORT_STATUS.md b/SUPPORT_STATUS.md index 29e2003..48dcc4d 100644 --- a/SUPPORT_STATUS.md +++ b/SUPPORT_STATUS.md @@ -86,8 +86,8 @@ Catalog source is not proof that a target can install or run a driver. | kstar | 1.1.1 | blixt-l1 | not_assessed | — | — | not_recorded | — | not_assessed | no | | mennekes | 1.0.3 | ftw-core | not_assessed | — | — | not_recorded | — | not_assessed | no | | mennekes | 1.0.3 | blixt-l1 | not_assessed | — | — | not_recorded | — | not_assessed | no | -| myuplink | 1.1.1 | ftw-core | not_assessed | — | — | not_recorded | — | not_assessed | no | -| myuplink | 1.1.1 | blixt-l1 | not_assessed | — | — | not_recorded | — | not_assessed | no | +| myuplink | 1.2.0 | ftw-core | not_assessed | — | — | not_recorded | — | not_assessed | no | +| myuplink | 1.2.0 | blixt-l1 | not_assessed | — | — | not_recorded | — | not_assessed | no | | nibe_local | 1.1.1 | ftw-core | not_assessed | — | — | not_recorded | — | not_assessed | no | | nibe_local | 1.1.1 | blixt-l1 | not_assessed | — | — | not_recorded | — | not_assessed | no | | opendtu | 1.0.2 | ftw-core | not_assessed | — | — | not_recorded | — | not_assessed | no | diff --git a/devices.yaml b/devices.yaml index 813d6df..b637890 100644 --- a/devices.yaml +++ b/devices.yaml @@ -985,7 +985,7 @@ manufacturers: protocols: - protocol: http driver: "myuplink" - version: "1.1.1" + version: "1.2.0" ders: [heatpump] control: false firmware_versions: "" diff --git a/drivers/lua/myuplink.lua b/drivers/lua/myuplink.lua index 8d44083..d337d1c 100644 --- a/drivers/lua/myuplink.lua +++ b/drivers/lua/myuplink.lua @@ -48,9 +48,18 @@ DRIVER = { id = "myuplink", name = "MyUplink Heat Pump (telemetry)", manufacturer = "MyUplink (NIBE, Bosch, Atlantic, Daikin, ...)", - version = "1.0.0", + version = "1.2.0", protocols = { "http" }, capabilities = { "apicreds" }, + -- Says what the header, the description and driver_command have always + -- said. Without it the channel infers control from the mere presence of a + -- driver_command entrypoint and publishes this driver write-capable. + read_only = true, + -- ...but it can read nothing until it has signed in, and it signs in with a + -- POST. The generated read-only guard allows POST only to a URL ending here + -- and refuses it everywhere else, so this narrows the exemption rather than + -- asserting it. A path, not a URL, because base_url is config-overridable. + auth_post_path = "/oauth/token", description = "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).", homepage = "https://dev.myuplink.com", http_hosts = { "api.myuplink.com" }, @@ -107,16 +116,24 @@ local function fetch_token() .. "&client_id=" .. url_encode(client_id) .. "&client_secret=" .. url_encode(client_secret) .. "&refresh_token=" .. url_encode(refresh_token) - local resp, err = host.http_post( + local post_ok, resp, err = pcall(host.http_post, BASE_URL .. "/oauth/token", body, { ["Content-Type"] = "application/x-www-form-urlencoded" }) + if not post_ok then + host.log("error", "MyUplink: token refresh failed: " .. tostring(resp)) + return false + end if err then host.log("error", "MyUplink: token refresh failed: " .. tostring(err)) return false end - local data = host.json_decode(resp) + local decode_ok, data, decode_err = pcall(host.json_decode, resp) + if not decode_ok then + host.log("error", "MyUplink: invalid refresh response: " .. tostring(data)) + return false + end if not data or not data.access_token then - host.log("error", "MyUplink: no access_token in refresh response") + host.log("error", "MyUplink: no access_token in refresh response: " .. tostring(decode_err)) return false end access_token = data.access_token @@ -147,10 +164,12 @@ end -- ---- API helpers --------------------------------------------------------- local function api_get(path) - local resp, err = host.http_get(BASE_URL .. path, auth_headers()) + local get_ok, resp, err = pcall(host.http_get, BASE_URL .. path, auth_headers()) + if not get_ok then return nil, tostring(resp) end if err then return nil, tostring(err) end - local data, derr = host.json_decode(resp) - if not data then return nil, tostring(derr) end + local decode_ok, data, derr = pcall(host.json_decode, resp) + if not decode_ok then return nil, tostring(data) end + if not data then return nil, tostring(derr or "empty JSON response") end return data, nil end diff --git a/drivers/tests/conftest.py b/drivers/tests/conftest.py index 1bc0bf4..5b63eb9 100644 --- a/drivers/tests/conftest.py +++ b/drivers/tests/conftest.py @@ -137,6 +137,15 @@ def read_manifest(name): return f.read() +def get_driver_connectivity(name): + """Return the manifest's local/cloud connectivity classification.""" + import re + + match = re.search(r'^connectivity:\s*(\S+)', read_manifest(name), re.MULTILINE) + assert match, f"{name}: manifest has no connectivity field" + return match.group(1).strip('"') + + def get_driver_names(): """Get all driver names for parametrization.""" files = glob.glob(os.path.join(DRIVERS_DIR, "*.lua")) diff --git a/drivers/tests/test_driver_contract.py b/drivers/tests/test_driver_contract.py index 5e42ff6..035253c 100644 --- a/drivers/tests/test_driver_contract.py +++ b/drivers/tests/test_driver_contract.py @@ -71,13 +71,13 @@ def test_calls_set_make_in_init(self, driver_name): assert 'host.set_make(' in code, \ f"{driver_name}: should call host.set_make() in driver_init" - def test_calls_emit_in_poll(self, driver_name): - """driver_poll should call host.emit().""" + def test_emits_in_poll(self, driver_name): + """driver_poll should emit DER data or a metric.""" if driver_name == "hello": pytest.skip("hello driver is a demo-only driver") code = read_driver(driver_name) - assert 'host.emit(' in code, \ - f"{driver_name}: should call host.emit() in driver_poll" + assert 'host.emit(' in code or 'host.emit_metric(' in code, \ + f"{driver_name}: should call host.emit() or host.emit_metric() in driver_poll" def test_no_forbidden_globals(self, driver_name): """Driver must not use forbidden sandbox-escaping functions.""" diff --git a/drivers/tests/test_http_drivers.py b/drivers/tests/test_http_drivers.py index c365727..da9975d 100644 --- a/drivers/tests/test_http_drivers.py +++ b/drivers/tests/test_http_drivers.py @@ -9,18 +9,29 @@ from conftest import ( read_driver, get_http_drivers, + get_driver_connectivity, strip_lua_comments, ) HTTP_DRIVERS = get_http_drivers() +def skip_if_cloud(driver_name): + """Local URL rules do not describe a driver that calls a vendor cloud.""" + if get_driver_connectivity(driver_name) == "cloud": + pytest.skip( + f"{driver_name}: cloud HTTP driver uses its vendor endpoint, " + "not a local config host" + ) + + @pytest.mark.parametrize("driver_name", HTTP_DRIVERS) class TestHttpPatterns: """Validate HTTP driver API usage patterns.""" def test_constructs_base_url(self, driver_name): - """HTTP drivers should construct a base URL from config.host.""" + """Local HTTP drivers should construct a base URL from config.host.""" + skip_if_cloud(driver_name) code = read_driver(driver_name) clean = strip_lua_comments(code) @@ -82,10 +93,11 @@ def test_has_http_get_json_helper_or_inline(self, driver_name): @pytest.mark.parametrize("driver_name", HTTP_DRIVERS) class TestHttpUrlSafety: - """Validate URL construction safety.""" + """Validate URL construction safety for local HTTP drivers.""" def test_uses_http_scheme(self, driver_name): - """HTTP drivers should use http:// scheme (not https on constrained devices).""" + """Local HTTP drivers should use http://, not HTTPS.""" + skip_if_cloud(driver_name) code = read_driver(driver_name) clean = strip_lua_comments(code) @@ -96,7 +108,8 @@ def test_uses_http_scheme(self, driver_name): ) def test_uses_config_port(self, driver_name): - """HTTP drivers should use config.port for the connection port.""" + """Local HTTP drivers should use config.port for the connection port.""" + skip_if_cloud(driver_name) code = read_driver(driver_name) clean = strip_lua_comments(code) diff --git a/index.yaml b/index.yaml index caa9a6b..7079416 100644 --- a/index.yaml +++ b/index.yaml @@ -378,15 +378,15 @@ drivers: size_bytes: 3766 sha256: "5f985b8917aea7b08fba13c02506b45e40232a763393dc6af7c6eeb6ab5853af" - name: "myuplink" - version: "1.1.1" + version: "1.2.0" tier: core protocol: http connectivity: cloud setup: [vendor_portal] ders: [heatpump] control: false - size_bytes: 15278 - sha256: "d754fd4af96c2047f9d9e458a776ad8f10510569836618f4631dd70e2510f404" + size_bytes: 16386 + sha256: "6074b70eb2bbe49481fcc59551d65474cb8129ae5370e9c1844c23b08a5f6fd6" - name: "nibe_local" version: "1.1.1" tier: core diff --git a/manifests/myuplink.yaml b/manifests/myuplink.yaml index c07bedd..ffc3bd2 100644 --- a/manifests/myuplink.yaml +++ b/manifests/myuplink.yaml @@ -1,5 +1,5 @@ name: "myuplink" -version: "1.1.1" +version: "1.2.0" tier: core author: "Sourceful Labs AB" protocol: http @@ -21,9 +21,9 @@ upstream_docs: kind: changelog url_stability: stable min_host_version: "2.0.0" -size_bytes: 15278 +size_bytes: 16386 dkb_id: "myuplink" -sha256: "d754fd4af96c2047f9d9e458a776ad8f10510569836618f4631dd70e2510f404" +sha256: "6074b70eb2bbe49481fcc59551d65474cb8129ae5370e9c1844c23b08a5f6fd6" signature: "" bytecode_sha256: "" diff --git a/support-status.json b/support-status.json index 65096ba..deaa390 100644 --- a/support-status.json +++ b/support-status.json @@ -1150,7 +1150,7 @@ }, { "catalog_source": true, - "catalog_version": "1.1.1", + "catalog_version": "1.2.0", "driver_id": "myuplink", "package_id": null, "targets": { diff --git a/tests/test_ftw_repository.py b/tests/test_ftw_repository.py index 94a978f..0ca12d7 100644 --- a/tests/test_ftw_repository.py +++ b/tests/test_ftw_repository.py @@ -23,6 +23,7 @@ from ftw_repository import ( # noqa: E402 RepositoryError, + _load_channel, build_publication, canonical_json, check_publication, @@ -266,6 +267,16 @@ def test_manifest_requires_exact_canonical_envelope_bytes( ), "read-only driver has a write-capable permission", ), + # http.post is the one write permission a read-only driver may hold, + # and only when its metadata declares the sign-in it was granted for. + # Undeclared, it is refused like any other write path. + ( + lambda driver: driver.update( + read_only=True, control_enabled=False, + permissions=["http.get", "http.post"], + ), + "read-only driver has a write-capable permission", + ), ( lambda driver: driver["metadata"].update(read_only=False), "Lua metadata read_only must match the manifest", @@ -683,3 +694,77 @@ def test_a_driver_that_declares_read_only_keeps_its_write_guards( assert sdm630["permissions"] == ["modbus.read"] assert sdm630["read_only"] is True assert sdm630["control_enabled"] is False + + +def test_a_read_only_driver_may_still_sign_in( + tmp_path: Path, keypair: tuple[str, str] +) -> None: + """Reading after authenticating is still reading. + + myuplink cannot actuate anything -- driver_command refuses every command it + is handed -- but it reads nothing until it has exchanged a refresh token, + and it exchanges it with a POST. Denying that POST would have cost a + read-only driver every reading it takes, so read-only would have been + unusable for the drivers that most obviously deserve it. + """ + manifest, output = build(tmp_path, keypair) + myuplink = next(d for d in manifest["drivers"] if d["id"] == "myuplink") + artifact = (output / Path(myuplink["url"]).name).read_text() + + assert myuplink["read_only"] is True + assert myuplink["control_enabled"] is False + assert myuplink["metadata"]["auth_post_path"] == "/oauth/token" + assert "http.post" in myuplink["permissions"] + + # The exemption is scoped, not a hole: POST reaches the real host function + # only for a URL ending in the declared path. + assert 'local __sourceful_ftw_auth_path = "/oauth/token"' in artifact + assert "path:sub(-#__sourceful_ftw_auth_path) == __sourceful_ftw_auth_path" in artifact + assert "POST is allowed only for authentication" in artifact + # Everything else a read-only driver must not do is still refused. + for denied in ("modbus_write", "modbus_write_multi", "mqtt_publish", "serial_write"): + assert f"host.{denied} = __sourceful_ftw_write_denied" in artifact + + +def test_signing_in_is_declared_or_it_does_not_happen( + tmp_path: Path, keypair: tuple[str, str] +) -> None: + """A read-only driver that declares nothing keeps the blanket denial.""" + manifest, output = build(tmp_path, keypair) + exempt = [] + for driver in manifest["drivers"]: + if not driver["read_only"]: + continue + artifact = (output / Path(driver["url"]).name).read_text() + if "host.http_post = __sourceful_ftw_write_denied" not in artifact: + exempt.append(driver["id"]) + else: + assert "auth_post_path" not in driver["metadata"], driver["id"] + assert "http.post" not in driver["permissions"], driver["id"] + assert exempt == ["myuplink"], f"unexpected drivers allowed to POST: {exempt}" + + +def test_auth_post_path_must_be_a_path_and_must_mean_something( + tmp_path: Path +) -> None: + """Two ways to declare it wrongly, both refused while building.""" + repo, config_path = single_driver_repo(tmp_path, "myuplink") + source = repo / "drivers" / "lua" / "myuplink.lua" + original = source.read_text(encoding="utf-8") + + # A whole URL rather than a path: the guard matches on the path, so a URL + # here would silently never match and the driver could not sign in. + source.write_text( + original.replace('auth_post_path = "/oauth/token"', + 'auth_post_path = "https://api.myuplink.com/oauth/token"'), + encoding="utf-8") + with pytest.raises(RepositoryError, match="must be a path beginning with"): + _load_channel(config_path, repo) + + # Declared on a driver that is not read-only, where it exempts nothing. + source.write_text( + original.replace(" read_only = true,\n", ""), encoding="utf-8") + with pytest.raises(RepositoryError, match="only means anything with read_only"): + _load_channel(config_path, repo) + + source.write_text(original, encoding="utf-8") diff --git a/tools/ftw_repository.py b/tools/ftw_repository.py index 7c1d745..7452852 100644 --- a/tools/ftw_repository.py +++ b/tools/ftw_repository.py @@ -268,7 +268,8 @@ def _lua_string_list(values: list[str]) -> str: return "{ " + ", ".join(_lua_string(value) for value in values) + " }" -def _ftw_artifact(raw: bytes, metadata: dict[str, Any], read_only: bool) -> bytes: +def _ftw_artifact(raw: bytes, metadata: dict[str, Any], read_only: bool, + auth_post_path: str = "") -> bytes: """Add FTW metadata and the host-call polyfills older hosts lack. A driver the catalog marks control: true keeps its control functions. The @@ -280,6 +281,15 @@ def _ftw_artifact(raw: bytes, metadata: dict[str, Any], read_only: bool) -> byte A driver that declares read_only in its own DRIVER table still gets the write guards: those are meters and telemetry gateways saying what they are, not a policy imposed on them. + + `auth_post_path` is for a driver that can only read once it has signed in. + Over HTTP a POST is not evidence of actuation -- it is also how a driver + exchanges a refresh token -- so denying it outright would cost such a + driver every reading it takes. The exemption is scoped rather than trusted: + POST is permitted only to a URL ending in the declared path, and denied + everywhere else, so the flag enforces "this POST is authentication" instead + of merely asserting it. Matching the path rather than a whole URL survives + a site pointing the driver at its own base URL. """ protocols = metadata.get("protocols", []) capabilities = metadata.get("capabilities", []) @@ -313,11 +323,28 @@ def _ftw_artifact(raw: bytes, metadata: dict[str, Any], read_only: bool) -> byte # marks control: true keeps the control path it was ported with. write_guards = "" if read_only: + if auth_post_path: + # Sign in, then read. Anything else this driver tries to POST is + # refused exactly as if it had no exemption at all. + http_post_guard = ( + "local __sourceful_ftw_http_post = host.http_post\n" + f"local __sourceful_ftw_auth_path = {_lua_string(auth_post_path)}\n" + "host.http_post = function(url, ...)\n" + " local path = type(url) == \"string\" and url:match(\"^[^?]*\") or \"\"\n" + " if path:sub(-#__sourceful_ftw_auth_path) == __sourceful_ftw_auth_path then\n" + " return __sourceful_ftw_http_post(url, ...)\n" + " end\n" + " error(\"this driver declares itself read-only: " + "POST is allowed only for authentication\")\n" + "end\n" + ) + else: + http_post_guard = "host.http_post = __sourceful_ftw_write_denied\n" write_guards = ( "local function __sourceful_ftw_write_denied()\n" " error(\"this driver declares itself read-only\")\n" "end\n" - "host.http_post = __sourceful_ftw_write_denied\n" + + http_post_guard + "host.modbus_write = __sourceful_ftw_write_denied\n" "host.modbus_write_multi = __sourceful_ftw_write_denied\n" "host.modbus_write_multiple = __sourceful_ftw_write_denied\n" @@ -421,6 +448,17 @@ def _load_channel(config_path: Path, repo_root: Path) -> list[dict[str, Any]]: r"^\s*read_only\s*=\s*true", body, re.MULTILINE ) is not None + # A read-only driver that has to sign in before it can read anything. + # It names the path its token exchange goes to, and the generated guard + # holds it to exactly that -- see _ftw_artifact. + auth_post_path = _string_field(body, "auth_post_path") if body else "" + if auth_post_path and not auth_post_path.startswith("/"): + raise RepositoryError( + f"{driver_id}: auth_post_path must be a path beginning with '/'") + if auth_post_path and not declares_read_only: + raise RepositoryError( + f"{driver_id}: auth_post_path only means anything with read_only") + has_driver_command = "driver_command" in set(ENTRYPOINT_RE.findall(source)) required_entrypoints = {"driver_init", "driver_poll"} @@ -517,13 +555,20 @@ def _load_channel(config_path: Path, repo_root: Path) -> list[dict[str, Any]]: if value: metadata[output_name] = value - artifact = _ftw_artifact(raw, metadata, read_only=not controls) + if auth_post_path and not controls: + metadata["auth_post_path"] = auth_post_path + + artifact = _ftw_artifact(raw, metadata, read_only=not controls, + auth_post_path=auth_post_path if not controls else "") if len(artifact) > MAX_DRIVER_BYTES: raise RepositoryError(f"{driver_id}: generated FTW artifact is too large") permissions = list(PROTOCOL_PERMISSIONS[protocol]) if controls: permissions += PROTOCOL_WRITE_PERMISSIONS[protocol] + elif auth_post_path: + # Read-only, but it cannot read a thing until it has signed in. + permissions += PROTOCOL_WRITE_PERMISSIONS[protocol] entries.append( { @@ -622,7 +667,14 @@ def _validate_manifest(manifest: dict[str, Any]) -> None: for values in PROTOCOL_WRITE_PERMISSIONS.values() for permission in values } - if read_only and write_permissions.intersection(permissions): + # A read-only driver may hold exactly one write-capable permission, and + # only the one its declared sign-in needs: the artifact's guard confines + # that POST to auth_post_path, so the permission cannot reach further + # than the token exchange it was granted for. + allowed_write = set() + if metadata.get("auth_post_path"): + allowed_write = {"http.post"} + if read_only and write_permissions.intersection(permissions) - allowed_write: raise RepositoryError(f"{driver_id}: read-only driver has a write-capable permission") if driver.get("channel") not in {"beta", "stable"}: raise RepositoryError(f"{driver_id}: invalid channel")