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.
This commit is contained in:
@@ -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
|
||||
```
|
||||
|
||||
|
||||
@@ -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] = {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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: >-
|
||||
|
||||
Reference in New Issue
Block a user