From 67012c57d7a67396ef72886c53ec82596ab39c20 Mon Sep 17 00:00:00 2001 From: "firstof9@gmail.com" Date: Wed, 5 Aug 2026 10:17:50 -0700 Subject: [PATCH 1/5] Allow non-blocking setup when device is offline (#295) When a device is offline or unreachable during Home Assistant startup, previously raised . HA's built-in retry mechanism uses exponential backoff up to 15 minutes, which leads to a poor user experience for local LAN devices. Catch initial connection errors during and log a warning instead of failing setup. This allows platforms to set up and entities to be created (in an unavailable state), while the coordinator continues background retry polling. --- custom_components/localthings/__init__.py | 13 +++------- tests/localthings/test_coordinator.py | 30 ++++++++--------------- 2 files changed, 14 insertions(+), 29 deletions(-) diff --git a/custom_components/localthings/__init__.py b/custom_components/localthings/__init__.py index 3b0803f..ce47f79 100644 --- a/custom_components/localthings/__init__.py +++ b/custom_components/localthings/__init__.py @@ -13,7 +13,6 @@ from homeassistant.const import ( ) from homeassistant.const import EVENT_HOMEASSISTANT_STOP from homeassistant.core import Event, HomeAssistant, callback -from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.helpers import device_registry as dr from homeassistant.helpers import entity_registry as er from homeassistant.helpers.typing import ConfigType @@ -263,14 +262,10 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: try: await coordinator.async_config_entry_first_refresh() except Exception as err: - # `_poll_once` deliberately leaves the session up on a TimeoutError - # (see its docstring), so a refresh failing that way leaves a live, - # bound UDP socket nothing would ever close. HA retries setup with a - # new coordinator, and the source port is fixed by design - # (`_local_source_port`), so an abandoned socket would squat the - # exact port the next attempt binds. - await coordinator.async_close() - raise ConfigEntryNotReady(f"Cannot connect to device: {err}") from err + _LOGGER.warning( + "Initial connection to device failed (%s); starting offline and retrying in background", + err, + ) hass.data[DOMAIN][entry.entry_id] = coordinator # Send the DTLS close_notify on Core shutdown, not just on unload (issue diff --git a/tests/localthings/test_coordinator.py b/tests/localthings/test_coordinator.py index 6dfd2d0..82cd159 100644 --- a/tests/localthings/test_coordinator.py +++ b/tests/localthings/test_coordinator.py @@ -70,8 +70,8 @@ async def test_summary_interval(hass: HomeAssistant, mock_entry, mock_coordinato assert coordinator.update_interval == timedelta(seconds=SUMMARY_INTERVAL_S) -async def test_update_failed_on_persistent_poll_error(hass: HomeAssistant, mock_entry) -> None: - """ConfigEntryNotReady raised when poll fails even after reconnect.""" +async def test_offline_initial_setup_succeeds(hass: HomeAssistant, mock_entry) -> None: + """ConfigEntry setup succeeds in offline mode when initial poll fails.""" with ( patch("custom_components.localthings.coordinator.LocalThingsCoordinator._connect_session"), @@ -84,8 +84,10 @@ async def test_update_failed_on_persistent_poll_error(hass: HomeAssistant, mock_ result = await hass.config_entries.async_setup(mock_entry.entry_id) await hass.async_block_till_done() - # Setup fails — entry should not be in hass.data - assert not result or mock_entry.entry_id not in hass.data.get(DOMAIN, {}) + # Setup succeeds — entry should be in hass.data and state is LOADED + assert result + assert mock_entry.entry_id in hass.data.get(DOMAIN, {}) + assert mock_entry.state == ConfigEntryState.LOADED # --------------------------------------------------------------------------- @@ -1821,13 +1823,11 @@ async def test_first_refresh_timeout_recovers_via_reconnect( assert coordinator._consecutive_poll_timeouts == 0 -async def test_first_refresh_persistent_timeout_fails_setup( +async def test_first_refresh_persistent_timeout_succeeds_offline( hass: HomeAssistant, mock_entry ) -> None: - """When the reconnect times out too, the first refresh must fail so HA - retries on its backoff -- not load an entity-less entry. The session it - left open is closed on the way out (`_poll_once` keeps it up on a - `TimeoutError`, and the source port is fixed per device).""" + """When initial refresh times out, setup still succeeds in offline mode + and retries in background.""" with ( patch("custom_components.localthings.coordinator.LocalThingsCoordinator._connect_session"), patch( @@ -1835,21 +1835,11 @@ async def test_first_refresh_persistent_timeout_fails_setup( side_effect=_HANDSHAKE_TIMEOUT, ), patch("custom_components.localthings.coordinator.LocalThingsCoordinator._close_session"), - # Asserted on `async_close` rather than `_close_session`: the - # reconnect path calls the latter on its own, so it is already true - # whether or not setup cleans up after itself. - patch.object( - LocalThingsCoordinator, - "async_close", - autospec=True, - ) as mock_async_close, ): await hass.config_entries.async_setup(mock_entry.entry_id) await hass.async_block_till_done() - assert mock_entry.state is ConfigEntryState.SETUP_RETRY - assert not hass.states.async_all() - mock_async_close.assert_awaited_once() + assert mock_entry.state is ConfigEntryState.LOADED async def test_post_discovery_timeout_is_still_deferred( From bc03ada2083e6c6f55e7ecf0dabb8391ab9864d0 Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Sat, 15 Aug 2026 19:38:28 +0000 Subject: [PATCH 2/5] docs(offline-setup): what PR #303 measures, and what a working version needs PR #303 loads the entry when the first poll fails. Measured on its branch, that produces an entry with zero bound entities and zero coordinator listeners, so DataUpdateCoordinator never reschedules and the device never recovers without a manual reload. Record why entities can't be created offline here (discovery is the only source of `bound`, and platforms enumerate it once), what a working version would need (persisted discovery snapshot, reconcile-on-reconnect, a listener that keeps polling alive), and the cheaper retry-and-reload option that solves the filed issue on its own. --- docs/investigations/offline-setup.md | 149 +++++++++++++++++++++++++++ 1 file changed, 149 insertions(+) create mode 100644 docs/investigations/offline-setup.md diff --git a/docs/investigations/offline-setup.md b/docs/investigations/offline-setup.md new file mode 100644 index 0000000..2b9c64c --- /dev/null +++ b/docs/investigations/offline-setup.md @@ -0,0 +1,149 @@ +# Loading a config entry while the appliance is offline + +Issue #295 asks for sub-30s recovery when a powered-off appliance comes back, +instead of HA's `ConfigEntryNotReady` backoff (30s → … → 15 min). PR #303 +tries to get there by catching the first-refresh failure in +`async_setup_entry` and loading the entry anyway. + +That doesn't work here, and the reason is worth writing down: this +integration has no static entity list. Every entity comes from discovery, +and discovery only happens inside a successful poll. + +## What PR #303 produces today + +Measured on the PR's branch — set up with `_poll_once` raising, then advance +the clock four summary intervals: + +| | | +| --- | --- | +| entry state | `LOADED` | +| `coordinator.bound` | 0 | +| entities in the state machine | 0 | +| registry entries | 1 (the disabled connection-mode sensor) | +| coordinator listeners | 0 | +| `_unsub_refresh` | `None` | +| poll attempts over the next 4 intervals | **0** | + +The entry loads and then never polls again. `DataUpdateCoordinator._async_refresh` +reschedules only `if not auth_failed and self._listeners and not +self.hass.is_stopping`; with no bound entities the only unconditional entity +is `LocalThingsConnectionModeSensor`, which is +`entity_registry_enabled_default = False` and so never added and never +subscribes. Nothing reloads the entry either. The device comes back online to +an entry that is permanently empty until a manual reload — strictly worse +than the backoff it replaces, which did recover on its own within 15 minutes. + +## Why entities can't just be created offline + +Four independent gates, all of which need live device data: + +1. `bound` is only ever assigned in `_run_discovery` (`coordinator.py:1081`), + which runs on a poll's `resources` dict. +2. All ten platforms enumerate `coordinator.bound` exactly once, at forward + time (`sensor.py:34` and siblings). Nothing adds entities later — the + invariant is already documented at `coordinator.py:1283-1286`. +3. `_is_included` (`entity.py:39`) returns False whenever `last_resources` + has no rep for the href. Even a fully reconstructed `bound` filters to + nothing while `StateCache` is empty. +4. `LocalThingsEntity` is a bare `CoordinatorEntity` with no `available` + override, no `RestoreEntity`, and no `Store` anywhere in the component. An + entity that did exist offline would be `unavailable` with no state. + +The issue cites ESPHome, Shelly, LIFX and WLED as precedent for setup that +never fails. Those integrations can do it because each one has a *persisted +device description* to build entities from — ESPHome keeps its entity list in +`.storage`, Shelly caches device info. The pattern is portable; the mechanism +underneath it is the part PR #303 is missing. + +## What a working version needs + +Three separable pieces, plus a gating rule. + +### 1. A persisted discovery snapshot + +Write the discovery result back onto the config entry after each successful +`_run_discovery`. There is already precedent for exactly this shape — +`_persist_identity` (`coordinator.py:973`), `CONF_LEARNED_MODES`, +`CONF_CLOUD_COURSES`. + +`BoundEntity` holds live `Capability`/`SamsungEntityDescription` objects, so +it isn't directly serializable, but it is re-derivable: persist +`device_type_name`, the materialized `Subdevice` list (plain str/tuple +fields), and per entity the `(subdevice key, capability, desc.key, instance, +key_override, instance_name)` tuple. Rehydrate by re-resolving through +`resolve_registry` + `CAPABILITIES`. + +Persist the *post-`_is_included`* set, so rehydration doesn't need reps, and +flag the coordinator as rehydrated so gate 3 above is skipped for that pass. + +### 2. Reconcile on reconnect + +The snapshot is a guess about a device we haven't talked to yet. When the +first successful poll lands, `_run_discovery` computes the real set; if it +differs from what was rehydrated, the entry has to reload, because gate 2 +means platforms can't add the difference in place. `async_schedule_reload` +covers it (available well below the 2025.1.0 floor in `hacs.json`). + +This is the piece that makes the whole thing safe against a firmware update, +a newly-appearing subdevice, or a different appliance at the same IP. PR #303 +has no equivalent. + +### 3. Keep polling with no listeners + +Independent of the above, and worth doing on its own merits: hold one +refresh alive for the entry's lifetime so scheduling never depends on entity +count. + +```python +entry.async_on_unload(coordinator.async_add_listener(lambda: None)) +``` + +That alone fixes the measured "never polls again" bug, and covers the case +where every rehydrated entity happens to be registry-disabled. + +### Gating rule: only load offline when there's a snapshot + +An entry that has never successfully polled has nothing to restore and keeps +raising `ConfigEntryNotReady`. This is what answers the objection in the PR +thread — with a snapshot we *do* have metadata to build a device from, and +without one HA's backoff is still the right behavior. It also leaves room for +the #168-style flows that need to interact with the device during setup: a +device that never completed setup still blocks. + +## What this still won't do + +Entities will be present and `unavailable` — not showing their last values. +Gate 4 means last-known values require either `RestoreEntity` per platform or +persisting `StateCache`, and both mean asserting state the integration cannot +verify: a washer unplugged for a week would read "Running". HA's convention +is that unreachable means unavailable, and the recorder keeps the history +either way, so long-term statistics and history graphs are unaffected by this +choice. + +Worth being explicit about, because it is the gap between what PR #303 +promises in the thread ("load their previously recorded states") and what any +correct version can deliver. + +## The cheaper alternative + +If the goal is only issue #295's — fast recovery — pieces 1 and 2 are +unnecessary. Keep `ConfigEntryNotReady`, and add a retry that probes on a +fixed short interval and calls `async_schedule_reload` on the first success. +No persistence, no reconcile, no new failure modes. The device tile still +reads "Retrying setup" while the appliance is off, which is true, and +recovery drops from up-to-15-min to one probe interval. + +The issue's other suggestion — wire zeroconf so the device's own boot +announcement triggers the retry — is a non-starter as things stand: there is +no `zeroconf` or `dhcp` key in `manifest.json` and the config flow is +user-driven only, so HA has no discovery signal for this integration to hang +a retry on. It would first need a confirmed mDNS service on the appliance. + +## Recommendation + +The cheap alternative solves the filed issue. The three-piece version solves +the goal stated later in the PR thread, at the cost of a new persisted +schema, a reconcile path, and a reload-on-mismatch — and it still leaves +entities unavailable, which is the part that was actually being asked for. +Do the cheap one first; treat offline entity materialization as a separate +change with its own issue. From e684146f61a4069c1590b53730d3288f7be2e1cf Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Sat, 15 Aug 2026 20:05:58 +0000 Subject: [PATCH 3/5] Load a config entry offline from the last discovery snapshot (#295) An appliance switched off at the wall used to take its whole config entry down with it: async_setup_entry raised ConfigEntryNotReady, so the device read as failed and its entities existed only as registry rows until the appliance came back. Loading the entry anyway isn't enough on its own. Entities here are the output of discovery, discovery only runs inside a successful poll, and platforms enumerate `bound` exactly once at forward time -- so an entry that loads while offline loads empty, and with no listeners subscribed the base coordinator stops rescheduling and never polls again. Bank the resources dict each successful first cycle hands _run_discovery, along with the subdevice candidate list and the /oic identity that route the registry, and replay it through _run_discovery when the first refresh fails. Storing the poll input rather than a rendered entity list keeps one implementation of discovery instead of two: the offline entity set is produced by the same code that produced the live one. Three things fall out of that: - Platforms judge entity existence against `discovery_resources`, not the live cache. The live cache deliberately stays empty, which is what keeps a restored entity `unavailable` rather than rendering a stale value for an appliance nobody can currently reach. - A live discovery that disagrees with the snapshot reloads the entry -- platforms can't adopt a changed set in place, so a firmware update or a sibling subdevice that starts answering needs a fresh setup. - The entry holds one coordinator listener for its lifetime, so polling is scheduled regardless of how many entities are live. An entry that has never reached the device has no snapshot, keeps raising ConfigEntryNotReady, and closes its session on the way out as before -- no metadata to build a device from, and it leaves room for setup flows that need to interact with the appliance (#168). Restores the two tests PR #303 rewrote, narrowed to that no-snapshot path. --- README.md | 6 + custom_components/localthings/__init__.py | 40 +- custom_components/localthings/coordinator.py | 178 ++++++++- custom_components/localthings/entity.py | 8 +- docs/investigations/offline-setup.md | 117 +++--- tests/localthings/test_coordinator.py | 36 +- tests/localthings/test_offline_setup.py | 366 ++++++++++++++++++ ...st_airconditioner_ailp_fac_capabilities.py | 9 + tests/test_entity.py | 9 + tests/test_unique_ids.py | 9 + 10 files changed, 709 insertions(+), 69 deletions(-) create mode 100644 tests/localthings/test_offline_setup.py diff --git a/README.md b/README.md index 550cd21..671effc 100644 --- a/README.md +++ b/README.md @@ -270,6 +270,12 @@ If reconnects become persistent (more than a handful per minute), something's ac Deregistering a device in SmartThings causes a reset of its network settings as soon as it accesses Samsung's servers, dropping it off Wi-Fi until it's re-onboarded through the SmartThings app. As such, consider keeping devices registered even if egress-blocked, to avoid them resetting upon brief internet access. +### Restarting while an appliance is powered off + +If Home Assistant restarts while an appliance is unplugged or switched off at the wall, its device and entities still load — restored from the last successful discovery, showing `unavailable` until the appliance answers again. Automations and dashboards keep referring to entities that exist, and the integration retries in the background, so the device comes back on its own within a poll cycle of being powered on. Entities read `unavailable` rather than their last known values on purpose: the integration can't verify what a disconnected appliance is doing, and recorded history is kept by the recorder either way. + +This only applies to an appliance the integration has reached at least once. A brand-new device that has never answered has nothing to restore from, so setting it up still requires it to be reachable. + ### Multi-subdevice ("2-in-1") air conditioner systems 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. diff --git a/custom_components/localthings/__init__.py b/custom_components/localthings/__init__.py index ce47f79..ed5417f 100644 --- a/custom_components/localthings/__init__.py +++ b/custom_components/localthings/__init__.py @@ -13,12 +13,13 @@ from homeassistant.const import ( ) from homeassistant.const import EVENT_HOMEASSISTANT_STOP from homeassistant.core import Event, HomeAssistant, callback +from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.helpers import device_registry as dr from homeassistant.helpers import entity_registry as er from homeassistant.helpers.typing import ConfigType from .const import CONF_DEVICE_TYPE, CONF_HOST, CONF_PORT, CONF_SERIAL, DOMAIN, PLATFORMS -from .coordinator import LocalThingsCoordinator +from .coordinator import LocalThingsCoordinator, snapshot_store from .registry.identity import resolve_serial from .services import async_setup_services @@ -259,13 +260,30 @@ async def async_migrate_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: hass.data.setdefault(DOMAIN, {}) coordinator = LocalThingsCoordinator(hass, entry) + + # Before the first refresh, so the coordinator keeps rescheduling even + # when that refresh fails and leaves nothing subscribed: the base class + # only re-arms its timer while it has listeners, and an offline load can + # legitimately have zero live entities (every one of them disabled, say). + # Without this the entry loads once and never polls again (issue #295). + entry.async_on_unload(coordinator.async_add_listener(lambda: None)) + try: await coordinator.async_config_entry_first_refresh() - except Exception as err: - _LOGGER.warning( - "Initial connection to device failed (%s); starting offline and retrying in background", - err, - ) + except ConfigEntryNotReady: + # An entry that has polled successfully before comes up on its last + # known entity set and keeps retrying on the normal poll interval, + # rather than sitting in setup-retry with a device that reads as + # broken and entities that exist only as registry rows (issue #295). + # + # An entry that has never reached the device has no snapshot, so + # there is nothing to show and no device metadata to name it with -- + # that case still fails, which is also what keeps the door open for + # setup flows that need to interact with the device (issue #168). + if not await coordinator.async_rehydrate(): + await coordinator.async_close() + raise + hass.data[DOMAIN][entry.entry_id] = coordinator # Send the DTLS close_notify on Core shutdown, not just on unload (issue @@ -322,6 +340,16 @@ async def async_remove_config_entry_device( return not (device.identifiers & live) +async def async_remove_entry(hass: HomeAssistant, entry: ConfigEntry) -> None: + """Delete the discovery snapshot this entry accumulated (issue #295). + + Nothing else would: the store is keyed on entry_id, so re-adding the same + appliance mints a new one and the old file would linger in .storage + forever. + """ + await snapshot_store(hass, entry).async_remove() + + async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: unloaded = await hass.config_entries.async_unload_platforms(entry, PLATFORMS) if unloaded: diff --git a/custom_components/localthings/coordinator.py b/custom_components/localthings/coordinator.py index c41785c..e02124a 100644 --- a/custom_components/localthings/coordinator.py +++ b/custom_components/localthings/coordinator.py @@ -9,6 +9,7 @@ import logging import threading import time import zlib +from dataclasses import asdict from datetime import timedelta from typing import Any, cast @@ -18,6 +19,7 @@ from homeassistant.core import HomeAssistant, callback from homeassistant.exceptions import HomeAssistantError, ServiceValidationError from homeassistant.helpers import issue_registry as ir from homeassistant.helpers.device_registry import DeviceInfo +from homeassistant.helpers.storage import Store from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed from smartthings_local.ocf.state_cache import StateCache from smartthings_local.protocol.dtls_session import DtlsCoapSession @@ -47,7 +49,7 @@ from .const import ( from .learned import LEARNABLE, LearnedModes, persist from .observe import GRACE_PERIOD_S, MODE_OBSERVE, MODE_POLL, ObserveManager from .registry import CAPABILITIES -from .registry.adapter import flatten +from .registry.adapter import _key, flatten from .registry.batch import parse_device0_batch from .registry.by_type import resolve as resolve_registry from .registry.capabilities.common import ( @@ -83,6 +85,21 @@ _LOGGER = logging.getLogger(__name__) _SEED_PATH = ["device", "0"] +# Discovery snapshot (issue #295): exactly what the last successful first +# cycle fed _run_discovery, so a restart can register the same entities +# while the appliance is unreachable. Kept in .storage rather than on the +# config entry -- it's device state, not configuration, and runs to tens of +# kilobytes. +_SNAPSHOT_VERSION = 1 +_SNAPSHOT_SAVE_DELAY_S = 10.0 + + +def snapshot_store(hass: HomeAssistant, entry: ConfigEntry) -> Store[dict[str, Any]]: + """This entry's discovery-snapshot store. A free function so + `async_remove_entry` can delete the file without standing up a whole + coordinator to reach it.""" + return Store(hass, _SNAPSHOT_VERSION, f"{DOMAIN}.{entry.entry_id}.discovery") + class _NoOpDescriptor: """No-op: StateCache requires an on_observation hook; this integration @@ -248,6 +265,11 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): self._identity: DeviceIdentity | None = None self._discovered = False self.bound = [] + self._snapshot_store = snapshot_store(hass, entry) + # Both set only when this entry loaded from a snapshot instead of a + # live poll -- see async_rehydrate. + self._rehydrate_resources: dict[str, dict] | None = None + self._rehydrated_keys: frozenset[tuple[str, str]] | None = None # Sibling indoor subdevices on this connection (issue #177); set # once at first discovery, narrowed to the ones with live state (see # subdevices.discover_partitioned). Never includes MAIN itself. @@ -405,6 +427,35 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): self._canonical_cache[view_key] = view return view + @property + def rehydrated(self) -> bool: + """True while this entry's entities came from a snapshot rather than + a live poll (issue #295).""" + return self._rehydrate_resources is not None + + @property + def discovery_resources(self) -> dict[str, dict]: + """What entity._is_included should judge an entity's existence + against: the rehydration snapshot on an offline load, the live cache + otherwise. + + Deliberately separate from `last_resources`, which stays empty until + the device answers -- that emptiness is what keeps a rehydrated + entity `unavailable` instead of rendering a snapshot's stale value. + Only read while platforms are being forwarded; nothing consults it + once the entities exist. + """ + if self._rehydrate_resources is None: + return self.last_resources + return self._rehydrate_resources + + def discovery_canonical(self, subdevice: Subdevice) -> dict[str, dict]: + """`discovery_resources` in `subdevice`'s canonical view -- the + exists_fn counterpart to canonical_resources.""" + if self._rehydrate_resources is None: + return self.canonical_resources(subdevice) + return canonical_view(subdevice, self._rehydrate_resources, self.subdevices) + # ------------------------------------------------------------------ # Learned modes (issue #327) # ------------------------------------------------------------------ @@ -999,6 +1050,128 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): self._entry, data={**self._entry.data, **identity} ) + # ------------------------------------------------------------------ + # Discovery snapshot (issue #295) + # ------------------------------------------------------------------ + + def _bound_keys(self) -> frozenset[tuple[str, str]]: + """This entity set's identity: one (subdevice, state key) pair per + bound entity. `_key` is the unique_id suffix, so two discoveries that + agree here would register byte-identical entities.""" + return frozenset((b.subdevice.key, _key(b)) for b in self.bound) + + @callback + def _save_snapshot(self, resources: dict[str, dict], candidates: list[Subdevice]) -> None: + """Record what this first cycle handed `_run_discovery`, so the next + restart can replay it while the appliance is unreachable. + + `candidates` is the pre-narrowing subdevice list (issue #177): + `discover_partitioned` takes candidates and returns the live ones, so + replaying against the narrowed list would rediscover nothing for a + composite appliance's siblings. + """ + ident = self._identity + payload = { + "resources": dict(resources), + "subdevice_candidates": [asdict(su) for su in candidates], + "identity": asdict(ident) if ident is not None else None, + } + self._snapshot_store.async_delay_save(lambda: payload, _SNAPSHOT_SAVE_DELAY_S) + + async def async_rehydrate(self) -> bool: + """Register the last known entity set without reaching the device. + + Replays the stored snapshot through `_run_discovery`, which is what + makes this faithful: same code path, same registry resolution, so an + offline load produces the entity set the device last actually + reported rather than a guess reconstructed from a parallel format. + + Returns False when there's nothing stored (an entry that has never + polled successfully) or the replay produced nothing usable -- the + caller raises ConfigEntryNotReady in that case, exactly as before. + """ + try: + stored = await self._snapshot_store.async_load() + except Exception as e: # corrupt or unreadable store + self._log.warning("could not read discovery snapshot: %s", e) + return False + if not stored or not stored.get("resources"): + return False + + ident = stored.get("identity") + if ident is not None: + self._identity = DeviceIdentity( + manufacturer=ident.get("manufacturer") or "", + model=ident.get("model") or "", + name=ident.get("name") or "", + serial=ident.get("serial"), + device_types=tuple(ident.get("device_types") or ()), + raw=ident.get("raw") or {}, + ) + # JSON gives lists back where Subdevice declares tuples, and it's a + # frozen (hashable) dataclass used as a dict key in flatten(). + self.subdevices = [ + Subdevice( + kind=su["kind"], + key=su["key"], + seed_path=tuple(su.get("seed_path") or ()), + flat_hrefs=tuple(su.get("flat_hrefs") or ()), + ) + for su in stored.get("subdevice_candidates") or () + ] + + resources = stored["resources"] + try: + self._run_discovery(resources) + except Exception as e: + # A snapshot written by an older release can outlive the registry + # shape it was discovered against. Fall back to the pre-#295 + # behavior rather than failing the entry outright. + self._log.warning("discovery snapshot could not be replayed: %s", e, exc_info=True) + self.bound = [] + return False + + # _run_discovery sets _discovered; put it back. The snapshot only + # supplied an entity set to register -- the first live poll must + # still enumerate subdevices and rediscover for real. + self._discovered = False + self._rehydrate_resources = resources + self._rehydrated_keys = self._bound_keys() + if not self.bound: + return False + self._log.info( + "device unreachable; restored %d entities from the last discovery " + "snapshot and will keep retrying every %ds", + len(self.bound), + SUMMARY_INTERVAL_S, + ) + return True + + @callback + def _reconcile_rehydrated(self) -> None: + """Reload the entry when a live discovery disagrees with the snapshot + this load registered from. + + Platforms enumerate `bound` exactly once, at forward time, so a + firmware update, a newly-answering sibling subdevice or a different + appliance at the same IP can't be picked up in place -- the entry has + to come back up against the live set. + """ + if self._rehydrated_keys is None: + return + stale = self._rehydrated_keys + self._rehydrated_keys = None + live = self._bound_keys() + if live == stale: + return + self._log.info( + "live discovery differs from the snapshot this entry loaded from " + "(%d entities gone, %d new); reloading", + len(stale - live), + len(live - stale), + ) + self.hass.config_entries.async_schedule_reload(self._entry.entry_id) + def _run_discovery(self, resources: dict[str, dict]) -> None: # Diagnostics only -- names the firmware generation (e.g. '7.0 Air # conditioner' is Tizen Lite); doesn't route, since every device @@ -1422,7 +1595,10 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): # StateCache has no eviction, so the only way to keep them out # is to not put them in. Safe to reorder: _run_discovery reads # the passed dict, never the cache. + candidates = list(self.subdevices) self._run_discovery(resources) + self._save_snapshot(resources, candidates) + self._reconcile_rehydrated() resources = self._live_subdevice_resources(resources) sweep_mismatch = False if self._observe.mode == MODE_OBSERVE: diff --git a/custom_components/localthings/entity.py b/custom_components/localthings/entity.py index 770390c..a6fc65c 100644 --- a/custom_components/localthings/entity.py +++ b/custom_components/localthings/entity.py @@ -35,12 +35,16 @@ def _is_included(bound: BoundEntity, coordinator: LocalThingsCoordinator) -> boo `bound`'s own subdevice's canonical view instead of the raw snapshot, same rule as everywhere else a whole-resources-dict scan happens -- this is a free function, so it can't use self._resources. + + Reads `discovery_resources`, not `last_resources`: on an offline load + (issue #295) the live cache is still empty, and judging existence + against it would filter every rehydrated entity away. """ - rep = coordinator.last_resources.get(bound.href) + rep = coordinator.discovery_resources.get(bound.href) if rep is None: return False if bound.desc.exists_fn is not None: - return bound.desc.exists_fn(rep, coordinator.canonical_resources(bound.subdevice)) + return bound.desc.exists_fn(rep, coordinator.discovery_canonical(bound.subdevice)) if bound.desc.field: if not rep or is_stub_rep(rep): return True diff --git a/docs/investigations/offline-setup.md b/docs/investigations/offline-setup.md index 2b9c64c..70c334c 100644 --- a/docs/investigations/offline-setup.md +++ b/docs/investigations/offline-setup.md @@ -1,14 +1,19 @@ # Loading a config entry while the appliance is offline -Issue #295 asks for sub-30s recovery when a powered-off appliance comes back, -instead of HA's `ConfigEntryNotReady` backoff (30s → … → 15 min). PR #303 -tries to get there by catching the first-refresh failure in -`async_setup_entry` and loading the entry anyway. +Issue #295 asks for faster recovery when a powered-off appliance comes back, +instead of waiting out HA's `ConfigEntryNotReady` backoff. PR #303 tried to +get there by catching the first-refresh failure in `async_setup_entry` and +loading the entry anyway. That doesn't work here, and the reason is worth writing down: this integration has no static entity list. Every entity comes from discovery, and discovery only happens inside a successful poll. +(The issue's "up to 15 minutes" is out of date, incidentally. Current HA +retries on `2 ** min(tries, 4) * 5` seconds — capped at 80s, not 900. The +backoff was never the worst part; a device card reading "Retrying setup" with +no entities behind it is.) + ## What PR #303 produces today Measured on the PR's branch — set up with `_poll_once` raising, then advance @@ -55,51 +60,63 @@ device description* to build entities from — ESPHome keeps its entity list in `.storage`, Shelly caches device info. The pattern is portable; the mechanism underneath it is the part PR #303 is missing. -## What a working version needs +## How the implemented version works -Three separable pieces, plus a gating rule. +Three pieces, plus a gating rule. ### 1. A persisted discovery snapshot -Write the discovery result back onto the config entry after each successful -`_run_discovery`. There is already precedent for exactly this shape — -`_persist_identity` (`coordinator.py:973`), `CONF_LEARNED_MODES`, -`CONF_CLOUD_COURSES`. +After each successful first cycle, `_save_snapshot` banks exactly the +`resources` dict that cycle handed `_run_discovery`, along with the +pre-narrowing subdevice candidate list and the `DeviceIdentity` read from +`/oic/*`. -`BoundEntity` holds live `Capability`/`SamsungEntityDescription` objects, so -it isn't directly serializable, but it is re-derivable: persist -`device_type_name`, the materialized `Subdevice` list (plain str/tuple -fields), and per entity the `(subdevice key, capability, desc.key, instance, -key_override, instance_name)` tuple. Rehydrate by re-resolving through -`resolve_registry` + `CAPABILITIES`. +Storing the poll input rather than a rendered entity list is the decision +that keeps this honest. `BoundEntity` holds live +`Capability`/`SamsungEntityDescription` objects and isn't serializable, so +the alternative was a parallel format plus a re-resolution path — a second +implementation of discovery that could drift from the real one. Replaying the +input through `_run_discovery` means the same code, the same registry +resolution, and no second source of truth. -Persist the *post-`_is_included`* set, so rehydration doesn't need reps, and -flag the coordinator as rehydrated so gate 3 above is skipped for that pass. +Three things ride along because `_run_discovery` reads them off `self` +rather than out of `resources`, and getting them wrong would silently resolve +a *different* registry offline than online — which reconciliation below would +then see as a real change and reload on every restart: + +- `_identity.device_types` routes `resolve_registry`. +- `self.subdevices` is the candidate list `discover_partitioned` narrows; + replaying against the already-narrowed list finds no siblings at all. +- `_identity.manufacturer`/`model` feed `device_info`. + +It lives in `.storage` (`Store`, keyed on entry_id) rather than on the config +entry: it's device state, not configuration, and runs to tens of kilobytes. +`async_remove_entry` deletes it with the entry. ### 2. Reconcile on reconnect -The snapshot is a guess about a device we haven't talked to yet. When the -first successful poll lands, `_run_discovery` computes the real set; if it -differs from what was rehydrated, the entry has to reload, because gate 2 -means platforms can't add the difference in place. `async_schedule_reload` -covers it (available well below the 2025.1.0 floor in `hacs.json`). +The snapshot is a claim about a device we haven't talked to yet. When the +first live poll lands, `_reconcile_rehydrated` compares the live entity set +against the rehydrated one — as `(subdevice key, _key(bound))` pairs, which +is the unique_id identity — and calls `async_schedule_reload` if they differ. -This is the piece that makes the whole thing safe against a firmware update, -a newly-appearing subdevice, or a different appliance at the same IP. PR #303 -has no equivalent. +Gate 2 above is why this has to be a reload rather than an in-place fixup. +It's what makes the feature safe against a firmware update, a sibling +subdevice that starts answering, or a different appliance at the same IP. ### 3. Keep polling with no listeners -Independent of the above, and worth doing on its own merits: hold one -refresh alive for the entry's lifetime so scheduling never depends on entity -count. +`async_setup_entry` holds one listener for the entry's lifetime: ```python entry.async_on_unload(coordinator.async_add_listener(lambda: None)) ``` -That alone fixes the measured "never polls again" bug, and covers the case -where every rehydrated entity happens to be registry-disabled. +Registered *before* the first refresh, so scheduling survives a refresh that +fails. This alone fixes the measured "never polls again" bug, and covers a +rehydrated set whose entities are all registry-disabled. Removing the last +listener unschedules the timer, and HA runs `async_on_unload` callbacks when +setup raises, so the setup-retry path doesn't leak a polling coordinator. ### Gating rule: only load offline when there's a snapshot @@ -110,6 +127,10 @@ without one HA's backoff is still the right behavior. It also leaves room for the #168-style flows that need to interact with the device during setup: a device that never completed setup still blocks. +It also means `async_remove_config_entry_device` is no longer reachable with +an empty `coordinator.subdevices`, so an offline load can't offer to delete a +real-but-unreachable subdevice. + ## What this still won't do Entities will be present and `unavailable` — not showing their last values. @@ -124,26 +145,22 @@ Worth being explicit about, because it is the gap between what PR #303 promises in the thread ("load their previously recorded states") and what any correct version can deliver. -## The cheaper alternative - -If the goal is only issue #295's — fast recovery — pieces 1 and 2 are -unnecessary. Keep `ConfigEntryNotReady`, and add a retry that probes on a -fixed short interval and calls `async_schedule_reload` on the first success. -No persistence, no reconcile, no new failure modes. The device tile still -reads "Retrying setup" while the appliance is off, which is true, and -recovery drops from up-to-15-min to one probe interval. +## Rejected: zeroconf The issue's other suggestion — wire zeroconf so the device's own boot -announcement triggers the retry — is a non-starter as things stand: there is -no `zeroconf` or `dhcp` key in `manifest.json` and the config flow is -user-driven only, so HA has no discovery signal for this integration to hang -a retry on. It would first need a confirmed mDNS service on the appliance. +announcement triggers a retry, which is the genuinely idiomatic HA answer — +is a non-starter as things stand: there is no `zeroconf` or `dhcp` key in +`manifest.json` and the config flow is user-driven only, so HA has no +discovery signal for this integration to hang a retry on. It would first need +a confirmed mDNS service on the appliance. Worth revisiting if one turns up; +it would make recovery near-instant instead of within one poll interval. -## Recommendation +## Rejected: the cheap version -The cheap alternative solves the filed issue. The three-piece version solves -the goal stated later in the PR thread, at the cost of a new persisted -schema, a reconcile path, and a reload-on-mismatch — and it still leaves -entities unavailable, which is the part that was actually being asked for. -Do the cheap one first; treat offline entity materialization as a separate -change with its own issue. +Keeping `ConfigEntryNotReady` and adding a probe that calls +`async_schedule_reload` on first success would have fixed the recovery *time* +in about twenty lines, with no persistence and no reconcile. It was rejected +because it leaves the device reading as broken for as long as the appliance +is off, which is the half of issue #295 that actually bites — an appliance +switched off at the wall is offline for days, not seconds, and a whole +integration that looks failed for that entire window is the complaint. diff --git a/tests/localthings/test_coordinator.py b/tests/localthings/test_coordinator.py index 82cd159..2429e3c 100644 --- a/tests/localthings/test_coordinator.py +++ b/tests/localthings/test_coordinator.py @@ -70,8 +70,13 @@ async def test_summary_interval(hass: HomeAssistant, mock_entry, mock_coordinato assert coordinator.update_interval == timedelta(seconds=SUMMARY_INTERVAL_S) -async def test_offline_initial_setup_succeeds(hass: HomeAssistant, mock_entry) -> None: - """ConfigEntry setup succeeds in offline mode when initial poll fails.""" +async def test_update_failed_on_persistent_poll_error(hass: HomeAssistant, mock_entry) -> None: + """ConfigEntryNotReady raised when poll fails even after reconnect. + + An entry with no stored discovery snapshot has never reached this device, + so there is nothing to load offline from (issue #295) -- it stays on HA's + backoff rather than loading empty. + """ with ( patch("custom_components.localthings.coordinator.LocalThingsCoordinator._connect_session"), @@ -84,10 +89,8 @@ async def test_offline_initial_setup_succeeds(hass: HomeAssistant, mock_entry) - result = await hass.config_entries.async_setup(mock_entry.entry_id) await hass.async_block_till_done() - # Setup succeeds — entry should be in hass.data and state is LOADED - assert result - assert mock_entry.entry_id in hass.data.get(DOMAIN, {}) - assert mock_entry.state == ConfigEntryState.LOADED + # Setup fails — entry should not be in hass.data + assert not result or mock_entry.entry_id not in hass.data.get(DOMAIN, {}) # --------------------------------------------------------------------------- @@ -1823,11 +1826,14 @@ async def test_first_refresh_timeout_recovers_via_reconnect( assert coordinator._consecutive_poll_timeouts == 0 -async def test_first_refresh_persistent_timeout_succeeds_offline( +async def test_first_refresh_persistent_timeout_fails_setup( hass: HomeAssistant, mock_entry ) -> None: - """When initial refresh times out, setup still succeeds in offline mode - and retries in background.""" + """With no snapshot to load from, a reconnect that times out too must + fail the first refresh so HA retries on its backoff -- not load an + entity-less entry. The session it left open is closed on the way out + (`_poll_once` keeps it up on a `TimeoutError`, and the source port is + fixed per device).""" with ( patch("custom_components.localthings.coordinator.LocalThingsCoordinator._connect_session"), patch( @@ -1835,11 +1841,21 @@ async def test_first_refresh_persistent_timeout_succeeds_offline( side_effect=_HANDSHAKE_TIMEOUT, ), patch("custom_components.localthings.coordinator.LocalThingsCoordinator._close_session"), + # Asserted on `async_close` rather than `_close_session`: the + # reconnect path calls the latter on its own, so it is already true + # whether or not setup cleans up after itself. + patch.object( + LocalThingsCoordinator, + "async_close", + autospec=True, + ) as mock_async_close, ): await hass.config_entries.async_setup(mock_entry.entry_id) await hass.async_block_till_done() - assert mock_entry.state is ConfigEntryState.LOADED + assert mock_entry.state is ConfigEntryState.SETUP_RETRY + assert not hass.states.async_all() + mock_async_close.assert_awaited_once() async def test_post_discovery_timeout_is_still_deferred( diff --git a/tests/localthings/test_offline_setup.py b/tests/localthings/test_offline_setup.py new file mode 100644 index 0000000..37a659e --- /dev/null +++ b/tests/localthings/test_offline_setup.py @@ -0,0 +1,366 @@ +"""Loading a config entry while the appliance is unreachable (issue #295). + +The device's entity set only exists as the output of a live poll, so coming +up offline means replaying the last successful discovery from a stored +snapshot. These tests pin the four things that makes load-bearing: the +snapshot gets written, it produces the same entity set offline, the +coordinator keeps polling until the device answers, and a live discovery that +disagrees with the snapshot reloads the entry rather than silently keeping a +stale set. +""" + +from __future__ import annotations + +from contextlib import contextmanager +from datetime import timedelta +from unittest.mock import patch + +import pytest +from homeassistant.config_entries import ConfigEntryState +from homeassistant.core import HomeAssistant +from homeassistant.util import dt as dt_util +from pytest_homeassistant_custom_component.common import async_fire_time_changed + +from custom_components.localthings.const import DOMAIN, SUMMARY_INTERVAL_S +from custom_components.localthings.coordinator import ( + _SNAPSHOT_SAVE_DELAY_S, + LocalThingsCoordinator, +) +from custom_components.localthings.registry.identity import DeviceIdentity + +from .conftest import _load_fridge_resources as _load_fridge + +_COORD = "custom_components.localthings.coordinator.LocalThingsCoordinator" + + +@contextmanager +def _reachable(resources: dict, identity: DeviceIdentity | None = None): + """A device that answers, optionally with an /oic/* identity -- which + `_connect_session` is what normally reads, so a test that patches it out + otherwise leaves `_identity` None.""" + + def _connect(self) -> None: + self._identity = identity + + with ( + patch(f"{_COORD}._connect_session", _connect), + patch(f"{_COORD}._poll_once", return_value=resources), + patch(f"{_COORD}._close_session"), + ): + yield + + +@contextmanager +def _unreachable(): + with ( + patch(f"{_COORD}._connect_session"), + patch(f"{_COORD}._poll_once", side_effect=OSError("device offline")), + patch(f"{_COORD}._close_session"), + ): + yield + + +def _store_key(entry) -> str: + return f"{DOMAIN}.{entry.entry_id}.discovery" + + +async def _flush_snapshot(hass: HomeAssistant) -> None: + """Run out `async_delay_save`'s timer without reaching the next poll.""" + async_fire_time_changed(hass, dt_util.utcnow() + timedelta(seconds=_SNAPSHOT_SAVE_DELAY_S + 1)) + await hass.async_block_till_done() + + +async def _tick(hass: HomeAssistant) -> None: + """Advance past one summary interval so the coordinator polls again. + + `wait_background_tasks` is load-bearing: DataUpdateCoordinator runs its + interval refresh as a background task, which a plain block_till_done + doesn't await -- the poll would still be in flight at the assertion. + """ + async_fire_time_changed(hass, dt_util.utcnow() + timedelta(seconds=SUMMARY_INTERVAL_S + 1)) + await hass.async_block_till_done(wait_background_tasks=True) + + +async def _setup_online_then_unload(hass: HomeAssistant, entry, resources: dict) -> set[str]: + """Bring the entry up against a live device, bank the snapshot, and take + it back down. Returns the entity_ids that run produced.""" + with _reachable(resources): + await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + entity_ids = {s.entity_id for s in hass.states.async_all()} + await _flush_snapshot(hass) + await hass.config_entries.async_unload(entry.entry_id) + await hass.async_block_till_done() + return entity_ids + + +# --------------------------------------------------------------------------- +# Writing the snapshot +# --------------------------------------------------------------------------- + + +async def test_snapshot_written_after_first_discovery( + hass: HomeAssistant, mock_entry, mock_coordinator_session, hass_storage +) -> None: + """A successful first cycle banks what it handed _run_discovery.""" + await hass.config_entries.async_setup(mock_entry.entry_id) + await hass.async_block_till_done() + await _flush_snapshot(hass) + + stored = hass_storage[_store_key(mock_entry)]["data"] + assert stored["resources"] + assert "/information/vs/0" in stored["resources"] + assert "subdevice_candidates" in stored + + +async def test_snapshot_not_written_when_device_never_answers( + hass: HomeAssistant, mock_entry, hass_storage +) -> None: + """Nothing to bank, so nothing is -- this is what keeps the no-snapshot + gate meaningful on a brand-new entry.""" + with _unreachable(): + await hass.config_entries.async_setup(mock_entry.entry_id) + await hass.async_block_till_done() + # Asserted before the flush: running the save timer out also runs out + # HA's own setup-retry timer, which puts the entry back into + # SETUP_IN_PROGRESS. + assert mock_entry.state is ConfigEntryState.SETUP_RETRY + await _flush_snapshot(hass) + + assert _store_key(mock_entry) not in hass_storage + + +async def test_snapshot_removed_when_entry_removed( + hass: HomeAssistant, mock_entry, mock_coordinator_session, hass_storage +) -> None: + """The store is keyed on entry_id, so re-adding the appliance mints a new + one -- the old file has to go with the entry that wrote it.""" + await hass.config_entries.async_setup(mock_entry.entry_id) + await hass.async_block_till_done() + await _flush_snapshot(hass) + assert _store_key(mock_entry) in hass_storage + + await hass.config_entries.async_remove(mock_entry.entry_id) + await hass.async_block_till_done() + + assert hass_storage.get(_store_key(mock_entry), {}).get("data") is None + + +# --------------------------------------------------------------------------- +# Loading from it +# --------------------------------------------------------------------------- + + +async def test_offline_load_restores_the_same_entity_set( + hass: HomeAssistant, mock_entry, hass_storage +) -> None: + """The whole point: a restart with the appliance powered off comes up on + the entity set the device last actually reported.""" + resources = _load_fridge() + online_ids = await _setup_online_then_unload(hass, mock_entry, resources) + assert online_ids # guard: the online run must actually produce entities + + with _unreachable(): + await hass.config_entries.async_setup(mock_entry.entry_id) + await hass.async_block_till_done() + + assert mock_entry.state is ConfigEntryState.LOADED + assert {s.entity_id for s in hass.states.async_all()} == online_ids + + +async def test_offline_entities_are_unavailable_not_stale(hass: HomeAssistant, mock_entry) -> None: + """Restored entities must not render the snapshot's values -- the + appliance is unreachable, so `unavailable` is the honest state and the + live cache stays empty to enforce it.""" + resources = _load_fridge() + await _setup_online_then_unload(hass, mock_entry, resources) + + with _unreachable(): + await hass.config_entries.async_setup(mock_entry.entry_id) + await hass.async_block_till_done() + + states = hass.states.async_all() + assert states + assert all(s.state == "unavailable" for s in states) + + coordinator: LocalThingsCoordinator = hass.data[DOMAIN][mock_entry.entry_id] + assert coordinator.rehydrated + assert not coordinator.last_resources + + +async def test_offline_load_without_snapshot_still_fails(hass: HomeAssistant, mock_entry) -> None: + """No snapshot means no device metadata to build anything from, so the + entry stays on HA's backoff rather than loading empty.""" + with _unreachable(): + await hass.config_entries.async_setup(mock_entry.entry_id) + await hass.async_block_till_done() + + assert mock_entry.state is ConfigEntryState.SETUP_RETRY + assert not hass.states.async_all() + + +async def test_corrupt_snapshot_falls_back_to_setup_retry( + hass: HomeAssistant, mock_entry, hass_storage +) -> None: + """A snapshot whose resources no longer replay cleanly must not take the + entry down with it.""" + key = _store_key(mock_entry) + hass_storage[key] = { + "version": 1, + "minor_version": 1, + "key": key, + "data": {"resources": {"/information/vs/0": "not-a-rep"}, "subdevice_candidates": []}, + } + + with _unreachable(): + await hass.config_entries.async_setup(mock_entry.entry_id) + await hass.async_block_till_done() + + assert mock_entry.state is ConfigEntryState.SETUP_RETRY + + +async def test_snapshot_restores_identity(hass: HomeAssistant, mock_entry) -> None: + """`/oic/d`'s device types route the registry, so an offline load that + lost them could resolve a different one than the live poll did -- which + would show up as a spurious reconcile reload every restart.""" + resources = _load_fridge() + identity = DeviceIdentity( + manufacturer="Samsung", + model="TEST-MODEL", + name="Fridge", + serial=None, + device_types=("oic.d.refrigerator",), + raw={"/oic/p": {}, "/oic/d": {}, "/oic/res": []}, + ) + with _reachable(resources, identity): + await hass.config_entries.async_setup(mock_entry.entry_id) + await hass.async_block_till_done() + await _flush_snapshot(hass) + await hass.config_entries.async_unload(mock_entry.entry_id) + await hass.async_block_till_done() + + with _unreachable(): + await hass.config_entries.async_setup(mock_entry.entry_id) + await hass.async_block_till_done() + + coordinator: LocalThingsCoordinator = hass.data[DOMAIN][mock_entry.entry_id] + assert coordinator._identity is not None + assert coordinator._identity.device_types == ("oic.d.refrigerator",) + + +# --------------------------------------------------------------------------- +# Recovery and reconciliation +# --------------------------------------------------------------------------- + + +async def test_offline_load_keeps_polling_and_recovers(hass: HomeAssistant, mock_entry) -> None: + """The failure the PR this replaces actually shipped: with zero live + listeners the base coordinator stops rescheduling, and the entry never + polls again. Entities must go available on the next interval once the + appliance answers.""" + resources = _load_fridge() + await _setup_online_then_unload(hass, mock_entry, resources) + + with _unreachable(): + await hass.config_entries.async_setup(mock_entry.entry_id) + await hass.async_block_till_done() + + coordinator: LocalThingsCoordinator = hass.data[DOMAIN][mock_entry.entry_id] + assert coordinator._unsub_refresh is not None # a poll is actually queued + assert not coordinator.last_update_success + + with _reachable(resources): + await _tick(hass) + + assert coordinator.last_update_success + assert any(s.state != "unavailable" for s in hass.states.async_all()) + + +async def test_reconcile_reloads_when_live_discovery_differs( + hass: HomeAssistant, mock_entry +) -> None: + """Platforms enumerate `bound` once, so a live set that disagrees with the + snapshot can only be adopted by bringing the entry back up.""" + resources = _load_fridge() + + with _reachable(resources): + await hass.config_entries.async_setup(mock_entry.entry_id) + await hass.async_block_till_done() + coordinator: LocalThingsCoordinator = hass.data[DOMAIN][mock_entry.entry_id] + victim = coordinator.bound[0].href + await _flush_snapshot(hass) + await hass.config_entries.async_unload(mock_entry.entry_id) + await hass.async_block_till_done() + + with _unreachable(): + await hass.config_entries.async_setup(mock_entry.entry_id) + await hass.async_block_till_done() + + reduced = {href: rep for href, rep in resources.items() if href != victim} + with ( + _reachable(reduced), + patch.object(hass.config_entries, "async_schedule_reload") as reload, + ): + await _tick(hass) + + reload.assert_called_once_with(mock_entry.entry_id) + + +async def test_reconcile_is_quiet_when_live_discovery_agrees( + hass: HomeAssistant, mock_entry +) -> None: + """The common case -- same appliance, same firmware -- must not reload, + or every offline restart would cost a second setup cycle.""" + resources = _load_fridge() + await _setup_online_then_unload(hass, mock_entry, resources) + + with _unreachable(): + await hass.config_entries.async_setup(mock_entry.entry_id) + await hass.async_block_till_done() + + with ( + _reachable(resources), + patch.object(hass.config_entries, "async_schedule_reload") as reload, + ): + await _tick(hass) + + reload.assert_not_called() + + +async def test_live_load_never_reconciles( + hass: HomeAssistant, mock_entry, mock_coordinator_session +) -> None: + """An entry that came up against a live device has nothing to reconcile + against; the reload path must stay out of the normal startup entirely.""" + with patch.object(hass.config_entries, "async_schedule_reload") as reload: + await hass.config_entries.async_setup(mock_entry.entry_id) + await hass.async_block_till_done() + + coordinator: LocalThingsCoordinator = hass.data[DOMAIN][mock_entry.entry_id] + assert not coordinator.rehydrated + reload.assert_not_called() + + +@pytest.mark.parametrize("failures", [1, 3]) +async def test_offline_load_survives_repeated_poll_failures( + hass: HomeAssistant, mock_entry, failures: int +) -> None: + """Recovery isn't one-shot: the entry keeps its entities and keeps + retrying across however many intervals the appliance stays dark.""" + resources = _load_fridge() + online_ids = await _setup_online_then_unload(hass, mock_entry, resources) + + with _unreachable(): + await hass.config_entries.async_setup(mock_entry.entry_id) + await hass.async_block_till_done() + for _ in range(failures): + await _tick(hass) + + assert mock_entry.state is ConfigEntryState.LOADED + assert {s.entity_id for s in hass.states.async_all()} == online_ids + + with _reachable(resources): + await _tick(hass) + + coordinator: LocalThingsCoordinator = hass.data[DOMAIN][mock_entry.entry_id] + assert coordinator.last_update_success diff --git a/tests/test_airconditioner_ailp_fac_capabilities.py b/tests/test_airconditioner_ailp_fac_capabilities.py index 9316c8f..4cff87a 100644 --- a/tests/test_airconditioner_ailp_fac_capabilities.py +++ b/tests/test_airconditioner_ailp_fac_capabilities.py @@ -36,6 +36,15 @@ class _FakeCoordinator: def canonical_resources(self, subdevice): return self.last_resources + # _is_included judges existence against the discovery view, which is the + # live cache for everything but an offline load (issue #295). + @property + def discovery_resources(self): + return self.last_resources + + def discovery_canonical(self, subdevice): + return self.canonical_resources(subdevice) + def _resources(): return _load_device("airconditioner_ailp_fac") diff --git a/tests/test_entity.py b/tests/test_entity.py index f0c0b9b..2eb234e 100644 --- a/tests/test_entity.py +++ b/tests/test_entity.py @@ -29,6 +29,15 @@ class _FakeCoordinator: # subdevices (issue #177). return self.last_resources + # _is_included judges existence against the discovery view, which is the + # live cache for everything but an offline load (issue #295). + @property + def discovery_resources(self): + return self.last_resources + + def discovery_canonical(self, subdevice): + return self.canonical_resources(subdevice) + def _coord(last_resources) -> LocalThingsCoordinator: return cast(LocalThingsCoordinator, _FakeCoordinator(last_resources)) diff --git a/tests/test_unique_ids.py b/tests/test_unique_ids.py index f679508..643a406 100644 --- a/tests/test_unique_ids.py +++ b/tests/test_unique_ids.py @@ -34,6 +34,15 @@ class _FakeCoordinator: return canonical_view(subdevice, self.last_resources, self._subdevices) + # _is_included judges existence against the discovery view, which is the + # live cache for everything but an offline load (issue #295). + @property + def discovery_resources(self): + return self.last_resources + + def discovery_canonical(self, subdevice): + return self.canonical_resources(subdevice) + @pytest.mark.parametrize("name", _FIXTURE_NAMES) def test_key_is_unique_across_all_bound_entities(name): From edff7385c6e2ee9c04749db017b253e4d5824e07 Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Sat, 15 Aug 2026 20:13:02 +0000 Subject: [PATCH 4/5] Raise the coverage-gap Repair only from a live poll A coverage gap is a claim about what the device currently reports, so replaying a discovery snapshot shouldn't make it. Offline it would restate the last live poll's conclusion while pointing the user at a diagnostics download that stays empty until the appliance answers, and any drift in the resolved device name between snapshot and live would churn the issue. Not deduplication: HA already keys issues on (domain, issue_id), preserves dismissed_version across async_get_or_create, and reloads non-persistent issues with their dismissal intact -- one row per entry, and an "Ignore" survives restarts. --- custom_components/localthings/coordinator.py | 12 +++++++--- docs/investigations/offline-setup.md | 13 +++++++++++ tests/localthings/test_offline_setup.py | 23 ++++++++++++++++++++ 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/custom_components/localthings/coordinator.py b/custom_components/localthings/coordinator.py index e02124a..5682454 100644 --- a/custom_components/localthings/coordinator.py +++ b/custom_components/localthings/coordinator.py @@ -1122,7 +1122,7 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): resources = stored["resources"] try: - self._run_discovery(resources) + self._run_discovery(resources, from_snapshot=True) except Exception as e: # A snapshot written by an older release can outlive the registry # shape it was discovered against. Fall back to the pre-#295 @@ -1172,7 +1172,7 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): ) self.hass.config_entries.async_schedule_reload(self._entry.entry_id) - def _run_discovery(self, resources: dict[str, dict]) -> None: + def _run_discovery(self, resources: dict[str, dict], from_snapshot: bool = False) -> None: # Diagnostics only -- names the firmware generation (e.g. '7.0 Air # conditioner' is Tizen Lite); doesn't route, since every device # that reports it is already typed by modelNum. @@ -1288,7 +1288,13 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): model=model, ) self._persist_identity(serial, model, mfr, device_type_name) - self._update_coverage_gap_issue(device_type_name is None, unbound, name) + if not from_snapshot: + # A coverage gap is a claim about what the device reports, so only + # a live poll gets to make it. Replaying a snapshot would restate + # last run's conclusion while pointing the user at a diagnostics + # download that is empty until the appliance answers, and any + # drift in the device name between the two would churn the issue. + self._update_coverage_gap_issue(device_type_name is None, unbound, name) self._hot_hrefs = sorted(hot) self._warm_hrefs = sorted(warm) diff --git a/docs/investigations/offline-setup.md b/docs/investigations/offline-setup.md index 70c334c..95b812e 100644 --- a/docs/investigations/offline-setup.md +++ b/docs/investigations/offline-setup.md @@ -131,6 +131,19 @@ It also means `async_remove_config_entry_device` is no longer reachable with an empty `coordinator.subdevices`, so an offline load can't offer to delete a real-but-unreachable subdevice. +### The coverage-gap Repair stays live-only + +`_run_discovery(..., from_snapshot=True)` skips `_update_coverage_gap_issue`. +The Repair points the user at a diagnostics download, which is empty until +the appliance answers, and a device name that drifts between the snapshot and +the live poll would churn the issue for no reason. + +Not a de-duplication measure — HA already handles that. `async_create_issue` +is keyed on `(domain, issue_id)`, `dataclasses.replace` in +`async_get_or_create` leaves `dismissed_version` alone, and the registry +reloads non-persistent issues with their dismissal intact, so one row per +entry survives restarts and an "Ignore" sticks. + ## What this still won't do Entities will be present and `unavailable` — not showing their last values. diff --git a/tests/localthings/test_offline_setup.py b/tests/localthings/test_offline_setup.py index 37a659e..067038f 100644 --- a/tests/localthings/test_offline_setup.py +++ b/tests/localthings/test_offline_setup.py @@ -18,6 +18,7 @@ from unittest.mock import patch import pytest from homeassistant.config_entries import ConfigEntryState from homeassistant.core import HomeAssistant +from homeassistant.helpers import issue_registry as ir from homeassistant.util import dt as dt_util from pytest_homeassistant_custom_component.common import async_fire_time_changed @@ -327,6 +328,28 @@ async def test_reconcile_is_quiet_when_live_discovery_agrees( reload.assert_not_called() +def test_coverage_gap_repair_is_live_only(hass: HomeAssistant, mock_entry) -> None: + """A coverage gap is a claim about what the device reports, so replaying + a snapshot must not raise the Repair -- it would restate last run's + conclusion while the diagnostics download it points at is still empty.""" + gappy = { + "/information/vs/0": { + "x.com.samsung.da.modelNum": "TOTALLY_UNKNOWN_BOARD", + "x.com.samsung.da.serialNum": "TEST-SERIAL-0000", + }, + "/nothing/maps/this/vs/0": {"someField": 1}, + } + issue_id = f"device_gap_{mock_entry.entry_id}" + coordinator = LocalThingsCoordinator(hass, mock_entry) + + coordinator._run_discovery(gappy, from_snapshot=True) + assert coordinator._unbound_hrefs # the gap is real, it just stays quiet + assert ir.async_get(hass).async_get_issue(DOMAIN, issue_id) is None + + coordinator._run_discovery(gappy) + assert ir.async_get(hass).async_get_issue(DOMAIN, issue_id) is not None + + async def test_live_load_never_reconciles( hass: HomeAssistant, mock_entry, mock_coordinator_session ) -> None: From 9d3782a29afa55387c713053973bc562a54b449a Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Sat, 15 Aug 2026 20:32:46 +0000 Subject: [PATCH 5/5] Harden the snapshot path against three review findings Widen async_rehydrate's guard to cover the identity and Subdevice rebuild, not just the replay. A stored row missing a field the current dataclass declares raised KeyError straight out of async_setup_entry, which only handles ConfigEntryNotReady -- so the entry landed in SETUP_ERROR, which HA never retries, with its DTLS session left open on the fixed source port the next attempt binds. It now fails the same way an unreachable device does. Write the snapshot immediately instead of through async_delay_save. A deferred write outlives whatever queued it: removing an entry inside the delay window deleted the file and then had it recreated, orphaned, when the timer fired; and a reload scheduled by _reconcile_rehydrated read the pre-reload snapshot back off disk, so a device going quiet again mid-reload rehydrated the stale set and reconciled a second time. Banking it before the reconcile fixes the ordering. Failures are logged rather than raised -- a board reporting something the JSON encoder rejects must not break polling. --- custom_components/localthings/coordinator.py | 91 ++++++++++++-------- docs/investigations/offline-setup.md | 8 ++ tests/localthings/test_offline_setup.py | 71 ++++++++++----- 3 files changed, 114 insertions(+), 56 deletions(-) diff --git a/custom_components/localthings/coordinator.py b/custom_components/localthings/coordinator.py index 5682454..3a92961 100644 --- a/custom_components/localthings/coordinator.py +++ b/custom_components/localthings/coordinator.py @@ -91,7 +91,6 @@ _SEED_PATH = ["device", "0"] # config entry -- it's device state, not configuration, and runs to tens of # kilobytes. _SNAPSHOT_VERSION = 1 -_SNAPSHOT_SAVE_DELAY_S = 10.0 def snapshot_store(hass: HomeAssistant, entry: ConfigEntry) -> Store[dict[str, Any]]: @@ -1060,8 +1059,9 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): agree here would register byte-identical entities.""" return frozenset((b.subdevice.key, _key(b)) for b in self.bound) - @callback - def _save_snapshot(self, resources: dict[str, dict], candidates: list[Subdevice]) -> None: + async def _async_save_snapshot( + self, resources: dict[str, dict], candidates: list[Subdevice] + ) -> None: """Record what this first cycle handed `_run_discovery`, so the next restart can replay it while the appliance is unreachable. @@ -1069,14 +1069,29 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): `discover_partitioned` takes candidates and returns the live ones, so replaying against the narrowed list would rediscover nothing for a composite appliance's siblings. + + Written now rather than through `async_delay_save`, because a pending + delayed write outlives whatever queued it: it lands after + `async_remove_entry` has deleted the file and recreates it orphaned, + and a reload scheduled by `_reconcile_rehydrated` would read the + pre-reload snapshot back off disk. This runs once per entry load, so + the immediate write costs nothing worth deferring. """ ident = self._identity - payload = { - "resources": dict(resources), - "subdevice_candidates": [asdict(su) for su in candidates], - "identity": asdict(ident) if ident is not None else None, - } - self._snapshot_store.async_delay_save(lambda: payload, _SNAPSHOT_SAVE_DELAY_S) + try: + await self._snapshot_store.async_save( + { + "resources": dict(resources), + "subdevice_candidates": [asdict(su) for su in candidates], + "identity": asdict(ident) if ident is not None else None, + } + ) + except Exception as e: + # Never fail a poll over the snapshot -- a board reporting + # something the JSON encoder rejects would otherwise break + # polling outright. Worst case this entry can't load offline, + # which is where it was before any of this existed. + self._log.warning("could not write discovery snapshot: %s", e) async def async_rehydrate(self) -> bool: """Register the last known entity set without reaching the device. @@ -1098,37 +1113,40 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): if not stored or not stored.get("resources"): return False - ident = stored.get("identity") - if ident is not None: - self._identity = DeviceIdentity( - manufacturer=ident.get("manufacturer") or "", - model=ident.get("model") or "", - name=ident.get("name") or "", - serial=ident.get("serial"), - device_types=tuple(ident.get("device_types") or ()), - raw=ident.get("raw") or {}, - ) - # JSON gives lists back where Subdevice declares tuples, and it's a - # frozen (hashable) dataclass used as a dict key in flatten(). - self.subdevices = [ - Subdevice( - kind=su["kind"], - key=su["key"], - seed_path=tuple(su.get("seed_path") or ()), - flat_hrefs=tuple(su.get("flat_hrefs") or ()), - ) - for su in stored.get("subdevice_candidates") or () - ] - resources = stored["resources"] try: + ident = stored.get("identity") + if ident is not None: + self._identity = DeviceIdentity( + manufacturer=ident.get("manufacturer") or "", + model=ident.get("model") or "", + name=ident.get("name") or "", + serial=ident.get("serial"), + device_types=tuple(ident.get("device_types") or ()), + raw=ident.get("raw") or {}, + ) + # JSON gives lists back where Subdevice declares tuples, and it's + # a frozen (hashable) dataclass used as a dict key in flatten(). + self.subdevices = [ + Subdevice( + kind=su["kind"], + key=su["key"], + seed_path=tuple(su.get("seed_path") or ()), + flat_hrefs=tuple(su.get("flat_hrefs") or ()), + ) + for su in stored.get("subdevice_candidates") or () + ] self._run_discovery(resources, from_snapshot=True) except Exception as e: - # A snapshot written by an older release can outlive the registry - # shape it was discovered against. Fall back to the pre-#295 - # behavior rather than failing the entry outright. + # A snapshot written by an older release can outlive both the + # stored shape and the registry it was discovered against. This + # has to catch the rebuild as well as the replay: an exception + # escaping here reaches async_setup_entry, which only handles + # ConfigEntryNotReady, so the entry would land in SETUP_ERROR -- + # never retried, and with its session left open. self._log.warning("discovery snapshot could not be replayed: %s", e, exc_info=True) self.bound = [] + self.subdevices = [] return False # _run_discovery sets _discovered; put it back. The snapshot only @@ -1603,7 +1621,10 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): # the passed dict, never the cache. candidates = list(self.subdevices) self._run_discovery(resources) - self._save_snapshot(resources, candidates) + # Banked before the reconcile below, so a reload it schedules + # comes up against this discovery rather than the one that is + # being replaced. + await self._async_save_snapshot(resources, candidates) self._reconcile_rehydrated() resources = self._live_subdevice_resources(resources) sweep_mismatch = False diff --git a/docs/investigations/offline-setup.md b/docs/investigations/offline-setup.md index 95b812e..7ff350f 100644 --- a/docs/investigations/offline-setup.md +++ b/docs/investigations/offline-setup.md @@ -93,6 +93,14 @@ It lives in `.storage` (`Store`, keyed on entry_id) rather than on the config entry: it's device state, not configuration, and runs to tens of kilobytes. `async_remove_entry` deletes it with the entry. +The write is awaited, not `async_delay_save`d. A deferred write outlives +whatever queued it: it lands after `async_remove_entry` has deleted the file +and recreates it orphaned, and a reload scheduled by the reconcile below +would read the pre-reload snapshot back off disk. It runs once per entry +load, so there's nothing worth deferring. A write that fails is logged and +swallowed — a board reporting something the JSON encoder rejects must not +break polling. + ### 2. Reconcile on reconnect The snapshot is a claim about a device we haven't talked to yet. When the diff --git a/tests/localthings/test_offline_setup.py b/tests/localthings/test_offline_setup.py index 067038f..dc9597f 100644 --- a/tests/localthings/test_offline_setup.py +++ b/tests/localthings/test_offline_setup.py @@ -23,10 +23,7 @@ from homeassistant.util import dt as dt_util from pytest_homeassistant_custom_component.common import async_fire_time_changed from custom_components.localthings.const import DOMAIN, SUMMARY_INTERVAL_S -from custom_components.localthings.coordinator import ( - _SNAPSHOT_SAVE_DELAY_S, - LocalThingsCoordinator, -) +from custom_components.localthings.coordinator import LocalThingsCoordinator from custom_components.localthings.registry.identity import DeviceIdentity from .conftest import _load_fridge_resources as _load_fridge @@ -65,12 +62,6 @@ def _store_key(entry) -> str: return f"{DOMAIN}.{entry.entry_id}.discovery" -async def _flush_snapshot(hass: HomeAssistant) -> None: - """Run out `async_delay_save`'s timer without reaching the next poll.""" - async_fire_time_changed(hass, dt_util.utcnow() + timedelta(seconds=_SNAPSHOT_SAVE_DELAY_S + 1)) - await hass.async_block_till_done() - - async def _tick(hass: HomeAssistant) -> None: """Advance past one summary interval so the coordinator polls again. @@ -89,7 +80,6 @@ async def _setup_online_then_unload(hass: HomeAssistant, entry, resources: dict) await hass.config_entries.async_setup(entry.entry_id) await hass.async_block_till_done() entity_ids = {s.entity_id for s in hass.states.async_all()} - await _flush_snapshot(hass) await hass.config_entries.async_unload(entry.entry_id) await hass.async_block_till_done() return entity_ids @@ -106,7 +96,6 @@ async def test_snapshot_written_after_first_discovery( """A successful first cycle banks what it handed _run_discovery.""" await hass.config_entries.async_setup(mock_entry.entry_id) await hass.async_block_till_done() - await _flush_snapshot(hass) stored = hass_storage[_store_key(mock_entry)]["data"] assert stored["resources"] @@ -122,11 +111,7 @@ async def test_snapshot_not_written_when_device_never_answers( with _unreachable(): await hass.config_entries.async_setup(mock_entry.entry_id) await hass.async_block_till_done() - # Asserted before the flush: running the save timer out also runs out - # HA's own setup-retry timer, which puts the entry back into - # SETUP_IN_PROGRESS. assert mock_entry.state is ConfigEntryState.SETUP_RETRY - await _flush_snapshot(hass) assert _store_key(mock_entry) not in hass_storage @@ -135,14 +120,22 @@ async def test_snapshot_removed_when_entry_removed( hass: HomeAssistant, mock_entry, mock_coordinator_session, hass_storage ) -> None: """The store is keyed on entry_id, so re-adding the appliance mints a new - one -- the old file has to go with the entry that wrote it.""" + one -- the old file has to go with the entry that wrote it. + + The clock is run on afterwards because a deferred write would land here: + with `async_delay_save` the removal was undone a few seconds later by the + save the last poll had queued, leaving the file orphaned for good. + """ await hass.config_entries.async_setup(mock_entry.entry_id) await hass.async_block_till_done() - await _flush_snapshot(hass) assert _store_key(mock_entry) in hass_storage await hass.config_entries.async_remove(mock_entry.entry_id) await hass.async_block_till_done() + assert hass_storage.get(_store_key(mock_entry), {}).get("data") is None + + async_fire_time_changed(hass, dt_util.utcnow() + timedelta(seconds=60)) + await hass.async_block_till_done(wait_background_tasks=True) assert hass_storage.get(_store_key(mock_entry), {}).get("data") is None @@ -200,6 +193,39 @@ async def test_offline_load_without_snapshot_still_fails(hass: HomeAssistant, mo assert not hass.states.async_all() +async def test_malformed_snapshot_falls_back_to_setup_retry( + hass: HomeAssistant, mock_entry, hass_storage +) -> None: + """A stored row missing a field the current dataclass declares must fail + the same way an unreachable device does. + + Anything escaping async_rehydrate reaches async_setup_entry, which only + handles ConfigEntryNotReady -- so the entry would land in SETUP_ERROR, + which HA never retries, with its DTLS session left open on the fixed + source port the next attempt binds. + """ + key = _store_key(mock_entry) + hass_storage[key] = { + "version": 1, + "minor_version": 1, + "key": key, + "data": { + "resources": _load_fridge(), + "subdevice_candidates": [{"key": "1"}], # no "kind" + }, + } + + with ( + _unreachable(), + patch.object(LocalThingsCoordinator, "async_close", autospec=True) as close, + ): + await hass.config_entries.async_setup(mock_entry.entry_id) + await hass.async_block_till_done() + + assert mock_entry.state is ConfigEntryState.SETUP_RETRY + close.assert_awaited_once() + + async def test_corrupt_snapshot_falls_back_to_setup_retry( hass: HomeAssistant, mock_entry, hass_storage ) -> None: @@ -236,7 +262,6 @@ async def test_snapshot_restores_identity(hass: HomeAssistant, mock_entry) -> No with _reachable(resources, identity): await hass.config_entries.async_setup(mock_entry.entry_id) await hass.async_block_till_done() - await _flush_snapshot(hass) await hass.config_entries.async_unload(mock_entry.entry_id) await hass.async_block_till_done() @@ -278,7 +303,7 @@ async def test_offline_load_keeps_polling_and_recovers(hass: HomeAssistant, mock async def test_reconcile_reloads_when_live_discovery_differs( - hass: HomeAssistant, mock_entry + hass: HomeAssistant, mock_entry, hass_storage ) -> None: """Platforms enumerate `bound` once, so a live set that disagrees with the snapshot can only be adopted by bringing the entry back up.""" @@ -289,7 +314,6 @@ async def test_reconcile_reloads_when_live_discovery_differs( await hass.async_block_till_done() coordinator: LocalThingsCoordinator = hass.data[DOMAIN][mock_entry.entry_id] victim = coordinator.bound[0].href - await _flush_snapshot(hass) await hass.config_entries.async_unload(mock_entry.entry_id) await hass.async_block_till_done() @@ -305,6 +329,11 @@ async def test_reconcile_reloads_when_live_discovery_differs( await _tick(hass) reload.assert_called_once_with(mock_entry.entry_id) + # Banked before the reload is scheduled, so the entry that comes back up + # replays this discovery rather than the one it is replacing -- otherwise + # a device that goes quiet again mid-reload rehydrates the stale set and + # reconciles all over again. + assert victim not in hass_storage[_store_key(mock_entry)]["data"]["resources"] async def test_reconcile_is_quiet_when_live_discovery_agrees(