diff --git a/homeassistant/components/mysensors/__init__.py b/homeassistant/components/mysensors/__init__.py index e9c17bf5a286..811eadaa2fe8 100644 --- a/homeassistant/components/mysensors/__init__.py +++ b/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: diff --git a/homeassistant/components/mysensors/const.py b/homeassistant/components/mysensors/const.py index 8093bd92a9db..33dd43fbce0b 100644 --- a/homeassistant/components/mysensors/const.py +++ b/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_{}_{}_{}_{}" diff --git a/homeassistant/components/mysensors/entity.py b/homeassistant/components/mysensors/entity.py index ce1584555205..318810cc5f80 100644 --- a/homeassistant/components/mysensors/entity.py +++ b/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: diff --git a/homeassistant/components/mysensors/handler.py b/homeassistant/components/mysensors/handler.py index a00a6ca92e53..25eb7f68647c 100644 --- a/homeassistant/components/mysensors/handler.py +++ b/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) diff --git a/homeassistant/components/mysensors/helpers.py b/homeassistant/components/mysensors/helpers.py index 6de3cb27c069..0b03f33a2a7e 100644 --- a/homeassistant/components/mysensors/helpers.py +++ b/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: diff --git a/tests/components/mysensors/test_init.py b/tests/components/mysensors/test_init.py index 5f1b5889aac2..587e04f4b144 100644 --- a/tests/components/mysensors/test_init.py +++ b/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,