From ec8a33b52d1fe20553d72f126d94cafedf7c1ea3 Mon Sep 17 00:00:00 2001 From: Christopher Bailey Date: Wed, 10 Jan 2024 23:06:45 -0500 Subject: [PATCH] Rework state change detection for UniFi Protect entities (#107766) --- .../components/unifiprotect/binary_sensor.py | 58 +++++++++-------- .../components/unifiprotect/button.py | 21 ------- .../components/unifiprotect/camera.py | 16 ++++- .../components/unifiprotect/entity.py | 30 ++++++++- .../components/unifiprotect/light.py | 10 +++ homeassistant/components/unifiprotect/lock.py | 16 +++++ .../components/unifiprotect/media_player.py | 33 ++-------- .../components/unifiprotect/number.py | 30 +++------ .../components/unifiprotect/select.py | 33 ++-------- .../components/unifiprotect/sensor.py | 63 ++++++++----------- .../components/unifiprotect/switch.py | 29 ++------- homeassistant/components/unifiprotect/text.py | 11 ++++ .../unifiprotect/test_binary_sensor.py | 2 +- tests/components/unifiprotect/utils.py | 10 +++ 14 files changed, 173 insertions(+), 189 deletions(-) diff --git a/homeassistant/components/unifiprotect/binary_sensor.py b/homeassistant/components/unifiprotect/binary_sensor.py index 8cb3311a40d..5aff0b732df 100644 --- a/homeassistant/components/unifiprotect/binary_sensor.py +++ b/homeassistant/components/unifiprotect/binary_sensor.py @@ -3,6 +3,7 @@ from __future__ import annotations import dataclasses import logging +from typing import Any from pyunifiprotect.data import ( NVR, @@ -573,6 +574,16 @@ class ProtectDeviceBinarySensor(ProtectDeviceEntity, BinarySensorEntity): else: self._attr_device_class = self.entity_description.device_class + @callback + def _async_get_state_attrs(self) -> tuple[Any, ...]: + """Retrieve data that goes into the current state of the entity. + + Called before and after updating entity and state is only written if there + is a change. + """ + + return (self._attr_available, self._attr_is_on, self._attr_device_class) + class ProtectDiskBinarySensor(ProtectNVREntity, BinarySensorEntity): """A UniFi Protect NVR Disk Binary Sensor.""" @@ -617,6 +628,16 @@ class ProtectDiskBinarySensor(ProtectNVREntity, BinarySensorEntity): self._attr_is_on = not self._disk.is_healthy + @callback + def _async_get_state_attrs(self) -> tuple[Any, ...]: + """Retrieve data that goes into the current state of the entity. + + Called before and after updating entity and state is only written if there + is a change. + """ + + return (self._attr_available, self._attr_is_on) + class ProtectEventBinarySensor(EventEntityMixin, BinarySensorEntity): """A UniFi Protect Device Binary Sensor for events.""" @@ -633,32 +654,15 @@ class ProtectEventBinarySensor(EventEntityMixin, BinarySensorEntity): self._attr_extra_state_attributes = {} @callback - def _async_updated_event(self, device: ProtectModelWithId) -> None: - """Call back for incoming data that only writes when state has changed. + def _async_get_state_attrs(self) -> tuple[Any, ...]: + """Retrieve data that goes into the current state of the entity. - Only the is_on, _attr_extra_state_attributes, and available are ever - updated for these entities, and since the websocket update for the - device will trigger an update for all entities connected to the device, - we want to avoid writing state unless something has actually changed. + Called before and after updating entity and state is only written if there + is a change. """ - previous_is_on = self._attr_is_on - previous_available = self._attr_available - previous_extra_state_attributes = self._attr_extra_state_attributes - self._async_update_device_from_protect(device) - if ( - self._attr_is_on != previous_is_on - or self._attr_extra_state_attributes != previous_extra_state_attributes - or self._attr_available != previous_available - ): - _LOGGER.debug( - "Updating state [%s (%s)] %s (%s, %s) -> %s (%s, %s)", - device.name, - device.mac, - previous_is_on, - previous_available, - previous_extra_state_attributes, - self._attr_is_on, - self._attr_available, - self._attr_extra_state_attributes, - ) - self.async_write_ha_state() + + return ( + self._attr_available, + self._attr_is_on, + self._attr_extra_state_attributes, + ) diff --git a/homeassistant/components/unifiprotect/button.py b/homeassistant/components/unifiprotect/button.py index b69fbb95970..c0872e03f03 100644 --- a/homeassistant/components/unifiprotect/button.py +++ b/homeassistant/components/unifiprotect/button.py @@ -193,24 +193,3 @@ class ProtectButton(ProtectDeviceEntity, ButtonEntity): if self.entity_description.ufp_press is not None: await getattr(self.device, self.entity_description.ufp_press)() - - @callback - def _async_updated_event(self, device: ProtectModelWithId) -> None: - """Call back for incoming data that only writes when state has changed. - - Only available is updated for these entities, and since the websocket - update for the device will trigger an update for all entities connected - to the device, we want to avoid writing state unless something has - actually changed. - """ - previous_available = self._attr_available - self._async_update_device_from_protect(device) - if self._attr_available != previous_available: - _LOGGER.debug( - "Updating state [%s (%s)] %s -> %s", - device.name, - device.mac, - previous_available, - self._attr_available, - ) - self.async_write_ha_state() diff --git a/homeassistant/components/unifiprotect/camera.py b/homeassistant/components/unifiprotect/camera.py index dc579ad6b7c..6d82e2fc989 100644 --- a/homeassistant/components/unifiprotect/camera.py +++ b/homeassistant/components/unifiprotect/camera.py @@ -3,7 +3,7 @@ from __future__ import annotations from collections.abc import Generator import logging -from typing import cast +from typing import Any, cast from pyunifiprotect.data import ( Camera as UFPCamera, @@ -181,6 +181,20 @@ class ProtectCamera(ProtectDeviceEntity, Camera): else: self._attr_supported_features = CameraEntityFeature(0) + @callback + def _async_get_state_attrs(self) -> tuple[Any, ...]: + """Retrieve data that goes into the current state of the entity. + + Called before and after updating entity and state is only written if there + is a change. + """ + + return ( + self._attr_available, + self._attr_is_recording, + self._attr_motion_detection_enabled, + ) + @callback def _async_update_device_from_protect(self, device: ProtectModelWithId) -> None: super()._async_update_device_from_protect(device) diff --git a/homeassistant/components/unifiprotect/entity.py b/homeassistant/components/unifiprotect/entity.py index 00160005fe0..59c716d4aa4 100644 --- a/homeassistant/components/unifiprotect/entity.py +++ b/homeassistant/components/unifiprotect/entity.py @@ -255,11 +255,37 @@ class ProtectDeviceEntity(Entity): and (not async_get_ufp_enabled or async_get_ufp_enabled(device)) ) + @callback + def _async_get_state_attrs(self) -> tuple[Any, ...]: + """Retrieve data that goes into the current state of the entity. + + Called before and after updating entity and state is only written if there + is a change. + """ + + return (self._attr_available,) + @callback def _async_updated_event(self, device: ProtectModelWithId) -> None: - """Call back for incoming data.""" + """When device is updated from Protect.""" + + previous_attrs = self._async_get_state_attrs() self._async_update_device_from_protect(device) - self.async_write_ha_state() + current_attrs = self._async_get_state_attrs() + if previous_attrs != current_attrs: + if _LOGGER.isEnabledFor(logging.DEBUG): + device_name = device.name + if hasattr(self, "entity_description") and self.entity_description.name: + device_name += f" {self.entity_description.name}" + + _LOGGER.debug( + "Updating state [%s (%s)] %s -> %s", + device_name, + device.mac, + previous_attrs, + current_attrs, + ) + self.async_write_ha_state() async def async_added_to_hass(self) -> None: """When entity is added to hass.""" diff --git a/homeassistant/components/unifiprotect/light.py b/homeassistant/components/unifiprotect/light.py index 6cc56009cea..485e715ea39 100644 --- a/homeassistant/components/unifiprotect/light.py +++ b/homeassistant/components/unifiprotect/light.py @@ -70,6 +70,16 @@ class ProtectLight(ProtectDeviceEntity, LightEntity): _attr_color_mode = ColorMode.BRIGHTNESS _attr_supported_color_modes = {ColorMode.BRIGHTNESS} + @callback + def _async_get_state_attrs(self) -> tuple[Any, ...]: + """Retrieve data that goes into the current state of the entity. + + Called before and after updating entity and state is only written if there + is a change. + """ + + return (self._attr_available, self._attr_brightness) + @callback def _async_update_device_from_protect(self, device: ProtectModelWithId) -> None: super()._async_update_device_from_protect(device) diff --git a/homeassistant/components/unifiprotect/lock.py b/homeassistant/components/unifiprotect/lock.py index 2b7ce4f1147..57ade8ad220 100644 --- a/homeassistant/components/unifiprotect/lock.py +++ b/homeassistant/components/unifiprotect/lock.py @@ -70,6 +70,22 @@ class ProtectLock(ProtectDeviceEntity, LockEntity): self._attr_name = f"{self.device.display_name} Lock" + @callback + def _async_get_state_attrs(self) -> tuple[Any, ...]: + """Retrieve data that goes into the current state of the entity. + + Called before and after updating entity and state is only written if there + is a change. + """ + + return ( + self._attr_available, + self._attr_is_locked, + self._attr_is_locking, + self._attr_is_unlocking, + self._attr_is_jammed, + ) + @callback def _async_update_device_from_protect(self, device: ProtectModelWithId) -> None: super()._async_update_device_from_protect(device) diff --git a/homeassistant/components/unifiprotect/media_player.py b/homeassistant/components/unifiprotect/media_player.py index 5daac033048..e0f247eef72 100644 --- a/homeassistant/components/unifiprotect/media_player.py +++ b/homeassistant/components/unifiprotect/media_player.py @@ -116,35 +116,14 @@ class ProtectMediaPlayer(ProtectDeviceEntity, MediaPlayerEntity): self._attr_available = is_connected and updated_device.feature_flags.has_speaker @callback - def _async_updated_event(self, device: ProtectModelWithId) -> None: - """Call back for incoming data that only writes when state has changed. + def _async_get_state_attrs(self) -> tuple[Any, ...]: + """Retrieve data that goes into the current state of the entity. - Only the state, volume, and available are ever updated for these - entities, and since the websocket update for the device will trigger - an update for all entities connected to the device, we want to avoid - writing state unless something has actually changed. + Called before and after updating entity and state is only written if there + is a change. """ - previous_state = self._attr_state - previous_available = self._attr_available - previous_volume_level = self._attr_volume_level - self._async_update_device_from_protect(device) - if ( - self._attr_state != previous_state - or self._attr_volume_level != previous_volume_level - or self._attr_available != previous_available - ): - _LOGGER.debug( - "Updating state [%s (%s)] %s (%s, %s) -> %s (%s, %s)", - device.name, - device.mac, - previous_state, - previous_available, - previous_volume_level, - self._attr_state, - self._attr_available, - self._attr_volume_level, - ) - self.async_write_ha_state() + + return (self._attr_available, self._attr_state, self._attr_volume_level) async def async_set_volume_level(self, volume: float) -> None: """Set volume level, range 0..1.""" diff --git a/homeassistant/components/unifiprotect/number.py b/homeassistant/components/unifiprotect/number.py index c02753a9401..04f779ecbd7 100644 --- a/homeassistant/components/unifiprotect/number.py +++ b/homeassistant/components/unifiprotect/number.py @@ -4,6 +4,7 @@ from __future__ import annotations from dataclasses import dataclass from datetime import timedelta import logging +from typing import Any from pyunifiprotect.data import ( Camera, @@ -273,28 +274,11 @@ class ProtectNumbers(ProtectDeviceEntity, NumberEntity): await self.entity_description.ufp_set(self.device, value) @callback - def _async_updated_event(self, device: ProtectModelWithId) -> None: - """Call back for incoming data that only writes when state has changed. + def _async_get_state_attrs(self) -> tuple[Any, ...]: + """Retrieve data that goes into the current state of the entity. - Only the native value and available are ever updated for these - entities, and since the websocket update for the device will trigger - an update for all entities connected to the device, we want to avoid - writing state unless something has actually changed. + Called before and after updating entity and state is only written if there + is a change. """ - previous_value = self._attr_native_value - previous_available = self._attr_available - self._async_update_device_from_protect(device) - if ( - self._attr_native_value != previous_value - or self._attr_available != previous_available - ): - _LOGGER.debug( - "Updating state [%s (%s)] %s (%s) -> %s (%s)", - device.name, - device.mac, - previous_value, - previous_available, - self._attr_native_value, - self._attr_available, - ) - self.async_write_ha_state() + + return (self._attr_available, self._attr_native_value) diff --git a/homeassistant/components/unifiprotect/select.py b/homeassistant/components/unifiprotect/select.py index ecbc22f5787..a3688916959 100644 --- a/homeassistant/components/unifiprotect/select.py +++ b/homeassistant/components/unifiprotect/select.py @@ -403,32 +403,11 @@ class ProtectSelects(ProtectDeviceEntity, SelectEntity): await self.entity_description.ufp_set(self.device, unifi_value) @callback - def _async_updated_event(self, device: ProtectModelWithId) -> None: - """Call back for incoming data that only writes when state has changed. + def _async_get_state_attrs(self) -> tuple[Any, ...]: + """Retrieve data that goes into the current state of the entity. - Only the options, option, and available are ever updated for these - entities, and since the websocket update for the device will trigger - an update for all entities connected to the device, we want to avoid - writing state unless something has actually changed. + Called before and after updating entity and state is only written if there + is a change. """ - previous_option = self._attr_current_option - previous_options = self._attr_options - previous_available = self._attr_available - self._async_update_device_from_protect(device) - if ( - self._attr_current_option != previous_option - or self._attr_options != previous_options - or self._attr_available != previous_available - ): - _LOGGER.debug( - "Updating state [%s (%s)] %s (%s, %s) -> %s (%s, %s)", - device.name, - device.mac, - previous_option, - previous_available, - previous_options, - self._attr_current_option, - self._attr_available, - self._attr_options, - ) - self.async_write_ha_state() + + return (self._attr_available, self._attr_options, self._attr_current_option) diff --git a/homeassistant/components/unifiprotect/sensor.py b/homeassistant/components/unifiprotect/sensor.py index 3e2bd6ee858..3b9dfbd1f83 100644 --- a/homeassistant/components/unifiprotect/sensor.py +++ b/homeassistant/components/unifiprotect/sensor.py @@ -715,31 +715,14 @@ class ProtectDeviceSensor(ProtectDeviceEntity, SensorEntity): self._attr_native_value = self.entity_description.get_ufp_value(self.device) @callback - def _async_updated_event(self, device: ProtectModelWithId) -> None: - """Call back for incoming data that only writes when state has changed. + def _async_get_state_attrs(self) -> tuple[Any, ...]: + """Retrieve data that goes into the current state of the entity. - Only the native value and available are ever updated for these - entities, and since the websocket update for the device will trigger - an update for all entities connected to the device, we want to avoid - writing state unless something has actually changed. + Called before and after updating entity and state is only written if there + is a change. """ - previous_value = self._attr_native_value - previous_available = self._attr_available - self._async_update_device_from_protect(device) - if ( - self._attr_native_value != previous_value - or self._attr_available != previous_available - ): - _LOGGER.debug( - "Updating state [%s (%s)] %s (%s) -> %s (%s)", - device.name, - device.mac, - previous_value, - previous_available, - self._attr_native_value, - self._attr_available, - ) - self.async_write_ha_state() + + return (self._attr_available, self._attr_native_value) class ProtectNVRSensor(ProtectNVREntity, SensorEntity): @@ -752,22 +735,14 @@ class ProtectNVRSensor(ProtectNVREntity, SensorEntity): self._attr_native_value = self.entity_description.get_ufp_value(self.device) @callback - def _async_updated_event(self, device: ProtectModelWithId) -> None: - """Call back for incoming data that only writes when state has changed. + def _async_get_state_attrs(self) -> tuple[Any, ...]: + """Retrieve data that goes into the current state of the entity. - Only the native value and available are ever updated for these - entities, and since the websocket update for the device will trigger - an update for all entities connected to the device, we want to avoid - writing state unless something has actually changed. + Called before and after updating entity and state is only written if there + is a change. """ - previous_value = self._attr_native_value - previous_available = self._attr_available - self._async_update_device_from_protect(device) - if ( - self._attr_native_value != previous_value - or self._attr_available != previous_available - ): - self.async_write_ha_state() + + return (self._attr_available, self._attr_native_value) class ProtectEventSensor(EventEntityMixin, SensorEntity): @@ -803,3 +778,17 @@ class ProtectEventSensor(EventEntityMixin, SensorEntity): self._attr_native_value = event.metadata.license_plate.name # type: ignore[union-attr] else: self._attr_native_value = event.smart_detect_types[0].value + + @callback + def _async_get_state_attrs(self) -> tuple[Any, ...]: + """Retrieve data that goes into the current state of the entity. + + Called before and after updating entity and state is only written if there + is a change. + """ + + return ( + self._attr_available, + self._attr_native_value, + self._attr_extra_state_attributes, + ) diff --git a/homeassistant/components/unifiprotect/switch.py b/homeassistant/components/unifiprotect/switch.py index d8a3fc1c5bc..324fe56cea2 100644 --- a/homeassistant/components/unifiprotect/switch.py +++ b/homeassistant/components/unifiprotect/switch.py @@ -445,31 +445,14 @@ class ProtectSwitch(ProtectDeviceEntity, SwitchEntity): await self.entity_description.ufp_set(self.device, False) @callback - def _async_updated_event(self, device: ProtectModelWithId) -> None: - """Call back for incoming data that only writes when state has changed. + def _async_get_state_attrs(self) -> tuple[Any, ...]: + """Retrieve data that goes into the current state of the entity. - Only the is_on and available are ever updated for these - entities, and since the websocket update for the device will trigger - an update for all entities connected to the device, we want to avoid - writing state unless something has actually changed. + Called before and after updating entity and state is only written if there + is a change. """ - previous_is_on = self._attr_is_on - previous_available = self._attr_available - self._async_update_device_from_protect(device) - if ( - self._attr_is_on != previous_is_on - or self._attr_available != previous_available - ): - _LOGGER.debug( - "Updating state [%s (%s)] %s (%s) -> %s (%s)", - device.name, - device.mac, - previous_is_on, - previous_available, - self._attr_is_on, - self._attr_available, - ) - self.async_write_ha_state() + + return (self._attr_available, self._attr_is_on) class ProtectNVRSwitch(ProtectNVREntity, SwitchEntity): diff --git a/homeassistant/components/unifiprotect/text.py b/homeassistant/components/unifiprotect/text.py index de777121ff5..7fb66d7c8e3 100644 --- a/homeassistant/components/unifiprotect/text.py +++ b/homeassistant/components/unifiprotect/text.py @@ -2,6 +2,7 @@ from __future__ import annotations from dataclasses import dataclass +from typing import Any from pyunifiprotect.data import ( Camera, @@ -101,6 +102,16 @@ class ProtectDeviceText(ProtectDeviceEntity, TextEntity): super()._async_update_device_from_protect(device) self._attr_native_value = self.entity_description.get_ufp_value(self.device) + @callback + def _async_get_state_attrs(self) -> tuple[Any, ...]: + """Retrieve data that goes into the current state of the entity. + + Called before and after updating entity and state is only written if there + is a change. + """ + + return (self._attr_available, self._attr_native_value) + async def async_set_value(self, value: str) -> None: """Change the value.""" diff --git a/tests/components/unifiprotect/test_binary_sensor.py b/tests/components/unifiprotect/test_binary_sensor.py index f07c86d64c2..2c6a7c90065 100644 --- a/tests/components/unifiprotect/test_binary_sensor.py +++ b/tests/components/unifiprotect/test_binary_sensor.py @@ -387,7 +387,7 @@ async def test_binary_sensor_update_mount_type_garage( ) -> None: """Test binary_sensor motion entity.""" - await init_entry(hass, ufp, [sensor_all]) + await init_entry(hass, ufp, [sensor_all], debug=True) assert_entity_counts(hass, Platform.BINARY_SENSOR, 11, 11) _, entity_id = ids_from_device_description( diff --git a/tests/components/unifiprotect/utils.py b/tests/components/unifiprotect/utils.py index 2a0a0eb0655..1ade39dafca 100644 --- a/tests/components/unifiprotect/utils.py +++ b/tests/components/unifiprotect/utils.py @@ -25,6 +25,7 @@ from homeassistant.const import Platform from homeassistant.core import HomeAssistant, split_entity_id from homeassistant.helpers import entity_registry as er from homeassistant.helpers.entity import EntityDescription +from homeassistant.setup import async_setup_component import homeassistant.util.dt as dt_util from tests.common import MockConfigEntry, async_fire_time_changed @@ -161,6 +162,7 @@ async def init_entry( ufp: MockUFPFixture, devices: Sequence[ProtectAdoptableDeviceModel], regenerate_ids: bool = True, + debug: bool = False, ) -> None: """Initialize Protect entry with given devices.""" @@ -168,6 +170,14 @@ async def init_entry( for device in devices: add_device(ufp.api.bootstrap, device, regenerate_ids) + if debug: + assert await async_setup_component(hass, "logger", {"logger": {}}) + await hass.services.async_call( + "logger", + "set_level", + {"homeassistant.components.unifiprotect": "DEBUG"}, + blocking=True, + ) await hass.config_entries.async_setup(ufp.entry.entry_id) await hass.async_block_till_done()