Refactor ZHA device keep alive checker (home-assistant#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

@@ -3,6 +3,7 @@
 from datetime import timedelta
 from enum import Enum
 import logging
+import random
 import time
 
 from zigpy import types
@@ -61,7 +62,7 @@
 
 _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 @@ def __init__(
             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 @@ def new(
         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."""

tests/components/zha/test_device.py

@@ -82,21 +82,21 @@ def _update_last_seen(*args, **kwargs):
     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