From 687eb8933569d1f8e93db6036a6472cf155df353 Mon Sep 17 00:00:00 2001 From: sdebasek Date: Fri, 21 Aug 2026 13:00:51 +0200 Subject: [PATCH] fix: Correct seven defects found by review Device triggers stopped firing once the official BleBox integration was also configured. The callback receiver resolved the device row by identifier, and that identifier is deliberately the official integration's, so Home Assistant's identifier-domain narrowing handed back the official row while device triggers are only ever offered on ours. The automation editor stored one row id and the bus event carried the other, so every device automation built in the UI silently never ran. Event entities kept working, which is what hid it. The row is now resolved scoped to this config entry, and a cached id is trusted only while it still names a row this entry owns. A failed settings or network read published an empty object as live state. Those reads are best effort, but the empty result went into the snapshot as though the device had answered with it, while the entry stayed available, so the cloud tunnel, backlight and access point all read as off and the overload and restart controls went unknown for up to a minute. Home Assistant recorded those as real state changes. A failed read now carries the previous value forward, and a device that genuinely answers with an empty object is still believed. A device that had never answered left the coordinator with no listeners, and the interval timer is only armed once something listens, so the entry went permanently inert: no polling, no recovery when the device came back, no callback healing. One listener is now registered in that case only, and it reloads the entry as soon as a snapshot arrives. Free and owned action slots were selected on different fields, trigger type versus action type, so a slot the firmware had half cleared counted in both pools. Capacity was overstated and a run could clear the callback it had just written. The pools are now a partition, a plan that double books is refused before the first write, and removal still erases our URL from a slot whose trigger the firmware already zeroed. Firmware, hardware and model on the device page were frozen at setup, so an update never showed. They now follow what the device reports, and a firmware change is remembered for the next offline start rather than seeding the version from before the update. Enabling events for an input that had none reloaded the entry twice, because re-enabling its entity wrote to the registry. The disable is now handed over and cleared in one step that Home Assistant does not treat as a reason to reload. --- custom_components/blebox_advanced/__init__.py | 7 + custom_components/blebox_advanced/api.py | 56 +++- .../blebox_advanced/blebox_actions.py | 50 ++- .../blebox_advanced/coordinator.py | 118 ++++++- custom_components/blebox_advanced/entity.py | 88 ++++- custom_components/blebox_advanced/event.py | 30 +- tests/test_advanced.py | 181 ++++++++++ tests/test_blebox_actions.py | 131 ++++++++ tests/test_integration.py | 315 +++++++++++++++++- tests/test_settings_entities.py | 70 ++++ 10 files changed, 1012 insertions(+), 34 deletions(-) diff --git a/custom_components/blebox_advanced/__init__.py b/custom_components/blebox_advanced/__init__.py index 6c7f6d4..0693801 100644 --- a/custom_components/blebox_advanced/__init__.py +++ b/custom_components/blebox_advanced/__init__.py @@ -99,6 +99,13 @@ async def async_setup_entry( hass, entry, state=snapshot.actions if snapshot else None ) + if snapshot is None: + # Nothing has ever been observed on this device - it is not answering + # now and it has never been remembered - so every polled platform below + # creates nothing, and with no entity listening the coordinator would + # stop polling and never notice the device coming back. + entry.async_on_unload(coordinator.async_keep_polling_without_entities()) + await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS) entry.async_on_unload(entry.add_update_listener(_async_options_updated)) return True diff --git a/custom_components/blebox_advanced/api.py b/custom_components/blebox_advanced/api.py index f098a61..119426a 100644 --- a/custom_components/blebox_advanced/api.py +++ b/custom_components/blebox_advanced/api.py @@ -321,7 +321,7 @@ def _handle( ATTR_BLEBOX_ID: data.blebox_id, **hints, } - device_id = _async_ha_device_id(hass, data) + device_id = _async_ha_device_id(hass, entry_id, data) if device_id is not None: event_data[CONF_DEVICE_ID] = device_id hass.bus.async_fire(HA_EVENT, event_data) @@ -363,20 +363,56 @@ def _parse_state_hints(query: Mapping[str, str]) -> dict[str, Any]: @callback -def _async_ha_device_id(hass: HomeAssistant, data: BleBoxEventsData) -> str | None: +def _async_ha_device_id( + hass: HomeAssistant, entry_id: str, data: BleBoxEventsData +) -> str | None: """Resolve (and cache) the device registry id backing this entry. Looked up lazily because the device entry may not exist yet the first time a callback arrives, and its id never changes once it does. + + Scoped to this config entry rather than looked up by identifier alone, and + that scoping is the whole point. We claim ``(blebox, )`` - the + *official* integration's domain, deliberately, so our entities land on the + same device as its relay and power entities. That makes a bare + ``async_get_device(identifiers=...)`` ambiguous the moment the official + integration is configured for the same device, which README documents as a + supported setup and which is the normal state mid-migration: the registry + narrows several matches by identifier-domain priority and keeps only rows + whose config entry domain equals the looked-up identifier's domain, so the + official row wins outright and ours is filtered out before the + "prefer the calling integration's own row" fallback is ever reached. + Device triggers are only ever offered on *our* row (see + :mod:`.device_trigger`, which skips config entries of any other domain), so + publishing the official row's id here meant the automation editor stored one + id and the bus carried another, and every device trigger built in the UI + silently never fired. The event entities kept working throughout because + they run off the dispatcher rather than the bus, which is what made it + quiet. + + ``async_entries_for_config_entry`` rather than the newer + ``async_get_device_by_identifier``: identifiers are unique within a config + entry so the two mean the same thing here, but the module-level helper has + been in Home Assistant for years, and the receiver is the one module that + should not acquire a version floor of its own. """ - if data.ha_device_id is not None: - return data.ha_device_id - device = dr.async_get(hass).async_get_device( - identifiers={(BLEBOX_DOMAIN, data.blebox_id)} - ) - if device is not None: - data.ha_device_id = device.id - return data.ha_device_id + registry = dr.async_get(hass) + if (cached := data.ha_device_id) is not None: + # Only trust the cache while it still names a row this entry owns. A + # user can delete the device from the UI, and an id cached by a build + # from before the scoping fix names the official integration's row - + # neither may be allowed to outlive its truth for the rest of a session. + device = registry.async_get(cached) + if device is not None and entry_id in device.config_entries: + return cached + data.ha_device_id = None + + identifier = (BLEBOX_DOMAIN, data.blebox_id) + for device in dr.async_entries_for_config_entry(registry, entry_id): + if identifier in device.identifiers: + data.ha_device_id = device.id + return device.id + return None @callback diff --git a/custom_components/blebox_advanced/blebox_actions.py b/custom_components/blebox_advanced/blebox_actions.py index 30905a2..a1fe661 100644 --- a/custom_components/blebox_advanced/blebox_actions.py +++ b/custom_components/blebox_advanced/blebox_actions.py @@ -368,12 +368,32 @@ def supports_http_action(self, trigger_type: int) -> bool: return ACTION_HTTP_GET in allowed if allowed else not self.field_preferences def free_slots(self) -> list[dict[str, Any]]: - """Unconfigured slots, in slot order.""" + """Unconfigured slots, in slot order. + + These three accessors partition the slot array: a slot is free, or ours, + or somebody else's, and never two of those at once. Provisioning adds + the pools up to decide whether a run fits, so an overlap between any two + of them would overcount the device's real capacity. + """ return [a for a in self.actions if not is_configured(a)] def owned_actions(self) -> list[dict[str, Any]]: - """Slots created by this integration, in slot order.""" - return [a for a in self.actions if is_owned(a)] + """Slots holding a live callback of this integration, in slot order. + + ``is_configured`` is required as well as the ownership marker, and it is + not redundant with it: trigger type and action type are separate fields, + so a slot can carry our URL while its trigger reads as unconfigured. + Firmware that honours the ``triggerType: 0`` half of a clear and keeps + the ``actionType``/``param`` half leaves exactly that behind. + + Such a slot used to appear in this list *and* in :meth:`free_slots`, + which let one physical slot be counted twice towards capacity and be + handed out twice in one run: the second callback silently destroyed the + first, and a slot taken from the free list while still sitting in the + reclaimable list was wiped by the clearing pass that followed. A slot + with no trigger never fires, so it is simply free. + """ + return [a for a in self.actions if is_configured(a) and is_owned(a)] def foreign_actions(self) -> list[dict[str, Any]]: """Return configured slots owned by someone else; these are never touched.""" @@ -942,6 +962,19 @@ async def _async_sync_locked(self, desired: list[DesiredAction]) -> SyncResult: writes.append(build_action_payload(stale, template, **_clear_overrides())) result.cleared.append(_slot_id(stale)) + # The whole plan is checked before the first request leaves, because a + # run that wrote two things into one slot would break the promise this + # method exists to keep in the quietest possible way: the second write + # destroys the first, and `SyncResult` still reports both. The pools are + # disjoint by construction, so this can only fire if that ever stops + # being true - in which case refusing the run leaves the device exactly + # as it was, which is the outcome design rule 2 asks for. + planned = [_slot_id(payload) for payload in writes] + if len(set(planned)) != len(planned): + raise BleBoxActionApiError( + f"Refusing to write the same action slot twice in one run: {planned}" + ) + for payload in writes: await self.async_save_action(payload) @@ -956,10 +989,19 @@ async def async_remove_owned_actions(self) -> list[int]: async with self._action_lock: state = await self.async_get_actions_state() template = _field_template(state.actions) + # Every slot carrying our marker, rather than `owned_actions()`: + # removal has to erase our callback URL even from a slot whose + # trigger the firmware has already zeroed, or the entry goes away + # and the callback token stays readable in the wBox app. + # # Slot ids are resolved before the first write, so a device that # describes a slot without one fails before anything is touched # rather than half way through the clearing. - targets = [(_slot_id(action), action) for action in state.owned_actions()] + targets = [ + (_slot_id(action), action) + for action in state.actions + if is_owned(action) + ] for _, action in targets: await self.async_save_action( build_action_payload(action, template, **_clear_overrides()) diff --git a/custom_components/blebox_advanced/coordinator.py b/custom_components/blebox_advanced/coordinator.py index 6d11fbc..02f449c 100644 --- a/custom_components/blebox_advanced/coordinator.py +++ b/custom_components/blebox_advanced/coordinator.py @@ -23,7 +23,7 @@ from typing import Any from homeassistant.config_entries import ConfigEntry -from homeassistant.core import HomeAssistant, callback +from homeassistant.core import CALLBACK_TYPE, HomeAssistant, callback from homeassistant.helpers import issue_registry as ir from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed @@ -191,6 +191,19 @@ def _measured_values(state: dict[str, Any]) -> tuple[str, ...]: return tuple(sorted(types)) +def _identity(info: DeviceInfo | None) -> tuple[str, ...] | None: + """Return the identity fields a device page shows, or None if it never said.""" + if info is None: + return None + return ( + info.device_type, + info.product, + info.firmware_version, + info.hardware_version, + info.api_level, + ) + + def _timed_relays(state: dict[str, Any]) -> tuple[int, ...]: """Return the relays reporting a countdown, one sensor each.""" relays = state.get("relays") @@ -228,7 +241,13 @@ def capability_signature(snapshot: DeviceSnapshot) -> tuple[Any, ...]: _measured_values(state), _timed_relays(state), snapshot.uptime_s is not None, - snapshot.info is not None, + # The identity is compared by value, not merely by presence. It decides + # no entity, so it is not a capability, but it is what a device page + # shows; remembering only "an identity existed" left an offline start + # seeding the firmware version from before the last update, which is + # exactly the field someone checks to confirm an update worked. These + # move about as often as the firmware does, so they cost no writes. + _identity(snapshot.info), ) @@ -492,6 +511,59 @@ def async_request_full_refresh(self) -> None: """Ask for settings and actions on the next refresh, not just state.""" self._force_full = True + @callback + def async_keep_polling_without_entities(self) -> CALLBACK_TYPE: + """Poll on with nothing listening, and reload once the device answers. + + A device that has never answered has nothing remembered, so every polled + platform creates nothing and no ``CoordinatorEntity`` is ever added. + ``DataUpdateCoordinator`` arms its interval only while something is + listening, so without this the entry stays inert for good: it never + polls, so automatic callbacks are never healed and the entities never + arrive however long the device has been back, until somebody reloads the + entry by hand. Setup deliberately succeeds while the device is down, so + Home Assistant will not retry it either. + + Registering a listener is deliberately the whole mechanism. A timer of + our own would poll a device that *does* have entities a second time on + every interval; a listener shares the single interval the coordinator + already keeps, and it is only ever registered when there are no entities + to keep it. + + Polling alone would still not produce the entities, because platform + setup has already run and nothing runs it again - so the first answer + reloads the entry. Setup refreshes before it forwards the platforms, so + by then they see a live snapshot and create everything the device turned + out to have. Stopping first makes this a one-shot: the reloaded entry + either has entities of its own or lands right back here. + + Returns the callback that stops it, for the caller to hand to the config + entry so that an unload takes it down with everything else. + """ + remove: CALLBACK_TYPE | None = None + + @callback + def _async_stop() -> None: + """Stop watching, at most once: removing a listener twice raises.""" + nonlocal remove + if remove is None: + return + unsubscribe, remove = remove, None + unsubscribe() + + @callback + def _async_answered() -> None: + """Reload the entry as soon as there is something to build from.""" + # Listeners are told about a *failed* refresh too, which is what + # keeps entities unavailable, so the snapshot is what to check. + if self.data is None: + return + _async_stop() + self.hass.config_entries.async_schedule_reload(self.config_entry.entry_id) + + remove = self.async_add_listener(_async_answered) + return _async_stop + @property def settings(self) -> dict[str, Any]: """The device's settings, preferring a value just written from here. @@ -575,20 +647,31 @@ async def _async_update_data(self) -> DeviceSnapshot: # Best-effort: a device that answers its identity but not these still # delivers events, so a failure here must not fail the whole update. - settings: dict[str, Any] = {} - settings_read = True + # + # What a failure must not do either is publish an empty payload as live + # state. The update still succeeds, so every entity stays *available* + # and simply reports the wrong thing: the cloud tunnel, the backlight + # and the access point all read as off, the overload threshold and the + # restart select go unknown, and the access point blanks its SSID. Home + # Assistant records each of those as a genuine state change, and the + # relay-only polls in between carry it forward until the next slow + # cycle, so an automation watching the cloud tunnel fires on nothing at + # all. The payload the device last gave is still the best description of + # it, so that is what a failed read carries forward - only ever on + # failure, so a device that really did answer with an empty object is + # still believed. + settings: dict[str, Any] try: settings = await self.manager.async_get_settings() except BleBoxError as err: - settings_read = False + settings = previous.settings _LOGGER.debug("Settings unavailable on %s: %s", info.name, err) - network: dict[str, Any] = {} - network_read = True + network: dict[str, Any] try: network = await self.manager.async_get_network() except BleBoxError as err: - network_read = False + network = previous.network _LOGGER.debug("Network state unavailable on %s: %s", info.name, err) uptime = await self.manager.async_get_uptime() @@ -612,13 +695,18 @@ async def _async_update_data(self) -> DeviceSnapshot: health=health, ) - # A fetch that failed is not evidence that a capability went away: the - # best-effort reads above report an empty object either way, and - # remembering that would leave the next offline start without those - # entities. Uptime is best-effort inside the API layer itself, so a - # device that has reported one before has to keep reporting it to count. - uptime_read = uptime is not None or previous.uptime_s is None - if settings_read and network_read and uptime_read: + # A fetch that failed is not evidence that a capability went away, but + # it no longer has to block this either: the best-effort reads above + # carry the last payload forward, so the snapshot describes the same + # capabilities the last successful read did. Demanding that all of them + # succeed instead meant firmware without ``/api/device/network`` never + # had its shape remembered at all, and a device that has never been + # remembered comes up with no entities at all when it is unreachable at + # startup. Uptime is the one read still worth guarding: it is + # best-effort inside the API layer itself and answers ``None`` either + # way, so a device that has reported one before has to keep reporting it + # to count as having answered. + if uptime is not None or previous.uptime_s is None: self._async_remember_capabilities(snapshot) return snapshot diff --git a/custom_components/blebox_advanced/entity.py b/custom_components/blebox_advanced/entity.py index 96f094c..a454b30 100644 --- a/custom_components/blebox_advanced/entity.py +++ b/custom_components/blebox_advanced/entity.py @@ -12,9 +12,11 @@ from contextlib import AbstractContextManager, contextmanager from typing import Any +from homeassistant.core import callback from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers import device_registry as dr from homeassistant.helpers.device_registry import DeviceInfo +from homeassistant.helpers.typing import UNDEFINED from homeassistant.helpers.update_coordinator import CoordinatorEntity from .blebox_actions import BleBoxError, InsufficientSlotsError @@ -97,6 +99,42 @@ def deep_merge(base: dict[str, Any], patch: dict[str, Any]) -> dict[str, Any]: return merged +def device_identity( + entry: BleBoxEventsConfigEntry, data: BleBoxEventsData +) -> dict[str, str | None]: + """Return the model, firmware and hardware version the device reports now. + + Keyed to match both :class:`DeviceInfo` and + ``device_registry.async_update_device``, because the device page has to be + told the same three values in two different ways: once when an entity is + added, and again whenever a poll finds them changed. + + The coordinator re-reads identity on every slow cycle, so its snapshot is + the live answer and wins. ``entry.data`` is only what the config flow wrote + at setup and nothing ever updates it, which is why it is the fallback rather + than the source: it is all a device that has never answered has, and the + offline-start path depends on those values still producing valid device + info. + + ``device_type`` and not ``product`` on purpose. The config flow stored the + device type as the model, so reading the marketing name here instead would + silently rename the model on every existing device page. + """ + snapshot = data.coordinator.data + info = snapshot.info if snapshot else None + return { + "model": (info.device_type if info else "") + or entry.data.get(CONF_MODEL) + or None, + "sw_version": (info.firmware_version if info else "") + or entry.data.get(CONF_SW_VERSION) + or None, + "hw_version": (info.hardware_version if info else "") + or entry.data.get(CONF_HW_VERSION) + or None, + } + + def build_device_info( entry: BleBoxEventsConfigEntry, data: BleBoxEventsData ) -> DeviceInfo: @@ -110,13 +148,14 @@ def build_device_info( is advertised too, so the link survives if the official integration ever changes how it builds identifiers. """ + identity = device_identity(entry, data) device_info = DeviceInfo( identifiers={(BLEBOX_DOMAIN, data.blebox_id)}, manufacturer=MANUFACTURER, name=entry.title, - model=entry.data.get(CONF_MODEL) or None, - sw_version=entry.data.get(CONF_SW_VERSION) or None, - hw_version=entry.data.get(CONF_HW_VERSION) or None, + model=identity["model"], + sw_version=identity["sw_version"], + hw_version=identity["hw_version"], configuration_url=data.manager.base_url, ) if (mac := mac_connection(data.blebox_id)) is not None: @@ -152,6 +191,49 @@ def __init__( if placeholders: self._attr_translation_placeholders = placeholders self._attr_device_info = build_device_info(entry, data) + # What the device page was last told, so an unchanged identity costs a + # dict comparison rather than a registry lookup on every poll. + self._identity = device_identity(entry, data) + + @callback + def _handle_coordinator_update(self) -> None: + """Write the new state, taking any identity change to the device page.""" + self._async_apply_device_identity() + super()._handle_coordinator_update() + + @callback + def _async_apply_device_identity(self) -> None: + """Push a changed firmware, hardware or model version into the registry. + + Home Assistant reads ``device_info`` once, when the entity is added, so + without this the device page keeps showing whatever the device reported + at setup for as long as the entry stays loaded - and firmware version is + exactly the field a user checks to confirm an update worked, on an + integration that can start that update itself. Sourcing the values live + is not enough on its own: nothing re-reads them until a reload. + + The registry row is addressed by id rather than looked up by identifier. + Our identifier is the *official* integration's (see + :func:`build_device_info`), and looking one up by identifier finds that + integration's row instead of ours when both are configured for the same + device. + """ + identity = device_identity(self._entry, self._data) + # Nothing new to say, or no device page to say it to. The identity is + # recorded as applied only once it really has been, so an entity that + # somehow has no device row yet tries again on the next poll. + if identity == self._identity or self.device_entry is None: + return + self._identity = identity + # UNDEFINED rather than None for anything unreported, so firmware that + # stops sending ``hv`` leaves the hardware version the page already + # shows alone instead of blanking it. + dr.async_get(self.hass).async_update_device( + self.device_entry.id, + model=identity["model"] or UNDEFINED, + sw_version=identity["sw_version"] or UNDEFINED, + hw_version=identity["hw_version"] or UNDEFINED, + ) @property def settings(self) -> dict[str, Any]: diff --git a/custom_components/blebox_advanced/event.py b/custom_components/blebox_advanced/event.py index af067f7..0a8fbaa 100644 --- a/custom_components/blebox_advanced/event.py +++ b/custom_components/blebox_advanced/event.py @@ -72,7 +72,35 @@ def _async_enable_selected_inputs(hass: HomeAssistant, data: BleBoxEventsData) - registry_entry is not None and registry_entry.disabled_by is er.RegistryEntryDisabler.INTEGRATION ): - registry.async_update_entity(entity_id, disabled_by=None) + _async_enable_quietly(registry, entity_id) + + +@callback +def _async_enable_quietly(registry: er.EntityRegistry, entity_id: str) -> None: + """Clear our own disable without making Home Assistant reload the entry. + + Enabling a registry entry normally schedules a reload of its config entry + thirty seconds later, which is how an entity switched on in the UI comes + into existence. Here that reload is pure waste: this runs inside platform + setup and the entity is added, enabled, a few lines further down, so all the + reload does is tear down every entity of the entry and re-run provisioning + to arrive at what it already had. Ticking events for a new input in the + options therefore reloaded the entry twice, the second time for no visible + reason at all. + + Home Assistant skips that scheduling for exactly one transition: an entity + coming back from ``CONFIG_ENTRY``, because enabling a config entry reloads + it anyway. Handing our own disable over to ``CONFIG_ENTRY`` first therefore + reaches the same end state quietly - the hand-over is ignored too, since the + entity is still disabled at that point, and only an entity that ends up + *enabled* is worth a reload. Both writes are synchronous with nothing + awaited between them, so no other code can observe the entry in between and + the registry only ever persists the final value. + """ + registry.async_update_entity( + entity_id, disabled_by=er.RegistryEntryDisabler.CONFIG_ENTRY + ) + registry.async_update_entity(entity_id, disabled_by=None) async def async_setup_entry( diff --git a/tests/test_advanced.py b/tests/test_advanced.py index 11f1a3d..e3e0732 100644 --- a/tests/test_advanced.py +++ b/tests/test_advanced.py @@ -22,6 +22,7 @@ from homeassistant.helpers import issue_registry as ir from homeassistant.helpers.dispatcher import async_dispatcher_send from homeassistant.helpers.network import NoURLAvailableError +from homeassistant.setup import async_setup_component from pytest_homeassistant_custom_component.common import MockConfigEntry from custom_components.blebox_advanced import device_trigger @@ -1717,6 +1718,186 @@ async def test_triggers_are_only_listed_for_devices_that_are_ours( assert await device_trigger.async_get_triggers(hass, "no-such-device") == [] +# --- the device row a press names ------------------------------------------- +# +# The event has to name the same registry row the triggers were offered on, or +# every device automation built in the editor stores one id while the bus +# carries another and nothing ever fires. The event entities keep working +# either way - they run off the dispatcher, not the bus - so only a test that +# looks at the id itself can see this. + + +def _official_row(hass: HomeAssistant) -> dr.DeviceEntry: + """Register the official BleBox integration's row for the same device. + + Both integrations claim ``(blebox, )`` on purpose, which is what + puts our entities on the official device's page. Each config entry still + gets its own registry row, so from here on a lookup by that identifier + alone is ambiguous - and Home Assistant resolves it in the official + integration's favour, its domain being the one in the identifier. README + documents running both side by side, and it is the normal state during a + migration. + """ + official_entry = MockConfigEntry(domain=BLEBOX_DOMAIN, unique_id=BLEBOX_ID) + official_entry.add_to_hass(hass) + return dr.async_get(hass).async_get_or_create( + config_entry_id=official_entry.entry_id, + identifiers={(BLEBOX_DOMAIN, BLEBOX_ID)}, + connections={(dr.CONNECTION_NETWORK_MAC, "ae:0b:fb:f9:27:ba")}, + manufacturer="BleBox", + name="Simon GO Switch", + model="switchBox", + ) + + +def _our_row(hass: HomeAssistant) -> dr.DeviceEntry: + """Return the registry row this integration's own entities live on. + + Read back through one of our entities rather than by identifier, so the + test does not resolve the row the same ambiguous way the bug did. + """ + entity = er.async_get(hass).async_get(_eid(hass, "event", "input_0")) + row = dr.async_get(hass).async_get(entity.device_id) + assert row is not None + return row + + +async def test_a_press_names_the_row_its_triggers_were_offered_on( + hass: HomeAssistant, hass_client_no_auth +) -> None: + """With the official integration also configured, the event names our row. + + Triggers are only ever offered on the row carrying one of *our* config + entries, so this is the id the automation editor stores. Firing the event + against the official integration's row instead breaks every one of them at + once, silently. + """ + official = _official_row(hass) + await _setup(hass, _entry()) + ours = _our_row(hass) + + assert ours.id != official.id + assert len(await device_trigger.async_get_triggers(hass, ours.id)) == 8 + assert await device_trigger.async_get_triggers(hass, official.id) == [] + + fired: list[Any] = [] + hass.bus.async_listen(HA_EVENT, lambda event: fired.append(event)) + + client = await hass_client_no_auth() + assert (await client.get(f"/api/{DOMAIN}/{TOKEN}/0/short_press")).status == 200 + await hass.async_block_till_done() + + assert len(fired) == 1 + assert fired[0].data["device_id"] == ours.id + + +async def test_a_device_trigger_still_fires_beside_the_official_integration( + hass: HomeAssistant, hass_client_no_auth +) -> None: + """The whole chain, with both integrations set up for the one device. + + Editor-shaped automation on the row the editor would have offered, then a + real callback over HTTP. This is the acceptance criterion the id-level + assertions above only stand in for. + """ + _official_row(hass) + await _setup(hass, _entry()) + ours = _our_row(hass) + + assert await async_setup_component( + hass, + "automation", + { + "automation": { + "triggers": { + "trigger": "device", + "domain": DOMAIN, + "device_id": ours.id, + "type": "long_press", + "subtype": "1", + }, + "actions": {"event": "blebox_advanced_test_fired"}, + } + }, + ) + await hass.async_block_till_done() + + fired: list[Any] = [] + hass.bus.async_listen( + "blebox_advanced_test_fired", lambda event: fired.append(event) + ) + + client = await hass_client_no_auth() + assert (await client.get(f"/api/{DOMAIN}/{TOKEN}/0/long_press")).status == 200 + await hass.async_block_till_done() + + assert len(fired) == 1 + + +async def test_a_press_names_our_row_when_we_are_the_only_integration( + hass: HomeAssistant, hass_client_no_auth +) -> None: + """Scoping the lookup to our entry leaves the ordinary setup untouched. + + The row is picked by the BleBox identifier it carries, not by being the + only row the entry happens to own, so a second row registered against the + same entry cannot be mistaken for the device. + """ + entry = _entry() + entry.add_to_hass(hass) + decoy = dr.async_get(hass).async_get_or_create( + config_entry_id=entry.entry_id, + identifiers={(DOMAIN, f"{BLEBOX_ID}_not_the_device")}, + name="Something else this entry owns", + ) + await _setup(hass, entry) + ours = _our_row(hass) + + assert ours.id != decoy.id + assert len(await device_trigger.async_get_triggers(hass, ours.id)) == 8 + + fired: list[Any] = [] + hass.bus.async_listen(HA_EVENT, lambda event: fired.append(event)) + + client = await hass_client_no_auth() + assert (await client.get(f"/api/{DOMAIN}/{TOKEN}/0/short_press")).status == 200 + await hass.async_block_till_done() + + assert len(fired) == 1 + assert fired[0].data["device_id"] == ours.id + + +async def test_a_cached_row_id_that_is_not_ours_is_discarded( + hass: HomeAssistant, hass_client_no_auth +) -> None: + """The cached id corrects itself rather than outliving its truth. + + The id is cached because the row may not exist the first time a callback + arrives and never changes once it does. Two things can still falsify it: a + value written by a build from before the lookup was scoped, which names the + official integration's row, and a row deleted from the device page. Neither + may be allowed to stand for the rest of the session, so a cached id is + trusted only while it still names a row this entry owns. + """ + official = _official_row(hass) + entry = _entry() + await _setup(hass, entry) + ours = _our_row(hass) + + entry.runtime_data.ha_device_id = official.id + + fired: list[Any] = [] + hass.bus.async_listen(HA_EVENT, lambda event: fired.append(event)) + + client = await hass_client_no_auth() + assert (await client.get(f"/api/{DOMAIN}/{TOKEN}/0/short_press")).status == 200 + await hass.async_block_till_done() + + assert len(fired) == 1 + assert fired[0].data["device_id"] == ours.id + assert entry.runtime_data.ha_device_id == ours.id + + # --- more of the callback endpoint ------------------------------------------- diff --git a/tests/test_blebox_actions.py b/tests/test_blebox_actions.py index 428baa7..449813c 100644 --- a/tests/test_blebox_actions.py +++ b/tests/test_blebox_actions.py @@ -120,6 +120,20 @@ def owned_slot( ) +def half_cleared_owned_slot(slot_id: int, input_id: int) -> dict[str, Any]: + """Build a slot of ours that the device reports as having no trigger. + + Trigger type and action type are separate fields, so firmware that honours + the ``triggerType: 0`` half of a clear and keeps the ``actionType``/``param`` + half leaves this behind: a slot the device considers empty which still + carries our callback URL. It is the one shape that used to satisfy both + "free" and "ours". + """ + slot = owned_slot(slot_id, input_id, TRIGGER_SHORT_CLICK, "short_press") + slot["triggerType"] = TRIGGER_UNCONFIGURED + return slot + + def make_state(actions: list[dict[str, Any]], total: int = 8) -> ActionsState: """Build an ActionsState padded out to `total` fixed slots.""" slots = list(actions) @@ -768,6 +782,123 @@ async def test_a_recycled_slot_is_repurposed_in_one_write() -> None: assert result.slots_free == 0 +# --- Slots that read as both free and ours ---------------------------------- + + +async def test_a_half_cleared_slot_is_spent_once_not_filled_and_wiped() -> None: + """A slot that reads as both empty and ours is used once and left alone. + + Regression: `free_slots()` selected on trigger type while `owned_actions()` + selected on action type plus our URL marker, and neither excluded the other. + A half-cleared slot of ours therefore sat in both lists, so the run took it + from the free list, wrote the callback into it, and then cleared the very + same slot on the way out because it was still queued as stale. The device + was left with an empty slot while `SyncResult` reported a creation, and the + user's button silently never fired. + """ + manager = RecordingManager(make_state([half_cleared_owned_slot(0, 0)], total=1)) + + url = owned_url(0, "long_press") + result = await manager.async_sync_http_actions( + [DesiredAction(0, TRIGGER_LONG_CLICK, url, "HA IN1 long_press")] + ) + + assert [write["id"] for write in manager.writes] == [0] + assert manager.writes[0]["param"] == url + assert manager.writes[0]["triggerType"] == TRIGGER_LONG_CLICK + assert result.created == [0] + assert result.cleared == [] + + +async def test_a_half_cleared_slot_is_counted_once_towards_capacity() -> None: + """One physical slot counts once, and a run that needs two still refuses. + + Regression: counted as free *and* as reclaimable, a single half-cleared slot + of ours made a two-callback run look like it fitted on a device with one + usable slot. Both callbacks were then planned into that slot and the second + write destroyed the first, which is exactly the "either fits entirely or + changes nothing" promise the capacity check exists to keep. + """ + theirs = configured_slot(1, 1, TRIGGER_SHORT_CLICK, 1, "", "user action") + manager = RecordingManager( + make_state([half_cleared_owned_slot(0, 0), theirs], total=2) + ) + + desired = [ + DesiredAction( + 0, TRIGGER_LONG_CLICK, owned_url(0, "long_press"), "HA IN1 long_press" + ), + DesiredAction( + 1, TRIGGER_LONG_CLICK, owned_url(1, "long_press"), "HA IN2 long_press" + ), + ] + with pytest.raises(InsufficientSlotsError) as err: + await manager.async_sync_http_actions(desired) + + assert manager.writes == [] + assert err.value.needed == 2 + # The half-cleared slot is usable, once. The user's slot never is. + assert err.value.available == 1 + assert err.value.total == 2 + + +async def test_a_plan_that_double_books_a_slot_writes_nothing() -> None: + """Two writes planned into one slot abort the run before the first request. + + The slot pools are disjoint by construction, so this drives the check with a + state whose pools deliberately overlap the way they used to. What it pins is + the failure mode: a run that would write a slot twice must leave the device + exactly as it was rather than create a callback and wipe it moments later. + """ + + class OverlappingState(ActionsState): + """Slot pools that overlap, as they did before they were made disjoint.""" + + def free_slots(self) -> list[dict[str, Any]]: + """Offer every slot as free, including the ones we already own.""" + return list(self.actions) + + base = make_state([owned_slot(0, 0, TRIGGER_SHORT_CLICK, "short_press")], total=1) + manager = RecordingManager( + OverlappingState(base.actions, base.items_limit, base.field_preferences) + ) + + with pytest.raises(BleBoxActionApiError) as err: + await manager.async_sync_http_actions( + [ + DesiredAction( + 0, + TRIGGER_LONG_CLICK, + owned_url(0, "long_press"), + "HA IN1 long_press", + ) + ] + ) + + assert manager.writes == [] + assert "twice" in str(err.value) + + +async def test_removal_erases_our_url_from_a_half_cleared_slot() -> None: + """Deleting the integration takes our URL out of a dormant slot as well. + + A half-cleared slot no longer counts as one of ours for provisioning, since + a slot with no trigger never fires and is simply free. Removal is the one + place that still has to visit it: leaving it be would keep the callback + token readable in the wBox app after the entry it belonged to was gone. + """ + theirs = configured_slot(1, 1, TRIGGER_LONG_CLICK, 1, "", "user action") + manager = RecordingManager( + make_state([half_cleared_owned_slot(0, 0), theirs], total=2) + ) + + cleared = await manager.async_remove_owned_actions() + + assert cleared == [0] + assert [write["id"] for write in manager.writes] == [0] + assert manager.writes[0]["param"] == "" + + # --- Malformed device payloads ---------------------------------------------- diff --git a/tests/test_integration.py b/tests/test_integration.py index e44a3e4..3d1e51a 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -8,19 +8,24 @@ from __future__ import annotations +import dataclasses from contextlib import ExitStack +from datetime import timedelta from typing import Any from unittest.mock import patch import pytest from homeassistant.components.device_automation import DeviceAutomationType -from homeassistant.const import CONF_HOST, CONF_PORT, STATE_UNAVAILABLE +from homeassistant.config_entries import RELOAD_AFTER_UPDATE_DELAY +from homeassistant.const import CONF_HOST, CONF_PORT, STATE_ON, STATE_UNAVAILABLE from homeassistant.core import HomeAssistant from homeassistant.helpers import device_registry as dr from homeassistant.helpers import entity_registry as er from homeassistant.helpers import issue_registry as ir +from homeassistant.util import dt as dt_util from pytest_homeassistant_custom_component.common import ( MockConfigEntry, + async_fire_time_changed, async_get_device_automations, ) @@ -50,6 +55,8 @@ HA_EVENT, MODE_AUTOMATIC, MODE_MANUAL, + RESTART_STATE_RESTORE, + SCAN_INTERVAL_SECONDS, ) BLEBOX_ID = "ae0bfbf927ba" @@ -728,6 +735,12 @@ async def test_a_half_answered_poll_does_not_forget_the_shape( answers its identity but times out on one of those reports exactly what a device that genuinely has none of it reports. Remembering that would leave the next offline start missing entities the device really does have. + + Settings and network defend themselves by carrying the last payload + forward, so what is remembered is still what the device last said about + itself. Uptime cannot do that - it only ever counts up, so a carried-forward + one would be a lie - and it is therefore still what decides whether a poll + is worth remembering at all. """ entry = _entry() await _setup(hass, entry) @@ -748,6 +761,16 @@ async def test_a_half_answered_poll_does_not_forget_the_shape( assert entry.data[CONF_DEVICE_CACHE] == remembered assert entry.data[CONF_DEVICE_CACHE]["settings"]["relays"] + # A device that has reported an uptime and now will not is the one case + # that still has to hold the shape back, rather than remember a device + # without an uptime sensor. + with _device_reads(uptime=None): + coordinator.async_request_full_refresh() + await coordinator.async_refresh() + await hass.async_block_till_done() + + assert entry.data[CONF_DEVICE_CACHE] == remembered + async def test_a_changed_shape_is_remembered_without_reloading( hass: HomeAssistant, @@ -777,6 +800,32 @@ async def test_a_changed_shape_is_remembered_without_reloading( assert entry.runtime_data.coordinator is coordinator +async def test_new_firmware_is_remembered_for_the_next_offline_start( + hass: HomeAssistant, +) -> None: + """A firmware update reaches the remembered identity, not just the live one. + + The signature recorded only whether an identity existed, so a firmware + version change never rewrote the cache. Starting up unreachable then seeded + the device page from before the update, which is the one field someone + checks to confirm an update worked. + """ + entry = _entry() + await _setup(hass, entry) + assert entry.data[CONF_DEVICE_CACHE]["info"]["fv"] == DEVICE.firmware_version + + updated = dataclasses.replace(DEVICE, firmware_version="0.1600") + with ( + _device_reads(), + patch(f"{MANAGER}.async_get_device_info", return_value=updated), + ): + entry.runtime_data.coordinator.async_request_full_refresh() + await entry.runtime_data.coordinator.async_refresh() + await hass.async_block_till_done() + + assert entry.data[CONF_DEVICE_CACHE]["info"]["fv"] == "0.1600" + + async def test_removing_the_entry_clears_its_repair_issues( hass: HomeAssistant, ) -> None: @@ -836,3 +885,267 @@ async def test_diagnostics_redact_the_token(hass: HomeAssistant) -> None: assert TOKEN not in repr(diagnostics) assert diagnostics["inputs"]["detected"] == [0, 1] assert len(diagnostics["callback_mappings"]) == 3 + + +# --- a best-effort read that failed is not a device that answered emptily ---- + + +async def test_a_failed_settings_read_keeps_the_settings_it_had( + hass: HomeAssistant, +) -> None: + """A settings read that times out leaves the settings-backed entities alone. + + Regression: settings are a best-effort read, so a failure was swallowed and + the empty payload left behind went into the snapshot as though the device + had answered with it. The poll still counted as a success, so nothing went + unavailable and the entities simply inverted: Home Assistant recorded that + as a genuine state change, so the switch the README singles out as the + security relevant one reported itself turned off and any automation watching + the cloud tunnel fired for nothing. + """ + entry = _entry() + await _setup(hass, entry) + coordinator = entry.runtime_data.coordinator + + tunnel = _registered(hass, "switch", "cloud_tunnel") + backlight = _registered(hass, "light", "buttons_backlight") + overload = _registered(hass, "number", "overload_threshold") + restart = _registered(hass, "select", "state_after_restart") + assert hass.states.get(tunnel).state == STATE_ON + + with ( + _device_reads(), + patch( + f"{MANAGER}.async_get_settings", + side_effect=BleBoxConnectionError("timed out"), + ), + ): + coordinator.async_request_full_refresh() + await coordinator.async_refresh() + await hass.async_block_till_done() + + # Still believed, because the device did answer everything else - and still + # reporting what it last said about itself rather than the absence of it. + assert coordinator.last_update_success + assert hass.states.get(tunnel).state == STATE_ON + assert hass.states.get(backlight).state == STATE_ON + # The colour matters as much as the state here: with the payload blanked, + # turning the backlight on wrote the default colour over the user's own. + assert hass.states.get(backlight).attributes["rgb_color"] == (255, 255, 255) + assert hass.states.get(overload).state == "0.0" + assert hass.states.get(restart).state == RESTART_STATE_RESTORE + + # Every relay-only poll until the next slow cycle carries the same snapshot + # forward, which is what stretched a single failed read to a whole minute. + with _device_reads(): + await coordinator.async_refresh() + await hass.async_block_till_done() + assert hass.states.get(tunnel).state == STATE_ON + + # A device that really does answer with an empty object is still believed: + # carrying values forward must not outlive the failure it covers for. + with _device_reads(), patch(f"{MANAGER}.async_get_settings", return_value={}): + coordinator.async_request_full_refresh() + await coordinator.async_refresh() + await hass.async_block_till_done() + assert hass.states.get(tunnel).state == "off" + + +async def test_a_failed_network_read_keeps_the_access_point_it_had( + hass: HomeAssistant, +) -> None: + """The same for the network read, which the access point switch is built on. + + A blanked network payload turned the access point switch off and emptied the + SSID it publishes, so the device looked as though it had stopped + broadcasting - the opposite of the state the entity exists to warn about. + """ + entry = _entry() + await _setup(hass, entry) + coordinator = entry.runtime_data.coordinator + + access_point = _registered(hass, "switch", "access_point") + assert hass.states.get(access_point).state == STATE_ON + + with ( + _device_reads(), + patch( + f"{MANAGER}.async_get_network", + side_effect=BleBoxConnectionError("timed out"), + ), + ): + coordinator.async_request_full_refresh() + await coordinator.async_refresh() + await hass.async_block_till_done() + + assert coordinator.last_update_success + state = hass.states.get(access_point) + assert state.state == STATE_ON + assert state.attributes["ssid"] == NETWORK["apSSID"] + + +async def test_a_device_without_a_network_endpoint_is_still_remembered( + hass: HomeAssistant, +) -> None: + """Firmware without ``/api/device/network`` still has its shape remembered. + + The remembered shape is the only thing that gives a device its entities back + after a restart while it is unreachable. It used to be written only when + settings, network *and* uptime had all answered on the same poll, so a + device whose firmware simply does not have that endpoint was never + remembered at all and fell into the offline-start trap on every restart it + was unlucky with. + """ + entry = _entry() + entry.add_to_hass(hass) + with ( + _device_reads(), + patch( + f"{MANAGER}.async_get_network", + side_effect=BleBoxConnectionError("no such endpoint"), + ), + patch(f"{MANAGER}.async_save_action"), + ): + assert await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + cache = entry.data.get(CONF_DEVICE_CACHE) + assert cache, "a device that answered everything it has was not remembered" + assert cache["settings"]["relays"] + # Nothing was invented for the endpoint it does not have, so the access + # point switch is absent here exactly as it is on a live poll. + assert cache["network"] == {} + assert _registered(hass, "switch", "access_point") is None + + # What all of that is for: the entities come back on an offline restart. + persisted = dict(entry.data) + with patch(f"{MANAGER}.async_remove_owned_actions", return_value=[]): + await hass.config_entries.async_remove(entry.entry_id) + await hass.async_block_till_done() + + restarted = MockConfigEntry( + domain=DOMAIN, + title=entry.title, + unique_id=BLEBOX_ID, + data=persisted, + options=dict(entry.options), + ) + restarted.add_to_hass(hass) + with _unreachable(): + assert await hass.config_entries.async_setup(restarted.entry_id) + await hass.async_block_till_done() + + assert _registered(hass, "switch", "relay") is not None + assert _registered(hass, "switch", "cloud_tunnel") is not None + + # Unloaded inside the patch: a poll would otherwise reach a real socket. + await hass.config_entries.async_unload(restarted.entry_id) + await hass.async_block_till_done() + + +async def test_a_device_that_has_never_answered_comes_back_by_itself( + hass: HomeAssistant, +) -> None: + """An entry set up while the device was down still recovers on its own. + + Regression: setup deliberately succeeds when the device is unreachable, so + Home Assistant never retries it, and with nothing remembered every polled + platform creates nothing. `DataUpdateCoordinator` arms its interval only + while something is listening to it, so the entry stopped polling altogether: + the device could come back and nothing would notice, no automatic callback + would ever be healed, and only a manual reload got the entry out of it. + """ + entry = _entry() + entry.add_to_hass(hass) + with _unreachable(): + assert await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + assert _registered(hass, "switch", "relay") is None + # Kept across the reload below, which replaces the runtime data. + coordinator = entry.runtime_data.coordinator + + # A poll that failed is announced just as a successful one is, and must + # not be taken for the device answering: there is still nothing to build + # anything from, so nothing should happen but another poll. + async_fire_time_changed( + hass, dt_util.utcnow() + timedelta(seconds=SCAN_INTERVAL_SECONDS + 1) + ) + await hass.async_block_till_done() + assert not coordinator.last_update_success + assert _registered(hass, "switch", "relay") is None + + # The announcement a poll makes when it fails after a good one, which is + # how entities go unavailable. It carries no snapshot either. + coordinator.async_update_listeners() + await hass.async_block_till_done() + assert _registered(hass, "switch", "relay") is None + + with _device_reads(), patch(f"{MANAGER}.async_save_action"): + async_fire_time_changed( + hass, dt_util.utcnow() + timedelta(seconds=2 * SCAN_INTERVAL_SECONDS + 2) + ) + await hass.async_block_till_done() + + # It polled at all, which is what an entry with no entities stopped + # doing, and then acted on what the poll finally told it. + assert coordinator.last_update_success + relay = _registered(hass, "switch", "relay") + assert relay is not None, "the device answered and nothing was created" + assert hass.states.get(relay).state == STATE_ON + # The pushed entities were there all along and survive the recovery. + assert hass.states.get(_entity_id(hass, 0)) is not None + + await hass.config_entries.async_unload(entry.entry_id) + await hass.async_block_till_done() + + +async def test_enabling_events_for_an_input_reloads_the_entry_once( + hass: HomeAssistant, +) -> None: + """Ticking events for an unused input reloads the entry once, not twice. + + An input with nothing selected is registered disabled, and + ``entity_registry_enabled_default`` is read only at that first registration, + so platform setup has to clear the disable by hand for the option to mean + anything. Doing that the obvious way makes Home Assistant schedule a reload + of the config entry thirty seconds later, which tears down every entity and + re-runs provisioning to arrive at exactly what the reload the user's own + change already triggered had just finished building. + """ + entry = _entry(**{CONF_ENABLED_EVENTS: {"0": ["short_press"], "1": []}}) + await _setup(hass, entry) + registry = er.async_get(hass) + spare = _entity_id(hass, 1) + assert registry.async_get(spare).disabled_by is er.RegistryEntryDisabler.INTEGRATION + + with ( + _device_reads(), + patch(f"{MANAGER}.async_save_action"), + patch.object( + hass.config_entries, + "async_reload", + wraps=hass.config_entries.async_reload, + ) as reload, + ): + hass.config_entries.async_update_entry( + entry, + options={ + **entry.options, + CONF_ENABLED_EVENTS: {"0": ["short_press"], "1": ["long_press"]}, + }, + ) + await hass.async_block_till_done() + + # The option took effect on the one reload the change itself asked for. + assert reload.call_count == 1 + assert registry.async_get(spare).disabled_by is None + assert hass.states.get(spare) is not None + + async_fire_time_changed( + hass, dt_util.utcnow() + timedelta(seconds=RELOAD_AFTER_UPDATE_DELAY + 1) + ) + await hass.async_block_till_done() + assert reload.call_count == 1, "the entry was reloaded a second time" + + await hass.config_entries.async_unload(entry.entry_id) + await hass.async_block_till_done() diff --git a/tests/test_settings_entities.py b/tests/test_settings_entities.py index 1082715..a54d881 100644 --- a/tests/test_settings_entities.py +++ b/tests/test_settings_entities.py @@ -8,12 +8,14 @@ import time from contextlib import ExitStack +from dataclasses import replace from typing import Any from unittest.mock import patch import pytest from homeassistant.core import HomeAssistant from homeassistant.exceptions import HomeAssistantError, ServiceValidationError +from homeassistant.helpers import device_registry as dr from homeassistant.helpers import entity_registry as er from pytest_homeassistant_custom_component.common import MockConfigEntry @@ -36,6 +38,7 @@ UPTIME_S, _actions_state, _entry, + _unreachable, ) @@ -677,3 +680,70 @@ async def test_device_settings_win_again_after_the_settle_window( await hass.async_block_till_done() assert hass.states.get(entity_id).state == "on" + + +# --- device identity -------------------------------------------------------- + + +async def test_a_firmware_update_reaches_the_device_page(hass: HomeAssistant) -> None: + """A new firmware version shows on the device page without a reload. + + Regression: model, firmware and hardware version were read out of + `entry.data`, which the config flow writes once and nothing ever updates. + The coordinator re-read the identity on every slow cycle and the update + entity showed the new version, but the device page kept the old one + indefinitely - and that page is exactly where a user looks to confirm a + firmware update, which this integration can start itself, actually worked. + """ + entry = _entry() + await _setup_with(hass, entry) + + registry = dr.async_get(hass) + device = next(iter(dr.async_entries_for_config_entry(registry, entry.entry_id))) + assert device.sw_version == "0.1502" + + coordinator = entry.runtime_data.coordinator + with ( + _reads(), + patch( + f"{MANAGER}.async_get_device_info", + return_value=replace(DEVICE, firmware_version="0.1600"), + ), + ): + # Forced, because identity is only re-read on the slow cycle. + coordinator.async_request_full_refresh() + await coordinator.async_refresh() + await hass.async_block_till_done() + + assert registry.async_get(device.id).sw_version == "0.1600" + firmware = hass.states.get(_eid(hass, "update", "firmware")) + # The device page and the update entity next to it must not disagree. + assert firmware.attributes["installed_version"] == "0.1600" + + +async def test_a_device_that_never_answered_still_has_a_device_page( + hass: HomeAssistant, +) -> None: + """The versions the config flow stored carry an entry that starts offline. + + Live identity is preferred over `entry.data`, but a device that has never + answered has no live identity to offer: the fallback is all it has, and it + has to keep producing device info Home Assistant accepts, or an offline + start would leave the pushed event entities with no device page at all. + """ + entry = _entry() + entry.add_to_hass(hass) + with _unreachable(): + assert await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + registry = dr.async_get(hass) + device = next(iter(dr.async_entries_for_config_entry(registry, entry.entry_id))) + assert device.model == "switchBox" + assert device.sw_version == "0.1502" + assert device.hw_version == "s_KS.swB.1.5.T.p55ST-0.3" + + # Unloaded inside the patch: the retry timer would otherwise fire + # against a real socket during teardown. + await hass.config_entries.async_unload(entry.entry_id) + await hass.async_block_till_done()