Use opt in device removal for rfxtrx (#58252)

homeassistant/components/rfxtrx/__init__.py

@@ -5,7 +5,6 @@ import asyncio
 import binascii
 from collections.abc import Callable
 import copy
-import functools
 import logging
 from typing import NamedTuple
 
@@ -25,9 +24,13 @@ from homeassistant.const import (
     EVENT_HOMEASSISTANT_STOP,
     Platform,
 )
-from homeassistant.core import HomeAssistant, ServiceCall, callback
+from homeassistant.core import Event, HomeAssistant, ServiceCall, callback
 import homeassistant.helpers.config_validation as cv
-from homeassistant.helpers.device_registry import DeviceRegistry
+from homeassistant.helpers.device_registry import (
+    EVENT_DEVICE_REGISTRY_UPDATED,
+    DeviceEntry,
+    DeviceRegistry,
+)
 from homeassistant.helpers.entity import DeviceInfo, Entity
 from homeassistant.helpers.entity_platform import AddEntitiesCallback
 from homeassistant.helpers.restore_state import RestoreEntity
@@ -37,7 +40,6 @@ from .const import (
     COMMAND_GROUP_LIST,
     CONF_AUTOMATIC_ADD,
     CONF_DATA_BITS,
-    CONF_REMOVE_DEVICE,
     DATA_RFXOBJECT,
     DEVICE_PACKET_TYPE_LIGHTING4,
     EVENT_RFXTRX_EVENT,
@@ -82,7 +84,9 @@ PLATFORMS = [
 ]
 
 
-async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
+async def async_setup_entry(
+    hass: HomeAssistant, entry: config_entries.ConfigEntry
+) -> bool:
     """Set up the RFXtrx component."""
     hass.data.setdefault(DOMAIN, {})
 
@@ -224,6 +228,27 @@ async def async_setup_internal(hass, entry: config_entries.ConfigEntry):
         hass.config_entries.async_update_entry(entry=entry, data=data)
         devices[device_id] = config
 
+    @callback
+    def _remove_device(event: Event):
+        if event.data["action"] != "remove":
+            return
+        device_entry = device_registry.deleted_devices[event.data["device_id"]]
+        device_id = next(iter(device_entry.identifiers))[1:]
+        data = {
+            **entry.data,
+            CONF_DEVICES: {
+                packet_id: entity_info
+                for packet_id, entity_info in entry.data[CONF_DEVICES].items()
+                if tuple(entity_info.get(CONF_DEVICE_ID)) != device_id
+            },
+        }
+        hass.config_entries.async_update_entry(entry=entry, data=data)
+        devices.pop(device_id)
+
+    entry.async_on_unload(
+        hass.bus.async_listen(EVENT_DEVICE_REGISTRY_UPDATED, _remove_device)
+    )
+
     def _shutdown_rfxtrx(event):
         """Close connection with RFXtrx."""
         rfx_object.close_connection()
@@ -388,6 +413,16 @@ def get_device_id(
     return DeviceTuple(f"{device.packettype:x}", f"{device.subtype:x}", id_string)
 
 
+async def async_remove_config_entry_device(
+    hass: HomeAssistant, config_entry: ConfigEntry, device_entry: DeviceEntry
+) -> bool:
+    """Remove config entry from a device.
+
+    The actual cleanup is done in the device registry event
+    """
+    return True
+
+
 class RfxtrxEntity(RestoreEntity):
     """Represents a Rfxtrx device.
 
@@ -424,13 +459,6 @@ class RfxtrxEntity(RestoreEntity):
             )
         )
 
-        self.async_on_remove(
-            self.hass.helpers.dispatcher.async_dispatcher_connect(
-                f"{DOMAIN}_{CONF_REMOVE_DEVICE}_{self._device_id}",
-                functools.partial(self.async_remove, force_remove=True),
-            )
-        )
-
     @property
     def should_poll(self):
         """No polling needed for a RFXtrx switch."""

homeassistant/components/rfxtrx/config_flow.py

@@ -23,7 +23,6 @@ from homeassistant.const import (
     CONF_TYPE,
 )
 from homeassistant.core import callback
-from homeassistant.helpers import config_validation as cv
 from homeassistant.helpers.device_registry import (
     DeviceEntry,
     DeviceRegistry,
@@ -41,7 +40,6 @@ from .const import (
     CONF_AUTOMATIC_ADD,
     CONF_DATA_BITS,
     CONF_OFF_DELAY,
-    CONF_REMOVE_DEVICE,
     CONF_REPLACE_DEVICE,
     CONF_SIGNAL_REPETITIONS,
     CONF_VENETIAN_BLIND_MODE,
@@ -110,26 +108,6 @@ class OptionsFlow(config_entries.OptionsFlow):
                 ]
                 self._selected_device_object = get_rfx_object(event_code)
                 return await self.async_step_set_device_options()
-            if CONF_REMOVE_DEVICE in user_input:
-                remove_devices = user_input[CONF_REMOVE_DEVICE]
-                devices = {}
-                for entry_id in remove_devices:
-                    device_data = self._get_device_data(entry_id)
-
-                    event_code = device_data[CONF_EVENT_CODE]
-                    device_id = device_data[CONF_DEVICE_ID]
-                    self.hass.helpers.dispatcher.async_dispatcher_send(
-                        f"{DOMAIN}_{CONF_REMOVE_DEVICE}_{device_id}"
-                    )
-                    self._device_registry.async_remove_device(entry_id)
-                    if event_code is not None:
-                        devices[event_code] = None
-
-                self.update_config_data(
-                    global_options=self._global_options, devices=devices
-                )
-
-                return self.async_create_entry(title="", data={})
             if CONF_EVENT_CODE in user_input:
                 self._selected_device_event_code = user_input[CONF_EVENT_CODE]
                 self._selected_device = {}
@@ -156,11 +134,6 @@ class OptionsFlow(config_entries.OptionsFlow):
         self._device_registry = device_registry
         self._device_entries = device_entries
 
-        remove_devices = {
-            entry.id: entry.name_by_user if entry.name_by_user else entry.name
-            for entry in device_entries
-        }
-
         configure_devices = {
             entry.id: entry.name_by_user if entry.name_by_user else entry.name
             for entry in device_entries
@@ -174,7 +147,6 @@ class OptionsFlow(config_entries.OptionsFlow):
             ): bool,
             vol.Optional(CONF_EVENT_CODE): str,
             vol.Optional(CONF_DEVICE): vol.In(configure_devices),
-            vol.Optional(CONF_REMOVE_DEVICE): cv.multi_select(remove_devices),
         }
 
         return self.async_show_form(

homeassistant/components/rfxtrx/const.py

@@ -6,7 +6,6 @@ CONF_SIGNAL_REPETITIONS = "signal_repetitions"
 CONF_OFF_DELAY = "off_delay"
 CONF_VENETIAN_BLIND_MODE = "venetian_blind_mode"
 
-CONF_REMOVE_DEVICE = "remove_device"
 CONF_REPLACE_DEVICE = "replace_device"
 
 CONST_VENETIAN_BLIND_MODE_DEFAULT = "Unknown"

homeassistant/components/rfxtrx/strings.json

@@ -42,8 +42,7 @@
           "debug": "Enable debugging",
           "automatic_add": "Enable automatic add",
           "event_code": "Enter event code to add",
-          "device": "Select device to configure",
+          "device": "Select device to configure"
-          "remove_device": "Select device to delete"
         },
         "title": "Rfxtrx Options"
       },

tests/components/rfxtrx/conftest.py

@@ -1,4 +1,6 @@
 """Common test tools."""
+from __future__ import annotations
+
 from datetime import timedelta
 from unittest.mock import patch
 
@@ -24,6 +26,23 @@ def create_rfx_test_cfg(device="abcd", automatic_add=False, devices=None):
     }
 
 
+async def setup_rfx_test_cfg(
+    hass, device="abcd", automatic_add=False, devices: dict[str, dict] | None = None
+):
+    """Construct a rfxtrx config entry."""
+    entry_data = create_rfx_test_cfg(
+        device=device, automatic_add=automatic_add, devices=devices
+    )
+    mock_entry = MockConfigEntry(domain="rfxtrx", unique_id=DOMAIN, data=entry_data)
+    mock_entry.supports_remove_device = True
+    mock_entry.add_to_hass(hass)
+
+    await hass.config_entries.async_setup(mock_entry.entry_id)
+    await hass.async_block_till_done()
+    await hass.async_start()
+    return mock_entry
+
+
 @pytest.fixture(autouse=True, name="rfxtrx")
 async def rfxtrx_fixture(hass):
     """Fixture that cleans up threads from integration."""
@@ -50,14 +69,7 @@ async def rfxtrx_fixture(hass):
 @pytest.fixture(name="rfxtrx_automatic")
 async def rfxtrx_automatic_fixture(hass, rfxtrx):
     """Fixture that starts up with automatic additions."""
-    entry_data = create_rfx_test_cfg(automatic_add=True, devices={})
+    await setup_rfx_test_cfg(hass, automatic_add=True, devices={})
-    mock_entry = MockConfigEntry(domain="rfxtrx", unique_id=DOMAIN, data=entry_data)
-
-    mock_entry.add_to_hass(hass)
-
-    await hass.config_entries.async_setup(mock_entry.entry_id)
-    await hass.async_block_till_done()
-    await hass.async_start()
     yield rfxtrx
 
 

tests/components/rfxtrx/test_config_flow.py

@@ -408,88 +408,6 @@ async def test_options_add_duplicate_device(hass):
     assert result["errors"]["event_code"] == "already_configured_device"
 
 
-async def test_options_add_remove_device(hass):
-    """Test we can add a device."""
-
-    entry = MockConfigEntry(
-        domain=DOMAIN,
-        data={
-            "host": None,
-            "port": None,
-            "device": "/dev/tty123",
-            "automatic_add": False,
-            "devices": {},
-        },
-        unique_id=DOMAIN,
-    )
-    entry.add_to_hass(hass)
-
-    result = await hass.config_entries.options.async_init(entry.entry_id)
-
-    assert result["type"] == "form"
-    assert result["step_id"] == "prompt_options"
-
-    result = await hass.config_entries.options.async_configure(
-        result["flow_id"],
-        user_input={
-            "automatic_add": True,
-            "event_code": "0b1100cd0213c7f230010f71",
-        },
-    )
-
-    assert result["type"] == "form"
-    assert result["step_id"] == "set_device_options"
-
-    result = await hass.config_entries.options.async_configure(
-        result["flow_id"],
-        user_input={"signal_repetitions": 5, "off_delay": "4"},
-    )
-
-    assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY
-
-    await hass.async_block_till_done()
-
-    assert entry.data["automatic_add"]
-
-    assert entry.data["devices"]["0b1100cd0213c7f230010f71"]
-    assert entry.data["devices"]["0b1100cd0213c7f230010f71"]["signal_repetitions"] == 5
-    assert entry.data["devices"]["0b1100cd0213c7f230010f71"]["off_delay"] == 4
-
-    state = hass.states.get("binary_sensor.ac_213c7f2_48")
-    assert state
-    assert state.state == STATE_UNKNOWN
-    assert state.attributes.get("friendly_name") == "AC 213c7f2:48"
-
-    device_registry = dr.async_get(hass)
-    device_entries = dr.async_entries_for_config_entry(device_registry, entry.entry_id)
-
-    assert device_entries[0].id
-
-    result = await hass.config_entries.options.async_init(entry.entry_id)
-
-    assert result["type"] == "form"
-    assert result["step_id"] == "prompt_options"
-
-    result = await hass.config_entries.options.async_configure(
-        result["flow_id"],
-        user_input={
-            "automatic_add": False,
-            "remove_device": [device_entries[0].id],
-        },
-    )
-
-    assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY
-
-    await hass.async_block_till_done()
-
-    assert not entry.data["automatic_add"]
-
-    assert "0b1100cd0213c7f230010f71" not in entry.data["devices"]
-
-    state = hass.states.get("binary_sensor.ac_213c7f2_48")
-    assert not state
-
-
 async def test_options_replace_sensor_device(hass):
     """Test we can replace a sensor device."""
 
@@ -758,76 +676,6 @@ async def test_options_replace_control_device(hass):
     assert not state
 
 
-async def test_options_remove_multiple_devices(hass):
-    """Test we can add a device."""
-
-    entry = MockConfigEntry(
-        domain=DOMAIN,
-        data={
-            "host": None,
-            "port": None,
-            "device": "/dev/tty123",
-            "automatic_add": False,
-            "devices": {
-                "0b1100cd0213c7f230010f71": {"device_id": ["11", "0", "213c7f2:48"]},
-                "0b1100100118cdea02010f70": {"device_id": ["11", "0", "118cdea:2"]},
-                "0b1100101118cdea02010f70": {"device_id": ["11", "0", "1118cdea:2"]},
-            },
-        },
-        unique_id=DOMAIN,
-    )
-    entry.add_to_hass(hass)
-
-    await hass.config_entries.async_setup(entry.entry_id)
-    await hass.async_block_till_done()
-
-    state = hass.states.get("binary_sensor.ac_213c7f2_48")
-    assert state
-    state = hass.states.get("binary_sensor.ac_118cdea_2")
-    assert state
-    state = hass.states.get("binary_sensor.ac_1118cdea_2")
-    assert state
-
-    device_registry = dr.async_get(hass)
-    device_entries = dr.async_entries_for_config_entry(device_registry, entry.entry_id)
-
-    assert len(device_entries) == 3
-
-    def match_device_id(entry):
-        device_id = next(iter(entry.identifiers))[1:]
-        if device_id == ("11", "0", "213c7f2:48"):
-            return True
-        if device_id == ("11", "0", "118cdea:2"):
-            return True
-        return False
-
-    remove_devices = [elem.id for elem in device_entries if match_device_id(elem)]
-
-    result = await hass.config_entries.options.async_init(entry.entry_id)
-
-    assert result["type"] == "form"
-    assert result["step_id"] == "prompt_options"
-
-    result = await hass.config_entries.options.async_configure(
-        result["flow_id"],
-        user_input={
-            "automatic_add": False,
-            "remove_device": remove_devices,
-        },
-    )
-
-    assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY
-
-    await hass.async_block_till_done()
-
-    state = hass.states.get("binary_sensor.ac_213c7f2_48")
-    assert not state
-    state = hass.states.get("binary_sensor.ac_118cdea_2")
-    assert not state
-    state = hass.states.get("binary_sensor.ac_1118cdea_2")
-    assert state
-
-
 async def test_options_add_and_configure_device(hass):
     """Test we can add a device."""
 

tests/components/rfxtrx/test_init.py

@@ -1,19 +1,20 @@
 """The tests for the Rfxtrx component."""
+from __future__ import annotations
 
 from unittest.mock import call
 
-from homeassistant.components.rfxtrx import DOMAIN
 from homeassistant.components.rfxtrx.const import EVENT_RFXTRX_EVENT
 from homeassistant.core import callback
 from homeassistant.helpers import device_registry as dr
+from homeassistant.setup import async_setup_component
 
-from tests.common import MockConfigEntry
+from tests.components.rfxtrx.conftest import setup_rfx_test_cfg
-from tests.components.rfxtrx.conftest import create_rfx_test_cfg
 
 
 async def test_fire_event(hass, rfxtrx):
     """Test fire event."""
-    entry_data = create_rfx_test_cfg(
+    await setup_rfx_test_cfg(
+        hass,
         device="/dev/serial/by-id/usb-RFXCOM_RFXtrx433_A1Y0NJGR-if00-port0",
         automatic_add=True,
         devices={
@@ -21,13 +22,6 @@ async def test_fire_event(hass, rfxtrx):
             "0716000100900970": {},
         },
     )
-    mock_entry = MockConfigEntry(domain="rfxtrx", unique_id=DOMAIN, data=entry_data)
-
-    mock_entry.add_to_hass(hass)
-
-    await hass.config_entries.async_setup(mock_entry.entry_id)
-    await hass.async_block_till_done()
-    await hass.async_start()
 
     device_registry: dr.DeviceRegistry = dr.async_get(hass)
 
@@ -78,13 +72,7 @@ async def test_fire_event(hass, rfxtrx):
 
 async def test_send(hass, rfxtrx):
     """Test configuration."""
-    entry_data = create_rfx_test_cfg(device="/dev/null", devices={})
+    await setup_rfx_test_cfg(hass, device="/dev/null", devices={})
-    mock_entry = MockConfigEntry(domain="rfxtrx", unique_id=DOMAIN, data=entry_data)
-
-    mock_entry.add_to_hass(hass)
-
-    await hass.config_entries.async_setup(mock_entry.entry_id)
-    await hass.async_block_till_done()
 
     await hass.services.async_call(
         "rfxtrx", "send", {"event": "0a520802060101ff0f0269"}, blocking=True
@@ -93,3 +81,40 @@ async def test_send(hass, rfxtrx):
     assert rfxtrx.transport.send.mock_calls == [
         call(bytearray(b"\x0a\x52\x08\x02\x06\x01\x01\xff\x0f\x02\x69"))
     ]
+
+
+async def test_ws_device_remove(hass, hass_ws_client):
+    """Test removing a device through device registry."""
+    assert await async_setup_component(hass, "config", {})
+
+    device_id = ["11", "0", "213c7f2:16"]
+    mock_entry = await setup_rfx_test_cfg(
+        hass,
+        devices={
+            "0b1100cd0213c7f210010f51": {"fire_event": True, "device_id": device_id},
+        },
+    )
+
+    device_reg = dr.async_get(hass)
+
+    device_entry = device_reg.async_get_device(identifiers={("rfxtrx", *device_id)})
+    assert device_entry
+
+    # Ask to remove existing device
+    client = await hass_ws_client(hass)
+    await client.send_json(
+        {
+            "id": 5,
+            "type": "config/device_registry/remove_config_entry",
+            "config_entry_id": mock_entry.entry_id,
+            "device_id": device_entry.id,
+        }
+    )
+    response = await client.receive_json()
+    assert response["success"]
+
+    # Verify device entry is removed
+    assert device_reg.async_get_device(identifiers={("rfxtrx", *device_id)}) is None
+
+    # Verify that the config entry has removed the device
+    assert mock_entry.data["devices"] == {}