From 1ded3ac51ebc1915a5026af1998eb119972f6117 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 14 Nov 2022 10:54:28 -0600 Subject: [PATCH] Poll HomeKit Controller locks for state after lock operation (#82058) --- .../homekit_controller/connection.py | 23 +++++++++++++++++-- .../components/homekit_controller/const.py | 7 ++++++ .../components/homekit_controller/entity.py | 4 ++++ .../components/homekit_controller/lock.py | 4 ++++ tests/components/homekit_controller/common.py | 3 +++ 5 files changed, 39 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/homekit_controller/connection.py b/homeassistant/components/homekit_controller/connection.py index 3df5ab8aaed..f158cd49e9c 100644 --- a/homeassistant/components/homekit_controller/connection.py +++ b/homeassistant/components/homekit_controller/connection.py @@ -3,7 +3,7 @@ from __future__ import annotations import asyncio from collections.abc import Callable, Iterable -from datetime import timedelta +from datetime import datetime, timedelta import logging from types import MappingProxyType from typing import Any @@ -22,6 +22,7 @@ from homeassistant.config_entries import ConfigEntry from homeassistant.const import ATTR_VIA_DEVICE, EVENT_HOMEASSISTANT_STARTED from homeassistant.core import CALLBACK_TYPE, CoreState, Event, HomeAssistant, callback from homeassistant.helpers import device_registry as dr, entity_registry as er +from homeassistant.helpers.debounce import Debouncer from homeassistant.helpers.dispatcher import async_dispatcher_send from homeassistant.helpers.entity import DeviceInfo from homeassistant.helpers.event import async_track_time_interval @@ -29,6 +30,7 @@ from homeassistant.helpers.event import async_track_time_interval from .const import ( CHARACTERISTIC_PLATFORMS, CONTROLLER, + DEBOUNCE_COOLDOWN, DOMAIN, HOMEKIT_ACCESSORY_DISPATCH, IDENTIFIER_ACCESSORY_ID, @@ -41,6 +43,8 @@ from .device_trigger import async_fire_triggers, async_setup_triggers_for_entry RETRY_INTERVAL = 60 # seconds MAX_POLL_FAILURES_TO_DECLARE_UNAVAILABLE = 3 + + BLE_AVAILABILITY_CHECK_INTERVAL = 1800 # seconds _LOGGER = logging.getLogger(__name__) @@ -127,6 +131,14 @@ class HKDevice: self.watchable_characteristics: list[tuple[int, int]] = [] + self._debounced_update = Debouncer( + hass, + _LOGGER, + cooldown=DEBOUNCE_COOLDOWN, + immediate=False, + function=self.async_update, + ) + @property def entity_map(self) -> Accessories: """Return the accessories from the pairing.""" @@ -240,8 +252,11 @@ class HKDevice: self.async_set_available_state(self.pairing.is_available) + # We use async_request_update to avoid multiple updates + # at the same time which would generate a spurious warning + # in the log about concurrent polling. self._polling_interval_remover = async_track_time_interval( - self.hass, self.async_update, self.pairing.poll_interval + self.hass, self.async_request_update, self.pairing.poll_interval ) if transport == Transport.BLE: @@ -631,6 +646,10 @@ class HKDevice: """Update the available state of the device.""" self.async_set_available_state(self.pairing.is_available) + async def async_request_update(self, now: datetime | None = None) -> None: + """Request an debounced update from the accessory.""" + await self._debounced_update.async_call() + async def async_update(self, now=None): """Poll state of all entities attached to this bridge/accessory.""" if not self.pollable_characteristics: diff --git a/homeassistant/components/homekit_controller/const.py b/homeassistant/components/homekit_controller/const.py index 8c7db4dad00..7ed844ae9ae 100644 --- a/homeassistant/components/homekit_controller/const.py +++ b/homeassistant/components/homekit_controller/const.py @@ -107,3 +107,10 @@ STARTUP_EXCEPTIONS = ( EncryptionError, AccessoryDisconnectedError, ) + +# 10 seconds was chosen because its soon enough +# for most state changes to happen but not too +# long that the BLE connection is dropped. It +# also happens to be the same value used by +# the update coordinator. +DEBOUNCE_COOLDOWN = 10 # seconds diff --git a/homeassistant/components/homekit_controller/entity.py b/homeassistant/components/homekit_controller/entity.py index a4e1b2b41b3..1c492decf49 100644 --- a/homeassistant/components/homekit_controller/entity.py +++ b/homeassistant/components/homekit_controller/entity.py @@ -175,6 +175,10 @@ class HomeKitEntity(Entity): """Define the homekit characteristics the entity cares about.""" raise NotImplementedError + async def async_update(self) -> None: + """Update the entity.""" + await self._accessory.async_request_update() + class AccessoryEntity(HomeKitEntity): """A HomeKit entity that is related to an entire accessory rather than a specific service or characteristic.""" diff --git a/homeassistant/components/homekit_controller/lock.py b/homeassistant/components/homekit_controller/lock.py index a6c8a3672a3..df03a1fef34 100644 --- a/homeassistant/components/homekit_controller/lock.py +++ b/homeassistant/components/homekit_controller/lock.py @@ -124,6 +124,10 @@ class HomeKitLock(HomeKitEntity, LockEntity): await self.async_put_characteristics( {CharacteristicsTypes.LOCK_MECHANISM_TARGET_STATE: TARGET_STATE_MAP[state]} ) + # Some locks need to be polled to update the current state + # after a target state change. + # https://github.com/home-assistant/core/issues/81887 + await self._accessory.async_request_update() @property def extra_state_attributes(self) -> dict[str, Any]: diff --git a/tests/components/homekit_controller/common.py b/tests/components/homekit_controller/common.py index b30ba6236a9..ec30d541a93 100644 --- a/tests/components/homekit_controller/common.py +++ b/tests/components/homekit_controller/common.py @@ -21,6 +21,7 @@ from aiohomekit.zeroconf import HomeKitService from homeassistant.components.device_automation import DeviceAutomationType from homeassistant.components.homekit_controller.const import ( CONTROLLER, + DEBOUNCE_COOLDOWN, DOMAIN, HOMEKIT_ACCESSORY_DISPATCH, IDENTIFIER_ACCESSORY_ID, @@ -146,6 +147,7 @@ class Helper: # If they are enabled, then HA will pick up the changes next time # we yield control await time_changed(self.hass, 60) + await time_changed(self.hass, DEBOUNCE_COOLDOWN) await self.hass.async_block_till_done() @@ -165,6 +167,7 @@ class Helper: async def poll_and_get_state(self) -> State: """Trigger a time based poll and return the current entity state.""" await time_changed(self.hass, 60) + await time_changed(self.hass, DEBOUNCE_COOLDOWN) state = self.hass.states.get(self.entity_id) assert state is not None