Reolink replace automatic removal of devices by manual removal (home-…

…assistant#120981)

Co-authored-by: Robert Resch <robert@resch.dev>

homeassistant/components/reolink/__init__.py

@@ -147,9 +147,7 @@ async def async_check_firmware_update() -> None:
         firmware_coordinator=firmware_coordinator,
     )
 
-    # first migrate and then cleanup, otherwise entities lost
     migrate_entity_ids(hass, config_entry.entry_id, host)
-    cleanup_disconnected_cams(hass, config_entry.entry_id, host)
 
     await hass.config_entries.async_forward_entry_setups(config_entry, PLATFORMS)
 
@@ -179,6 +177,50 @@ async def async_unload_entry(hass: HomeAssistant, config_entry: ConfigEntry) ->
     return unload_ok
 
 
+async def async_remove_config_entry_device(
+    hass: HomeAssistant, config_entry: ConfigEntry, device: dr.DeviceEntry
+) -> bool:
+    """Remove a device from a config entry."""
+    host: ReolinkHost = hass.data[DOMAIN][config_entry.entry_id].host
+    (device_uid, ch) = get_device_uid_and_ch(device, host)
+
+    if not host.api.is_nvr or ch is None:
+        _LOGGER.warning(
+            "Cannot remove Reolink device %s, because it is not a camera connected "
+            "to a NVR/Hub, please remove the integration entry instead",
+            device.name,
+        )
+        return False  # Do not remove the host/NVR itself
+
+    if ch not in host.api.channels:
+        _LOGGER.debug(
+            "Removing Reolink device %s, "
+            "since no camera is connected to NVR channel %s anymore",
+            device.name,
+            ch,
+        )
+        return True
+
+    await host.api.get_state(cmd="GetChannelstatus")  # update the camera_online status
+    if not host.api.camera_online(ch):
+        _LOGGER.debug(
+            "Removing Reolink device %s, "
+            "since the camera connected to channel %s is offline",
+            device.name,
+            ch,
+        )
+        return True
+
+    _LOGGER.warning(
+        "Cannot remove Reolink device %s on channel %s, because it is still connected "
+        "to the NVR/Hub, please first remove the camera from the NVR/Hub "
+        "in the reolink app",
+        device.name,
+        ch,
+    )
+    return False
+
+
 def get_device_uid_and_ch(
     device: dr.DeviceEntry, host: ReolinkHost
 ) -> tuple[list[str], int | None]:
@@ -197,47 +239,6 @@ def get_device_uid_and_ch(
     return (device_uid, ch)
 
 
-def cleanup_disconnected_cams(
-    hass: HomeAssistant, config_entry_id: str, host: ReolinkHost
-) -> None:
-    """Clean-up disconnected camera channels."""
-    if not host.api.is_nvr:
-        return
-
-    device_reg = dr.async_get(hass)
-    devices = dr.async_entries_for_config_entry(device_reg, config_entry_id)
-    for device in devices:
-        (device_uid, ch) = get_device_uid_and_ch(device, host)
-        if ch is None:
-            continue  # Do not consider the NVR itself
-
-        ch_model = host.api.camera_model(ch)
-        remove = False
-        if ch not in host.api.channels:
-            remove = True
-            _LOGGER.debug(
-                "Removing Reolink device %s, "
-                "since no camera is connected to NVR channel %s anymore",
-                device.name,
-                ch,
-            )
-        if ch_model not in [device.model, "Unknown"]:
-            remove = True
-            _LOGGER.debug(
-                "Removing Reolink device %s, "
-                "since the camera model connected to channel %s changed from %s to %s",
-                device.name,
-                ch,
-                device.model,
-                ch_model,
-            )
-        if not remove:
-            continue
-
-        # clean device registry and associated entities
-        device_reg.async_remove_device(device.id)
-
-
 def migrate_entity_ids(
     hass: HomeAssistant, config_entry_id: str, host: ReolinkHost
 ) -> None:

tests/components/reolink/test_init.py

@@ -36,6 +36,7 @@
 )
 
 from tests.common import MockConfigEntry, async_fire_time_changed
+from tests.typing import WebSocketGenerator
 
 pytestmark = pytest.mark.usefixtures("reolink_connect", "reolink_platforms")
 
@@ -179,16 +180,27 @@ async def test_entry_reloading(
             None,
             [TEST_HOST_MODEL, TEST_CAM_MODEL],
         ),
+        (
+            "is_nvr",
+            False,
+            [TEST_HOST_MODEL, TEST_CAM_MODEL],
+        ),
         ("channels", [], [TEST_HOST_MODEL]),
         (
-            "camera_model",
-            Mock(return_value="RLC-567"),
-            [TEST_HOST_MODEL, "RLC-567"],
+            "camera_online",
+            Mock(return_value=False),
+            [TEST_HOST_MODEL],
+        ),
+        (
+            "channel_for_uid",
+            Mock(return_value=-1),
+            [TEST_HOST_MODEL],
         ),
     ],
 )
-async def test_cleanup_disconnected_cams(
+async def test_removing_disconnected_cams(
     hass: HomeAssistant,
+    hass_ws_client: WebSocketGenerator,
     config_entry: MockConfigEntry,
     reolink_connect: MagicMock,
     device_registry: dr.DeviceRegistry,
@@ -197,8 +209,10 @@ async def test_cleanup_disconnected_cams(
     value: Any,
     expected_models: list[str],
 ) -> None:
-    """Test device and entity registry are cleaned up when camera is disconnected from NVR."""
+    """Test device and entity registry are cleaned up when camera is removed."""
     reolink_connect.channels = [0]
+    assert await async_setup_component(hass, "config", {})
+    client = await hass_ws_client(hass)
     # setup CH 0 and NVR switch entities/device
     with patch("homeassistant.components.reolink.PLATFORMS", [Platform.SWITCH]):
         assert await hass.config_entries.async_setup(config_entry.entry_id)
@@ -215,6 +229,13 @@ async def test_cleanup_disconnected_cams(
         setattr(reolink_connect, attr, value)
     with patch("homeassistant.components.reolink.PLATFORMS", [Platform.SWITCH]):
         assert await hass.config_entries.async_reload(config_entry.entry_id)
+    await hass.async_block_till_done()
+
+    expected_success = TEST_CAM_MODEL not in expected_models
+    for device in device_entries:
+        if device.model == TEST_CAM_MODEL:
+            response = await client.remove_device(device.id, config_entry.entry_id)
+            assert response["success"] == expected_success
 
     device_entries = dr.async_entries_for_config_entry(
         device_registry, config_entry.entry_id