Remove stale devices automatically for Roborock (#140991)

* Remove stale devices

* Add test

* extra test + fix networking patch bug

homeassistant/components/roborock/__init__.py

@@ -23,6 +23,7 @@ from roborock.web_api import RoborockApiClient
 from homeassistant.const import CONF_USERNAME, EVENT_HOMEASSISTANT_STOP
 from homeassistant.core import HomeAssistant
 from homeassistant.exceptions import ConfigEntryAuthFailed, ConfigEntryNotReady
+from homeassistant.helpers import device_registry as dr
 
 from .const import CONF_BASE_URL, CONF_USER_DATA, DOMAIN, PLATFORMS
 from .coordinator import (
@@ -134,6 +135,27 @@ async def async_setup_entry(hass: HomeAssistant, entry: RoborockConfigEntry) ->
 
     await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS)
 
+    device_registry = dr.async_get(hass)
+    device_entries = dr.async_entries_for_config_entry(
+        device_registry, config_entry_id=entry.entry_id
+    )
+    for device in device_entries:
+        # Remove any devices that are no longer in the account.
+        # The API returns all devices, even if they are offline
+        device_duids = {
+            identifier[1].replace("_dock", "") for identifier in device.identifiers
+        }
+        if any(device_duid in device_map for device_duid in device_duids):
+            continue
+        _LOGGER.info(
+            "Removing device: %s because it is no longer exists in your account",
+            device.name,
+        )
+        device_registry.async_update_device(
+            device_id=device.id,
+            remove_config_entry_id=entry.entry_id,
+        )
+
     return True
 
 

homeassistant/components/roborock/quality_scale.yaml

@@ -70,11 +70,7 @@ rules:
   repair-issues:
     status: todo
     comment: The Cloud vs Local API warning should probably be a repair issue.
-  stale-devices:
-    status: todo
-    comment: |
-      The integration does not yet handle stale devices. The roborock app does
-      support deleting devices and this is a gap #132590
+  stale-devices: done
   # Platinum
   async-dependency: todo
   inject-websession:

tests/components/roborock/conftest.py

@@ -11,6 +11,7 @@ import uuid
 import pytest
 from roborock import RoborockCategory, RoomMapping
 from roborock.code_mappings import DyadError, RoborockDyadStateCode, ZeoError, ZeoState
+from roborock.containers import NetworkInfo
 from roborock.roborock_message import RoborockDyadDataProtocol, RoborockZeoProtocol
 from roborock.version_a01_apis import RoborockMqttClientA01
 
@@ -29,6 +30,7 @@ from .mock_data import (
     MAP_DATA,
     MULTI_MAP_LIST,
     NETWORK_INFO,
+    NETWORK_INFO_2,
     PROP,
     SCENES,
     USER_DATA,
@@ -87,6 +89,13 @@ def bypass_api_client_fixture() -> None:
         yield
 
 
+def cycle_network_info() -> Generator[NetworkInfo]:
+    """Return the appropriate network info for the corresponding device."""
+    while True:
+        yield NETWORK_INFO
+        yield NETWORK_INFO_2
+
+
 @pytest.fixture(name="bypass_api_fixture")
 def bypass_api_fixture(bypass_api_client_fixture: Any) -> None:
     """Skip calls to the API."""
@@ -98,7 +107,7 @@ def bypass_api_fixture(bypass_api_client_fixture: Any) -> None:
         ),
         patch(
             "homeassistant.components.roborock.RoborockMqttClientV1.get_networking",
-            return_value=NETWORK_INFO,
+            side_effect=cycle_network_info(),
         ),
         patch(
             "homeassistant.components.roborock.coordinator.RoborockLocalClientV1.get_prop",

tests/components/roborock/mock_data.py

@@ -1122,6 +1122,9 @@ PROP = DeviceProp(
 NETWORK_INFO = NetworkInfo(
     ip="123.232.12.1", ssid="wifi", mac="ac:cc:cc:cc:cc", bssid="bssid", rssi=90
 )
+NETWORK_INFO_2 = NetworkInfo(
+    ip="123.232.12.2", ssid="wifi", mac="ac:cc:cc:cc:cd", bssid="bssid", rssi=90
+)
 
 MULTI_MAP_LIST = MultiMapsList.from_dict(
     {

tests/components/roborock/snapshots/test_diagnostics.ambr

@@ -357,7 +357,7 @@
           }),
           'network_info': dict({
             'bssid': '**REDACTED**',
-            'ip': '123.232.12.1',
+            'ip': '123.232.12.2',
             'mac': '**REDACTED**',
             'rssi': 90,
             'ssid': 'wifi',

tests/components/roborock/test_init.py

@@ -17,9 +17,10 @@ from homeassistant.components.roborock.const import DOMAIN
 from homeassistant.config_entries import ConfigEntryState
 from homeassistant.const import Platform
 from homeassistant.core import HomeAssistant
+from homeassistant.helpers.device_registry import DeviceRegistry
 from homeassistant.setup import async_setup_component
 
-from .mock_data import HOME_DATA
+from .mock_data import HOME_DATA, NETWORK_INFO
 
 from tests.common import MockConfigEntry
 from tests.typing import ClientSessionGenerator
@@ -295,3 +296,60 @@ async def test_no_user_agreement(
         await hass.config_entries.async_setup(mock_roborock_entry.entry_id)
         assert mock_roborock_entry.state is ConfigEntryState.SETUP_RETRY
         assert mock_roborock_entry.error_reason_translation_key == "no_user_agreement"
+
+
+async def test_stale_device(
+    hass: HomeAssistant,
+    bypass_api_fixture,
+    mock_roborock_entry: MockConfigEntry,
+    device_registry: DeviceRegistry,
+) -> None:
+    """Test that we remove a device if it no longer is given by home_data."""
+    await hass.config_entries.async_setup(mock_roborock_entry.entry_id)
+    assert mock_roborock_entry.state is ConfigEntryState.LOADED
+    existing_devices = device_registry.devices.get_devices_for_config_entry_id(
+        mock_roborock_entry.entry_id
+    )
+    assert len(existing_devices) == 6  # 2 for each robot, 1 for A01, 1 for Zeo
+    hd = deepcopy(HOME_DATA)
+    hd.devices = [hd.devices[0]]
+
+    with patch(
+        "homeassistant.components.roborock.RoborockApiClient.get_home_data_v2",
+        return_value=hd,
+    ):
+        await hass.config_entries.async_reload(mock_roborock_entry.entry_id)
+        await hass.async_block_till_done()
+    new_devices = device_registry.devices.get_devices_for_config_entry_id(
+        mock_roborock_entry.entry_id
+    )
+    assert (
+        len(new_devices) == 4
+    )  # 2 for the one remaining robot. 1 for both the A01s which are shared and
+    # therefore not deleted.
+
+
+async def test_no_stale_device(
+    hass: HomeAssistant,
+    bypass_api_fixture,
+    mock_roborock_entry: MockConfigEntry,
+    device_registry: DeviceRegistry,
+) -> None:
+    """Test that we don't remove a device if fails to setup."""
+    await hass.config_entries.async_setup(mock_roborock_entry.entry_id)
+    assert mock_roborock_entry.state is ConfigEntryState.LOADED
+    existing_devices = device_registry.devices.get_devices_for_config_entry_id(
+        mock_roborock_entry.entry_id
+    )
+    assert len(existing_devices) == 6  # 2 for each robot, 1 for A01, 1 for Zeo
+
+    with patch(
+        "homeassistant.components.roborock.RoborockMqttClientV1.get_networking",
+        side_effect=[NETWORK_INFO, RoborockException],
+    ):
+        await hass.config_entries.async_reload(mock_roborock_entry.entry_id)
+        await hass.async_block_till_done()
+    new_devices = device_registry.devices.get_devices_for_config_entry_id(
+        mock_roborock_entry.entry_id
+    )
+    assert len(new_devices) == 6  # 2 for each robot, 1 for A01, 1 for Zeo