From 10d5c81d9a976157de1d8331c3a075b2b717aa47 Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Wed, 29 Jul 2026 04:08:19 +0000 Subject: [PATCH] feat(diagnostics): speculatively probe /device/1 and /device/2 /oic/res's baseline-Interface response only lists resources with the discoverable policy bit set, and a real dump (issue #177 follow-up, TP1X_REF_21K) confirms /device/0's whole x.com.samsung.da.* tree is registered without it -- so a second logical Device's Collection, if one exists, would be just as invisible to /oic/res as /device/0 is. Probe /device/1 and /device/2 directly instead: a plain non-mutating RETRIEVE, tolerated-404 same as every other speculative read in this module. Parsed with the same parse_device0_batch used for /device/0 itself, and folded into identity.raw so diagnostics can tell "checked, found nothing" apart from "never checked". --- .../localthings/registry/identity.py | 41 +++++++++++- tests/localthings/test_diagnostics.py | 36 +++++++++++ tests/test_identity.py | 63 +++++++++++++++---- 3 files changed, 128 insertions(+), 12 deletions(-) diff --git a/custom_components/localthings/registry/identity.py b/custom_components/localthings/registry/identity.py index 81d6d5a..b602be5 100644 --- a/custom_components/localthings/registry/identity.py +++ b/custom_components/localthings/registry/identity.py @@ -6,6 +6,20 @@ from typing import Optional import cbor2 +from .batch import parse_device0_batch + +# Speculative /device/ siblings to probe alongside the coordinator's own +# /device/0 seed poll (issue #177). Confirmed against a real dump: /oic/res's +# baseline-Interface response only lists resources with the discoverable +# policy bit set, and /device/0's whole x.com.samsung.da.* tree is registered +# without it -- so a second logical Device's Collection, if one exists, would +# be just as invisible to /oic/res as /device/0 is. A direct GET is the only +# way left to check, and it's a plain RETRIEVE (non-mutating, tolerated-404 +# already the norm throughout this module) -- not the kind of guess the +# write-contract 'don't guess' rule is about. Bounded to a couple of indices; +# widen only if a real Composite Device ever turns out to need more. +_SPECULATIVE_DEVICE_INDICES = (1, 2) + @dataclass(frozen=True) class DeviceIdentity: @@ -42,6 +56,24 @@ def _get_links(sess, path) -> list: return [] +def _get_device_batch(sess, index: int) -> dict[str, dict]: + """GET /device/ and parse it the same way the coordinator parses + /device/0 -- a Samsung Collection RETRIEVE returns + [devcol-rep, {href, rep}, {href, rep}, ...], not a bare Property map or + Link array. Missing/malformed responses fall through to {} (via + parse_device0_batch on an empty/non-list body), same tolerated-absence + posture as _get/_get_links above.""" + try: + code, pl = sess.get(['device', str(index)], timeout=10.0) + if code == 0x45 and pl: + body = cbor2.loads(pl) + if isinstance(body, list): + return parse_device0_batch(body) + except Exception: + pass + return {} + + def _device_types(d: dict) -> tuple[str, ...]: """/oic/d's `rt` -- the device's own OCF device-type declaration. @@ -76,6 +108,10 @@ def read_identity(sess, serial: Optional[str]) -> DeviceIdentity: # multi-unit device shows us whether its firmware actually implements # that model before any code assumes it does. res = _get_links(sess, ['oic', 'res']) + extra_devices = { + f'/device/{n}': _get_device_batch(sess, n) + for n in _SPECULATIVE_DEVICE_INDICES + } return DeviceIdentity( manufacturer=p.get('mnmn') or 'Samsung', model=p.get('mnmo') or '', @@ -85,5 +121,8 @@ def read_identity(sess, serial: Optional[str]) -> DeviceIdentity: # Kept whole rather than field-by-field: these resources are outside # the /device/0 dump diagnostics already captures, and we don't yet # know which of their fields will turn out to identify a device type. - raw={'/oic/p': p, '/oic/d': d, '/oic/res': res}, + # /device/1 and /device/2 are always present here (empty {} when the + # device didn't answer) so a diagnostics reader can tell "checked, + # nothing there" apart from "never checked". + raw={'/oic/p': p, '/oic/d': d, '/oic/res': res, **extra_devices}, ) diff --git a/tests/localthings/test_diagnostics.py b/tests/localthings/test_diagnostics.py index f27c739..0a522f6 100644 --- a/tests/localthings/test_diagnostics.py +++ b/tests/localthings/test_diagnostics.py @@ -122,6 +122,42 @@ async def test_diagnostics_include_oic_res_links( assert links[1]['rt'] == ['x.com.samsung.devcol', 'oic.wk.col'] +async def test_diagnostics_include_speculative_device_probes( + hass: HomeAssistant, mock_entry, mock_coordinator_session +) -> None: + """/device/1 and /device/2 (issue #177's Composite Device follow-up) ride + along in identity.raw the same {href: rep} shape as the main /device/0 + dump under "resources" -- ordinary redaction, no special-casing needed.""" + from custom_components.localthings.registry.identity import DeviceIdentity + + await hass.config_entries.async_setup(mock_entry.entry_id) + await hass.async_block_till_done() + + coordinator = hass.data[DOMAIN][mock_entry.entry_id] + coordinator._identity = DeviceIdentity( + manufacturer='Samsung Electronics', + model='RF9000B', + name='Family Hub', + serial=None, + device_types=('oic.wk.d', 'oic.d.refrigerator'), + raw={ + '/oic/p': {}, '/oic/d': {}, '/oic/res': [], + '/device/1': { + '/information/vs/0': {'x.com.samsung.da.serialNum': 'SECRET123'}, + '/power/vs/0': {'x.com.samsung.da.power': 'On'}, + }, + '/device/2': {}, + }, + ) + + diag = await async_get_config_entry_diagnostics(hass, mock_entry) + + device1 = diag['identity']['resources']['/device/1'] + assert device1['/information/vs/0']['x.com.samsung.da.serialNum'] == REDACTED + assert device1['/power/vs/0']['x.com.samsung.da.power'] == 'On' + assert diag['identity']['resources']['/device/2'] == {} + + async def test_diagnostics_identity_none_when_unavailable( hass: HomeAssistant, mock_entry, mock_coordinator_session ) -> None: diff --git a/tests/test_identity.py b/tests/test_identity.py index de922fc..d63b6ef 100644 --- a/tests/test_identity.py +++ b/tests/test_identity.py @@ -32,7 +32,10 @@ def test_read_identity_tolerates_missing_resources(): assert ident.model == '' assert ident.serial is None assert ident.device_types == () - assert ident.raw == {'/oic/p': {}, '/oic/d': {}, '/oic/res': []} + assert ident.raw == { + '/oic/p': {}, '/oic/d': {}, '/oic/res': [], + '/device/1': {}, '/device/2': {}, + } def test_read_identity_captures_oic_d_device_types(): @@ -72,22 +75,31 @@ def test_read_identity_keeps_raw_payloads_for_diagnostics(): def test_read_identity_captures_oic_res_links(): - """/oic/res is OCF's discovery endpoint: a baseline RETRIEVE returns an - array of Link objects, one per Resource/Collection the endpoint hosts -- - including, per the OCF 'Composite Device' model, a second logical - Device's Collection on a multi-unit system (issue #177). Nothing routes - on this yet; captured so a report from such a device shows us whether - its firmware actually exposes more than the one /device/0 seed href.""" + """/oic/res is OCF's discovery endpoint. Real hardware (issue #177 + follow-up, a TP1X_REF_21K fridge dump) groups the response by `di`: one + entry per logical Device, each carrying its own `links` array -- not a + flat array of individually-`di`-tagged links. Every entry that dump + returned had its discoverable policy bit set (`bm`'s bit 0); the whole + x.com.samsung.da.* tree (including /device/0 itself) did not appear at + all, meaning it's registered non-discoverable and simply invisible to + this endpoint -- see _SPECULATIVE_DEVICE_INDICES' docstring for why that + motivated probing /device/1 and /device/2 directly instead.""" sess = FakeSession({ ('oic', 'res'): [ - {'di': 'aaaa', 'href': '/device/0', 'rt': ['x.com.samsung.devcol', 'oic.wk.col']}, - {'di': 'bbbb', 'href': '/device/1', 'rt': ['x.com.samsung.devcol', 'oic.wk.col']}, + {'di': 'aaaa', 'links': [ + {'href': '/oic/d', 'rt': ['oic.wk.d', 'oic.d.refrigerator'], + 'p': {'bm': 1}}, + {'href': '/oic/sec/doxm', 'rt': ['oic.r.doxm'], 'p': {'bm': 1}}, + ]}, ], }) ident = read_identity(sess, serial=None) assert ident.raw['/oic/res'] == [ - {'di': 'aaaa', 'href': '/device/0', 'rt': ['x.com.samsung.devcol', 'oic.wk.col']}, - {'di': 'bbbb', 'href': '/device/1', 'rt': ['x.com.samsung.devcol', 'oic.wk.col']}, + {'di': 'aaaa', 'links': [ + {'href': '/oic/d', 'rt': ['oic.wk.d', 'oic.d.refrigerator'], + 'p': {'bm': 1}}, + {'href': '/oic/sec/doxm', 'rt': ['oic.r.doxm'], 'p': {'bm': 1}}, + ]}, ] @@ -99,3 +111,32 @@ def test_read_identity_tolerates_malformed_oic_res(): FakeSession({('oic', 'res'): {'not': 'a list'}}), serial=None ) assert ident.raw['/oic/res'] == [] + + +def test_read_identity_probes_device_1_and_2(): + """/oic/res won't reveal a second logical Device's Collection on this + firmware family (see test_read_identity_captures_oic_res_links), so + /device/1 and /device/2 are probed directly -- same [devcol-rep, + {href, rep}, ...] batch shape /device/0 itself returns, parsed the same + way (parse_device0_batch).""" + sess = FakeSession({ + ('device', '1'): [ + {'rt': ['x.com.samsung.devcol', 'oic.wk.col']}, + {'href': '/power/vs/0', 'rep': {'x.com.samsung.da.power': 'On'}}, + ], + }) + ident = read_identity(sess, serial=None) + assert ident.raw['/device/1'] == { + '/power/vs/0': {'x.com.samsung.da.power': 'On'}, + } + # /device/2 was never in the table -> 4.04 -> tolerated absence. + assert ident.raw['/device/2'] == {} + + +def test_read_identity_tolerates_malformed_device_probe(): + """A bare Property map instead of a batch array (or anything else + non-list-shaped) must not explode.""" + ident = read_identity( + FakeSession({('device', '1'): {'not': 'a batch'}}), serial=None + ) + assert ident.raw['/device/1'] == {}