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 3b0803f..ed5417f 100644 --- a/custom_components/localthings/__init__.py +++ b/custom_components/localthings/__init__.py @@ -19,7 +19,7 @@ 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 @@ -260,17 +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: - # `_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 + 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 @@ -327,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..3a92961 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,20 @@ _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 + + +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 +264,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 +426,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,7 +1049,148 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): self._entry, data={**self._entry.data, **identity} ) - def _run_discovery(self, resources: dict[str, dict]) -> None: + # ------------------------------------------------------------------ + # 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) + + 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. + + `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. + + 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 + 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. + + 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 + + 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 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 + # 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], 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. @@ -1115,7 +1306,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) @@ -1422,7 +1619,13 @@ 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) + # 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 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 new file mode 100644 index 0000000..7ff350f --- /dev/null +++ b/docs/investigations/offline-setup.md @@ -0,0 +1,187 @@ +# Loading a config entry while the appliance is offline + +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 +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. + +## How the implemented version works + +Three pieces, plus a gating rule. + +### 1. A persisted discovery snapshot + +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/*`. + +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. + +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. + +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 +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. + +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 + +`async_setup_entry` holds one listener for the entry's lifetime: + +```python +entry.async_on_unload(coordinator.async_add_listener(lambda: None)) +``` + +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 + +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. + +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. +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. + +## Rejected: zeroconf + +The issue's other suggestion — wire zeroconf so the device's own boot +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. + +## Rejected: the cheap version + +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 6dfd2d0..2429e3c 100644 --- a/tests/localthings/test_coordinator.py +++ b/tests/localthings/test_coordinator.py @@ -71,7 +71,12 @@ async def test_summary_interval(hass: HomeAssistant, mock_entry, mock_coordinato async def test_update_failed_on_persistent_poll_error(hass: HomeAssistant, mock_entry) -> None: - """ConfigEntryNotReady raised when poll fails even after reconnect.""" + """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"), @@ -1824,10 +1829,11 @@ async def test_first_refresh_timeout_recovers_via_reconnect( async def test_first_refresh_persistent_timeout_fails_setup( 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).""" + """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( diff --git a/tests/localthings/test_offline_setup.py b/tests/localthings/test_offline_setup.py new file mode 100644 index 0000000..dc9597f --- /dev/null +++ b/tests/localthings/test_offline_setup.py @@ -0,0 +1,418 @@ +"""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.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 + +from custom_components.localthings.const import DOMAIN, SUMMARY_INTERVAL_S +from custom_components.localthings.coordinator import 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 _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 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() + + 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() + assert mock_entry.state is ConfigEntryState.SETUP_RETRY + + 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. + + 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() + 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 + + +# --------------------------------------------------------------------------- +# 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_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: + """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 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, 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.""" + 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 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) + # 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( + 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() + + +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: + """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):