diff --git a/.claude/skills/adding-device-support/SKILL.md b/.claude/skills/adding-device-support/SKILL.md index df717c2..a627311 100644 --- a/.claude/skills/adding-device-support/SKILL.md +++ b/.claude/skills/adding-device-support/SKILL.md @@ -218,10 +218,14 @@ This is a rule about **writes and entities**, not about reading. A speculative `GET` of an href a dump doesn't contain is fine and the codebase already relies on it: `read_identity` reads `/oic/p`, `/oic/d` and `/oic/res`, and `subdevices.enumerate_subdevices` probes `/device/`, `//device/0` and -`/multidevice/vs/0` on every device. A RETRIEVE is non-mutating and a 4.04 is -tolerated everywhere in that path, so the cost of a wrong guess is one wasted -round trip. Guessing a *write* against live hardware is the thing this rule -forbids — as is materializing an entity from a field you can't explain. +`/multidevice/vs/0` on every device — and, when a prefixed candidate's own +`//device/0` doesn't answer (issue #205: not guaranteed even on the +board this pattern was built against), every href the master itself +answered this cycle, individually under that UUID's prefix (see §11). A +RETRIEVE is non-mutating and a 4.04 is tolerated everywhere in that path, so +the cost of a wrong guess is one wasted round trip. Guessing a *write* +against live hardware is the thing this rule forbids — as is materializing +an entity from a field you can't explain. ## 6. Select options: read them from the device, don't hardcode @@ -364,6 +368,19 @@ The new fixture is picked up automatically by the corpus-wide checks (the board token fails the build rather than silently mistyping someone's appliance. +**Don't put a reporter's name or GitHub username in code.** Fixture data +gets serials/MACs/other device PII scrubbed per point 1 above — the same +rule applies to the *prose* you write while fixing the issue: comments, +docstrings, `seeds_note`, and test/function names should say "the +reporter," "issue #NNN's reporter," or (when a module already distinguishes +multiple reporters, like `subdevices.py`'s Pattern A/Pattern B) "the +Pattern A reporter," never a real name or handle. That prose ships in the +package and lives in git history indefinitely — unlike an issue thread or a +release-notes thank-you (both fine places to credit someone by name), it's +not somewhere a person would expect to stay named forever. If you're fixing +an issue and about to write `'s board`/`'s dump` in a +comment, stop and swap in a generic reference instead. + ## 11. Triage: "one of my subdevices is missing" For an appliance that exposes several logical indoor subdevices over one IP — @@ -372,22 +389,41 @@ the dump in this order; each step rules out a different cause. 1. **`subdevice_probes`** — did we even look? Every seed attempted appears here with what it returned. An absent seed means enumeration never tried - that path; a `false` means it tried and got nothing. -2. **`subdevices_skipped`** — did we find it and reject it? A candidate lands - here when its seed answered but it produced no *primary* (non-diagnostic) - entity with a populated value. Its `resources` block holds the exact reps - the gate judged, so you can check the call yourself. If every - power/mode/temperature rep is `{}`, the subdevice is an unused slot and the - skip is correct. If they're populated, the gate is wrong — that's a bug - worth a fixture. -3. **`multidevice.numofsubdevice`** — the board's own count, where it - reports one. Disagreement with `len(subdevices) + 1` is a strong hint, - not proof; only one board family is known to expose it. -4. **Which pattern is this board?** `identity.resources['/oic/res']` listing + that path; a `false` means it tried and got nothing. On a UUID-prefixed + board whose `//device/0` reads `false` (issue #205 — this isn't + rare, not even on the board the pattern was built against), the report + also carries one probe per href the master itself answered that cycle, + individually under that prefix (`subdevices.enumerate_subdevices`'s flat + fallback) — a `true` there is real, confirmed-live evidence for that one + href, not a guess. +2. **`subdevices`**/**`flat_hrefs`** — for a *materialized* subdevice found + this way, `flat_hrefs` lists exactly which hrefs it's actually being + polled on (individually, no Collection endpoint to batch through) — + compare against the master's own hrefs to see what's still unconfirmed + for that sibling. +3. **`subdevices_skipped`** — did we find it and reject it? A candidate lands + here when its seed(s) answered but it produced no *primary* + (non-diagnostic) entity with a populated value. Its `resources` block + holds the exact reps the gate judged, so you can check the call + yourself. If every power/mode/temperature rep is `{}`, the subdevice is + an unused slot and the skip is correct. If they're populated, the gate + is wrong — that's a bug worth a fixture. A flat-fallback candidate whose + only confirmed href is `/information/vs/0` (never bound to any entity — + only ever read for device-type resolution) will *always* land here until + more of its hrefs are confirmed live; that's the gate working as + intended, not a bug to chase. +4. **`multidevice.numofsubdevice`** — the board's own count, where it + reports one. `coordinator._run_discovery` compares it against + `len(materialized) + 1` (materialized subdevices plus the master) and + only warns on disagreement — `subdevices_skipped` entries don't count + toward either side, since they never materialized. A strong hint, not + proof; only one board family is known to expose it. +5. **Which pattern is this board?** `identity.resources['/oic/res']` listing `/device/1`, `/device/2` means indexed siblings. `resources['/subdevices/ vs/0']` carrying a `subdeviceIdList` means a UUID-prefixed tree, and that - same UUID usually shows up as an href prefix in `/oic/res` too. Neither - present, on a device the owner insists has two subdevices, is the + same UUID usually shows up as an href prefix in `/oic/res` too — enumerate + whether or not `//device/0` itself answers, per §5's fallback. + Neither present, on a device the owner insists has two subdevices, is the interesting case — that's a third mechanism and needs a new dump, not a code guess. diff --git a/custom_components/localthings/coordinator.py b/custom_components/localthings/coordinator.py index d2c0899..ab5a174 100644 --- a/custom_components/localthings/coordinator.py +++ b/custom_components/localthings/coordinator.py @@ -174,9 +174,10 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): # it's the *other* subdevices sharing this DTLS session, if any. self.subdevices: list[Subdevice] = [] # Candidates _run_discovery's gate rejected (an unused SmartThings - # slot that still answers its seed, e.g. HJcom's /device/2) -- - # surfaced in diagnostics alongside the materialized ones so a - # report shows what was found and why it didn't become an entity. + # slot that still answers its seed, e.g. the issue #177 reporter's + # /device/2) -- surfaced in diagnostics alongside the materialized + # ones so a report shows what was found and why it didn't become an + # entity. self._skipped_subdevices: list = [] # Those rejected candidates' raw reps, kept aside for diagnostics # only (see _live_subdevice_resources). They are deliberately not in the @@ -285,8 +286,8 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): # identity resource -- fall back to a generic per-subdevice label # rather than leaving the device unnamed. 'Subdevice ' only # makes sense for an indexed subdevice (the key is a small - # ordinal); jhkwon19-pattern (prefixed) subdevices are never more - # than one per connection today, so there's no ordinal to show. + # ordinal); UUID-prefixed subdevices are never more than one per + # connection today, so there's no ordinal to show. label = ( f'Subdevice {subdevice.key}' if subdevice.kind == 'indexed' else 'Secondary Subdevice' @@ -408,12 +409,15 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): """GET one subdevice's seed Collection and return its batch, normalized to real hrefs. A sibling failing to answer is a debug log, never a failed poll -- the master must not go unavailable - because a sibling timed out or dropped off (e.g. HJcom's - /device/2, a SmartThings-unused component that may not always - respond). Blocking -- called from _poll_once, already in executor.""" + because a sibling timed out or dropped off (e.g. the issue #177 + reporter's /device/2, a SmartThings-unused component that may not + always respond). Blocking -- called from _poll_once, already in + executor.""" sess = self._session if sess is None: return {} + if subdevice.flat_hrefs: + return self._poll_subdevice_flat_hrefs(subdevice, sess) try: code, payload = sess.get(list(subdevice.seed_path), timeout=10.0) if code == 0x45 and payload: @@ -424,6 +428,53 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): self._log.debug("subdevice %s seed poll failed: %s", subdevice.key, e) return {} + def _poll_subdevice_flat_hrefs(self, subdevice: Subdevice, sess) -> dict[str, dict]: + """Re-poll a flat-mode prefixed subdevice's hrefs individually + (issue #205) -- it has no Collection endpoint to batch-refresh + through (see enumerate_subdevices' fallback), so each canonical + href confirmed at enumeration time gets its own GET under the + subdevice's prefix. A href failing to answer this cycle just drops + out of the result, same "never let a sibling's flakiness fail the + master's poll" posture as the Collection path above. + + Takes `sess` from the caller (already None-checked there) rather + than re-reading self._session -- async_close() can null that + without holding _session_lock, and pace()/get() both need a live + session on every iteration, not just the first. + + Skips any href already covered by the hot/warm sub-poll tiers + (self._hot_hrefs/_warm_hrefs, in the same actual/on-the-wire form + this method builds) -- those are already refreshed every 3s/6s by + _run_subpolls, strictly more current than this once-per-summary-poll + pass could offer, so re-fetching them here would only add GETs + without adding freshness. A subdevice with many confirmed hrefs + (unlike a Collection batch, which is always one GET regardless of + count) is otherwise a summary-poll cost that scales with its href + count.""" + skip = set(self._hot_hrefs) | set(self._warm_hrefs) + result: dict[str, dict] = {} + first = True + for href in subdevice.flat_hrefs: + actual = subdevice.to_actual(href) + if actual in skip: + continue + try: + if not first: + sess.pace() + first = False + path = [s for s in actual.strip('/').split('/')] + code, payload = sess.get(path, timeout=10.0) + if code == 0x45 and payload: + rep = cbor2.loads(payload) + if isinstance(rep, dict): + result[actual] = rep + except Exception as e: + self._log.debug( + "subdevice %s flat href %s poll failed: %s", + subdevice.key, href, e, + ) + return result + def _poll_hrefs_blocking(self, hrefs: list[str]) -> dict[str, dict]: """GET individual hrefs sequentially. Does not reconnect on failure. Blocking.""" if self._session is None: @@ -494,8 +545,9 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): _run_discovery sees every candidate's state without a second poll round trip. `_run_discovery` is what narrows self.subdevices down to the ones that are actually live (see discover_partitioned) -- this - method doesn't know how to tell an unused SmartThings slot (HJcom's - /device/2) from a real sibling, only that something answered. + method doesn't know how to tell an unused SmartThings slot (the + issue #177 reporter's /device/2) from a real sibling, only that + something answered. """ if self._session is None: self._connect_session() @@ -573,8 +625,8 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): # its own /information/vs/0 when it reports one and falling back to # the master's registry otherwise. See subdevices.discover_partitioned # -- it also gates each candidate down to whether it actually - # produced live primary state (HJcom's /device/2, an unused - # SmartThings slot, answers its seed but never does), so + # produced live primary state (the issue #177 reporter's /device/2, + # an unused SmartThings slot, answers its seed but never does), so # self.subdevices below is narrowed to the ones that passed, not # every candidate _enumerate_subdevices_blocking found. For a device # with no candidates (self.subdevices == []) this is exactly the @@ -592,11 +644,12 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): skip.subdevice.key, skip.subdevice.kind, list(skip.hrefs), ) # Corroborating signal, not a gate (DESIGN-177.md section 4): - # /multidevice/vs/0's numofsubdevice is a plain count HJcom's board - # reports independently of the liveness gate above. Log, don't - # raise, on a disagreement -- only this one board family is known to - # expose the resource at all, so a mismatch is a "look into this" - # signal for triage, not proof either side is wrong. + # /multidevice/vs/0's numofsubdevice is a plain count the issue + # #177 reporter's board reports independently of the liveness gate + # above. Log, don't raise, on a disagreement -- only this one board + # family is known to expose the resource at all, so a mismatch is a + # "look into this" signal for triage, not proof either side is + # wrong. numofsubdevice = self._multidevice.get( 'x.com.samsung.da.numofsubdevice') if numofsubdevice is not None: diff --git a/custom_components/localthings/diagnostics.py b/custom_components/localthings/diagnostics.py index 2a1c010..8190856 100644 --- a/custom_components/localthings/diagnostics.py +++ b/custom_components/localthings/diagnostics.py @@ -41,6 +41,17 @@ async def async_get_config_entry_diagnostics( # registry/identity.py. identity = coordinator._identity + def _seed_diag(su) -> dict: + # A flat-mode subdevice (issue #205 -- no working //device/0 + # Collection, so its state comes from individually-polled hrefs + # instead) has no meaningful seed_path; report the flat_hrefs list + # in its place rather than the misleading bare "/" a joined empty + # tuple would otherwise produce. + return { + "seed_path": ("/" + "/".join(su.seed_path)) if su.seed_path else None, + "flat_hrefs": list(su.flat_hrefs), + } + def _subdevice_diag(su) -> dict: # One pass over coordinator.bound for both fields below (count and # the distinct hrefs), and one redaction of this subdevice's canonical @@ -53,7 +64,7 @@ async def async_get_config_entry_diagnostics( return { "kind": su.kind, "key": su.key, - "seed_path": "/" + "/".join(su.seed_path), + **_seed_diag(su), "bound_entity_count": len(matching), "hrefs": sorted({b.href for b in matching}), "model": res.get('/information/vs/0', {}).get('x.com.samsung.da.modelNum', ''), @@ -101,16 +112,16 @@ async def async_get_config_entry_diagnostics( "subdevices": [_subdevice_diag(su) for su in coordinator.subdevices], # Candidates that answered their seed but that discover_partitioned's # entity-level liveness gate rejected -- an unused SmartThings slot - # (HJcom's /device/2) that still answers a same-shaped batch, not a - # real second subdevice. Reported alongside subdevices above so a report - # shows what was found *and* why it didn't become an entity, not - # just silence where a third climate card might otherwise be - # expected. + # (the issue #177 reporter's /device/2) that still answers a + # same-shaped batch, not a real second subdevice. Reported alongside + # subdevices above so a report shows what was found *and* why it + # didn't become an entity, not just silence where a third climate + # card might otherwise be expected. "subdevices_skipped": [ { "kind": skip.subdevice.kind, "key": skip.subdevice.key, - "seed_path": "/" + "/".join(skip.subdevice.seed_path), + **_seed_diag(skip.subdevice), "hrefs": list(skip.hrefs), # The reps the liveness gate actually judged, canonicalized # like the materialized subdevices above. These are the one diff --git a/custom_components/localthings/registry/capabilities/airconditioner.py b/custom_components/localthings/registry/capabilities/airconditioner.py index b817e3f..b753039 100644 --- a/custom_components/localthings/registry/capabilities/airconditioner.py +++ b/custom_components/localthings/registry/capabilities/airconditioner.py @@ -958,7 +958,7 @@ _AC_IGNORED = [ # Undocumented single int (runningMode: 0 on every dump seen), no # supported-values list to interpret it against -- 'don't guess'. '/runn/vs/0', - # 2-in-1/multi-indoor-subdevice systems (issue #177, HJcom's + # 2-in-1/multi-indoor-subdevice systems (issue #177, the reporter's # ARTIK051_DONGLE_FAC_18K): x.com.samsung.da.numofsubdevice, a plain # corroborating count of indoor subdevices on this connection. Confirmed # read-only (a write attempt returned CoAP 4.00). Absent from diff --git a/custom_components/localthings/registry/identity.py b/custom_components/localthings/registry/identity.py index 25907f6..9168456 100644 --- a/custom_components/localthings/registry/identity.py +++ b/custom_components/localthings/registry/identity.py @@ -73,7 +73,7 @@ def read_identity(sess, serial: Optional[str]) -> DeviceIdentity: # physical device -- one IP, one /oic/p -- exposing more than one logical # subdevice, each as its own Collection resource, same rt shape as our own # /device/0). This is what registry.subdevices.enumerate_subdevices reads - # to find a board's `/device/` siblings (Pattern A -- HJcom's + # to find a board's `/device/` siblings (Pattern A -- the reporter's # ARTIK051_DONGLE_FAC_18K) -- that probing, plus the /device/1 and # /device/2 speculative fallback it used to run right here on every # _connect_session (including every reconnect), moved to that module so diff --git a/custom_components/localthings/registry/subdevices.py b/custom_components/localthings/registry/subdevices.py index 8d5590e..a6f48b2 100644 --- a/custom_components/localthings/registry/subdevices.py +++ b/custom_components/localthings/registry/subdevices.py @@ -4,50 +4,64 @@ more than one logical indoor subdevice -- issue #177. Two reporters, two different board families, two genuinely different mechanisms for exposing a second indoor subdevice over one IP / one DTLS session (see DESIGN-177.md section 1 for the full evidence trail; the two -diagnostics dumps this was built against are HJcom's and jhkwon19's -- they -each filed one of the two reports this module unifies): +diagnostics dumps this was built against come from the Pattern A and +Pattern B reporters, respectively -- they each filed one of the two +reports this module unifies): -Pattern A -- indexed siblings (`ARTIK051_DONGLE_FAC_18K`, HJcom's board). -`/oic/res` lists three complete parallel resource sets whose trailing path -segment is the index (`/mode/vs/0`, `/mode/vs/1`, `/mode/vs/2`, ... on both -OCF-standard and vendor hrefs), and `/device/0`'s batch carries only the -index-0 hrefs -- the sibling subdevices are reachable only via their own -`/device/` collection. +Pattern A -- indexed siblings (`ARTIK051_DONGLE_FAC_18K`, that reporter's +board). `/oic/res` lists three complete parallel resource sets whose +trailing path segment is the index (`/mode/vs/0`, `/mode/vs/1`, +`/mode/vs/2`, ... on both OCF-standard and vendor hrefs), and `/device/0`'s +batch carries only the index-0 hrefs -- the sibling subdevices are +reachable only via their own `/device/` collection. -Pattern B -- UUID-prefixed tree (`TP2X_FAC_BORA_21K`, jhkwon19's board). +Pattern B -- UUID-prefixed tree (`TP2X_FAC_BORA_21K`, that reporter's board). `/oic/res` hides the whole appliance tree; `/device/0`'s batch instead carries `x.com.samsung.da.subdeviceIdList` on `/subdevices/vs/0`, and that same UUID appears as a literal href prefix in `/oic/res` -(`//file/list/vs/0`, ...). `GET //device/0` returns the second -subdevice's own Collection batch, confirmed live by the reporter to carry a -different model/serial than the master (`TP2X_FAC_BORA_RAC_21K`, the -wall-mounted subdevice, vs. the master's `TP2X_FAC_BORA_21K`, the floor -subdevice). +(`//file/list/vs/0`, ...). What's actually been confirmed live on +that reporter's unit is narrower than early issue #177 writeups suggested: a +single individual `GET //information/vs/0` was read by hand through +the debug panel and came back carrying a different model/serial than the +master (`TP2X_FAC_BORA_RAC_21K`, the wall-mounted subdevice, vs. the +master's `TP2X_FAC_BORA_21K`, the floor subdevice) -- real evidence a +second subdevice exists at that prefix, but not evidence that `GET +//device/0` (the Collection batch PR #199 built this pattern's seed +around) itself returns anything. Issue #205, the same unit on a later +version, is that assumption failing: `//device/0` comes back empty. +So `enumerate_subdevices` tries it first (a future board might genuinely +expose it) and falls back, when it's empty, to probing every href the +master itself answered this cycle individually under the UUID prefix -- +the only thing ever actually confirmed to work for this pattern -- on the +assumption that a composite device's siblings share the master's resource +surface. See `Subdevice.flat_hrefs`. Both are "the same thing wearing different clothes": a logical subdevice is a seed collection path to poll, plus an href transform between the canonical href the registry knows (`/mode/vs/0`) and the actual on-the-wire href. The -detection signals don't overlap on either captured board (HJcom's has no -`/subdevices/vs/0` at all; jhkwon19's has no `/device/1`), so no -disambiguation logic is needed -- `enumerate_subdevices` checks both and -materializes any candidate whose seed answers with a non-empty batch. +detection signals don't overlap on either captured board (the Pattern A +reporter's has no `/subdevices/vs/0` at all; the Pattern B reporter's has +no `/device/1`), so no disambiguation logic is needed -- +`enumerate_subdevices` checks both and materializes any candidate whose +seed answers with a non-empty batch. A non-empty seed batch is necessary but not sufficient for the *candidate* -to actually be a live second subdevice, though: HJcom's own board also has a -`/device/2` -- a third, unused SmartThings slot -- that answers with the -exact same 14-href shape as the real `/device/1` sibling, populated with -three constant/echoed/shape-only reps (a region code identical to every -other subdevice's, an /information rep echoing the *same* model string as -subdevice 1, and a /temperatures items[] entry with an id/description but no -current/desired/minimum/maximum reading) and nothing resembling live -climate state. Gating on *resource* shape/hrefs turned out to be the wrong -layer -- it would need per-family domain knowledge (which hrefs mean "in -use" for a washer's second drum, a fridge's second compartment, ...) baked -into a registry field before any of those families could use this module -at all. `discover_partitioned` instead gates at the *entity* layer, after -discovery+flattening: a candidate is only kept if it produced at least one -*primary* (no `entity_category`) bound entity whose flattened value isn't -`None` -- e.g. HJcom's /device/2 does flatten to an `alarm_code` value, but +to actually be a live second subdevice, though: the Pattern A reporter's +own board also has a `/device/2` -- a third, unused SmartThings slot -- +that answers with the exact same 14-href shape as the real `/device/1` +sibling, populated with three constant/echoed/shape-only reps (a region +code identical to every other subdevice's, an /information rep echoing the +*same* model string as subdevice 1, and a /temperatures items[] entry with +an id/description but no current/desired/minimum/maximum reading) and +nothing resembling live climate state. Gating on *resource* shape/hrefs +turned out to be the wrong layer -- it would need per-family domain +knowledge (which hrefs mean "in use" for a washer's second drum, a +fridge's second compartment, ...) baked into a registry field before any +of those families could use this module at all. `discover_partitioned` +instead gates at the *entity* layer, after discovery+flattening: a +candidate is only kept if it produced at least one *primary* (no +`entity_category`) bound entity whose flattened value isn't `None` -- e.g. +the Pattern A reporter's /device/2 does flatten to an `alarm_code` value, but that entity is diagnostic-category and derived from an empty /alarms/vs/2, so it doesn't count. This reuses the same primary/config/diagnostic taxonomy every registry already declares (see the adding-device-support @@ -90,10 +104,19 @@ class Subdevice: ('1', '2', ...) or the full subdevice UUID, and `seed_path` is the Collection href (as path segments) whose batch response enumerates/refreshes that subdevice. + + `flat_hrefs` is non-empty only for a 'prefixed' subdevice that doesn't + expose its own Collection at `seed_path` (issue #205 -- not even + TP2X_FAC_BORA_21K, the board this pattern was built against, always + does). When set, `seed_path` is meaningless (left as `()`) and this + subdevice's state comes from GETting each of these canonical hrefs + individually under its prefix instead of one Collection batch -- see + enumerate_subdevices' fallback and coordinator._poll_subdevice_seed. """ kind: str # 'main' | 'indexed' | 'prefixed' key: str # '' | '1' | '6c2dff6d-ee5c-dad1-6a5e-000000000001' seed_path: tuple[str, ...] + flat_hrefs: tuple[str, ...] = () def to_actual(self, canonical: str) -> str: """Canonical registry href (e.g. '/mode/vs/0') -> the real, @@ -196,10 +219,11 @@ def normalize_seed_batch(subdevice: Subdevice, batch: dict[str, dict]) -> dict[s normalized so every href actually carries this subdevice's prefix/index. Indexed subdevices need no change -- the device echoes the real `/x/` - href in its own `/device/` batch (confirmed against HJcom's dump). - A prefixed subdevice's batch entries may or may not already carry the - `/` prefix (unconfirmed which -- jhkwon19's board was never probed - live before the subdevice id was known), so it's added when missing. + href in its own `/device/` batch (confirmed against the Pattern A + reporter's dump). A prefixed subdevice's batch entries may or may not + already carry the `/` prefix (unconfirmed which -- the Pattern B + reporter's board was never probed live before the subdevice id was + known), so it's added when missing. """ if subdevice.kind != 'prefixed': return batch @@ -262,9 +286,9 @@ def _get_batch(sess, path_segs: tuple[str, ...]) -> dict[str, dict]: def _get_property(sess, path_segs: tuple[str, ...]) -> dict: """GET a plain OCF Property-map resource (a bare dict, not a Collection batch). Used for `/multidevice/vs/0` (issue #177 follow-up): listed in - `/oic/res` on HJcom's board but absent from `/device/0`'s batch, so it - needs its own RETRIEVE, and it answers a single Property map, not a - [devcol-rep, ...] list.""" + `/oic/res` on the Pattern A reporter's board but absent from + `/device/0`'s batch, so it needs its own RETRIEVE, and it answers a + single Property map, not a [devcol-rep, ...] list.""" body = _get_raw(sess, path_segs) return body if isinstance(body, dict) else {} @@ -293,9 +317,10 @@ def enumerate_subdevices( Every candidate whose seed answers with a non-empty batch is returned here -- this function has no way to tell a real sibling from an unused - SmartThings slot that merely answers the same shape (HJcom's - `/device/2`); that requires discovering+flattening the candidate's own - entities first, which is `discover_partitioned`'s job, not this one's. + SmartThings slot that merely answers the same shape (the Pattern A + reporter's `/device/2`); that requires discovering+flattening the + candidate's own entities first, which is `discover_partitioned`'s job, + not this one's. See this module's docstring. """ subdevices: list[Subdevice] = [] @@ -319,11 +344,52 @@ def enumerate_subdevices( seed = (sub_id, 'device', '0') batch = _get_batch(sess, seed) _probed(_seed_href(seed), batch) - if not batch: + if batch: + subdevice = Subdevice(kind='prefixed', key=sub_id, seed_path=seed) + fetched.update(normalize_seed_batch(subdevice, batch)) + subdevices.append(subdevice) continue - subdevice = Subdevice(kind='prefixed', key=sub_id, seed_path=seed) - fetched.update(normalize_seed_batch(subdevice, batch)) - subdevices.append(subdevice) + # Fallback (issue #205): TP2X_FAC_BORA_21K itself -- the board this + # pattern was built against -- turns out not to always expose its own + # `//device/0` Collection either, so "every prefixed subdevice + # has one" doesn't hold even on the reference hardware. With no + # Collection to seed from and no per-UUID entry in `/oic/res` to + # enumerate hrefs from, the only signal left is that a composite + # device's siblings are the same physical board family as the + # subdevice this config entry already talks to -- so probe every + # href the master itself answered this cycle, individually, under + # this UUID's prefix, and keep whichever ones answer. Each is a + # plain tolerated-404 RETRIEVE, same posture as every other probe in + # this function. + # + # Known gap, not yet guarded against: a firmware that answers *any* + # request under an unrecognized prefix (echoing the master's own + # state back rather than 4.04ing) would pass every one of these + # probes and, if the echoed state also clears discover_partitioned's + # liveness gate, materialize a phantom duplicate of the master + # rather than a real sibling. Every board seen so far genuinely + # 4.04s on paths it doesn't own (issue #205's own unit answered only + # 1 of 31 probes), so this hasn't been built -- the one place it + # could hook in later is comparing a candidate's confirmed reps + # against the master's own values for those same canonical hrefs. + flat_hrefs = [] + first = True + for href in sorted(resources): + if not first: + sess.pace() + first = False + actual = f'/{sub_id}{href}' + rep = _get_property(sess, tuple(actual.strip('/').split('/'))) + _probed(actual, bool(rep)) + if rep: + flat_hrefs.append(href) + fetched[actual] = rep + if not flat_hrefs: + continue + subdevices.append(Subdevice( + kind='prefixed', key=sub_id, seed_path=(), + flat_hrefs=tuple(flat_hrefs), + )) # --- Pattern A: indexed siblings (ARTIK051_DONGLE_FAC_18K) -------------- indices = sorted({ @@ -334,7 +400,7 @@ def enumerate_subdevices( }) if not indices: # A board that hides its whole tree from /oic/res (Pattern B's - # jhkwon19 board does this too, but it has no /device/ to find + # reporter board does this too, but it has no /device/ to find # regardless) gives us nothing to enumerate from -- fall back to the # bounded speculative probe this replaces from identity.py. indices = list(_SPECULATIVE_DEVICE_INDICES) @@ -348,9 +414,9 @@ def enumerate_subdevices( fetched.update(batch) # already real /x/ hrefs, no normalization needed subdevices.append(subdevice) - # /multidevice/vs/0 (issue #177 follow-up): HJcom's board lists it in - # /oic/res but it never appears in /device/0's batch, so it needs its - # own RETRIEVE. It's a plain corroborating count + # /multidevice/vs/0 (issue #177 follow-up): the Pattern A reporter's + # board lists it in /oic/res but it never appears in /device/0's batch, + # so it needs its own RETRIEVE. It's a plain corroborating count # (x.com.samsung.da.numofsubdevice), confirmed read-only (a write # attempt returned CoAP 4.00) -- captured for diagnostics only, folded # into the merged resources dict like any other href (see @@ -374,9 +440,9 @@ def enumerate_subdevices( class SkippedSubdevice: """A candidate `enumerate_subdevices` found whose seed answered, but that `discover_partitioned`'s entity-level liveness gate rejected -- an - unused SmartThings slot (HJcom's `/device/2`), not a real second subdevice. - Kept around (rather than silently dropped) so a caller can log/report - what was skipped and why.""" + unused SmartThings slot (the Pattern A reporter's `/device/2`), not a + real second subdevice. Kept around (rather than silently dropped) so a + caller can log/report what was skipped and why.""" subdevice: Subdevice hrefs: tuple[str, ...] @@ -388,7 +454,7 @@ def _has_live_primary_entity(bound, state: dict) -> bool: tier (see the adding-device-support skill's entity-taxonomy section). This is the materialization gate itself (see this module's docstring): - HJcom's `/device/2` does flatten to one non-`None` value + the Pattern A reporter's `/device/2` does flatten to one non-`None` value (`alarm_code`), but that entity is `diagnostic`-category and derived from an empty `/alarms/vs/2` -- a config/diagnostic entity reading "something" proves nothing about whether a physical subdevice is actually @@ -418,8 +484,8 @@ def discover_partitioned( device's registry claims that literal href) and raise a spurious coverage-gap repair. Then one pass per *candidate* subdevice over its own canonical view, resolving that subdevice's own device type from its own - `/information/vs/0` when it reports one (e.g. jhkwon19's wall subdevice - reports `TP2X_FAC_BORA_RAC_21K` -> the 'RAC' board token -> + `/information/vs/0` when it reports one (e.g. the Pattern B reporter's + wall subdevice reports `TP2X_FAC_BORA_RAC_21K` -> the 'RAC' board token -> airconditioner), falling back to the master's registry otherwise -- every AC family shares the same resource surface, and a sibling that fails to answer its own identity resource is still the same appliance diff --git a/tests/fixtures/airconditioner_artik051_dongle_fac_18k_device.json b/tests/fixtures/airconditioner_artik051_dongle_fac_18k_device.json index c83e4fd..59f0238 100644 --- a/tests/fixtures/airconditioner_artik051_dongle_fac_18k_device.json +++ b/tests/fixtures/airconditioner_artik051_dongle_fac_18k_device.json @@ -2029,5 +2029,5 @@ "x.com.samsung.da.numofsubdevice": "2" } }, - "seeds_note": "ALL REAL CAPTURED DATA -- issue #177, HJcom's ARTIK051_DONGLE_FAC_18K (Samsung 2-in-1: floor-standing master + wall-mounted second indoor subdevice). device0, oic_res and both /device/ seeds come verbatim from their v0.16.0 diagnostics download, which is the first release that probes /device/1 and /device/2. Nothing here is reconstructed. The reporter masked a handful of values in their own dump before posting (serialNum, and the airflow `direction` / vendor `humidity` readings); those are normalized onto this repo's usual **REDACTED** marker. /device/1 is the real second subdevice -- populated power/mode/temperature and its own /information/vs/1 reporting ARTIK051_DONGLE_FAC_RAC_18K (RAC = the wall-mounted subdevice) against the master's ARTIK051_DONGLE_FAC_18K. /device/2 answers with the same 14-href shape but every state rep is empty {} -- it is the unused third slot SmartThings shows disabled, and it is why subdevice materialization gates on populated climate state rather than on a seed merely answering. `probes` is not part of any batch response: /multidevice/vs/0 is listed in oic_res but absent from /device/0's batch, and the reporter read it by hand through the debug panel (a write attempt returned CoAP 4.00, i.e. read-only). Its numofsubdevice=2 independently corroborates 2 real subdevices, not 3." + "seeds_note": "ALL REAL CAPTURED DATA -- issue #177, the reporter's ARTIK051_DONGLE_FAC_18K (Samsung 2-in-1: floor-standing master + wall-mounted second indoor subdevice). device0, oic_res and both /device/ seeds come verbatim from their v0.16.0 diagnostics download, which is the first release that probes /device/1 and /device/2. Nothing here is reconstructed. The reporter masked a handful of values in their own dump before posting (serialNum, and the airflow `direction` / vendor `humidity` readings); those are normalized onto this repo's usual **REDACTED** marker. /device/1 is the real second subdevice -- populated power/mode/temperature and its own /information/vs/1 reporting ARTIK051_DONGLE_FAC_RAC_18K (RAC = the wall-mounted subdevice) against the master's ARTIK051_DONGLE_FAC_18K. /device/2 answers with the same 14-href shape but every state rep is empty {} -- it is the unused third slot SmartThings shows disabled, and it is why subdevice materialization gates on populated climate state rather than on a seed merely answering. `probes` is not part of any batch response: /multidevice/vs/0 is listed in oic_res but absent from /device/0's batch, and the reporter read it by hand through the debug panel (a write attempt returned CoAP 4.00, i.e. read-only). Its numofsubdevice=2 independently corroborates 2 real subdevices, not 3." } diff --git a/tests/fixtures/airconditioner_fac_bora_205_flat_device.json b/tests/fixtures/airconditioner_fac_bora_205_flat_device.json new file mode 100644 index 0000000..8421ae0 --- /dev/null +++ b/tests/fixtures/airconditioner_fac_bora_205_flat_device.json @@ -0,0 +1,720 @@ +{ + "device0": [ + { + "rt": [ + "x.com.samsung.devcol", + "oic.wk.col" + ], + "if": [ + "oic.if.baseline", + "oic.if.ll", + "oic.if.b" + ] + }, + { + "href": "/alarms/vs/0", + "rep": { + "x.com.samsung.da.items": [ + { + "x.com.samsung.da.id": "0", + "x.com.samsung.da.description": "Alarm", + "x.com.samsung.da.alarmType": "Device", + "x.com.samsung.da.code": "ErrorCode_OFF", + "x.com.samsung.da.triggeredTime": "2026-07-30T01:48:51" + }, + { + "x.com.samsung.da.id": "2", + "x.com.samsung.da.description": "Alarm", + "x.com.samsung.da.alarmType": "Device", + "x.com.samsung.da.code": "AC_V_0002_OFF", + "x.com.samsung.da.triggeredTime": "2026-07-30T01:48:51" + } + ] + } + }, + { + "href": "/availablecontrolsets/vs/0", + "rep": { + "x.com.samsung.da.sets": "000000B4012C0000404B04000000", + "x.com.samsung.da.id": "FAC", + "x.com.samsung.da.version": "1.0" + } + }, + { + "href": "/configuration/vs/0", + "rep": { + "x.com.samsung.da.region": "3017000000", + "x.com.samsung.da.airconOptionList": [ + "HOMECARE_WIZARD_V2", + "ENERGY_2.0", + "AI_2.0", + "DeviceTypeMaster", + "SingleCommand_1" + ] + } + }, + { + "href": "/diagnosis/vs/0", + "rep": {} + }, + { + "href": "/drlc/0", + "rep": { + "DRLevel": 0, + "start": "1970-01-01T00:00:00Z", + "duration": 0, + "override": false + } + }, + { + "href": "/drlc/vs/0", + "rep": { + "x.com.samsung.da.drlcLevel": "0", + "x.com.samsung.da.duration": "00:00:00", + "x.com.samsung.da.drlcStartTime": "1970-01-01T00:00:00Z", + "x.com.samsung.da.override": "Off" + } + }, + { + "href": "/energy/consumption/0", + "rep": { + "energy": 800.0, + "power": 65278.0 + } + }, + { + "href": "/energy/consumption/vs/0", + "rep": { + "x.com.samsung.da.cumulativeConsumption": "800.000000", + "x.com.samsung.da.instantaneousPower": "65278.000000", + "x.com.samsung.da.usageThreshold": "0.000000", + "x.com.samsung.da.cumulativePower": "544088", + "x.com.samsung.da.cumulativeUnit": "Wh", + "x.com.samsung.da.instantaneousPowerUnit": "W", + "x.com.samsung.da.cumulativePowerType": "individual" + } + }, + { + "href": "/file/information/vs/0", + "rep": { + "x.com.samsung.timeoffset": "+09:00", + "x.com.samsung.supprtedtype": 1 + } + }, + { + "href": "/filter/airdustfilter/vs/0", + "rep": { + "x.com.samsung.da.filterUsage": "0", + "x.com.samsung.da.filterUsageResolution": "1", + "x.com.samsung.da.filterDesiredUsage": "112", + "x.com.samsung.da.filterStatus": "normal", + "x.com.samsung.da.filterCapacity": "112", + "x.com.samsung.da.filterCapacityUnit": "Hour", + "x.com.samsung.da.filterResetType": [ + "washable" + ], + "x.com.samsung.da.supportedFilterDesiredUsage": [ + "112", + "224", + "336", + "448" + ] + } + }, + { + "href": "/humidity/0", + "rep": { + "humidity": 0 + } + }, + { + "href": "/humidity/vs/0", + "rep": { + "x.com.samsung.da.humidity": "0.000000", + "x.com.samsung.da.fivepercentHumidity": "58" + } + }, + { + "href": "/information/vs/0", + "rep": { + "x.com.samsung.da.modelNum": "TP2X_FAC_BORA_21K|10233041|600001110015110006000C1200830000", + "x.com.samsung.da.description": "TP2X_FAC_BORA_21K", + "x.com.samsung.da.serialNum": "**REDACTED**", + "x.com.samsung.da.otnDUID": "**REDACTED**", + "x.com.samsung.da.diagProtocolType": "WIFI_HTTPS", + "x.com.samsung.da.diagLogType": [ + "errCode", + "dump" + ], + "x.com.samsung.da.diagDumpType": "file", + "x.com.samsung.da.diagEndPoint": "SSM", + "x.com.samsung.da.diagMnid": "0AJT", + "x.com.samsung.da.diagSetupid": "000", + "x.com.samsung.da.diagMinVersion": "1.0", + "x.com.samsung.da.serialNumOption": "**REDACTED**", + "x.com.samsung.da.items": [ + { + "x.com.samsung.da.id": "0", + "x.com.samsung.da.description": "Version", + "x.com.samsung.da.type": "Software", + "x.com.samsung.da.number": "02337A260424", + "x.com.samsung.da.newVersionAvailable": "0" + }, + { + "x.com.samsung.da.id": "1", + "x.com.samsung.da.description": "Version", + "x.com.samsung.da.type": "Firmware", + "x.com.samsung.da.number": "2102240021022200", + "x.com.samsung.da.newVersionAvailable": "0" + }, + { + "x.com.samsung.da.id": "2", + "x.com.samsung.da.description": "Version", + "x.com.samsung.da.type": "Outdoor", + "x.com.samsung.da.number": "2103300110000300" + } + ] + } + }, + { + "href": "/keepnormalstate/vs/0", + "rep": { + "x.com.samsung.da.keepnormal": 5 + } + }, + { + "href": "/mode/convenient/vs/0", + "rep": { + "x.com.samsung.da.modes": "Off", + "x.com.samsung.da.supportedModes": [ + "Off", + "Sleep", + "Quiet", + "Speed" + ] + } + }, + { + "href": "/mode/vs/0", + "rep": { + "x.com.samsung.da.supportedModes": [ + "AIComfort", + "Cool", + "Dry", + "Wind" + ], + "x.com.samsung.da.modes": [ + "Wind" + ], + "x.com.samsung.da.options": [ + "Operation_Family", + "Blooming_0", + "OnTimer_0", + "OffTimer_0", + "Sleep_16", + "ArtificialWorking_Off", + "ComfortAICooling_Off", + "AiTempChanged_Off", + "AiTemp_270", + "welcomecare_Off", + "Panel_Close", + "Weather_Off", + "Volume_100", + "StopAutoClean_Idle", + "DiagnosisAI_Off", + "Display_Off", + "ProgressDiagnosisAI_1", + "ResultDiagnosisAI_Normal", + "Service_Off", + "SmartCoolClean_Off", + "ProgressSmartClean_0", + "OutDoorVentil_Off", + "FreezeAlarmSetting_Off", + "DesiredFreezeAlarm_240", + "OptionCode_529", + "ExtendOptionCode_16975", + "RacInfo_First", + "RacInfo_None_Second", + "ModelInfo_16K_BORA_VENT2", + "UpdateAllow_NotAllowed", + "EnergySaveIcon_Off", + "DurationOn_0", + "welcomecareElapsedTime_0", + "welcomecareThresholdTemp_0", + "welcomecareStartDate_0000", + "welcomecareEndDate_0000", + "welcomecareSeason_None" + ] + } + }, + { + "href": "/option/autoclean/vs/0", + "rep": { + "x.com.samsung.da.status": "Stop", + "x.com.samsung.da.settingStatus": "On", + "x.com.samsung.da.progress": "0", + "x.com.samsung.da.supportedStatus": [ + "Start", + "SpeedClean", + "QuietClean", + "Stop" + ], + "x.com.samsung.da.supportedSettingStatus": [ + "On", + "SpeedClean", + "QuietClean", + "Off" + ] + } + }, + { + "href": "/otninformation/vs/0", + "rep": { + "x.com.samsung.da.target": "", + "x.com.samsung.da.newVersionAvailable": "false", + "x.com.samsung.da.newVersionNo": "00000000", + "x.com.samsung.da.currentVersionInfo": "00000000", + "otnStatus": "None", + "flashingProgress": "", + "otnTarget": "main", + "otnCompleteDate": "noHistory", + "otnList": [ + { + "type": "WIFI", + "modelId": "AFA-KR-TP2-21-AF9X00", + "versions": [ + "10260424" + ], + "visVersion": "260424" + }, + { + "type": "Micom", + "modelId": "04511023304110232941", + "versions": [ + "21022400", + "21022200" + ], + "visVersion": "210224" + }, + { + "type": "Micom", + "modelId": "04511022974110229941", + "versions": [ + "21033001", + "10000300" + ], + "visVersion": "210330" + }, + { + "type": "Micom", + "modelId": "045110230741FFFFFFFF", + "versions": [ + "22050300", + "FFFFFFFF" + ], + "visVersion": "220503" + } + ] + } + }, + { + "href": "/personality/presence/vs/0", + "rep": { + "x.com.samsung.da.items": [ + { + "x.com.samsung.da.id": "", + "x.com.samsung.da.deviceId": "**REDACTED**", + "x.com.samsung.da.value": "" + } + ] + } + }, + { + "href": "/power/0", + "rep": { + "value": false + } + }, + { + "href": "/power/vs/0", + "rep": { + "x.com.samsung.da.power": "Off" + } + }, + { + "href": "/realtimenotiforclient/vs/0", + "rep": { + "x.com.samsung.da.timeforshortnoti": "0", + "x.com.samsung.da.longnotisubscription": "true", + "x.com.samsung.da.periodicnotisubscription": "true" + } + }, + { + "href": "/runn/vs/0", + "rep": { + "x.com.samsung.da.runningMode": 0 + } + }, + { + "href": "/subdevices/vs/0", + "rep": { + "x.com.samsung.da.subdeviceIdList": [ + "6c2dff6d-ee5c-dad1-6a5e-000000000001" + ] + } + }, + { + "href": "/temperature/control/vs/0", + "rep": { + "x.com.samsung.da.increment": "1" + } + }, + { + "href": "/temperature/current/0", + "rep": { + "range": [ + 18.0, + 30.0 + ], + "units": "C", + "temperature": 32.0 + } + }, + { + "href": "/temperature/desired/0", + "rep": { + "range": [ + 18.0, + 30.0 + ], + "units": "C", + "temperature": 24.0 + } + }, + { + "href": "/temperatures/vs/0", + "rep": { + "x.com.samsung.da.items": [ + { + "x.com.samsung.da.id": "0", + "x.com.samsung.da.description": "Temperature", + "x.com.samsung.da.desired": "24.0", + "x.com.samsung.da.current": "32.0", + "x.com.samsung.da.maximum": "30", + "x.com.samsung.da.minimum": "18", + "x.com.samsung.da.increment": "1.0", + "x.com.samsung.da.unit": "Celsius" + } + ] + } + }, + { + "href": "/timezone/vs/0", + "rep": { + "timezoneid": "Asia/Seoul", + "offset": "+09:00", + "DST": "OFF" + } + }, + { + "href": "/wind/direction/vs/0", + "rep": { + "x.com.samsung.da.modes": "NotSupported", + "x.com.samsung.da.supportedModes": [ + "NotSupported" + ] + } + }, + { + "href": "/wind/strength/vs/0", + "rep": { + "x.com.samsung.da.modes": "2", + "x.com.samsung.da.supportedModes": [ + "0", + "2", + "3", + "4" + ], + "x.com.samsung.da.modesName": [ + "Auto", + "Mid", + "High", + "Turbo" + ] + } + } + ], + "oic_res": [ + { + "di": "**REDACTED**", + "links": [ + { + "href": "/oic/sec/doxm", + "rt": [ + "oic.r.doxm" + ], + "if": [ + "oic.if.baseline" + ], + "p": { + "bm": 1, + "sec": true, + "port": 49154, + "x.org.iotivity.tls": 0 + } + }, + { + "href": "/oic/sec/pstat", + "rt": [ + "oic.r.pstat" + ], + "if": [ + "oic.if.baseline" + ], + "p": { + "bm": 1, + "sec": true, + "port": 49154, + "x.org.iotivity.tls": 0 + } + }, + { + "href": "/oic/d", + "rt": [ + "oic.wk.d", + "oic.d.airconditioner" + ], + "if": [ + "oic.if.baseline", + "oic.if.r" + ], + "p": { + "bm": 1, + "sec": false, + "x.org.iotivity.tcp": 0 + } + }, + { + "href": "/oic/p", + "rt": [ + "oic.wk.p" + ], + "if": [ + "oic.if.baseline", + "oic.if.r" + ], + "p": { + "bm": 1, + "sec": false, + "x.org.iotivity.tcp": 0 + } + }, + { + "href": "/hass/state/vs/0", + "rt": [ + "x.com.samsung.da.hass.state" + ], + "if": [ + "oic.if.baseline", + "oic.if.a" + ], + "p": { + "bm": 3, + "sec": false, + "x.org.iotivity.tcp": 0 + } + }, + { + "href": "/hass/command/vs/0", + "rt": [ + "x.com.samsung.da.hass.command" + ], + "if": [ + "oic.if.baseline", + "oic.if.a" + ], + "p": { + "bm": 3, + "sec": false, + "x.org.iotivity.tcp": 0 + } + }, + { + "href": "/file/transfer/chunk/vs/0", + "rt": [ + "x.com.samsung.file.chunk" + ], + "if": [ + "oic.if.baseline", + "oic.if.a" + ], + "p": { + "bm": 1, + "sec": false, + "x.org.iotivity.tcp": 0 + } + }, + { + "href": "/file/list/vs/0", + "rt": [ + "x.com.samsung.file.list" + ], + "if": [ + "oic.if.baseline", + "oic.if.s" + ], + "p": { + "bm": 1, + "sec": false, + "x.org.iotivity.tcp": 0 + } + }, + { + "href": "/file/transfer/vs/0", + "rt": [ + "x.com.samsung.file.transfer" + ], + "if": [ + "oic.if.baseline", + "oic.if.a" + ], + "p": { + "bm": 3, + "sec": false, + "x.org.iotivity.tcp": 0 + } + }, + { + "href": "/6c2dff6d-ee5c-dad1-6a5e-000000000001/file/list/vs/0", + "rt": [ + "x.com.samsung.file.list" + ], + "if": [ + "oic.if.baseline", + "oic.if.s" + ], + "p": { + "bm": 1, + "sec": false, + "x.org.iotivity.tcp": 0 + } + }, + { + "href": "/6c2dff6d-ee5c-dad1-6a5e-000000000001/file/transfer/vs/0", + "rt": [ + "x.com.samsung.file.transfer" + ], + "if": [ + "oic.if.baseline", + "oic.if.a" + ], + "p": { + "bm": 3, + "sec": false, + "x.org.iotivity.tcp": 0 + } + }, + { + "href": "/EasySetupResURI", + "rt": [ + "oic.r.easysetup" + ], + "if": [ + "oic.if.baseline", + "oic.if.ll", + "oic.if.b" + ], + "p": { + "bm": 1, + "sec": true, + "port": 49154, + "x.org.iotivity.tls": 0 + } + }, + { + "href": "/WiFiConfResURI", + "rt": [ + "oic.wk.wifi" + ], + "if": [ + "oic.if.baseline" + ], + "p": { + "bm": 1, + "sec": true, + "port": 49154, + "x.org.iotivity.tls": 0 + } + }, + { + "href": "/CoapCloudConfResURI", + "rt": [ + "oic.wk.cloudserver" + ], + "if": [ + "oic.if.baseline" + ], + "p": { + "bm": 1, + "sec": true, + "port": 49154, + "x.org.iotivity.tls": 0 + } + }, + { + "href": "/DevConfResURI", + "rt": [ + "oic.wk.devconf" + ], + "if": [ + "oic.if.baseline" + ], + "p": { + "bm": 1, + "sec": true, + "port": 49154, + "x.org.iotivity.tls": 0 + } + }, + { + "href": "/sec/provisioninginfo", + "rt": [ + "x.com.samsung.provisioninginfo" + ], + "if": [ + "oic.if.baseline", + "oic.if.a" + ], + "p": { + "bm": 1, + "sec": false, + "x.org.iotivity.tcp": 0 + } + }, + { + "href": "/sec/accesspointlist", + "rt": [ + "x.com.samsung.accesspointlist" + ], + "if": [ + "oic.if.baseline", + "oic.if.s" + ], + "p": { + "bm": 1, + "sec": false, + "x.org.iotivity.tcp": 0 + } + } + ] + } + ], + "probes": { + "/6c2dff6d-ee5c-dad1-6a5e-000000000001/information/vs/0": { + "x.com.samsung.da.modelNum": "TP2X_FAC_BORA_RAC_21K|10233041|60010610001500014600081200810000", + "x.com.samsung.da.description": "TP2X_FAC_BORA_RAC_21K", + "x.com.samsung.da.serialNum": "TEST-SUBDEVICE-SERIAL-0000" + } + }, + "seeds_note": "device0 and oic_res are real, captured from the reporter's issue #205 report. subdeviceIdList in device0's /subdevices/vs/0 is restored to the real ['6c2dff6d-ee5c-dad1-6a5e-000000000001'] (HA's diagnostics download redacts it, matching redact.py's 'deviceid' substring rule, not because it's account data -- same restoration airconditioner_fac_bora_2in1_device.json documents for the same field), and every other value is otherwise exactly what HA's download redacts (serials/otnDUID etc.) against the same physical TP2X_FAC_BORA_21K unit as airconditioner_fac_bora_2in1_device.json -- same subdeviceIdList UUID, same wall subdevice. Filed specifically because, contrary to the assumption that fixture's seed batch was built on, this unit's /6c2dff6d-ee5c-dad1-6a5e-000000000001/device/0 does NOT answer (subdevice_probes in the report shows it False), so the Collection-batch fallback this fixture exercises (registry.subdevices.enumerate_subdevices' per-href probe, issue #205) is what has to find the subdevice instead. The one probes entry, /6c2dff6d-ee5c-dad1-6a5e-000000000001/information/vs/0, is the same real capture already used in airconditioner_fac_bora_2in1_device.json's seed batch (issue #177 comment 5113518087) -- the only href ever actually confirmed to answer under this UUID prefix. No other /6c2dff6d-ee5c-dad1-6a5e-000000000001/* href has been confirmed live yet, so none are seeded here; this fixture's expected outcome is the candidate being found by the flat-href probe but then correctly held back by discover_partitioned's liveness gate (information alone binds no entity), matching where the real issue stands -- not a materialized climate entity, which would require guessing at unconfirmed hrefs. The probe's serialNum (\"TEST-SUBDEVICE-SERIAL-0000\") is a hand-placed placeholder for the real value, not HA's own redaction output -- same placeholder airconditioner_fac_bora_2in1_device.json uses for the identical field." +} diff --git a/tests/fixtures/airconditioner_fac_bora_2in1_device.json b/tests/fixtures/airconditioner_fac_bora_2in1_device.json index cb656e5..320f0bf 100644 --- a/tests/fixtures/airconditioner_fac_bora_2in1_device.json +++ b/tests/fixtures/airconditioner_fac_bora_2in1_device.json @@ -805,5 +805,5 @@ } ] }, - "seeds_note": "device0 and oic_res are real, captured from jhkwon19's diagnostics dump for the TP2X_FAC_BORA_21K board (issue #177, the same physical device as tests/fixtures/airconditioner_fac_bora_device.json -- that fixture's redacted x.com.samsung.da.subdeviceIdList string is a deliberate, separate regression case and is NOT changed here). subdeviceIdList above is restored to the real ['6c2dff6d-ee5c-dad1-6a5e-000000000001'] reported in the issue thread (redacted in the raw capture the same way serialNum/otnDUID/deviceId are -- it matches redact.py's 'deviceid' substring rule, not because it's actually account data). The seed's /information/vs/0 rep is REAL, verbatim from the reporter's live debug-panel read of /6c2dff6d-.../information/vs/0 (serial replaced with a placeholder) -- modelNum TP2X_FAC_BORA_RAC_21K confirms this is the wall-mounted room subdevice, distinct from the master's floor subdevice TP2X_FAC_BORA_21K. Every other href in this seed batch is CONSTRUCTED (never read from this subdevice) so the subdevice's climate entity has a resource surface to bind -- per DESIGN-177.md section 4, retrieval for this pattern is batch-only, and confirming '/6c2dff6d-.../device/0' actually returns a full batch (as opposed to just answering the information probe the reporter tried by hand) is listed as a follow-up, not something this fixture can attest to." + "seeds_note": "device0 and oic_res are real, captured from the reporter's diagnostics dump for the TP2X_FAC_BORA_21K board (issue #177, the same physical device as tests/fixtures/airconditioner_fac_bora_device.json -- that fixture's redacted x.com.samsung.da.subdeviceIdList string is a deliberate, separate regression case and is NOT changed here). subdeviceIdList above is restored to the real ['6c2dff6d-ee5c-dad1-6a5e-000000000001'] reported in the issue thread (redacted in the raw capture the same way serialNum/otnDUID/deviceId are -- it matches redact.py's 'deviceid' substring rule, not because it's actually account data). The seed's /information/vs/0 rep is REAL, verbatim from the reporter's live debug-panel read of /6c2dff6d-.../information/vs/0 (serial replaced with a placeholder) -- modelNum TP2X_FAC_BORA_RAC_21K confirms this is the wall-mounted room subdevice, distinct from the master's floor subdevice TP2X_FAC_BORA_21K. Every other href in this seed batch is CONSTRUCTED (never read from this subdevice) so the subdevice's climate entity has a resource surface to bind -- per DESIGN-177.md section 4, retrieval for this pattern is batch-only, and confirming '/6c2dff6d-.../device/0' actually returns a full batch (as opposed to just answering the information probe the reporter tried by hand) is listed as a follow-up, not something this fixture can attest to." } diff --git a/tests/fixtures/golden/airconditioner_fac_bora_205_flat.json b/tests/fixtures/golden/airconditioner_fac_bora_205_flat.json new file mode 100644 index 0000000..aca70e7 --- /dev/null +++ b/tests/fixtures/golden/airconditioner_fac_bora_205_flat.json @@ -0,0 +1,20 @@ +{ + "state_keys": [ + "air_filter_status", + "air_filter_threshold", + "air_filter_usage", + "air_filter_usage_hours", + "alarm_code", + "auto_clean", + "beep", + "climate", + "current_temperature_c", + "diagnosis_status", + "energy_kwh", + "firmware_update", + "humidity", + "power_energy_kwh", + "power_watts", + "tropical_night_mode" + ] +} diff --git a/tests/test_climate_subdevice.py b/tests/test_climate_subdevice.py index e5c45a1..9fedc1e 100644 --- a/tests/test_climate_subdevice.py +++ b/tests/test_climate_subdevice.py @@ -3,10 +3,11 @@ two -- and that the legacy-board test (is_legacy_board/_legacy_airflow) is evaluated per subdevice rather than once globally. -Uses HJcom's ARTIK051_DONGLE_FAC_18K fixture deliberately: it's a legacy -`/airflow/vs/` board (no `/wind/*` at all) on *both* the master and its -materialized sibling, which is exactly the shape climate.py's own comments -warn is easy to get wrong if the canonical view leaks between subdevices. +Uses the issue #177 reporter's ARTIK051_DONGLE_FAC_18K fixture deliberately: +it's a legacy `/airflow/vs/` board (no `/wind/*` at all) on *both* the +master and its materialized sibling, which is exactly the shape climate.py's +own comments warn is easy to get wrong if the canonical view leaks between +subdevices. """ from __future__ import annotations @@ -71,15 +72,16 @@ async def test_subdevice_climate_reads_its_own_power_state(climates): async def test_legacy_board_test_is_evaluated_per_subdevice(climates): - """HJcom's board has no /wind/* resources at all on *either* subdevice -- - is_legacy_board(self._resources) must independently evaluate True for - the master's own canonical view and for the subdevice's own canonical - view. If is_legacy_board were fed the raw, unpartitioned snapshot (or - if canonical_view leaked one subdevice's hrefs into the other's), this - wouldn't distinguish "this subdevice is legacy" from "some subdevice on - this connection is legacy" -- and a future board with one legacy + one - modern subdevice sharing a connection would silently read the wrong - fan/swing channel on one side.""" + """The reporter's board has no /wind/* resources at all on *either* + subdevice -- is_legacy_board(self._resources) must independently + evaluate True for the master's own canonical view and for the + subdevice's own canonical view. If is_legacy_board were fed the raw, + unpartitioned snapshot (or if canonical_view leaked one subdevice's + hrefs into the other's), this wouldn't distinguish "this subdevice is + legacy" from "some subdevice on this connection is legacy" -- and a + future board with one legacy + one modern subdevice sharing a + connection would silently read the wrong fan/swing channel on one + side.""" main, sub1 = climates[None], climates['1'] assert main._legacy_airflow() != {} assert sub1._legacy_airflow() != {} diff --git a/tests/test_diagnostics_subdevices.py b/tests/test_diagnostics_subdevices.py index 2cba348..1406272 100644 --- a/tests/test_diagnostics_subdevices.py +++ b/tests/test_diagnostics_subdevices.py @@ -105,3 +105,25 @@ async def test_diagnostics_reports_prefixed_subdevice( # not the master's TP2X_FAC_BORA_21K. assert diag['subdevices'][0]['model'].startswith('TP2X_FAC_BORA_RAC_21K') assert diag['subdevices_skipped'] == [] + + +async def test_diagnostics_reports_flat_hrefs_for_skipped_prefixed_candidate( + hass: HomeAssistant, enable_custom_integrations, +) -> None: + """issue #205: a prefixed candidate found through the per-href flat + fallback (no working //device/0 Collection) has no meaningful + seed_path -- diagnostics reports None there instead of the misleading + bare "/" an empty tuple would otherwise join to, and lists the actual + hrefs the fallback confirmed instead.""" + coordinator = _coordinator(hass) + await _discover(coordinator, 'airconditioner_fac_bora_205_flat') + hass.data.setdefault(DOMAIN, {})[coordinator._entry.entry_id] = coordinator + + diag = await async_get_config_entry_diagnostics(hass, coordinator._entry) + + assert diag['subdevices'] == [] + assert len(diag['subdevices_skipped']) == 1 + skipped = diag['subdevices_skipped'][0] + assert skipped['kind'] == 'prefixed' + assert skipped['seed_path'] is None + assert skipped['flat_hrefs'] == ['/information/vs/0'] diff --git a/tests/test_golden_regression.py b/tests/test_golden_regression.py index 8752069..eeff63a 100644 --- a/tests/test_golden_regression.py +++ b/tests/test_golden_regression.py @@ -903,7 +903,7 @@ def _new_subdevice_aware_state_keys(name): def test_registry_reproduces_golden_state_keys_for_airconditioner_artik051_dongle_fac_18k(): - """HJcom's ARTIK051_DONGLE_FAC_18K (issue #177, Pattern A -- indexed + """The reporter's ARTIK051_DONGLE_FAC_18K (issue #177, Pattern A -- indexed siblings): a real v0.16.0 dump with a genuine second indoor subdevice at `/device/1` (subdevice1_-prefixed keys below) and an unused SmartThings slot at `/device/2` that answers its seed but never produces a @@ -942,7 +942,7 @@ def test_registry_reproduces_golden_state_keys_for_airconditioner_cac(): def test_registry_reproduces_golden_state_keys_for_airconditioner_fac_bora_2in1(): - """jhkwon19's TP2X_FAC_BORA_21K (issue #177, Pattern B -- UUID-prefixed + """The reporter's TP2X_FAC_BORA_21K (issue #177, Pattern B -- UUID-prefixed tree): device0/oic_res are real; the wall-mounted subdevice's own /information/vs/0 is real (confirmed live by the reporter), the rest of its seed tree is constructed (see the fixture's own seeds_note) -- just @@ -961,6 +961,30 @@ def test_registry_reproduces_golden_state_keys_for_airconditioner_fac_bora_2in1( ) +def test_registry_reproduces_golden_state_keys_for_airconditioner_fac_bora_205_flat(): + """The same reporter's same physical TP2X_FAC_BORA_21K unit as the _2in1 + fixture above, but a later capture (issue #205) where //device/0 doesn't + answer -- contrary to what that fixture's own seed batch assumed the + Collection endpoint would do. device0/oic_res are real; the only + UUID-prefixed data is the one href ever actually confirmed live + (/information/vs/0, same real capture the _2in1 fixture uses), fed + through registry.subdevices.enumerate_subdevices' per-href flat + fallback instead of a Collection batch. /information/vs/0 alone binds + no entity, so the candidate is found but never materializes -- this + golden has no `subdevice_...`-prefixed keys at all, same shape as + tests/fixtures/golden/airconditioner_fac_bora.json, which is the point: + a device whose sibling can't yet be confirmed live must regress to + exactly the master-only state, never a phantom or partial subdevice.""" + name = 'airconditioner_fac_bora_205_flat' + golden = json.loads((GOLDEN / f'{name}.json').read_text()) + state_keys = _new_subdevice_aware_state_keys(name) + assert set(state_keys) == set(golden['state_keys']), ( + f"state_keys mismatch:\n" + f" extra: {sorted(set(state_keys) - set(golden['state_keys']))}\n" + f" missing: {sorted(set(golden['state_keys']) - set(state_keys))}" + ) + + def test_registry_reproduces_golden_state_keys_for_air_purifier_avt_ww(): """AVT-WW-TP1-23-AXX500 (issue #190) -- next-gen BESPOKE Cube Air board; reports device_type 'unknown' with empty oneUiVersion because 'VTWW' as a diff --git a/tests/test_subdevice_discovery.py b/tests/test_subdevice_discovery.py index 5e96ca4..50065c5 100644 --- a/tests/test_subdevice_discovery.py +++ b/tests/test_subdevice_discovery.py @@ -36,12 +36,14 @@ def _coordinator(hass: HomeAssistant) -> LocalThingsCoordinator: return LocalThingsCoordinator(hass, entry) -async def _discover(coordinator: LocalThingsCoordinator, name: str) -> None: +async def _discover_with( + coordinator: LocalThingsCoordinator, resources: dict, oic_res, seeds: dict, +) -> None: """Run the same two-step sequence _async_update_data's first cycle does - (enumerate, then discover) against fixture data, without the polling/ - reconnect machinery around it -- see coordinator.py's - _enumerate_subdevices_blocking/_run_discovery.""" - resources, oic_res, seeds = _load_device_full(name) + (enumerate, then discover) against arbitrary resources/oic_res/seeds, + without the polling/reconnect machinery around it -- see coordinator.py's + _enumerate_subdevices_blocking/_run_discovery. `_discover` below is the + fixture-file-backed convenience wrapper most tests want.""" coordinator._session = FakeCoapSession(seeds) # _connect_session (skipped here -- the session is pre-set) is what # normally populates _identity via read_identity; set it directly with @@ -67,6 +69,12 @@ async def _discover(coordinator: LocalThingsCoordinator, name: str) -> None: coordinator._observe.apply(href, rep, source='poll') +async def _discover(coordinator: LocalThingsCoordinator, name: str) -> None: + """Fixture-file-backed convenience wrapper around _discover_with.""" + resources, oic_res, seeds = _load_device_full(name) + await _discover_with(coordinator, resources, oic_res, seeds) + + def _climate_bound(coordinator, subdevice_key: str): from custom_components.localthings.registry.subdevices import MAIN for b in coordinator.bound: @@ -79,10 +87,10 @@ def _climate_bound(coordinator, subdevice_key: str): # --------------------------------------------------------------------------- -# HJcom -- ARTIK051_DONGLE_FAC_18K, Pattern A (indexed siblings) +# Pattern A reporter -- ARTIK051_DONGLE_FAC_18K, indexed siblings # --------------------------------------------------------------------------- -async def test_hjcom_materializes_master_and_bedroom_subdevice(hass: HomeAssistant): +async def test_pattern_a_materializes_master_and_bedroom_subdevice(hass: HomeAssistant): coordinator = _coordinator(hass) await _discover(coordinator, 'airconditioner_artik051_dongle_fac_18k') @@ -96,11 +104,11 @@ async def test_hjcom_materializes_master_and_bedroom_subdevice(hass: HomeAssista assert sub1_climate.href == '/mode/vs/1' -async def test_hjcom_device_2_produces_no_entities_at_all(hass: HomeAssistant): - """HJcom's /device/2 is the unused SmartThings slot (DESIGN-177.md - section 4): it answers its seed with a full-shaped batch, but every - climate-state rep on it is empty. It must be recorded as skipped, not - materialized, and must contribute zero bound entities.""" +async def test_pattern_a_device_2_produces_no_entities_at_all(hass: HomeAssistant): + """The reporter's /device/2 is the unused SmartThings slot + (DESIGN-177.md section 4): it answers its seed with a full-shaped + batch, but every climate-state rep on it is empty. It must be recorded + as skipped, not materialized, and must contribute zero bound entities.""" coordinator = _coordinator(hass) await _discover(coordinator, 'airconditioner_artik051_dongle_fac_18k') @@ -130,7 +138,7 @@ async def test_hjcom_sub1_device_info_links_via_device_to_master(hass: HomeAssis # --------------------------------------------------------------------------- -# jhkwon19 -- TP2X_FAC_BORA_21K, Pattern B (UUID-prefixed tree) +# Issue #177's Pattern B reporter -- TP2X_FAC_BORA_21K, UUID-prefixed tree # --------------------------------------------------------------------------- _SUB_UUID = '6c2dff6d-ee5c-dad1-6a5e-000000000001' @@ -182,6 +190,230 @@ async def test_fac_bora_2in1_unique_ids_include_subdevice_prefix(hass: HomeAssis ) +# --------------------------------------------------------------------------- +# Same reporter and physical unit/UUID again -- issue #205, but this +# time //device/0 doesn't answer. Exercises enumerate_subdevices' +# per-href flat-probe fallback (registry/subdevices.py) against a real +# capture instead of the synthetic sessions test_subdevices.py uses. +# --------------------------------------------------------------------------- + +async def test_fac_bora_205_flat_fallback_finds_candidate_but_gate_holds_it_back( + hass: HomeAssistant, +): + """The fixture's only seeded UUID-prefixed href is /information/vs/0 -- + the one href ever actually confirmed live under this prefix (issue #177 + comment 5113518087) -- since nothing else has been confirmed yet for + this unit. That's enough for the flat-probe fallback to find a + candidate, but /information/vs/0 binds no entity on its own (it's only + ever read for device-type resolution, never bound as a capability), so + discover_partitioned's liveness gate correctly holds it back rather than + materializing a phantom climate card from unconfirmed hrefs. This is the + honest current state of issue #205, not a guess at its resolution.""" + coordinator = _coordinator(hass) + await _discover(coordinator, 'airconditioner_fac_bora_205_flat') + + assert coordinator.subdevices == [] + assert [s.subdevice.key for s in coordinator._skipped_subdevices] == [_SUB_UUID] + skipped = coordinator._skipped_subdevices[0].subdevice + assert skipped.kind == 'prefixed' + assert skipped.seed_path == () + assert skipped.flat_hrefs == ('/information/vs/0',) + + assert coordinator._subdevice_probes[f'/{_SUB_UUID}/device/0'] is False + assert coordinator._subdevice_probes[f'/{_SUB_UUID}/information/vs/0'] is True + + # Confirms the master itself is completely unaffected by its sibling's + # Collection endpoint not answering -- same guarantee every other + # subdevice test in this file relies on. + assert _climate_bound(coordinator, None) is not None + + +async def test_flat_subdevice_materializes_and_repolls_end_to_end(hass: HomeAssistant): + """Synthetic (not a real capture, unlike the fixture-driven test above) -- + exercises the one path nothing else covers: a flat-mode prefixed + subdevice with *enough* confirmed hrefs to actually pass + discover_partitioned's liveness gate and materialize a real climate + entity, then a subsequent _poll_subdevice_seed re-poll refreshing its + state all the way through to canonical_resources -- the path a real + resolution of issue #205 (once more hrefs are confirmed live for some + unit) would actually need. + + Also pins _poll_subdevice_flat_hrefs' hot/warm skip: climate-critical + hrefs (power/mode/temperature) land on the warm tier by discovery's own + rules, so they're already kept fresh every few seconds by _run_subpolls + -- re-fetching them again on this once-per-summary-poll pass would only + add GETs, not freshness, so they're the ones this method must skip. + /option/autoclean/vs/0 is cold-tier and is what actually needs this + path.""" + resources, oic_res, _real_seeds = _load_device_full('airconditioner_fac_bora_2in1') + seeds = { + # No (_SUB_UUID, 'device', '0') entry -- forces the flat fallback, + # same as the real issue #205 capture above, but this time with + # enough hrefs answering to actually materialize. power/mode/ + # temperature values copied verbatim from that fixture's own (real) + # Collection-batch seed, just served individually instead of + # batched, to isolate "does flat mode produce the same result as + # Collection mode" as the only variable. + f'/{_SUB_UUID}/power/vs/0': {'x.com.samsung.da.power': 'On'}, + f'/{_SUB_UUID}/mode/vs/0': { + 'x.com.samsung.da.supportedModes': ['Cool', 'Dry', 'Wind', 'Auto'], + 'x.com.samsung.da.modes': ['Cool'], + 'x.com.samsung.da.options': [], + }, + f'/{_SUB_UUID}/temperature/current/0': { + 'range': [18.0, 30.0], 'units': 'C', 'temperature': 26.0, + }, + f'/{_SUB_UUID}/temperature/desired/0': { + 'range': [18.0, 30.0], 'units': 'C', 'temperature': 24.0, + }, + # Cold-tier -- not covered by _run_subpolls, so this is the href + # that actually depends on _poll_subdevice_flat_hrefs to ever + # refresh at all. + f'/{_SUB_UUID}/option/autoclean/vs/0': { + 'x.com.samsung.da.settingStatus': 'Off', + }, + } + coordinator = _coordinator(hass) + await _discover_with(coordinator, resources, oic_res, seeds) + + assert [su.key for su in coordinator.subdevices] == [_SUB_UUID] + subdevice = coordinator.subdevices[0] + assert subdevice.seed_path == () + assert subdevice.flat_hrefs != () + + sub_climate = _climate_bound(coordinator, _SUB_UUID) + assert sub_climate is not None + + # Re-poll: a fresh reading under the prefix should reach + # canonical_resources through _poll_subdevice_seed's flat-mode branch, + # not just sit frozen at the one-time enumeration snapshot. + coordinator._session.seeds[f'/{_SUB_UUID}/temperature/current/0'] = { + 'range': [18.0, 30.0], 'units': 'C', 'temperature': 27.5, + } + coordinator._session.seeds[f'/{_SUB_UUID}/option/autoclean/vs/0'] = { + 'x.com.samsung.da.settingStatus': 'On', + } + refreshed = coordinator._poll_subdevice_seed(subdevice) + + # The warm-tier temperature href is skipped here -- already covered by + # _run_subpolls at a faster cadence -- so it does NOT show up refreshed + # through this path. + assert f'/{_SUB_UUID}/temperature/current/0' not in refreshed + assert refreshed == { + f'/{_SUB_UUID}/option/autoclean/vs/0': {'x.com.samsung.da.settingStatus': 'On'}, + } + + for href, rep in refreshed.items(): + coordinator._observe.apply(href, rep, source='poll') + res = coordinator.canonical_resources(subdevice) + assert res['/option/autoclean/vs/0']['x.com.samsung.da.settingStatus'] == 'On' + # Confirms the skip is about redundant re-fetching, not stale data -- + # the warm-tier value from initial discovery is still there, untouched. + assert res['/temperature/current/0']['temperature'] == 26.0 + + +class _FakeCollectionSession: + """Minimal session that only ever answers a Collection GET -- used to + prove the flat-mode re-poll path (issue #205) is only taken when + flat_hrefs is actually set, not whenever seed_path happens to be + unusual.""" + + def __init__(self, table): + self.table = table + self.calls: list[tuple[str, ...]] = [] + + def get(self, path, timeout=10.0): + self.calls.append(tuple(path)) + body = self.table.get(tuple(path)) + if body is None: + return 0x84, b'' + import cbor2 + return 0x45, cbor2.dumps(body) + + def pace(self): + pass + + +def test_poll_subdevice_seed_collection_mode_unaffected_by_flat_fallback( + hass: HomeAssistant, +): + """A subdevice with a working Collection endpoint (flat_hrefs empty) + keeps re-polling it with a single Collection GET, unchanged by issue + #205's fallback.""" + from custom_components.localthings.registry.subdevices import Subdevice + + coordinator = _coordinator(hass) + devcol_rep = {'rt': ['x.com.samsung.devcol', 'oic.wk.col']} + sess = _FakeCollectionSession({ + (_SUB_UUID, 'device', '0'): [ + devcol_rep, {'href': '/mode/vs/0', 'rep': {'mode': 'cool'}}, + ], + }) + coordinator._session = sess + subdevice = Subdevice(kind='prefixed', key=_SUB_UUID, seed_path=(_SUB_UUID, 'device', '0')) + + result = coordinator._poll_subdevice_seed(subdevice) + + assert result == {f'/{_SUB_UUID}/mode/vs/0': {'mode': 'cool'}} + assert sess.calls == [(_SUB_UUID, 'device', '0')] + + +def test_poll_subdevice_seed_flat_mode_polls_each_href_individually( + hass: HomeAssistant, +): + """A flat-mode subdevice (issue #205) has no Collection to batch-refresh + through, so each confirmed href is GET individually under the prefix on + every re-poll -- a href that stops answering just drops out, same + "never fail the master's poll over a sibling" posture as the Collection + path.""" + from custom_components.localthings.registry.subdevices import Subdevice + + coordinator = _coordinator(hass) + sess = _FakeCollectionSession({ + (_SUB_UUID, 'mode', 'vs', '0'): {'mode': 'cool'}, + # (_SUB_UUID, 'power', 'vs', '0') deliberately absent -> drops out. + }) + coordinator._session = sess + subdevice = Subdevice( + kind='prefixed', key=_SUB_UUID, seed_path=(), + flat_hrefs=('/mode/vs/0', '/power/vs/0'), + ) + + result = coordinator._poll_subdevice_seed(subdevice) + + assert result == {f'/{_SUB_UUID}/mode/vs/0': {'mode': 'cool'}} + assert sess.calls == [ + (_SUB_UUID, 'mode', 'vs', '0'), (_SUB_UUID, 'power', 'vs', '0'), + ] + + +def test_poll_subdevice_seed_flat_mode_skips_hrefs_covered_by_hot_warm_subpolls( + hass: HomeAssistant, +): + """A flat href already in the hot/warm sub-poll tiers is refreshed every + few seconds by _run_subpolls -- re-fetching it again on this + once-per-summary-poll pass would only add GETs, not freshness, so + _poll_subdevice_flat_hrefs must not even attempt it.""" + from custom_components.localthings.registry.subdevices import Subdevice + + coordinator = _coordinator(hass) + sess = _FakeCollectionSession({ + (_SUB_UUID, 'mode', 'vs', '0'): {'mode': 'cool'}, + (_SUB_UUID, 'power', 'vs', '0'): {'power': 'On'}, + }) + coordinator._session = sess + coordinator._warm_hrefs = [f'/{_SUB_UUID}/mode/vs/0'] + subdevice = Subdevice( + kind='prefixed', key=_SUB_UUID, seed_path=(), + flat_hrefs=('/mode/vs/0', '/power/vs/0'), + ) + + result = coordinator._poll_subdevice_seed(subdevice) + + assert result == {f'/{_SUB_UUID}/power/vs/0': {'power': 'On'}} + assert sess.calls == [(_SUB_UUID, 'power', 'vs', '0')] + + async def test_multidevice_probe_never_reaches_discovery_or_the_cache( hass: HomeAssistant, ): diff --git a/tests/test_subdevices.py b/tests/test_subdevices.py index ea698c5..38d2f72 100644 --- a/tests/test_subdevices.py +++ b/tests/test_subdevices.py @@ -206,6 +206,9 @@ class _FakeSession: return 0x84, b'' return 0x45, cbor2.dumps(body) + def pace(self): + pass + _DEVCOL_REP = {'rt': ['x.com.samsung.devcol', 'oic.wk.col']} @@ -264,6 +267,102 @@ def test_enumerate_prefixed_from_subdevice_id_list(): assert extra == {f'/{_UUID}/mode/vs/0': {'m': 'cool'}} +def test_enumerate_prefixed_falls_back_to_flat_hrefs_when_device0_collection_is_empty(): + """issue #205: not every prefixed subdevice exposes its own + //device/0 Collection -- not even TP2X_FAC_BORA_21K, the board + this pattern was built against, always does. When it doesn't, + enumeration probes every href the master itself answered this cycle, + individually, under the UUID prefix, and keeps whichever ones answer.""" + resources = { + '/subdevices/vs/0': {'x.com.samsung.da.subdeviceIdList': [_UUID]}, + '/mode/vs/0': {'m': 'cool'}, + '/power/vs/0': {'p': 'On'}, + } + sess = _FakeSession({ + # (_UUID, 'device', '0') deliberately absent -> Collection probe fails. + (_UUID, 'mode', 'vs', '0'): {'mode': 'cool'}, + # (_UUID, 'power', 'vs', '0') deliberately absent -> drops out. + }) + subdevices, extra = enumerate_subdevices(sess, resources, oic_res_links=[]) + assert len(subdevices) == 1 + subdevice = subdevices[0] + assert (subdevice.kind, subdevice.key) == ('prefixed', _UUID) + assert subdevice.seed_path == () + assert subdevice.flat_hrefs == ('/mode/vs/0',) + assert extra == {f'/{_UUID}/mode/vs/0': {'mode': 'cool'}} + + +def test_enumerate_prefixed_flat_fallback_materializes_nothing_when_no_href_answers(): + """Same posture as every other candidate check in this module: nothing + answering means no candidate, not a crash.""" + resources = { + '/subdevices/vs/0': {'x.com.samsung.da.subdeviceIdList': [_UUID]}, + '/mode/vs/0': {'m': 'cool'}, + } + subdevices, extra = enumerate_subdevices(_FakeSession({}), resources, oic_res_links=[]) + assert subdevices == [] + assert extra == {} + + +def test_enumerate_prefixed_flat_fallback_probe_log_reports_every_href_tried(): + resources = { + '/subdevices/vs/0': {'x.com.samsung.da.subdeviceIdList': [_UUID]}, + '/mode/vs/0': {'m': 'cool'}, + } + sess = _FakeSession({ + (_UUID, 'mode', 'vs', '0'): {'mode': 'cool'}, + }) + probes: dict[str, bool] = {} + enumerate_subdevices(sess, resources, oic_res_links=[], probe_log=probes.__setitem__) + assert probes[f'/{_UUID}/device/0'] is False + assert probes[f'/{_UUID}/mode/vs/0'] is True + + +def test_enumerate_prefixed_flat_fallback_with_no_master_hrefs_to_probe_is_a_no_op(): + """The master itself having nothing but /subdevices/vs/0 in its own + resources this cycle (e.g. a very first, mostly-empty poll) must not + crash the fallback loop -- the only href in `resources` is + /subdevices/vs/0 itself, which the fake session doesn't answer under + the prefix either, so nothing materializes.""" + resources = { + '/subdevices/vs/0': {'x.com.samsung.da.subdeviceIdList': [_UUID]}, + } + subdevices, extra = enumerate_subdevices(_FakeSession({}), resources, oic_res_links=[]) + assert subdevices == [] + assert extra == {} + + +def test_enumerate_prefixed_flat_fallback_does_not_cross_contaminate_a_second_uuid(): + """Two prefixed candidates in the same subdeviceIdList, one whose + Collection endpoint works and one that needs the flat fallback -- each + must end up with only its own hrefs, no bleed between them.""" + uuid_a, uuid_b = _UUID, '11111111-1111-1111-1111-111111111111' + resources = { + '/subdevices/vs/0': {'x.com.samsung.da.subdeviceIdList': [uuid_a, uuid_b]}, + '/mode/vs/0': {'m': 'cool'}, + } + sess = _FakeSession({ + (uuid_a, 'device', '0'): [ + _DEVCOL_REP, {'href': '/mode/vs/0', 'rep': {'m': 'a-collection'}}, + ], + # uuid_b's Collection deliberately absent -> falls back to the flat + # per-href probe. + (uuid_b, 'mode', 'vs', '0'): {'m': 'b-flat'}, + }) + subdevices, extra = enumerate_subdevices(sess, resources, oic_res_links=[]) + + by_key = {u.key: u for u in subdevices} + assert set(by_key) == {uuid_a, uuid_b} + assert by_key[uuid_a].seed_path == (uuid_a, 'device', '0') + assert by_key[uuid_a].flat_hrefs == () + assert by_key[uuid_b].seed_path == () + assert by_key[uuid_b].flat_hrefs == ('/mode/vs/0',) + assert extra == { + f'/{uuid_a}/mode/vs/0': {'m': 'a-collection'}, + f'/{uuid_b}/mode/vs/0': {'m': 'b-flat'}, + } + + def test_enumerate_prefixed_tolerates_redacted_string_id_list(): """subdeviceIdList matches redact.py's 'deviceid' substring rule, and the real airconditioner_fac_bora_device.json fixture carries the literal @@ -284,10 +383,10 @@ def test_enumerate_no_subdevices_resource_at_all(): def test_enumerate_indexed_materializes_candidate_regardless_of_content(): """enumerate_subdevices itself has no way to tell a real sibling from an - unused slot that merely answers the same shape (HJcom's /device/2) -- - that's discover_partitioned's job (see its own tests below). A - same-shaped batch of otherwise-empty reps is still returned as a - candidate here.""" + unused slot that merely answers the same shape (the reporter's + /device/2) -- that's discover_partitioned's job (see its own tests + below). A same-shaped batch of otherwise-empty reps is still returned + as a candidate here.""" oic_res = [{'di': 'a', 'links': [{'href': '/device/2'}]}] sess = _FakeSession({ ('device', '2'): [_DEVCOL_REP, {'href': '/power/vs/2', 'rep': {}}, @@ -425,8 +524,8 @@ def test_discover_partitioned_main_pass_excludes_subdevice_hrefs_from_unbound(): def test_discover_partitioned_subdevice_resolves_its_own_registry(): """A subdevice reporting its own /information/vs/0 resolves its own - device type (jhkwon19's wall subdevice: TP2X_FAC_BORA_RAC_21K -> RAC -> - airconditioner) independent of the master's.""" + device type (issue #177's real wall subdevice: TP2X_FAC_BORA_RAC_21K -> + RAC -> airconditioner) independent of the master's.""" main_cap = Capability(href='/mode/vs/0', entities=(BinarySensorDesc(key='m', field='x'),)) sub_cap = Capability(href='/mode/vs/0', entities=(BinarySensorDesc(key='m2', field='x'),)) main_reg = _FakeRegistry('main_type', {'/mode/vs/0': [main_cap]}) @@ -499,13 +598,14 @@ def test_discover_partitioned_no_subdevices_matches_plain_discover(): # --------------------------------------------------------------------------- # discover_partitioned's materialization gate: a candidate is only kept if # it produced at least one *primary* (no entity_category) bound entity whose -# flattened value isn't None. This is what tells HJcom's real /device/1 -# sibling apart from the unused /device/2 slot that answers the same shape. +# flattened value isn't None. This is what tells the reporter's real +# /device/1 sibling apart from the unused /device/2 slot that answers the +# same shape. # --------------------------------------------------------------------------- def test_discover_partitioned_skips_candidate_with_no_live_primary_entity(): """A candidate whose only populated entity is diagnostic-category - doesn't count -- exactly HJcom's /device/2 shape (an alarm_code + doesn't count -- exactly the reporter's /device/2 shape (an alarm_code sensor reading something even though the subdevice itself is empty).""" diag_cap = Capability( href='/alarms/vs/0', @@ -533,7 +633,7 @@ def test_discover_partitioned_skips_candidate_with_no_live_primary_entity(): def test_discover_partitioned_materializes_candidate_with_live_primary_entity(): """A candidate with a populated *primary* (no entity_category) entity is materialized, even alongside an all-empty diagnostic sibling href -- - exactly HJcom's /device/1 shape.""" + exactly the reporter's /device/1 shape.""" climate_cap = Capability( href='/mode/vs/0', entities=(BinarySensorDesc(key='mode', field='m'),), # no entity_category -> primary