Add timeout / debounce (for brightness and others) (#13534)

* Add async timeout feature

* Decorator for setter methods to limit service calls to HA
* Changed to async
* Use async_call_later
* Use lastargs, async_add_job

* Use dict for lastargs

* Updated tests to stop patch

homeassistant/components/homekit/accessories.py

@@ -1,21 +1,64 @@
 """Extend the basic Accessory and Bridge functions."""
+from datetime import timedelta
+from functools import wraps
+from inspect import getmodule
 import logging
 
 from pyhap.accessory import Accessory, Bridge, Category
 from pyhap.accessory_driver import AccessoryDriver
 
-from homeassistant.helpers.event import async_track_state_change
+from homeassistant.core import callback
+from homeassistant.helpers.event import (
+    async_track_state_change, track_point_in_utc_time)
+from homeassistant.util import dt as dt_util
 
 from .const import (
-    ACCESSORY_MODEL, ACCESSORY_NAME, BRIDGE_MODEL, BRIDGE_NAME,
+    DEBOUNCE_TIMEOUT, ACCESSORY_MODEL, ACCESSORY_NAME, BRIDGE_MODEL,
-    MANUFACTURER, SERV_ACCESSORY_INFO, CHAR_MANUFACTURER, CHAR_MODEL,
+    BRIDGE_NAME, MANUFACTURER, SERV_ACCESSORY_INFO, CHAR_MANUFACTURER,
-    CHAR_NAME, CHAR_SERIAL_NUMBER)
+    CHAR_MODEL, CHAR_NAME, CHAR_SERIAL_NUMBER)
 from .util import (
     show_setup_message, dismiss_setup_message)
 
 _LOGGER = logging.getLogger(__name__)
 
 
+def debounce(func):
+    """Decorator function. Debounce callbacks form HomeKit."""
+    @callback
+    def call_later_listener(*args):
+        """Callback listener called from call_later."""
+        # pylint: disable=unsubscriptable-object
+        nonlocal lastargs, remove_listener
+        hass = lastargs['hass']
+        hass.async_add_job(func, *lastargs['args'])
+        lastargs = remove_listener = None
+
+    @wraps(func)
+    def wrapper(*args):
+        """Wrapper starts async timer.
+
+        The accessory must have 'self.hass' and 'self.entity_id' as attributes.
+        """
+        # pylint: disable=not-callable
+        hass = args[0].hass
+        nonlocal lastargs, remove_listener
+        if remove_listener:
+            remove_listener()
+            lastargs = remove_listener = None
+        lastargs = {'hass': hass, 'args': [*args]}
+        remove_listener = track_point_in_utc_time(
+            hass, call_later_listener,
+            dt_util.utcnow() + timedelta(seconds=DEBOUNCE_TIMEOUT))
+        logger.debug('%s: Start %s timeout', args[0].entity_id,
+                     func.__name__.replace('set_', ''))
+
+    remove_listener = None
+    lastargs = None
+    name = getmodule(func).__name__
+    logger = logging.getLogger(name)
+    return wrapper
+
+
 def add_preload_service(acc, service, chars=None):
     """Define and return a service to be available for the accessory."""
     from pyhap.loader import get_serv_loader, get_char_loader

homeassistant/components/homekit/const.py

@@ -1,5 +1,6 @@
 """Constants used be the HomeKit component."""
 # #### MISC ####
+DEBOUNCE_TIMEOUT = 0.5
 DOMAIN = 'homekit'
 HOMEKIT_FILE = '.homekit.state'
 HOMEKIT_NOTIFY_ID = 4663548

homeassistant/components/homekit/type_lights.py

@@ -7,7 +7,7 @@ from homeassistant.components.light import (
 from homeassistant.const import ATTR_SUPPORTED_FEATURES, STATE_ON, STATE_OFF
 
 from . import TYPES
-from .accessories import HomeAccessory, add_preload_service
+from .accessories import HomeAccessory, add_preload_service, debounce
 from .const import (
     CATEGORY_LIGHT, SERV_LIGHTBULB, CHAR_COLOR_TEMPERATURE,
     CHAR_BRIGHTNESS, CHAR_HUE, CHAR_ON, CHAR_SATURATION)
@@ -93,6 +93,7 @@ class Light(HomeAccessory):
         elif value == 0:
             self.hass.components.light.turn_off(self.entity_id)
 
+    @debounce
     def set_brightness(self, value):
         """Set brightness if call came from HomeKit."""
         _LOGGER.debug('%s: Set brightness to %d', self.entity_id, value)

homeassistant/components/homekit/type_thermostats.py

@@ -10,7 +10,7 @@ from homeassistant.const import (
     ATTR_UNIT_OF_MEASUREMENT, STATE_OFF, TEMP_CELSIUS, TEMP_FAHRENHEIT)
 
 from . import TYPES
-from .accessories import HomeAccessory, add_preload_service
+from .accessories import HomeAccessory, add_preload_service, debounce
 from .const import (
     CATEGORY_THERMOSTAT, SERV_THERMOSTAT, CHAR_CURRENT_HEATING_COOLING,
     CHAR_TARGET_HEATING_COOLING, CHAR_CURRENT_TEMPERATURE,
@@ -104,6 +104,7 @@ class Thermostat(HomeAccessory):
             self.hass.components.climate.set_operation_mode(
                 operation_mode=hass_value, entity_id=self.entity_id)
 
+    @debounce
     def set_cooling_threshold(self, value):
         """Set cooling threshold temp to value if call came from HomeKit."""
         _LOGGER.debug('%s: Set cooling threshold temperature to %.2f°C',
@@ -116,6 +117,7 @@ class Thermostat(HomeAccessory):
             entity_id=self.entity_id, target_temp_high=value,
             target_temp_low=low)
 
+    @debounce
     def set_heating_threshold(self, value):
         """Set heating threshold temp to value if call came from HomeKit."""
         _LOGGER.debug('%s: Set heating threshold temperature to %.2f°C',
@@ -129,6 +131,7 @@ class Thermostat(HomeAccessory):
             entity_id=self.entity_id, target_temp_high=high,
             target_temp_low=value)
 
+    @debounce
     def set_target_temperature(self, value):
         """Set target temperature to value if call came from HomeKit."""
         _LOGGER.debug('%s: Set target temperature to %.2f°C',

tests/components/homekit/test_accessories.py

@@ -2,21 +2,67 @@
 
 This includes tests for all mock object types.
 """
+from datetime import datetime, timedelta
 import unittest
 from unittest.mock import call, patch, Mock
 
 from homeassistant.components.homekit.accessories import (
     add_preload_service, set_accessory_info,
-    HomeAccessory, HomeBridge, HomeDriver)
+    debounce, HomeAccessory, HomeBridge, HomeDriver)
 from homeassistant.components.homekit.const import (
     ACCESSORY_MODEL, ACCESSORY_NAME, BRIDGE_MODEL, BRIDGE_NAME,
     SERV_ACCESSORY_INFO, CHAR_MANUFACTURER, CHAR_MODEL,
     CHAR_NAME, CHAR_SERIAL_NUMBER)
+from homeassistant.const import ATTR_NOW, EVENT_TIME_CHANGED
+import homeassistant.util.dt as dt_util
+
+from tests.common import get_test_home_assistant
+
+
+def patch_debounce():
+    """Return patch for debounce method."""
+    return patch('homeassistant.components.homekit.accessories.debounce',
+                 lambda f: lambda *args, **kwargs: f(*args, **kwargs))
 
 
 class TestAccessories(unittest.TestCase):
     """Test pyhap adapter methods."""
 
+    def test_debounce(self):
+        """Test add_timeout decorator function."""
+        def demo_func(*args):
+            nonlocal arguments, counter
+            counter += 1
+            arguments = args
+
+        arguments = None
+        counter = 0
+        hass = get_test_home_assistant()
+        mock = Mock(hass=hass)
+
+        debounce_demo = debounce(demo_func)
+        self.assertEqual(debounce_demo.__name__, 'demo_func')
+        now = datetime(2018, 1, 1, 20, 0, 0, tzinfo=dt_util.UTC)
+
+        with patch('homeassistant.util.dt.utcnow', return_value=now):
+            debounce_demo(mock, 'value')
+        hass.bus.fire(
+            EVENT_TIME_CHANGED, {ATTR_NOW: now + timedelta(seconds=3)})
+        hass.block_till_done()
+        assert counter == 1
+        assert len(arguments) == 2
+
+        with patch('homeassistant.util.dt.utcnow', return_value=now):
+            debounce_demo(mock, 'value')
+            debounce_demo(mock, 'value')
+
+        hass.bus.fire(
+            EVENT_TIME_CHANGED, {ATTR_NOW: now + timedelta(seconds=3)})
+        hass.block_till_done()
+        assert counter == 2
+
+        hass.stop()
+
     def test_add_preload_service(self):
         """Test add_preload_service without additional characteristics."""
         acc = Mock()

tests/components/homekit/test_homekit.py

@@ -14,6 +14,7 @@ from homeassistant.const import (
     CONF_PORT, EVENT_HOMEASSISTANT_START, EVENT_HOMEASSISTANT_STOP)
 
 from tests.common import get_test_home_assistant
+from tests.components.homekit.test_accessories import patch_debounce
 
 IP_ADDRESS = '127.0.0.1'
 PATH_HOMEKIT = 'homeassistant.components.homekit'
@@ -22,6 +23,17 @@ PATH_HOMEKIT = 'homeassistant.components.homekit'
 class TestHomeKit(unittest.TestCase):
     """Test setup of HomeKit component and HomeKit class."""
 
+    @classmethod
+    def setUpClass(cls):
+        """Setup debounce patcher."""
+        cls.patcher = patch_debounce()
+        cls.patcher.start()
+
+    @classmethod
+    def tearDownClass(cls):
+        """Stop debounce patcher."""
+        cls.patcher.stop()
+
     def setUp(self):
         """Setup things to be run when tests are started."""
         self.hass = get_test_home_assistant()

tests/components/homekit/test_type_lights.py

@@ -2,7 +2,6 @@
 import unittest
 
 from homeassistant.core import callback
-from homeassistant.components.homekit.type_lights import Light
 from homeassistant.components.light import (
     DOMAIN, ATTR_BRIGHTNESS, ATTR_BRIGHTNESS_PCT, ATTR_COLOR_TEMP,
     ATTR_HS_COLOR, SUPPORT_BRIGHTNESS, SUPPORT_COLOR_TEMP, SUPPORT_COLOR)
@@ -12,11 +11,26 @@ from homeassistant.const import (
     SERVICE_TURN_OFF, STATE_ON, STATE_OFF, STATE_UNKNOWN)
 
 from tests.common import get_test_home_assistant
+from tests.components.homekit.test_accessories import patch_debounce
 
 
 class TestHomekitLights(unittest.TestCase):
     """Test class for all accessory types regarding lights."""
 
+    @classmethod
+    def setUpClass(cls):
+        """Setup Light class import and debounce patcher."""
+        cls.patcher = patch_debounce()
+        cls.patcher.start()
+        _import = __import__('homeassistant.components.homekit.type_lights',
+                             fromlist=['Light'])
+        cls.light_cls = _import.Light
+
+    @classmethod
+    def tearDownClass(cls):
+        """Stop debounce patcher."""
+        cls.patcher.stop()
+
     def setUp(self):
         """Setup things to be run when tests are started."""
         self.hass = get_test_home_assistant()
@@ -38,7 +52,7 @@ class TestHomekitLights(unittest.TestCase):
         entity_id = 'light.demo'
         self.hass.states.set(entity_id, STATE_ON,
                              {ATTR_SUPPORTED_FEATURES: 0})
-        acc = Light(self.hass, entity_id, 'Light', aid=2)
+        acc = self.light_cls(self.hass, entity_id, 'Light', aid=2)
         self.assertEqual(acc.aid, 2)
         self.assertEqual(acc.category, 5)  # Lightbulb
         self.assertEqual(acc.char_on.value, 0)
@@ -82,7 +96,7 @@ class TestHomekitLights(unittest.TestCase):
         entity_id = 'light.demo'
         self.hass.states.set(entity_id, STATE_ON, {
             ATTR_SUPPORTED_FEATURES: SUPPORT_BRIGHTNESS, ATTR_BRIGHTNESS: 255})
-        acc = Light(self.hass, entity_id, 'Light', aid=2)
+        acc = self.light_cls(self.hass, entity_id, 'Light', aid=2)
         self.assertEqual(acc.char_brightness.value, 0)
 
         acc.run()
@@ -124,7 +138,7 @@ class TestHomekitLights(unittest.TestCase):
         self.hass.states.set(entity_id, STATE_ON, {
             ATTR_SUPPORTED_FEATURES: SUPPORT_COLOR_TEMP,
             ATTR_COLOR_TEMP: 190})
-        acc = Light(self.hass, entity_id, 'Light', aid=2)
+        acc = self.light_cls(self.hass, entity_id, 'Light', aid=2)
         self.assertEqual(acc.char_color_temperature.value, 153)
 
         acc.run()
@@ -146,7 +160,7 @@ class TestHomekitLights(unittest.TestCase):
         self.hass.states.set(entity_id, STATE_ON, {
             ATTR_SUPPORTED_FEATURES: SUPPORT_COLOR,
             ATTR_HS_COLOR: (260, 90)})
-        acc = Light(self.hass, entity_id, 'Light', aid=2)
+        acc = self.light_cls(self.hass, entity_id, 'Light', aid=2)
         self.assertEqual(acc.char_hue.value, 0)
         self.assertEqual(acc.char_saturation.value, 75)
 

tests/components/homekit/test_type_thermostats.py

@@ -6,17 +6,32 @@ from homeassistant.components.climate import (
     ATTR_CURRENT_TEMPERATURE, ATTR_TEMPERATURE,
     ATTR_TARGET_TEMP_LOW, ATTR_TARGET_TEMP_HIGH, ATTR_OPERATION_MODE,
     ATTR_OPERATION_LIST, STATE_COOL, STATE_HEAT, STATE_AUTO)
-from homeassistant.components.homekit.type_thermostats import Thermostat
 from homeassistant.const import (
     ATTR_SERVICE, EVENT_CALL_SERVICE, ATTR_SERVICE_DATA,
     ATTR_UNIT_OF_MEASUREMENT, STATE_OFF, TEMP_CELSIUS, TEMP_FAHRENHEIT)
 
 from tests.common import get_test_home_assistant
+from tests.components.homekit.test_accessories import patch_debounce
 
 
 class TestHomekitThermostats(unittest.TestCase):
     """Test class for all accessory types regarding thermostats."""
 
+    @classmethod
+    def setUpClass(cls):
+        """Setup Thermostat class import and debounce patcher."""
+        cls.patcher = patch_debounce()
+        cls.patcher.start()
+        _import = __import__(
+            'homeassistant.components.homekit.type_thermostats',
+            fromlist=['Thermostat'])
+        cls.thermostat_cls = _import.Thermostat
+
+    @classmethod
+    def tearDownClass(cls):
+        """Stop debounce patcher."""
+        cls.patcher.stop()
+
     def setUp(self):
         """Setup things to be run when tests are started."""
         self.hass = get_test_home_assistant()
@@ -37,7 +52,7 @@ class TestHomekitThermostats(unittest.TestCase):
         """Test if accessory and HA are updated accordingly."""
         climate = 'climate.test'
 
-        acc = Thermostat(self.hass, climate, 'Climate', False, aid=2)
+        acc = self.thermostat_cls(self.hass, climate, 'Climate', False, aid=2)
         acc.run()
 
         self.assertEqual(acc.aid, 2)
@@ -172,7 +187,7 @@ class TestHomekitThermostats(unittest.TestCase):
         """Test if accessory and HA are updated accordingly."""
         climate = 'climate.test'
 
-        acc = Thermostat(self.hass, climate, 'Climate', True)
+        acc = self.thermostat_cls(self.hass, climate, 'Climate', True)
         acc.run()
 
         self.assertEqual(acc.char_cooling_thresh_temp.value, 23.0)
@@ -242,7 +257,7 @@ class TestHomekitThermostats(unittest.TestCase):
         """Test if accessory and HA are updated accordingly."""
         climate = 'climate.test'
 
-        acc = Thermostat(self.hass, climate, 'Climate', True)
+        acc = self.thermostat_cls(self.hass, climate, 'Climate', True)
         acc.run()
 
         self.hass.states.set(climate, STATE_AUTO,