Merge pull request #500 from balloob/arest-fix

Throttle per instance (fixes arest)

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)
+        response = requests.get(resource, timeout=10).json()
-    except exceptions.MissingSchema:
+    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:
+        if variable['name'] not in response['variables']:
-            variable['unit'] = ' '
-        if variable['name'] not in response.json()['variables']:
             _LOGGER.error('Variable: "%s" does not exist', variable['name'])
-        else:
+            continue
-            dev.append(ArestSensor(rest,
+
-                                   response.json()['name'],
+        dev.append(ArestSensor(arest, response['name'], variable['name'],
-                                   variable['name'],
+                               variable.get('unit')))
-                                   variable['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):
+    def __init__(self, arest, location, variable, unit_of_measurement):
-        self.rest = rest
+        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
+        values = self.arest.data
-
-    def update(self):
-        """ Gets the latest data from aREST API and updates the state. """
-        self.rest.update()
-        values = self.rest.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)
+            response = requests.get(self.resource, timeout=10)
-            if 'error' in self.data:
-                del self.data['error']
             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'}

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 '.<locals>.' so we strip that out.
+        is_func = '.' not in method.__qualname__.split('.<locals>.')[-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()
+                host._throttle_lock.release()
-
-        wrapper.last_call = None
 
         return wrapper
 

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())