From eb0828efdb672c4861125fa4c02933a3943ea911 Mon Sep 17 00:00:00 2001 From: Jc2k Date: Sat, 20 Aug 2022 21:58:59 +0100 Subject: [PATCH] Dont rely on config flow to monitor homekit_controller c# changes (#76861) --- .../components/homekit_controller/__init__.py | 4 +- .../homekit_controller/config_flow.py | 18 ------- .../homekit_controller/connection.py | 24 +-------- .../homekit_controller/manifest.json | 2 +- .../components/homekit_controller/utils.py | 4 ++ requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- tests/components/homekit_controller/common.py | 50 ++++++++----------- .../specific_devices/test_ecobee3.py | 7 ++- .../homekit_controller/test_config_flow.py | 4 +- .../homekit_controller/test_init.py | 2 + 11 files changed, 36 insertions(+), 83 deletions(-) diff --git a/homeassistant/components/homekit_controller/__init__.py b/homeassistant/components/homekit_controller/__init__.py index 3a5ba42848c..9b431b09c9e 100644 --- a/homeassistant/components/homekit_controller/__init__.py +++ b/homeassistant/components/homekit_controller/__init__.py @@ -21,7 +21,7 @@ from homeassistant.helpers.typing import ConfigType from .config_flow import normalize_hkid from .connection import HKDevice from .const import ENTITY_MAP, KNOWN_DEVICES, TRIGGERS -from .storage import EntityMapStorage, async_get_entity_storage +from .storage import EntityMapStorage from .utils import async_get_controller _LOGGER = logging.getLogger(__name__) @@ -50,8 +50,6 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: """Set up for Homekit devices.""" - await async_get_entity_storage(hass) - await async_get_controller(hass) hass.data[KNOWN_DEVICES] = {} diff --git a/homeassistant/components/homekit_controller/config_flow.py b/homeassistant/components/homekit_controller/config_flow.py index 2ccdd557a5b..62144077a94 100644 --- a/homeassistant/components/homekit_controller/config_flow.py +++ b/homeassistant/components/homekit_controller/config_flow.py @@ -24,7 +24,6 @@ from homeassistant.core import HomeAssistant, callback from homeassistant.data_entry_flow import AbortFlow, FlowResult from homeassistant.helpers import device_registry as dr -from .connection import HKDevice from .const import DOMAIN, KNOWN_DEVICES from .storage import async_get_entity_storage from .utils import async_get_controller @@ -253,17 +252,6 @@ class HomekitControllerFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): category = Categories(int(properties.get("ci", 0))) paired = not status_flags & 0x01 - # The configuration number increases every time the characteristic map - # needs updating. Some devices use a slightly off-spec name so handle - # both cases. - try: - config_num = int(properties["c#"]) - except KeyError: - _LOGGER.warning( - "HomeKit device %s: c# not exposed, in violation of spec", hkid - ) - config_num = None - # Set unique-id and error out if it's already configured existing_entry = await self.async_set_unique_id( normalized_hkid, raise_on_progress=False @@ -280,12 +268,6 @@ class HomekitControllerFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): self.hass.config_entries.async_update_entry( existing_entry, data={**existing_entry.data, **updated_ip_port} ) - conn: HKDevice = self.hass.data[KNOWN_DEVICES][hkid] - if config_num and conn.config_num != config_num: - _LOGGER.debug( - "HomeKit info %s: c# incremented, refreshing entities", hkid - ) - conn.async_notify_config_changed(config_num) return self.async_abort(reason="already_configured") _LOGGER.debug("Discovered device %s (%s - %s)", name, model, hkid) diff --git a/homeassistant/components/homekit_controller/connection.py b/homeassistant/components/homekit_controller/connection.py index b2f3b3ae8e0..b4aaab5acf0 100644 --- a/homeassistant/components/homekit_controller/connection.py +++ b/homeassistant/components/homekit_controller/connection.py @@ -30,7 +30,6 @@ from .const import ( CHARACTERISTIC_PLATFORMS, CONTROLLER, DOMAIN, - ENTITY_MAP, HOMEKIT_ACCESSORY_DISPATCH, IDENTIFIER_ACCESSORY_ID, IDENTIFIER_LEGACY_ACCESSORY_ID, @@ -38,7 +37,6 @@ from .const import ( IDENTIFIER_SERIAL_NUMBER, ) from .device_trigger import async_fire_triggers, async_setup_triggers_for_entry -from .storage import EntityMapStorage RETRY_INTERVAL = 60 # seconds MAX_POLL_FAILURES_TO_DECLARE_UNAVAILABLE = 3 @@ -182,14 +180,10 @@ class HKDevice: async def async_setup(self) -> None: """Prepare to use a paired HomeKit device in Home Assistant.""" - entity_storage: EntityMapStorage = self.hass.data[ENTITY_MAP] pairing = self.pairing transport = pairing.transport entry = self.config_entry - if cache := entity_storage.get_map(self.unique_id): - pairing.restore_accessories_state(cache["accessories"], cache["config_num"]) - # We need to force an update here to make sure we have # the latest values since the async_update we do in # async_process_entity_map will no values to poll yet @@ -203,7 +197,7 @@ class HKDevice: try: await self.pairing.async_populate_accessories_state(force_update=True) except AccessoryNotFoundError: - if transport != Transport.BLE or not cache: + if transport != Transport.BLE or not pairing.accessories: # BLE devices may sleep and we can't force a connection raise @@ -217,9 +211,6 @@ class HKDevice: await self.async_process_entity_map() - if not cache: - # If its missing from the cache, make sure we save it - self.async_save_entity_map() # If everything is up to date, we can create the entities # since we know the data is not stale. await self.async_add_new_entities() @@ -438,31 +429,18 @@ class HKDevice: self.config_entry, self.platforms ) - def async_notify_config_changed(self, config_num: int) -> None: - """Notify the pairing of a config change.""" - self.pairing.notify_config_changed(config_num) - def process_config_changed(self, config_num: int) -> None: """Handle a config change notification from the pairing.""" self.hass.async_create_task(self.async_update_new_accessories_state()) async def async_update_new_accessories_state(self) -> None: """Process a change in the pairings accessories state.""" - self.async_save_entity_map() await self.async_process_entity_map() if self.watchable_characteristics: await self.pairing.subscribe(self.watchable_characteristics) await self.async_update() await self.async_add_new_entities() - @callback - def async_save_entity_map(self) -> None: - """Save the entity map.""" - entity_storage: EntityMapStorage = self.hass.data[ENTITY_MAP] - entity_storage.async_create_or_update_map( - self.unique_id, self.config_num, self.entity_map.serialize() - ) - def add_accessory_factory(self, add_entities_cb) -> None: """Add a callback to run when discovering new entities for accessories.""" self.accessory_factories.append(add_entities_cb) diff --git a/homeassistant/components/homekit_controller/manifest.json b/homeassistant/components/homekit_controller/manifest.json index 3f8d7828236..143627fe7f0 100644 --- a/homeassistant/components/homekit_controller/manifest.json +++ b/homeassistant/components/homekit_controller/manifest.json @@ -3,7 +3,7 @@ "name": "HomeKit Controller", "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/homekit_controller", - "requirements": ["aiohomekit==1.3.0"], + "requirements": ["aiohomekit==1.4.0"], "zeroconf": ["_hap._tcp.local.", "_hap._udp.local."], "bluetooth": [{ "manufacturer_id": 76, "manufacturer_data_start": [6] }], "dependencies": ["bluetooth", "zeroconf"], diff --git a/homeassistant/components/homekit_controller/utils.py b/homeassistant/components/homekit_controller/utils.py index 6e272067b54..b43f1ee05f7 100644 --- a/homeassistant/components/homekit_controller/utils.py +++ b/homeassistant/components/homekit_controller/utils.py @@ -8,6 +8,7 @@ from homeassistant.const import EVENT_HOMEASSISTANT_STOP from homeassistant.core import Event, HomeAssistant from .const import CONTROLLER +from .storage import async_get_entity_storage def folded_name(name: str) -> str: @@ -22,6 +23,8 @@ async def async_get_controller(hass: HomeAssistant) -> Controller: async_zeroconf_instance = await zeroconf.async_get_async_instance(hass) + char_cache = await async_get_entity_storage(hass) + # In theory another call to async_get_controller could have run while we were # trying to get the zeroconf instance. So we check again to make sure we # don't leak a Controller instance here. @@ -33,6 +36,7 @@ async def async_get_controller(hass: HomeAssistant) -> Controller: controller = Controller( async_zeroconf_instance=async_zeroconf_instance, bleak_scanner_instance=bleak_scanner_instance, # type: ignore[arg-type] + char_cache=char_cache, ) hass.data[CONTROLLER] = controller diff --git a/requirements_all.txt b/requirements_all.txt index b9820f94872..7901c1594a2 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -168,7 +168,7 @@ aioguardian==2022.07.0 aioharmony==0.2.9 # homeassistant.components.homekit_controller -aiohomekit==1.3.0 +aiohomekit==1.4.0 # homeassistant.components.emulated_hue # homeassistant.components.http diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 5cb5b40a2c4..e6b9004cecf 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -152,7 +152,7 @@ aioguardian==2022.07.0 aioharmony==0.2.9 # homeassistant.components.homekit_controller -aiohomekit==1.3.0 +aiohomekit==1.4.0 # homeassistant.components.emulated_hue # homeassistant.components.http diff --git a/tests/components/homekit_controller/common.py b/tests/components/homekit_controller/common.py index 18367d28f63..fd543d55ffb 100644 --- a/tests/components/homekit_controller/common.py +++ b/tests/components/homekit_controller/common.py @@ -11,10 +11,9 @@ from unittest import mock from aiohomekit.model import Accessories, AccessoriesState, Accessory from aiohomekit.testing import FakeController, FakePairing +from aiohomekit.zeroconf import HomeKitService -from homeassistant.components import zeroconf from homeassistant.components.device_automation import DeviceAutomationType -from homeassistant.components.homekit_controller import config_flow from homeassistant.components.homekit_controller.const import ( CONTROLLER, DOMAIN, @@ -22,6 +21,7 @@ from homeassistant.components.homekit_controller.const import ( IDENTIFIER_ACCESSORY_ID, IDENTIFIER_SERIAL_NUMBER, ) +from homeassistant.components.homekit_controller.utils import async_get_controller from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant, State, callback from homeassistant.helpers import device_registry as dr, entity_registry as er @@ -175,12 +175,11 @@ async def setup_platform(hass): config = {"discovery": {}} with mock.patch( - "homeassistant.components.homekit_controller.utils.Controller" - ) as controller: - fake_controller = controller.return_value = FakeController() + "homeassistant.components.homekit_controller.utils.Controller", FakeController + ): await async_setup_component(hass, DOMAIN, config) - return fake_controller + return await async_get_controller(hass) async def setup_test_accessories(hass, accessories): @@ -228,31 +227,24 @@ async def device_config_changed(hass, accessories): pairing._accessories_state = AccessoriesState( accessories_obj, pairing.config_num + 1 ) - - discovery_info = zeroconf.ZeroconfServiceInfo( - host="127.0.0.1", - addresses=["127.0.0.1"], - hostname="mock_hostname", - name="TestDevice._hap._tcp.local.", - port=8080, - properties={ - "md": "TestDevice", - "id": "00:00:00:00:00:00", - "c#": "2", - "sf": "0", - }, - type="mock_type", + pairing._async_description_update( + HomeKitService( + name="TestDevice.local", + id="00:00:00:00:00:00", + model="", + config_num=2, + state_num=3, + feature_flags=0, + status_flags=0, + category=1, + protocol_version="1.0", + type="_hap._tcp.local.", + address="127.0.0.1", + addresses=["127.0.0.1"], + port=8080, + ) ) - # Config Flow will abort and notify us if the discovery event is of - # interest - in this case c# has incremented - flow = config_flow.HomekitControllerFlowHandler() - flow.hass = hass - flow.context = {} - result = await flow.async_step_zeroconf(discovery_info) - assert result["type"] == "abort" - assert result["reason"] == "already_configured" - # Wait for services to reconfigure await hass.async_block_till_done() await hass.async_block_till_done() diff --git a/tests/components/homekit_controller/specific_devices/test_ecobee3.py b/tests/components/homekit_controller/specific_devices/test_ecobee3.py index 3c47195b442..4da2f572626 100644 --- a/tests/components/homekit_controller/specific_devices/test_ecobee3.py +++ b/tests/components/homekit_controller/specific_devices/test_ecobee3.py @@ -6,7 +6,7 @@ https://github.com/home-assistant/core/issues/15336 from unittest import mock -from aiohomekit import AccessoryDisconnectedError +from aiohomekit import AccessoryNotFoundError from aiohomekit.testing import FakePairing from homeassistant.components.climate.const import ( @@ -184,9 +184,8 @@ async def test_ecobee3_setup_connection_failure(hass): # Test that the connection fails during initial setup. # No entities should be created. - list_accessories = "list_accessories_and_characteristics" - with mock.patch.object(FakePairing, list_accessories) as laac: - laac.side_effect = AccessoryDisconnectedError("Connection failed") + with mock.patch.object(FakePairing, "async_populate_accessories_state") as laac: + laac.side_effect = AccessoryNotFoundError("Connection failed") # If there is no cached entity map and the accessory connection is # failing then we have to fail the config entry setup. diff --git a/tests/components/homekit_controller/test_config_flow.py b/tests/components/homekit_controller/test_config_flow.py index 3f545be8931..5e2c8249560 100644 --- a/tests/components/homekit_controller/test_config_flow.py +++ b/tests/components/homekit_controller/test_config_flow.py @@ -1,7 +1,7 @@ """Tests for homekit_controller config flow.""" import asyncio import unittest.mock -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, patch import aiohomekit from aiohomekit.exceptions import AuthenticationError @@ -524,7 +524,6 @@ async def test_discovery_already_configured_update_csharp(hass, controller): entry.add_to_hass(hass) connection_mock = AsyncMock() - connection_mock.async_notify_config_changed = MagicMock() hass.data[KNOWN_DEVICES] = {"AA:BB:CC:DD:EE:FF": connection_mock} device = setup_mock_accessory(controller) @@ -547,7 +546,6 @@ async def test_discovery_already_configured_update_csharp(hass, controller): assert entry.data["AccessoryIP"] == discovery_info.host assert entry.data["AccessoryPort"] == discovery_info.port - assert connection_mock.async_notify_config_changed.call_count == 1 @pytest.mark.parametrize("exception,expected", PAIRING_START_ABORT_ERRORS) diff --git a/tests/components/homekit_controller/test_init.py b/tests/components/homekit_controller/test_init.py index 37d41fcf372..a91700f699c 100644 --- a/tests/components/homekit_controller/test_init.py +++ b/tests/components/homekit_controller/test_init.py @@ -119,6 +119,7 @@ async def test_offline_device_raises(hass, controller): nonlocal is_connected if not is_connected: raise AccessoryNotFoundError("any") + await super().async_populate_accessories_state(*args, **kwargs) async def get_characteristics(self, chars, *args, **kwargs): nonlocal is_connected @@ -173,6 +174,7 @@ async def test_ble_device_only_checks_is_available(hass, controller): nonlocal is_available if not is_available: raise AccessoryNotFoundError("any") + await super().async_populate_accessories_state(*args, **kwargs) async def get_characteristics(self, chars, *args, **kwargs): nonlocal is_available