Remove duplicated device before daikin migration (#99900)

Co-authored-by: Erik Montnemery <erik@montnemery.com>

homeassistant/components/daikin/__init__.py

@@ -135,9 +135,11 @@ async def async_migrate_unique_id(
 ) -> None:
     """Migrate old entry."""
     dev_reg = dr.async_get(hass)
+    ent_reg = er.async_get(hass)
     old_unique_id = config_entry.unique_id
     new_unique_id = api.device.mac
-    new_name = api.device.values.get("name")
+    new_mac = dr.format_mac(new_unique_id)
+    new_name = api.name
 
     @callback
     def _update_unique_id(entity_entry: er.RegistryEntry) -> dict[str, str] | None:
@@ -147,15 +149,36 @@ async def async_migrate_unique_id(
     if new_unique_id == old_unique_id:
         return
 
+    duplicate = dev_reg.async_get_device(
+        connections={(CONNECTION_NETWORK_MAC, new_mac)}, identifiers=None
+    )
+
+    # Remove duplicated device
+    if duplicate is not None:
+        if config_entry.entry_id in duplicate.config_entries:
+            _LOGGER.debug(
+                "Removing duplicated device %s",
+                duplicate.name,
+            )
+
+            # The automatic cleanup in entity registry is scheduled as a task, remove
+            # the entities manually to avoid unique_id collision when the entities
+            # are migrated.
+            duplicate_entities = er.async_entries_for_device(
+                ent_reg, duplicate.id, True
+            )
+            for entity in duplicate_entities:
+                ent_reg.async_remove(entity.entity_id)
+
+            dev_reg.async_remove_device(duplicate.id)
+
     # Migrate devices
     for device_entry in dr.async_entries_for_config_entry(
         dev_reg, config_entry.entry_id
     ):
         for connection in device_entry.connections:
             if connection[1] == old_unique_id:
-                new_connections = {
-                    (CONNECTION_NETWORK_MAC, dr.format_mac(new_unique_id))
-                }
+                new_connections = {(CONNECTION_NETWORK_MAC, new_mac)}
 
                 _LOGGER.debug(
                     "Migrating device %s connections to %s",

tests/components/daikin/test_init.py

@@ -1,11 +1,13 @@
 """Define tests for the Daikin init."""
 import asyncio
+from datetime import timedelta
 from unittest.mock import AsyncMock, PropertyMock, patch
 
 from aiohttp import ClientConnectionError
+from freezegun.api import FrozenDateTimeFactory
 import pytest
 
-from homeassistant.components.daikin import update_unique_id
+from homeassistant.components.daikin import DaikinApi, update_unique_id
 from homeassistant.components.daikin.const import DOMAIN, KEY_MAC
 from homeassistant.config_entries import ConfigEntryState
 from homeassistant.const import CONF_HOST
@@ -14,7 +16,7 @@ from homeassistant.helpers import device_registry as dr, entity_registry as er
 
 from .test_config_flow import HOST, MAC
 
-from tests.common import MockConfigEntry
+from tests.common import MockConfigEntry, async_fire_time_changed
 
 
 @pytest.fixture
@@ -28,6 +30,7 @@ def mock_daikin():
     with patch("homeassistant.components.daikin.Appliance") as Appliance:
         Appliance.factory.side_effect = mock_daikin_factory
         type(Appliance).update_status = AsyncMock()
+        type(Appliance).device_ip = PropertyMock(return_value=HOST)
         type(Appliance).inside_temperature = PropertyMock(return_value=22)
         type(Appliance).target_temperature = PropertyMock(return_value=22)
         type(Appliance).zones = PropertyMock(return_value=[("Zone 1", "0", 0)])
@@ -47,6 +50,67 @@ DATA = {
 INVALID_DATA = {**DATA, "name": None, "mac": HOST}
 
 
+async def test_duplicate_removal(hass: HomeAssistant, mock_daikin) -> None:
+    """Test duplicate device removal."""
+    config_entry = MockConfigEntry(
+        domain=DOMAIN,
+        unique_id=HOST,
+        title=None,
+        data={CONF_HOST: HOST, KEY_MAC: HOST},
+    )
+    config_entry.add_to_hass(hass)
+    entity_registry = er.async_get(hass)
+    device_registry = dr.async_get(hass)
+
+    type(mock_daikin).mac = PropertyMock(return_value=HOST)
+    type(mock_daikin).values = PropertyMock(return_value=INVALID_DATA)
+
+    with patch(
+        "homeassistant.components.daikin.async_migrate_unique_id", return_value=None
+    ):
+        assert await hass.config_entries.async_setup(config_entry.entry_id)
+
+        await hass.async_block_till_done()
+
+        assert config_entry.unique_id != MAC
+
+        type(mock_daikin).mac = PropertyMock(return_value=MAC)
+        type(mock_daikin).values = PropertyMock(return_value=DATA)
+
+        assert await hass.config_entries.async_reload(config_entry.entry_id)
+        await hass.async_block_till_done()
+
+        assert (
+            device_registry.async_get_device({}, {(KEY_MAC, MAC)}).name
+            == "DaikinAP00000"
+        )
+
+        assert device_registry.async_get_device({}, {(KEY_MAC, HOST)}).name is None
+
+        assert entity_registry.async_get("climate.daikin_127_0_0_1").unique_id == HOST
+        assert entity_registry.async_get("switch.none_zone_1").unique_id.startswith(
+            HOST
+        )
+
+        assert entity_registry.async_get("climate.daikinap00000").unique_id == MAC
+        assert entity_registry.async_get(
+            "switch.daikinap00000_zone_1"
+        ).unique_id.startswith(MAC)
+
+    assert await hass.config_entries.async_reload(config_entry.entry_id)
+    await hass.async_block_till_done()
+
+    assert (
+        device_registry.async_get_device({}, {(KEY_MAC, MAC)}).name == "DaikinAP00000"
+    )
+
+    assert entity_registry.async_get("climate.daikinap00000") is None
+    assert entity_registry.async_get("switch.daikinap00000_zone_1") is None
+
+    assert entity_registry.async_get("climate.daikin_127_0_0_1").unique_id == MAC
+    assert entity_registry.async_get("switch.none_zone_1").unique_id.startswith(MAC)
+
+
 async def test_unique_id_migrate(hass: HomeAssistant, mock_daikin) -> None:
     """Test unique id migration."""
     config_entry = MockConfigEntry(
@@ -97,8 +161,41 @@ async def test_unique_id_migrate(hass: HomeAssistant, mock_daikin) -> None:
     assert entity_registry.async_get("switch.none_zone_1").unique_id.startswith(MAC)
 
 
+async def test_client_update_connection_error(
+    hass: HomeAssistant, mock_daikin, freezer: FrozenDateTimeFactory
+) -> None:
+    """Test client connection error on update."""
+    config_entry = MockConfigEntry(
+        domain=DOMAIN,
+        unique_id=MAC,
+        data={CONF_HOST: HOST, KEY_MAC: MAC},
+    )
+    config_entry.add_to_hass(hass)
+    er.async_get(hass)
+
+    type(mock_daikin).mac = PropertyMock(return_value=MAC)
+    type(mock_daikin).values = PropertyMock(return_value=DATA)
+
+    await hass.config_entries.async_setup(config_entry.entry_id)
+
+    api: DaikinApi = hass.data[DOMAIN][config_entry.entry_id]
+
+    assert api.available is True
+
+    type(mock_daikin).update_status.side_effect = ClientConnectionError
+
+    freezer.tick(timedelta(seconds=90))
+    async_fire_time_changed(hass)
+
+    await hass.async_block_till_done()
+
+    assert api.available is False
+
+    assert mock_daikin.update_status.call_count == 2
+
+
 async def test_client_connection_error(hass: HomeAssistant, mock_daikin) -> None:
-    """Test unique id migration."""
+    """Test client connection error on setup."""
     config_entry = MockConfigEntry(
         domain=DOMAIN,
         unique_id=MAC,
@@ -114,7 +211,7 @@ async def test_client_connection_error(hass: HomeAssistant, mock_daikin) -> None
 
 
 async def test_timeout_error(hass: HomeAssistant, mock_daikin) -> None:
-    """Test unique id migration."""
+    """Test timeout error on setup."""
     config_entry = MockConfigEntry(
         domain=DOMAIN,
         unique_id=MAC,