From fffe923afcbde3c1a2977b02f376fdc13986c6d1 Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Fri, 7 Aug 2026 22:58:48 +0000 Subject: [PATCH] Take the device as a field, not a service target Hassfest rejects a filtered device target outright ("Services do not support device filters on target, use a device selector instead"), and an unfiltered one would offer every device in the installation. Both services now take device_id as a required field with a device selector scoped to this integration -- the shape fully_kiosk, guardian and unifi already use. No schema change needed: cv.TARGET_SERVICE_FIELDS already accepts device_id, so the options-flow panel's target= call keeps working. Also trims the comments added with the review fixes back to the one or two sentences CONTRIBUTING asks for. --- README.md | 8 ++-- custom_components/localthings/coordinator.py | 45 ++++++-------------- custom_components/localthings/services.py | 15 +++---- custom_components/localthings/services.yaml | 20 ++++++--- 4 files changed, 35 insertions(+), 53 deletions(-) diff --git a/README.md b/README.md index 28aaaf7..6d69cdc 100644 --- a/README.md +++ b/README.md @@ -102,15 +102,14 @@ Each device has its own **Configure** option in Settings > Devices & Services, u Two HA actions, `localthings.write_resource` and `localthings.read_resource`, talk to a device's OCF resources directly instead of through this integration's entity model. They exist for two overlapping jobs: pinning down a device-specific write contract (the reverse-engineering work `docs/investigations/` and the provenance comments throughout `registry/capabilities/` are all about), and driving a resource this integration doesn't model as an entity yet, without waiting on a release. -Both target a device (`target: device:`, filtered to `integration: localthings` in the picker) and resolve to exactly one appliance — targeting an area or label that expands to more than one LocalThings device is rejected rather than silently fanned out across all of them. `href` is always canonical (e.g. `/mode/vs/0`); if the device you targeted is a subdevice — an oven's second cavity, an AC's second indoor unit — it's translated to the real on-the-wire href for you (`/mode/vs/1`, say), and the response reports both forms so there's no ambiguity about what was actually sent. +Both take a `device_id` (a device picker filtered to this integration) and resolve to exactly one appliance — a target that expands to more than one LocalThings device is rejected rather than silently fanned out across all of them. `href` is always canonical (e.g. `/mode/vs/0`); if the device you targeted is a subdevice — an oven's second cavity, an AC's second indoor unit — it's translated to the real on-the-wire href for you (`/mode/vs/1`, say), and the response reports both forms so there's no ambiguity about what was actually sent. `write_resource` exists because a single write, one at a time, isn't enough to probe some boards. Issue #300's Samsung wall oven answers `2.04 Changed` to a settings write while idle and then silently reverts it — the write only sticks once a cycle is already running. Finding what actually triggers a cycle needs an *ordered sequence* of writes to different resources, with real delays between them, and a way to check afterward whether anything actually held: ```yaml action: localthings.write_resource -target: - device_id: abc123... data: + device_id: abc123... writes: - href: /mode/vs/0 payload: @@ -150,9 +149,8 @@ If the session drops partway through a sequence, the action raises rather than r ```yaml action: localthings.read_resource -target: - device_id: abc123... data: + device_id: abc123... href: /mode/vs/0 ``` diff --git a/custom_components/localthings/coordinator.py b/custom_components/localthings/coordinator.py index 5c3b502..37f6136 100644 --- a/custom_components/localthings/coordinator.py +++ b/custom_components/localthings/coordinator.py @@ -115,18 +115,10 @@ def _href_to_path_segs(href: str) -> list[str]: def normalize_href(href: str) -> str: - """A user-typed href in the one canonical spelling the rest of the debug - path assumes: exactly one leading slash, no trailing slash, no empty - segments. - - Public because services.py has to normalize *before* handing an href to - `Subdevice.to_actual` (issue #300). That transform is purely textual -- - an indexed subdevice rewrites only a trailing '0' segment -- so - '/mode/vs/0/' slips through it unchanged and then normalizes here to - '/mode/vs/0', silently landing the write on the master's resource - instead of the subdevice's while still reporting 2.04. Normalizing on - the way in makes the two agree. - """ + """A user-typed href in one canonical spelling. Public because + services.py must normalize before `Subdevice.to_actual`, which rewrites + only a trailing '0' segment: '/mode/vs/0/' slips through it unchanged + and would land on the master's resource, not the subdevice's.""" return "/" + "/".join(_href_to_path_segs(href)) @@ -1315,23 +1307,15 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): "changed": all(after.get(k) == v for k, v in payload.items()), } ) - # settle is "how long to wait before the next write" -- the - # last item has no next write, so it gets no wait here; - # verify_after (below) is the equivalent wait after the - # sequence as a whole. - # - # Held across this wait, unlike verify_after's below: a - # poll landing between two writes is exactly what this - # sequence exists to rule out, since it blurs which write - # the appliance was reacting to. Bounded by the caps above - # -- 10 writes x 30s is the worst a caller can ask for. + # The lock is deliberately held across this wait, unlike + # verify_after's below: a poll landing between two writes + # blurs which one the appliance reacted to, which is what + # the sequence exists to rule out. The caps bound it. if settle and i < len(parsed) - 1: await asyncio.sleep(settle) except Exception as err: - # A session drop partway leaves the appliance holding whatever - # already landed. Raising bare would throw that away, and knowing - # which writes got through is the difference between a usable - # probe result and having to start the sequence over blind. + # A drop partway leaves the appliance holding whatever already + # landed, so the error has to say which writes got through. done = ", ".join(r["href"] for r in results) or "none" self._log.warning( "raw write sequence failed after %d of %d writes (completed: %s): %s", @@ -1360,12 +1344,9 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): vcode, vrep = await self.hass.async_add_executor_job( self._raw_read_blocking, _href_to_path_segs(href), href ) - # `held` is None, not False, when the re-read itself - # didn't come back with a representation to compare. - # _raw_read_blocking answers a non-2.05 with an empty - # rep, against which every payload comparison is False -- - # so a 4.04 or a dropped read would otherwise be reported - # as "the board reverted your write", which is the exact + # None, not False, when the re-read brought back nothing + # to compare: every comparison against an empty rep is + # False, which would report a 4.04 as a revert -- the one # distinction verify_after exists to draw. read_ok = _coap_accepted(vcode) and bool(vrep) verified[href] = { diff --git a/custom_components/localthings/services.py b/custom_components/localthings/services.py index 82f260d..73d4c55 100644 --- a/custom_components/localthings/services.py +++ b/custom_components/localthings/services.py @@ -116,14 +116,9 @@ async def _async_write_resource(hass: HomeAssistant, call: ServiceCall) -> Servi writes_in: list[dict[str, Any]] = call.data[ATTR_WRITES] # Canonical -> actual translation happens here, not in the coordinator - # (issue #177): the coordinator's raw-write primitive has no notion of - # subdevices, only the hrefs it's told to hit -- identity transform for - # MAIN, so a device targeting MAIN behaves exactly as it always has. - # - # Normalized first: to_actual is a textual transform, so an un-normalized - # '/mode/vs/0/' passes through it untouched and only gets tidied up - # downstream, quietly hitting the master's resource instead of the - # subdevice's (see coordinator.normalize_href). + # (issue #177), whose raw-write primitive has no notion of subdevices -- + # identity transform for MAIN. Normalized first, or a trailing slash + # slips past to_actual onto the master (see coordinator.normalize_href). canonicals = [normalize_href(w[ATTR_HREF]) for w in writes_in] raw_writes = [ { @@ -153,8 +148,8 @@ async def _async_write_resource(hass: HomeAssistant, call: ServiceCall) -> Servi response: dict[str, Any] = {"device_id": device_id, "results": results} if "verified" in sequence: # Keyed off the same normalized canonicals the sequence was built - # from, so the lookup can't miss and hand back an actual href where - # the documented contract promises a canonical one. + # from, so the lookup can't miss and return an actual href where the + # contract promises a canonical one. canonical_by_actual = {subdevice.to_actual(c): c for c in canonicals} response["verified"] = { canonical_by_actual.get(actual_href, actual_href): verified diff --git a/custom_components/localthings/services.yaml b/custom_components/localthings/services.yaml index 4afcbdb..b08ef21 100644 --- a/custom_components/localthings/services.yaml +++ b/custom_components/localthings/services.yaml @@ -10,10 +10,14 @@ write_resource: it can misconfigure your appliance. Prefer a real entity, or the Debug write panel in the integration's Configure menu, for anything this integration already models. - target: - device: - integration: localthings fields: + device_id: + name: Device + description: The appliance to write to, or one of its subdevices. + required: true + selector: + device: + integration: localthings writes: name: Writes description: >- @@ -52,10 +56,14 @@ read_resource: deliberately not the cache, which can be up to a poll interval stale -- or omit it to get the cached snapshot of every resource this integration currently tracks on that device. - target: - device: - integration: localthings fields: + device_id: + name: Device + description: The appliance to read from, or one of its subdevices. + required: true + selector: + device: + integration: localthings href: name: Resource href description: >-