From 5581f58aadbbd6adadb1a424aebe192e65a9a68d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" <nick@koston.org> Date: Wed, 27 Oct 2021 06:24:57 -0500 Subject: [PATCH] Handle accessories without a serial number in homekit_controller (#58498) --- .../components/homekit_controller/__init__.py | 35 +- .../homekit_controller/connection.py | 37 +- .../components/homekit_controller/const.py | 4 + .../test_ryse_smart_bridge.py | 73 +++ .../homekit_controller/ryse_smart_bridge.json | 596 ++++++++++++++++++ 5 files changed, 726 insertions(+), 19 deletions(-) create mode 100644 tests/components/homekit_controller/specific_devices/test_ryse_smart_bridge.py create mode 100644 tests/fixtures/homekit_controller/ryse_smart_bridge.json diff --git a/homeassistant/components/homekit_controller/__init__.py b/homeassistant/components/homekit_controller/__init__.py index d80b849bf80..bab989ba9bc 100644 --- a/homeassistant/components/homekit_controller/__init__.py +++ b/homeassistant/components/homekit_controller/__init__.py @@ -20,7 +20,15 @@ from homeassistant.helpers.entity import DeviceInfo, Entity from .config_flow import normalize_hkid from .connection import HKDevice -from .const import CONTROLLER, DOMAIN, ENTITY_MAP, KNOWN_DEVICES, TRIGGERS +from .const import ( + CONTROLLER, + DOMAIN, + ENTITY_MAP, + IDENTIFIER_ACCESSORY_ID, + IDENTIFIER_SERIAL_NUMBER, + KNOWN_DEVICES, + TRIGGERS, +) from .storage import EntityMapStorage @@ -131,8 +139,12 @@ class HomeKitEntity(Entity): @property def unique_id(self) -> str: """Return the ID of this device.""" - serial = self.accessory_info.value(CharacteristicsTypes.SERIAL_NUMBER) - return f"homekit-{serial}-{self._iid}" + info = self.accessory_info + serial = info.value(CharacteristicsTypes.SERIAL_NUMBER) + if serial: + return f"homekit-{serial}-{self._iid}" + # Some accessories do not have a serial number + return f"homekit-{self._accessory.unique_id}-{self._aid}-{self._iid}" @property def name(self) -> str: @@ -149,9 +161,18 @@ class HomeKitEntity(Entity): """Return the device info.""" info = self.accessory_info accessory_serial = info.value(CharacteristicsTypes.SERIAL_NUMBER) + if accessory_serial: + # Some accessories do not have a serial number + identifier = (DOMAIN, IDENTIFIER_SERIAL_NUMBER, accessory_serial) + else: + identifier = ( + DOMAIN, + IDENTIFIER_ACCESSORY_ID, + f"{self._accessory.unique_id}_{self._aid}", + ) device_info = DeviceInfo( - identifiers={(DOMAIN, "serial-number", accessory_serial)}, + identifiers={identifier}, manufacturer=info.value(CharacteristicsTypes.MANUFACTURER, ""), model=info.value(CharacteristicsTypes.MODEL, ""), name=info.value(CharacteristicsTypes.NAME), @@ -162,7 +183,11 @@ class HomeKitEntity(Entity): # via_device otherwise it would be self referential. bridge_serial = self._accessory.connection_info["serial-number"] if accessory_serial != bridge_serial: - device_info[ATTR_VIA_DEVICE] = (DOMAIN, "serial-number", bridge_serial) + device_info[ATTR_VIA_DEVICE] = ( + DOMAIN, + IDENTIFIER_SERIAL_NUMBER, + bridge_serial, + ) return device_info diff --git a/homeassistant/components/homekit_controller/connection.py b/homeassistant/components/homekit_controller/connection.py index 12ea3b8a397..cf8381a9a28 100644 --- a/homeassistant/components/homekit_controller/connection.py +++ b/homeassistant/components/homekit_controller/connection.py @@ -12,7 +12,7 @@ from aiohomekit.model import Accessories from aiohomekit.model.characteristics import CharacteristicsTypes from aiohomekit.model.services import ServicesTypes -from homeassistant.const import ATTR_IDENTIFIERS, ATTR_VIA_DEVICE +from homeassistant.const import ATTR_VIA_DEVICE from homeassistant.core import callback from homeassistant.helpers import device_registry as dr from homeassistant.helpers.entity import DeviceInfo @@ -24,6 +24,8 @@ from .const import ( DOMAIN, ENTITY_MAP, HOMEKIT_ACCESSORY_DISPATCH, + IDENTIFIER_ACCESSORY_ID, + IDENTIFIER_SERIAL_NUMBER, ) from .device_trigger import async_fire_triggers, async_setup_triggers_for_entry @@ -207,33 +209,40 @@ class HKDevice: service_type=ServicesTypes.ACCESSORY_INFORMATION, ) - device_info = DeviceInfo( - identifiers={ + serial_number = info.value(CharacteristicsTypes.SERIAL_NUMBER) + + if serial_number: + identifiers = {(DOMAIN, IDENTIFIER_SERIAL_NUMBER, serial_number)} + else: + # Some accessories do not have a serial number + identifiers = { ( DOMAIN, - "serial-number", - info.value(CharacteristicsTypes.SERIAL_NUMBER), + IDENTIFIER_ACCESSORY_ID, + f"{self.unique_id}_{accessory.aid}", ) - }, + } + + if accessory.aid == 1: + # Accessory 1 is the root device (sometimes the only device, sometimes a bridge) + # Link the root device to the pairing id for the connection. + identifiers.add((DOMAIN, IDENTIFIER_ACCESSORY_ID, self.unique_id)) + + device_info = DeviceInfo( + identifiers=identifiers, name=info.value(CharacteristicsTypes.NAME), manufacturer=info.value(CharacteristicsTypes.MANUFACTURER, ""), model=info.value(CharacteristicsTypes.MODEL, ""), sw_version=info.value(CharacteristicsTypes.FIRMWARE_REVISION, ""), ) - if accessory.aid == 1: - # Accessory 1 is the root device (sometimes the only device, sometimes a bridge) - # Link the root device to the pairing id for the connection. - device_info[ATTR_IDENTIFIERS].add( - (DOMAIN, "accessory-id", self.unique_id) - ) - else: + if accessory.aid != 1: # Every pairing has an accessory 1 # It *doesn't* have a via_device, as it is the device we are connecting to # Every other accessory should use it as its via device. device_info[ATTR_VIA_DEVICE] = ( DOMAIN, - "serial-number", + IDENTIFIER_SERIAL_NUMBER, self.connection_info["serial-number"], ) diff --git a/homeassistant/components/homekit_controller/const.py b/homeassistant/components/homekit_controller/const.py index fa28bab7606..3c9372f96db 100644 --- a/homeassistant/components/homekit_controller/const.py +++ b/homeassistant/components/homekit_controller/const.py @@ -11,6 +11,10 @@ TRIGGERS = f"{DOMAIN}-triggers" HOMEKIT_DIR = ".homekit" PAIRING_FILE = "pairing.json" +IDENTIFIER_SERIAL_NUMBER = "serial-number" +IDENTIFIER_ACCESSORY_ID = "accessory-id" + + # Mapping from Homekit type to component. HOMEKIT_ACCESSORY_DISPATCH = { "lightbulb": "light", diff --git a/tests/components/homekit_controller/specific_devices/test_ryse_smart_bridge.py b/tests/components/homekit_controller/specific_devices/test_ryse_smart_bridge.py new file mode 100644 index 00000000000..8430919297c --- /dev/null +++ b/tests/components/homekit_controller/specific_devices/test_ryse_smart_bridge.py @@ -0,0 +1,73 @@ +"""Test against characteristics captured from a ryse smart bridge platforms.""" + +from homeassistant.helpers import device_registry as dr, entity_registry as er + +from tests.components.homekit_controller.common import ( + Helper, + setup_accessories_from_file, + setup_test_accessories, +) + + +async def test_ryse_smart_bridge_setup(hass): + """Test that a Ryse smart bridge can be correctly setup in HA.""" + accessories = await setup_accessories_from_file(hass, "ryse_smart_bridge.json") + config_entry, pairing = await setup_test_accessories(hass, accessories) + + entity_registry = er.async_get(hass) + + # Check that the cover.master_bath_south is correctly found and set up + cover_id = "cover.master_bath_south" + cover = entity_registry.async_get(cover_id) + assert cover.unique_id == "homekit-1.0.0-48" + + cover_helper = Helper( + hass, + cover_id, + pairing, + accessories[0], + config_entry, + ) + + cover_state = await cover_helper.poll_and_get_state() + assert cover_state.attributes["friendly_name"] == "Master Bath South" + assert cover_state.state == "closed" + + device_registry = dr.async_get(hass) + + device = device_registry.async_get(cover.device_id) + assert device.manufacturer == "RYSE Inc." + assert device.name == "Master Bath South" + assert device.model == "RYSE Shade" + assert device.sw_version == "3.0.8" + + bridge = device_registry.async_get(device.via_device_id) + assert bridge.manufacturer == "RYSE Inc." + assert bridge.name == "RYSE SmartBridge" + assert bridge.model == "RYSE SmartBridge" + assert bridge.sw_version == "1.3.0" + + # Check that the cover.ryse_smartshade is correctly found and set up + cover_id = "cover.ryse_smartshade" + cover = entity_registry.async_get(cover_id) + assert cover.unique_id == "homekit-00:00:00:00:00:00-3-48" + + cover_helper = Helper( + hass, + cover_id, + pairing, + accessories[0], + config_entry, + ) + + cover_state = await cover_helper.poll_and_get_state() + assert cover_state.attributes["friendly_name"] == "RYSE SmartShade" + assert cover_state.state == "open" + + device_registry = dr.async_get(hass) + + device = device_registry.async_get(cover.device_id) + assert device.manufacturer == "RYSE Inc." + assert device.name == "RYSE SmartShade" + assert device.model == "RYSE Shade" + assert device.sw_version == "" diff --git a/tests/fixtures/homekit_controller/ryse_smart_bridge.json b/tests/fixtures/homekit_controller/ryse_smart_bridge.json new file mode 100644 index 00000000000..6f6f818e5e2 --- /dev/null +++ b/tests/fixtures/homekit_controller/ryse_smart_bridge.json @@ -0,0 +1,596 @@ +[ + { + "aid": 1, + "services": [ + { + "iid": 1, + "type": "0000003E-0000-1000-8000-0026BB765291", + "primary": false, + "hidden": false, + "linked": [], + "characteristics": [ + { + "iid": 2, + "type": "00000014-0000-1000-8000-0026BB765291", + "format": "bool", + "perms": [ + "pw" + ] + }, + { + "iid": 3, + "type": "00000020-0000-1000-8000-0026BB765291", + "format": "string", + "value": "RYSE Inc.", + "perms": [ + "pr" + ], + "ev": false + }, + { + "iid": 4, + "type": "00000021-0000-1000-8000-0026BB765291", + "format": "string", + "value": "RYSE SmartBridge", + "perms": [ + "pr" + ], + "ev": false + }, + { + "iid": 5, + "type": "00000023-0000-1000-8000-0026BB765291", + "format": "string", + "value": "RYSE SmartBridge", + "perms": [ + "pr" + ], + "ev": false + }, + { + "iid": 6, + "type": "00000030-0000-1000-8000-0026BB765291", + "format": "string", + "value": "0101.3521.0436", + "perms": [ + "pr" + ], + "ev": false + }, + { + "iid": 7, + "type": "00000052-0000-1000-8000-0026BB765291", + "format": "string", + "value": "1.3.0", + "perms": [ + "pr" + ], + "ev": false + }, + { + "iid": 8, + "type": "00000053-0000-1000-8000-0026BB765291", + "format": "string", + "value": "0101.3521.0436", + "perms": [ + "pr" + ], + "ev": false + }, + { + "iid": 9, + "type": "34AB8811-AC7F-4340-BAC3-FD6A85F9943B", + "format": "string", + "value": "4.1;3fac0fb4", + "perms": [ + "pr", + "hd" + ], + "ev": false + }, + { + "iid": 10, + "type": "220", + "format": "data", + "value": "Yhl9CmseEb8=", + "perms": [ + "pr", + "hd" + ], + "ev": false, + "maxDataLen": 8 + } + ] + }, + { + "iid": 16, + "type": "000000A2-0000-1000-8000-0026BB765291", + "primary": false, + "hidden": false, + "linked": [], + "characteristics": [ + { + "iid": 18, + "type": "00000037-0000-1000-8000-0026BB765291", + "format": "string", + "value": "1.1.0", + "perms": [ + "pr" + ], + "ev": false + } + ] + } + ] + }, + { + "aid": 2, + "services": [ + { + "iid": 1, + "type": "0000003E-0000-1000-8000-0026BB765291", + "primary": false, + "hidden": false, + "linked": [], + "characteristics": [ + { + "iid": 2, + "type": "00000014-0000-1000-8000-0026BB765291", + "format": "bool", + "perms": [ + "pw" + ] + }, + { + "iid": 3, + "type": "00000020-0000-1000-8000-0026BB765291", + "format": "string", + "value": "RYSE Inc.", + "perms": [ + "pr" + ], + "ev": false + }, + { + "iid": 4, + "type": "00000021-0000-1000-8000-0026BB765291", + "format": "string", + "value": "RYSE Shade", + "perms": [ + "pr" + ], + "ev": false + }, + { + "iid": 5, + "type": "00000023-0000-1000-8000-0026BB765291", + "format": "string", + "value": "Master Bath South", + "perms": [ + "pr" + ], + "ev": false + }, + { + "iid": 6, + "type": "00000030-0000-1000-8000-0026BB765291", + "format": "string", + "value": "1.0.0", + "perms": [ + "pr" + ], + "ev": false + }, + { + "iid": 7, + "type": "00000052-0000-1000-8000-0026BB765291", + "format": "string", + "value": "3.0.8", + "perms": [ + "pr" + ], + "ev": false + }, + { + "iid": 8, + "type": "00000053-0000-1000-8000-0026BB765291", + "format": "string", + "value": "1.0.0", + "perms": [ + "pr" + ], + "ev": false + }, + { + "iid": 11, + "type": "000000A6-0000-1000-8000-0026BB765291", + "format": "uint32", + "value": 0, + "perms": [ + "pr", + "ev" + ], + "ev": false, + "minValue": 0, + "maxValue": 1, + "minStep": 1 + } + ] + }, + { + "iid": 48, + "type": "0000008C-0000-1000-8000-0026BB765291", + "primary": true, + "hidden": false, + "linked": [ + 64 + ], + "characteristics": [ + { + "iid": 52, + "type": "0000007C-0000-1000-8000-0026BB765291", + "format": "uint8", + "value": 0, + "perms": [ + "pr", + "pw", + "ev" + ], + "ev": true, + "unit": "percentage", + "minValue": 0, + "maxValue": 100, + "minStep": 1 + }, + { + "iid": 53, + "type": "0000006D-0000-1000-8000-0026BB765291", + "format": "uint8", + "value": 0, + "perms": [ + "pr", + "ev" + ], + "ev": true, + "unit": "percentage", + "minValue": 0, + "maxValue": 100, + "minStep": 1 + }, + { + "iid": 54, + "type": "00000072-0000-1000-8000-0026BB765291", + "format": "uint8", + "value": 2, + "perms": [ + "pr", + "ev" + ], + "ev": true, + "minValue": 0, + "maxValue": 2, + "minStep": 1 + }, + { + "iid": 50, + "type": "00000023-0000-1000-8000-0026BB765291", + "format": "string", + "value": "RYSE Shade", + "perms": [ + "pr" + ], + "ev": false + }, + { + "iid": 55, + "type": "00000024-0000-1000-8000-0026BB765291", + "format": "bool", + "value": 0, + "perms": [ + "pr", + "ev" + ], + "ev": true + } + ] + }, + { + "iid": 64, + "type": "00000096-0000-1000-8000-0026BB765291", + "primary": false, + "hidden": false, + "linked": [], + "characteristics": [ + { + "iid": 67, + "type": "00000068-0000-1000-8000-0026BB765291", + "format": "uint8", + "value": 100, + "perms": [ + "pr", + "ev" + ], + "ev": true, + "unit": "percentage", + "minValue": 0, + "maxValue": 100, + "minStep": 1 + }, + { + "iid": 68, + "type": "0000008F-0000-1000-8000-0026BB765291", + "format": "uint8", + "value": 2, + "perms": [ + "pr", + "ev" + ], + "ev": true, + "minValue": 0, + "maxValue": 2, + "minStep": 1 + }, + { + "iid": 70, + "type": "00000079-0000-1000-8000-0026BB765291", + "format": "uint8", + "value": 0, + "perms": [ + "pr", + "ev" + ], + "ev": true, + "minValue": 0, + "maxValue": 1, + "minStep": 1 + }, + { + "iid": 66, + "type": "00000023-0000-1000-8000-0026BB765291", + "format": "string", + "value": "RYSE Shade", + "perms": [ + "pr" + ], + "ev": false + } + ] + } + ] + }, + { + "aid": 3, + "services": [ + { + "iid": 1, + "type": "0000003E-0000-1000-8000-0026BB765291", + "primary": false, + "hidden": false, + "linked": [], + "characteristics": [ + { + "iid": 2, + "type": "00000014-0000-1000-8000-0026BB765291", + "format": "bool", + "perms": [ + "pw" + ] + }, + { + "iid": 3, + "type": "00000020-0000-1000-8000-0026BB765291", + "format": "string", + "value": "RYSE Inc.", + "perms": [ + "pr" + ], + "ev": false + }, + { + "iid": 4, + "type": "00000021-0000-1000-8000-0026BB765291", + "format": "string", + "value": "RYSE Shade", + "perms": [ + "pr" + ], + "ev": false + }, + { + "iid": 5, + "type": "00000023-0000-1000-8000-0026BB765291", + "format": "string", + "value": "RYSE SmartShade", + "perms": [ + "pr" + ], + "ev": false + }, + { + "iid": 6, + "type": "00000030-0000-1000-8000-0026BB765291", + "format": "string", + "value": "", + "perms": [ + "pr" + ], + "ev": false + }, + { + "iid": 7, + "type": "00000052-0000-1000-8000-0026BB765291", + "format": "string", + "value": "", + "perms": [ + "pr" + ], + "ev": false + }, + { + "iid": 8, + "type": "00000053-0000-1000-8000-0026BB765291", + "format": "string", + "value": "", + "perms": [ + "pr" + ], + "ev": false + }, + { + "iid": 11, + "type": "000000A6-0000-1000-8000-0026BB765291", + "format": "uint32", + "value": 1, + "perms": [ + "pr", + "ev" + ], + "ev": false, + "minValue": 0, + "maxValue": 1, + "minStep": 1 + } + ] + }, + { + "iid": 48, + "type": "0000008C-0000-1000-8000-0026BB765291", + "primary": true, + "hidden": false, + "linked": [ + 64 + ], + "characteristics": [ + { + "iid": 52, + "type": "0000007C-0000-1000-8000-0026BB765291", + "format": "uint8", + "value": 100, + "perms": [ + "pr", + "pw", + "ev" + ], + "ev": false, + "unit": "percentage", + "minValue": 0, + "maxValue": 100, + "minStep": 1 + }, + { + "iid": 53, + "type": "0000006D-0000-1000-8000-0026BB765291", + "format": "uint8", + "value": 100, + "perms": [ + "pr", + "ev" + ], + "ev": false, + "unit": "percentage", + "minValue": 0, + "maxValue": 100, + "minStep": 1 + }, + { + "iid": 54, + "type": "00000072-0000-1000-8000-0026BB765291", + "format": "uint8", + "value": 2, + "perms": [ + "pr", + "ev" + ], + "ev": false, + "minValue": 0, + "maxValue": 2, + "minStep": 1 + }, + { + "iid": 50, + "type": "00000023-0000-1000-8000-0026BB765291", + "format": "string", + "value": "RYSE Shade", + "perms": [ + "pr" + ], + "ev": false + }, + { + "iid": 55, + "type": "00000024-0000-1000-8000-0026BB765291", + "format": "bool", + "value": 0, + "perms": [ + "pr", + "ev" + ], + "ev": false + } + ] + }, + { + "iid": 64, + "type": "00000096-0000-1000-8000-0026BB765291", + "primary": false, + "hidden": false, + "linked": [], + "characteristics": [ + { + "iid": 67, + "type": "00000068-0000-1000-8000-0026BB765291", + "format": "uint8", + "value": 100, + "perms": [ + "pr", + "ev" + ], + "ev": false, + "unit": "percentage", + "minValue": 0, + "maxValue": 100, + "minStep": 1 + }, + { + "iid": 68, + "type": "0000008F-0000-1000-8000-0026BB765291", + "format": "uint8", + "value": 2, + "perms": [ + "pr", + "ev" + ], + "ev": false, + "minValue": 0, + "maxValue": 2, + "minStep": 1 + }, + { + "iid": 70, + "type": "00000079-0000-1000-8000-0026BB765291", + "format": "uint8", + "value": 0, + "perms": [ + "pr", + "ev" + ], + "ev": false, + "minValue": 0, + "maxValue": 1, + "minStep": 1 + }, + { + "iid": 66, + "type": "00000023-0000-1000-8000-0026BB765291", + "format": "string", + "value": "RYSE Shade", + "perms": [ + "pr" + ], + "ev": false + } + ] + } + ] + } +] \ No newline at end of file -- GitLab