diff --git a/homeassistant/components/homematicip_cloud/alarm_control_panel.py b/homeassistant/components/homematicip_cloud/alarm_control_panel.py index 1b8509f6c0df..77026cd84449 100644 --- a/homeassistant/components/homematicip_cloud/alarm_control_panel.py +++ b/homeassistant/components/homematicip_cloud/alarm_control_panel.py @@ -131,6 +131,9 @@ class HomematicipAlarmControlPanelEntity(AlarmControlPanelEntity): async def async_added_to_hass(self) -> None: """Register callbacks.""" self._home.on_update(self._async_device_changed) + self.async_on_remove( + lambda: self._home.remove_callback(self._async_device_changed) + ) @callback def _async_device_changed(self, *args, **kwargs) -> None: diff --git a/homeassistant/components/homematicip_cloud/entity.py b/homeassistant/components/homematicip_cloud/entity.py index 9573567e2fb3..388318138106 100644 --- a/homeassistant/components/homematicip_cloud/entity.py +++ b/homeassistant/components/homematicip_cloud/entity.py @@ -171,6 +171,12 @@ class HomematicipGenericEntity(Entity): self._hap.hmip_device_by_entity_id[self.entity_id] = self._device self._device.on_update(self._async_device_changed) self._device.on_remove(self._async_device_removed) + self.async_on_remove( + lambda: self._device.remove_callback(self._async_device_changed) + ) + self.async_on_remove( + lambda: self._device.remove_callback(self._async_device_removed) + ) @callback def _async_device_changed(self, *args, **kwargs) -> None: @@ -189,24 +195,16 @@ class HomematicipGenericEntity(Entity): @override async def async_will_remove_from_hass(self) -> None: """Run when hmip device will be removed from hass.""" + self._hap.hmip_device_by_entity_id.pop(self.entity_id, None) # Only go further if the device/entity should be removed from registries # due to a removal of the HmIP device. - if self.hmip_device_removed: - try: - del self._hap.hmip_device_by_entity_id[self.entity_id] - self.async_remove_from_registries() - except KeyError as err: - _LOGGER.debug("Error removing HMIP device from registry: %s", err) + self.async_remove_from_registries() @callback def async_remove_from_registries(self) -> None: """Remove entity/device from registry.""" - # Remove callback from device. - self._device.remove_callback(self._async_device_changed) - self._device.remove_callback(self._async_device_removed) - if not self.registry_entry: return diff --git a/homeassistant/components/homematicip_cloud/event.py b/homeassistant/components/homematicip_cloud/event.py index 57a5bd24c8da..38c5b807b54d 100644 --- a/homeassistant/components/homematicip_cloud/event.py +++ b/homeassistant/components/homematicip_cloud/event.py @@ -2,6 +2,7 @@ from collections.abc import Callable from dataclasses import dataclass +from functools import partial from typing import override from homematicip.base.channel_event import ChannelEvent @@ -15,6 +16,10 @@ from homeassistant.components.event import ( EventEntityDescription, ) from homeassistant.core import HomeAssistant, callback +from homeassistant.helpers.dispatcher import ( + async_dispatcher_connect, + async_dispatcher_send, +) from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback from .entity import HomematicipGenericEntity @@ -102,6 +107,7 @@ class HomematicipChannelEvent(HomematicipGenericEntity, EventEntity): use_description_name=description.is_multi_channel, ) self.entity_description = description + self._channel_event_signal: str | None = None if description.is_multi_channel: self._attr_translation_placeholders = {"channel": str(channel.index)} @@ -110,8 +116,21 @@ class HomematicipChannelEvent(HomematicipGenericEntity, EventEntity): """Register callbacks.""" await super().async_added_to_hass() - channel = self.get_channel_or_raise() - channel.add_on_channel_event_handler(self._async_handle_event) + # homematicip can't remove channel event handlers, so register a + # forwarder once and (un)subscribe the entity through the dispatcher. + if self._channel_event_signal is None: + self._channel_event_signal = ( + f"homematicip_cloud_channel_event_{self.unique_id}" + ) + channel = self.get_channel_or_raise() + channel.add_on_channel_event_handler( + partial(async_dispatcher_send, self.hass, self._channel_event_signal) + ) + self.async_on_remove( + async_dispatcher_connect( + self.hass, self._channel_event_signal, self._async_handle_event + ) + ) @callback def _async_handle_event(self, *args, **kwargs) -> None: diff --git a/tests/components/homematicip_cloud/test_alarm_control_panel.py b/tests/components/homematicip_cloud/test_alarm_control_panel.py index 1ca88581584e..5b7d3ed072a2 100644 --- a/tests/components/homematicip_cloud/test_alarm_control_panel.py +++ b/tests/components/homematicip_cloud/test_alarm_control_panel.py @@ -1,5 +1,6 @@ """Tests for HomematicIP Cloud alarm control panel.""" +from typing import Any from unittest.mock import Mock from homematicip.async_home import AsyncHome @@ -8,6 +9,7 @@ import pytest from homeassistant.components.alarm_control_panel import AlarmControlPanelState from homeassistant.core import HomeAssistant from homeassistant.exceptions import HomeAssistantError +from homeassistant.helpers import entity_registry as er from .helper import HomeFactory, get_and_check_entity_basics @@ -155,3 +157,37 @@ async def test_hmip_alarm_control_panel_activation_failed( {"entity_id": entity_id}, blocking=True, ) + + +async def test_hmip_alarm_control_panel_removal_removes_home_callback( + hass: HomeAssistant, + entity_registry: er.EntityRegistry, + default_mock_hap_factory: HomeFactory, +) -> None: + """Test removing the alarm panel removes its home update handler.""" + entity_id = "alarm_control_panel.hmip_alarm_control_panel" + mock_hap = await default_mock_hap_factory.async_get_mock_hap( + test_groups=["EXTERNAL", "INTERNAL"] + ) + home = mock_hap.home + + def _entity_handlers() -> list[Any]: + return [ + handler + for handler in home._on_update + if getattr(getattr(handler, "__self__", None), "entity_id", None) + == entity_id + ] + + assert len(_entity_handlers()) == 1 + + entity_registry.async_remove(entity_id) + await hass.async_block_till_done() + + assert _entity_handlers() == [] + assert hass.states.get(entity_id) is None + + await _async_manipulate_security_zones( + hass, home, internal_active=True, external_active=True + ) + assert hass.states.get(entity_id) is None diff --git a/tests/components/homematicip_cloud/test_device.py b/tests/components/homematicip_cloud/test_device.py index 3d160a19bcfd..ba0295273675 100644 --- a/tests/components/homematicip_cloud/test_device.py +++ b/tests/components/homematicip_cloud/test_device.py @@ -1,5 +1,6 @@ """Common tests for HomematicIP devices.""" +from collections.abc import Callable from typing import Any from unittest.mock import patch @@ -364,3 +365,57 @@ async def test_hmip_unknown_device_type( state = hass.states.get(entry.entity_id) assert state is not None assert state.state != STATE_UNAVAILABLE + + +def _entity_handlers(handlers: list[Callable[..., Any]], entity_id: str) -> list[Any]: + """Return the handlers bound to the entity with the given entity_id.""" + return [ + handler + for handler in handlers + if getattr(getattr(handler, "__self__", None), "entity_id", None) == entity_id + ] + + +async def test_hmip_entity_removal_removes_device_callbacks( + hass: HomeAssistant, + entity_registry: er.EntityRegistry, + default_mock_hap_factory: HomeFactory, +) -> None: + """Test removing an entity drops its device handlers and mapping.""" + entity_id = "switch.schrank" + mock_hap = await default_mock_hap_factory.async_get_mock_hap( + test_devices=["Schrank"] + ) + hmip_device = mock_hap.hmip_device_by_entity_id[entity_id] + assert len(_entity_handlers(hmip_device._on_update, entity_id)) == 1 + assert len(_entity_handlers(hmip_device._on_remove, entity_id)) == 1 + + entity_registry.async_remove(entity_id) + await hass.async_block_till_done() + + assert _entity_handlers(hmip_device._on_update, entity_id) == [] + assert _entity_handlers(hmip_device._on_remove, entity_id) == [] + assert entity_id not in mock_hap.hmip_device_by_entity_id + + +async def test_hmip_entity_id_change_keeps_single_device_callbacks( + hass: HomeAssistant, + entity_registry: er.EntityRegistry, + default_mock_hap_factory: HomeFactory, +) -> None: + """Test an entity_id change re-keys the mapping without duplicate handlers.""" + entity_id = "switch.schrank" + new_entity_id = "switch.renamed" + mock_hap = await default_mock_hap_factory.async_get_mock_hap( + test_devices=["Schrank"] + ) + hmip_device = mock_hap.hmip_device_by_entity_id[entity_id] + + # Changing the entity_id removes and re-adds the same entity object. + entity_registry.async_update_entity(entity_id, new_entity_id=new_entity_id) + await hass.async_block_till_done() + + assert entity_id not in mock_hap.hmip_device_by_entity_id + assert mock_hap.hmip_device_by_entity_id[new_entity_id] is hmip_device + assert len(_entity_handlers(hmip_device._on_update, new_entity_id)) == 1 + assert len(_entity_handlers(hmip_device._on_remove, new_entity_id)) == 1 diff --git a/tests/components/homematicip_cloud/test_event.py b/tests/components/homematicip_cloud/test_event.py index cf6c32b0badd..c10e860faf4d 100644 --- a/tests/components/homematicip_cloud/test_event.py +++ b/tests/components/homematicip_cloud/test_event.py @@ -1,10 +1,16 @@ """Tests for the HomematicIP Cloud event.""" +from unittest.mock import patch + from homematicip.base.channel_event import ChannelEvent +from homematicip.base.functionalChannels import FunctionalChannel import pytest +from homeassistant.components.event import DOMAIN as EVENT_DOMAIN from homeassistant.const import STATE_UNKNOWN from homeassistant.core import HomeAssistant +from homeassistant.helpers import entity_registry as er +from homeassistant.helpers.entity_component import DATA_INSTANCES from .helper import HomeFactory, get_and_check_entity_basics @@ -143,3 +149,69 @@ async def test_wrc6_button_ignores_repeating_long( state_after_long = hass.states.get(entity_id) assert state_after_long.state == state_after_short.state assert state_after_long.attributes["event_type"] == "short_release" + + +DOORBELL_ENTITY_ID = "event.dsdpcb_klingel_doorbell" + + +def _door_bell_event(channel: FunctionalChannel) -> ChannelEvent: + """Return a door bell channel event for the channel.""" + return ChannelEvent( + channelEventType="DOOR_BELL_SENSOR_EVENT", + channelIndex=channel.index, + deviceId=channel.device.id, + ) + + +async def test_door_bell_event_after_entity_removed( + hass: HomeAssistant, + entity_registry: er.EntityRegistry, + default_mock_hap_factory: HomeFactory, +) -> None: + """Test a removed event entity no longer handles channel events.""" + mock_hap = await default_mock_hap_factory.async_get_mock_hap( + test_devices=["dsdpcb_klingel"] + ) + channel = mock_hap.hmip_device_by_entity_id[DOORBELL_ENTITY_ID].functionalChannels[ + 1 + ] + entity = hass.data[DATA_INSTANCES][EVENT_DOMAIN].get_entity(DOORBELL_ENTITY_ID) + + entity_registry.async_remove(DOORBELL_ENTITY_ID) + await hass.async_block_till_done() + + with patch.object(entity, "_trigger_event") as mock_trigger_event: + channel.fire_channel_event(_door_bell_event(channel)) + await hass.async_block_till_done() + + mock_trigger_event.assert_not_called() + + +async def test_door_bell_event_after_entity_id_change( + hass: HomeAssistant, + entity_registry: er.EntityRegistry, + default_mock_hap_factory: HomeFactory, +) -> None: + """Test a renamed event entity handles each channel event once.""" + mock_hap = await default_mock_hap_factory.async_get_mock_hap( + test_devices=["dsdpcb_klingel"] + ) + channel = mock_hap.hmip_device_by_entity_id[DOORBELL_ENTITY_ID].functionalChannels[ + 1 + ] + entity = hass.data[DATA_INSTANCES][EVENT_DOMAIN].get_entity(DOORBELL_ENTITY_ID) + handler_count = len(channel._on_channel_event_handler) + + # Changing the entity_id removes and re-adds the same entity object. + entity_registry.async_update_entity( + DOORBELL_ENTITY_ID, new_entity_id="event.renamed_doorbell" + ) + await hass.async_block_till_done() + + assert len(channel._on_channel_event_handler) == handler_count + + with patch.object(entity, "_trigger_event") as mock_trigger_event: + channel.fire_channel_event(_door_bell_event(channel)) + await hass.async_block_till_done() + + mock_trigger_event.assert_called_once_with(event_type="ring")