diff --git a/homeassistant/components/homekit/__init__.py b/homeassistant/components/homekit/__init__.py index aeb8cb7e4b8..6e67828ac61 100644 --- a/homeassistant/components/homekit/__init__.py +++ b/homeassistant/components/homekit/__init__.py @@ -1,7 +1,6 @@ """Support for Apple HomeKit.""" import ipaddress import logging -from zlib import adler32 import voluptuous as vol from zeroconf import InterfaceChoice @@ -32,7 +31,9 @@ from homeassistant.helpers.entityfilter import FILTER_SCHEMA from homeassistant.util import get_local_ip from homeassistant.util.decorator import Registry +from .aidmanager import AccessoryAidStorage from .const import ( + AID_STORAGE, BRIDGE_NAME, CONF_ADVERTISE_IP, CONF_AUTO_START, @@ -120,6 +121,9 @@ async def async_setup(hass, config): """Set up the HomeKit component.""" _LOGGER.debug("Begin setup HomeKit") + aid_storage = hass.data[AID_STORAGE] = AccessoryAidStorage(hass) + await aid_storage.async_initialize() + conf = config[DOMAIN] name = conf[CONF_NAME] port = conf[CONF_PORT] @@ -277,14 +281,6 @@ def get_accessory(hass, driver, state, aid, config): return TYPES[a_type](hass, driver, name, state.entity_id, aid, config) -def generate_aid(entity_id): - """Generate accessory aid with zlib adler32.""" - aid = adler32(entity_id.encode("utf-8")) - if aid in (0, 1): - return None - return aid - - class HomeKit: """Class to handle all actions between HomeKit and Home Assistant.""" @@ -339,9 +335,10 @@ class HomeKit: def reset_accessories(self, entity_ids): """Reset the accessory to load the latest configuration.""" + aid_storage = self.hass.data[AID_STORAGE] removed = [] for entity_id in entity_ids: - aid = generate_aid(entity_id) + aid = aid_storage.get_or_allocate_aid_for_entity_id(entity_id) if aid not in self.bridge.accessories: _LOGGER.warning( "Could not reset accessory. entity_id not found %s", entity_id @@ -359,7 +356,19 @@ class HomeKit: """Try adding accessory to bridge if configured beforehand.""" if not state or not self._filter(state.entity_id): return - aid = generate_aid(state.entity_id) + + # The bridge itself counts as an accessory + if len(self.bridge.accessories) + 1 >= MAX_DEVICES: + _LOGGER.warning( + "Cannot add %s as this would exceeded the %d device limit. Consider using the filter option.", + state.entity_id, + MAX_DEVICES, + ) + return + + aid = self.hass.data[AID_STORAGE].get_or_allocate_aid_for_entity_id( + state.entity_id + ) conf = self._config.pop(state.entity_id, {}) # If an accessory cannot be created or added due to an exception # of any kind (usually in pyhap) it should not prevent @@ -400,17 +409,12 @@ class HomeKit: for state in self.hass.states.all(): self.add_bridge_accessory(state) + self.driver.add_accessory(self.bridge) if not self.driver.state.paired: show_setup_message(self.hass, self.driver.state.pincode) - if len(self.bridge.accessories) > MAX_DEVICES: - _LOGGER.warning( - "You have exceeded the device limit, which might " - "cause issues. Consider using the filter option." - ) - _LOGGER.debug("Driver start") self.hass.add_job(self.driver.start) self.status = STATUS_RUNNING diff --git a/homeassistant/components/homekit/aidmanager.py b/homeassistant/components/homekit/aidmanager.py new file mode 100644 index 00000000000..61a922d17fa --- /dev/null +++ b/homeassistant/components/homekit/aidmanager.py @@ -0,0 +1,142 @@ +""" +Manage allocation of accessory ID's. + +HomeKit needs to allocate unique numbers to each accessory. These need to +be stable between reboots and upgrades. + +Using a hash function to generate them means collisions. It also means you +can't change the hash without causing breakages for HA users. + +This module generates and stores them in a HA storage. +""" +import logging +import random +from zlib import adler32 + +from fnvhash import fnv1a_32 + +from homeassistant.core import HomeAssistant, callback +from homeassistant.helpers.entity_registry import RegistryEntry +from homeassistant.helpers.storage import Store + +from .const import DOMAIN + +AID_MANAGER_STORAGE_KEY = f"{DOMAIN}.aids" +AID_MANAGER_STORAGE_VERSION = 1 +AID_MANAGER_SAVE_DELAY = 2 + +INVALID_AIDS = (0, 1) + +AID_MIN = 2 +AID_MAX = 18446744073709551615 + +_LOGGER = logging.getLogger(__name__) + + +def get_system_unique_id(entity: RegistryEntry): + """Determine the system wide unique_id for an entity.""" + return f"{entity.platform}.{entity.domain}.{entity.unique_id}" + + +def _generate_aids(unique_id: str, entity_id: str) -> int: + """Generate accessory aid.""" + + # Backward compatibility: Previously HA used to *only* do adler32 on the entity id. + # Not stable if entity ID changes + # Not robust against collisions + yield adler32(entity_id.encode("utf-8")) + + # Use fnv1a_32 of the unique id as + # fnv1a_32 has less collisions than + # adler32 + yield fnv1a_32(unique_id.encode("utf-8")) + + # If called again resort to random allocations. + # Given the size of the range its unlikely we'll encounter duplicates + # But try a few times regardless + for _ in range(5): + yield random.randrange(AID_MIN, AID_MAX) + + +class AccessoryAidStorage: + """ + Holds a map of entity ID to HomeKit ID. + + Will generate new ID's, ensure they are unique and store them to make sure they + persist over reboots. + """ + + def __init__(self, hass: HomeAssistant): + """Create a new entity map store.""" + self.hass = hass + self.store = Store(hass, AID_MANAGER_STORAGE_VERSION, AID_MANAGER_STORAGE_KEY) + self.allocations = {} + self.allocated_aids = set() + + self._entity_registry = None + + async def async_initialize(self): + """Load the latest AID data.""" + self._entity_registry = ( + await self.hass.helpers.entity_registry.async_get_registry() + ) + + raw_storage = await self.store.async_load() + if not raw_storage: + # There is no data about aid allocations yet + return + + self.allocations = raw_storage.get("unique_ids", {}) + self.allocated_aids = set(self.allocations.values()) + + def get_or_allocate_aid_for_entity_id(self, entity_id: str): + """Generate a stable aid for an entity id.""" + entity = self._entity_registry.async_get(entity_id) + + if entity: + return self._get_or_allocate_aid( + get_system_unique_id(entity), entity.entity_id + ) + + _LOGGER.warning( + "Entity '%s' does not have a stable unique identifier so aid allocation will be unstable and may cause collisions", + entity_id, + ) + return adler32(entity_id.encode("utf-8")) + + def _get_or_allocate_aid(self, unique_id: str, entity_id: str): + """Allocate (and return) a new aid for an accessory.""" + if unique_id in self.allocations: + return self.allocations[unique_id] + + for aid in _generate_aids(unique_id, entity_id): + if aid in INVALID_AIDS: + continue + if aid not in self.allocated_aids: + self.allocations[unique_id] = aid + self.allocated_aids.add(aid) + self.async_schedule_save() + return aid + + raise ValueError( + f"Unable to generate unique aid allocation for {entity_id} [{unique_id}]" + ) + + def delete_aid(self, unique_id: str): + """Delete an aid allocation.""" + if unique_id not in self.allocations: + return + + aid = self.allocations.pop(unique_id) + self.allocated_aids.discard(aid) + self.async_schedule_save() + + @callback + def async_schedule_save(self): + """Schedule saving the entity map cache.""" + self.store.async_delay_save(self._data_to_save, AID_MANAGER_SAVE_DELAY) + + @callback + def _data_to_save(self): + """Return data of entity map to store in a file.""" + return {"unique_ids": self.allocations} diff --git a/homeassistant/components/homekit/const.py b/homeassistant/components/homekit/const.py index 5f48b122e64..ac8d2571ac6 100644 --- a/homeassistant/components/homekit/const.py +++ b/homeassistant/components/homekit/const.py @@ -5,6 +5,7 @@ DEVICE_PRECISION_LEEWAY = 6 DOMAIN = "homekit" HOMEKIT_FILE = ".homekit.state" HOMEKIT_NOTIFY_ID = 4663548 +AID_STORAGE = "homekit-aid-allocations" # #### Attributes #### diff --git a/homeassistant/components/homekit/manifest.json b/homeassistant/components/homekit/manifest.json index 1e4c39f3ed9..ca6ab7a00a5 100644 --- a/homeassistant/components/homekit/manifest.json +++ b/homeassistant/components/homekit/manifest.json @@ -2,6 +2,6 @@ "domain": "homekit", "name": "HomeKit", "documentation": "https://www.home-assistant.io/integrations/homekit", - "requirements": ["HAP-python==2.8.2"], + "requirements": ["HAP-python==2.8.2","fnvhash==0.1.0"], "codeowners": ["@bdraco"] } diff --git a/requirements_all.txt b/requirements_all.txt index fb099483325..7354178e84c 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -561,6 +561,9 @@ fixerio==1.0.0a0 # homeassistant.components.flux_led flux_led==0.22 +# homeassistant.components.homekit +fnvhash==0.1.0 + # homeassistant.components.foobot foobot_async==0.3.1 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 9d4ebf4a2a0..5034b53c50d 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -213,6 +213,9 @@ ephem==3.7.7.0 # homeassistant.components.feedreader feedparser-homeassistant==5.2.2.dev1 +# homeassistant.components.homekit +fnvhash==0.1.0 + # homeassistant.components.foobot foobot_async==0.3.1 diff --git a/tests/components/homekit/test_aidmanager.py b/tests/components/homekit/test_aidmanager.py new file mode 100644 index 00000000000..bcadac953ab --- /dev/null +++ b/tests/components/homekit/test_aidmanager.py @@ -0,0 +1,120 @@ +"""Tests for the HomeKit AID manager.""" +from asynctest import patch +import pytest + +from homeassistant.components.homekit.aidmanager import ( + AccessoryAidStorage, + get_system_unique_id, +) +from homeassistant.helpers import device_registry + +from tests.common import MockConfigEntry, mock_device_registry, mock_registry + + +@pytest.fixture +def device_reg(hass): + """Return an empty, loaded, registry.""" + return mock_device_registry(hass) + + +@pytest.fixture +def entity_reg(hass): + """Return an empty, loaded, registry.""" + return mock_registry(hass) + + +async def test_aid_generation(hass, device_reg, entity_reg): + """Test generating aids.""" + config_entry = MockConfigEntry(domain="test", data={}) + config_entry.add_to_hass(hass) + device_entry = device_reg.async_get_or_create( + config_entry_id=config_entry.entry_id, + connections={(device_registry.CONNECTION_NETWORK_MAC, "12:34:56:AB:CD:EF")}, + ) + light_ent = entity_reg.async_get_or_create( + "light", "device", "unique_id", device_id=device_entry.id + ) + light_ent2 = entity_reg.async_get_or_create( + "light", "device", "other_unique_id", device_id=device_entry.id + ) + remote_ent = entity_reg.async_get_or_create( + "remote", "device", "unique_id", device_id=device_entry.id + ) + hass.states.async_set(light_ent.entity_id, "on") + hass.states.async_set(light_ent2.entity_id, "on") + hass.states.async_set(remote_ent.entity_id, "on") + hass.states.async_set("remote.has_no_unique_id", "on") + + with patch( + "homeassistant.components.homekit.aidmanager.AccessoryAidStorage.async_schedule_save" + ): + aid_storage = AccessoryAidStorage(hass) + await aid_storage.async_initialize() + + for _ in range(0, 2): + assert ( + aid_storage.get_or_allocate_aid_for_entity_id(light_ent.entity_id) + == 1692141785 + ) + assert ( + aid_storage.get_or_allocate_aid_for_entity_id(light_ent2.entity_id) + == 2732133210 + ) + assert ( + aid_storage.get_or_allocate_aid_for_entity_id(remote_ent.entity_id) + == 1867188557 + ) + assert ( + aid_storage.get_or_allocate_aid_for_entity_id("remote.has_no_unique_id") + == 1872038229 + ) + + aid_storage.delete_aid(get_system_unique_id(light_ent)) + aid_storage.delete_aid(get_system_unique_id(light_ent2)) + aid_storage.delete_aid(get_system_unique_id(remote_ent)) + aid_storage.delete_aid("non-existant-one") + + for _ in range(0, 2): + assert ( + aid_storage.get_or_allocate_aid_for_entity_id(light_ent.entity_id) + == 1692141785 + ) + assert ( + aid_storage.get_or_allocate_aid_for_entity_id(light_ent2.entity_id) + == 2732133210 + ) + assert ( + aid_storage.get_or_allocate_aid_for_entity_id(remote_ent.entity_id) + == 1867188557 + ) + assert ( + aid_storage.get_or_allocate_aid_for_entity_id("remote.has_no_unique_id") + == 1872038229 + ) + + +async def test_aid_adler32_collision(hass, device_reg, entity_reg): + """Test generating aids.""" + config_entry = MockConfigEntry(domain="test", data={}) + config_entry.add_to_hass(hass) + device_entry = device_reg.async_get_or_create( + config_entry_id=config_entry.entry_id, + connections={(device_registry.CONNECTION_NETWORK_MAC, "12:34:56:AB:CD:EF")}, + ) + + with patch( + "homeassistant.components.homekit.aidmanager.AccessoryAidStorage.async_schedule_save" + ): + aid_storage = AccessoryAidStorage(hass) + await aid_storage.async_initialize() + + seen_aids = set() + + for unique_id in range(0, 202): + ent = entity_reg.async_get_or_create( + "light", "device", unique_id, device_id=device_entry.id + ) + hass.states.async_set(ent.entity_id, "on") + aid = aid_storage.get_or_allocate_aid_for_entity_id(ent.entity_id) + assert aid not in seen_aids + seen_aids.add(aid) diff --git a/tests/components/homekit/test_homekit.py b/tests/components/homekit/test_homekit.py index 721d25d09a0..d5ae720dbc3 100644 --- a/tests/components/homekit/test_homekit.py +++ b/tests/components/homekit/test_homekit.py @@ -12,10 +12,10 @@ from homeassistant.components.homekit import ( STATUS_STOPPED, STATUS_WAIT, HomeKit, - generate_aid, ) from homeassistant.components.homekit.accessories import HomeBridge from homeassistant.components.homekit.const import ( + AID_STORAGE, BRIDGE_NAME, CONF_AUTO_START, CONF_SAFE_MODE, @@ -51,17 +51,6 @@ def debounce_patcher(): patcher.stop() -def test_generate_aid(): - """Test generate aid method.""" - aid = generate_aid("demo.entity") - assert isinstance(aid, int) - assert aid >= 2 and aid <= 18446744073709551615 - - with patch(f"{PATH_HOMEKIT}.adler32") as mock_adler32: - mock_adler32.side_effect = [0, 1] - assert generate_aid("demo.entity") is None - - async def test_setup_min(hass): """Test async_setup with min config options.""" with patch(f"{PATH_HOMEKIT}.HomeKit") as mock_homekit: @@ -223,29 +212,31 @@ async def test_homekit_setup_safe_mode(hass, hk_driver): assert homekit.driver.safe_mode is True -async def test_homekit_add_accessory(): +async def test_homekit_add_accessory(hass): """Add accessory if config exists and get_acc returns an accessory.""" - homekit = HomeKit("hass", None, None, None, lambda entity_id: True, {}, None, None) + homekit = HomeKit(hass, None, None, None, lambda entity_id: True, {}, None, None) homekit.driver = "driver" homekit.bridge = mock_bridge = Mock() + homekit.bridge.accessories = range(10) + + assert await setup.async_setup_component(hass, DOMAIN, {DOMAIN: {}}) with patch(f"{PATH_HOMEKIT}.get_accessory") as mock_get_acc: - mock_get_acc.side_effect = [None, "acc", None] homekit.add_bridge_accessory(State("light.demo", "on")) - mock_get_acc.assert_called_with("hass", "driver", ANY, 363398124, {}) + mock_get_acc.assert_called_with(hass, "driver", ANY, 363398124, {}) assert not mock_bridge.add_accessory.called homekit.add_bridge_accessory(State("demo.test", "on")) - mock_get_acc.assert_called_with("hass", "driver", ANY, 294192020, {}) + mock_get_acc.assert_called_with(hass, "driver", ANY, 294192020, {}) assert mock_bridge.add_accessory.called homekit.add_bridge_accessory(State("demo.test_2", "on")) - mock_get_acc.assert_called_with("hass", "driver", ANY, 429982757, {}) + mock_get_acc.assert_called_with(hass, "driver", ANY, 429982757, {}) mock_bridge.add_accessory.assert_called_with("acc") -async def test_homekit_remove_accessory(): +async def test_homekit_remove_accessory(hass): """Remove accessory from bridge.""" homekit = HomeKit("hass", None, None, None, lambda entity_id: True, {}, None, None) homekit.driver = "driver" @@ -259,8 +250,12 @@ async def test_homekit_remove_accessory(): async def test_homekit_entity_filter(hass): """Test the entity filter.""" + assert await setup.async_setup_component(hass, DOMAIN, {DOMAIN: {}}) + entity_filter = generate_filter(["cover"], ["demo.test"], [], []) homekit = HomeKit(hass, None, None, None, entity_filter, {}, None, None) + homekit.bridge = Mock() + homekit.bridge.accessories = {} with patch(f"{PATH_HOMEKIT}.get_accessory") as mock_get_acc: mock_get_acc.return_value = None @@ -314,6 +309,8 @@ async def test_homekit_start_with_a_broken_accessory(hass, hk_driver, debounce_p pin = b"123-45-678" entity_filter = generate_filter(["cover", "light"], ["demo.test"], [], []) + assert await setup.async_setup_component(hass, DOMAIN, {DOMAIN: {}}) + homekit = HomeKit(hass, None, None, None, entity_filter, {}, None, None) homekit.bridge = Mock() homekit.bridge.accessories = [] @@ -366,6 +363,7 @@ async def test_homekit_reset_accessories(hass): entity_id = "light.demo" homekit = HomeKit(hass, None, None, None, {}, {entity_id: {}}, None) homekit.bridge = Mock() + homekit.bridge.accessories = {} with patch(f"{PATH_HOMEKIT}.HomeKit", return_value=homekit), patch( f"{PATH_HOMEKIT}.HomeKit.setup" @@ -375,7 +373,7 @@ async def test_homekit_reset_accessories(hass): assert await setup.async_setup_component(hass, DOMAIN, {DOMAIN: {}}) - aid = generate_aid(entity_id) + aid = hass.data[AID_STORAGE].get_or_allocate_aid_for_entity_id(entity_id) homekit.bridge.accessories = {aid: "acc"} homekit.status = STATUS_RUNNING @@ -394,11 +392,17 @@ async def test_homekit_reset_accessories(hass): async def test_homekit_too_many_accessories(hass, hk_driver): """Test adding too many accessories to HomeKit.""" - homekit = HomeKit(hass, None, None, None, None, None, None) + + entity_filter = generate_filter(["cover", "light"], ["demo.test"], [], []) + + homekit = HomeKit(hass, None, None, None, entity_filter, {}, None, None) homekit.bridge = Mock() - homekit.bridge.accessories = range(MAX_DEVICES + 1) + # The bridge itself counts as an accessory + homekit.bridge.accessories = range(MAX_DEVICES) homekit.driver = hk_driver + hass.states.async_set("light.demo", "on") + with patch("pyhap.accessory_driver.AccessoryDriver.start"), patch( "pyhap.accessory_driver.AccessoryDriver.add_accessory" ), patch("homeassistant.components.homekit._LOGGER.warning") as mock_warn: