From f5b389faa8ecf51f248005ed3dcbaa7ae4a15a2f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 10 Jan 2021 09:38:41 -1000 Subject: [PATCH] Warn users when their HomeKit configuration may be unstable (#44999) --- homeassistant/components/homekit/__init__.py | 20 ++++- homeassistant/components/homekit/strings.json | 4 +- .../components/homekit/translations/en.json | 83 +++++++++---------- tests/components/homekit/test_homekit.py | 51 +++++++++++- 4 files changed, 109 insertions(+), 49 deletions(-) diff --git a/homeassistant/components/homekit/__init__.py b/homeassistant/components/homekit/__init__.py index 1155d3ef18e..2b11aee0bbd 100644 --- a/homeassistant/components/homekit/__init__.py +++ b/homeassistant/components/homekit/__init__.py @@ -5,7 +5,7 @@ import logging import os from aiohttp import web -from pyhap.const import STANDALONE_AID +from pyhap.const import CATEGORY_CAMERA, CATEGORY_TELEVISION, STANDALONE_AID import voluptuous as vol from homeassistant.components import zeroconf @@ -530,6 +530,24 @@ class HomeKit: try: acc = get_accessory(self.hass, self.driver, state, aid, conf) if acc is not None: + if acc.category == CATEGORY_CAMERA: + _LOGGER.warning( + "The bridge %s has camera %s. For best performance, " + "and to prevent unexpected unavailability, create and " + "pair a separate HomeKit instance in accessory mode for " + "each camera.", + self._name, + acc.entity_id, + ) + elif acc.category == CATEGORY_TELEVISION: + _LOGGER.warning( + "The bridge %s has tv %s. For best performance, " + "and to prevent unexpected unavailability, create and " + "pair a separate HomeKit instance in accessory mode for " + "each tv media player.", + self._name, + acc.entity_id, + ) self.bridge.add_accessory(acc) except Exception: # pylint: disable=broad-except _LOGGER.exception( diff --git a/homeassistant/components/homekit/strings.json b/homeassistant/components/homekit/strings.json index 3f12eca0f5f..9e3413e1d20 100644 --- a/homeassistant/components/homekit/strings.json +++ b/homeassistant/components/homekit/strings.json @@ -18,7 +18,7 @@ "mode": "[%key:common::config_flow::data::mode%]", "entities": "Entities" }, - "description": "Choose the entities to be exposed. In accessory mode, only a single entity is exposed. In bridge include mode, all entities in the domain will be exposed unless specific entities are selected. In bridge exclude mode, all entities in the domain will be exposed except for the excluded entities.", + "description": "Choose the entities to be exposed. In accessory mode, only a single entity is exposed. In bridge include mode, all entities in the domain will be exposed unless specific entities are selected. In bridge exclude mode, all entities in the domain will be exposed except for the excluded entities. For best performance, and to prevent unexpected unavailability, create and pair a separate HomeKit instance in accessory mode for each tv media player and camera.", "title": "Select entities to be exposed" }, "cameras": { @@ -44,7 +44,7 @@ "data": { "include_domains": "Domains to include" }, - "description": "The HomeKit integration will allow you to access your Home Assistant entities in HomeKit. In bridge mode, HomeKit Bridges are limited to 150 accessories per instance including the bridge itself. If you wish to bridge more than the maximum number of accessories, it is recommended that you use multiple HomeKit bridges for different domains. Detailed entity configuration is only available via YAML for the primary bridge.", + "description": "The HomeKit integration will allow you to access your Home Assistant entities in HomeKit. In bridge mode, HomeKit Bridges are limited to 150 accessories per instance including the bridge itself. If you wish to bridge more than the maximum number of accessories, it is recommended that you use multiple HomeKit bridges for different domains. Detailed entity configuration is only available via YAML. For best performance, and to prevent unexpected unavailability, create and pair a separate HomeKit instance in accessory mode for each tv media player and camera.", "title": "Activate HomeKit" }, "pairing": { diff --git a/homeassistant/components/homekit/translations/en.json b/homeassistant/components/homekit/translations/en.json index 39aa3522025..9e3413e1d20 100644 --- a/homeassistant/components/homekit/translations/en.json +++ b/homeassistant/components/homekit/translations/en.json @@ -1,32 +1,25 @@ { - "config": { - "abort": { - "port_name_in_use": "An accessory or bridge with the same name or port is already configured." - }, - "step": { - "pairing": { - "description": "As soon as the {name} is ready, pairing will be available in \u201cNotifications\u201d as \u201cHomeKit Bridge Setup\u201d.", - "title": "Pair HomeKit" - }, - "user": { - "data": { - "auto_start": "Autostart (disable if using Z-Wave or other delayed start system)", - "include_domains": "Domains to include" - }, - "description": "The HomeKit integration will allow you to access your Home Assistant entities in HomeKit. In bridge mode, HomeKit Bridges are limited to 150 accessories per instance including the bridge itself. If you wish to bridge more than the maximum number of accessories, it is recommended that you use multiple HomeKit bridges for different domains. Detailed entity configuration is only available via YAML for the primary bridge.", - "title": "Activate HomeKit" - } - } - }, "options": { "step": { - "advanced": { + "yaml": { + "title": "Adjust HomeKit Options", + "description": "This entry is controlled via YAML" + }, + "init": { "data": { - "auto_start": "Autostart (disable if using Z-Wave or other delayed start system)", - "safe_mode": "Safe Mode (enable only if pairing fails)" + "mode": "[%key:common::config_flow::data::mode%]", + "include_domains": "[%key:component::homekit::config::step::user::data::include_domains%]" }, - "description": "These settings only need to be adjusted if HomeKit is not functional.", - "title": "Advanced Configuration" + "description": "HomeKit can be configured expose a bridge or a single accessory. In accessory mode, only a single entity can be used. Accessory mode is required for media players with the TV device class to function properly. Entities in the \u201cDomains to include\u201d will be exposed to HomeKit. You will be able to select which entities to include or exclude from this list on the next screen.", + "title": "Select domains to expose." + }, + "include_exclude": { + "data": { + "mode": "[%key:common::config_flow::data::mode%]", + "entities": "Entities" + }, + "description": "Choose the entities to be exposed. In accessory mode, only a single entity is exposed. In bridge include mode, all entities in the domain will be exposed unless specific entities are selected. In bridge exclude mode, all entities in the domain will be exposed except for the excluded entities. For best performance, and to prevent unexpected unavailability, create and pair a separate HomeKit instance in accessory mode for each tv media player and camera.", + "title": "Select entities to be exposed" }, "cameras": { "data": { @@ -35,26 +28,32 @@ "description": "Check all cameras that support native H.264 streams. If the camera does not output a H.264 stream, the system will transcode the video to H.264 for HomeKit. Transcoding requires a performant CPU and is unlikely to work on single board computers.", "title": "Select camera video codec." }, - "include_exclude": { + "advanced": { "data": { - "entities": "Entities", - "mode": "Mode" + "auto_start": "Autostart (disable if you are calling the homekit.start service manually)", + "safe_mode": "Safe Mode (enable only if pairing fails)" }, - "description": "Choose the entities to be exposed. In accessory mode, only a single entity is exposed. In bridge include mode, all entities in the domain will be exposed unless specific entities are selected. In bridge exclude mode, all entities in the domain will be exposed except for the excluded entities.", - "title": "Select entities to be exposed" - }, - "init": { - "data": { - "include_domains": "Domains to include", - "mode": "Mode" - }, - "description": "HomeKit can be configured expose a bridge or a single accessory. In accessory mode, only a single entity can be used. Accessory mode is required for media players with the TV device class to function properly. Entities in the \u201cDomains to include\u201d will be exposed to HomeKit. You will be able to select which entities to include or exclude from this list on the next screen.", - "title": "Select domains to expose." - }, - "yaml": { - "description": "This entry is controlled via YAML", - "title": "Adjust HomeKit Options" + "description": "These settings only need to be adjusted if HomeKit is not functional.", + "title": "Advanced Configuration" } } + }, + "config": { + "step": { + "user": { + "data": { + "include_domains": "Domains to include" + }, + "description": "The HomeKit integration will allow you to access your Home Assistant entities in HomeKit. In bridge mode, HomeKit Bridges are limited to 150 accessories per instance including the bridge itself. If you wish to bridge more than the maximum number of accessories, it is recommended that you use multiple HomeKit bridges for different domains. Detailed entity configuration is only available via YAML. For best performance, and to prevent unexpected unavailability, create and pair a separate HomeKit instance in accessory mode for each tv media player and camera.", + "title": "Activate HomeKit" + }, + "pairing": { + "title": "Pair HomeKit", + "description": "As soon as the {name} is ready, pairing will be available in \u201cNotifications\u201d as \u201cHomeKit Bridge Setup\u201d." + } + }, + "abort": { + "port_name_in_use": "An accessory or bridge with the same name or port is already configured." + } } -} \ No newline at end of file +} diff --git a/tests/components/homekit/test_homekit.py b/tests/components/homekit/test_homekit.py index 32d8a69417b..a3f6c1f57d2 100644 --- a/tests/components/homekit/test_homekit.py +++ b/tests/components/homekit/test_homekit.py @@ -4,6 +4,7 @@ from typing import Dict from unittest.mock import ANY, AsyncMock, MagicMock, Mock, patch from pyhap.accessory import Accessory +from pyhap.const import CATEGORY_CAMERA, CATEGORY_TELEVISION import pytest from homeassistant import config as hass_config @@ -340,7 +341,7 @@ async def test_homekit_setup_safe_mode(hass, hk_driver): assert homekit.driver.safe_mode is True -async def test_homekit_add_accessory(hass): +async def test_homekit_add_accessory(hass, mock_zeroconf): """Add accessory if config exists and get_acc returns an accessory.""" entry = await async_init_integration(hass) @@ -360,10 +361,12 @@ async def test_homekit_add_accessory(hass): homekit.bridge = mock_bridge = Mock() homekit.bridge.accessories = range(10) + mock_acc = Mock(category="any") + await async_init_integration(hass) with patch(f"{PATH_HOMEKIT}.get_accessory") as mock_get_acc: - mock_get_acc.side_effect = [None, "acc", None] + mock_get_acc.side_effect = [None, mock_acc, None] homekit.add_bridge_accessory(State("light.demo", "on")) mock_get_acc.assert_called_with(hass, "driver", ANY, 1403373688, {}) assert not mock_bridge.add_accessory.called @@ -374,10 +377,50 @@ async def test_homekit_add_accessory(hass): homekit.add_bridge_accessory(State("demo.test_2", "on")) mock_get_acc.assert_called_with(hass, "driver", ANY, 1467253281, {}) - mock_bridge.add_accessory.assert_called_with("acc") + mock_bridge.add_accessory.assert_called_with(mock_acc) -async def test_homekit_remove_accessory(hass): +@pytest.mark.parametrize("acc_category", [CATEGORY_TELEVISION, CATEGORY_CAMERA]) +async def test_homekit_warn_add_accessory_bridge( + hass, acc_category, mock_zeroconf, caplog +): + """Test we warn when adding cameras or tvs to a bridge.""" + entry = await async_init_integration(hass) + + homekit = HomeKit( + hass, + None, + None, + None, + lambda entity_id: True, + {}, + DEFAULT_SAFE_MODE, + HOMEKIT_MODE_BRIDGE, + advertise_ip=None, + entry_id=entry.entry_id, + ) + homekit.driver = "driver" + homekit.bridge = mock_bridge = Mock() + homekit.bridge.accessories = range(10) + + mock_camera_acc = Mock(category=acc_category) + + await async_init_integration(hass) + + with patch(f"{PATH_HOMEKIT}.get_accessory") as mock_get_acc: + mock_get_acc.side_effect = [None, mock_camera_acc, None] + homekit.add_bridge_accessory(State("light.demo", "on")) + mock_get_acc.assert_called_with(hass, "driver", ANY, 1403373688, {}) + assert not mock_bridge.add_accessory.called + + homekit.add_bridge_accessory(State("camera.test", "on")) + mock_get_acc.assert_called_with(hass, "driver", ANY, 1508819236, {}) + assert mock_bridge.add_accessory.called + + assert "accessory mode" in caplog.text + + +async def test_homekit_remove_accessory(hass, mock_zeroconf): """Remove accessory from bridge.""" entry = await async_init_integration(hass)