From cbe85126cb8e01d99037eb69e852f6bb965551f4 Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Mon, 20 Mar 2023 18:30:56 -0400 Subject: [PATCH] Introduce a delay between update entity calls (#89737) * Introduce a delay between update entity calls * Update homeassistant/components/zwave_js/update.py Co-authored-by: Martin Hjelmare * move delay to constant and patch * rename constant * Switch to async_call_later * Remove failing test * Reimplement to solve task problem * comment * pass count directly so that value doesn't mutate before we store it * lines * Fix logic and tests * Comments * Readd missed coverage * Add test for delays * cleanup * Fix async_added_to_hass logic * flip conditional * Store firmware info in extra data so we can restore it along with latest version * Comment * comment * Add test for is_running check and fix bugs * comment * Add tests for various restore state scenarios * move comment so it's less confusing * improve typing * consolidate into constant and remove unused one * Update update.py * update test to unknown state during partial restore * fix elif check * Fix type * clean up test docstrings and function names --------- Co-authored-by: Martin Hjelmare --- homeassistant/components/zwave_js/update.py | 101 +++++-- tests/components/zwave_js/conftest.py | 6 + tests/components/zwave_js/test_update.py | 296 +++++++++++++++++--- 3 files changed, 350 insertions(+), 53 deletions(-) diff --git a/homeassistant/components/zwave_js/update.py b/homeassistant/components/zwave_js/update.py index 5485870dc5f..33cb0a1c5a8 100644 --- a/homeassistant/components/zwave_js/update.py +++ b/homeassistant/components/zwave_js/update.py @@ -2,9 +2,11 @@ from __future__ import annotations import asyncio +from collections import Counter from collections.abc import Callable +from dataclasses import asdict, dataclass from datetime import datetime, timedelta -from typing import Any +from typing import Any, Final from awesomeversion import AwesomeVersion from zwave_js_server.client import Client as ZwaveClient @@ -19,41 +21,72 @@ from zwave_js_server.model.node.firmware import ( ) from homeassistant.components.update import ( + ATTR_LATEST_VERSION, UpdateDeviceClass, UpdateEntity, UpdateEntityFeature, ) from homeassistant.config_entries import ConfigEntry from homeassistant.const import EntityCategory -from homeassistant.core import HomeAssistant, callback +from homeassistant.core import CoreState, HomeAssistant, callback from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.dispatcher import async_dispatcher_connect from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.event import async_call_later -from homeassistant.helpers.start import async_at_start +from homeassistant.helpers.restore_state import ExtraStoredData from .const import API_KEY_FIRMWARE_UPDATE_SERVICE, DATA_CLIENT, DOMAIN, LOGGER from .helpers import get_device_info, get_valueless_base_unique_id PARALLEL_UPDATES = 1 +UPDATE_DELAY_STRING = "delay" +UPDATE_DELAY_INTERVAL = 5 # In minutes + + +@dataclass +class ZWaveNodeFirmwareUpdateExtraStoredData(ExtraStoredData): + """Extra stored data for Z-Wave node firmware update entity.""" + + latest_version_firmware: NodeFirmwareUpdateInfo | None + + def as_dict(self) -> dict[str, Any]: + """Return a dict representation of the extra data.""" + return { + "latest_version_firmware": asdict(self.latest_version_firmware) + if self.latest_version_firmware + else None + } + + @classmethod + def from_dict(cls, data: dict[str, Any]) -> ZWaveNodeFirmwareUpdateExtraStoredData: + """Initialize the extra data from a dict.""" + if not (firmware_dict := data["latest_version_firmware"]): + return cls(None) + + return cls(NodeFirmwareUpdateInfo.from_dict(firmware_dict)) + async def async_setup_entry( hass: HomeAssistant, config_entry: ConfigEntry, async_add_entities: AddEntitiesCallback, ) -> None: - """Set up Z-Wave button from config entry.""" + """Set up Z-Wave update entity from config entry.""" client: ZwaveClient = hass.data[DOMAIN][config_entry.entry_id][DATA_CLIENT] - - semaphore = asyncio.Semaphore(3) + cnt: Counter = Counter() @callback def async_add_firmware_update_entity(node: ZwaveNode) -> None: """Add firmware update entity.""" + # We need to delay the first update of each entity to avoid flooding the network + # so we maintain a counter to schedule first update in UPDATE_DELAY_INTERVAL + # minute increments. + cnt[UPDATE_DELAY_STRING] += 1 + delay = timedelta(minutes=(cnt[UPDATE_DELAY_STRING] * UPDATE_DELAY_INTERVAL)) driver = client.driver assert driver is not None # Driver is ready before platforms are loaded. - async_add_entities([ZWaveNodeFirmwareUpdate(driver, node, semaphore)]) + async_add_entities([ZWaveNodeFirmwareUpdate(driver, node, delay)]) config_entry.async_on_unload( async_dispatcher_connect( @@ -77,13 +110,10 @@ class ZWaveNodeFirmwareUpdate(UpdateEntity): _attr_has_entity_name = True _attr_should_poll = False - def __init__( - self, driver: Driver, node: ZwaveNode, semaphore: asyncio.Semaphore - ) -> None: + def __init__(self, driver: Driver, node: ZwaveNode, delay: timedelta) -> None: """Initialize a Z-Wave device firmware update entity.""" self.driver = driver self.node = node - self.semaphore = semaphore self._latest_version_firmware: NodeFirmwareUpdateInfo | None = None self._status_unsub: Callable[[], None] | None = None self._poll_unsub: Callable[[], None] | None = None @@ -91,6 +121,7 @@ class ZWaveNodeFirmwareUpdate(UpdateEntity): self._finished_unsub: Callable[[], None] | None = None self._finished_event = asyncio.Event() self._result: NodeFirmwareUpdateResult | None = None + self._delay: Final[timedelta] = delay # Entity class attributes self._attr_name = "Firmware" @@ -100,6 +131,11 @@ class ZWaveNodeFirmwareUpdate(UpdateEntity): # device may not be precreated in main handler yet self._attr_device_info = get_device_info(driver, node) + @property + def extra_restore_state_data(self) -> ExtraStoredData: + """Return ZWave Node Firmware Update specific state data to be restored.""" + return ZWaveNodeFirmwareUpdateExtraStoredData(self._latest_version_firmware) + @callback def _update_on_status_change(self, _: dict[str, Any]) -> None: """Update the entity when node is awake.""" @@ -143,7 +179,17 @@ class ZWaveNodeFirmwareUpdate(UpdateEntity): async def _async_update(self, _: HomeAssistant | datetime | None = None) -> None: """Update the entity.""" - self._poll_unsub = None + if self._poll_unsub: + self._poll_unsub() + self._poll_unsub = None + + # If hass hasn't started yet, push the next update to the next day so that we + # can preserve the offsets we've created between each node + if self.hass.state != CoreState.running: + self._poll_unsub = async_call_later( + self.hass, timedelta(days=1), self._async_update + ) + return # If device is asleep/dead, wait for it to wake up/become alive before # attempting an update @@ -159,12 +205,11 @@ class ZWaveNodeFirmwareUpdate(UpdateEntity): return try: - async with self.semaphore: - available_firmware_updates = ( - await self.driver.controller.async_get_available_firmware_updates( - self.node, API_KEY_FIRMWARE_UPDATE_SERVICE - ) + available_firmware_updates = ( + await self.driver.controller.async_get_available_firmware_updates( + self.node, API_KEY_FIRMWARE_UPDATE_SERVICE ) + ) except FailedZWaveCommand as err: LOGGER.debug( "Failed to get firmware updates for node %s: %s", @@ -277,7 +322,27 @@ class ZWaveNodeFirmwareUpdate(UpdateEntity): ) ) - self.async_on_remove(async_at_start(self.hass, self._async_update)) + # If we have a complete previous state, use that to set the latest version + if (state := await self.async_get_last_state()) and ( + extra_data := await self.async_get_last_extra_data() + ): + self._attr_latest_version = state.attributes[ATTR_LATEST_VERSION] + self._latest_version_firmware = ( + ZWaveNodeFirmwareUpdateExtraStoredData.from_dict( + extra_data.as_dict() + ).latest_version_firmware + ) + # If we have no state to restore, we can set the latest version to installed + # so that the entity starts as off. If we have partial restore data due to an + # upgrade to an HA version where this feature is released from one that is not + # the entity will start in an unknown state until we can correct on next update + elif not state: + self._attr_latest_version = self._attr_installed_version + + # Spread updates out in 5 minute increments to avoid flooding the network + self.async_on_remove( + async_call_later(self.hass, self._delay, self._async_update) + ) async def async_will_remove_from_hass(self) -> None: """Call when entity will be removed.""" diff --git a/tests/components/zwave_js/conftest.py b/tests/components/zwave_js/conftest.py index f20c814bdc3..32082a0bb85 100644 --- a/tests/components/zwave_js/conftest.py +++ b/tests/components/zwave_js/conftest.py @@ -235,6 +235,9 @@ def create_backup_fixture(): yield create_backup +# State fixtures + + @pytest.fixture(name="controller_state", scope="session") def controller_state_fixture(): """Load the controller state fixture data.""" @@ -601,6 +604,9 @@ def lock_home_connect_620_state_fixture(): return json.loads(load_fixture("zwave_js/lock_home_connect_620_state.json")) +# model fixtures + + @pytest.fixture(name="client") def mock_client_fixture( controller_state, controller_node_state, version_state, log_config_state diff --git a/tests/components/zwave_js/test_update.py b/tests/components/zwave_js/test_update.py index 30371904a47..36f16d0b502 100644 --- a/tests/components/zwave_js/test_update.py +++ b/tests/components/zwave_js/test_update.py @@ -20,16 +20,26 @@ from homeassistant.components.update import ( ) from homeassistant.components.zwave_js.const import DOMAIN, SERVICE_REFRESH_VALUE from homeassistant.components.zwave_js.helpers import get_valueless_base_unique_id -from homeassistant.const import ATTR_ENTITY_ID, STATE_OFF, STATE_ON -from homeassistant.core import HomeAssistant +from homeassistant.const import ATTR_ENTITY_ID, STATE_OFF, STATE_ON, STATE_UNKNOWN +from homeassistant.core import CoreState, HomeAssistant, State from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.entity_registry import async_get from homeassistant.util import dt as dt_util -from tests.common import MockConfigEntry, async_fire_time_changed +from tests.common import ( + MockConfigEntry, + async_fire_time_changed, + mock_restore_cache, + mock_restore_cache_with_extra_data, +) from tests.typing import WebSocketGenerator UPDATE_ENTITY = "update.z_wave_thermostat_firmware" +LATEST_VERSION_FIRMWARE = { + "version": "11.2.4", + "changelog": "blah 2", + "files": [{"target": 0, "url": "https://example2.com", "integrity": "sha2"}], +} FIRMWARE_UPDATES = { "updates": [ { @@ -39,13 +49,7 @@ FIRMWARE_UPDATES = { {"target": 0, "url": "https://example1.com", "integrity": "sha1"} ], }, - { - "version": "11.2.4", - "changelog": "blah 2", - "files": [ - {"target": 0, "url": "https://example2.com", "integrity": "sha2"} - ], - }, + LATEST_VERSION_FIRMWARE, { "version": "11.1.5", "changelog": "blah 3", @@ -56,19 +60,6 @@ FIRMWARE_UPDATES = { ] } -FIRMWARE_UPDATE_MULTIPLE_FILES = { - "updates": [ - { - "version": "11.2.4", - "changelog": "blah 2", - "files": [ - {"target": 0, "url": "https://example2.com", "integrity": "sha2"}, - {"target": 1, "url": "https://example4.com", "integrity": "sha4"}, - ], - }, - ] -} - async def test_update_entity_states( hass: HomeAssistant, @@ -85,7 +76,7 @@ async def test_update_entity_states( client.async_send_command.return_value = {"updates": []} - async_fire_time_changed(hass, dt_util.utcnow() + timedelta(days=1)) + async_fire_time_changed(hass, dt_util.utcnow() + timedelta(minutes=5, days=1)) await hass.async_block_till_done() state = hass.states.get(UPDATE_ENTITY) @@ -104,7 +95,7 @@ async def test_update_entity_states( client.async_send_command.return_value = FIRMWARE_UPDATES - async_fire_time_changed(hass, dt_util.utcnow() + timedelta(days=2)) + async_fire_time_changed(hass, dt_util.utcnow() + timedelta(minutes=5, days=2)) await hass.async_block_till_done() state = hass.states.get(UPDATE_ENTITY) @@ -139,6 +130,15 @@ async def test_update_entity_states( assert "There is no value to refresh for this entity" in caplog.text + client.async_send_command.return_value = {"updates": []} + + async_fire_time_changed(hass, dt_util.utcnow() + timedelta(minutes=5, days=3)) + await hass.async_block_till_done() + + state = hass.states.get(UPDATE_ENTITY) + assert state + assert state.state == STATE_OFF + # Assert a node firmware update entity is not created for the controller driver = client.driver node = driver.controller.nodes[1] @@ -164,7 +164,7 @@ async def test_update_entity_install_raises( """Test update entity install raises exception.""" client.async_send_command.return_value = FIRMWARE_UPDATES - async_fire_time_changed(hass, dt_util.utcnow() + timedelta(days=1)) + async_fire_time_changed(hass, dt_util.utcnow() + timedelta(minutes=5, days=1)) await hass.async_block_till_done() # Test failed installation by driver @@ -197,7 +197,7 @@ async def test_update_entity_sleep( client.async_send_command.return_value = FIRMWARE_UPDATES - async_fire_time_changed(hass, dt_util.utcnow() + timedelta(days=1)) + async_fire_time_changed(hass, dt_util.utcnow() + timedelta(minutes=5, days=1)) await hass.async_block_till_done() # Because node is asleep we shouldn't attempt to check for firmware updates @@ -234,7 +234,7 @@ async def test_update_entity_dead( client.async_send_command.return_value = FIRMWARE_UPDATES - async_fire_time_changed(hass, dt_util.utcnow() + timedelta(days=1)) + async_fire_time_changed(hass, dt_util.utcnow() + timedelta(minutes=5, days=1)) await hass.async_block_till_done() # Because node is asleep we shouldn't attempt to check for firmware updates @@ -261,7 +261,7 @@ async def test_update_entity_ha_not_running( zen_31, hass_ws_client: WebSocketGenerator, ) -> None: - """Test update occurs after HA starts.""" + """Test update occurs only after HA is running.""" await hass.async_stop() entry = MockConfigEntry(domain="zwave_js", data={"url": "ws://test.org"}) @@ -272,6 +272,22 @@ async def test_update_entity_ha_not_running( assert len(client.async_send_command.call_args_list) == 0 await hass.async_start() + await hass.async_block_till_done() + + assert len(client.async_send_command.call_args_list) == 0 + + # Update should be delayed by a day because HA is not running + hass.state = CoreState.starting + + async_fire_time_changed(hass, dt_util.utcnow() + timedelta(minutes=5)) + await hass.async_block_till_done() + + assert len(client.async_send_command.call_args_list) == 0 + + hass.state = CoreState.running + + async_fire_time_changed(hass, dt_util.utcnow() + timedelta(minutes=5, days=1)) + await hass.async_block_till_done() assert len(client.async_send_command.call_args_list) == 1 args = client.async_send_command.call_args_list[0][0][0] @@ -289,7 +305,7 @@ async def test_update_entity_update_failure( assert len(client.async_send_command.call_args_list) == 0 client.async_send_command.side_effect = FailedZWaveCommand("test", 260, "test") - async_fire_time_changed(hass, dt_util.utcnow() + timedelta(days=1)) + async_fire_time_changed(hass, dt_util.utcnow() + timedelta(minutes=5, days=1)) await hass.async_block_till_done() state = hass.states.get(UPDATE_ENTITY) @@ -314,7 +330,7 @@ async def test_update_entity_progress( node = climate_radio_thermostat_ct100_plus_different_endpoints client.async_send_command.return_value = FIRMWARE_UPDATES - async_fire_time_changed(hass, dt_util.utcnow() + timedelta(days=1)) + async_fire_time_changed(hass, dt_util.utcnow() + timedelta(minutes=5, days=1)) await hass.async_block_till_done() state = hass.states.get(UPDATE_ENTITY) @@ -410,7 +426,7 @@ async def test_update_entity_install_failed( node = climate_radio_thermostat_ct100_plus_different_endpoints client.async_send_command.return_value = FIRMWARE_UPDATES - async_fire_time_changed(hass, dt_util.utcnow() + timedelta(days=1)) + async_fire_time_changed(hass, dt_util.utcnow() + timedelta(minutes=5, days=1)) await hass.async_block_till_done() state = hass.states.get(UPDATE_ENTITY) @@ -503,7 +519,7 @@ async def test_update_entity_reload( client.async_send_command.return_value = {"updates": []} - async_fire_time_changed(hass, dt_util.utcnow() + timedelta(days=1)) + async_fire_time_changed(hass, dt_util.utcnow() + timedelta(minutes=5, days=1)) await hass.async_block_till_done() state = hass.states.get(UPDATE_ENTITY) @@ -512,7 +528,7 @@ async def test_update_entity_reload( client.async_send_command.return_value = FIRMWARE_UPDATES - async_fire_time_changed(hass, dt_util.utcnow() + timedelta(days=2)) + async_fire_time_changed(hass, dt_util.utcnow() + timedelta(minutes=5, days=2)) await hass.async_block_till_done() state = hass.states.get(UPDATE_ENTITY) @@ -543,10 +559,220 @@ async def test_update_entity_reload( await hass.async_block_till_done() # Trigger another update and make sure the skipped version is still skipped - async_fire_time_changed(hass, dt_util.utcnow() + timedelta(days=4)) + async_fire_time_changed(hass, dt_util.utcnow() + timedelta(minutes=5, days=4)) await hass.async_block_till_done() state = hass.states.get(UPDATE_ENTITY) assert state assert state.state == STATE_OFF assert state.attributes[ATTR_SKIPPED_VERSION] == "11.2.4" + + +async def test_update_entity_delay( + hass: HomeAssistant, + client, + ge_in_wall_dimmer_switch, + zen_31, + hass_ws_client: WebSocketGenerator, +) -> None: + """Test update occurs on a delay after HA starts.""" + client.async_send_command.reset_mock() + await hass.async_stop() + + entry = MockConfigEntry(domain="zwave_js", data={"url": "ws://test.org"}) + entry.add_to_hass(hass) + await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + assert len(client.async_send_command.call_args_list) == 0 + + await hass.async_start() + await hass.async_block_till_done() + + assert len(client.async_send_command.call_args_list) == 0 + + async_fire_time_changed(hass, dt_util.utcnow() + timedelta(minutes=5)) + await hass.async_block_till_done() + + assert len(client.async_send_command.call_args_list) == 1 + args = client.async_send_command.call_args_list[0][0][0] + assert args["command"] == "controller.get_available_firmware_updates" + assert args["nodeId"] == ge_in_wall_dimmer_switch.node_id + + async_fire_time_changed(hass, dt_util.utcnow() + timedelta(minutes=10)) + await hass.async_block_till_done() + + assert len(client.async_send_command.call_args_list) == 2 + args = client.async_send_command.call_args_list[1][0][0] + assert args["command"] == "controller.get_available_firmware_updates" + assert args["nodeId"] == zen_31.node_id + + +async def test_update_entity_partial_restore_data( + hass: HomeAssistant, + client, + climate_radio_thermostat_ct100_plus_different_endpoints, + hass_ws_client: WebSocketGenerator, +) -> None: + """Test update entity with partial restore data resets state.""" + mock_restore_cache( + hass, + [ + State( + UPDATE_ENTITY, + STATE_OFF, + { + ATTR_INSTALLED_VERSION: "10.7", + ATTR_LATEST_VERSION: "11.2.4", + ATTR_SKIPPED_VERSION: "11.2.4", + }, + ) + ], + ) + entry = MockConfigEntry(domain="zwave_js", data={"url": "ws://test.org"}) + entry.add_to_hass(hass) + await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + state = hass.states.get(UPDATE_ENTITY) + assert state + assert state.state == STATE_UNKNOWN + + +async def test_update_entity_full_restore_data_skipped_version( + hass: HomeAssistant, + client, + climate_radio_thermostat_ct100_plus_different_endpoints, + hass_ws_client: WebSocketGenerator, +) -> None: + """Test update entity with full restore data (skipped version) restores state.""" + mock_restore_cache_with_extra_data( + hass, + [ + ( + State( + UPDATE_ENTITY, + STATE_OFF, + { + ATTR_INSTALLED_VERSION: "10.7", + ATTR_LATEST_VERSION: "11.2.4", + ATTR_SKIPPED_VERSION: "11.2.4", + }, + ), + {"latest_version_firmware": LATEST_VERSION_FIRMWARE}, + ) + ], + ) + entry = MockConfigEntry(domain="zwave_js", data={"url": "ws://test.org"}) + entry.add_to_hass(hass) + await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + state = hass.states.get(UPDATE_ENTITY) + assert state + assert state.state == STATE_OFF + assert state.attributes[ATTR_SKIPPED_VERSION] == "11.2.4" + assert state.attributes[ATTR_LATEST_VERSION] == "11.2.4" + + +async def test_update_entity_full_restore_data_update_available( + hass: HomeAssistant, + client, + climate_radio_thermostat_ct100_plus_different_endpoints, + hass_ws_client: WebSocketGenerator, +) -> None: + """Test update entity with full restore data (update available) restores state.""" + mock_restore_cache_with_extra_data( + hass, + [ + ( + State( + UPDATE_ENTITY, + STATE_OFF, + { + ATTR_INSTALLED_VERSION: "10.7", + ATTR_LATEST_VERSION: "11.2.4", + ATTR_SKIPPED_VERSION: None, + }, + ), + {"latest_version_firmware": LATEST_VERSION_FIRMWARE}, + ) + ], + ) + entry = MockConfigEntry(domain="zwave_js", data={"url": "ws://test.org"}) + entry.add_to_hass(hass) + await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + state = hass.states.get(UPDATE_ENTITY) + assert state + assert state.state == STATE_ON + assert state.attributes[ATTR_SKIPPED_VERSION] is None + assert state.attributes[ATTR_LATEST_VERSION] == "11.2.4" + + client.async_send_command.return_value = {"success": True} + + # Test successful install call without a version + install_task = hass.async_create_task( + hass.services.async_call( + UPDATE_DOMAIN, + SERVICE_INSTALL, + { + ATTR_ENTITY_ID: UPDATE_ENTITY, + }, + blocking=True, + ) + ) + + # Sleep so that task starts + await asyncio.sleep(0.1) + + state = hass.states.get(UPDATE_ENTITY) + assert state + attrs = state.attributes + assert attrs[ATTR_IN_PROGRESS] is True + + assert len(client.async_send_command.call_args_list) == 1 + assert client.async_send_command.call_args_list[0][0][0] == { + "command": "controller.firmware_update_ota", + "nodeId": climate_radio_thermostat_ct100_plus_different_endpoints.node_id, + "updates": [{"target": 0, "url": "https://example2.com", "integrity": "sha2"}], + } + + install_task.cancel() + + +async def test_update_entity_full_restore_data_no_update_available( + hass: HomeAssistant, + client, + climate_radio_thermostat_ct100_plus_different_endpoints, + hass_ws_client: WebSocketGenerator, +) -> None: + """Test entity with full restore data (no update available) restores state.""" + mock_restore_cache_with_extra_data( + hass, + [ + ( + State( + UPDATE_ENTITY, + STATE_OFF, + { + ATTR_INSTALLED_VERSION: "10.7", + ATTR_LATEST_VERSION: "10.7", + ATTR_SKIPPED_VERSION: None, + }, + ), + {"latest_version_firmware": None}, + ) + ], + ) + entry = MockConfigEntry(domain="zwave_js", data={"url": "ws://test.org"}) + entry.add_to_hass(hass) + await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + state = hass.states.get(UPDATE_ENTITY) + assert state + assert state.state == STATE_OFF + assert state.attributes[ATTR_SKIPPED_VERSION] is None + assert state.attributes[ATTR_LATEST_VERSION] == "10.7"