From 2e71c8d43b03443ddef87979a83eead8c8d65683 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 12 Jul 2024 09:29:55 -0500 Subject: [PATCH] Fix homekit linked doorbell and motion firing on reload (#121818) --- .../components/homekit/type_cameras.py | 37 ++++---- tests/components/homekit/test_type_cameras.py | 85 ++++++++++++++++++- 2 files changed, 106 insertions(+), 16 deletions(-) diff --git a/homeassistant/components/homekit/type_cameras.py b/homeassistant/components/homekit/type_cameras.py index 40fd6b2aade..3851bb43541 100644 --- a/homeassistant/components/homekit/type_cameras.py +++ b/homeassistant/components/homekit/type_cameras.py @@ -233,8 +233,7 @@ class Camera(HomeAccessory, PyhapCamera): # type: ignore[misc] self._char_motion_detected = serv_motion.configure_char( CHAR_MOTION_DETECTED, value=False ) - if not self.motion_is_event: - self._async_update_motion_state(state) + self._async_update_motion_state(None, state) self._char_doorbell_detected = None self._char_doorbell_detected_switch = None @@ -264,9 +263,7 @@ class Camera(HomeAccessory, PyhapCamera): # type: ignore[misc] ) serv_speaker = self.add_preload_service(SERV_SPEAKER) serv_speaker.configure_char(CHAR_MUTE, value=0) - - if not self.doorbell_is_event: - self._async_update_doorbell_state(state) + self._async_update_doorbell_state(None, state) @pyhap_callback # type: ignore[misc] @callback @@ -304,20 +301,25 @@ class Camera(HomeAccessory, PyhapCamera): # type: ignore[misc] self, event: Event[EventStateChangedData] ) -> None: """Handle state change event listener callback.""" - if not state_changed_event_is_same_state(event): - self._async_update_motion_state(event.data["new_state"]) + if not state_changed_event_is_same_state(event) and ( + new_state := event.data["new_state"] + ): + self._async_update_motion_state(event.data["old_state"], new_state) @callback - def _async_update_motion_state(self, new_state: State | None) -> None: + def _async_update_motion_state( + self, old_state: State | None, new_state: State + ) -> None: """Handle link motion sensor state change to update HomeKit value.""" - if not new_state: - return - state = new_state.state char = self._char_motion_detected assert char is not None if self.motion_is_event: - if state in (STATE_UNKNOWN, STATE_UNAVAILABLE): + if ( + old_state is None + or old_state.state == STATE_UNAVAILABLE + or state in (STATE_UNKNOWN, STATE_UNAVAILABLE) + ): return _LOGGER.debug( "%s: Set linked motion %s sensor to True/False", @@ -348,16 +350,21 @@ class Camera(HomeAccessory, PyhapCamera): # type: ignore[misc] if not state_changed_event_is_same_state(event) and ( new_state := event.data["new_state"] ): - self._async_update_doorbell_state(new_state) + self._async_update_doorbell_state(event.data["old_state"], new_state) @callback - def _async_update_doorbell_state(self, new_state: State) -> None: + def _async_update_doorbell_state( + self, old_state: State | None, new_state: State + ) -> None: """Handle link doorbell sensor state change to update HomeKit value.""" assert self._char_doorbell_detected assert self._char_doorbell_detected_switch state = new_state.state if state == STATE_ON or ( - self.doorbell_is_event and state not in (STATE_UNKNOWN, STATE_UNAVAILABLE) + self.doorbell_is_event + and old_state is not None + and old_state.state != STATE_UNAVAILABLE + and state not in (STATE_UNKNOWN, STATE_UNAVAILABLE) ): self._char_doorbell_detected.set_value(DOORBELL_SINGLE_PRESS) self._char_doorbell_detected_switch.set_value(DOORBELL_SINGLE_PRESS) diff --git a/tests/components/homekit/test_type_cameras.py b/tests/components/homekit/test_type_cameras.py index 69f76006163..a32656e9f2b 100644 --- a/tests/components/homekit/test_type_cameras.py +++ b/tests/components/homekit/test_type_cameras.py @@ -31,7 +31,13 @@ from homeassistant.components.homekit.const import ( ) from homeassistant.components.homekit.type_cameras import Camera from homeassistant.components.homekit.type_switches import Switch -from homeassistant.const import ATTR_DEVICE_CLASS, STATE_OFF, STATE_ON, STATE_UNKNOWN +from homeassistant.const import ( + ATTR_DEVICE_CLASS, + STATE_OFF, + STATE_ON, + STATE_UNAVAILABLE, + STATE_UNKNOWN, +) from homeassistant.core import HomeAssistant from homeassistant.exceptions import HomeAssistantError from homeassistant.setup import async_setup_component @@ -883,6 +889,54 @@ async def test_camera_with_linked_motion_event(hass: HomeAssistant, run_driver) await hass.async_block_till_done() assert char.value is False + # Ensure re-adding does not fire an event + hass.states.async_set( + motion_entity_id, + dt_util.utcnow().isoformat(), + {ATTR_DEVICE_CLASS: EventDeviceClass.MOTION, "other": "attr"}, + ) + await hass.async_block_till_done() + assert not broker.mock_calls + + # But a second update does + broker.reset_mock() + hass.states.async_set( + motion_entity_id, + dt_util.utcnow().isoformat(), + {ATTR_DEVICE_CLASS: EventDeviceClass.MOTION}, + ) + await hass.async_block_till_done() + assert broker.mock_calls + + # Now go unavailable + broker.reset_mock() + hass.states.async_set( + motion_entity_id, + STATE_UNAVAILABLE, + {ATTR_DEVICE_CLASS: EventDeviceClass.MOTION}, + ) + await hass.async_block_till_done() + assert not broker.mock_calls + + # Going from unavailable to a state should not fire an event + hass.states.async_set( + motion_entity_id, + dt_util.utcnow().isoformat(), + {ATTR_DEVICE_CLASS: EventDeviceClass.MOTION}, + ) + await hass.async_block_till_done() + assert not broker.mock_calls + + # But a another update does + broker.reset_mock() + hass.states.async_set( + motion_entity_id, + dt_util.utcnow().isoformat(), + {ATTR_DEVICE_CLASS: EventDeviceClass.MOTION, "other": "attr"}, + ) + await hass.async_block_till_done() + assert broker.mock_calls + async def test_camera_with_a_missing_linked_motion_sensor( hass: HomeAssistant, run_driver @@ -1148,6 +1202,35 @@ async def test_camera_with_linked_doorbell_event( assert char.value is None assert char2.value is None + await hass.async_block_till_done() + hass.states.async_set( + doorbell_entity_id, + STATE_UNAVAILABLE, + {ATTR_DEVICE_CLASS: EventDeviceClass.DOORBELL}, + ) + await hass.async_block_till_done() + # Ensure re-adding does not fire an event + assert not broker.mock_calls + broker.reset_mock() + + # going from unavailable to a state should not fire an event + hass.states.async_set( + doorbell_entity_id, + dt_util.utcnow().isoformat(), + {ATTR_DEVICE_CLASS: EventDeviceClass.DOORBELL}, + ) + await hass.async_block_till_done() + assert not broker.mock_calls + + # But a second update does + hass.states.async_set( + doorbell_entity_id, + dt_util.utcnow().isoformat(), + {ATTR_DEVICE_CLASS: EventDeviceClass.DOORBELL}, + ) + await hass.async_block_till_done() + assert broker.mock_calls + async def test_camera_with_a_missing_linked_doorbell_sensor( hass: HomeAssistant, run_driver