Fix Mysensors device id tracking (#180959)

homeassistant/components/mysensors/__init__.py

@@ -21,8 +21,9 @@ from .const import (
     DiscoveryInfo,
     SensorType,
 )
-from .entity import MySensorsChildEntity, get_mysensors_devices
+from .entity import MySensorsChildEntity
 from .gateway import finish_setup, gw_stop, setup_gateway
+from .helpers import get_discovered_dev_ids, remove_gateway_dev_ids, remove_node_dev_ids
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -62,6 +63,7 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
 
     del hass.data[DOMAIN][MYSENSORS_GATEWAYS][entry.entry_id]
     hass.data[DOMAIN].pop(MYSENSORS_DISCOVERED_NODES.format(entry.entry_id), None)
+    remove_gateway_dev_ids(hass, entry.entry_id)
 
     await gw_stop(hass, entry, gateway)
     return True
@@ -87,7 +89,8 @@ async def async_remove_config_entry_device(
     # remove node from discovered nodes
     hass.data[DOMAIN].setdefault(
         MYSENSORS_DISCOVERED_NODES.format(config_entry.entry_id), set()
-    ).remove(node_id)
+    ).discard(node_id)
+    remove_node_dev_ids(hass, config_entry.entry_id, node_id)
 
     return True
 
@@ -119,9 +122,9 @@ def setup_mysensors_platform(
         device_args = ()
     new_devices: list[MySensorsChildEntity] = []
     new_dev_ids: list[DevId] = discovery_info[ATTR_DEVICES]
+    dev_ids = get_discovered_dev_ids(hass, domain)
     for dev_id in new_dev_ids:
-        devices: dict[DevId, MySensorsChildEntity] = get_mysensors_devices(hass, domain)
-        if dev_id in devices:
+        if dev_id in dev_ids:
             _LOGGER.debug(
                 "Skipping setup of %s for platform %s as it already exists",
                 dev_id,
@@ -139,8 +142,8 @@ def setup_mysensors_platform(
             device_class_copy = device_class
 
         args_copy = (*device_args, gateway_id, gateway, node_id, child_id, value_type)
-        devices[dev_id] = device_class_copy(*args_copy)
-        new_devices.append(devices[dev_id])
+        dev_ids.add(dev_id)
+        new_devices.append(device_class_copy(*args_copy))
     if new_devices:
         _LOGGER.debug("Adding new devices: %s", new_devices)
         if async_add_entities is not None:

homeassistant/components/mysensors/const.py

@@ -26,6 +26,7 @@ DOMAIN: Final = "mysensors"
 MYSENSORS_GATEWAY_START_TASK: str = "mysensors_gateway_start_task_{}"
 MYSENSORS_GATEWAYS: Final = "mysensors_gateways"
 MYSENSORS_DISCOVERED_NODES: Final = "mysensors_discovered_nodes_{}"
+MYSENSORS_DISCOVERED_DEV_IDS: Final = "mysensors_discovered_dev_ids_{}"
 PLATFORM: Final = "platform"
 SCHEMA: Final = "schema"
 CHILD_CALLBACK: str = "mysensors_child_callback_{}_{}_{}_{}"

homeassistant/components/mysensors/entity.py

@@ -1,5 +1,4 @@
 """Handle MySensors devices."""
-# pylint: disable=home-assistant-use-runtime-data  # Uses legacy hass.data[DOMAIN] pattern
 
 from abc import abstractmethod
 import logging
@@ -8,28 +7,14 @@ from typing import Any, override
 from mysensors import BaseAsyncGateway, Sensor
 from mysensors.sensor import ChildSensor
 
-from homeassistant.const import (
-    ATTR_BATTERY_LEVEL,
-    CONF_DEVICE,
-    STATE_OFF,
-    STATE_ON,
-    Platform,
-)
+from homeassistant.const import ATTR_BATTERY_LEVEL, CONF_DEVICE, STATE_OFF, STATE_ON
 from homeassistant.core import HomeAssistant, callback
 from homeassistant.helpers.debounce import Debouncer
 from homeassistant.helpers.device_registry import DeviceInfo
 from homeassistant.helpers.dispatcher import async_dispatcher_connect
 from homeassistant.helpers.entity import Entity
 
-from .const import (
-    CHILD_CALLBACK,
-    DOMAIN,
-    NODE_CALLBACK,
-    PLATFORM_TYPES,
-    UPDATE_DELAY,
-    DevId,
-    GatewayId,
-)
+from .const import CHILD_CALLBACK, DOMAIN, NODE_CALLBACK, UPDATE_DELAY, DevId, GatewayId
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -38,7 +23,6 @@ ATTR_DESCRIPTION = "description"
 ATTR_DEVICE = "device"
 ATTR_NODE_ID = "node_id"
 ATTR_HEARTBEAT = "heartbeat"
-MYSENSORS_PLATFORM_DEVICES = "mysensors_devices_{}"
 
 
 class MySensorNodeEntity(Entity):
@@ -136,18 +120,6 @@ class MySensorNodeEntity(Entity):
         self._async_update_callback()
 
 
-def get_mysensors_devices(
-    hass: HomeAssistant, domain: Platform
-) -> dict[DevId, MySensorsChildEntity]:
-    """Return MySensors devices for a hass platform name."""
-    if MYSENSORS_PLATFORM_DEVICES.format(domain) not in hass.data[DOMAIN]:
-        hass.data[DOMAIN][MYSENSORS_PLATFORM_DEVICES.format(domain)] = {}
-    devices: dict[DevId, MySensorsChildEntity] = hass.data[DOMAIN][
-        MYSENSORS_PLATFORM_DEVICES.format(domain)
-    ]
-    return devices
-
-
 class MySensorsChildEntity(MySensorNodeEntity):
     """Representation of a MySensors entity."""
 
@@ -197,17 +169,6 @@ class MySensorsChildEntity(MySensorNodeEntity):
             return str(child.description)
         return f"{self.node_name} {self.child_id}"
 
-    @override
-    async def async_will_remove_from_hass(self) -> None:
-        """Remove this entity from home assistant."""
-        for platform in PLATFORM_TYPES:
-            platform_str = MYSENSORS_PLATFORM_DEVICES.format(platform)
-            if platform_str in self.hass.data[DOMAIN]:
-                platform_dict = self.hass.data[DOMAIN][platform_str]
-                if self.dev_id in platform_dict:
-                    del platform_dict[self.dev_id]
-                    _LOGGER.debug("Deleted %s from platform %s", self.dev_id, platform)
-
     @property
     @override
     def available(self) -> bool:

homeassistant/components/mysensors/handler.py

@@ -11,10 +11,10 @@ from homeassistant.helpers.dispatcher import async_dispatcher_send
 from homeassistant.util import decorator
 
 from .const import CHILD_CALLBACK, NODE_CALLBACK, DevId, GatewayId
-from .entity import get_mysensors_devices
 from .helpers import (
     discover_mysensors_node,
     discover_mysensors_platform,
+    get_discovered_dev_ids,
     validate_set_msg,
 )
 
@@ -95,10 +95,10 @@ def _handle_child_update(
     # Update all platforms for the device via dispatcher.
     # Add/update entity for validated children.
     for platform, dev_ids in validated.items():
-        devices = get_mysensors_devices(hass, platform)
+        discovered_dev_ids = get_discovered_dev_ids(hass, platform)
         new_dev_ids: list[DevId] = []
         for dev_id in dev_ids:
-            if dev_id in devices:
+            if dev_id in discovered_dev_ids:
                 signals.append(CHILD_CALLBACK.format(*dev_id))
             else:
                 new_dev_ids.append(dev_id)

homeassistant/components/mysensors/helpers.py

@@ -22,9 +22,11 @@ from .const import (
     ATTR_NODE_ID,
     DOMAIN,
     FLAT_PLATFORM_TYPES,
+    MYSENSORS_DISCOVERED_DEV_IDS,
     MYSENSORS_DISCOVERED_NODES,
     MYSENSORS_DISCOVERY,
     MYSENSORS_NODE_DISCOVERY,
+    PLATFORM_TYPES,
     TYPE_TO_PLATFORMS,
     DevId,
     GatewayId,
@@ -78,6 +80,43 @@ def discover_mysensors_node(
         )
 
 
+@callback
+def get_discovered_dev_ids(hass: HomeAssistant, platform: Platform) -> set[DevId]:
+    """Return the dev ids that have been set up for a hass platform."""
+    # Uses legacy hass.data[DOMAIN] pattern
+    # pylint: disable-next=home-assistant-use-runtime-data
+    dev_ids = hass.data[DOMAIN].setdefault(
+        MYSENSORS_DISCOVERED_DEV_IDS.format(platform), set()
+    )
+    return cast(set[DevId], dev_ids)
+
+
+@callback
+def remove_gateway_dev_ids(hass: HomeAssistant, gateway_id: GatewayId) -> None:
+    """Remove all discovered dev ids belonging to a gateway."""
+    for platform in PLATFORM_TYPES:
+        dev_ids = get_discovered_dev_ids(hass, platform)
+        dev_ids.difference_update(
+            {dev_id for dev_id in dev_ids if dev_id[0] == gateway_id}
+        )
+
+
+@callback
+def remove_node_dev_ids(
+    hass: HomeAssistant, gateway_id: GatewayId, node_id: int
+) -> None:
+    """Remove all discovered dev ids belonging to a node."""
+    for platform in PLATFORM_TYPES:
+        dev_ids = get_discovered_dev_ids(hass, platform)
+        dev_ids.difference_update(
+            {
+                dev_id
+                for dev_id in dev_ids
+                if dev_id[0] == gateway_id and dev_id[1] == node_id
+            }
+        )
+
+
 def default_schema(
     gateway: BaseAsyncGateway, child: ChildSensor, value_type_name: ValueType
 ) -> vol.Schema:

tests/components/mysensors/test_init.py

@@ -5,6 +5,7 @@ from unittest.mock import MagicMock
 
 from mysensors import BaseSyncGateway
 from mysensors.sensor import Sensor
+import pytest
 
 from homeassistant.components.mysensors import DOMAIN
 from homeassistant.config_entries import ConfigEntryState
@@ -61,6 +62,72 @@ async def test_load_unload(
     assert state.state == STATE_UNAVAILABLE
 
 
+@pytest.mark.usefixtures("door_sensor")
+async def test_reload(
+    hass: HomeAssistant,
+    transport: MagicMock,
+    integration: MockConfigEntry,
+) -> None:
+    """Test reloading the MySensors config entry recreates entities."""
+    config_entry = integration
+
+    entity_id = "binary_sensor.door_sensor_1_1"
+    state = hass.states.get(entity_id)
+
+    assert state
+    assert state.state != STATE_UNAVAILABLE
+
+    assert await hass.config_entries.async_reload(config_entry.entry_id)
+
+    assert config_entry.state is ConfigEntryState.LOADED
+    assert transport.return_value.disconnect.call_count == 1
+
+    state = hass.states.get(entity_id)
+
+    assert state
+    assert state.state != STATE_UNAVAILABLE
+
+
+@pytest.mark.usefixtures("text_node", "integration")
+async def test_disabling_entity_keeps_other_platforms_dev_id(
+    hass: HomeAssistant,
+    entity_registry: er.EntityRegistry,
+    receive_message: Callable[[str], None],
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    """Test that disabling one entity does not discard another platform's dev id.
+
+    An S_INFO/V_TEXT child is set up on both the sensor and text platforms, sharing
+    the same dev id. Disabling only the text entity must not discard that dev id
+    for the sensor platform too, or the next message for it would make the
+    integration attempt to recreate the still-loaded sensor entity as a duplicate.
+    """
+    sensor_entity_id = "sensor.text_node_1_1"
+    text_entity_id = "text.text_node_1_1"
+
+    assert hass.states.get(sensor_entity_id)
+    assert hass.states.get(text_entity_id)
+
+    entity_registry.async_update_entity(
+        text_entity_id, disabled_by=er.RegistryEntryDisabler.USER
+    )
+    await hass.async_block_till_done()
+
+    assert not hass.states.get(text_entity_id)
+    assert hass.states.get(sensor_entity_id)
+
+    receive_message("1;1;1;0;47;test\n")
+    await hass.async_block_till_done()
+
+    assert "already exists" not in caplog.text
+    assert hass.states.get(sensor_entity_id)
+    assert not hass.states.get(text_entity_id)
+
+    text_entry = entity_registry.async_get(text_entity_id)
+    assert text_entry
+    assert text_entry.disabled_by is er.RegistryEntryDisabler.USER
+
+
 async def test_remove_config_entry_device(
     hass: HomeAssistant,
     device_registry: dr.DeviceRegistry,