From 262ea14e5a9bc63ba9b8f915874df0cf9517f412 Mon Sep 17 00:00:00 2001 From: cdce8p <30130371+cdce8p@users.noreply.github.com> Date: Fri, 6 Apr 2018 23:11:53 +0200 Subject: [PATCH] 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 --- .../components/homekit/accessories.py | 51 +++++++++++++++++-- homeassistant/components/homekit/const.py | 1 + .../components/homekit/type_lights.py | 3 +- .../components/homekit/type_thermostats.py | 5 +- tests/components/homekit/test_accessories.py | 48 ++++++++++++++++- tests/components/homekit/test_homekit.py | 12 +++++ tests/components/homekit/test_type_lights.py | 24 +++++++-- .../homekit/test_type_thermostats.py | 23 +++++++-- 8 files changed, 151 insertions(+), 16 deletions(-) diff --git a/homeassistant/components/homekit/accessories.py b/homeassistant/components/homekit/accessories.py index da45bee9e90..ec2c49f5e43 100644 --- a/homeassistant/components/homekit/accessories.py +++ b/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, - MANUFACTURER, SERV_ACCESSORY_INFO, CHAR_MANUFACTURER, CHAR_MODEL, - CHAR_NAME, CHAR_SERIAL_NUMBER) + DEBOUNCE_TIMEOUT, ACCESSORY_MODEL, ACCESSORY_NAME, BRIDGE_MODEL, + BRIDGE_NAME, MANUFACTURER, SERV_ACCESSORY_INFO, CHAR_MANUFACTURER, + 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 diff --git a/homeassistant/components/homekit/const.py b/homeassistant/components/homekit/const.py index d1c3d84b517..18d02a89e18 100644 --- a/homeassistant/components/homekit/const.py +++ b/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 diff --git a/homeassistant/components/homekit/type_lights.py b/homeassistant/components/homekit/type_lights.py index 1110981fe10..4fbfb995859 100644 --- a/homeassistant/components/homekit/type_lights.py +++ b/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) diff --git a/homeassistant/components/homekit/type_thermostats.py b/homeassistant/components/homekit/type_thermostats.py index de8ecbdfe3e..daf81c51c4d 100644 --- a/homeassistant/components/homekit/type_thermostats.py +++ b/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', diff --git a/tests/components/homekit/test_accessories.py b/tests/components/homekit/test_accessories.py index a2facd826e4..b7bf625a2d6 100644 --- a/tests/components/homekit/test_accessories.py +++ b/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() diff --git a/tests/components/homekit/test_homekit.py b/tests/components/homekit/test_homekit.py index c6d79545487..51a965b5817 100644 --- a/tests/components/homekit/test_homekit.py +++ b/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() diff --git a/tests/components/homekit/test_type_lights.py b/tests/components/homekit/test_type_lights.py index 1d18235d4a1..af8676dfd74 100644 --- a/tests/components/homekit/test_type_lights.py +++ b/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) diff --git a/tests/components/homekit/test_type_thermostats.py b/tests/components/homekit/test_type_thermostats.py index d363e26d712..feea5c0d01a 100644 --- a/tests/components/homekit/test_type_thermostats.py +++ b/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,