From a388863e6291338f0a13a9e6f87239eef10a0f67 Mon Sep 17 00:00:00 2001 From: Luke Lashley Date: Thu, 20 Mar 2025 21:28:37 -0400 Subject: [PATCH] Remove stale devices automatically for Roborock (#140991) * Remove stale devices * Add test * extra test + fix networking patch bug --- homeassistant/components/roborock/__init__.py | 22 +++++++ .../components/roborock/quality_scale.yaml | 6 +- tests/components/roborock/conftest.py | 11 +++- tests/components/roborock/mock_data.py | 3 + .../roborock/snapshots/test_diagnostics.ambr | 2 +- tests/components/roborock/test_init.py | 60 ++++++++++++++++++- 6 files changed, 96 insertions(+), 8 deletions(-) diff --git a/homeassistant/components/roborock/__init__.py b/homeassistant/components/roborock/__init__.py index 1b90adaf6ec..a3ccf0c6eed 100644 --- a/homeassistant/components/roborock/__init__.py +++ b/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 diff --git a/homeassistant/components/roborock/quality_scale.yaml b/homeassistant/components/roborock/quality_scale.yaml index feee5cb434c..06a7638c222 100644 --- a/homeassistant/components/roborock/quality_scale.yaml +++ b/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: diff --git a/tests/components/roborock/conftest.py b/tests/components/roborock/conftest.py index 332a9143c51..fcd469ca10a 100644 --- a/tests/components/roborock/conftest.py +++ b/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", diff --git a/tests/components/roborock/mock_data.py b/tests/components/roborock/mock_data.py index 87acc85b2aa..507e8060653 100644 --- a/tests/components/roborock/mock_data.py +++ b/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( { diff --git a/tests/components/roborock/snapshots/test_diagnostics.ambr b/tests/components/roborock/snapshots/test_diagnostics.ambr index 26ecb729312..313824e70ec 100644 --- a/tests/components/roborock/snapshots/test_diagnostics.ambr +++ b/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', diff --git a/tests/components/roborock/test_init.py b/tests/components/roborock/test_init.py index 9a749a71e30..226eea816b9 100644 --- a/tests/components/roborock/test_init.py +++ b/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