From 47fc1deecbd29b9154cbcdf03f165299a8af642c Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Thu, 8 Oct 2015 23:49:55 -0700 Subject: [PATCH 1/2] Fix throttle to work on instance-level --- homeassistant/util/__init__.py | 31 +++++++++++++++++++------------ tests/util/test_init.py | 11 +++++++++++ 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/homeassistant/util/__init__.py b/homeassistant/util/__init__.py index 805937376a0..2d449285493 100644 --- a/homeassistant/util/__init__.py +++ b/homeassistant/util/__init__.py @@ -233,35 +233,42 @@ class Throttle(object): self.limit_no_throttle = limit_no_throttle def __call__(self, method): - lock = threading.Lock() - if self.limit_no_throttle is not None: method = Throttle(self.limit_no_throttle)(method) + # We want to be able to differentiate between function and method calls + # All methods have the classname in their qualname seperated by a '.' + # Functions have a '.' in their qualname if defined inline, but will + # be prefixed by '..' so we strip that out. + is_func = '.' not in method.__qualname__.split('..')[-1] + @wraps(method) def wrapper(*args, **kwargs): """ Wrapper that allows wrapped to be called only once per min_time. If we cannot acquire the lock, it is running so return None. """ - if not lock.acquire(False): + # pylint: disable=protected-access + host = wrapper if is_func else args[0] + if not hasattr(host, '_throttle_lock'): + host._throttle_lock = threading.Lock() + + if not host._throttle_lock.acquire(False): return None + + last_call = getattr(host, '_throttle_last_call', None) + # Check if method is never called or no_throttle is given + force = not last_call or kwargs.pop('no_throttle', False) + try: - last_call = wrapper.last_call - - # Check if method is never called or no_throttle is given - force = not last_call or kwargs.pop('no_throttle', False) - if force or utcnow() - last_call > self.min_time: result = method(*args, **kwargs) - wrapper.last_call = utcnow() + host._throttle_last_call = utcnow() return result else: return None finally: - lock.release() - - wrapper.last_call = None + host._throttle_lock.release() return wrapper diff --git a/tests/util/test_init.py b/tests/util/test_init.py index 94358f5eb51..2bf917f4e25 100644 --- a/tests/util/test_init.py +++ b/tests/util/test_init.py @@ -218,3 +218,14 @@ class TestUtil(unittest.TestCase): self.assertEqual(3, len(calls1)) self.assertEqual(2, len(calls2)) + + def test_throttle_per_instance(self): + """ Test that the throttle method is done per instance of a class. """ + + class Tester(object): + @util.Throttle(timedelta(seconds=1)) + def hello(self): + return True + + self.assertTrue(Tester().hello()) + self.assertTrue(Tester().hello()) From be8089bcde5f28a531fd086e47deb4d564766562 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Thu, 8 Oct 2015 23:50:04 -0700 Subject: [PATCH 2/2] Cleanup arest --- homeassistant/components/sensor/arest.py | 63 +++++++++++++----------- 1 file changed, 33 insertions(+), 30 deletions(-) diff --git a/homeassistant/components/sensor/arest.py b/homeassistant/components/sensor/arest.py index cfe88e0f0d6..7d2192e8920 100644 --- a/homeassistant/components/sensor/arest.py +++ b/homeassistant/components/sensor/arest.py @@ -47,7 +47,7 @@ Format of a default JSON response by aREST: } """ import logging -from requests import get, exceptions +import requests from datetime import timedelta from homeassistant.util import Throttle @@ -58,36 +58,42 @@ _LOGGER = logging.getLogger(__name__) # Return cached results if last scan was less then this time ago MIN_TIME_BETWEEN_UPDATES = timedelta(seconds=60) +CONF_RESOURCE = 'resource' +CONF_MONITORED_VARIABLES = 'monitored_variables' + def setup_platform(hass, config, add_devices, discovery_info=None): """ Get the aREST sensor. """ - resource = config.get('resource', None) + resource = config.get(CONF_RESOURCE) + var_conf = config.get(CONF_MONITORED_VARIABLES) + + if None in (resource, var_conf): + _LOGGER.error('Not all required config keys present: %s', + ', '.join((CONF_RESOURCE, CONF_MONITORED_VARIABLES))) + return False try: - response = get(resource, timeout=10) - except exceptions.MissingSchema: + response = requests.get(resource, timeout=10).json() + except requests.exceptions.MissingSchema: _LOGGER.error("Missing resource or schema in configuration. " "Add http:// to your URL.") return False - except exceptions.ConnectionError: + except requests.exceptions.ConnectionError: _LOGGER.error("No route to device. " "Please check the IP address in the configuration file.") return False - rest = ArestData(resource) + arest = ArestData(resource) dev = [] for variable in config['monitored_variables']: - if 'unit' not in variable: - variable['unit'] = ' ' - if variable['name'] not in response.json()['variables']: + if variable['name'] not in response['variables']: _LOGGER.error('Variable: "%s" does not exist', variable['name']) - else: - dev.append(ArestSensor(rest, - response.json()['name'], - variable['name'], - variable['unit'])) + continue + + dev.append(ArestSensor(arest, response['name'], variable['name'], + variable.get('unit'))) add_devices(dev) @@ -95,8 +101,8 @@ def setup_platform(hass, config, add_devices, discovery_info=None): class ArestSensor(Entity): """ Implements an aREST sensor. """ - def __init__(self, rest, location, variable, unit_of_measurement): - self.rest = rest + def __init__(self, arest, location, variable, unit_of_measurement): + self.arest = arest self._name = '{} {}'.format(location.title(), variable.title()) self._variable = variable self._state = 'n/a' @@ -116,17 +122,16 @@ class ArestSensor(Entity): @property def state(self): """ Returns the state of the device. """ - return self._state - - def update(self): - """ Gets the latest data from aREST API and updates the state. """ - self.rest.update() - values = self.rest.data + values = self.arest.data if 'error' in values: - self._state = values['error'] + return values['error'] else: - self._state = values[self._variable] + return values.get(self._variable, 'n/a') + + def update(self): + """ Gets the latest data from aREST API. """ + self.arest.update() # pylint: disable=too-few-public-methods @@ -135,16 +140,14 @@ class ArestData(object): def __init__(self, resource): self.resource = resource - self.data = dict() + self.data = {} @Throttle(MIN_TIME_BETWEEN_UPDATES) def update(self): """ Gets the latest data from aREST device. """ try: - response = get(self.resource, timeout=10) - if 'error' in self.data: - del self.data['error'] + response = requests.get(self.resource, timeout=10) self.data = response.json()['variables'] - except exceptions.ConnectionError: + except requests.exceptions.ConnectionError: _LOGGER.error("No route to device. Is device offline?") - self.data['error'] = 'n/a' + self.data = {'error': 'error fetching'}