Fix child device identity collisions (#5842)

Author: @nitaybz

Use both device_id and device_cid to construct unique ids for
subdevices.

Some hubs do not randomly generate cids, but assign sequentially, making
a high likelihood of clashes if there are two hubs on the network.

Fixes #5811

Co-authored-by: Nitay Ben Zvi <nitaybz@gmail.com>

custom_components/tuya_local/__init__.py

@@ -990,6 +990,29 @@ async def async_migrate_entry(hass, entry: ConfigEntry):
 
         await async_migrate_entries(hass, entry.entry_id, update_unique_id13_21)
         hass.config_entries.async_update_entry(entry, minor_version=21)
+
+    if entry.version == 13 and entry.minor_version < 22:
+        # A child device ID is only unique within its gateway. Scope it by the
+        # parent device ID so children on separate gateways can coexist.
+        old_device_id = get_device_unique_id(entry)
+        new_device_id = get_device_id(entry.data)
+        if old_device_id != new_device_id:
+
+            @callback
+            def update_gateway_scoped_unique_id(entity_entry):
+                """Scope entity identities by the parent gateway."""
+                if entity_entry.unique_id.startswith(old_device_id):
+                    return {
+                        "new_unique_id": entity_entry.unique_id.replace(
+                            old_device_id, new_device_id, 1
+                        )
+                    }
+
+            await async_migrate_entries(
+                hass, entry.entry_id, update_gateway_scoped_unique_id
+            )
+            hass.config_entries.async_update_entry(entry, unique_id=new_device_id)
+        hass.config_entries.async_update_entry(entry, minor_version=22)
     return True
 
 

custom_components/tuya_local/config_flow.py

@@ -53,7 +53,7 @@ DEVICE_DETAILS_URL = (
 
 class ConfigFlowHandler(ConfigFlow, domain=DOMAIN):
     VERSION = 13
-    MINOR_VERSION = 21
+    MINOR_VERSION = 22
     CONNECTION_CLASS = CONN_CLASS_LOCAL_PUSH
     device = None
     data = {}
@@ -416,9 +416,7 @@ class ConfigFlowHandler(ConfigFlow, domain=DOMAIN):
                         self.device.set_detected_product_id(
                             self.__cloud_device.get("local_product_id")
                         )
-                await self.async_set_unique_id(
-                    user_input.get(CONF_DEVICE_CID, user_input[CONF_DEVICE_ID])
-                )
+                await self.async_set_unique_id(get_device_id(user_input))
                 self._abort_if_unique_id_configured()
                 return await self.async_step_select_type()
             else:

custom_components/tuya_local/device.py

@@ -89,6 +89,7 @@ class TuyaLocalDevice(object):
         self._api_protocol_version_index = None
         self._api_protocol_working = False
         self._api_working_protocol_failures = 0
+        self.dev_id = dev_id
         self.dev_cid = dev_cid
         try:
             if dev_cid:
@@ -172,8 +173,12 @@ class TuyaLocalDevice(object):
 
     @property
     def unique_id(self):
-        """Return the unique id for this device (the dev_id or dev_cid)."""
-        return self.dev_cid or self._api.id
+        """Return the unique ID for this device."""
+        if self.dev_cid:
+            return get_device_id(
+                {CONF_DEVICE_ID: self.dev_id, CONF_DEVICE_CID: self.dev_cid}
+            )
+        return self._api.id
 
     @property
     def device_info(self):

custom_components/tuya_local/helpers/config.py

@@ -45,8 +45,8 @@ async def async_tuya_setup_platform(
 
 
 def get_device_id(config: dict):
-    return (
-        config[CONF_DEVICE_CID]
-        if CONF_DEVICE_CID in config and config[CONF_DEVICE_CID] != ""
-        else config[CONF_DEVICE_ID]
-    )
+    device_id = config.get(CONF_DEVICE_ID)
+    device_cid = config.get(CONF_DEVICE_CID)
+    if device_id and device_cid:
+        return f"{device_id}/{device_cid}"
+    return device_cid or device_id

tests/test_config_flow.py

@@ -7,6 +7,7 @@ import voluptuous as vol
 from homeassistant.const import CONF_HOST, CONF_NAME
 from homeassistant.data_entry_flow import FlowResultType
 from homeassistant.exceptions import ConfigEntryNotReady
+from homeassistant.helpers import entity_registry as er
 from pytest_homeassistant_custom_component.common import MockConfigEntry
 
 from custom_components.tuya_local import (
@@ -346,6 +347,37 @@ async def test_migrate_entry(hass, mocker):
     assert await async_migrate_entry(hass, entry)
 
 
+@pytest.mark.asyncio
+async def test_migrate_child_device_identity_is_scoped_by_gateway(hass):
+    """Child device identities include their parent gateway after migration."""
+    entry = MockConfigEntry(
+        domain=DOMAIN,
+        version=13,
+        minor_version=21,
+        unique_id="001",
+        title="Kitchen AC",
+        data={
+            CONF_DEVICE_ID: "gatewayid",
+            CONF_DEVICE_CID: "001",
+            CONF_HOST: "hostname",
+            CONF_LOCAL_KEY: TESTKEY,
+            CONF_TYPE: "idea_heatingbelt_airconditioner",
+            CONF_PROTOCOL_VERSION: 3.3,
+            CONF_POLL_ONLY: True,
+        },
+    )
+    entry.add_to_hass(hass)
+    registry = er.async_get(hass)
+    entity = registry.async_get_or_create(
+        "climate", DOMAIN, "001-climate", config_entry=entry
+    )
+
+    assert await async_migrate_entry(hass, entry)
+    assert entry.unique_id == "gatewayid/001"
+    assert entry.minor_version == 22
+    assert registry.async_get(entity.entity_id).unique_id == "gatewayid/001-climate"
+
+
 @pytest.mark.asyncio
 async def test_flow_user_init(hass, mocker):
     """Test the initialisation of the form in the first page of the manual config flow path."""
@@ -447,7 +479,7 @@ async def test_async_test_connection_for_subdevice_valid(hass, mocker):
     mock_instance.pause = mocker.MagicMock()
     mock_instance.resume = mocker.MagicMock()
     mock_device.return_value = mock_instance
-    hass.data[DOMAIN] = {"subdeviceid": {"device": mock_instance}}
+    hass.data[DOMAIN] = {"deviceid/subdeviceid": {"device": mock_instance}}
 
     device = await config_flow.async_test_connection(
         {

tests/test_device.py

@@ -61,6 +61,21 @@ def test_unique_id(subject, mock_api):
     assert subject.unique_id is mock_api().id
 
 
+def test_subdevice_unique_id_is_scoped_by_gateway(patched_hass, mock_api):
+    """Returns a gateway-scoped ID for a child device."""
+    subject = TuyaLocalDevice(
+        "Some name",
+        "gateway_id",
+        "some.ip.address",
+        "some_local_key",
+        "3.3",
+        "child_id",
+        patched_hass,
+    )
+
+    assert subject.unique_id == "gateway_id/child_id"
+
+
 def test_device_info(subject, mock_api):
     """Returns generic info plus the unique ID for categorisation."""
     assert subject.device_info == {

tests/test_device_config.py

@@ -777,10 +777,11 @@ def test_values_with_mirror(mocker):
 
 
 def test_get_device_id():
-    """Test that check if device id is correct"""
+    """Test that child devices are scoped to their gateway."""
     assert "my-device-id" == get_device_id({"device_id": "my-device-id"})
     assert "sub-id" == get_device_id({"device_cid": "sub-id"})
-    assert "s" == get_device_id({"device_id": "d", "device_cid": "s"})
+    assert "d/s" == get_device_id({"device_id": "d", "device_cid": "s"})
+    assert "other/s" == get_device_id({"device_id": "other", "device_cid": "s"})
 
 
 def test_getting_masked_hex(mocker):