From 5e3e4bda282491740c4ab81eb53cce71a95d7e22 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 15 Apr 2020 21:40:38 -0500 Subject: [PATCH] Prevent a single accessory setup failure from breaking all HomeKit accessories (#34263) * Prevent a single accessory setup failure from breaking all HomeKit accessories Raise the max devices to 150 as the spec allows for this many. Previously 100 made sense because of the event storms when homekit started would sometimes break pairing, as these have largely been fixed in 0.109 (still a few to cleanup) using the HAP spec limit of 150 is now possible. * Handle both failure states --- homeassistant/components/homekit/__init__.py | 16 +++++++--- tests/components/homekit/test_homekit.py | 33 ++++++++++++++++++++ 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/homeassistant/components/homekit/__init__.py b/homeassistant/components/homekit/__init__.py index c2ba5a46525..aeb8cb7e4b8 100644 --- a/homeassistant/components/homekit/__init__.py +++ b/homeassistant/components/homekit/__init__.py @@ -67,7 +67,7 @@ from .util import ( _LOGGER = logging.getLogger(__name__) -MAX_DEVICES = 100 +MAX_DEVICES = 150 TYPES = Registry() # #### Driver Status #### @@ -361,9 +361,17 @@ class HomeKit: return aid = generate_aid(state.entity_id) conf = self._config.pop(state.entity_id, {}) - acc = get_accessory(self.hass, self.driver, state, aid, conf) - if acc is not None: - self.bridge.add_accessory(acc) + # If an accessory cannot be created or added due to an exception + # of any kind (usually in pyhap) it should not prevent + # the rest of the accessories from being created + try: + acc = get_accessory(self.hass, self.driver, state, aid, conf) + if acc is not None: + self.bridge.add_accessory(acc) + except Exception: # pylint: disable=broad-except + _LOGGER.exception( + "Failed to create a HomeKit accessory for %s", state.entity_id + ) def remove_bridge_accessory(self, aid): """Try adding accessory to bridge if configured beforehand.""" diff --git a/tests/components/homekit/test_homekit.py b/tests/components/homekit/test_homekit.py index 124366ba241..721d25d09a0 100644 --- a/tests/components/homekit/test_homekit.py +++ b/tests/components/homekit/test_homekit.py @@ -309,6 +309,39 @@ async def test_homekit_start(hass, hk_driver, debounce_patcher): assert not hk_driver_start.called +async def test_homekit_start_with_a_broken_accessory(hass, hk_driver, debounce_patcher): + """Test HomeKit start method.""" + pin = b"123-45-678" + entity_filter = generate_filter(["cover", "light"], ["demo.test"], [], []) + + homekit = HomeKit(hass, None, None, None, entity_filter, {}, None, None) + homekit.bridge = Mock() + homekit.bridge.accessories = [] + homekit.driver = hk_driver + + hass.states.async_set("light.demo", "on") + hass.states.async_set("light.broken", "on") + + with patch(f"{PATH_HOMEKIT}.get_accessory", side_effect=Exception), patch( + f"{PATH_HOMEKIT}.show_setup_message" + ) as mock_setup_msg, patch( + "pyhap.accessory_driver.AccessoryDriver.add_accessory", + ) as hk_driver_add_acc, patch( + "pyhap.accessory_driver.AccessoryDriver.start" + ) as hk_driver_start: + await hass.async_add_executor_job(homekit.start) + + mock_setup_msg.assert_called_with(hass, pin) + hk_driver_add_acc.assert_called_with(homekit.bridge) + assert hk_driver_start.called + assert homekit.status == STATUS_RUNNING + + # Test start() if already started + hk_driver_start.reset_mock() + await hass.async_add_executor_job(homekit.start) + assert not hk_driver_start.called + + async def test_homekit_stop(hass): """Test HomeKit stop method.""" homekit = HomeKit(hass, None, None, None, None, None, None)