Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pvizeli merged 3 commits into from
Apr 6, 2018
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2c2c830
Add async timeout feature
cdce8p Mar 29, 2018
f9ec5fa
Use dict for lastargs
cdce8p Apr 6, 2018
2649bd0
Updated tests to stop patch
cdce8p Apr 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 47 additions & 4 deletions homeassistant/components/homekit/accessories.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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[0]
hass.async_add_job(func, *lastargs[1:])

@balloob balloob Apr 6, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this weird. it used to take hass but now we don't pass that on?

@balloob balloob Apr 6, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see now how you're storing this. Can we store it in a dictionary? That makes things a lot more readable.

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, *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
Expand Down
1 change: 1 addition & 0 deletions homeassistant/components/homekit/const.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down
3 changes: 2 additions & 1 deletion homeassistant/components/homekit/type_lights.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
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)
Expand Down Expand Up @@ -94,6 +94,7 @@ def set_state(self, value):
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)
Expand Down
5 changes: 4 additions & 1 deletion homeassistant/components/homekit/type_thermostats.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
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,
Expand Down Expand Up @@ -105,6 +105,7 @@ def set_heat_cool(self, value):
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',
Expand All @@ -118,6 +119,7 @@ def set_cooling_threshold(self, value):
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',
Expand All @@ -132,6 +134,7 @@ def set_heating_threshold(self, value):
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',
Expand Down
42 changes: 41 additions & 1 deletion tests/components/homekit/test_accessories.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,61 @@

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


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()
Expand Down
6 changes: 5 additions & 1 deletion tests/components/homekit/test_type_lights.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
"""Test different accessory types: Lights."""
import unittest
from unittest.mock import patch

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)
Expand All @@ -13,6 +13,10 @@

from tests.common import get_test_home_assistant

patch('homeassistant.components.homekit.accessories.debounce',

@balloob balloob Apr 6, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is bad as now loading this file impacts all other tests that use homekit.

Instead, you should use a pytest fixture. But to be able to leverage pytest fixtures, you will need to rewrite your tests to be just methods. Something like this: (written from top of my head, didn't test)

import pytest

@pytest.fixture(autouse=True)
def mock_debounce():
    with patch('…'):
        yield


@pytest.fixture
def mock_call_service(hass):
    events = []

    @callback
    def record_event(event):
        …

    hass.bus.listen(…)
    return events


def test_light_basic(hass, mock_call_service):
    # hass is a new test instance, will be automatically stopped for you
    # mock_call_service is equivalent to self.events
    # Instead of self.assertEqual, just use assert
    assert acc.aid == 2

lambda f: lambda *args, **kwargs: f(*args, **kwargs)).start()
from homeassistant.components.homekit.type_lights import Light # noqa: E402


class TestHomekitLights(unittest.TestCase):
"""Test class for all accessory types regarding lights."""
Expand Down
7 changes: 6 additions & 1 deletion tests/components/homekit/test_type_thermostats.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,23 @@
"""Test different accessory types: Thermostats."""
import unittest
from unittest.mock import patch

from homeassistant.core import callback
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

patch('homeassistant.components.homekit.accessories.debounce',

@balloob balloob Apr 6, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be removed too.

lambda f: lambda *args, **kwargs: f(*args, **kwargs)).start()
from homeassistant.components.homekit.type_thermostats import ( # noqa: E402
Thermostat)


class TestHomekitThermostats(unittest.TestCase):
"""Test class for all accessory types regarding thermostats."""
Expand Down