diff --git a/.claude/skills/adding-device-support/SKILL.md b/.claude/skills/adding-device-support/SKILL.md index 78a4ff9..df717c2 100644 --- a/.claude/skills/adding-device-support/SKILL.md +++ b/.claude/skills/adding-device-support/SKILL.md @@ -10,9 +10,9 @@ description: >- OCF-standard vs vendor hrefs, the diagnostic/config/normal entity taxonomy, preferring dynamic (device-reported) select options over hardcoded lists, ensuring every href is bound or ignored, and locking it in with a fixture + - golden + test. Also covers multi-unit ("composite") appliances that expose - several logical indoor units over one IP — triaging a missing second unit, - and why registry hrefs stay canonical rather than indexed. + golden + test. Also covers multi-subdevice ("composite") appliances that + expose several logical indoor subdevices over one IP — triaging a missing + second subdevice, and why registry hrefs stay canonical rather than indexed. --- # Adding device support @@ -27,21 +27,21 @@ dump into coverage. A user's diagnostics download (`config_entry-localthings-*.json`) has, under `data`: - `resources`: `{href: rep}` — the parsed `/device/0` snapshot. **This is the - source of truth**, not code comments. On a multi-unit appliance this is the - unit the config entry connects to and *only* that unit; siblings report - their own (see below). + source of truth**, not code comments. On a multi-subdevice appliance this is + the subdevice the config entry connects to and *only* that subdevice; + siblings report their own (see below). - `unbound_hrefs`: resources that bound to no capability. The "incomplete capability coverage" repair fires whenever this is **non-empty or the device type is unrecognized** (`coordinator._update_coverage_gap_issue`). -Multi-unit appliances (one IP, one DTLS session, several logical indoor -units — issue #177) add four more, all absent/empty on an ordinary device: -- `sub_units`: one entry per materialized sibling — `kind`/`key`/`seed_path`, +Multi-subdevice appliances (one IP, one DTLS session, several logical indoor +subdevices — issue #177) add four more, all absent/empty on an ordinary device: +- `subdevices`: one entry per materialized sibling — `kind`/`key`/`seed_path`, its own `model`, its bound hrefs, and its own `resources`. -- `sub_units_skipped`: candidates whose seed answered but that produced no +- `subdevices_skipped`: candidates whose seed answered but that produced no live primary state, with the reps the gate actually judged. An unused - SmartThings slot lands here, not in `sub_units`. -- `sub_unit_probes`: `{seed_href: found}` for every seed attempted — tells + SmartThings slot lands here, not in `subdevices`. +- `subdevice_probes`: `{seed_href: found}` for every seed attempted — tells "checked, nothing there" apart from "never checked". - `multidevice`: `/multidevice/vs/0`'s rep if the board answers it. Its `numofsubdevice` is a corroborating count, not a gate. @@ -77,11 +77,11 @@ print('state_keys:', sorted(state)) `exists_fn` and produces the final entity values. Use the same routine to regenerate a golden. -**A sibling unit's block runs through this unchanged.** `sub_units[i].resources` -(and `sub_units_skipped[i].resources`) are keyed by *canonical* hrefs — -`/mode/vs/0`, never the `/mode/vs/1` or `//mode/vs/0` that unit actually -answers on — precisely so you can paste one into `resources` above and read the -result exactly like the master's. No de-indexing by hand. +**A sibling subdevice's block runs through this unchanged.** `subdevices[i].resources` +(and `subdevices_skipped[i].resources`) are keyed by *canonical* hrefs — +`/mode/vs/0`, never the `/mode/vs/1` or `//mode/vs/0` that subdevice +actually answers on — precisely so you can paste one into `resources` above and +read the result exactly like the master's. No de-indexing by hand. ## 3. Route the device to a registry — add a row, never a branch @@ -217,7 +217,7 @@ entity on a hunch (`ignored.py`'s rule). 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 -`subunits.enumerate_sub_units` probes `/device/`, `//device/0` 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 @@ -307,15 +307,15 @@ friendlier href**. that another binds, scope the ignore to that family's registry. - **Registry hrefs are always canonical — never index or prefix one.** On a - multi-unit appliance, `unbound_hrefs` reports the *real* href a gap was seen - on, so a sibling's gap shows up as `/foo/vs/1` or + multi-subdevice appliance, `unbound_hrefs` reports the *real* href a gap was + seen on, so a sibling's gap shows up as `/foo/vs/1` or `//foo/vs/0`. Do **not** write `Capability(href='/foo/vs/1')` for it. - Binding runs against each unit's canonical view, so an indexed or prefixed - href in a registry matches nothing on any device and fails silently — no - error, no entity, and the gap stays open. Fix it on the `/foo/vs/0` form and - every unit gets it at once. (`registry/subunits.py` owns the canonical ⇄ - actual translation; nothing under `capabilities/` or `by_type/` should ever - mention a unit index.) + Binding runs against each subdevice's canonical view, so an indexed or + prefixed href in a registry matches nothing on any device and fails + silently — no error, no entity, and the gap stays open. Fix it on the + `/foo/vs/0` form and every subdevice gets it at once. + (`registry/subdevices.py` owns the canonical ⇄ actual translation; nothing + under `capabilities/` or `by_type/` should ever mention a subdevice index.) ## 9. Reuse before writing new code @@ -332,7 +332,7 @@ shared module rather than copying. (`{"device0": [ {devcol rep}, {href, rep}, ... ]}`) — replace serials, MACs, and other PII with placeholders. - A multi-unit dump (issue #177) may carry three more top-level keys, all + A multi-subdevice dump (issue #177) may carry three more top-level keys, all optional and defaulted for every other fixture — load them with `conftest._load_device_full` rather than `_load_device`: - `oic_res`: the raw `/oic/res` link array, which is what enumeration reads @@ -346,9 +346,10 @@ shared module rather than copying. constructed. A fixture that quietly mixes the two is worse than no fixture: the whole point of the corpus is that it records what hardware actually did. 2. Generate `tests/fixtures/golden/.json` (`{"state_keys": [...]}`) with - the harness in §2. A multi-unit fixture's golden carries a sibling's keys - under a prefix (`unit1_climate`, `sub__climate`) alongside the - unprefixed master keys — that's the entity-ID namespacing, not a bug. + the harness in §2. A multi-subdevice fixture's golden carries a sibling's + keys under a prefix (`subdevice1_climate`, `subdevice__climate`) + alongside the unprefixed master keys — that's the entity-ID namespacing, + not a bug. The master's keys are unprefixed *by design* and must never gain one: that's what keeps every pre-#177 device's `unique_id` stable. 3. Add the type to `test_golden_regression.py` and write a @@ -363,31 +364,32 @@ 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. -## 11. Triage: "one of my units is missing" +## 11. Triage: "one of my subdevices is missing" -For an appliance that exposes several logical indoor units over one IP — +For an appliance that exposes several logical indoor subdevices over one IP — a 2-in-1 air conditioner, plausibly a multi-drum washer (#19). Work down the dump in this order; each step rules out a different cause. -1. **`sub_unit_probes`** — did we even look? Every seed attempted appears +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. **`sub_units_skipped`** — did we find it and reject it? A candidate lands +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 unit is an unused slot and the + 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(sub_units) + 1` is a strong hint, + 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 `/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 units, is the interesting - case — that's a third mechanism and needs a new dump, not a code guess. + 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. Two things that are *not* the fix: adding a capability for an indexed href (see §8), and loosening the liveness gate to "any populated entity" — a @@ -395,8 +397,8 @@ rejected slot routinely reports a non-`None` *diagnostic* value off an empty resource, which is exactly what the primary-entity filter exists to ignore. ## Key files -- `registry/subunits.py` — `SubUnit`, enumeration, canonical ⇄ actual href - translation, and the materialization gate for multi-unit appliances. +- `registry/subdevices.py` — `Subdevice`, enumeration, canonical ⇄ actual href + translation, and the materialization gate for multi-subdevice appliances. - `registry/discovery.py` — `discover()`, unbound reporting, pattern caps. - `registry/capability.py`, `registry/entities.py` — the `Capability` and descriptor shapes (`rt_filter`, `match_fn`, `exists_fn`, `rep_fn`, `write_fn`). diff --git a/README.md b/README.md index 173fc3e..7aad706 100644 --- a/README.md +++ b/README.md @@ -185,16 +185,16 @@ Samsung's firmware occasionally drops the DTLS session briefly — this is norma If reconnects become persistent (more than a handful per minute), something's actually wrong. Check the appliance's Wi-Fi link first, then look for a competing DTLS client on the LAN — only one active session per appliance is allowed at a time. -### Multi-indoor-unit ("2-in-1") air conditioner systems +### Multi-subdevice ("2-in-1") air conditioner systems -Some Samsung installs run more than one indoor unit off a single outdoor unit, all reachable over the *one* IP/DTLS session your config entry connects to (a floor-standing + wall-mounted 2-in-1 is a common shape). The integration discovers any sibling units automatically, once, right after the first successful poll — there's nothing to configure. Each discovered unit gets its own HA device (linked to the main one via "via device") and its own `climate` card, so it lands in its own room in the dashboard instead of being invisible or mixed into the master unit's state. +Some Samsung installs run more than one indoor subdevice off a single outdoor unit, all reachable over the *one* IP/DTLS session your config entry connects to (a floor-standing + wall-mounted 2-in-1 is a common shape). The integration discovers any sibling subdevices automatically, once, right after the first successful poll — there's nothing to configure. Each discovered subdevice gets its own HA device (linked to the main one via "via device") and its own `climate` card, so it lands in its own room in the dashboard instead of being invisible or mixed into the master's state. Two on-the-wire shapes are supported, both keyed off what the appliance itself reports: - **Indexed siblings** — the device answers a `/device/1`, `/device/2`, ... collection alongside its own `/device/0`, mirroring every resource at that index. - **UUID-prefixed tree** — the device reports a sibling's id in `x.com.samsung.da.subdeviceIdList`, and that id doubles as a literal href prefix for the sibling's own resource tree. -A candidate that answers but never produces any real, user-facing state (an unused slot some installs report alongside a genuine second unit) is silently skipped rather than turned into a phantom entity — check diagnostics' `sub_units`/`sub_units_skipped` blocks if a unit you expect to see isn't showing up, and file an issue with that diagnostics download attached. +A candidate that answers but never produces any real, user-facing state (an unused slot some installs report alongside a genuine second subdevice) is silently skipped rather than turned into a phantom entity — check diagnostics' `subdevices`/`subdevices_skipped` blocks if a subdevice you expect to see isn't showing up, and file an issue with that diagnostics download attached. --- diff --git a/custom_components/localthings/climate.py b/custom_components/localthings/climate.py index 9c622ee..63f8ae4 100644 --- a/custom_components/localthings/climate.py +++ b/custom_components/localthings/climate.py @@ -278,10 +278,10 @@ class LocalThingsClimate(LocalThingsEntity, ClimateEntity): silently reintroducing the drift this delegation exists to prevent. Reads the actual href through self._rep rather than - coordinator.resource() directly -- on a sub-unit (a legacy-board + coordinator.resource() directly -- on a subdevice (a legacy-board sibling has its own /airflow/vs/1, or //airflow/vs/0), the canonical AIRFLOW_HREF must be translated through this bound - entity's own sub_unit first, exactly like every other sibling read + entity's own subdevice first, exactly like every other sibling read below. """ if not is_legacy_board(self._resources): @@ -297,23 +297,23 @@ class LocalThingsClimate(LocalThingsEntity, ClimateEntity): the preset read (and write) over to the token path. Deliberately reads the *raw* href (translated through this bound - entity's own sub_unit, not through self._rep) rather than going + entity's own subdevice, not through self._rep) rather than going through _rep's own CONVENIENT_HREF fallback branch -- that fallback is exactly the legacy_convenient() rep this method is deciding whether to use, so routing through it here would make the resource never look empty and this always resolve to the wrong side. """ - convenient_href = self._bound.sub_unit.to_actual(CONVENIENT_HREF) + convenient_href = self._bound.subdevice.to_actual(CONVENIENT_HREF) return (not self.coordinator.resource(convenient_href) and bool(self._legacy_airflow())) def _rep(self, href: str) -> dict: """`href` is one of this module's canonical HREF_* constants -- - translated through this bound entity's own sub_unit (issue #177) to + translated through this bound entity's own subdevice (issue #177) to the real, on-the-wire href before the single-href cache lookup - (identity for MAIN, so a device with no sub-units reads exactly the + (identity for MAIN, so a device with no subdevices reads exactly the href it always did).""" - rep = self.coordinator.resource(self._bound.sub_unit.to_actual(href)) or {} + rep = self.coordinator.resource(self._bound.subdevice.to_actual(href)) or {} if not rep and href == CONVENIENT_HREF and self._legacy_airflow(): return self._legacy_convenient() return rep diff --git a/custom_components/localthings/coordinator.py b/custom_components/localthings/coordinator.py index c18acef..d2c0899 100644 --- a/custom_components/localthings/coordinator.py +++ b/custom_components/localthings/coordinator.py @@ -34,8 +34,8 @@ from .registry.discovery import BoundEntity from .registry import CAPABILITIES from .registry.adapter import flatten from .registry.identity import read_identity, DeviceIdentity -from .registry.subunits import ( - MAIN, SubUnit, canonical_view, discover_partitioned, enumerate_sub_units, +from .registry.subdevices import ( + MAIN, Subdevice, canonical_view, discover_partitioned, enumerate_subdevices, normalize_seed_batch, ) from .observe import ObserveManager, MODE_OBSERVE, MODE_POLL, GRACE_PERIOD_S @@ -165,35 +165,35 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): self._identity: DeviceIdentity | None = None self._discovered = False self.bound = [] - # Sibling indoor units discovered on this connection (issue #177) -- - # candidates set once, at first discovery, by - # _enumerate_sub_units_blocking; narrowed by _run_discovery to the + # Sibling indoor subdevices discovered on this connection (issue + # #177) -- candidates set once, at first discovery, by + # _enumerate_subdevices_blocking; narrowed by _run_discovery to the # ones that actually produced live primary state (see - # subunits.discover_partitioned). MAIN itself is never in this list - # (see subunits.canonical_view's docstring for why that's safe): - # it's the *other* units sharing this DTLS session, if any. - self.sub_units: list[SubUnit] = [] + # subdevices.discover_partitioned). MAIN itself is never in this list + # (see subdevices.canonical_view's docstring for why that's safe): + # 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. - self._skipped_sub_units: list = [] + self._skipped_subdevices: list = [] # Those rejected candidates' raw reps, kept aside for diagnostics - # only (see _live_unit_resources). They are deliberately not in the + # only (see _live_subdevice_resources). They are deliberately not in the # state cache: nothing polls them again, so anything applied there # would sit frozen at its first-discovery value while looking as # live as every other href in `last_resources`. - self._skipped_sub_unit_resources: dict[str, dict] = {} - # /multidevice/vs/0's rep, if this board answers it -- a plain unit - # count that corroborates the liveness gate without deciding it. - # Deliberately outside `resources`; see _enumerate_sub_units_blocking. + self._skipped_subdevice_resources: dict[str, dict] = {} + # /multidevice/vs/0's rep, if this board answers it -- a plain + # subdevice count that corroborates the liveness gate without deciding it. + # Deliberately outside `resources`; see _enumerate_subdevices_blocking. self._multidevice: dict = {} - # What each sub-unit probe found, keyed by the seed href attempted -- + # What each subdevice probe found, keyed by the seed href attempted -- # surfaced in diagnostics so a report can tell "checked, nothing # there" apart from "never checked" (the same posture the # speculative-probe code this replaced documented in identity.py). - self._sub_unit_probes: dict[str, bool] = {} - # canonical_resources() memo, keyed by (sub_unit.kind, sub_unit.key). + self._subdevice_probes: dict[str, bool] = {} + # canonical_resources() memo, keyed by (subdevice.kind, subdevice.key). # Invalidated in _on_cache_changed -- climate.py reads this on every # property access (is_legacy_board and friends), so it must not # rebuild an O(hrefs) view from scratch on every single property. @@ -234,13 +234,13 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): direct O(1) cache lookup.""" return self._cache.get(href) or {} - def canonical_resources(self, sub_unit: SubUnit) -> dict[str, dict]: - """`sub_unit`'s own view of the live snapshot, rewritten into the + def canonical_resources(self, subdevice: Subdevice) -> dict[str, dict]: + """`subdevice`'s own view of the live snapshot, rewritten into the canonical hrefs (issue #177) the registry/platforms are written - against -- see subunits.canonical_view. A platform property that + against -- see subdevices.canonical_view. A platform property that needs the *whole* resources dict (as opposed to one href via `resource()`/`last_resources.get(href)`) must use this instead of - `last_resources`, or a sibling unit's own `/mode/vs/1` would leak + `last_resources`, or a sibling subdevice's own `/mode/vs/1` would leak into MAIN's canonical `/mode/vs/0` view (or vice versa) under exists_fn/is_legacy_board-style checks that scan the whole dict. @@ -249,31 +249,31 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): O(hrefs) -- _on_cache_changed clears the memo whenever the snapshot actually changes, not on every property access. """ - view_key = (sub_unit.kind, sub_unit.key) + view_key = (subdevice.kind, subdevice.key) cached = self._canonical_cache.get(view_key) if cached is not None: return cached - view = canonical_view(sub_unit, self._cache.snapshot(), self.sub_units) + view = canonical_view(subdevice, self._cache.snapshot(), self.subdevices) self._canonical_cache[view_key] = view return view - def device_info_for(self, sub_unit: SubUnit) -> DeviceInfo: - """DeviceInfo for one logical unit sharing this connection (issue - #177) -- the master's own (unchanged) device_info for MAIN, or a - linked child device for a discovered sub-unit. + def device_info_for(self, subdevice: Subdevice) -> DeviceInfo: + """DeviceInfo for one logical subdevice sharing this connection + (issue #177) -- the master's own (unchanged) device_info for MAIN, or + a linked child device for a discovered subdevice. Identifiers derive from the *master's* serial (device_serial) plus - this unit's stable key, never from whatever serial the sub-unit - itself reports (or fails to) -- deterministic across reconnects - whether or not this unit's own identity resource + this subdevice's stable key, never from whatever serial the + subdevice itself reports (or fails to) -- deterministic across + reconnects whether or not this subdevice's own identity resource (/information/vs/, or //information/vs/0) answered on the poll that first created the HA device. `serial_number` is set from that resource when present anyway -- it's informational, not an identifier. """ - if sub_unit.kind == 'main': + if subdevice.kind == 'main': return self.device_info - info = self.canonical_resources(sub_unit).get('/information/vs/0', {}) + info = self.canonical_resources(subdevice).get('/information/vs/0', {}) model_num = info.get('x.com.samsung.da.modelNum', '') model = model_num.split('|', 1)[0] if model_num else '' serial = info.get('x.com.samsung.da.serialNum') or None @@ -281,15 +281,18 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): if model: label = model.replace('_', ' ').title() else: - # This poll never got (or never will get) the sub-unit's own - # identity resource -- fall back to a generic per-unit label - # rather than leaving the device unnamed. 'Unit ' only makes - # sense for an indexed unit (the key is a small ordinal); - # jhkwon19-pattern (prefixed) units are never more than one per - # connection today, so there's no ordinal to show. - label = f'Unit {sub_unit.key}' if sub_unit.kind == 'indexed' else 'Secondary Unit' + # This poll never got (or never will get) the subdevice's own + # 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. + label = ( + f'Subdevice {subdevice.key}' if subdevice.kind == 'indexed' + else 'Secondary Subdevice' + ) return DeviceInfo( - identifiers={(DOMAIN, f"{self.device_serial}_{sub_unit.key}")}, + identifiers={(DOMAIN, f"{self.device_serial}_{subdevice.key}")}, via_device=(DOMAIN, self.device_serial), name=f"{base_name} {label}", manufacturer=self.device_info.get('manufacturer') or 'Samsung', @@ -394,17 +397,17 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): except Exception as e: raise RuntimeError(f"poll cbor decode: {e}") from e result = parse_device0_batch(body) if isinstance(body, list) else {} - # Refresh every already-enumerated sibling unit's seed collection on - # this same summary poll (issue #177) -- without this, a sub-unit's + # Refresh every already-enumerated sibling subdevice's seed collection + # on this same summary poll (issue #177) -- without this, a subdevice's # climate card would show only its enumeration-time snapshot forever. - for sub_unit in self.sub_units: - result.update(self._poll_sub_unit_seed(sub_unit)) + for subdevice in self.subdevices: + result.update(self._poll_subdevice_seed(subdevice)) return result - def _poll_sub_unit_seed(self, sub_unit: SubUnit) -> dict[str, dict]: - """GET one sub-unit's seed Collection and return its batch, + def _poll_subdevice_seed(self, subdevice: Subdevice) -> dict[str, dict]: + """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 unit must not go unavailable + 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.""" @@ -412,13 +415,13 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): if sess is None: return {} try: - code, payload = sess.get(list(sub_unit.seed_path), timeout=10.0) + code, payload = sess.get(list(subdevice.seed_path), timeout=10.0) if code == 0x45 and payload: body = cbor2.loads(payload) if isinstance(body, list): - return normalize_seed_batch(sub_unit, parse_device0_batch(body)) + return normalize_seed_batch(subdevice, parse_device0_batch(body)) except Exception as e: - self._log.debug("sub-unit %s seed poll failed: %s", sub_unit.key, e) + self._log.debug("subdevice %s seed poll failed: %s", subdevice.key, e) return {} def _poll_hrefs_blocking(self, hrefs: list[str]) -> dict[str, dict]: @@ -478,18 +481,18 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): # Discovery (runs once on first successful poll) # ------------------------------------------------------------------ - def _enumerate_sub_units_blocking(self, resources: dict[str, dict]) -> dict[str, dict]: - """One-time (first discovery only) probe for sibling indoor units + def _enumerate_subdevices_blocking(self, resources: dict[str, dict]) -> dict[str, dict]: + """One-time (first discovery only) probe for sibling indoor subdevices sharing this connection (issue #177) -- see - registry.subunits.enumerate_sub_units for the two detection + registry.subdevices.enumerate_subdevices for the two detection patterns. Blocking -- runs in executor, under the session lock (shares the same DTLS session _poll_once just used this cycle). - Sets self.sub_units to every *candidate* the probes turned up - (self._sub_unit_probes as a side effect too) and returns `resources` + Sets self.subdevices to every *candidate* the probes turned up + (self._subdevice_probes as a side effect too) and returns `resources` merged with whatever each candidate's seed returned, so this cycle's _run_discovery sees every candidate's state without a second poll - round trip. `_run_discovery` is what narrows self.sub_units down to + 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. @@ -501,12 +504,12 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): return resources oic_res = self._identity.raw.get('/oic/res', []) if self._identity else [] probes: dict[str, bool] = {} - sub_units, extra = enumerate_sub_units( + subdevices, extra = enumerate_subdevices( sess, resources, oic_res, probe_log=lambda href, found: probes.__setitem__(href, found), ) - self.sub_units = sub_units - self._sub_unit_probes = probes + self.subdevices = subdevices + self._subdevice_probes = probes # /multidevice/vs/0 is corroborating metadata, not appliance state, # and it is probed on *every* device -- so it must not join the # returned resources dict. Two things go wrong if it does. It would @@ -516,31 +519,31 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): # or fridge whose firmware happens to answer it. And it is fetched # once here and never polled again, so applying it to the state # cache would freeze it there exactly like a rejected candidate's - # reps (see _live_unit_resources). Kept aside for diagnostics and + # reps (see _live_subdevice_resources). Kept aside for diagnostics and # for the numofsubdevice cross-check in _run_discovery instead. self._multidevice = extra.pop('/multidevice/vs/0', {}) return {**resources, **extra} - def _live_unit_resources(self, resources: dict[str, dict]) -> dict[str, dict]: - """`resources` minus every href belonging to a candidate sub-unit the + def _live_subdevice_resources(self, resources: dict[str, dict]) -> dict[str, dict]: + """`resources` minus every href belonging to a candidate subdevice the liveness gate rejected (issue #177). Called once, between _run_discovery and the first cache apply, so a rejected slot's reps are seen by the gate and then dropped rather than frozen into the cache forever -- see the call site. The reps - themselves are kept in _skipped_sub_unit_resources for diagnostics, + themselves are kept in _skipped_subdevice_resources for diagnostics, which is the only thing that still wants them. """ - if not self._skipped_sub_units: + if not self._skipped_subdevices: return resources kept: dict[str, dict] = {} skipped: dict[str, dict] = {} for href, rep in resources.items(): bucket = skipped if any( - skip.sub_unit.owns(href) for skip in self._skipped_sub_units + skip.subdevice.owns(href) for skip in self._skipped_subdevices ) else kept bucket[href] = rep - self._skipped_sub_unit_resources = skipped + self._skipped_subdevice_resources = skipped return kept def _run_discovery(self, resources: dict[str, dict]) -> None: @@ -565,28 +568,28 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): description = info.get('x.com.samsung.da.description', '') # Partitioned discovery (issue #177): the main pass binds every href - # owned by no sub-unit; one further pass per *candidate* sub-unit + # owned by no subdevice; one further pass per *candidate* subdevice # binds its own canonical view, resolving its own device type from # its own /information/vs/0 when it reports one and falling back to - # the master's registry otherwise. See subunits.discover_partitioned + # 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 - # self.sub_units below is narrowed to the ones that passed, not - # every candidate _enumerate_sub_units_blocking found. For a device - # with no candidates (self.sub_units == []) this is exactly the + # 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 # single discover() call this method used to make. bound, device_type_name, materialized, skipped = discover_partitioned( - resources, self.sub_units, resolve_registry, CAPABILITIES, + resources, self.subdevices, resolve_registry, CAPABILITIES, log=unbound.append, tier_log=_tier_log, ) - self.sub_units = materialized - self._skipped_sub_units = skipped + self.subdevices = materialized + self._skipped_subdevices = skipped for skip in skipped: self._log.info( - "sub-unit %s (%s) answered its seed but produced no live " + "subdevice %s (%s) answered its seed but produced no live " "primary state; not materialized (hrefs=%s)", - skip.sub_unit.key, skip.sub_unit.kind, list(skip.hrefs), + 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 @@ -601,12 +604,12 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): reported = int(numofsubdevice) except (TypeError, ValueError): reported = None - unit_count = len(materialized) + 1 # +1 for the master itself - if reported is not None and reported != unit_count: + subdevice_count = len(materialized) + 1 # +1 for the master itself + if reported is not None and reported != subdevice_count: self._log.debug( "/multidevice/vs/0 reports numofsubdevice=%r but %d " - "unit(s) materialized (including the master)", - numofsubdevice, unit_count, + "subdevice(s) materialized (including the master)", + numofsubdevice, subdevice_count, ) if device_type_name is not None: self._log.debug("device type: %s (modelNum=%r)", device_type_name, model_num) @@ -646,9 +649,9 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): self._discovered = True self._log.info( - "discovered %d entities (serial=%s) hot=%s warm=%s sub_units=%s", + "discovered %d entities (serial=%s) hot=%s warm=%s subdevices=%s", len(bound), serial, self._hot_hrefs, self._warm_hrefs, - [su.key for su in self.sub_units], + [su.key for su in self.subdevices], ) def _update_coverage_gap_issue( @@ -803,15 +806,15 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): if not self._discovered: # One-time (issue #177): find out whether this connection has - # sibling indoor units before the first discovery pass, and fold - # their seed resources into this cycle's snapshot so discovery - # sees every unit's state on the very first poll rather than - # waiting a cycle. Runs under its own session-lock scope (the - # poll above already released the lock) since it shares the same - # DTLS session. + # sibling indoor subdevices before the first discovery pass, and + # fold their seed resources into this cycle's snapshot so + # discovery sees every subdevice's state on the very first poll + # rather than waiting a cycle. Runs under its own session-lock + # scope (the poll above already released the lock) since it + # shares the same DTLS session. async with self._session_lock: resources = await self.hass.async_add_executor_job( - self._enumerate_sub_units_blocking, resources + self._enumerate_subdevices_blocking, resources ) source = 'sweep' if self._discovered else 'poll' @@ -820,7 +823,7 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): # Discovery runs *before* the apply loop below, not after it, so # a rejected candidate's resources never reach the state cache # at all (issue #177). Enumeration has to fetch every candidate's - # seed to evaluate the liveness gate, but only the units that + # seed to evaluate the liveness gate, but only the subdevices that # pass it are ever polled again -- applying the rest would freeze # ~14 hrefs per rejected slot into the cache on this one cycle # and leave them there forever, indistinguishable from live @@ -831,7 +834,7 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): # log_sweep_discrepancies below can't fire on a first cycle # (observe mode is only ever attempted after discovery). self._run_discovery(resources) - resources = self._live_unit_resources(resources) + resources = self._live_subdevice_resources(resources) sweep_mismatch = False if self._observe.mode == MODE_OBSERVE: # A sweep/cache mismatch never tears down a still-live OBSERVE @@ -934,15 +937,15 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): # in issues #17/#53, which survived the earlier optimistic-apply fix # (issue #27) because that fix applied to the wrong href too. # - # write_fn's path_segs are canonical (issue #177) -- a sub-unit's + # write_fn's path_segs are canonical (issue #177) -- a subdevice's # ClimateDesc is bound to its own *actual* /mode/vs/1 (or # //mode/vs/0) href, but _climate_write only knows the canonical # sibling hrefs (e.g. ['power', 'vs', '0']). Translate through this - # bound entity's own sub_unit so the optimistic apply, the settle - # guard and the POST below all target that unit's real resource -- + # bound entity's own subdevice so the optimistic apply, the settle + # guard and the POST below all target that subdevice's real resource -- # to_actual is the identity transform for MAIN, so a device with no - # sub-units writes exactly where it always did. - write_href = bound_entity.sub_unit.to_actual('/' + '/'.join(path_segs)) + # subdevices writes exactly where it always did. + write_href = bound_entity.subdevice.to_actual('/' + '/'.join(path_segs)) path_segs = [s for s in write_href.strip('/').split('/') if s] # Apply the write optimistically before starting the settle guard, diff --git a/custom_components/localthings/diagnostics.py b/custom_components/localthings/diagnostics.py index c94ce42..2a1c010 100644 --- a/custom_components/localthings/diagnostics.py +++ b/custom_components/localthings/diagnostics.py @@ -18,7 +18,7 @@ from homeassistant.loader import async_get_integration from .const import DOMAIN from .coordinator import LocalThingsCoordinator from .registry.redact import redact_resources -from .registry.subunits import MAIN +from .registry.subdevices import MAIN async def async_get_config_entry_diagnostics( @@ -37,18 +37,18 @@ async def async_get_config_entry_diagnostics( # is OCF's standard device-type declaration; /oic/res is OCF's # discovery endpoint, listing every href/Collection the connection # hosts -- relevant to the "Composite Device" model (issue #177) where - # a single physical unit exposes more than one logical Device. See + # a single physical device exposes more than one logical subdevice. See # registry/identity.py. identity = coordinator._identity - def _sub_unit_diag(su) -> dict: + def _subdevice_diag(su) -> dict: # One pass over coordinator.bound for both fields below (count and - # the distinct hrefs), and one redaction of this unit's canonical + # the distinct hrefs), and one redaction of this subdevice's canonical # view -- `model` reads modelNum off the already-redacted `resources` # rather than redacting /information/vs/0 a second time. modelNum # itself never matches redact.py's substring rules, so which side of # redact_resources it's read from doesn't change the value. - matching = [b for b in coordinator.bound if b.sub_unit == su] + matching = [b for b in coordinator.bound if b.subdevice == su] res = redact_resources(coordinator.canonical_resources(su)) return { "kind": su.kind, @@ -57,7 +57,7 @@ async def async_get_config_entry_diagnostics( "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', ''), - # Keyed by this unit's *canonical* hrefs, not the real ones + # Keyed by this subdevice's *canonical* hrefs, not the real ones # it answers on -- '/mode/vs/0' rather than '/mode/vs/1' or # '//mode/vs/0'. That's the form the registry and every # capability are written against, so a sibling's block can be @@ -77,66 +77,70 @@ async def async_get_config_entry_diagnostics( "resources": redact_resources(identity.raw), } if identity is not None else None, "unbound_hrefs": sorted(coordinator._unbound_hrefs), - # This unit's own resources, and only this unit's -- what the - # module docstring and the adding-device-support skill have always - # described it as ("the parsed /device/0 snapshot"). On a composite - # device (issue #177) `last_resources` is the union across every - # live unit keyed by real hrefs, so reporting it raw here would mix - # a sibling's /mode/vs/1 in with the master's /mode/vs/0 under no - # attribution at all. Each sibling reports its own resources in its - # own `sub_units` entry below instead. For a device with no sub-units - # -- almost every device -- this is byte-identical to `last_resources`. + # This subdevice's own resources, and only this subdevice's -- what + # the module docstring and the adding-device-support skill have + # always described it as ("the parsed /device/0 snapshot"). On a + # composite device (issue #177) `last_resources` is the union across + # every live subdevice keyed by real hrefs, so reporting it raw here + # would mix a sibling's /mode/vs/1 in with the master's /mode/vs/0 + # under no attribution at all. Each sibling reports its own + # resources in its own `subdevices` entry below instead. For a + # device with no subdevices -- almost every device -- this is + # byte-identical to `last_resources`. "resources": redact_resources(coordinator.canonical_resources(MAIN)), - # Sibling indoor units discovered on this connection (issue #177) -- - # per-unit kind/key/seed path plus what actually bound to it, so a - # report shows whether a composite device's sub-unit was found at - # all and what it resolved to. subdeviceIdList (the UUID a prefixed - # unit's key comes from) is deliberately NOT redacted here even + # Sibling indoor subdevices discovered on this connection (issue + # #177) -- per-subdevice kind/key/seed path plus what actually bound + # to it, so a report shows whether a composite device's subdevice + # was found at all and what it resolved to. subdeviceIdList (the + # UUID a prefixed subdevice's key comes from) is deliberately NOT + # redacted here even # though the field matches redact.py's 'deviceid' substring rule # elsewhere in `resources` above -- it's an appliance-internal # pairing id, not account data, and reporting the key is what makes # this block actionable. - "sub_units": [_sub_unit_diag(su) for su in coordinator.sub_units], + "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 unit. Reported alongside sub_units above so a report + # 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. - "sub_units_skipped": [ + "subdevices_skipped": [ { - "kind": skip.sub_unit.kind, - "key": skip.sub_unit.key, - "seed_path": "/" + "/".join(skip.sub_unit.seed_path), + "kind": skip.subdevice.kind, + "key": skip.subdevice.key, + "seed_path": "/" + "/".join(skip.subdevice.seed_path), "hrefs": list(skip.hrefs), # The reps the liveness gate actually judged, canonicalized - # like the materialized units above. These are the one thing - # a reader needs to second-guess a skip ("is my second unit - # really absent, or did the gate get it wrong?"), and they - # exist nowhere else in this dump: a rejected candidate is - # never polled again and never enters the state cache, so - # `resources` above cannot contain them by construction. + # like the materialized subdevices above. These are the one + # thing a reader needs to second-guess a skip ("is my second + # subdevice really absent, or did the gate get it wrong?"), + # and they exist nowhere else in this dump: a rejected + # candidate is never polled again and never enters the state + # cache, so `resources` above cannot contain them by + # construction. "resources": redact_resources({ canon: rep - for href, rep in coordinator._skipped_sub_unit_resources.items() - if (canon := skip.sub_unit.to_canonical(href)) is not None + for href, rep in coordinator._skipped_subdevice_resources.items() + if (canon := skip.subdevice.to_canonical(href)) is not None }), } - for skip in coordinator._skipped_sub_units + for skip in coordinator._skipped_subdevices ], # What each enumeration probe returned ({} vs a batch), keyed by the # seed href attempted -- lets a report distinguish "checked, nothing # there" from "never checked", the same posture the speculative # /device/1 //device/2 probe this replaced used to document directly - # in identity.py before it moved to registry/subunits.py. - "sub_unit_probes": dict(sorted(coordinator._sub_unit_probes.items())), + # in identity.py before it moved to registry/subdevices.py. + "subdevice_probes": dict(sorted(coordinator._subdevice_probes.items())), # /multidevice/vs/0's rep ({} when the board doesn't answer it). # Reported on its own rather than inside `resources` because it is - # metadata about the connection rather than state of any one unit -- - # and because nothing polls it after discovery, so it would go stale - # in there. Its numofsubdevice count is what independently - # corroborates the sub_units/sub_units_skipped split above. + # metadata about the connection rather than state of any one + # subdevice -- and because nothing polls it after discovery, so it + # would go stale in there. Its numofsubdevice count is what + # independently corroborates the subdevices/subdevices_skipped split + # above. "multidevice": redact_resources(coordinator._multidevice), "integration_version": integration.version, "smartthings_local_version": stl_version, diff --git a/custom_components/localthings/entity.py b/custom_components/localthings/entity.py index c23d190..27c8daa 100644 --- a/custom_components/localthings/entity.py +++ b/custom_components/localthings/entity.py @@ -35,8 +35,8 @@ def _is_included(bound: BoundEntity, coordinator: 'LocalThingsCoordinator') -> b common.ENERGY_METER, issue #127) -- this default stays permissive. `bound.href` is already the *actual* href (issue #177 -- see - BoundEntity/SubUnit), so the direct cache lookup below is correct as-is; - `exists_fn` gets `bound`'s own sub-unit's *canonical* view instead of the + BoundEntity/Subdevice), so the direct cache lookup below is correct as-is; + `exists_fn` gets `bound`'s own subdevice's *canonical* view instead of the raw snapshot, same rule as everywhere else a whole-resources-dict scan happens (coordinator.canonical_resources) -- this is a free function, not an LocalThingsEntity method, so it can't use self._resources. @@ -45,7 +45,7 @@ def _is_included(bound: BoundEntity, coordinator: 'LocalThingsCoordinator') -> b if rep is None: return False if bound.desc.exists_fn is not None: - return bound.desc.exists_fn(rep, coordinator.canonical_resources(bound.sub_unit)) + return bound.desc.exists_fn(rep, coordinator.canonical_resources(bound.subdevice)) if bound.desc.field: if not rep or is_stub_rep(rep): return True @@ -134,16 +134,16 @@ class LocalThingsEntity(CoordinatorEntity[LocalThingsCoordinator]): @property def _resources(self) -> dict: - """This entity's own sub-unit's canonical resources view (issue + """This entity's own subdevice's canonical resources view (issue #177) -- see coordinator.canonical_resources. Every platform property that needs the *whole* resources dict, as opposed to one href via `coordinator.resource(href)`, must read through this - instead of `coordinator.last_resources`, or a sibling unit's own + instead of `coordinator.last_resources`, or a sibling subdevice's own actual hrefs would leak into (or be missing from) this entity's - view. For MAIN (every device with no sub-units) this is exactly + view. For MAIN (every device with no subdevices) this is exactly `coordinator.last_resources`.""" - return self.coordinator.canonical_resources(self._bound.sub_unit) + return self.coordinator.canonical_resources(self._bound.subdevice) @property def device_info(self) -> DeviceInfo: - return self.coordinator.device_info_for(self._bound.sub_unit) + return self.coordinator.device_info_for(self._bound.subdevice) diff --git a/custom_components/localthings/registry/adapter.py b/custom_components/localthings/registry/adapter.py index 2a362cd..5224e4e 100644 --- a/custom_components/localthings/registry/adapter.py +++ b/custom_components/localthings/registry/adapter.py @@ -4,42 +4,42 @@ from __future__ import annotations from typing import Any from .discovery import BoundEntity -from .subunits import SubUnit, canonical_view +from .subdevices import Subdevice, canonical_view def _key(b: BoundEntity) -> str: - # b.sub_unit.key_prefix is '' for MAIN, so a device with no sub-units + # b.subdevice.key_prefix is '' for MAIN, so a device with no subdevices # (every device this integration shipped before issue #177) gets a # byte-identical key to before -- a hard regression guard, not a nicety # (see test_unique_ids.py and every golden file under tests/fixtures/golden/). - return f"{b.sub_unit.key_prefix}{b.key_override or b.desc.key}{b.instance}" + return f"{b.subdevice.key_prefix}{b.key_override or b.desc.key}{b.instance}" def flatten(bound: list[BoundEntity], resources: dict) -> dict[str, Any]: """Map bound entities to their current scalar values. - `exists_fn(rep, resources)` receives that entity's own sub-unit's - *canonical* resources view (see subunits.canonical_view), not the raw + `exists_fn(rep, resources)` receives that entity's own subdevice's + *canonical* resources view (see subdevices.canonical_view), not the raw actual-href snapshot -- an exists_fn that scans the whole resources dict - for a sibling href (e.g. is_legacy_board) must judge each unit on its - own resources, not see another unit's hrefs bleed in under the same - canonical key. Views are built once per distinct sub-unit per call, not - once per entity -- O(sub-units), not O(bound entities). + for a sibling href (e.g. is_legacy_board) must judge each subdevice on its + own resources, not see another subdevice's hrefs bleed in under the same + canonical key. Views are built once per distinct subdevice per call, not + once per entity -- O(subdevices), not O(bound entities). """ out: dict[str, Any] = {} - # SubUnit is a frozen dataclass (hashable, equal by value), so it can key + # Subdevice is a frozen dataclass (hashable, equal by value), so it can key # `views` directly -- no need to re-derive an identity for it out of # (kind, key) first. - all_units = list(dict.fromkeys(b.sub_unit for b in bound)) - views: dict[SubUnit, dict] = {} + all_subdevices = list(dict.fromkeys(b.subdevice for b in bound)) + views: dict[Subdevice, dict] = {} for b in bound: rep = resources.get(b.href) or {} if b.desc.exists_fn is not None: - view = views.get(b.sub_unit) + view = views.get(b.subdevice) if view is None: - view = canonical_view(b.sub_unit, resources, all_units) - views[b.sub_unit] = view + view = canonical_view(b.subdevice, resources, all_subdevices) + views[b.subdevice] = view if not b.desc.exists_fn(rep, view): continue if b.desc.rep_fn is not None: diff --git a/custom_components/localthings/registry/capabilities/airconditioner.py b/custom_components/localthings/registry/capabilities/airconditioner.py index a4038f9..b817e3f 100644 --- a/custom_components/localthings/registry/capabilities/airconditioner.py +++ b/custom_components/localthings/registry/capabilities/airconditioner.py @@ -939,29 +939,30 @@ _AC_IGNORED = [ '/stepcontrol/vs/0', '/reserverulesets/vs/0', # opaque hex-encoded schedule reservation blob '/welcome/temperature/vs/0', # welcome-cooling plumbing - # System-AC-only (multi-indoor-unit commercial installs, e.g. + # System-AC-only (multi-indoor-subdevice commercial installs, e.g. # A-CAWW-TP2-20-COMMON, issue #52): opaque hex-encoded installation - # topology -- indoor/outdoor unit pairing, per-unit serials, MCU info. - # Commissioning-time plumbing, not user-actionable appliance state. + # topology -- indoor/outdoor unit pairing, per-subdevice serials, MCU + # info. Commissioning-time plumbing, not user-actionable appliance state. '/sac/installationinfo/vs/0', # Wind-Free 2-in-1 systems (one outdoor unit driving a floor-standing - # *and* a wall-mounted indoor unit over one shared local IP, e.g. + # *and* a wall-mounted indoor subdevice over one shared local IP, e.g. # TP2X_FAC_BORA_21K, issues #150/#153): an opaque paired-subdevice id # list, same "remote device ids, not user-actionable locally" role as - # /remotedeviceinfo/vs/0 above. This integration talks to whichever - # single local endpoint the config entry was set up against; the - # second indoor unit isn't independently reachable through this - # resource (or any other in the dump) -- it would need its own local - # DTLS session/IP, which SmartThings pairing doesn't expose here. + # /remotedeviceinfo/vs/0 above -- true whenever this field is redacted + # (the shipped airconditioner_fac_bora fixture) or absent. When it does + # carry a real id (issue #177's airconditioner_fac_bora_2in1 fixture), + # registry/subdevices.py's enumerate_subdevices reads this same + # subdeviceIdList to reach the second indoor subdevice over this same + # connection instead -- see that module's Pattern B. '/subdevices/vs/0', # 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-unit systems (issue #177, HJcom's + # 2-in-1/multi-indoor-subdevice systems (issue #177, HJcom's # ARTIK051_DONGLE_FAC_18K): x.com.samsung.da.numofsubdevice, a plain - # corroborating count of indoor units on this connection. Confirmed + # corroborating count of indoor subdevices on this connection. Confirmed # read-only (a write attempt returned CoAP 4.00). Absent from - # /device/0's batch entirely -- registry.subunits.enumerate_sub_units + # /device/0's batch entirely -- registry.subdevices.enumerate_subdevices # fetches it with its own RETRIEVE and folds it into the resources dict # for diagnostics, which is why it needs an entry here rather than # surfacing as an unbound-href gap on every board that has it. diff --git a/custom_components/localthings/registry/discovery.py b/custom_components/localthings/registry/discovery.py index 3533af6..3ba4555 100644 --- a/custom_components/localthings/registry/discovery.py +++ b/custom_components/localthings/registry/discovery.py @@ -18,7 +18,7 @@ from typing import Callable, Iterable, Optional from .capability import Capability from .entities import SamsungEntityDescription -from .subunits import MAIN, SubUnit +from .subdevices import MAIN, Subdevice @dataclass @@ -29,11 +29,12 @@ class BoundEntity: instance: str = '' key_override: Optional[str] = None instance_name: Optional[str] = None - # Which logical indoor unit (issue #177) this entity belongs to. `href` - # above is always the *actual*, on-the-wire href for that unit -- MAIN's - # to_actual is the identity transform, so every device with no sub-units + # Which logical indoor subdevice (issue #177) this entity belongs to. + # `href` above is always the *actual*, on-the-wire href for that + # subdevice -- MAIN's + # to_actual is the identity transform, so every device with no subdevices # behaves exactly as before this field existed. - sub_unit: SubUnit = MAIN + subdevice: Subdevice = MAIN def _snake_to_title(s: str) -> str: @@ -64,19 +65,19 @@ def instance_suffix(href: str) -> str: def _bind(cap: Capability, href: str, inst: str, inst_name: Optional[str], key_prefix: Optional[str] = None, - sub_unit: SubUnit = MAIN) -> list[BoundEntity]: + subdevice: Subdevice = MAIN) -> list[BoundEntity]: """Build one BoundEntity per entity on `cap`, sharing the instance/ key-prefix/instance-name computed once by the caller. `href` here is the *canonical* href discover() is iterating over; - `sub_unit.to_actual` maps it to the real, on-the-wire href the entity - actually reads/writes (identity for MAIN, so single-unit devices are - unaffected -- see subunits.py).""" + `subdevice.to_actual` maps it to the real, on-the-wire href the entity + actually reads/writes (identity for MAIN, so single-subdevice devices are + unaffected -- see subdevices.py).""" return [ - BoundEntity(href=sub_unit.to_actual(href), capability=cap, desc=desc, + BoundEntity(href=subdevice.to_actual(href), capability=cap, desc=desc, instance=inst, key_override=f'{key_prefix}_{desc.key}' if key_prefix else None, - instance_name=inst_name, sub_unit=sub_unit) + instance_name=inst_name, subdevice=subdevice) for desc in cap.entities ] @@ -87,7 +88,7 @@ def discover( pattern_caps: Iterable[Capability] = (), log: Optional[Callable[[str], None]] = None, tier_log: Optional[Callable[[str, str], None]] = None, - sub_unit: SubUnit = MAIN, + subdevice: Subdevice = MAIN, ) -> list[BoundEntity]: """`tier_log(href, poll_tier)` fires for every href a capability actually matches, even a no-entity "coverage-only" capability (see COVERAGE lists @@ -97,12 +98,13 @@ def discover( coverage-only capability's `poll_tier` would otherwise be silently dropped since it never appears in `bound`. - `resources` is always keyed by *canonical* hrefs -- for a sub-unit + `resources` is always keyed by *canonical* hrefs -- for a subdevice (issue #177) that means its own canonical view (see - subunits.canonical_view), the same shape as a plain single-unit device's - resources dict, so registry lookups/rt_filter/match_fn/instance_suffix - all behave identically regardless of which unit is being discovered. - `sub_unit` only affects the *href* stamped onto each BoundEntity (via + subdevices.canonical_view), the same shape as a plain single-subdevice + device's resources dict, so registry lookups/rt_filter/match_fn/ + instance_suffix all behave identically regardless of which subdevice is + being discovered. + `subdevice` only affects the *href* stamped onto each BoundEntity (via `_bind`, see above) and the href `log`/`tier_log` report -- both the real, subscribable/pollable path, not the canonical one the registry is keyed on. @@ -122,10 +124,10 @@ def discover( if cap.match_fn is not None and not cap.match_fn(rep, resources): continue inst = instance_suffix(href) - out.extend(_bind(cap, href, inst, _instance_name(cap, rep), sub_unit=sub_unit)) + out.extend(_bind(cap, href, inst, _instance_name(cap, rep), subdevice=subdevice)) matched = True if tier_log is not None: - tier_log(sub_unit.to_actual(href), cap.poll_tier) + tier_log(subdevice.to_actual(href), cap.poll_tier) if matched: continue @@ -143,13 +145,13 @@ def discover( src = href[len(cap.href_prefix):] if (cap.strip_prefix_in_key and cap.href_prefix) else href segs = [s for s in src.strip('/').split('/') if s and not s.isdigit() and s != 'vs'] out.extend(_bind(cap, href, inst, _instance_name(cap, rep), '_'.join(segs), - sub_unit=sub_unit)) + subdevice=subdevice)) matched = True if tier_log is not None: - tier_log(sub_unit.to_actual(href), cap.poll_tier) + tier_log(subdevice.to_actual(href), cap.poll_tier) break if not matched and not caps and log is not None: - log(sub_unit.to_actual(href)) + log(subdevice.to_actual(href)) return out diff --git a/custom_components/localthings/registry/identity.py b/custom_components/localthings/registry/identity.py index 209287b..25907f6 100644 --- a/custom_components/localthings/registry/identity.py +++ b/custom_components/localthings/registry/identity.py @@ -70,9 +70,9 @@ def read_identity(sess, serial: Optional[str]) -> DeviceIdentity: # RETRIEVE on it returns every Resource/Collection href this endpoint # hosts, not just the one /device/0 seed path the coordinator polls. # Relevant for the OCF "Composite Device" model (issue #177: a single - # physical unit -- one IP, one /oic/p -- exposing more than one logical - # Device, each as its own Collection resource, same rt shape as our own - # /device/0). This is what registry.subunits.enumerate_sub_units reads + # 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 # ARTIK051_DONGLE_FAC_18K) -- that probing, plus the /device/1 and # /device/2 speculative fallback it used to run right here on every diff --git a/custom_components/localthings/registry/subunits.py b/custom_components/localthings/registry/subdevices.py similarity index 74% rename from custom_components/localthings/registry/subunits.py rename to custom_components/localthings/registry/subdevices.py index c72b8bb..8d5590e 100644 --- a/custom_components/localthings/registry/subunits.py +++ b/custom_components/localthings/registry/subdevices.py @@ -1,17 +1,17 @@ -"""Sub-unit ("composite device") support for one physical connection exposing -more than one logical indoor unit -- issue #177. +"""Subdevice ("composite device") support for one physical connection exposing +more than one logical indoor subdevice -- issue #177. Two reporters, two different board families, two genuinely different -mechanisms for exposing a second indoor unit 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): +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): 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 units are reachable only via their own +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). @@ -19,25 +19,26 @@ Pattern B -- UUID-prefixed tree (`TP2X_FAC_BORA_21K`, jhkwon19's board). 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 -unit's own Collection batch, confirmed live by the reporter to carry a +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 unit, vs. the master's `TP2X_FAC_BORA_21K`, the floor unit). +wall-mounted subdevice, vs. the master's `TP2X_FAC_BORA_21K`, the floor +subdevice). -Both are "the same thing wearing different clothes": a logical unit is a +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_sub_units` checks both and +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 unit, though: HJcom's own board also has a +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 unit's, an /information rep echoing the *same* model string as unit -1, and a /temperatures items[] entry with an id/description but no +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 @@ -66,8 +67,8 @@ from .batch import parse_device0_batch _INDEXED_HREF_RE = re.compile(r'^/device/(\d+)$') # Speculative /device/ siblings probed when /oic/res doesn't reveal a -# second logical Device's Collection on this board (moved here from -# identity.py, issue #177 -- see enumerate_sub_units' docstring for why: the +# second logical subdevice's Collection on this board (moved here from +# identity.py, issue #177 -- see enumerate_subdevices' docstring for why: the # old read_identity fired these two extra RETRIEVEs on *every* _connect_session, # including every reconnect, for information enumeration only needs once). # Same bound as before: a plain, tolerated-404 RETRIEVE, not the kind of @@ -77,16 +78,18 @@ _SPECULATIVE_DEVICE_INDICES = (1, 2) @dataclass(frozen=True) -class SubUnit: - """One logical indoor unit reachable over a single physical connection. +class Subdevice: + """One logical indoor subdevice reachable over a single physical + connection. - `kind='main'` is the unit this config entry actually connects to and + `kind='main'` is the subdevice this config entry actually connects to and always exists (see MAIN below) -- its `to_actual`/`to_canonical` are the - identity transform, so every existing single-unit device keeps behaving - exactly as it did before this module existed. `'indexed'`/`'prefixed'` - are Pattern A/B above; `key` is the trailing index string ('1', '2', ...) - or the full subdevice UUID, and `seed_path` is the Collection href (as - path segments) whose batch response enumerates/refreshes that unit. + identity transform, so every existing single-subdevice device keeps + behaving exactly as it did before this module existed. `'indexed'`/ + `'prefixed'` are Pattern A/B above; `key` is the trailing index string + ('1', '2', ...) or the full subdevice UUID, and `seed_path` is the + Collection href (as path segments) whose batch response + enumerates/refreshes that subdevice. """ kind: str # 'main' | 'indexed' | 'prefixed' key: str # '' | '1' | '6c2dff6d-ee5c-dad1-6a5e-000000000001' @@ -94,13 +97,13 @@ class SubUnit: def to_actual(self, canonical: str) -> str: """Canonical registry href (e.g. '/mode/vs/0') -> the real, - on-the-wire href for this unit.""" + on-the-wire href for this subdevice.""" if self.kind == 'indexed': head, sep, tail = canonical.rpartition('/') # Only the index-0 trailing segment is ours to rewrite -- # deliberately not a "replace any trailing digit" rule, which # would misread a genuine multi-instance resource (the fridge's - # pattern-cap hrefs, e.g. '/door/vs/1') as a sub-unit's. No + # pattern-cap hrefs, e.g. '/door/vs/1') as a subdevice's. No # registry declares a non-zero trailing index today and no # fixture in the corpus contains one (verified across the whole # corpus), so the strict rule costs nothing. @@ -112,7 +115,7 @@ class SubUnit: return canonical def to_canonical(self, actual: str) -> Optional[str]: - """Inverse of to_actual, or None when `actual` isn't this unit's.""" + """Inverse of to_actual, or None when `actual` isn't this subdevice's.""" if self.kind == 'indexed': head, sep, tail = actual.rpartition('/') if tail == self.key: @@ -126,9 +129,9 @@ class SubUnit: return actual def owns(self, actual: str) -> bool: - """True if `actual` belongs to this unit's namespace. MAIN never + """True if `actual` belongs to this subdevice's namespace. MAIN never "owns" anything by this definition -- it gets whatever's left after - every other sub-unit's hrefs are excluded (see canonical_view).""" + every other subdevice's hrefs are excluded (see canonical_view).""" if self.kind == 'main': return False return self.to_canonical(actual) is not None @@ -136,7 +139,7 @@ class SubUnit: @property def key_prefix(self) -> str: """Prefix that guarantees a unique entity key/unique_id (see - adapter._key). '' for MAIN -- the master unit's flattened state + adapter._key). '' for MAIN -- the master's flattened state keys must stay byte-identical to every device this integration shipped before issue #177, so no golden file changes. The full subdevice UUID is used verbatim (non-alphanumerics stripped, not @@ -147,60 +150,60 @@ class SubUnit: the device name + entity name, not from unique_id. """ if self.kind == 'indexed': - return f'unit{self.key}_' + return f'subdevice{self.key}_' if self.kind == 'prefixed': slug = re.sub(r'[^a-zA-Z0-9]', '', self.key) - return f'sub_{slug}_' + return f'subdevice_{slug}_' return '' -MAIN = SubUnit(kind='main', key='', seed_path=('device', '0')) +MAIN = Subdevice(kind='main', key='', seed_path=('device', '0')) def canonical_view( - sub_unit: SubUnit, resources: dict[str, dict], sub_units: list['SubUnit'], + subdevice: Subdevice, resources: dict[str, dict], subdevices: list['Subdevice'], ) -> dict[str, dict]: - """Rewrite `resources` (real, on-the-wire hrefs) into `sub_unit`'s own + """Rewrite `resources` (real, on-the-wire hrefs) into `subdevice`'s own canonical namespace -- what discover()/exists_fn/rep_fn/is_legacy_board and friends are written against. For MAIN this is the snapshot *minus* every href owned by one of the - other units in `sub_units` -- otherwise a sibling's own `/mode/vs/1` + other subdevices in `subdevices` -- otherwise a sibling's own `/mode/vs/1` would leak into the master's view under the same canonical key ('/mode/vs/0') that the master's actual `/mode/vs/0` also maps to, - silently mixing two units' state together. For an indexed/prefixed unit - it's the reverse: only the hrefs that unit owns, rewritten back through - `to_canonical`. + silently mixing two subdevices' state together. For an indexed/prefixed + subdevice it's the reverse: only the hrefs that subdevice owns, rewritten + back through `to_canonical`. - `sub_units` may or may not include MAIN itself -- MAIN.owns() is always + `subdevices` may or may not include MAIN itself -- MAIN.owns() is always False, so including it is harmless. """ - if sub_unit.kind == 'main': + if subdevice.kind == 'main': owned_elsewhere = { href for href in resources - if any(su.owns(href) for su in sub_units) + if any(su.owns(href) for su in subdevices) } return {h: r for h, r in resources.items() if h not in owned_elsewhere} return { canon: resources[actual] for actual in resources - if (canon := sub_unit.to_canonical(actual)) is not None + if (canon := subdevice.to_canonical(actual)) is not None } -def normalize_seed_batch(sub_unit: SubUnit, batch: dict[str, dict]) -> dict[str, dict]: - """Real, on-the-wire hrefs from one sub-unit's seed-collection batch, - normalized so every href actually carries this unit's prefix/index. +def normalize_seed_batch(subdevice: Subdevice, batch: dict[str, dict]) -> dict[str, dict]: + """Real, on-the-wire hrefs from one subdevice's seed-collection batch, + normalized so every href actually carries this subdevice's prefix/index. - Indexed units need no change -- the device echoes the real `/x/` + Indexed subdevices need no change -- the device echoes the real `/x/` href in its own `/device/` batch (confirmed against HJcom's dump). - A prefixed unit's batch entries may or may not already carry the + 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 sub-unit id was known), so it's added when missing. + live before the subdevice id was known), so it's added when missing. """ - if sub_unit.kind != 'prefixed': + if subdevice.kind != 'prefixed': return batch - prefix = f'/{sub_unit.key}' + prefix = f'/{subdevice.key}' return { (href if href.startswith(prefix + '/') else f'{prefix}{href}'): rep for href, rep in batch.items() @@ -266,23 +269,24 @@ def _get_property(sess, path_segs: tuple[str, ...]) -> dict: return body if isinstance(body, dict) else {} -def enumerate_sub_units( +def enumerate_subdevices( sess, resources: dict[str, dict], oic_res_links, probe_log: Optional[Callable[[str, bool], None]] = None, -) -> tuple[list['SubUnit'], dict[str, dict]]: - """Discover every sibling indoor unit reachable over `sess`'s connection. +) -> tuple[list['Subdevice'], dict[str, dict]]: + """Discover every sibling indoor subdevice reachable over `sess`'s + connection. Runs once, at first discovery, in an executor, under the coordinator's session lock -- every GET here is a plain RETRIEVE (the write-contract 'don't guess' rule doesn't apply to reading an extra resource to find - out whether it's there). Returns the *candidate* units and the resources + out whether it's there). Returns the *candidate* subdevices and the resources already fetched while probing them (already normalized to real hrefs), so the coordinator's first discovery poll doesn't need to re-poll them. `probe_log(seed_href, found)` fires for every seed attempted, whether or - not it answered -- so diagnostics (see diagnostics.py's sub_unit_probes) + not it answered -- so diagnostics (see diagnostics.py's subdevice_probes) can tell "checked, nothing there" apart from "never checked", the same posture the speculative-probe code this replaces used to document in identity.py. @@ -294,7 +298,7 @@ def enumerate_sub_units( entities first, which is `discover_partitioned`'s job, not this one's. See this module's docstring. """ - units: list[SubUnit] = [] + subdevices: list[Subdevice] = [] fetched: dict[str, dict] = {} def _probed(seed_href: str, batch: dict) -> None: @@ -307,7 +311,7 @@ def enumerate_sub_units( # Tolerate anything but a list of strings -- this field is redaction-prone # (it matches the 'deviceid' substring rule in redact.py) and the existing # airconditioner_fac_bora fixture carries the literal string - # '**REDACTED**'/'REDACTED' there. That must yield zero sub-units, not a + # '**REDACTED**'/'REDACTED' there. That must yield zero subdevices, not a # crash -- issue #177 is additive, it must never break an already-working # single-climate-entity device. ids = raw_ids if isinstance(raw_ids, list) else [] @@ -317,9 +321,9 @@ def enumerate_sub_units( _probed(_seed_href(seed), batch) if not batch: continue - unit = SubUnit(kind='prefixed', key=sub_id, seed_path=seed) - fetched.update(normalize_seed_batch(unit, batch)) - units.append(unit) + subdevice = Subdevice(kind='prefixed', key=sub_id, seed_path=seed) + fetched.update(normalize_seed_batch(subdevice, batch)) + subdevices.append(subdevice) # --- Pattern A: indexed siblings (ARTIK051_DONGLE_FAC_18K) -------------- indices = sorted({ @@ -340,9 +344,9 @@ def enumerate_sub_units( _probed(_seed_href(seed), batch) if not batch: continue - unit = SubUnit(kind='indexed', key=str(n), seed_path=seed) + subdevice = Subdevice(kind='indexed', key=str(n), seed_path=seed) fetched.update(batch) # already real /x/ hrefs, no normalization needed - units.append(unit) + 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 @@ -354,31 +358,31 @@ def enumerate_sub_units( # an unbound-href gap). NOT a gate: discover_partitioned's entity-level # liveness check decides materialization correctly without it, and only # this one board family is known to expose it at all. Whether it agrees - # with the number of units actually materialized is the coordinator's - # call to log (it owns the logger; this module doesn't), not this - # function's. + # with the number of subdevices actually materialized is the + # coordinator's call to log (it owns the logger; this module doesn't), + # not this function's. multidevice_seed = ('multidevice', 'vs', '0') multidevice = _get_property(sess, multidevice_seed) _probed(_seed_href(multidevice_seed), multidevice) if multidevice: fetched['/multidevice/vs/0'] = multidevice - return units, fetched + return subdevices, fetched @dataclass(frozen=True) -class SkippedSubUnit: - """A candidate `enumerate_sub_units` found whose seed answered, but that +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 unit. + 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.""" - sub_unit: SubUnit + subdevice: Subdevice hrefs: tuple[str, ...] def _has_live_primary_entity(bound, state: dict) -> bool: - """True if flattening `bound` (one candidate sub-unit's BoundEntity + """True if flattening `bound` (one candidate subdevice's BoundEntity list) produced at least one non-`None` value for a *primary* entity -- `entity_category` unset, HA's own "the user acts on or watches this" tier (see the adding-device-support skill's entity-taxonomy section). @@ -387,7 +391,7 @@ def _has_live_primary_entity(bound, state: dict) -> bool: HJcom'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 unit is actually + "something" proves nothing about whether a physical subdevice is actually installed there, so it's deliberately excluded from this check. """ from .adapter import _key # see discover_partitioned's deferred-import note @@ -399,27 +403,27 @@ def _has_live_primary_entity(bound, state: dict) -> bool: def discover_partitioned( resources: dict[str, dict], - sub_units: list['SubUnit'], + subdevices: list['Subdevice'], resolve_registry: Callable[[dict], object], fallback_capabilities: dict, log: Optional[Callable[[str], None]] = None, tier_log: Optional[Callable[[str, str], None]] = None, ): """Bind every href in `resources` (the merged, real-href snapshot -- main - plus every enumerated sub-unit's seed) to entities, partitioned by which - unit owns it. + plus every enumerated subdevice's seed) to entities, partitioned by which + subdevice owns it. - Main pass runs over hrefs owned by no sub-unit -- otherwise every + Main pass runs over hrefs owned by no subdevice -- otherwise every `/mode/vs/1` would land in `unbound_hrefs` too (nothing in the main device's registry claims that literal href) and raise a spurious - coverage-gap repair. Then one pass per *candidate* sub-unit over its own - canonical view, resolving that unit's own device type from its own - `/information/vs/0` when it reports one (e.g. jhkwon19's wall unit + 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 -> 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 - type as the unit this config entry was set up against. + type as the subdevice this config entry was set up against. Each candidate is discovered and flattened *twice*: once silently to evaluate `_has_live_primary_entity` (this module's materialization @@ -429,21 +433,21 @@ def discover_partitioned( contributes nothing at all -- no bound entities, no unbound-href report, no hot/warm href -- as if it had never answered its seed. Discovering an unused slot's small, fixed resource set twice at - first-discovery time only is a non-issue; getting a phantom sub-unit + first-discovery time only is a non-issue; getting a phantom subdevice silently counted into unbound_hrefs or hot/warm tiers is not. Returns `(bound, device_type_name, materialized, skipped)`: - `bound`: the concatenated BoundEntity list (main + every materialized - sub-unit). + subdevice). - `device_type_name`: the *master's* resolved device type (used for - logging/device naming; each sub-unit's own resolved type only affects + logging/device naming; each subdevice's own resolved type only affects which capabilities bind its hrefs, not this). - - `materialized`: the subset of `sub_units` that passed the gate, in the - same order -- what the caller should keep as its live sub-unit roster + - `materialized`: the subset of `subdevices` that passed the gate, in the + same order -- what the caller should keep as its live subdevice roster going forward (poll seeds, canonical_resources, device_info_for, ...). - - `skipped`: `SkippedSubUnit` entries for every candidate that didn't. + - `skipped`: `SkippedSubdevice` entries for every candidate that didn't. """ - # Deferred import: discovery.py imports SubUnit/MAIN from this module at + # Deferred import: discovery.py imports Subdevice/MAIN from this module at # module scope, so importing discover() back here at module scope would # be a circular import. By the time this function actually runs both # modules are fully loaded. adapter.py imports discovery.py, so the same @@ -452,9 +456,9 @@ def discover_partitioned( from .discovery import discover # Same computation canonical_view does for MAIN (snapshot minus every - # other sub-unit's owned hrefs) -- reuse it rather than re-deriving + # other subdevice's owned hrefs) -- reuse it rather than re-deriving # owned_elsewhere here too. - main_view = canonical_view(MAIN, resources, sub_units) + main_view = canonical_view(MAIN, resources, subdevices) reg = resolve_registry(main_view) caps, pats = ( @@ -463,29 +467,29 @@ def discover_partitioned( ) # MAIN is never gated -- the config entry's own physical connection # always materializes regardless of what its entities' values are. - bound = discover(main_view, caps, pats, log=log, tier_log=tier_log, sub_unit=MAIN) + bound = discover(main_view, caps, pats, log=log, tier_log=tier_log, subdevice=MAIN) device_type_name = reg.name if reg is not None else None - materialized: list[SubUnit] = [] - skipped: list[SkippedSubUnit] = [] + materialized: list[Subdevice] = [] + skipped: list[SkippedSubdevice] = [] - for su in sub_units: - view = canonical_view(su, resources, sub_units) + for su in subdevices: + view = canonical_view(su, resources, subdevices) su_reg = resolve_registry(view) or reg su_caps, su_pats = ( (su_reg.capabilities, su_reg.pattern_capabilities) if su_reg is not None else (fallback_capabilities, []) ) - probe_bound = discover(view, su_caps, su_pats, sub_unit=su) + probe_bound = discover(view, su_caps, su_pats, subdevice=su) probe_state = flatten(probe_bound, resources) if _has_live_primary_entity(probe_bound, probe_state): materialized.append(su) bound = bound + discover( - view, su_caps, su_pats, log=log, tier_log=tier_log, sub_unit=su, + view, su_caps, su_pats, log=log, tier_log=tier_log, subdevice=su, ) else: - skipped.append(SkippedSubUnit( - sub_unit=su, + skipped.append(SkippedSubdevice( + subdevice=su, hrefs=tuple(sorted({b.href for b in probe_bound})), )) diff --git a/custom_components/localthings/select.py b/custom_components/localthings/select.py index 0747510..da56992 100644 --- a/custom_components/localthings/select.py +++ b/custom_components/localthings/select.py @@ -104,7 +104,7 @@ class LocalThingsSelect(LocalThingsEntity, SelectEntity): # list decoded from a sibling resource. There is no static # fallback: when that resource isn't populated the callable # returns [] and the entity's exists_fn suppresses it entirely. - # This entity's own sub-unit's canonical view (issue #177), not + # This entity's own subdevice's canonical view (issue #177), not # the raw actual-href snapshot -- see LocalThingsEntity._resources. return list(desc.options(self._resources) or []) if desc.options_field: diff --git a/tests/conftest.py b/tests/conftest.py index 511e422..70a937e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -18,7 +18,7 @@ def _load_device(name: str) -> dict[str, dict]: def _load_device_full(name: str): """Like _load_device, but also returns the optional `oic_res`/`seeds` - keys a sub-unit-capable fixture (issue #177) may carry alongside + keys a subdevice-capable fixture (issue #177) may carry alongside `device0` -- see the two `airconditioner_*` fixtures with a `seeds_note` field. `oic_res`/`seeds` default to `[]`/`{}` for every other fixture, so this is safe to call on any fixture in the corpus. @@ -44,9 +44,9 @@ class FakeCoapSession: fixture's `seeds` map (raw device0-batch-shaped lists keyed by seed href -- plus any `probes` entries, which are plain Property maps rather than batch lists; both are just CBOR bodies at this layer, and the two - readers in registry.subunits already type-check what they get back). - Enough surface for registry.subunits.enumerate_sub_units and - LocalThingsCoordinator's blocking sub-unit polls to run against fixture + readers in registry.subdevices already type-check what they get back). + Enough surface for registry.subdevices.enumerate_subdevices and + LocalThingsCoordinator's blocking subdevice polls to run against fixture data without a live device -- same idea as test_identity.py's FakeSession, but keyed by href string (post path-join) rather than a path tuple, since callers here pass a `seed_path` tuple straight @@ -69,16 +69,16 @@ class FakeCoapSession: def _discover_full(resources: dict[str, dict], oic_res, seeds: dict[str, list]): - """Run the *whole* sub-unit-aware discovery pipeline against fixture + """Run the *whole* subdevice-aware discovery pipeline against fixture data, HA-free -- mirrors exactly what LocalThingsCoordinator does across - _enumerate_sub_units_blocking + _run_discovery (issue #177), so a test + _enumerate_subdevices_blocking + _run_discovery (issue #177), so a test exercising this exercises the real code path, not a re-implementation of it. See the adding-device-support skill's section 2 for the plain - (non-sub-unit) equivalent this extends. + (non-subdevice) equivalent this extends. Returns `(bound, materialized, skipped, full_resources, device_type_name)`: - - `bound`: every BoundEntity, main + every materialized sub-unit. - - `materialized`/`skipped`: SubUnit / SkippedSubUnit lists straight from + - `bound`: every BoundEntity, main + every materialized subdevice. + - `materialized`/`skipped`: Subdevice / SkippedSubdevice lists straight from discover_partitioned. - `full_resources`: `resources` merged with every candidate's seed data (actual hrefs) -- what a coordinator's cache would hold. @@ -86,12 +86,12 @@ def _discover_full(resources: dict[str, dict], oic_res, seeds: dict[str, list]): """ from custom_components.localthings.registry.by_type import resolve from custom_components.localthings.registry.registry import CAPABILITIES - from custom_components.localthings.registry.subunits import ( - discover_partitioned, enumerate_sub_units, + from custom_components.localthings.registry.subdevices import ( + discover_partitioned, enumerate_subdevices, ) sess = FakeCoapSession(seeds) - candidates, extra = enumerate_sub_units(sess, resources, oic_res) + candidates, extra = enumerate_subdevices(sess, resources, oic_res) full_resources = {**resources, **extra} bound, device_type_name, materialized, skipped = discover_partitioned( full_resources, candidates, resolve, CAPABILITIES, diff --git a/tests/fixtures/airconditioner_artik051_dongle_fac_18k_device.json b/tests/fixtures/airconditioner_artik051_dongle_fac_18k_device.json index f5ede42..c83e4fd 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 unit). 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 unit -- populated power/mode/temperature and its own /information/vs/1 reporting ARTIK051_DONGLE_FAC_RAC_18K (RAC = the wall-mounted unit) 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 sub-unit 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 units, not 3." + "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." } diff --git a/tests/fixtures/airconditioner_fac_bora_2in1_device.json b/tests/fixtures/airconditioner_fac_bora_2in1_device.json index fa53abf..cb656e5 100644 --- a/tests/fixtures/airconditioner_fac_bora_2in1_device.json +++ b/tests/fixtures/airconditioner_fac_bora_2in1_device.json @@ -727,7 +727,7 @@ "rep": { "x.com.samsung.da.modelNum": "TP2X_FAC_BORA_RAC_21K|10233041|600001110015110006000C1200830000", "x.com.samsung.da.description": "TP2X_FAC_BORA_RAC_21K", - "x.com.samsung.da.serialNum": "TEST-SUBUNIT-SERIAL-0000" + "x.com.samsung.da.serialNum": "TEST-SUBDEVICE-SERIAL-0000" } }, { @@ -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 unit 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 unit, distinct from the master's floor-unit TP2X_FAC_BORA_21K. Every other href in this seed batch is CONSTRUCTED (never read from this unit) so the sub-unit'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 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." } diff --git a/tests/fixtures/golden/airconditioner_artik051_dongle_fac_18k.json b/tests/fixtures/golden/airconditioner_artik051_dongle_fac_18k.json index 304b395..379fcbb 100644 --- a/tests/fixtures/golden/airconditioner_artik051_dongle_fac_18k.json +++ b/tests/fixtures/golden/airconditioner_artik051_dongle_fac_18k.json @@ -16,12 +16,12 @@ "power_energy_kwh", "power_watts", "super_fine_dust", - "unit1_alarm_code", - "unit1_auto_clean_legacy", - "unit1_beep", - "unit1_climate", - "unit1_current_temperature_c", - "unit1_good_sleep", - "unit1_humidity" + "subdevice1_alarm_code", + "subdevice1_auto_clean_legacy", + "subdevice1_beep", + "subdevice1_climate", + "subdevice1_current_temperature_c", + "subdevice1_good_sleep", + "subdevice1_humidity" ] } diff --git a/tests/fixtures/golden/airconditioner_fac_bora_2in1.json b/tests/fixtures/golden/airconditioner_fac_bora_2in1.json index 065ef57..52a2513 100644 --- a/tests/fixtures/golden/airconditioner_fac_bora_2in1.json +++ b/tests/fixtures/golden/airconditioner_fac_bora_2in1.json @@ -15,8 +15,8 @@ "humidity", "power_energy_kwh", "power_watts", - "sub_6c2dff6dee5cdad16a5e000000000001_climate", - "sub_6c2dff6dee5cdad16a5e000000000001_current_temperature_c", + "subdevice_6c2dff6dee5cdad16a5e000000000001_climate", + "subdevice_6c2dff6dee5cdad16a5e000000000001_current_temperature_c", "tropical_night_mode" ] } diff --git a/tests/test_air_purifier_airflow_fan.py b/tests/test_air_purifier_airflow_fan.py index c28a358..f67626b 100644 --- a/tests/test_air_purifier_airflow_fan.py +++ b/tests/test_air_purifier_airflow_fan.py @@ -20,9 +20,9 @@ class _FakeCoordinator: def resource(self, href): return self.last_resources.get(href, {}) - def canonical_resources(self, sub_unit): + def canonical_resources(self, subdevice): # Every bound entity in this test uses the default MAIN - # sub-unit, so the canonical view is just the raw snapshot + # subdevice, so the canonical view is just the raw snapshot # (issue #177 -- see LocalThingsEntity._resources). return self.last_resources diff --git a/tests/test_air_purifier_vtww_fan.py b/tests/test_air_purifier_vtww_fan.py index c3e2496..e3e7742 100644 --- a/tests/test_air_purifier_vtww_fan.py +++ b/tests/test_air_purifier_vtww_fan.py @@ -29,9 +29,9 @@ class _FakeCoordinator: def resource(self, href): return self.last_resources.get(href, {}) - def canonical_resources(self, sub_unit): + def canonical_resources(self, subdevice): # Every bound entity in this test uses the default MAIN - # sub-unit, so the canonical view is just the raw snapshot + # subdevice, so the canonical view is just the raw snapshot # (issue #177 -- see LocalThingsEntity._resources). return self.last_resources diff --git a/tests/test_airconditioner_artik051_krac.py b/tests/test_airconditioner_artik051_krac.py index fa0eb29..432e517 100644 --- a/tests/test_airconditioner_artik051_krac.py +++ b/tests/test_airconditioner_artik051_krac.py @@ -47,9 +47,9 @@ class _FakeCoordinator: def resource(self, href): return self.last_resources.get(href, {}) - def canonical_resources(self, sub_unit): + def canonical_resources(self, subdevice): # Every bound entity in this test uses the default MAIN - # sub-unit, so the canonical view is just the raw snapshot + # subdevice, so the canonical view is just the raw snapshot # (issue #177 -- see LocalThingsEntity._resources). return self.last_resources diff --git a/tests/test_airconditioner_tp1x_rac_01001_fan.py b/tests/test_airconditioner_tp1x_rac_01001_fan.py index 0c208ce..1085728 100644 --- a/tests/test_airconditioner_tp1x_rac_01001_fan.py +++ b/tests/test_airconditioner_tp1x_rac_01001_fan.py @@ -34,9 +34,9 @@ class _FakeCoordinator: def resource(self, href): return self.last_resources.get(href, {}) - def canonical_resources(self, sub_unit): + def canonical_resources(self, subdevice): # Every bound entity in this test uses the default MAIN - # sub-unit, so the canonical view is just the raw snapshot + # subdevice, so the canonical view is just the raw snapshot # (issue #177 -- see LocalThingsEntity._resources). return self.last_resources diff --git a/tests/test_climate_ac_modes.py b/tests/test_climate_ac_modes.py index e3b36e1..c92abd4 100644 --- a/tests/test_climate_ac_modes.py +++ b/tests/test_climate_ac_modes.py @@ -106,8 +106,8 @@ def test_fac_bora_wind_strength_codes_fit_the_standard_scale(): def resource(self, href): return self.last_resources.get(href, {}) - def canonical_resources(self, sub_unit): - # This test's entity uses the default MAIN sub-unit, so the + def canonical_resources(self, subdevice): + # This test's entity uses the default MAIN subdevice, so the # canonical view is just the raw snapshot (issue #177). return self.last_resources diff --git a/tests/test_climate_subunit.py b/tests/test_climate_subdevice.py similarity index 52% rename from tests/test_climate_subunit.py rename to tests/test_climate_subdevice.py index 097c0a7..e5c45a1 100644 --- a/tests/test_climate_subunit.py +++ b/tests/test_climate_subdevice.py @@ -1,12 +1,12 @@ -"""Tests that a sub-unit's LocalThingsClimate entity (issue #177) reads its +"""Tests that a subdevice's LocalThingsClimate entity (issue #177) reads its *own* power/mode/temperature -- not the master's, and not some mix of the two -- and that the legacy-board test (is_legacy_board/_legacy_airflow) is -evaluated per unit rather than once globally. +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 units. +warn is easy to get wrong if the canonical view leaks between subdevices. """ from __future__ import annotations @@ -21,12 +21,12 @@ from tests.test_subdevice_discovery import _coordinator, _discover def _climate_entities(coordinator): - """{sub_unit_key_or_None: LocalThingsClimate}, None standing for MAIN.""" - from custom_components.localthings.registry.subunits import MAIN + """{subdevice_key_or_None: LocalThingsClimate}, None standing for MAIN.""" + from custom_components.localthings.registry.subdevices import MAIN out = {} for b in coordinator.bound: if isinstance(b.desc, ClimateDesc): - key = None if b.sub_unit == MAIN else b.sub_unit.key + key = None if b.subdevice == MAIN else b.subdevice.key out[key] = LocalThingsClimate(coordinator, b) return out @@ -38,66 +38,66 @@ async def climates(hass: HomeAssistant): return _climate_entities(coordinator) -async def test_sub_unit_climate_reads_its_own_mode_and_power(climates): - """Master reports 'Auto'; the bedroom unit (/device/1) reports 'Cool' -- - confirmed distinct in the real captured fixture. If the sub-unit - entity's _rep() weren't translating through its own sub_unit, it would +async def test_subdevice_climate_reads_its_own_mode_and_power(climates): + """Master reports 'Auto'; the bedroom subdevice (/device/1) reports 'Cool' -- + confirmed distinct in the real captured fixture. If the subdevice + entity's _rep() weren't translating through its own subdevice, it would read the master's /mode/vs/0 instead and report the master's mode.""" - main, unit1 = climates[None], climates['1'] + main, sub1 = climates[None], climates['1'] assert main.hvac_mode == HVACMode.AUTO - assert unit1.hvac_mode == HVACMode.COOL + assert sub1.hvac_mode == HVACMode.COOL -async def test_sub_unit_climate_reads_its_own_temperature(climates): - """Master: current 25.0 / desired 26.0. Unit 1: current 27.0 / desired +async def test_subdevice_climate_reads_its_own_temperature(climates): + """Master: current 25.0 / desired 26.0. Subdevice 1: current 27.0 / desired 28.0 -- distinct values in the real fixture, so a href mix-up here would show up as a wrong number, not just a wrong mode string.""" - main, unit1 = climates[None], climates['1'] + main, sub1 = climates[None], climates['1'] assert main.current_temperature == 25.0 assert main.target_temperature == 26.0 - assert unit1.current_temperature == 27.0 - assert unit1.target_temperature == 28.0 + assert sub1.current_temperature == 27.0 + assert sub1.target_temperature == 28.0 -async def test_sub_unit_climate_reads_its_own_power_state(climates): - """Both units happen to report power On in this fixture -- this at - least confirms _is_on() reads the *unit's own* /power/vs/, not a +async def test_subdevice_climate_reads_its_own_power_state(climates): + """Both subdevices happen to report power On in this fixture -- this at + least confirms _is_on() reads the *subdevice's own* /power/vs/, not a hardcoded /power/vs/0, by checking the entity resolves without falling back to OFF (which _is_on() would do if it silently read an absent - href instead of the sub-unit's actual one).""" - main, unit1 = climates[None], climates['1'] + href instead of the subdevice's actual one).""" + main, sub1 = climates[None], climates['1'] assert main.hvac_mode != HVACMode.OFF - assert unit1.hvac_mode != HVACMode.OFF + assert sub1.hvac_mode != HVACMode.OFF -async def test_legacy_board_test_is_evaluated_per_unit(climates): - """HJcom's board has no /wind/* resources at all on *either* unit -- +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 sub-unit's own canonical + 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 unit's hrefs into the other's), this - wouldn't distinguish "this unit is legacy" from "some unit on this - connection is legacy" -- and a future board with one legacy + one - modern unit sharing a connection would silently read the wrong fan/ - swing channel on one side.""" - main, unit1 = climates[None], climates['1'] + 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 unit1._legacy_airflow() != {} + assert sub1._legacy_airflow() != {} # Both resolve to *some* fan mode via the legacy path rather than the # /wind/strength/vs/0 channel (absent on this board) -- confirms # is_legacy_board(self._resources) actually gated fan_mode's branch, # not just that _legacy_airflow() itself returned something. assert main.fan_mode is not None - assert unit1.fan_mode is not None + assert sub1.fan_mode is not None -async def test_sub_unit_climate_writes_are_scoped_to_its_own_bound_entity(climates): +async def test_subdevice_climate_writes_are_scoped_to_its_own_bound_entity(climates): """A sanity check that the two entities are backed by genuinely different BoundEntity objects (different hrefs), which is what makes - per-unit reads/writes possible at all -- see async_send_command's + per-subdevice reads/writes possible at all -- see async_send_command's translation test in test_coordinator_send_command.py for the write side of this.""" - main, unit1 = climates[None], climates['1'] + main, sub1 = climates[None], climates['1'] assert main._bound.href == '/mode/vs/0' - assert unit1._bound.href == '/mode/vs/1' - assert main._bound is not unit1._bound + assert sub1._bound.href == '/mode/vs/1' + assert main._bound is not sub1._bound diff --git a/tests/test_coordinator_send_command.py b/tests/test_coordinator_send_command.py index d611950..dec826e 100644 --- a/tests/test_coordinator_send_command.py +++ b/tests/test_coordinator_send_command.py @@ -28,7 +28,7 @@ from custom_components.localthings.registry.capabilities import laundry from custom_components.localthings.registry.capabilities.airconditioner import _climate_write from custom_components.localthings.registry.discovery import BoundEntity from custom_components.localthings.registry.entities import ClimateDesc -from custom_components.localthings.registry.subunits import SubUnit +from custom_components.localthings.registry.subdevices import Subdevice ENTRY_DATA = { CONF_HOST: '10.0.0.199', @@ -98,26 +98,26 @@ async def test_options_write_optimistic_cache_keeps_sibling_tokens(coordinator) # --------------------------------------------------------------------------- -# Sub-unit write translation (issue #177): a composite-entity write_fn (here +# Subdevice write translation (issue #177): a composite-entity write_fn (here # airconditioner._climate_write) returns *canonical* path_segs # (['power', 'vs', '0']) -- async_send_command must translate that through -# bound_entity.sub_unit.to_actual before POSTing, applying the optimistic -# value, and starting the settle guard, or a sub-unit's climate card would +# bound_entity.subdevice.to_actual before POSTing, applying the optimistic +# value, and starting the settle guard, or a subdevice's climate card would # write to (and read confirmation from) the master's resource instead of # its own. # --------------------------------------------------------------------------- -def _climate_bound(href: str, sub_unit: SubUnit) -> BoundEntity: +def _climate_bound(href: str, subdevice: Subdevice) -> BoundEntity: desc = ClimateDesc(key='climate', translation_key='airconditioner', write_fn=_climate_write) - return BoundEntity(href=href, capability=None, desc=desc, sub_unit=sub_unit) + return BoundEntity(href=href, capability=None, desc=desc, subdevice=subdevice) -async def test_indexed_sub_unit_write_posts_to_translated_path(coordinator) -> None: - """A power write from the bedroom unit's (indexed '1') climate entity +async def test_indexed_subdevice_write_posts_to_translated_path(coordinator) -> None: + """A power write from the bedroom subdevice's (indexed '1') climate entity must POST to /power/vs/1, not the master's /power/vs/0.""" - unit1 = SubUnit(kind='indexed', key='1', seed_path=('device', '1')) - bound = _climate_bound('/mode/vs/1', unit1) + sub1 = Subdevice(kind='indexed', key='1', seed_path=('device', '1')) + bound = _climate_bound('/mode/vs/1', sub1) await coordinator.async_send_command(bound, ('power', True)) @@ -126,11 +126,11 @@ async def test_indexed_sub_unit_write_posts_to_translated_path(coordinator) -> N assert cbor2.loads(posted_bytes) == {'x.com.samsung.da.power': 'On'} -async def test_indexed_sub_unit_write_applies_optimistic_value_to_translated_href( +async def test_indexed_subdevice_write_applies_optimistic_value_to_translated_href( coordinator, ) -> None: - unit1 = SubUnit(kind='indexed', key='1', seed_path=('device', '1')) - bound = _climate_bound('/mode/vs/1', unit1) + sub1 = Subdevice(kind='indexed', key='1', seed_path=('device', '1')) + bound = _climate_bound('/mode/vs/1', sub1) await coordinator.async_send_command(bound, ('power', True)) @@ -142,7 +142,7 @@ async def test_indexed_sub_unit_write_applies_optimistic_value_to_translated_hre # The settle guard is armed on that same translated href: a stale poll # reporting the pre-write value must be dropped, not allowed to revert # the optimistic 'On' (issue #27's regression, translated to a - # sub-unit's own resource). + # subdevice's own resource). applied = coordinator._observe.apply( '/power/vs/1', {'x.com.samsung.da.power': 'Off'}, source='poll', ) @@ -150,12 +150,12 @@ async def test_indexed_sub_unit_write_applies_optimistic_value_to_translated_hre assert coordinator._cache.get('/power/vs/1') == {'x.com.samsung.da.power': 'On'} -async def test_prefixed_sub_unit_write_posts_to_translated_path(coordinator) -> None: - """A power write from a UUID-prefixed unit's climate entity must POST +async def test_prefixed_subdevice_write_posts_to_translated_path(coordinator) -> None: + """A power write from a UUID-prefixed subdevice's climate entity must POST to //power/vs/0, not the bare canonical href.""" sub_id = '6c2dff6d-ee5c-dad1-6a5e-000000000001' - unit = SubUnit(kind='prefixed', key=sub_id, seed_path=(sub_id, 'device', '0')) - bound = _climate_bound(f'/{sub_id}/mode/vs/0', unit) + subdevice = Subdevice(kind='prefixed', key=sub_id, seed_path=(sub_id, 'device', '0')) + bound = _climate_bound(f'/{sub_id}/mode/vs/0', subdevice) await coordinator.async_send_command(bound, ('power', True)) @@ -167,11 +167,11 @@ async def test_prefixed_sub_unit_write_posts_to_translated_path(coordinator) -> } -async def test_main_climate_write_unaffected_by_sub_unit_translation(coordinator) -> None: +async def test_main_climate_write_unaffected_by_subdevice_translation(coordinator) -> None: """MAIN's to_actual is the identity transform -- a device with no - sub-units must keep posting to the exact same path as before this + subdevices must keep posting to the exact same path as before this translation step existed.""" - from custom_components.localthings.registry.subunits import MAIN + from custom_components.localthings.registry.subdevices import MAIN bound = _climate_bound('/mode/vs/0', MAIN) await coordinator.async_send_command(bound, ('power', True)) diff --git a/tests/test_diagnostics_subunits.py b/tests/test_diagnostics_subdevices.py similarity index 71% rename from tests/test_diagnostics_subunits.py rename to tests/test_diagnostics_subdevices.py index 3823d42..2cba348 100644 --- a/tests/test_diagnostics_subunits.py +++ b/tests/test_diagnostics_subdevices.py @@ -1,5 +1,5 @@ -"""Sanity check for diagnostics.py's issue #177 additions (sub_units, -sub_units_skipped, sub_unit_probes) against a real composite-device +"""Sanity check for diagnostics.py's issue #177 additions (subdevices, +subdevices_skipped, subdevice_probes) against a real composite-device discovery run -- makes sure the new blocks are actually reachable/shaped right, not just that the coordinator's own attributes look correct in isolation (test_subdevice_discovery.py covers that).""" @@ -15,7 +15,7 @@ from custom_components.localthings.diagnostics import ( from tests.test_subdevice_discovery import _coordinator, _discover -async def test_diagnostics_reports_materialized_and_skipped_sub_units( +async def test_diagnostics_reports_materialized_and_skipped_subdevices( hass: HomeAssistant, enable_custom_integrations, ) -> None: coordinator = _coordinator(hass) @@ -24,27 +24,27 @@ async def test_diagnostics_reports_materialized_and_skipped_sub_units( diag = await async_get_config_entry_diagnostics(hass, coordinator._entry) - sub_unit_keys = {su['key'] for su in diag['sub_units']} - skipped_keys = {su['key'] for su in diag['sub_units_skipped']} - assert sub_unit_keys == {'1'} + subdevice_keys = {su['key'] for su in diag['subdevices']} + skipped_keys = {su['key'] for su in diag['subdevices_skipped']} + assert subdevice_keys == {'1'} assert skipped_keys == {'2'} - assert diag['sub_units'][0]['bound_entity_count'] > 0 - assert diag['sub_units_skipped'][0]['hrefs'] - assert '/multidevice/vs/0' in diag['sub_unit_probes'] + assert diag['subdevices'][0]['bound_entity_count'] > 0 + assert diag['subdevices_skipped'][0]['hrefs'] + assert '/multidevice/vs/0' in diag['subdevice_probes'] # The reporter's hand-read value, carried in the fixture's `probes` map # (it belongs to no batch -- see that fixture's seeds_note). Two real - # units, master + bedroom, independently corroborating the gate's + # subdevices, master + bedroom, independently corroborating the gate's # decision to skip /device/2. Reported on its own, never as a resource. assert diag['multidevice'] == {'x.com.samsung.da.numofsubdevice': '2'} assert '/multidevice/vs/0' not in diag['resources'] -async def test_top_level_resources_is_the_master_unit_only( +async def test_top_level_resources_is_the_master_subdevice_only( hass: HomeAssistant, enable_custom_integrations, ) -> None: - """`resources` reports this unit's own hrefs and nothing else. + """`resources` reports this subdevice's own hrefs and nothing else. - It used to be `last_resources` raw -- the union across every live unit, + It used to be `last_resources` raw -- the union across every live subdevice, keyed by real hrefs -- so a sibling's /mode/vs/1 sat in it alongside the master's /mode/vs/0 with no attribution, contradicting both the module docstring and the adding-device-support skill ("the parsed /device/0 @@ -60,10 +60,10 @@ async def test_top_level_resources_is_the_master_unit_only( assert '/mode/vs/0' in diag['resources'] # The sibling's own block carries its state, canonicalized -- '/mode/vs/0', # not the '/mode/vs/1' it actually answers on. - unit1 = diag['sub_units'][0] - assert '/mode/vs/0' in unit1['resources'] - assert not [h for h in unit1['resources'] if h.endswith('/1')] - assert unit1['resources']['/power/vs/0']['x.com.samsung.da.power'] == 'On' + sub1 = diag["subdevices"][0] + assert '/mode/vs/0' in sub1["resources"] + assert not [h for h in sub1['resources'] if h.endswith('/1')] + assert sub1["resources"]["/power/vs/0"]['x.com.samsung.da.power'] == 'On' async def test_rejected_candidate_reps_reach_diagnostics_but_not_the_cache( @@ -72,7 +72,7 @@ async def test_rejected_candidate_reps_reach_diagnostics_but_not_the_cache( """A gate-rejected slot is never polled again, so anything applied to the state cache for it would sit frozen at its first-discovery value while looking as live as every other href. It's kept out of the cache entirely - and reported only under its own sub_units_skipped entry -- which is also + and reported only under its own subdevices_skipped entry -- which is also the only place a reader can go to second-guess the gate.""" coordinator = _coordinator(hass) await _discover(coordinator, 'airconditioner_artik051_dongle_fac_18k') @@ -81,14 +81,14 @@ async def test_rejected_candidate_reps_reach_diagnostics_but_not_the_cache( assert not [h for h in coordinator.last_resources if h.endswith('/2')] diag = await async_get_config_entry_diagnostics(hass, coordinator._entry) - skipped = diag['sub_units_skipped'][0] + skipped = diag['subdevices_skipped'][0] # Present, canonicalized, and visibly the empty state the gate rejected. assert skipped['resources']['/power/vs/0'] == {} assert skipped['resources']['/mode/vs/0'] == {} assert skipped['resources']['/information/vs/0'] -async def test_diagnostics_reports_prefixed_sub_unit( +async def test_diagnostics_reports_prefixed_subdevice( hass: HomeAssistant, enable_custom_integrations, ) -> None: coordinator = _coordinator(hass) @@ -97,11 +97,11 @@ async def test_diagnostics_reports_prefixed_sub_unit( diag = await async_get_config_entry_diagnostics(hass, coordinator._entry) - assert len(diag['sub_units']) == 1 - assert diag['sub_units'][0]['kind'] == 'prefixed' + assert len(diag['subdevices']) == 1 + assert diag['subdevices'][0]['kind'] == 'prefixed' # diagnostics reports the raw modelNum field verbatim (board revision/ # capability-bitmap suffix included), unlike device_info_for's model - # (split at '|') -- confirms it's still the wall unit's own identity, + # (split at '|') -- confirms it's still the wall subdevice's own identity, # not the master's TP2X_FAC_BORA_21K. - assert diag['sub_units'][0]['model'].startswith('TP2X_FAC_BORA_RAC_21K') - assert diag['sub_units_skipped'] == [] + assert diag['subdevices'][0]['model'].startswith('TP2X_FAC_BORA_RAC_21K') + assert diag['subdevices_skipped'] == [] diff --git a/tests/test_entity.py b/tests/test_entity.py index 05d0660..24ee791 100644 --- a/tests/test_entity.py +++ b/tests/test_entity.py @@ -17,12 +17,12 @@ class _FakeCoordinator: def __init__(self, last_resources): self.last_resources = last_resources - def canonical_resources(self, sub_unit): + def canonical_resources(self, subdevice): # Every bound entity in this test file uses the default MAIN - # sub-unit (identity transform), so the canonical view is just the + # subdevice (identity transform), so the canonical view is just the # raw snapshot -- same shape as the real # LocalThingsCoordinator.canonical_resources for a device with no - # sub-units (issue #177). + # subdevices (issue #177). return self.last_resources diff --git a/tests/test_golden_regression.py b/tests/test_golden_regression.py index 72800f4..1733eae 100644 --- a/tests/test_golden_regression.py +++ b/tests/test_golden_regression.py @@ -859,12 +859,12 @@ def test_registry_reproduces_golden_state_keys_for_airconditioner_fac_bora(): ) -def _new_sub_unit_aware_state_keys(name): - """Like _new_state_keys, but runs the full sub-unit-aware pipeline - (enumerate_sub_units + discover_partitioned, issue #177) instead of a +def _new_subdevice_aware_state_keys(name): + """Like _new_state_keys, but runs the full subdevice-aware pipeline + (enumerate_subdevices + discover_partitioned, issue #177) instead of a single discover() call, so the golden for a composite-device fixture - captures every materialized unit's keys (unit1_-/sub__-prefixed), - not just the master's.""" + captures every materialized subdevice's keys + (subdevice1_-/subdevice__-prefixed), not just the master's.""" from custom_components.localthings.registry.adapter import flatten from tests.conftest import _discover_full, _load_device_full resources, oic_res, seeds = _load_device_full(name) @@ -877,15 +877,16 @@ def _new_sub_unit_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 - siblings): a real v0.16.0 dump with a genuine second indoor unit at - `/device/1` (unit1_-prefixed keys below) and an unused SmartThings slot - at `/device/2` that answers its seed but never produces a materialized - unit (see DESIGN-177.md section 4 and test_subdevice_discovery.py's - explicit "/device/2 produces no entities" assertion) -- so this golden - has no `unit2_`-prefixed keys at all, which is the point.""" + 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 + materialized subdevice (see DESIGN-177.md section 4 and + test_subdevice_discovery.py's explicit "/device/2 produces no entities" + assertion) -- so this golden has no `subdevice2_`-prefixed keys at all, + which is the point.""" name = 'airconditioner_artik051_dongle_fac_18k' golden = json.loads((GOLDEN / f'{name}.json').read_text()) - state_keys = _new_sub_unit_aware_state_keys(name) + 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" @@ -915,17 +916,17 @@ 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 - tree): device0/oic_res are real; the wall-mounted sub-unit's own + 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 enough to bind a real climate card under the - `sub_6c2dff6dee5cdad16a5e000000000001_` prefix below. Distinct from + `subdevice_6c2dff6dee5cdad16a5e000000000001_` prefix below. Distinct from tests/fixtures/airconditioner_fac_bora_device.json, which is deliberately left unchanged as the redacted-subdeviceIdList regression - case (zero sub-units).""" + case (zero subdevices).""" name = 'airconditioner_fac_bora_2in1' golden = json.loads((GOLDEN / f'{name}.json').read_text()) - state_keys = _new_sub_unit_aware_state_keys(name) + 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" diff --git a/tests/test_range_hood_fan.py b/tests/test_range_hood_fan.py index b1c3e55..cdeb08e 100644 --- a/tests/test_range_hood_fan.py +++ b/tests/test_range_hood_fan.py @@ -19,9 +19,9 @@ class _FakeCoordinator: def resource(self, href): return self.last_resources.get(href, {}) - def canonical_resources(self, sub_unit): + def canonical_resources(self, subdevice): # Every bound entity in this test uses the default MAIN - # sub-unit, so the canonical view is just the raw snapshot + # subdevice, so the canonical view is just the raw snapshot # (issue #177 -- see LocalThingsEntity._resources). return self.last_resources diff --git a/tests/test_select_options.py b/tests/test_select_options.py index 61192a6..aaeba47 100644 --- a/tests/test_select_options.py +++ b/tests/test_select_options.py @@ -14,9 +14,9 @@ class _FakeCoordinator: def __init__(self, last_resources): self.last_resources = last_resources - def canonical_resources(self, sub_unit): + def canonical_resources(self, subdevice): # Every entity built by _make_select uses the default MAIN - # sub-unit, so the canonical view is just the raw snapshot + # subdevice, so the canonical view is just the raw snapshot # (issue #177 -- see LocalThingsEntity._resources). return self.last_resources diff --git a/tests/test_subdevice_discovery.py b/tests/test_subdevice_discovery.py index f172984..5e96ca4 100644 --- a/tests/test_subdevice_discovery.py +++ b/tests/test_subdevice_discovery.py @@ -1,7 +1,7 @@ """End-to-end discovery tests for issue #177's two composite-device fixtures, against the real LocalThingsCoordinator (not the HA-free -registry-level helpers test_subunits.py/test_unique_ids.py use) -- this is -what actually exercises _enumerate_sub_units_blocking + _run_discovery +registry-level helpers test_subdevices.py/test_unique_ids.py use) -- this is +what actually exercises _enumerate_subdevices_blocking + _run_discovery together, including device_info_for/via_device and the "no phantom /device/2 entities" guarantee. """ @@ -40,7 +40,7 @@ async def _discover(coordinator: LocalThingsCoordinator, name: str) -> 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_sub_units_blocking/_run_discovery.""" + _enumerate_subdevices_blocking/_run_discovery.""" resources, oic_res, seeds = _load_device_full(name) coordinator._session = FakeCoapSession(seeds) # _connect_session (skipped here -- the session is pre-set) is what @@ -52,7 +52,7 @@ async def _discover(coordinator: LocalThingsCoordinator, name: str) -> None: device_types=(), raw={'/oic/p': {}, '/oic/d': {}, '/oic/res': oic_res}, ) merged = await coordinator.hass.async_add_executor_job( - coordinator._enumerate_sub_units_blocking, resources, + coordinator._enumerate_subdevices_blocking, resources, ) # Mirror _async_update_data's first-cycle order exactly: discover, then # drop the candidates the liveness gate rejected, then apply what's left @@ -63,17 +63,17 @@ async def _discover(coordinator: LocalThingsCoordinator, name: str) -> None: # first here would leave this helper testing an ordering production no # longer uses. coordinator._run_discovery(merged) - for href, rep in coordinator._live_unit_resources(merged).items(): + for href, rep in coordinator._live_subdevice_resources(merged).items(): coordinator._observe.apply(href, rep, source='poll') -def _climate_bound(coordinator, sub_unit_key: str): - from custom_components.localthings.registry.subunits import MAIN +def _climate_bound(coordinator, subdevice_key: str): + from custom_components.localthings.registry.subdevices import MAIN for b in coordinator.bound: if isinstance(b.desc, ClimateDesc): - if sub_unit_key is None and b.sub_unit == MAIN: + if subdevice_key is None and b.subdevice == MAIN: return b - if sub_unit_key is not None and b.sub_unit.key == sub_unit_key: + if subdevice_key is not None and b.subdevice.key == subdevice_key: return b return None @@ -82,18 +82,18 @@ def _climate_bound(coordinator, sub_unit_key: str): # HJcom -- ARTIK051_DONGLE_FAC_18K, Pattern A (indexed siblings) # --------------------------------------------------------------------------- -async def test_hjcom_materializes_master_and_bedroom_unit(hass: HomeAssistant): +async def test_hjcom_materializes_master_and_bedroom_subdevice(hass: HomeAssistant): coordinator = _coordinator(hass) await _discover(coordinator, 'airconditioner_artik051_dongle_fac_18k') - assert [su.key for su in coordinator.sub_units] == ['1'] + assert [su.key for su in coordinator.subdevices] == ['1'] main_climate = _climate_bound(coordinator, None) - unit1_climate = _climate_bound(coordinator, '1') + sub1_climate = _climate_bound(coordinator, '1') assert main_climate is not None - assert unit1_climate is not None + assert sub1_climate is not None assert main_climate.href == '/mode/vs/0' - assert unit1_climate.href == '/mode/vs/1' + assert sub1_climate.href == '/mode/vs/1' async def test_hjcom_device_2_produces_no_entities_at_all(hass: HomeAssistant): @@ -104,27 +104,27 @@ async def test_hjcom_device_2_produces_no_entities_at_all(hass: HomeAssistant): coordinator = _coordinator(hass) await _discover(coordinator, 'airconditioner_artik051_dongle_fac_18k') - assert '2' not in [su.key for su in coordinator.sub_units] + assert '2' not in [su.key for su in coordinator.subdevices] assert any( - skip.sub_unit.kind == 'indexed' and skip.sub_unit.key == '2' - for skip in coordinator._skipped_sub_units + skip.subdevice.kind == 'indexed' and skip.subdevice.key == '2' + for skip in coordinator._skipped_subdevices ) - assert not any(b.sub_unit.key == '2' for b in coordinator.bound) + assert not any(b.subdevice.key == '2' for b in coordinator.bound) assert not any(href.endswith('/2') for href in coordinator._hot_hrefs) assert not any(href.endswith('/2') for href in coordinator._warm_hrefs) -async def test_hjcom_unit1_device_info_links_via_device_to_master(hass: HomeAssistant): +async def test_hjcom_sub1_device_info_links_via_device_to_master(hass: HomeAssistant): coordinator = _coordinator(hass) await _discover(coordinator, 'airconditioner_artik051_dongle_fac_18k') - unit1 = next(su for su in coordinator.sub_units if su.key == '1') - info = coordinator.device_info_for(unit1) + sub1 = next(su for su in coordinator.subdevices if su.key == '1') + info = coordinator.device_info_for(sub1) master_serial = coordinator.device_serial assert info['identifiers'] == {(DOMAIN, f'{master_serial}_1')} assert info['via_device'] == (DOMAIN, master_serial) - # The sub-unit's own /information/vs/1 (real, ARTIK051_DONGLE_FAC_RAC_18K) + # The subdevice's own /information/vs/1 (real, ARTIK051_DONGLE_FAC_RAC_18K) # is what names/models this device, not the master's. assert info['model'] == 'ARTIK051_DONGLE_FAC_RAC_18K' @@ -136,12 +136,12 @@ async def test_hjcom_unit1_device_info_links_via_device_to_master(hass: HomeAssi _SUB_UUID = '6c2dff6d-ee5c-dad1-6a5e-000000000001' -async def test_fac_bora_2in1_materializes_prefixed_wall_unit(hass: HomeAssistant): +async def test_fac_bora_2in1_materializes_prefixed_wall_subdevice(hass: HomeAssistant): coordinator = _coordinator(hass) await _discover(coordinator, 'airconditioner_fac_bora_2in1') - assert [su.key for su in coordinator.sub_units] == [_SUB_UUID] - assert coordinator.sub_units[0].kind == 'prefixed' + assert [su.key for su in coordinator.subdevices] == [_SUB_UUID] + assert coordinator.subdevices[0].kind == 'prefixed' main_climate = _climate_bound(coordinator, None) sub_climate = _climate_bound(coordinator, _SUB_UUID) @@ -151,25 +151,25 @@ async def test_fac_bora_2in1_materializes_prefixed_wall_unit(hass: HomeAssistant assert sub_climate.href == f'/{_SUB_UUID}/mode/vs/0' -async def test_fac_bora_2in1_sub_unit_device_info(hass: HomeAssistant): +async def test_fac_bora_2in1_subdevice_device_info(hass: HomeAssistant): coordinator = _coordinator(hass) await _discover(coordinator, 'airconditioner_fac_bora_2in1') - unit = coordinator.sub_units[0] - info = coordinator.device_info_for(unit) + subdevice = coordinator.subdevices[0] + info = coordinator.device_info_for(subdevice) master_serial = coordinator.device_serial assert info['identifiers'] == {(DOMAIN, f'{master_serial}_{_SUB_UUID}')} assert info['via_device'] == (DOMAIN, master_serial) # Confirmed live by the reporter (DESIGN-177.md section 1): the wall - # unit's own identity, distinct from the master's TP2X_FAC_BORA_21K. + # subdevice's own identity, distinct from the master's TP2X_FAC_BORA_21K. assert info['model'] == 'TP2X_FAC_BORA_RAC_21K' -async def test_fac_bora_2in1_unique_ids_include_sub_prefix(hass: HomeAssistant): - """The prefixed unit's unique_id carries the full subdevice UUID +async def test_fac_bora_2in1_unique_ids_include_subdevice_prefix(hass: HomeAssistant): + """The prefixed subdevice's unique_id carries the full subdevice UUID (non-alphanumerics stripped), not a truncation or an ordinal -- see - SubUnit.key_prefix.""" + Subdevice.key_prefix.""" coordinator = _coordinator(hass) await _discover(coordinator, 'airconditioner_fac_bora_2in1') @@ -178,7 +178,7 @@ async def test_fac_bora_2in1_unique_ids_include_sub_prefix(hass: HomeAssistant): entity = LocalThingsEntity(coordinator, sub_climate) expected_slug = _SUB_UUID.replace('-', '') assert entity._attr_unique_id == ( - f"{DOMAIN}_{coordinator.device_serial}_sub_{expected_slug}_climate" + f"{DOMAIN}_{coordinator.device_serial}_subdevice_{expected_slug}_climate" ) @@ -207,10 +207,10 @@ async def test_multidevice_probe_never_reaches_discovery_or_the_cache( device_types=(), raw={'/oic/p': {}, '/oic/d': {}, '/oic/res': []}, ) merged = await hass.async_add_executor_job( - coordinator._enumerate_sub_units_blocking, resources, + coordinator._enumerate_subdevices_blocking, resources, ) coordinator._run_discovery(merged) - for href, rep in coordinator._live_unit_resources(merged).items(): + for href, rep in coordinator._live_subdevice_resources(merged).items(): coordinator._observe.apply(href, rep, source='poll') assert '/multidevice/vs/0' not in merged diff --git a/tests/test_subunits.py b/tests/test_subdevices.py similarity index 70% rename from tests/test_subunits.py rename to tests/test_subdevices.py index afabc0a..ea698c5 100644 --- a/tests/test_subunits.py +++ b/tests/test_subdevices.py @@ -1,5 +1,5 @@ -"""Tests for registry/subunits.py -- multi-indoor-unit ("composite device") -support, issue #177. See DESIGN-177.md for the two board patterns +"""Tests for registry/subdevices.py -- multi-indoor-subdevice ("composite +device") support, issue #177. See DESIGN-177.md for the two board patterns (`ARTIK051_DONGLE_FAC_18K`'s indexed siblings, `TP2X_FAC_BORA_21K`'s UUID-prefixed tree) this module unifies. """ @@ -9,24 +9,24 @@ import cbor2 from custom_components.localthings.registry.capability import Capability from custom_components.localthings.registry.entities import BinarySensorDesc, SensorDesc -from custom_components.localthings.registry.subunits import ( - MAIN, SubUnit, canonical_view, discover_partitioned, enumerate_sub_units, +from custom_components.localthings.registry.subdevices import ( + MAIN, Subdevice, canonical_view, discover_partitioned, enumerate_subdevices, normalize_seed_batch, ) _UUID = '6c2dff6d-ee5c-dad1-6a5e-000000000001' -def _indexed(n: str) -> SubUnit: - return SubUnit(kind='indexed', key=n, seed_path=('device', n)) +def _indexed(n: str) -> Subdevice: + return Subdevice(kind='indexed', key=n, seed_path=('device', n)) -def _prefixed(sub_id: str) -> SubUnit: - return SubUnit(kind='prefixed', key=sub_id, seed_path=(sub_id, 'device', '0')) +def _prefixed(sub_id: str) -> Subdevice: + return Subdevice(kind='prefixed', key=sub_id, seed_path=(sub_id, 'device', '0')) # --------------------------------------------------------------------------- -# SubUnit.to_actual / to_canonical / owns / key_prefix +# Subdevice.to_actual / to_canonical / owns / key_prefix # --------------------------------------------------------------------------- class TestMainIsIdentity: @@ -38,79 +38,79 @@ class TestMainIsIdentity: def test_owns_is_always_false(self): """MAIN never "owns" a href by this definition -- it gets whatever's - left after every other sub-unit's hrefs are excluded (canonical_view).""" + left after every other subdevice's hrefs are excluded (canonical_view).""" assert MAIN.owns('/mode/vs/0') is False assert MAIN.owns('/mode/vs/1') is False def test_key_prefix_is_empty(self): - """The master unit's flattened state keys must stay byte-identical + """The master's flattened state keys must stay byte-identical to every device shipped before issue #177 -- see adapter._key.""" assert MAIN.key_prefix == '' class TestIndexedTransform: def test_to_actual_rewrites_trailing_zero(self): - unit = _indexed('1') - assert unit.to_actual('/mode/vs/0') == '/mode/vs/1' - assert unit.to_actual('/device/0') == '/device/1' + subdevice = _indexed('1') + assert subdevice.to_actual('/mode/vs/0') == '/mode/vs/1' + assert subdevice.to_actual('/device/0') == '/device/1' def test_to_actual_leaves_non_zero_trailing_segment_alone(self): """Deliberately not a "replace any trailing digit" rule -- a genuine multi-instance resource (the fridge's pattern-cap hrefs, e.g. - '/door/vs/1') must not be misread as a sub-unit's own href.""" - unit = _indexed('1') - assert unit.to_actual('/door/vs/1') == '/door/vs/1' + '/door/vs/1') must not be misread as a subdevice's own href.""" + subdevice = _indexed('1') + assert subdevice.to_actual('/door/vs/1') == '/door/vs/1' def test_to_canonical_round_trips(self): - unit = _indexed('1') - assert unit.to_canonical(unit.to_actual('/mode/vs/0')) == '/mode/vs/0' + subdevice = _indexed('1') + assert subdevice.to_canonical(subdevice.to_actual('/mode/vs/0')) == '/mode/vs/0' def test_to_canonical_rejects_wrong_index(self): - unit = _indexed('1') - assert unit.to_canonical('/mode/vs/2') is None + subdevice = _indexed('1') + assert subdevice.to_canonical('/mode/vs/2') is None def test_to_canonical_rejects_index_zero(self): - """Index 0 belongs to MAIN, never to an indexed sub-unit.""" - unit = _indexed('1') - assert unit.to_canonical('/mode/vs/0') is None + """Index 0 belongs to MAIN, never to an indexed subdevice.""" + subdevice = _indexed('1') + assert subdevice.to_canonical('/mode/vs/0') is None def test_owns(self): - unit = _indexed('1') - assert unit.owns('/mode/vs/1') is True - assert unit.owns('/mode/vs/0') is False - assert unit.owns('/mode/vs/2') is False + subdevice = _indexed('1') + assert subdevice.owns('/mode/vs/1') is True + assert subdevice.owns('/mode/vs/0') is False + assert subdevice.owns('/mode/vs/2') is False def test_key_prefix(self): - assert _indexed('1').key_prefix == 'unit1_' - assert _indexed('2').key_prefix == 'unit2_' + assert _indexed('1').key_prefix == 'subdevice1_' + assert _indexed('2').key_prefix == 'subdevice2_' class TestPrefixedTransform: def test_to_actual_prepends_id(self): - unit = _prefixed(_UUID) - assert unit.to_actual('/mode/vs/0') == f'/{_UUID}/mode/vs/0' + subdevice = _prefixed(_UUID) + assert subdevice.to_actual('/mode/vs/0') == f'/{_UUID}/mode/vs/0' def test_to_canonical_round_trips(self): - unit = _prefixed(_UUID) - assert unit.to_canonical(unit.to_actual('/mode/vs/0')) == '/mode/vs/0' + subdevice = _prefixed(_UUID) + assert subdevice.to_canonical(subdevice.to_actual('/mode/vs/0')) == '/mode/vs/0' def test_to_canonical_rejects_missing_prefix(self): - unit = _prefixed(_UUID) - assert unit.to_canonical('/mode/vs/0') is None + subdevice = _prefixed(_UUID) + assert subdevice.to_canonical('/mode/vs/0') is None def test_to_canonical_requires_path_boundary(self): """A different, longer id that merely starts with the same characters must not be mistaken for this one's own href.""" - unit = _prefixed(_UUID) - assert unit.to_canonical(f'/{_UUID}extra/mode/vs/0') is None + subdevice = _prefixed(_UUID) + assert subdevice.to_canonical(f'/{_UUID}extra/mode/vs/0') is None def test_owns(self): - unit = _prefixed(_UUID) - assert unit.owns(f'/{_UUID}/mode/vs/0') is True - assert unit.owns('/mode/vs/0') is False + subdevice = _prefixed(_UUID) + assert subdevice.owns(f'/{_UUID}/mode/vs/0') is True + assert subdevice.owns('/mode/vs/0') is False def test_key_prefix_strips_non_alphanumerics(self): - assert _prefixed(_UUID).key_prefix == 'sub_6c2dff6dee5cdad16a5e000000000001_' + assert _prefixed(_UUID).key_prefix == 'subdevice_6c2dff6dee5cdad16a5e000000000001_' # --------------------------------------------------------------------------- @@ -118,21 +118,21 @@ class TestPrefixedTransform: # --------------------------------------------------------------------------- class TestCanonicalView: - def test_main_with_no_sub_units_is_unchanged(self): + def test_main_with_no_subdevices_is_unchanged(self): resources = {'/mode/vs/0': {'a': 1}, '/power/vs/0': {'b': 2}} assert canonical_view(MAIN, resources, []) == resources - def test_main_excludes_sub_unit_owned_hrefs(self): + def test_main_excludes_subdevice_owned_hrefs(self): """A sibling's own /mode/vs/1 must not leak into the master's canonical /mode/vs/0 view -- otherwise exists_fn/is_legacy_board - checks that scan the whole dict would see two units' state mixed + checks that scan the whole dict would see two subdevices' state mixed together under one key.""" resources = { '/mode/vs/0': {'unit': 'main'}, '/mode/vs/1': {'unit': 'sibling'}, } - unit1 = _indexed('1') - view = canonical_view(MAIN, resources, [unit1]) + sub1 = _indexed('1') + view = canonical_view(MAIN, resources, [sub1]) assert view == {'/mode/vs/0': {'unit': 'main'}} def test_indexed_view_is_rewritten_to_canonical_hrefs(self): @@ -141,8 +141,8 @@ class TestCanonicalView: '/mode/vs/1': {'unit': 'sibling'}, '/power/vs/1': {'p': 'On'}, } - unit1 = _indexed('1') - view = canonical_view(unit1, resources, [unit1]) + sub1 = _indexed('1') + view = canonical_view(sub1, resources, [sub1]) assert view == { '/mode/vs/0': {'unit': 'sibling'}, '/power/vs/0': {'p': 'On'}, @@ -153,17 +153,17 @@ class TestCanonicalView: f'/{_UUID}/mode/vs/0': {'unit': 'sub'}, '/mode/vs/0': {'unit': 'main'}, } - unit = _prefixed(_UUID) - view = canonical_view(unit, resources, [unit]) + subdevice = _prefixed(_UUID) + view = canonical_view(subdevice, resources, [subdevice]) assert view == {'/mode/vs/0': {'unit': 'sub'}} - def test_including_main_in_sub_units_is_harmless(self): + def test_including_main_in_subdevices_is_harmless(self): """MAIN.owns() is always False, so passing the full roster (including MAIN itself) to canonical_view must not change anything.""" resources = {'/mode/vs/0': {'unit': 'main'}, '/mode/vs/1': {'unit': 'sib'}} - unit1 = _indexed('1') - assert (canonical_view(MAIN, resources, [MAIN, unit1]) - == canonical_view(MAIN, resources, [unit1])) + sub1 = _indexed('1') + assert (canonical_view(MAIN, resources, [MAIN, sub1]) + == canonical_view(MAIN, resources, [sub1])) # --------------------------------------------------------------------------- @@ -171,25 +171,25 @@ class TestCanonicalView: # --------------------------------------------------------------------------- def test_normalize_seed_batch_indexed_is_a_no_op(): - unit = _indexed('1') + subdevice = _indexed('1') batch = {'/mode/vs/1': {'a': 1}} - assert normalize_seed_batch(unit, batch) == batch + assert normalize_seed_batch(subdevice, batch) == batch def test_normalize_seed_batch_prefixed_adds_missing_prefix(): - unit = _prefixed(_UUID) + subdevice = _prefixed(_UUID) batch = {'/mode/vs/0': {'a': 1}} - assert normalize_seed_batch(unit, batch) == {f'/{_UUID}/mode/vs/0': {'a': 1}} + assert normalize_seed_batch(subdevice, batch) == {f'/{_UUID}/mode/vs/0': {'a': 1}} def test_normalize_seed_batch_prefixed_leaves_already_prefixed_alone(): - unit = _prefixed(_UUID) + subdevice = _prefixed(_UUID) batch = {f'/{_UUID}/mode/vs/0': {'a': 1}} - assert normalize_seed_batch(unit, batch) == batch + assert normalize_seed_batch(subdevice, batch) == batch # --------------------------------------------------------------------------- -# enumerate_sub_units +# enumerate_subdevices # --------------------------------------------------------------------------- class _FakeSession: @@ -218,8 +218,8 @@ def test_enumerate_indexed_from_oic_res_links(): ('device', '1'): [_DEVCOL_REP, {'href': '/mode/vs/1', 'rep': {'m': 1}}], ('device', '2'): [_DEVCOL_REP, {'href': '/mode/vs/2', 'rep': {'m': 2}}], }) - units, extra = enumerate_sub_units(sess, {}, oic_res) - assert sorted((u.kind, u.key) for u in units) == [ + subdevices, extra = enumerate_subdevices(sess, {}, oic_res) + assert sorted((u.kind, u.key) for u in subdevices) == [ ('indexed', '1'), ('indexed', '2'), ] assert extra == {'/mode/vs/1': {'m': 1}, '/mode/vs/2': {'m': 2}} @@ -234,18 +234,18 @@ def test_enumerate_indexed_falls_back_to_speculative_probe_when_oic_res_hides_th ('device', '1'): [_DEVCOL_REP, {'href': '/mode/vs/1', 'rep': {'m': 1}}], # /device/2 not in the table -> 4.04 -> not materialized. }) - units, extra = enumerate_sub_units(sess, {}, oic_res_links=[]) - assert [(u.kind, u.key) for u in units] == [('indexed', '1')] + subdevices, extra = enumerate_subdevices(sess, {}, oic_res_links=[]) + assert [(u.kind, u.key) for u in subdevices] == [('indexed', '1')] assert extra == {'/mode/vs/1': {'m': 1}} -def test_enumerate_indexed_only_materializes_units_with_a_non_empty_batch(): +def test_enumerate_indexed_only_materializes_subdevices_with_a_non_empty_batch(): """Per issue #177's design call: any seed that answers with a non-empty batch is materialized, but an empty ({}) answer is not -- no separate - "is this unit real" gate.""" + "is this subdevice real" gate.""" sess = _FakeSession({}) # neither /device/1 nor /device/2 answers - units, extra = enumerate_sub_units(sess, {}, oic_res_links=[]) - assert units == [] + subdevices, extra = enumerate_subdevices(sess, {}, oic_res_links=[]) + assert subdevices == [] assert extra == {} @@ -258,8 +258,8 @@ def test_enumerate_prefixed_from_subdevice_id_list(): _DEVCOL_REP, {'href': '/mode/vs/0', 'rep': {'m': 'cool'}}, ], }) - units, extra = enumerate_sub_units(sess, resources, oic_res_links=[]) - assert [(u.kind, u.key) for u in units] == [('prefixed', _UUID)] + subdevices, extra = enumerate_subdevices(sess, resources, oic_res_links=[]) + assert [(u.kind, u.key) for u in subdevices] == [('prefixed', _UUID)] # Batch echoed the bare (unprefixed) href -- normalized to carry the id. assert extra == {f'/{_UUID}/mode/vs/0': {'m': 'cool'}} @@ -267,23 +267,23 @@ def test_enumerate_prefixed_from_subdevice_id_list(): 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 - redacted string there -- this must yield zero sub-units, not crash.""" + redacted string there -- this must yield zero subdevices, not crash.""" resources = { '/subdevices/vs/0': {'x.com.samsung.da.subdeviceIdList': 'REDACTED'}, } - units, extra = enumerate_sub_units(_FakeSession({}), resources, oic_res_links=[]) - assert units == [] + subdevices, extra = enumerate_subdevices(_FakeSession({}), resources, oic_res_links=[]) + assert subdevices == [] assert extra == {} def test_enumerate_no_subdevices_resource_at_all(): - units, extra = enumerate_sub_units(_FakeSession({}), {}, oic_res_links=[]) - assert units == [] + subdevices, extra = enumerate_subdevices(_FakeSession({}), {}, oic_res_links=[]) + assert subdevices == [] assert extra == {} def test_enumerate_indexed_materializes_candidate_regardless_of_content(): - """enumerate_sub_units itself has no way to tell a real sibling from an + """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 @@ -293,8 +293,8 @@ def test_enumerate_indexed_materializes_candidate_regardless_of_content(): ('device', '2'): [_DEVCOL_REP, {'href': '/power/vs/2', 'rep': {}}, {'href': '/configuration/vs/2', 'rep': {'region': '123'}}], }) - units, extra = enumerate_sub_units(sess, {}, oic_res) - assert [(u.kind, u.key) for u in units] == [('indexed', '2')] + subdevices, extra = enumerate_subdevices(sess, {}, oic_res) + assert [(u.kind, u.key) for u in subdevices] == [('indexed', '2')] assert extra == {'/power/vs/2': {}, '/configuration/vs/2': {'region': '123'}} @@ -304,7 +304,7 @@ def test_enumerate_probe_log_reports_every_attempt(): ('device', '1'): [_DEVCOL_REP, {'href': '/mode/vs/1', 'rep': {'m': 1}}], }) probes: dict[str, bool] = {} - enumerate_sub_units(sess, {}, oic_res, probe_log=probes.__setitem__) + enumerate_subdevices(sess, {}, oic_res, probe_log=probes.__setitem__) # /multidevice/vs/0 is always probed too (issue #177 follow-up) -- not # in this session's table, so it reports "checked, nothing there". assert probes == {'/device/1': True, '/multidevice/vs/0': False} @@ -315,7 +315,7 @@ def test_enumerate_probe_log_reports_empty_answers_too(): posture the speculative-probe code this replaced used to document directly in identity.py).""" probes: dict[str, bool] = {} - enumerate_sub_units(_FakeSession({}), {}, oic_res_links=[], probe_log=probes.__setitem__) + enumerate_subdevices(_FakeSession({}), {}, oic_res_links=[], probe_log=probes.__setitem__) assert probes == { '/device/1': False, '/device/2': False, '/multidevice/vs/0': False, } @@ -328,8 +328,8 @@ def test_enumerate_multidevice_vs_0_is_captured_for_diagnostics_not_a_gate(): sess = _FakeSession({ ('multidevice', 'vs', '0'): {'x.com.samsung.da.numofsubdevice': '2'}, }) - units, extra = enumerate_sub_units(sess, {}, oic_res_links=[]) - assert units == [] # no /device/ or subdevice id answered + subdevices, extra = enumerate_subdevices(sess, {}, oic_res_links=[]) + assert subdevices == [] # no /device/ or subdevice id answered assert extra == { '/multidevice/vs/0': {'x.com.samsung.da.numofsubdevice': '2'}, } @@ -346,8 +346,8 @@ def test_enumerate_both_patterns_checked_independently(): (_UUID, 'device', '0'): [_DEVCOL_REP, {'href': '/mode/vs/0', 'rep': {'m': 1}}], ('device', '1'): [_DEVCOL_REP, {'href': '/mode/vs/1', 'rep': {'m': 2}}], }) - units, extra = enumerate_sub_units(sess, resources, oic_res) - assert sorted((u.kind, u.key) for u in units) == [ + subdevices, extra = enumerate_subdevices(sess, resources, oic_res) + assert sorted((u.kind, u.key) for u in subdevices) == [ ('indexed', '1'), ('prefixed', _UUID), ] @@ -363,7 +363,7 @@ class _FakeRegistry: self.pattern_capabilities = list(pattern_capabilities) -def test_discover_partitioned_binds_main_and_sub_unit_separately(): +def test_discover_partitioned_binds_main_and_subdevice_separately(): mode_cap = Capability( href='/mode/vs/0', entities=(BinarySensorDesc(key='mode', field='m'),), @@ -371,7 +371,7 @@ def test_discover_partitioned_binds_main_and_sub_unit_separately(): registry = {'/mode/vs/0': [mode_cap]} reg = _FakeRegistry('airconditioner', registry) - unit1 = _indexed('1') + sub1 = _indexed('1') resources = { '/mode/vs/0': {'m': 'main'}, '/mode/vs/1': {'m': 'sibling'}, @@ -381,35 +381,35 @@ def test_discover_partitioned_binds_main_and_sub_unit_separately(): return reg bound, device_type_name, materialized, skipped = discover_partitioned( - resources, [unit1], resolve, fallback_capabilities={}, + resources, [sub1], resolve, fallback_capabilities={}, ) assert device_type_name == 'airconditioner' - assert materialized == [unit1] + assert materialized == [sub1] assert skipped == [] by_href = {b.href: b for b in bound} assert set(by_href) == {'/mode/vs/0', '/mode/vs/1'} - assert by_href['/mode/vs/0'].sub_unit == MAIN - assert by_href['/mode/vs/1'].sub_unit == unit1 + assert by_href['/mode/vs/0'].subdevice == MAIN + assert by_href['/mode/vs/1'].subdevice == sub1 -def test_discover_partitioned_main_pass_excludes_sub_unit_hrefs_from_unbound(): - """A sub-unit's own /mode/vs/1 must not land in the main pass's unbound +def test_discover_partitioned_main_pass_excludes_subdevice_hrefs_from_unbound(): + """A subdevice's own /mode/vs/1 must not land in the main pass's unbound list -- otherwise it raises a spurious coverage-gap repair for a href nothing in the main registry claims literally, even though the *same* - canonical /mode/vs/0 href is properly claimed for both units. + canonical /mode/vs/0 href is properly claimed for both subdevices. A registry that binds /mode/vs/0 (so both the main pass and the - sub-unit pass over their own canonical views have something to claim + subdevice pass over their own canonical views have something to claim it) makes this unambiguous: if partitioning were broken -- e.g. the main pass iterating unfiltered `resources` instead of `main_view` -- /mode/vs/1 would show up in `unbound` because nothing in the registry is keyed on the literal string '/mode/vs/1'. With correct partitioning the main pass never sees that href at all (canonical_view excludes it), - and the sub-unit pass resolves it via its own canonical '/mode/vs/0'.""" + and the subdevice pass resolves it via its own canonical '/mode/vs/0'.""" cap = Capability(href='/mode/vs/0', entities=(BinarySensorDesc(key='mode', field='m'),)) reg = _FakeRegistry('airconditioner', {'/mode/vs/0': [cap]}) - unit1 = _indexed('1') + sub1 = _indexed('1') resources = { '/mode/vs/0': {'m': 'main'}, '/mode/vs/1': {'m': 'sibling'}, @@ -417,47 +417,47 @@ def test_discover_partitioned_main_pass_excludes_sub_unit_hrefs_from_unbound(): unbound = [] discover_partitioned( - resources, [unit1], lambda r: reg, fallback_capabilities={}, + resources, [sub1], lambda r: reg, fallback_capabilities={}, log=unbound.append, ) assert unbound == [] -def test_discover_partitioned_sub_unit_resolves_its_own_registry(): - """A sub-unit reporting its own /information/vs/0 resolves its own - device type (jhkwon19's wall unit: TP2X_FAC_BORA_RAC_21K -> RAC -> +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.""" 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]}) sub_reg = _FakeRegistry('sub_type', {'/mode/vs/0': [sub_cap]}) - unit1 = _indexed('1') + sub1 = _indexed('1') resources = {'/mode/vs/0': {'x': 1}, '/mode/vs/1': {'x': 2}} def resolve(view): - # The sub-unit's canonical view is exactly {'/mode/vs/0': {'x': 2}}. + # The subdevice's canonical view is exactly {'/mode/vs/0': {'x': 2}}. return sub_reg if view.get('/mode/vs/0', {}).get('x') == 2 else main_reg bound, device_type_name, materialized, skipped = discover_partitioned( - resources, [unit1], resolve, fallback_capabilities={}, + resources, [sub1], resolve, fallback_capabilities={}, ) assert device_type_name == 'main_type' - assert materialized == [unit1] + assert materialized == [sub1] assert skipped == [] keys = {(b.href, b.desc.key) for b in bound} assert ('/mode/vs/0', 'm') in keys assert ('/mode/vs/1', 'm2') in keys -def test_discover_partitioned_sub_unit_falls_back_to_master_registry(): - """A sub-unit that reports no identity of its own resolves through the +def test_discover_partitioned_subdevice_falls_back_to_master_registry(): + """A subdevice that reports no identity of its own resolves through the master's registry instead of the global fallback -- the master itself must actually resolve here (it reports /information/vs/0), or there is no master registry for the fallback to reach for.""" cap = Capability(href='/mode/vs/0', entities=(BinarySensorDesc(key='m', field='x'),)) main_reg = _FakeRegistry('airconditioner', {'/mode/vs/0': [cap]}) - unit1 = _indexed('1') + sub1 = _indexed('1') resources = { '/information/vs/0': {'model': 'main'}, '/mode/vs/0': {'x': 1}, @@ -468,16 +468,16 @@ def test_discover_partitioned_sub_unit_falls_back_to_master_registry(): return main_reg if view.get('/information/vs/0') else None bound, _, materialized, skipped = discover_partitioned( - resources, [unit1], resolve, fallback_capabilities={}, + resources, [sub1], resolve, fallback_capabilities={}, ) - assert materialized == [unit1] + assert materialized == [sub1] assert skipped == [] hrefs = {b.href for b in bound} assert '/mode/vs/1' in hrefs -def test_discover_partitioned_no_sub_units_matches_plain_discover(): - """For a device with no sub-units this must be exactly the single +def test_discover_partitioned_no_subdevices_matches_plain_discover(): + """For a device with no subdevices this must be exactly the single discover() call it replaces -- the hard regression guard the whole design exists to protect.""" from custom_components.localthings.registry.discovery import discover @@ -506,7 +506,7 @@ def test_discover_partitioned_no_sub_units_matches_plain_discover(): 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 - sensor reading something even though the unit itself is empty).""" + sensor reading something even though the subdevice itself is empty).""" diag_cap = Capability( href='/alarms/vs/0', entities=(SensorDesc(key='alarm_code', field='code', entity_category='diagnostic'),), @@ -523,11 +523,11 @@ def test_discover_partitioned_skips_candidate_with_no_live_primary_entity(): ) assert materialized == [] assert len(skipped) == 1 - assert skipped[0].sub_unit == unit2 + assert skipped[0].subdevice == unit2 assert skipped[0].hrefs == ('/alarms/vs/2',) # The candidate contributes nothing at all -- not even its diagnostic # entity -- once skipped. - assert all(b.sub_unit != unit2 for b in bound) + assert all(b.subdevice != unit2 for b in bound) def test_discover_partitioned_materializes_candidate_with_live_primary_entity(): @@ -545,20 +545,20 @@ def test_discover_partitioned_materializes_candidate_with_live_primary_entity(): reg = _FakeRegistry( 'airconditioner', {'/mode/vs/0': [climate_cap], '/alarms/vs/0': [diag_cap]}, ) - unit1 = _indexed('1') + sub1 = _indexed('1') resources = { '/mode/vs/0': {'m': 'main'}, '/alarms/vs/0': {'code': 'main-alarm'}, '/mode/vs/1': {'m': 'Cool'}, - '/alarms/vs/1': {}, # empty -- not what makes this unit live + '/alarms/vs/1': {}, # empty -- not what makes this subdevice live } bound, _, materialized, skipped = discover_partitioned( - resources, [unit1], lambda r: reg, fallback_capabilities={}, + resources, [sub1], lambda r: reg, fallback_capabilities={}, ) - assert materialized == [unit1] + assert materialized == [sub1] assert skipped == [] - hrefs = {b.href for b in bound if b.sub_unit == unit1} + hrefs = {b.href for b in bound if b.subdevice == sub1} assert hrefs == {'/mode/vs/1', '/alarms/vs/1'} diff --git a/tests/test_unique_ids.py b/tests/test_unique_ids.py index 181e83f..ddb9bca 100644 --- a/tests/test_unique_ids.py +++ b/tests/test_unique_ids.py @@ -1,8 +1,8 @@ """Corpus-wide invariant: adapter._key must be unique across every bound entity that would actually be registered as an HA entity, for every fixture -in the corpus -- issue #177's whole SubUnit/key_prefix design exists to +in the corpus -- issue #177's whole Subdevice/key_prefix design exists to protect this (see DESIGN-177.md section 3/6). Run over the entire fixture -set, not just the two new sub-unit fixtures, so a future dump -- sub-unit- +set, not just the two new subdevice fixtures, so a future dump -- subdevice- capable or not -- exercises it automatically. """ from collections import Counter @@ -25,13 +25,13 @@ class _FakeCoordinator: _is_included -- the same one-time entity-creation gate every platform's async_setup_entry runs (see entity.py's own module docstring).""" - def __init__(self, resources: dict[str, dict], sub_units): + def __init__(self, resources: dict[str, dict], subdevices): self.last_resources = resources - self._sub_units = list(sub_units) + self._subdevices = list(subdevices) - def canonical_resources(self, sub_unit): - from custom_components.localthings.registry.subunits import canonical_view - return canonical_view(sub_unit, self.last_resources, self._sub_units) + def canonical_resources(self, subdevice): + from custom_components.localthings.registry.subdevices import canonical_view + return canonical_view(subdevice, self.last_resources, self._subdevices) @pytest.mark.parametrize('name', _FIXTURE_NAMES) @@ -62,21 +62,21 @@ def test_key_is_unique_across_all_bound_entities(name): dupes = {k: n for k, n in Counter(keys).items() if n > 1} assert not dupes, ( f"{name}: duplicate (platform, _key) values across {len(included)} " - f"included entities (materialized sub-units: " + f"included entities (materialized subdevices: " f"{[su.key for su in materialized]}): {dupes}" ) -def test_sub_unit_capable_fixtures_actually_exercise_a_sub_unit(): - """A meta-check on the test above: if both sub-unit fixtures somehow - stopped materializing any sub-unit (a regression in enumeration or the +def test_subdevice_capable_fixtures_actually_exercise_a_subdevice(): + """A meta-check on the test above: if both subdevice fixtures somehow + stopped materializing any subdevice (a regression in enumeration or the materialization gate), the corpus-wide uniqueness test above would keep passing vacuously -- it never gets to check a single collision. Assert the two fixtures this design added actually produce a materialized - sub-unit, so that silent-vacuous-pass failure mode is caught here + subdevice, so that silent-vacuous-pass failure mode is caught here instead.""" for name in ('airconditioner_artik051_dongle_fac_18k', 'airconditioner_fac_bora_2in1'): resources, oic_res, seeds = _load_device_full(name) _, materialized, _, _, _ = _discover_full(resources, oic_res, seeds) - assert materialized, f"{name}: expected at least one materialized sub-unit" + assert materialized, f"{name}: expected at least one materialized subdevice"