From b32074c8dd89219af0db153706206fde691130e9 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 12:49:58 +0000 Subject: [PATCH] Stop probing every href for Pattern B/C subdevice fallback (issue #265) When //device/0 doesn't answer for a UUID-prefixed subdevice candidate, enumerate_subdevices used to fall back to probing every href the master answered that cycle individually under the prefix. Firmware that drops packets instead of 4.04ing (issue #265's ARTIK051_FAC_TIME_21K) turned that ~30-href loop into a ~300s hang at the default 10s RETRIEVE timeout, tripping Home Assistant's bootstrap-stage timeout and failing the whole config entry before the master device even finished setup. The fallback now trusts the device's own subdeviceIdList/oic_res-link claim outright instead of re-confirming it href by href: it clones the master's current hrefs and values verbatim under the prefix as the sibling's assumed state, with no extra RETRIEVEs. The existing per-summary-poll re-poll (_poll_subdevice_flat_hrefs) is unchanged and corrects the clone toward the sibling's real values as they're confirmed live; an href the firmware never answers under that prefix just keeps mirroring the master indefinitely. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01MuiJvThhi4WkNj2xb9xo8o --- custom_components/localthings/coordinator.py | 14 +- .../localthings/registry/subdevices.py | 103 ++++++++------- ...rconditioner_fac_bora_205_flat_device.json | 2 +- .../airconditioner_fac_bora_205_flat.json | 20 +++ tests/test_diagnostics_subdevices.py | 22 ++-- tests/test_golden_regression.py | 22 ++-- tests/test_subdevice_discovery.py | 121 ++++++++---------- tests/test_subdevices.py | 98 +++++++------- 8 files changed, 211 insertions(+), 191 deletions(-) diff --git a/custom_components/localthings/coordinator.py b/custom_components/localthings/coordinator.py index b1c4930..089e048 100644 --- a/custom_components/localthings/coordinator.py +++ b/custom_components/localthings/coordinator.py @@ -462,10 +462,14 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): """Re-poll a flat-mode prefixed subdevice's hrefs individually (issue #205) -- it has no Collection endpoint to batch-refresh through (see enumerate_subdevices' fallback), so each canonical - href confirmed at enumeration time gets its own GET under the - subdevice's prefix. A href failing to answer this cycle just drops - out of the result, same "never let a sibling's flakiness fail the - master's poll" posture as the Collection path above. + href assumed at enumeration time (cloned from the master's own + state, issue #265 -- not yet confirmed live under this subdevice's + own prefix) gets its own GET under the subdevice's prefix, which is + what turns the clone into this sibling's real value once it + answers. A href failing to answer this cycle just drops out of the + result and leaves whatever value is already cached (the clone, until + corrected) in place -- same "never let a sibling's flakiness fail + the master's poll" posture as the Collection path above. Takes `sess` from the caller (already None-checked there) rather than re-reading self._session -- async_close() can null that @@ -477,7 +481,7 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): this method builds) -- those are already refreshed every 3s/6s by _run_subpolls, strictly more current than this once-per-summary-poll pass could offer, so re-fetching them here would only add GETs - without adding freshness. A subdevice with many confirmed hrefs + without adding freshness. A subdevice with many assumed hrefs (unlike a Collection batch, which is always one GET regardless of count) is otherwise a summary-poll cost that scales with its href count.""" diff --git a/custom_components/localthings/registry/subdevices.py b/custom_components/localthings/registry/subdevices.py index 02fec48..20e9585 100644 --- a/custom_components/localthings/registry/subdevices.py +++ b/custom_components/localthings/registry/subdevices.py @@ -30,11 +30,30 @@ second subdevice exists at that prefix, but not evidence that `GET around) itself returns anything. Issue #205, the same unit on a later version, is that assumption failing: `//device/0` comes back empty. So `enumerate_subdevices` tries it first (a future board might genuinely -expose it) and falls back, when it's empty, to probing every href the -master itself answered this cycle individually under the UUID prefix -- -the only thing ever actually confirmed to work for this pattern -- on the -assumption that a composite device's siblings share the master's resource -surface. See `Subdevice.flat_hrefs`. +expose it) and, when it's empty, assumes the sibling shares the master's +resource surface outright rather than trying to confirm it href by href. + +That per-href confirmation is what this module did originally, and issue +#265 is why it doesn't anymore: `ARTIK051_FAC_TIME_21K`'s firmware doesn't +4.04 an href it doesn't recognize under a foreign prefix, it drops the +packet -- so probing the ~30 hrefs a real AC dump carries, each waiting out +the full 10s RETRIEVE timeout before moving on, took close enough to five +minutes to trip Home Assistant's own bootstrap-stage timeout and fail the +whole config entry, main unit included, before a single entity existed. +A device naming this UUID in its own `subdeviceIdList` (or `/oic/res` link +prefix, Pattern C) is already the device's own claim that a real subdevice +lives there; spending an unbounded, firmware-dependent amount of RETRIEVEs +trying to re-confirm that claim one href at a time bought unreliable +confirmation at a cost that could take the integration down with it. So the +fallback now trusts the claim and clones the master's current hrefs *and* +values verbatim under the prefix as this cycle's assumed state for the +sibling -- no extra RETRIEVEs at all. `coordinator._poll_subdevice_flat_hrefs` +still re-GETs each of those hrefs individually, under the prefix, on every +later summary poll (unchanged by this), correcting the assumption toward +the sibling's real values as responses arrive; an href the firmware never +answers under that prefix just keeps mirroring the master indefinitely -- +which, for a board proven not to expose that href at all, is what "assume +it exists" has to mean in practice. See `Subdevice.flat_hrefs`. Pattern C -- UUID prefix advertised only via `/oic/res` (`AWM-WW-AID-26-ONEBODY` washer+dryer combo, issue #241). The board answers @@ -134,10 +153,13 @@ class Subdevice: `flat_hrefs` is non-empty only for a 'prefixed' subdevice that doesn't expose its own Collection at `seed_path` (issue #205 -- not even TP2X_FAC_BORA_21K, the board this pattern was built against, always - does). When set, `seed_path` is meaningless (left as `()`) and this - subdevice's state comes from GETting each of these canonical hrefs - individually under its prefix instead of one Collection batch -- see - enumerate_subdevices' fallback and coordinator._poll_subdevice_seed. + does). When set, `seed_path` is meaningless (left as `()`); enumeration + seeds this subdevice's initial state by cloning the master's own current + hrefs/values under its prefix rather than confirming them live (issue + #265 -- see enumerate_subdevices' fallback), and from then on + coordinator._poll_subdevice_seed refreshes it by GETting each of these + canonical hrefs individually under its prefix instead of one Collection + batch, correcting the clone toward real values as responses arrive. """ kind: str # 'main' | 'indexed' | 'prefixed' @@ -379,49 +401,44 @@ def enumerate_subdevices( fetched.update(normalize_seed_batch(subdevice, batch)) subdevices.append(subdevice) return - # Fallback (issue #205): TP2X_FAC_BORA_21K itself -- the board this + # Fallback (issue #265, superseding the per-href probe issue #205 + # originally added here): TP2X_FAC_BORA_21K itself -- the board this # pattern was built against -- turns out not to always expose its own # `//device/0` Collection either, so "every prefixed subdevice - # has one" doesn't hold even on the reference hardware. With no - # Collection to seed from and no per-UUID entry in `/oic/res` to - # enumerate hrefs from, the only signal left is that a composite - # device's siblings are the same physical board family as the - # subdevice this config entry already talks to -- so probe every - # href the master itself answered this cycle, individually, under - # this UUID's prefix, and keep whichever ones answer. Each is a - # plain tolerated-404 RETRIEVE, same posture as every other probe in - # this function. - # - # Known gap, not yet guarded against: a firmware that answers *any* - # request under an unrecognized prefix (echoing the master's own - # state back rather than 4.04ing) would pass every one of these - # probes and, if the echoed state also clears discover_partitioned's - # liveness gate, materialize a phantom duplicate of the master - # rather than a real sibling. Every board seen so far genuinely - # 4.04s on paths it doesn't own (issue #205's own unit answered only - # 1 of 31 probes), so this hasn't been built -- the one place it - # could hook in later is comparing a candidate's confirmed reps - # against the master's own values for those same canonical hrefs. - flat_hrefs = [] - first = True - for href in sorted(resources): - if not first: - sess.pace() - first = False - actual = f"/{sub_id}{href}" - rep = _get_property(sess, tuple(actual.strip("/").split("/"))) - _probed(actual, rep) - if rep: - flat_hrefs.append(href) - fetched[actual] = rep + # has one" doesn't hold even on the reference hardware. This used to + # respond by probing every href the master itself answered this + # cycle, individually, under this UUID's prefix, keeping whichever + # ones answered -- but issue #265's ARTIK051_FAC_TIME_21K drops + # packets for hrefs it doesn't recognize under a foreign prefix + # instead of 4.04ing them, so that loop's ~30 RETRIEVEs (a real AC + # dump's href count) each ran out the full 10s timeout, took close + # to five minutes, and tripped Home Assistant's own bootstrap-stage + # timeout -- failing the whole config entry, main unit included, + # before a single entity existed. The device already named this + # UUID in its own subdeviceIdList (or an /oic/res link prefix, + # Pattern C) -- that's the device's own claim a real subdevice lives + # there, so trust it outright rather than spend an unbounded, + # firmware-dependent number of RETRIEVEs re-confirming it href by + # href: clone the master's current hrefs and values verbatim under + # this prefix as the sibling's assumed state, no further RETRIEVEs + # at all. coordinator._poll_subdevice_flat_hrefs re-GETs each of + # these hrefs individually, under the prefix, on every later summary + # poll (unchanged by this), correcting the clone toward the + # sibling's real values as responses arrive; an href the firmware + # never answers under this prefix just keeps mirroring the master + # indefinitely -- the necessary meaning of "assume it exists" for a + # board already proven not to expose that href at all. + flat_hrefs = tuple(sorted(resources)) if not flat_hrefs: return + for href in flat_hrefs: + fetched[f"/{sub_id}{href}"] = resources[href] subdevices.append( Subdevice( kind="prefixed", key=sub_id, seed_path=(), - flat_hrefs=tuple(flat_hrefs), + flat_hrefs=flat_hrefs, ) ) diff --git a/tests/fixtures/airconditioner_fac_bora_205_flat_device.json b/tests/fixtures/airconditioner_fac_bora_205_flat_device.json index 8421ae0..18e9b82 100644 --- a/tests/fixtures/airconditioner_fac_bora_205_flat_device.json +++ b/tests/fixtures/airconditioner_fac_bora_205_flat_device.json @@ -716,5 +716,5 @@ "x.com.samsung.da.serialNum": "TEST-SUBDEVICE-SERIAL-0000" } }, - "seeds_note": "device0 and oic_res are real, captured from the reporter's issue #205 report. subdeviceIdList in device0's /subdevices/vs/0 is restored to the real ['6c2dff6d-ee5c-dad1-6a5e-000000000001'] (HA's diagnostics download redacts it, matching redact.py's 'deviceid' substring rule, not because it's account data -- same restoration airconditioner_fac_bora_2in1_device.json documents for the same field), and every other value is otherwise exactly what HA's download redacts (serials/otnDUID etc.) against the same physical TP2X_FAC_BORA_21K unit as airconditioner_fac_bora_2in1_device.json -- same subdeviceIdList UUID, same wall subdevice. Filed specifically because, contrary to the assumption that fixture's seed batch was built on, this unit's /6c2dff6d-ee5c-dad1-6a5e-000000000001/device/0 does NOT answer (subdevice_probes in the report shows it False), so the Collection-batch fallback this fixture exercises (registry.subdevices.enumerate_subdevices' per-href probe, issue #205) is what has to find the subdevice instead. The one probes entry, /6c2dff6d-ee5c-dad1-6a5e-000000000001/information/vs/0, is the same real capture already used in airconditioner_fac_bora_2in1_device.json's seed batch (issue #177 comment 5113518087) -- the only href ever actually confirmed to answer under this UUID prefix. No other /6c2dff6d-ee5c-dad1-6a5e-000000000001/* href has been confirmed live yet, so none are seeded here; this fixture's expected outcome is the candidate being found by the flat-href probe but then correctly held back by discover_partitioned's liveness gate (information alone binds no entity), matching where the real issue stands -- not a materialized climate entity, which would require guessing at unconfirmed hrefs. The probe's serialNum (\"TEST-SUBDEVICE-SERIAL-0000\") is a hand-placed placeholder for the real value, not HA's own redaction output -- same placeholder airconditioner_fac_bora_2in1_device.json uses for the identical field." + "seeds_note": "device0 and oic_res are real, captured from the reporter's issue #205 report. subdeviceIdList in device0's /subdevices/vs/0 is restored to the real ['6c2dff6d-ee5c-dad1-6a5e-000000000001'] (HA's diagnostics download redacts it, matching redact.py's 'deviceid' substring rule, not because it's account data -- same restoration airconditioner_fac_bora_2in1_device.json documents for the same field), and every other value is otherwise exactly what HA's download redacts (serials/otnDUID etc.) against the same physical TP2X_FAC_BORA_21K unit as airconditioner_fac_bora_2in1_device.json -- same subdeviceIdList UUID, same wall subdevice. Filed specifically because this unit's /6c2dff6d-ee5c-dad1-6a5e-000000000001/device/0 does NOT answer (subdevice_probes in the report shows it False), which is what this fixture exercises: registry.subdevices.enumerate_subdevices' clone-the-master fallback (issue #265, which replaced the per-href probe loop this fixture originally exercised for issue #205). The one probes entry, /6c2dff6d-ee5c-dad1-6a5e-000000000001/information/vs/0, is the same real capture already used in airconditioner_fac_bora_2in1_device.json's seed batch (issue #177 comment 5113518087) -- the only href ever actually confirmed to answer under this UUID prefix. It's kept here as that real confirmation even though the current fallback no longer probes any href beyond /device/0 to decide whether the subdevice exists at all: since issue #265, this fixture's expected outcome is the candidate materializing immediately with the master's own device0 hrefs/values cloned under the prefix as its assumed state -- a full climate entity included -- rather than waiting on per-href confirmation this firmware may never give. The probe's serialNum (\"TEST-SUBDEVICE-SERIAL-0000\") is a hand-placed placeholder for the real value, not HA's own redaction output -- same placeholder airconditioner_fac_bora_2in1_device.json uses for the identical field." } diff --git a/tests/fixtures/golden/airconditioner_fac_bora_205_flat.json b/tests/fixtures/golden/airconditioner_fac_bora_205_flat.json index a75fc9e..eebb069 100644 --- a/tests/fixtures/golden/airconditioner_fac_bora_205_flat.json +++ b/tests/fixtures/golden/airconditioner_fac_bora_205_flat.json @@ -19,6 +19,26 @@ "odor_controller_progress", "power_energy_kwh", "power_watts", + "subdevice_6c2dff6dee5cdad16a5e000000000001_air_filter_status", + "subdevice_6c2dff6dee5cdad16a5e000000000001_air_filter_threshold", + "subdevice_6c2dff6dee5cdad16a5e000000000001_air_filter_usage", + "subdevice_6c2dff6dee5cdad16a5e000000000001_air_filter_usage_hours", + "subdevice_6c2dff6dee5cdad16a5e000000000001_alarm_code", + "subdevice_6c2dff6dee5cdad16a5e000000000001_auto_clean", + "subdevice_6c2dff6dee5cdad16a5e000000000001_auto_clean_progress", + "subdevice_6c2dff6dee5cdad16a5e000000000001_auto_clean_running", + "subdevice_6c2dff6dee5cdad16a5e000000000001_beep", + "subdevice_6c2dff6dee5cdad16a5e000000000001_climate", + "subdevice_6c2dff6dee5cdad16a5e000000000001_current_temperature_c", + "subdevice_6c2dff6dee5cdad16a5e000000000001_diagnosis_status", + "subdevice_6c2dff6dee5cdad16a5e000000000001_energy_kwh", + "subdevice_6c2dff6dee5cdad16a5e000000000001_firmware_update", + "subdevice_6c2dff6dee5cdad16a5e000000000001_humidity", + "subdevice_6c2dff6dee5cdad16a5e000000000001_odor_controller_active", + "subdevice_6c2dff6dee5cdad16a5e000000000001_odor_controller_progress", + "subdevice_6c2dff6dee5cdad16a5e000000000001_power_energy_kwh", + "subdevice_6c2dff6dee5cdad16a5e000000000001_power_watts", + "subdevice_6c2dff6dee5cdad16a5e000000000001_tropical_night_mode", "tropical_night_mode" ] } diff --git a/tests/test_diagnostics_subdevices.py b/tests/test_diagnostics_subdevices.py index bc5265e..693c184 100644 --- a/tests/test_diagnostics_subdevices.py +++ b/tests/test_diagnostics_subdevices.py @@ -111,24 +111,26 @@ async def test_diagnostics_reports_prefixed_subdevice( assert diag["subdevices_skipped"] == [] -async def test_diagnostics_reports_flat_hrefs_for_skipped_prefixed_candidate( +async def test_diagnostics_reports_flat_hrefs_for_materialized_prefixed_candidate( hass: HomeAssistant, enable_custom_integrations, ) -> None: - """issue #205: a prefixed candidate found through the per-href flat + """issue #205: a prefixed candidate found through the clone-the-master fallback (no working //device/0 Collection) has no meaningful seed_path -- diagnostics reports None there instead of the misleading - bare "/" an empty tuple would otherwise join to, and lists the actual - hrefs the fallback confirmed instead.""" + bare "/" an empty tuple would otherwise join to, and lists the full set + of hrefs the fallback assumed (issue #265 -- every href the master + itself has, not just a confirmed few) instead.""" coordinator = _coordinator(hass) await _discover(coordinator, "airconditioner_fac_bora_205_flat") hass.data.setdefault(DOMAIN, {})[coordinator._entry.entry_id] = coordinator diag = await async_get_config_entry_diagnostics(hass, coordinator._entry) - assert diag["subdevices"] == [] - assert len(diag["subdevices_skipped"]) == 1 - skipped = diag["subdevices_skipped"][0] - assert skipped["kind"] == "prefixed" - assert skipped["seed_path"] is None - assert skipped["flat_hrefs"] == ["/information/vs/0"] + assert diag["subdevices_skipped"] == [] + assert len(diag["subdevices"]) == 1 + subdevice = diag["subdevices"][0] + assert subdevice["kind"] == "prefixed" + assert subdevice["seed_path"] is None + assert subdevice["flat_hrefs"] == sorted(diag["resources"]) + assert subdevice["bound_entity_count"] > 0 diff --git a/tests/test_golden_regression.py b/tests/test_golden_regression.py index b88f5a0..abec4d5 100644 --- a/tests/test_golden_regression.py +++ b/tests/test_golden_regression.py @@ -1104,18 +1104,16 @@ def test_registry_reproduces_golden_state_keys_for_airconditioner_fac_bora_2in1( def test_registry_reproduces_golden_state_keys_for_airconditioner_fac_bora_205_flat(): """The same reporter's same physical TP2X_FAC_BORA_21K unit as the _2in1 - fixture above, but a later capture (issue #205) where //device/0 doesn't - answer -- contrary to what that fixture's own seed batch assumed the - Collection endpoint would do. device0/oic_res are real; the only - UUID-prefixed data is the one href ever actually confirmed live - (/information/vs/0, same real capture the _2in1 fixture uses), fed - through registry.subdevices.enumerate_subdevices' per-href flat - fallback instead of a Collection batch. /information/vs/0 alone binds - no entity, so the candidate is found but never materializes -- this - golden has no `subdevice_...`-prefixed keys at all, same shape as - tests/fixtures/golden/airconditioner_fac_bora.json, which is the point: - a device whose sibling can't yet be confirmed live must regress to - exactly the master-only state, never a phantom or partial subdevice.""" + fixture above, but a later capture (issue #205) where //device/0 + doesn't answer -- contrary to what that fixture's own seed batch assumed + the Collection endpoint would do. device0/oic_res are real; there is no + UUID-prefixed seed data at all, which is the point of this fixture: + issue #265 replaced the old per-href confirmation fallback with an + unconditional clone of the master's own hrefs/values under the prefix, + so this golden's `subdevice_...`-prefixed keys are now the *same set* as + the master's own unprefixed keys -- every entity the master has, the + assumed sibling gets too, not just the handful the _2in1 fixture's + synthetic partial seed happened to confirm.""" name = "airconditioner_fac_bora_205_flat" golden = json.loads((GOLDEN / f"{name}.json").read_text()) state_keys = _new_subdevice_aware_state_keys(name) diff --git a/tests/test_subdevice_discovery.py b/tests/test_subdevice_discovery.py index c839c2c..a02a051 100644 --- a/tests/test_subdevice_discovery.py +++ b/tests/test_subdevice_discovery.py @@ -273,35 +273,43 @@ async def test_fac_bora_2in1_unique_ids_include_subdevice_prefix(hass: HomeAssis # --------------------------------------------------------------------------- # Same reporter and physical unit/UUID again -- issue #205, but this # time //device/0 doesn't answer. Exercises enumerate_subdevices' -# per-href flat-probe fallback (registry/subdevices.py) against a real -# capture instead of the synthetic sessions test_subdevices.py uses. +# clone-the-master fallback (registry/subdevices.py, issue #265) against a +# real capture instead of the synthetic sessions test_subdevices.py uses. # --------------------------------------------------------------------------- -async def test_fac_bora_205_flat_fallback_finds_candidate_but_gate_holds_it_back( +async def test_fac_bora_205_flat_fallback_clones_master_and_materializes( hass: HomeAssistant, ): - """The fixture's only seeded UUID-prefixed href is /information/vs/0 -- - the one href ever actually confirmed live under this prefix (issue #177 - comment 5113518087) -- since nothing else has been confirmed yet for - this unit. That's enough for the flat-probe fallback to find a - candidate, but /information/vs/0 binds no entity on its own (it's only - ever read for device-type resolution, never bound as a capability), so - discover_partitioned's liveness gate correctly holds it back rather than - materializing a phantom climate card from unconfirmed hrefs. This is the - honest current state of issue #205, not a guess at its resolution.""" + """//device/0 still doesn't answer for this reporter's unit + (issue #205), but issue #265 replaced the old per-href confirmation loop + -- which used to hold this candidate back with only /information/vs/0 + ever confirmed live under the prefix -- with an unconditional clone of + the master's own hrefs and values under the prefix. So the sibling now + materializes immediately with the master's own climate state as its + assumed starting point, rather than waiting on per-href confirmation + that this firmware may never give (this test used to assert the + opposite: that the liveness gate correctly held the candidate back).""" coordinator = _coordinator(hass) + resources, _oic_res, _seeds = _load_device_full("airconditioner_fac_bora_205_flat") await _discover(coordinator, "airconditioner_fac_bora_205_flat") - assert coordinator.subdevices == [] - assert [s.subdevice.key for s in coordinator._skipped_subdevices] == [_SUB_UUID] - skipped = coordinator._skipped_subdevices[0].subdevice - assert skipped.kind == "prefixed" - assert skipped.seed_path == () - assert skipped.flat_hrefs == ("/information/vs/0",) + assert [su.key for su in coordinator.subdevices] == [_SUB_UUID] + assert coordinator._skipped_subdevices == [] + subdevice = coordinator.subdevices[0] + assert subdevice.kind == "prefixed" + assert subdevice.seed_path == () + assert subdevice.flat_hrefs == tuple(sorted(resources)) + # Only the seed Collection itself was ever probed over the network -- + # no more per-href confirmation loop to log. assert coordinator._subdevice_probes[f"/{_SUB_UUID}/device/0"] is False - assert coordinator._subdevice_probes[f"/{_SUB_UUID}/information/vs/0"] is True + assert not any( + href.startswith(f"/{_SUB_UUID}/") and href != f"/{_SUB_UUID}/device/0" + for href in coordinator._subdevice_probes + ) + + assert _climate_bound(coordinator, _SUB_UUID) is not None # Confirms the master itself is completely unaffected by its sibling's # Collection endpoint not answering -- same guarantee every other @@ -310,14 +318,12 @@ async def test_fac_bora_205_flat_fallback_finds_candidate_but_gate_holds_it_back async def test_flat_subdevice_materializes_and_repolls_end_to_end(hass: HomeAssistant): - """Synthetic (not a real capture, unlike the fixture-driven test above) -- - exercises the one path nothing else covers: a flat-mode prefixed - subdevice with *enough* confirmed hrefs to actually pass - discover_partitioned's liveness gate and materialize a real climate - entity, then a subsequent _poll_subdevice_seed re-poll refreshing its - state all the way through to canonical_resources -- the path a real - resolution of issue #205 (once more hrefs are confirmed live for some - unit) would actually need. + """Synthetic seeds (not a real capture, unlike the fixture-driven test + above) -- exercises the one path nothing else covers: a flat-mode + prefixed subdevice materializing off a clone of the master's own state + (issue #265) and then a subsequent _poll_subdevice_seed re-poll + refreshing its state all the way through to canonical_resources as real + per-href confirmation arrives. Also pins _poll_subdevice_flat_hrefs' hot/warm skip: climate-critical hrefs (power/mode/temperature) land on the warm tier by discovery's own @@ -327,62 +333,45 @@ async def test_flat_subdevice_materializes_and_repolls_end_to_end(hass: HomeAssi /option/autoclean/vs/0 is cold-tier and is what actually needs this path.""" resources, oic_res, _real_seeds = _load_device_full("airconditioner_fac_bora_2in1") - seeds = { - # No (_SUB_UUID, 'device', '0') entry -- forces the flat fallback, - # same as the real issue #205 capture above, but this time with - # enough hrefs answering to actually materialize. power/mode/ - # temperature values copied verbatim from that fixture's own (real) - # Collection-batch seed, just served individually instead of - # batched, to isolate "does flat mode produce the same result as - # Collection mode" as the only variable. - f"/{_SUB_UUID}/power/vs/0": {"x.com.samsung.da.power": "On"}, - f"/{_SUB_UUID}/mode/vs/0": { - "x.com.samsung.da.supportedModes": ["Cool", "Dry", "Wind", "Auto"], - "x.com.samsung.da.modes": ["Cool"], - "x.com.samsung.da.options": [], - }, - f"/{_SUB_UUID}/temperature/current/0": { - "range": [18.0, 30.0], - "units": "C", - "temperature": 26.0, - }, - f"/{_SUB_UUID}/temperature/desired/0": { - "range": [18.0, 30.0], - "units": "C", - "temperature": 24.0, - }, - # Cold-tier -- not covered by _run_subpolls, so this is the href - # that actually depends on _poll_subdevice_flat_hrefs to ever - # refresh at all. - f"/{_SUB_UUID}/option/autoclean/vs/0": { - "x.com.samsung.da.settingStatus": "Off", - }, - } + # No (_SUB_UUID, 'device', '0') entry -- forces the flat fallback, same + # as the real issue #205 capture above. No per-href seeds are needed for + # materialization itself anymore: the fallback clones the master's own + # `resources` (real climate state included) under the prefix, with no + # RETRIEVEs at all. coordinator = _coordinator(hass) - await _discover_with(coordinator, resources, oic_res, seeds) + await _discover_with(coordinator, resources, oic_res, seeds={}) assert [su.key for su in coordinator.subdevices] == [_SUB_UUID] subdevice = coordinator.subdevices[0] assert subdevice.seed_path == () - assert subdevice.flat_hrefs != () + assert subdevice.flat_hrefs == tuple(sorted(resources)) sub_climate = _climate_bound(coordinator, _SUB_UUID) assert sub_climate is not None + # Cloned initial state mirrors the master's own values verbatim. + initial_temp = resources["/temperature/current/0"]["temperature"] + initial_autoclean = resources["/option/autoclean/vs/0"]["x.com.samsung.da.settingStatus"] + res = coordinator.canonical_resources(subdevice) + assert res["/temperature/current/0"]["temperature"] == initial_temp + assert res["/option/autoclean/vs/0"]["x.com.samsung.da.settingStatus"] == initial_autoclean + # Re-poll: a fresh reading under the prefix should reach # canonical_resources through _poll_subdevice_seed's flat-mode branch, - # not just sit frozen at the one-time enumeration snapshot. + # correcting the clone toward the sibling's real, confirmed value -- + # not just sitting frozen at the one-time enumeration snapshot. # FakeCoapSession's `seeds` is typed `dict[str, list]` for the common # batch-list shape, but (per its own docstring) also legitimately holds # plain Property maps for probe-style hrefs like these two. + fresh_autoclean = "Off" if initial_autoclean != "Off" else "On" seeds_map = cast("dict[str, Any]", cast(FakeCoapSession, coordinator._session).seeds) seeds_map[f"/{_SUB_UUID}/temperature/current/0"] = { "range": [18.0, 30.0], "units": "C", - "temperature": 27.5, + "temperature": initial_temp + 1.5, } seeds_map[f"/{_SUB_UUID}/option/autoclean/vs/0"] = { - "x.com.samsung.da.settingStatus": "On", + "x.com.samsung.da.settingStatus": fresh_autoclean, } refreshed = coordinator._poll_subdevice_seed(subdevice) @@ -391,16 +380,16 @@ async def test_flat_subdevice_materializes_and_repolls_end_to_end(hass: HomeAssi # through this path. assert f"/{_SUB_UUID}/temperature/current/0" not in refreshed assert refreshed == { - f"/{_SUB_UUID}/option/autoclean/vs/0": {"x.com.samsung.da.settingStatus": "On"}, + f"/{_SUB_UUID}/option/autoclean/vs/0": {"x.com.samsung.da.settingStatus": fresh_autoclean}, } for href, rep in refreshed.items(): coordinator._observe.apply(href, rep, source="poll") res = coordinator.canonical_resources(subdevice) - assert res["/option/autoclean/vs/0"]["x.com.samsung.da.settingStatus"] == "On" + assert res["/option/autoclean/vs/0"]["x.com.samsung.da.settingStatus"] == fresh_autoclean # Confirms the skip is about redundant re-fetching, not stale data -- # the warm-tier value from initial discovery is still there, untouched. - assert res["/temperature/current/0"]["temperature"] == 26.0 + assert res["/temperature/current/0"]["temperature"] == initial_temp class _FakeCollectionSession: diff --git a/tests/test_subdevices.py b/tests/test_subdevices.py index 1ee6690..dc4026a 100644 --- a/tests/test_subdevices.py +++ b/tests/test_subdevices.py @@ -295,75 +295,66 @@ def test_enumerate_prefixed_from_subdevice_id_list(): assert extra == {f"/{_UUID}/mode/vs/0": {"m": "cool"}} -def test_enumerate_prefixed_falls_back_to_flat_hrefs_when_device0_collection_is_empty(): +def test_enumerate_prefixed_falls_back_to_cloning_master_state_when_device0_collection_is_empty(): """issue #205: not every prefixed subdevice exposes its own //device/0 Collection -- not even TP2X_FAC_BORA_21K, the board - this pattern was built against, always does. When it doesn't, - enumeration probes every href the master itself answered this cycle, - individually, under the UUID prefix, and keeps whichever ones answer.""" + this pattern was built against, always does. issue #265: probing every + master href individually under the prefix to confirm one used to be the + fallback, but a firmware that drops packets instead of 4.04ing turned + that into a ~300s hang that took the whole config entry's setup down + with it. So the fallback no longer touches the network at all -- it + trusts the device's own subdeviceIdList claim and clones the master's + current hrefs and values verbatim under the prefix.""" resources = { "/subdevices/vs/0": {"x.com.samsung.da.subdeviceIdList": [_UUID]}, "/mode/vs/0": {"m": "cool"}, "/power/vs/0": {"p": "On"}, } - sess = _FakeSession( - { - # (_UUID, 'device', '0') deliberately absent -> Collection probe fails. - (_UUID, "mode", "vs", "0"): {"mode": "cool"}, - # (_UUID, 'power', 'vs', '0') deliberately absent -> drops out. - } - ) + sess = _FakeSession({}) # (_UUID, 'device', '0') absent -> Collection probe fails. subdevices, extra = enumerate_subdevices(sess, resources, oic_res_links=[]) assert len(subdevices) == 1 subdevice = subdevices[0] assert (subdevice.kind, subdevice.key) == ("prefixed", _UUID) assert subdevice.seed_path == () - assert subdevice.flat_hrefs == ("/mode/vs/0",) - assert extra == {f"/{_UUID}/mode/vs/0": {"mode": "cool"}} - - -def test_enumerate_prefixed_flat_fallback_materializes_nothing_when_no_href_answers(): - """Same posture as every other candidate check in this module: nothing - answering means no candidate, not a crash.""" - resources = { - "/subdevices/vs/0": {"x.com.samsung.da.subdeviceIdList": [_UUID]}, - "/mode/vs/0": {"m": "cool"}, + assert subdevice.flat_hrefs == ("/mode/vs/0", "/power/vs/0", "/subdevices/vs/0") + assert extra == { + f"/{_UUID}/mode/vs/0": {"m": "cool"}, + f"/{_UUID}/power/vs/0": {"p": "On"}, + f"/{_UUID}/subdevices/vs/0": {"x.com.samsung.da.subdeviceIdList": [_UUID]}, } - subdevices, extra = enumerate_subdevices(_FakeSession({}), resources, oic_res_links=[]) - assert subdevices == [] - assert extra == {} -def test_enumerate_prefixed_flat_fallback_probe_log_reports_every_href_tried(): - resources = { - "/subdevices/vs/0": {"x.com.samsung.da.subdeviceIdList": [_UUID]}, - "/mode/vs/0": {"m": "cool"}, - } - sess = _FakeSession( - { - (_UUID, "mode", "vs", "0"): {"mode": "cool"}, - } +def test_enumerate_prefixed_flat_fallback_is_a_no_op_with_no_master_hrefs_to_clone(): + """A degenerate `resources` (nothing at all, not even /subdevices/vs/0 -- + can't happen via the subdeviceIdList path but shared by _probe_prefixed + with Pattern C, whose ids come from /oic/res instead) must not crash the + fallback -- there's nothing to clone, so nothing materializes.""" + subdevices, extra = enumerate_subdevices( + _FakeSession({}), + {}, + oic_res_links=[{"href": f"/{_UUID}/multidevice/vs/0"}], ) - probes: dict[str, bool] = {} - enumerate_subdevices(sess, resources, oic_res_links=[], probe_log=probes.__setitem__) - assert probes[f"/{_UUID}/device/0"] is False - assert probes[f"/{_UUID}/mode/vs/0"] is True - - -def test_enumerate_prefixed_flat_fallback_with_no_master_hrefs_to_probe_is_a_no_op(): - """The master itself having nothing but /subdevices/vs/0 in its own - resources this cycle (e.g. a very first, mostly-empty poll) must not - crash the fallback loop -- the only href in `resources` is - /subdevices/vs/0 itself, which the fake session doesn't answer under - the prefix either, so nothing materializes.""" - resources = { - "/subdevices/vs/0": {"x.com.samsung.da.subdeviceIdList": [_UUID]}, - } - subdevices, extra = enumerate_subdevices(_FakeSession({}), resources, oic_res_links=[]) assert subdevices == [] assert extra == {} +def test_enumerate_prefixed_flat_fallback_probe_log_only_reports_the_seed(): + """Only //device/0 is ever probed over the network now -- no more + per-href probing to log.""" + resources = { + "/subdevices/vs/0": {"x.com.samsung.da.subdeviceIdList": [_UUID]}, + "/mode/vs/0": {"m": "cool"}, + } + probes: dict[str, bool] = {} + enumerate_subdevices( + _FakeSession({}), resources, oic_res_links=[], probe_log=probes.__setitem__ + ) + assert probes[f"/{_UUID}/device/0"] is False + assert not any( + href.startswith(f"/{_UUID}/") and href != f"/{_UUID}/device/0" for href in probes + ) + + def test_enumerate_prefixed_flat_fallback_does_not_cross_contaminate_a_second_uuid(): """Two prefixed candidates in the same subdeviceIdList, one whose Collection endpoint works and one that needs the flat fallback -- each @@ -379,9 +370,7 @@ def test_enumerate_prefixed_flat_fallback_does_not_cross_contaminate_a_second_uu _DEVCOL_REP, {"href": "/mode/vs/0", "rep": {"m": "a-collection"}}, ], - # uuid_b's Collection deliberately absent -> falls back to the flat - # per-href probe. - (uuid_b, "mode", "vs", "0"): {"m": "b-flat"}, + # uuid_b's Collection deliberately absent -> falls back to cloning. } ) subdevices, extra = enumerate_subdevices(sess, resources, oic_res_links=[]) @@ -391,10 +380,11 @@ def test_enumerate_prefixed_flat_fallback_does_not_cross_contaminate_a_second_uu assert by_key[uuid_a].seed_path == (uuid_a, "device", "0") assert by_key[uuid_a].flat_hrefs == () assert by_key[uuid_b].seed_path == () - assert by_key[uuid_b].flat_hrefs == ("/mode/vs/0",) + assert by_key[uuid_b].flat_hrefs == ("/mode/vs/0", "/subdevices/vs/0") assert extra == { f"/{uuid_a}/mode/vs/0": {"m": "a-collection"}, - f"/{uuid_b}/mode/vs/0": {"m": "b-flat"}, + f"/{uuid_b}/mode/vs/0": {"m": "cool"}, + f"/{uuid_b}/subdevices/vs/0": {"x.com.samsung.da.subdeviceIdList": [uuid_a, uuid_b]}, }