From 711b8e10a3348fdbb156fb9b4ff118937d23dcac Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 5 Jul 2020 21:05:13 -0500 Subject: [PATCH] Switch homekit to use async_track_state_change_event (#37253) * Switch homekit to use async_track_state_change_event Calling async_track_state_change_event directly is faster than async_track_state_change and has slightly lower latency triggering state updates in homekit * check for deleted entities * Update additional tests for linked sensor removals * Ensure removing entities does not result in an exception --- .../components/homekit/accessories.py | 49 ++++++++------- tests/components/homekit/test_accessories.py | 60 ++++++++++++++++++- 2 files changed, 85 insertions(+), 24 deletions(-) diff --git a/homeassistant/components/homekit/accessories.py b/homeassistant/components/homekit/accessories.py index feced5d11ac..b2fe3ca3f54 100644 --- a/homeassistant/components/homekit/accessories.py +++ b/homeassistant/components/homekit/accessories.py @@ -33,7 +33,7 @@ from homeassistant.const import ( ) from homeassistant.core import callback as ha_callback, split_entity_id from homeassistant.helpers.event import ( - async_track_state_change, + async_track_state_change_event, track_point_in_utc_time, ) from homeassistant.util import dt as dt_util @@ -346,10 +346,10 @@ class HomeAccessory(Accessory): Run inside the Home Assistant event loop. """ state = self.hass.states.get(self.entity_id) - self.async_update_state_callback(None, None, state) + self.async_update_state_callback(state) self._subscriptions.append( - async_track_state_change( - self.hass, self.entity_id, self.async_update_state_callback + async_track_state_change_event( + self.hass, [self.entity_id], self.async_update_event_state_callback ) ) @@ -364,36 +364,37 @@ class HomeAccessory(Accessory): ATTR_BATTERY_CHARGING ) self._subscriptions.append( - async_track_state_change( + async_track_state_change_event( self.hass, - self.linked_battery_sensor, + [self.linked_battery_sensor], self.async_update_linked_battery_callback, ) ) - else: + elif state is not None: battery_state = state.attributes.get(ATTR_BATTERY_LEVEL) if self.linked_battery_charging_sensor: - battery_charging_state = ( - self.hass.states.get(self.linked_battery_charging_sensor).state - == STATE_ON - ) + state = self.hass.states.get(self.linked_battery_charging_sensor) + battery_charging_state = state and state.state == STATE_ON self._subscriptions.append( - async_track_state_change( + async_track_state_change_event( self.hass, - self.linked_battery_charging_sensor, + [self.linked_battery_charging_sensor], self.async_update_linked_battery_charging_callback, ) ) - elif battery_charging_state is None: + elif battery_charging_state is None and state is not None: battery_charging_state = state.attributes.get(ATTR_BATTERY_CHARGING) if battery_state is not None or battery_charging_state is not None: self.async_update_battery(battery_state, battery_charging_state) @ha_callback - def async_update_state_callback( - self, entity_id=None, old_state=None, new_state=None - ): + def async_update_event_state_callback(self, event): + """Handle state change event listener callback.""" + self.async_update_state_callback(event.data.get("new_state")) + + @ha_callback + def async_update_state_callback(self, new_state): """Handle state change listener callback.""" _LOGGER.debug("New_state: %s", new_state) if new_state is None: @@ -415,10 +416,11 @@ class HomeAccessory(Accessory): self.async_update_state(new_state) @ha_callback - def async_update_linked_battery_callback( - self, entity_id=None, old_state=None, new_state=None - ): + def async_update_linked_battery_callback(self, event): """Handle linked battery sensor state change listener callback.""" + new_state = event.data.get("new_state") + if new_state is None: + return if self.linked_battery_charging_sensor: battery_charging_state = None else: @@ -426,10 +428,11 @@ class HomeAccessory(Accessory): self.async_update_battery(new_state.state, battery_charging_state) @ha_callback - def async_update_linked_battery_charging_callback( - self, entity_id=None, old_state=None, new_state=None - ): + def async_update_linked_battery_charging_callback(self, event): """Handle linked battery charging sensor state change listener callback.""" + new_state = event.data.get("new_state") + if new_state is None: + return self.async_update_battery(None, new_state.state == STATE_ON) @ha_callback diff --git a/tests/components/homekit/test_accessories.py b/tests/components/homekit/test_accessories.py index b19e63d3b4c..91f02522126 100644 --- a/tests/components/homekit/test_accessories.py +++ b/tests/components/homekit/test_accessories.py @@ -207,6 +207,7 @@ async def test_battery_service(hass, hk_driver, caplog): await hass.async_block_till_done() state = hass.states.get(entity_id) mock_async_update_state.assert_called_with(state) + assert acc._char_battery.value == 15 assert acc._char_low_battery.value == 1 assert acc._char_charging.value == 2 @@ -218,6 +219,7 @@ async def test_battery_service(hass, hk_driver, caplog): await hass.async_block_till_done() state = hass.states.get(entity_id) mock_async_update_state.assert_called_with(state) + assert acc._char_battery.value == 15 assert acc._char_low_battery.value == 1 assert acc._char_charging.value == 2 @@ -341,7 +343,12 @@ async def test_linked_battery_sensor(hass, hk_driver, caplog): hass.states.async_set(linked_battery, 100, {ATTR_BATTERY_CHARGING: False}) await hass.async_block_till_done() - state = hass.states.get(entity_id) + assert acc._char_battery.value == 100 + assert acc._char_low_battery.value == 0 + assert acc._char_charging.value == 0 + + hass.states.async_remove(linked_battery) + await hass.async_block_till_done() assert acc._char_battery.value == 100 assert acc._char_low_battery.value == 0 assert acc._char_charging.value == 0 @@ -396,6 +403,14 @@ async def test_linked_battery_charging_sensor(hass, hk_driver, caplog): mock_async_update_state.assert_called_with(state) assert acc._char_charging.value == 1 + with patch( + "homeassistant.components.homekit.accessories.HomeAccessory.async_update_state" + ) as mock_async_update_state: + hass.states.async_remove(linked_battery_charging_sensor) + await acc.run_handler() + await hass.async_block_till_done() + assert acc._char_charging.value == 1 + async def test_linked_battery_sensor_and_linked_battery_charging_sensor( hass, hk_driver, caplog @@ -439,6 +454,12 @@ async def test_linked_battery_sensor_and_linked_battery_charging_sensor( assert acc._char_low_battery.value == 0 assert acc._char_charging.value == 0 + hass.states.async_remove(linked_battery_charging_sensor) + await hass.async_block_till_done() + assert acc._char_battery.value == 50 + assert acc._char_low_battery.value == 0 + assert acc._char_charging.value == 0 + async def test_missing_linked_battery_charging_sensor(hass, hk_driver, caplog): """Test battery service with linked_battery_charging_sensor that is mapping to a missing entity.""" @@ -457,6 +478,24 @@ async def test_missing_linked_battery_charging_sensor(hass, hk_driver, caplog): ) assert acc.linked_battery_charging_sensor is None + # Make sure we don't throw if the linked_battery_charging_sensor + # is removed + hass.states.async_remove(linked_battery_charging_sensor) + with patch( + "homeassistant.components.homekit.accessories.HomeAccessory.async_update_state" + ): + await acc.run_handler() + await hass.async_block_till_done() + + # Make sure we don't throw if the entity_id + # is removed + hass.states.async_remove(entity_id) + with patch( + "homeassistant.components.homekit.accessories.HomeAccessory.async_update_state" + ): + await acc.run_handler() + await hass.async_block_till_done() + async def test_missing_linked_battery_sensor(hass, hk_driver, caplog): """Test battery service with missing linked_battery_sensor.""" @@ -488,6 +527,18 @@ async def test_missing_linked_battery_sensor(hass, hk_driver, caplog): assert acc._char_low_battery is None assert acc._char_charging is None + with patch( + "homeassistant.components.homekit.accessories.HomeAccessory.async_update_state" + ) as mock_async_update_state: + hass.states.async_remove(entity_id) + await acc.run_handler() + await hass.async_block_till_done() + + assert not acc.linked_battery_sensor + assert acc._char_battery is None + assert acc._char_low_battery is None + assert acc._char_charging is None + async def test_battery_appears_after_startup(hass, hk_driver, caplog): """Test battery level appears after homekit is started.""" @@ -514,6 +565,13 @@ async def test_battery_appears_after_startup(hass, hk_driver, caplog): await hass.async_block_till_done() assert acc._char_battery is None + with patch( + "homeassistant.components.homekit.accessories.HomeAccessory.async_update_state" + ): + hass.states.async_remove(entity_id) + await hass.async_block_till_done() + assert acc._char_battery is None + async def test_call_service(hass, hk_driver, events): """Test call_service method."""