Merge pull request #374 from mbillow/claude/pr-303-code-review-6mghcs

Load a config entry offline from the last discovery snapshot
This commit is contained in:
Marc Billow
2026-08-15 15:54:25 -05:00
committed by GitHub
10 changed files with 894 additions and 20 deletions
+6
View File
@@ -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.
+33 -10
View File
@@ -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:
+206 -3
View File
@@ -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:
+6 -2
View File
@@ -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
+187
View File
@@ -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.
+11 -5
View File
@@ -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(
+418
View File
@@ -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
@@ -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")
+9
View File
@@ -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))
+9
View File
@@ -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):