From 93fafedf7213d72115cf74196ea4df95bfb3700f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 8 Feb 2021 13:37:32 -1000 Subject: [PATCH] Cleanup bond identifiers and device info (#46192) --- homeassistant/components/bond/__init__.py | 26 ++++++++-- homeassistant/components/bond/const.py | 2 + homeassistant/components/bond/entity.py | 18 ++++++- homeassistant/components/bond/utils.py | 29 ++++++++++- tests/components/bond/test_init.py | 60 ++++++++++++++++++++++- 5 files changed, 126 insertions(+), 9 deletions(-) diff --git a/homeassistant/components/bond/__init__.py b/homeassistant/components/bond/__init__.py index 9af92f6e7e7..88ea084d25f 100644 --- a/homeassistant/components/bond/__init__.py +++ b/homeassistant/components/bond/__init__.py @@ -7,12 +7,12 @@ from bond_api import Bond from homeassistant.config_entries import ConfigEntry from homeassistant.const import CONF_ACCESS_TOKEN, CONF_HOST -from homeassistant.core import HomeAssistant +from homeassistant.core import HomeAssistant, callback from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.helpers import device_registry as dr from homeassistant.helpers.entity import SLOW_UPDATE_WARNING -from .const import DOMAIN +from .const import BRIDGE_MAKE, DOMAIN from .utils import BondHub PLATFORMS = ["cover", "fan", "light", "switch"] @@ -29,6 +29,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry): """Set up Bond from a config entry.""" host = entry.data[CONF_HOST] token = entry.data[CONF_ACCESS_TOKEN] + config_entry_id = entry.entry_id bond = Bond(host=host, token=token, timeout=ClientTimeout(total=_API_TIMEOUT)) hub = BondHub(bond) @@ -37,21 +38,23 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry): except (ClientError, AsyncIOTimeoutError, OSError) as error: raise ConfigEntryNotReady from error - hass.data[DOMAIN][entry.entry_id] = hub + hass.data[DOMAIN][config_entry_id] = hub if not entry.unique_id: hass.config_entries.async_update_entry(entry, unique_id=hub.bond_id) device_registry = await dr.async_get_registry(hass) device_registry.async_get_or_create( - config_entry_id=entry.entry_id, + config_entry_id=config_entry_id, identifiers={(DOMAIN, hub.bond_id)}, - manufacturer="Olibra", + manufacturer=BRIDGE_MAKE, name=hub.bond_id, model=hub.target, sw_version=hub.fw_ver, ) + _async_remove_old_device_identifiers(config_entry_id, device_registry, hub) + for component in PLATFORMS: hass.async_create_task( hass.config_entries.async_forward_entry_setup(entry, component) @@ -75,3 +78,16 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: hass.data[DOMAIN].pop(entry.entry_id) return unload_ok + + +@callback +def _async_remove_old_device_identifiers( + config_entry_id: str, device_registry: dr.DeviceRegistry, hub: BondHub +): + """Remove the non-unique device registry entries.""" + for device in hub.devices: + dev = device_registry.async_get_device(identifiers={(DOMAIN, device.device_id)}) + if dev is None: + continue + if config_entry_id in dev.config_entries: + device_registry.async_remove_device(dev.id) diff --git a/homeassistant/components/bond/const.py b/homeassistant/components/bond/const.py index 843c3f9f1dc..3031c159b0f 100644 --- a/homeassistant/components/bond/const.py +++ b/homeassistant/components/bond/const.py @@ -1,5 +1,7 @@ """Constants for the Bond integration.""" +BRIDGE_MAKE = "Olibra" + DOMAIN = "bond" CONF_BOND_ID: str = "bond_id" diff --git a/homeassistant/components/bond/entity.py b/homeassistant/components/bond/entity.py index 501d6574960..2819182c9b5 100644 --- a/homeassistant/components/bond/entity.py +++ b/homeassistant/components/bond/entity.py @@ -43,11 +43,25 @@ class BondEntity(Entity): @property def device_info(self) -> Optional[Dict[str, Any]]: """Get a an HA device representing this Bond controlled device.""" - return { + device_info = { ATTR_NAME: self.name, - "identifiers": {(DOMAIN, self._device.device_id)}, + "manufacturer": self._hub.make, + "identifiers": {(DOMAIN, self._hub.bond_id, self._device.device_id)}, "via_device": (DOMAIN, self._hub.bond_id), } + if not self._hub.is_bridge: + device_info["model"] = self._hub.model + device_info["sw_version"] = self._hub.fw_ver + else: + model_data = [] + if self._device.branding_profile: + model_data.append(self._device.branding_profile) + if self._device.template: + model_data.append(self._device.template) + if model_data: + device_info["model"] = " ".join(model_data) + + return device_info @property def assumed_state(self) -> bool: diff --git a/homeassistant/components/bond/utils.py b/homeassistant/components/bond/utils.py index 5a9fff692fa..df3373ed7a1 100644 --- a/homeassistant/components/bond/utils.py +++ b/homeassistant/components/bond/utils.py @@ -4,6 +4,8 @@ from typing import List, Optional from bond_api import Action, Bond +from .const import BRIDGE_MAKE + _LOGGER = logging.getLogger(__name__) @@ -34,6 +36,21 @@ class BondDevice: """Get the type of this device.""" return self._attrs["type"] + @property + def location(self) -> str: + """Get the location of this device.""" + return self._attrs["location"] + + @property + def template(self) -> str: + """Return this model template.""" + return self._attrs.get("template") + + @property + def branding_profile(self) -> str: + """Return this branding profile.""" + return self.props.get("branding_profile") + @property def trust_state(self) -> bool: """Check if Trust State is turned on.""" @@ -100,9 +117,19 @@ class BondHub: @property def target(self) -> str: - """Return this hub model.""" + """Return this hub target.""" return self._version.get("target") + @property + def model(self) -> str: + """Return this hub model.""" + return self._version.get("model") + + @property + def make(self) -> str: + """Return this hub make.""" + return self._version.get("make", BRIDGE_MAKE) + @property def fw_ver(self) -> str: """Return this hub firmware version.""" diff --git a/tests/components/bond/test_init.py b/tests/components/bond/test_init.py index 98d86058c49..e2bb6314126 100644 --- a/tests/components/bond/test_init.py +++ b/tests/components/bond/test_init.py @@ -1,5 +1,6 @@ """Tests for the Bond module.""" from aiohttp import ClientConnectionError +from bond_api import DeviceType from homeassistant.components.bond.const import DOMAIN from homeassistant.config_entries import ( @@ -12,7 +13,15 @@ from homeassistant.core import HomeAssistant from homeassistant.helpers import device_registry as dr from homeassistant.setup import async_setup_component -from .common import patch_bond_version, patch_setup_entry, setup_bond_entity +from .common import ( + patch_bond_device, + patch_bond_device_ids, + patch_bond_device_properties, + patch_bond_device_state, + patch_bond_version, + patch_setup_entry, + setup_bond_entity, +) from tests.common import MockConfigEntry @@ -105,3 +114,52 @@ async def test_unload_config_entry(hass: HomeAssistant): assert config_entry.entry_id not in hass.data[DOMAIN] assert config_entry.state == ENTRY_STATE_NOT_LOADED + + +async def test_old_identifiers_are_removed(hass: HomeAssistant): + """Test we remove the old non-unique identifiers.""" + config_entry = MockConfigEntry( + domain=DOMAIN, + data={CONF_HOST: "some host", CONF_ACCESS_TOKEN: "test-token"}, + ) + + old_identifers = (DOMAIN, "device_id") + new_identifiers = (DOMAIN, "test-bond-id", "device_id") + device_registry = await hass.helpers.device_registry.async_get_registry() + device_registry.async_get_or_create( + config_entry_id=config_entry.entry_id, + identifiers={old_identifers}, + manufacturer="any", + name="old", + ) + + config_entry.add_to_hass(hass) + + with patch_bond_version( + return_value={ + "bondid": "test-bond-id", + "target": "test-model", + "fw_ver": "test-version", + } + ), patch_bond_device_ids( + return_value=["bond-device-id", "device_id"] + ), patch_bond_device( + return_value={ + "name": "test1", + "type": DeviceType.GENERIC_DEVICE, + } + ), patch_bond_device_properties( + return_value={} + ), patch_bond_device_state( + return_value={} + ): + assert await hass.config_entries.async_setup(config_entry.entry_id) is True + await hass.async_block_till_done() + + assert config_entry.entry_id in hass.data[DOMAIN] + assert config_entry.state == ENTRY_STATE_LOADED + assert config_entry.unique_id == "test-bond-id" + + # verify the device info is cleaned up + assert device_registry.async_get_device(identifiers={old_identifers}) is None + assert device_registry.async_get_device(identifiers={new_identifiers}) is not None