From 71f2101d11dc45c873276cdafe8581f1da346cca Mon Sep 17 00:00:00 2001 From: galaxysj Date: Sun, 2 Aug 2026 12:29:55 +0900 Subject: [PATCH] Bound subdevice discovery during setup --- custom_components/localthings/coordinator.py | 54 +++++++++ .../localthings/registry/subdevices.py | 93 ++++++++++++--- tests/test_subdevice_discovery.py | 17 +++ tests/test_subdevices.py | 111 ++++++++++++++++++ 4 files changed, 258 insertions(+), 17 deletions(-) diff --git a/custom_components/localthings/coordinator.py b/custom_components/localthings/coordinator.py index b76dd33..f025cb6 100644 --- a/custom_components/localthings/coordinator.py +++ b/custom_components/localthings/coordinator.py @@ -165,6 +165,17 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): _POST_TIMEOUT_S: float = 8.0 _POLL_TIMEOUT_S: float = 35.0 + # First-discovery subdevice enumeration is part of config-entry setup, so + # it must have a finite wall-clock cost. A UUID-prefixed AC whose + # //device/0 Collection is absent falls back to individual property + # probes; some firmware silently drops unknown prefixed paths instead of + # returning 4.04, making the old 10s-per-href scan take several minutes. + # Keep enough time for a real blockwise Collection response, then use + # short timeouts for the small Property resources, all under one budget. + _SUBDEVICE_ENUMERATION_BUDGET_S: float = 15.0 + _SUBDEVICE_COLLECTION_TIMEOUT_S: float = 10.0 + _SUBDEVICE_PROPERTY_TIMEOUT_S: float = 1.0 + def __init__(self, hass: HomeAssistant, entry: ConfigEntry) -> None: # Per-device logger (module logger scoped to this device's host) so # every log line — including the base DataUpdateCoordinator's own @@ -551,6 +562,45 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): # Discovery (runs once on first successful poll) # ------------------------------------------------------------------ + def _subdevice_probe_priority( + self, resources: dict[str, dict], + ) -> tuple[str, ...]: + """Return live primary-entity hrefs in hot/warm-first order. + + A prefixed subdevice without a Collection endpoint has to be probed + one Property href at a time. Resolve the master through the same + registry used by discovery and put resources that produce primary + entities first. This is metadata-driven rather than an AC-specific + list: a future composite appliance gets the priority its own registry + declares, while unknown devices simply retain the normal href order. + """ + registry = resolve_registry( + resources, + device_types=self._identity.device_types if self._identity else (), + ) + if registry is None: + return () + + tier_rank = {'hot': 0, 'warm': 1, 'cold': 2} + ranked = [] + for order, href in enumerate(resources): + primary_caps = [ + capability + for capability in registry.capabilities.get(href, ()) + if any( + desc.entity_category is None + for desc in capability.entities + ) + ] + if not primary_caps: + continue + rank = min( + tier_rank.get(capability.poll_tier, 2) + for capability in primary_caps + ) + ranked.append((rank, order, href)) + return tuple(href for _, _, href in sorted(ranked)) + 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 @@ -578,6 +628,10 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): subdevices, extra = enumerate_subdevices( sess, resources, oic_res, probe_log=lambda href, found: probes.__setitem__(href, found), + preferred_hrefs=self._subdevice_probe_priority(resources), + time_budget=self._SUBDEVICE_ENUMERATION_BUDGET_S, + collection_timeout=self._SUBDEVICE_COLLECTION_TIMEOUT_S, + property_timeout=self._SUBDEVICE_PROPERTY_TIMEOUT_S, ) self.subdevices = subdevices self._subdevice_probes = probes diff --git a/custom_components/localthings/registry/subdevices.py b/custom_components/localthings/registry/subdevices.py index aed9bb4..34e794e 100644 --- a/custom_components/localthings/registry/subdevices.py +++ b/custom_components/localthings/registry/subdevices.py @@ -79,6 +79,7 @@ that materialized the slot as a phantom second air conditioner. See from __future__ import annotations import re +import time from dataclasses import dataclass from typing import Callable, Optional, Sequence @@ -269,13 +270,13 @@ def _seed_href(path_segs: tuple[str, ...]) -> str: return '/' + '/'.join(path_segs) -def _get_raw(sess, path_segs: tuple[str, ...]): +def _get_raw(sess, path_segs: tuple[str, ...], timeout: float = 10.0): """GET `path_segs` and CBOR-decode the payload, or None on any missing/malformed response (a 4.04, a timeout, an empty payload) -- shared tolerated-absence posture for both callers below, which differ only in which body shape they accept.""" try: - code, pl = sess.get(list(path_segs), timeout=10.0) + code, pl = sess.get(list(path_segs), timeout=timeout) if code == 0x45 and pl: return cbor2.loads(pl) except Exception: @@ -283,21 +284,25 @@ def _get_raw(sess, path_segs: tuple[str, ...]): return None -def _get_batch(sess, path_segs: tuple[str, ...]) -> dict[str, dict]: +def _get_batch( + sess, path_segs: tuple[str, ...], timeout: float = 10.0, +) -> dict[str, dict]: """GET a Samsung Collection resource and parse it the same way /device/0 itself is parsed (parse_device0_batch): a [devcol-rep, {href, rep}, ...] CBOR list, not a bare Property map.""" - body = _get_raw(sess, path_segs) + body = _get_raw(sess, path_segs, timeout) return parse_device0_batch(body) if isinstance(body, list) else {} -def _get_property(sess, path_segs: tuple[str, ...]) -> dict: +def _get_property( + sess, path_segs: tuple[str, ...], timeout: float = 10.0, +) -> dict: """GET a plain OCF Property-map resource (a bare dict, not a Collection batch). Used for `/multidevice/vs/0` (issue #177 follow-up): listed in `/oic/res` on the Pattern A reporter's board but absent from `/device/0`'s batch, so it needs its own RETRIEVE, and it answers a single Property map, not a [devcol-rep, ...] list.""" - body = _get_raw(sess, path_segs) + body = _get_raw(sess, path_segs, timeout) return body if isinstance(body, dict) else {} @@ -306,6 +311,11 @@ def enumerate_subdevices( resources: dict[str, dict], oic_res_links, probe_log: Optional[Callable[[str, bool], None]] = None, + *, + preferred_hrefs: Sequence[str] = (), + time_budget: Optional[float] = None, + collection_timeout: float = 10.0, + property_timeout: float = 10.0, ) -> tuple[list['Subdevice'], dict[str, dict]]: """Discover every sibling indoor subdevice reachable over `sess`'s connection. @@ -323,6 +333,13 @@ def enumerate_subdevices( posture the speculative-probe code this replaces used to document in identity.py. + `preferred_hrefs` only changes the order of the flat Property fallback; + it never filters the device's resource surface. When `time_budget` is + supplied, probes are bounded by one shared monotonic deadline and this + returns every candidate/resource confirmed before it. This makes first + setup finite even when firmware silently drops unknown paths instead of + returning 4.04. + Every candidate whose seed answers with a non-empty batch is returned here -- this function has no way to tell a real sibling from an unused SmartThings slot that merely answers the same shape (the Pattern A @@ -333,6 +350,33 @@ def enumerate_subdevices( """ subdevices: list[Subdevice] = [] fetched: dict[str, dict] = {} + deadline = ( + time.monotonic() + max(0.0, time_budget) + if time_budget is not None else None + ) + budget_exhausted = False + + def _next_timeout(maximum: float) -> Optional[float]: + """Clamp one probe to the remaining enumeration wall-clock budget.""" + nonlocal budget_exhausted + if deadline is None: + return maximum + remaining = deadline - time.monotonic() + if remaining <= 0: + budget_exhausted = True + return None + return min(maximum, remaining) + + def _flat_probe_hrefs(): + """Preferred live-state hrefs first, then every remaining master href.""" + seen = set() + for href in preferred_hrefs: + if href in resources and href not in seen: + seen.add(href) + yield href + for href in sorted(resources): + if href not in seen: + yield href def _probed(seed_href: str, batch: dict) -> None: if probe_log is not None: @@ -350,7 +394,10 @@ def enumerate_subdevices( ids = raw_ids if isinstance(raw_ids, list) else [] for sub_id in sorted(i for i in ids if isinstance(i, str) and i): seed = (sub_id, 'device', '0') - batch = _get_batch(sess, seed) + timeout = _next_timeout(collection_timeout) + if timeout is None: + break + batch = _get_batch(sess, seed, timeout) _probed(_seed_href(seed), batch) if batch: subdevice = Subdevice(kind='prefixed', key=sub_id, seed_path=seed) @@ -366,9 +413,9 @@ def enumerate_subdevices( # device's siblings are the same physical board family as the # subdevice this config entry already talks to -- so probe every # href the master itself answered this cycle, individually, under - # this UUID's prefix, and keep whichever ones answer. Each is a - # plain tolerated-404 RETRIEVE, same posture as every other probe in - # this function. + # this UUID's prefix, and keep whichever ones answer before the + # optional enumeration deadline. Each is a plain tolerated-404 + # RETRIEVE, same posture as every other probe in this function. # # Known gap, not yet guarded against: a firmware that answers *any* # request under an unrecognized prefix (echoing the master's own @@ -382,12 +429,17 @@ def enumerate_subdevices( # against the master's own values for those same canonical hrefs. flat_hrefs = [] first = True - for href in sorted(resources): + for href in _flat_probe_hrefs(): if not first: sess.pace() first = False + timeout = _next_timeout(property_timeout) + if timeout is None: + break actual = f'/{sub_id}{href}' - rep = _get_property(sess, tuple(actual.strip('/').split('/'))) + rep = _get_property( + sess, tuple(actual.strip('/').split('/')), timeout, + ) _probed(actual, bool(rep)) if rep: flat_hrefs.append(href) @@ -398,6 +450,8 @@ def enumerate_subdevices( kind='prefixed', key=sub_id, seed_path=(), flat_hrefs=tuple(flat_hrefs), )) + if budget_exhausted: + break # --- Pattern A: indexed siblings (ARTIK051_DONGLE_FAC_18K) -------------- indices = sorted({ @@ -413,8 +467,11 @@ def enumerate_subdevices( # bounded speculative probe this replaces from identity.py. indices = list(_SPECULATIVE_DEVICE_INDICES) for n in indices: + timeout = _next_timeout(collection_timeout) + if timeout is None: + break seed = ('device', str(n)) - batch = _get_batch(sess, seed) + batch = _get_batch(sess, seed, timeout) _probed(_seed_href(seed), batch) if not batch: continue @@ -436,10 +493,12 @@ def enumerate_subdevices( # 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 + timeout = _next_timeout(property_timeout) + if timeout is not None: + multidevice = _get_property(sess, multidevice_seed, timeout) + _probed(_seed_href(multidevice_seed), multidevice) + if multidevice: + fetched['/multidevice/vs/0'] = multidevice return subdevices, fetched diff --git a/tests/test_subdevice_discovery.py b/tests/test_subdevice_discovery.py index b446e42..bf39e3a 100644 --- a/tests/test_subdevice_discovery.py +++ b/tests/test_subdevice_discovery.py @@ -253,6 +253,23 @@ async def test_fac_bora_2in1_unique_ids_include_subdevice_prefix(hass: HomeAssis # capture instead of the synthetic sessions test_subdevices.py uses. # --------------------------------------------------------------------------- +async def test_flat_probe_priority_puts_live_climate_state_before_cold_metrics( + hass: HomeAssistant, +): + """Registry metadata drives fallback order without a model-specific list.""" + resources, _oic_res, _seeds = _load_device_full( + 'airconditioner_fac_bora_205_flat' + ) + coordinator = _coordinator(hass) + + priority = coordinator._subdevice_probe_priority(resources) + + assert '/mode/vs/0' in priority[:4] + assert priority.index('/mode/vs/0') < priority.index( + '/energy/consumption/vs/0' + ) + + async def test_fac_bora_205_flat_fallback_finds_candidate_but_gate_holds_it_back( hass: HomeAssistant, ): diff --git a/tests/test_subdevices.py b/tests/test_subdevices.py index d501df6..d8e0794 100644 --- a/tests/test_subdevices.py +++ b/tests/test_subdevices.py @@ -7,6 +7,7 @@ from __future__ import annotations import cbor2 +from custom_components.localthings.registry import subdevices as subdevices_module from custom_components.localthings.registry.capability import Capability from custom_components.localthings.registry.entities import BinarySensorDesc, SensorDesc from custom_components.localthings.registry.subdevices import ( @@ -318,6 +319,116 @@ def test_enumerate_prefixed_flat_fallback_probe_log_reports_every_href_tried(): assert probes[f'/{_UUID}/mode/vs/0'] is True +def test_enumeration_budget_bounds_silent_prefixed_fallback_and_uses_priority( + monkeypatch, +): + """A firmware that drops unknown prefixed paths cannot stall setup. + + The Collection probe gets the larger blockwise allowance. The preferred + live-state href is then attempted first, and the last Property probe is + clamped to exactly the time left in the shared enumeration budget. + """ + class Clock: + now = 0.0 + + def monotonic(self): + return self.now + + class SilentSession: + def __init__(self, clock): + self.clock = clock + self.calls = [] + + def get(self, path, timeout=10.0): + self.calls.append((tuple(path), timeout)) + self.clock.now += timeout + raise TimeoutError + + def pace(self): + pass + + clock = Clock() + session = SilentSession(clock) + monkeypatch.setattr(subdevices_module.time, 'monotonic', clock.monotonic) + resources = { + '/subdevices/vs/0': { + 'x.com.samsung.da.subdeviceIdList': [_UUID], + }, + '/power/vs/0': {'power': 'On'}, + '/mode/vs/0': {'mode': 'Cool'}, + } + + found, extra = enumerate_subdevices( + session, + resources, + oic_res_links=[], + preferred_hrefs=('/mode/vs/0',), + time_budget=7.0, + collection_timeout=4.0, + property_timeout=2.0, + ) + + assert found == [] + assert extra == {} + assert clock.now == 7.0 + assert session.calls == [ + ((_UUID, 'device', '0'), 4.0), + ((_UUID, 'mode', 'vs', '0'), 2.0), + ((_UUID, 'power', 'vs', '0'), 1.0), + ] + + +def test_enumeration_keeps_preferred_response_found_before_budget_expires( + monkeypatch, +): + """A useful early response survives later silent probes hitting the cap.""" + class Clock: + now = 0.0 + + def monotonic(self): + return self.now + + class PartlyResponsiveSession: + def __init__(self, clock): + self.clock = clock + + def get(self, path, timeout=10.0): + if tuple(path) == (_UUID, 'mode', 'vs', '0'): + return 0x45, cbor2.dumps({'mode': 'Cool'}) + self.clock.now += timeout + raise TimeoutError + + def pace(self): + pass + + clock = Clock() + monkeypatch.setattr(subdevices_module.time, 'monotonic', clock.monotonic) + resources = { + '/subdevices/vs/0': { + 'x.com.samsung.da.subdeviceIdList': [_UUID], + }, + '/power/vs/0': {'power': 'On'}, + '/mode/vs/0': {'mode': 'Cool'}, + } + + found, extra = enumerate_subdevices( + PartlyResponsiveSession(clock), + resources, + oic_res_links=[], + preferred_hrefs=('/mode/vs/0',), + time_budget=7.0, + collection_timeout=4.0, + property_timeout=2.0, + ) + + assert [(subdevice.kind, subdevice.key) for subdevice in found] == [ + ('prefixed', _UUID), + ] + assert found[0].flat_hrefs == ('/mode/vs/0',) + assert extra == {f'/{_UUID}/mode/vs/0': {'mode': 'Cool'}} + assert clock.now == 7.0 + + def test_enumerate_prefixed_flat_fallback_with_no_master_hrefs_to_probe_is_a_no_op(): """The master itself having nothing but /subdevices/vs/0 in its own resources this cycle (e.g. a very first, mostly-empty poll) must not