From 7e781946fa30f41b08e4d1ca5440b517fbe7323c Mon Sep 17 00:00:00 2001 From: Alexei Chetroi Date: Sat, 7 Mar 2020 13:52:45 -0500 Subject: [PATCH] Refactor ZHA device keep alive checker (#32561) * Refactor zha core device _check_available(). Make it async, so we don't run it in a sync worker. * Use random keep alive interval for zha device pings. * Update tests. --- homeassistant/components/zha/core/device.py | 67 +++++++++++---------- tests/components/zha/test_device.py | 38 ++++++++++-- 2 files changed, 68 insertions(+), 37 deletions(-) diff --git a/homeassistant/components/zha/core/device.py b/homeassistant/components/zha/core/device.py index 76685180ea2..7297440624a 100644 --- a/homeassistant/components/zha/core/device.py +++ b/homeassistant/components/zha/core/device.py @@ -3,6 +3,7 @@ import asyncio from datetime import timedelta from enum import Enum import logging +import random import time from zigpy import types @@ -61,7 +62,7 @@ from .helpers import LogMixin _LOGGER = logging.getLogger(__name__) _KEEP_ALIVE_INTERVAL = 7200 -_UPDATE_ALIVE_INTERVAL = timedelta(seconds=60) +_UPDATE_ALIVE_INTERVAL = (60, 90) _CHECKIN_GRACE_PERIODS = 2 @@ -98,8 +99,9 @@ class ZHADevice(LogMixin): self._zigpy_device.__class__.__module__, self._zigpy_device.__class__.__name__, ) + keep_alive_interval = random.randint(*_UPDATE_ALIVE_INTERVAL) self._available_check = async_track_time_interval( - self.hass, self._check_available, _UPDATE_ALIVE_INTERVAL + self.hass, self._check_available, timedelta(seconds=keep_alive_interval) ) self._ha_device_id = None self.status = DeviceStatus.CREATED @@ -271,37 +273,40 @@ class ZHADevice(LogMixin): zha_dev.channels = channels.Channels.new(zha_dev) return zha_dev - def _check_available(self, *_): + async def _check_available(self, *_): if self.last_seen is None: self.update_available(False) - else: - difference = time.time() - self.last_seen - if difference > _KEEP_ALIVE_INTERVAL: - if self._checkins_missed_count < _CHECKIN_GRACE_PERIODS: - self._checkins_missed_count += 1 - if self.manufacturer != "LUMI": - self.debug( - "Attempting to checkin with device - missed checkins: %s", - self._checkins_missed_count, - ) - if not self._channels.pools: - return - try: - pool = self._channels.pools[0] - basic_ch = pool.all_channels[f"{pool.id}:0x0000"] - except KeyError: - self.debug("%s %s does not have a mandatory basic cluster") - return - self.hass.async_create_task( - basic_ch.get_attribute_value( - ATTR_MANUFACTURER, from_cache=False - ) - ) - else: - self.update_available(False) - else: - self.update_available(True) - self._checkins_missed_count = 0 + return + + difference = time.time() - self.last_seen + if difference < _KEEP_ALIVE_INTERVAL: + self.update_available(True) + self._checkins_missed_count = 0 + return + + if ( + self._checkins_missed_count >= _CHECKIN_GRACE_PERIODS + or self.manufacturer == "LUMI" + or not self._channels.pools + ): + self.update_available(False) + return + + self._checkins_missed_count += 1 + self.debug( + "Attempting to checkin with device - missed checkins: %s", + self._checkins_missed_count, + ) + try: + pool = self._channels.pools[0] + basic_ch = pool.all_channels[f"{pool.id}:0x0000"] + except KeyError: + self.debug("does not have a mandatory basic cluster") + self.update_available(False) + return + res = await basic_ch.get_attribute_value(ATTR_MANUFACTURER, from_cache=False) + if res is not None: + self._checkins_missed_count = 0 def update_available(self, available): """Set sensor availability.""" diff --git a/tests/components/zha/test_device.py b/tests/components/zha/test_device.py index 3ac22b136fb..0df975b732f 100644 --- a/tests/components/zha/test_device.py +++ b/tests/components/zha/test_device.py @@ -82,21 +82,21 @@ async def test_check_available_success( basic_ch.read_attributes.side_effect = _update_last_seen # successfully ping zigpy device, but zha_device is not yet available - _send_time_changed(hass, 61) + _send_time_changed(hass, 91) await hass.async_block_till_done() assert basic_ch.read_attributes.await_count == 1 assert basic_ch.read_attributes.await_args[0][0] == ["manufacturer"] assert zha_device.available is False # There was traffic from the device: pings, but not yet available - _send_time_changed(hass, 61) + _send_time_changed(hass, 91) await hass.async_block_till_done() assert basic_ch.read_attributes.await_count == 2 assert basic_ch.read_attributes.await_args[0][0] == ["manufacturer"] assert zha_device.available is False # There was traffic from the device: don't try to ping, marked as available - _send_time_changed(hass, 61) + _send_time_changed(hass, 91) await hass.async_block_till_done() assert basic_ch.read_attributes.await_count == 2 assert basic_ch.read_attributes.await_args[0][0] == ["manufacturer"] @@ -125,22 +125,48 @@ async def test_check_available_unsuccessful( ) # unsuccessfuly ping zigpy device, but zha_device is still available - _send_time_changed(hass, 61) + _send_time_changed(hass, 91) await hass.async_block_till_done() assert basic_ch.read_attributes.await_count == 1 assert basic_ch.read_attributes.await_args[0][0] == ["manufacturer"] assert zha_device.available is True # still no traffic, but zha_device is still available - _send_time_changed(hass, 61) + _send_time_changed(hass, 91) await hass.async_block_till_done() assert basic_ch.read_attributes.await_count == 2 assert basic_ch.read_attributes.await_args[0][0] == ["manufacturer"] assert zha_device.available is True # not even trying to update, device is unavailble - _send_time_changed(hass, 61) + _send_time_changed(hass, 91) await hass.async_block_till_done() assert basic_ch.read_attributes.await_count == 2 assert basic_ch.read_attributes.await_args[0][0] == ["manufacturer"] assert zha_device.available is False + + +@asynctest.patch( + "homeassistant.components.zha.core.channels.general.BasicChannel.async_initialize", + new=mock.MagicMock(), +) +async def test_check_available_no_basic_channel( + hass, device_without_basic_channel, zha_device_restored, caplog +): + """Check device availability for a device without basic cluster.""" + + # pylint: disable=protected-access + zha_device = await zha_device_restored(device_without_basic_channel) + await async_enable_traffic(hass, [zha_device]) + + assert zha_device.available is True + + device_without_basic_channel.last_seen = ( + time.time() - zha_core_device._KEEP_ALIVE_INTERVAL - 2 + ) + + assert "does not have a mandatory basic cluster" not in caplog.text + _send_time_changed(hass, 91) + await hass.async_block_till_done() + assert zha_device.available is False + assert "does not have a mandatory basic cluster" in caplog.text