Cleanup bond identifiers and device info (#46192)

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)

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"

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:

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."""

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