diff --git a/README.md b/README.md index 6c9ffe0..550cd21 100644 --- a/README.md +++ b/README.md @@ -159,7 +159,7 @@ data: href: /mode/vs/0 ``` -returning `{"href", "actual_href", "code", "raw_code", "rep"}` off a **live GET straight from the device**, not the cache — which can be up to a poll interval stale, exactly the staleness that would make `held` above meaningless. Omit `href` and you get `{"resources": {href: rep, ...}}`, the cached snapshot of everything this integration currently tracks on that device, with no GET at all — useful for seeing what's there before you start writing to it, without hammering the appliance. +returning `{"href", "actual_href", "code", "raw_code", "rep"}` off a **live GET straight from the device**, not the cache — which can be up to a poll interval stale, exactly the staleness that would make `held` above meaningless. A sixth key, `body`, appears only when the response isn't a Property map: a Collection (`/device/0`, and the `x.com.samsung.devcol` siblings some boards expose) answers a CBOR list, which `rep` can't carry, and which would otherwise read as an accepted-but-empty resource. Omit `href` and you get `{"resources": {href: rep, ...}}`, the cached snapshot of everything this integration currently tracks on that device, with no GET at all — useful for seeing what's there before you start writing to it, without hammering the appliance. The **Debug write** panel under a device's Configure menu (Part 4) is the friendlier single-write path over this same machinery — pick an href, type a payload, see the result — for when you don't need a sequence. diff --git a/custom_components/localthings/coordinator.py b/custom_components/localthings/coordinator.py index ac3c3c6..d0a7ada 100644 --- a/custom_components/localthings/coordinator.py +++ b/custom_components/localthings/coordinator.py @@ -530,7 +530,7 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): single missed read is not worth surfacing. """ try: - code, rep = await self.async_raw_read(cloudcourse.COURSE_HREF) + code, rep, _body = await self.async_raw_read(cloudcourse.COURSE_HREF) except Exception: # One missed probe; the caller is a retry loop. self._log.debug("cloud-course probe failed", exc_info=True) @@ -1576,12 +1576,24 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): self._log.debug("raw write follow-up read failed: %s", e) return code, new_rep - def _raw_read_blocking(self, path_segs: list[str], href: str) -> tuple[int, dict]: + def _raw_read_blocking(self, path_segs: list[str], href: str) -> tuple[int, dict, Any]: """Debug primitive: a live GET, deliberately bypassing the cache (issue #300) -- the cache can be up to a poll interval stale, exactly the staleness that makes testing whether a write held or got silently reverted by the board unreliable. Blocking -- runs in - executor.""" + executor. + + Returns `(code, rep, body)`. `rep` is the decoded body only when it + is a Property map, since that's the shape the observe cache and + every capability are written against. `body` is whatever CBOR + actually decoded to, and exists because a Collection answers a + *list*, not a map: `/device/0` and its `x.com.samsung.devcol` + siblings return the `[devcol rep, {href, rep}, ...]` batch + `parse_device0_batch` reads. Reporting only `rep` rendered those as + an accepted-but-empty `2.05 {}`, which reads as "the resource is + there and has nothing in it" -- the opposite of what a full batch + means, and how issue #335's `/sec/devices` was nearly written off. + """ if self._session is None: self._connect_session() sess = self._session @@ -1589,6 +1601,7 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): raise RuntimeError("no session") code, payload = sess.get(path_segs, timeout=10.0) rep: dict = {} + body: Any = None if code == 0x45 and payload: try: body = cbor2.loads(payload) @@ -1598,11 +1611,13 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): if isinstance(body, dict): self._observe.apply(href, body, source="poll") rep = body - return code, rep + return code, rep, body - async def async_raw_read(self, href: str) -> tuple[int, dict]: + async def async_raw_read(self, href: str) -> tuple[int, dict, Any]: """Debug-only live GET (issue #300, backs the read_resource - service). Same href validation as async_raw_write.""" + service). Same href validation as async_raw_write. Three-tuple -- + see `_raw_read_blocking` for why the raw body comes back alongside + the Property-map `rep`.""" path_segs = _href_to_path_segs(href) if not path_segs: raise ServiceValidationError( @@ -1721,7 +1736,7 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): verified: dict[str, Any] = {} async with self._session_lock: for href in dict.fromkeys(r["href"] for r in results): - vcode, vrep = await self.hass.async_add_executor_job( + vcode, vrep, _vbody = await self.hass.async_add_executor_job( self._raw_read_blocking, _href_to_path_segs(href), href ) # None, not False, when the re-read brought back nothing diff --git a/custom_components/localthings/services.py b/custom_components/localthings/services.py index 8220c6e..d17fec9 100644 --- a/custom_components/localthings/services.py +++ b/custom_components/localthings/services.py @@ -179,7 +179,7 @@ async def _async_read_resource(hass: HomeAssistant, call: ServiceCall) -> Servic # Same normalize-before-translate order as the write path above. canonical = normalize_href(href) actual_href = subdevice.to_actual(canonical) - code, rep = await coordinator.async_raw_read(actual_href) + code, rep, body = await coordinator.async_raw_read(actual_href) read_result: dict[str, Any] = { "href": canonical, "actual_href": actual_href, @@ -187,6 +187,13 @@ async def _async_read_resource(hass: HomeAssistant, call: ServiceCall) -> Servic "raw_code": code, "rep": rep, } + # `body` only when it isn't the Property map already in `rep` -- a + # Collection (`/device/0`, `/sec/devices`) answers a CBOR list, which + # `rep` can't carry and which used to vanish into an empty-looking + # 2.05 (issue #335). Omitted for the ordinary map case rather than + # duplicating every rep in every response. + if body is not None and not isinstance(body, dict): + read_result["body"] = body return cast(ServiceResponse, read_result) diff --git a/docs/investigations/composite-subdevice-hrefs.md b/docs/investigations/composite-subdevice-hrefs.md index fe2202e..dcce1c3 100644 --- a/docs/investigations/composite-subdevice-hrefs.md +++ b/docs/investigations/composite-subdevice-hrefs.md @@ -88,42 +88,69 @@ collection of devices, sitting alongside `/device/0`, never read by this project or by any issue thread. If the composite enumeration is exposed anywhere as a first-class resource, that is the shape it would take. -## Hrefs worth reading next, in priority order +## Results of the second probe round -All plain RETRIEVEs via `localthings.read_resource`; a wrong guess costs one -round trip. +The reporter ran these live. Three answers, all informative. -**1 — indexed leaves (the untried namespace).** `/information/vs/1` first: -on the TP2X 2-in-1 the sibling's `/information/vs/0` is what identified the -wall unit by `modelNum`, so a populated `/information/vs/1` both proves the -namespace and names the unit. +**Indexed leaves do not exist.** `/information/vs/1`, `/power/vs/1`, +`/mode/vs/1` → 4.04. Pattern A is ruled out on this board properly now: +not just the `/device/1` Collection, but the leaf namespace it would have +carried. - /information/vs/1 - /power/vs/1 - /mode/vs/1 - /temperatures/vs/1 - /temperature/current/1 - /airflow/vs/1 - /sensors/vs/1 - /power/1 +**The UUID prefix routes, and is empty of operational resources.** The +control pair settles it: -**2 — the device Collection.** `/sec/devices`, per the clause above. + /c24e25e9-.../file/list/vs/0 → 2.05, two items + /file/list/vs/0 → 2.05, the same two items + (/opt/data/energy.db, /opt/data/hass.db) -**3 — a positive control for the UUID namespace.** `/oic/res` advertises -three UUID-prefixed file resources on this board, so at least one path under -that prefix is supposed to answer: +So the sibling's prefix is a live, routed namespace — the 23 flat-fallback +4.04s under it are the firmware answering "no such resource", not a dead +prefix swallowing everything. Pattern B/C is ruled out on this board on +positive evidence rather than on absence. That the two listings are +identical is expected either way: one board, one flash, one filesystem. - /c24e25e9-55dd-ba18-d567-000000000001/file/list/vs/0 - /file/list/vs/0 +**`/sec/devices` exists — and this project could not see what's in it.** +It answered `2.05` with `rep: {}`, which reads as "the resource is there and +has nothing in it". It is not. `coordinator._raw_read_blocking` decoded the +CBOR body and then kept it *only if it was a Property map*: -The master's own href is the baseline. If the prefixed one answers, the -prefix routes and the sibling's operational resources are genuinely not -mounted there — stop probing that namespace on this family. If it 4.04s -while the master's answers, the prefix is advertised but unrouted, which -says the `/oic/res` advertisement is scaffolding and is worth knowing before -trusting it for Pattern C elsewhere. +```python +if isinstance(body, dict): + rep = body +``` -**4 — only if index 1 shows anything:** repeat tier 1 at index 2. +A Collection answers a **list** — the `[devcol rep, {href, rep}, ...]` batch +`parse_device0_batch` reads. `/device/0` itself would have rendered exactly +the same accepted-but-empty `2.05 {}` through `read_resource`. Fixed: the +read path now returns the decoded body alongside `rep`, and the service +response carries it as `body` whenever it isn't the map already in `rep`. + +`/sec/devices` therefore remains the one open lead, and needs one re-read on +a build carrying that fix. + +## Still worth reading + +**1 — `/sec/devices`, again.** Same `x.com.samsung.devcol` + `oic.wk.col` +pair as `/device/0`, so its body should be a batch naming its members. If a +composite enumeration is exposed anywhere, it is here. + +**2 — the file-transfer pair.** `/oic/res` advertises +`/c24e25e9-.../file/transfer/vs/0` alongside the master's, and the prefix is +now known to route. Issue #301 documents the shape: a baseline GET returns +one item, `x.com.samsung.name` plus `x.com.samsung.blob`, no write needed to +see whatever it currently serves. If the prefixed endpoint serves *different +bytes* than the master's, that is the first hard local evidence the wall +unit exists as a data producer, and `/opt/data/energy.db` would be where its +runtime history lives. + + /file/transfer/vs/0 + /c24e25e9-55dd-ba18-d567-000000000001/file/transfer/vs/0 + +Mind the blob: #301 measured 2172 B on a `KRAC_18K`, and a raw `bytes` value +in a service response is not guaranteed to survive rendering in Developer +Tools. Ask for `x.com.samsung.name` and whether a blob field appears, not +for the blob pasted into a comment. ## Dead ends, so they aren't re-tried @@ -138,10 +165,20 @@ trusting it for Pattern C elsewhere. - `/actions/vs/0` — GET returns `{}` on baseline and `oic.if.a`; publishes no schema (`ac-filter-reset.md`). -## If tier 1 comes back empty +## Where this lands if `/sec/devices` is empty too -A full 4.04 sweep is a real result, not a failed one: it would mean the -sibling is named in `subdeviceIdList` for the cloud's benefit and has no -local resource surface at all on this firmware. That closes issue #335 as a -firmware limitation rather than leaving it open against a probe strategy -that was never actually exercised. +Then the sibling is named in `subdeviceIdList` for the cloud's benefit and +has no local operational surface at all on this firmware — every namespace +it could occupy has now been read directly, and the UUID one was confirmed +routable first, so the negatives mean what they say. That closes issue #335 +as a firmware limitation rather than leaving it open against a probe +strategy that was never actually exercised. + +Worth keeping in view for the enumeration code either way: both remaining +patterns hinge on a Collection, and this board answers neither `/device/1` +nor a prefixed `/device/0`. An indexed flat-probe fallback — the mirror of +issue #205's prefixed one, gated on a board that claims a sibling but +materialized nothing — would have cost 8 round trips here and returned the +same 4.04s the reporter got by hand. It is worth building only if some +other board turns out to serve indexed leaves without their Collection; +this one does not. diff --git a/tests/test_services.py b/tests/test_services.py index e8f05e4..24aa7ee 100644 --- a/tests/test_services.py +++ b/tests/test_services.py @@ -56,11 +56,16 @@ class _FakeSession: self._post_code = post_code self._get_reps: dict[str, list[dict]] = {} - def queue_get(self, href: str, rep: dict) -> None: + def queue_get(self, href: str, rep: dict | list) -> None: """Queue one more canned rep for `href`'s next GET. Once an href's queue is down to one entry, that entry keeps answering every further GET -- a test only needs to queue the values that - actually change across calls.""" + actually change across calls. + + A list models a Collection's answer (the `[devcol rep, {href, rep}, + ...]` batch), which is not a Property map and so is a shape the + read path has to carry separately -- see the collection test below. + """ self._get_reps.setdefault(href.strip("/"), []).append(rep) def post(self, path_segs, payload, timeout=None): @@ -576,6 +581,28 @@ async def test_read_resource_with_href_does_live_get(hass, coordinator, device_i assert response["href"] == "/mode/vs/0" assert response["actual_href"] == "/mode/vs/0" assert response["rep"] == {"x.field": "live"} + # No duplicate copy of a Property map that `rep` already carries. + assert "body" not in response + + +async def test_read_resource_surfaces_a_collections_list_body(hass, coordinator, device_id): + """A Collection answers a CBOR list, not a Property map, so `rep` can't + hold it (issue #335: `/sec/devices` came back as an accepted-but-empty + 2.05, which reads as "exists, nothing in it" -- the opposite of what a + populated batch means).""" + batch = [ + {"rt": ["x.com.samsung.devcol", "oic.wk.col"]}, + {"href": "/mode/vs/0", "rep": {"x.field": "live"}}, + ] + fake = _FakeSession() + fake.queue_get("sec/devices", batch) + coordinator._session = fake + + response = await _call_read(hass, device_id, href="/sec/devices") + + assert response["code"] == "2.05" + assert response["rep"] == {} + assert response["body"] == batch async def test_read_resource_without_href_returns_cached_snapshot_and_does_not_get(