read_resource: a Collection's list body is not an empty resource (#335)
`_raw_read_blocking` decoded the CBOR body and kept it only when it was a
Property map, so a Collection -- which answers the `[devcol rep, {href,
rep}, ...]` batch `parse_device0_batch` reads -- came back as `2.05` with
`rep: {}`. That renders as "the resource exists and has nothing in it",
which is the opposite of what a populated batch means, and `/device/0`
itself would have read the same way.
It cost a real result: issue #335's board answers `/sec/devices` (the
`x.com.samsung.devcol` sibling of `/device/0`, and the one remaining place
a composite appliance could be enumerating its indoor units) with exactly
that empty-looking 2.05, and it was nearly written off as a dead end.
The read path now returns the decoded body alongside `rep`, and the service
response carries it as `body` whenever it isn't the map `rep` already has --
omitted for the ordinary case rather than duplicating every rep in every
response. Records the probe round this came out of: indexed leaves 4.04 on
that board, and the UUID prefix confirmed routable by a positive control, so
Patterns A/B/C are ruled out there on evidence rather than on absence.
This commit is contained in:
@@ -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.
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
+29
-2
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user