From 41bbcf060cde6eff0220f2360401c62ac28cb72e Mon Sep 17 00:00:00 2001
From: Stefan Lehmann <stefan.st.lehmann@gmail.com>
Date: Sun, 8 Oct 2017 16:03:16 +0200
Subject: [PATCH] ads component with platforms switch, binary_sensor, light,
 sensor

add locking

poll sensors at startup

update state of ads switch and light

update ads requirements

remove update() from constructors on ads platforms

omit ads coverage

ads catch read error when polling
---
 .coveragerc                                   |  3 +
 homeassistant/components/ads.py               | 77 +++++++++++--------
 homeassistant/components/binary_sensor/ads.py | 49 +++++++++---
 homeassistant/components/light/ads.py         | 25 ++++--
 homeassistant/components/sensor/ads.py        | 66 +++++++++++++---
 homeassistant/components/switch/ads.py        | 15 +++-
 requirements_all.txt                          |  3 +
 7 files changed, 177 insertions(+), 61 deletions(-)

diff --git a/.coveragerc b/.coveragerc
index 58bdac1ff31..d0a29acc897 100644
--- a/.coveragerc
+++ b/.coveragerc
@@ -11,6 +11,9 @@ omit =
     homeassistant/components/abode.py
     homeassistant/components/*/abode.py
 
+    homeassistant/components/ads.py
+    homeassistant/components/*/ads.py
+
     homeassistant/components/alarmdecoder.py
     homeassistant/components/*/alarmdecoder.py
 
diff --git a/homeassistant/components/ads.py b/homeassistant/components/ads.py
index 16cd16bc181..4866e0ee53b 100644
--- a/homeassistant/components/ads.py
+++ b/homeassistant/components/ads.py
@@ -4,8 +4,8 @@ ADS Component.
 For more details about this component, please refer to the documentation.
 
 """
+import threading
 import struct
-import time
 import logging
 import ctypes
 from collections import namedtuple
@@ -13,10 +13,8 @@ import voluptuous as vol
 from homeassistant.const import CONF_DEVICE, CONF_PORT, CONF_IP_ADDRESS, \
     EVENT_HOMEASSISTANT_STOP
 import homeassistant.helpers.config_validation as cv
-import pyads
-from pyads import PLCTYPE_BOOL, PLCTYPE_INT, PLCTYPE_UINT, PLCTYPE_BYTE
 
-REQUIREMENTS = ['pyads']
+REQUIREMENTS = ['pyads==2.2.6']
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -28,17 +26,17 @@ ADSTYPE_UINT = 'uint'
 ADSTYPE_BYTE = 'byte'
 ADSTYPE_BOOL = 'bool'
 
-ADS_TYPEMAP = {
-    ADSTYPE_BOOL: PLCTYPE_BOOL,
-    ADSTYPE_BYTE: PLCTYPE_BYTE,
-    ADSTYPE_INT: PLCTYPE_INT,
-    ADSTYPE_UINT: PLCTYPE_UINT,
-}
-
 
 ADS_PLATFORMS = ['switch', 'binary_sensor', 'light']
 DOMAIN = 'ads'
 
+# config variable names
+CONF_ADSVAR = 'adsvar'
+CONF_ADSTYPE = 'adstype'
+CONF_ADS_USE_NOTIFY = 'use_notify'
+CONF_ADS_POLL_INTERVAL = 'poll_interval'
+CONF_ADS_FACTOR = 'factor'
+
 CONFIG_SCHEMA = vol.Schema({
     DOMAIN: vol.Schema({
         vol.Required(CONF_DEVICE): cv.string,
@@ -47,8 +45,12 @@ CONFIG_SCHEMA = vol.Schema({
     })
 }, extra=vol.ALLOW_EXTRA)
 
+MAX_RETRIES = 5
+RETRY_SLEEPTIME_S = 0.1
+
 
 def setup(hass, config):
+    import pyads
     """ Set up the ADS component. """
     _LOGGER.info('created ADS client')
     conf = config[DOMAIN]
@@ -81,12 +83,29 @@ class AdsHub:
     """ Representation of a PyADS connection. """
 
     def __init__(self, ads_client):
+        from pyads import PLCTYPE_BOOL, PLCTYPE_BYTE, PLCTYPE_INT, \
+            PLCTYPE_UINT, ADSError
+
+        self.ADS_TYPEMAP = {
+            ADSTYPE_BOOL: PLCTYPE_BOOL,
+            ADSTYPE_BYTE: PLCTYPE_BYTE,
+            ADSTYPE_INT: PLCTYPE_INT,
+            ADSTYPE_UINT: PLCTYPE_UINT,
+        }
+
+        self.PLCTYPE_BOOL = PLCTYPE_BOOL
+        self.PLCTYPE_BYTE = PLCTYPE_BYTE
+        self.PLCTYPE_INT = PLCTYPE_INT
+        self.PLCTYPE_UINT = PLCTYPE_UINT
+        self.ADSError = ADSError
+
         self._client = ads_client
         self._client.open()
 
         # all ADS devices are registered here
         self._devices = []
         self._notification_items = {}
+        self._lock = threading.Lock()
 
     def shutdown(self, *args, **kwargs):
         _LOGGER.debug('Shutting down ADS')
@@ -105,28 +124,23 @@ class AdsHub:
         self._devices.append(device)
 
     def write_by_name(self, name, value, plc_datatype):
-        return self._client.write_by_name(name, value, plc_datatype)
+        with self._lock:
+            return self._client.write_by_name(name, value, plc_datatype)
 
     def read_by_name(self, name, plc_datatype):
-        return self._client.read_by_name(name, plc_datatype)
+        with self._lock:
+            return self._client.read_by_name(name, plc_datatype)
 
     def add_device_notification(self, name, plc_datatype, callback):
+        from pyads import NotificationAttrib
         """ Add a notification to the ADS devices. """
-        attr = pyads.NotificationAttrib(ctypes.sizeof(plc_datatype))
+        attr = NotificationAttrib(ctypes.sizeof(plc_datatype))
 
-        for i in range(5):
-            try:
-                hnotify, huser = self._client.add_device_notification(
-                    name, attr, self._device_notification_callback
-                )
-                hnotify = int(hnotify)
-                break
-            except pyads.pyads.ADSError:
-                _LOGGER.debug('Could not add notification for "{0}". Retrying...'
-                              .format(name))
-                time.sleep(0.1)
-        else:
-            return False
+        with self._lock:
+            hnotify, huser = self._client.add_device_notification(
+                name, attr, self._device_notification_callback
+            )
+            hnotify = int(hnotify)
 
         _LOGGER.debug('Added Device Notification {0} for variable {1}'
                       .format(hnotify, name))
@@ -136,6 +150,7 @@ class AdsHub:
         )
 
     def _device_notification_callback(self, addr, notification, huser):
+        from pyads import PLCTYPE_BOOL, PLCTYPE_INT, PLCTYPE_BYTE, PLCTYPE_UINT
         contents = notification.contents
 
         hnotify = int(contents.hNotification)
@@ -150,13 +165,13 @@ class AdsHub:
             return
 
         # parse data to desired datatype
-        if notification_item.plc_datatype == pyads.PLCTYPE_BOOL:
+        if notification_item.plc_datatype == PLCTYPE_BOOL:
             value = bool(struct.unpack('<?', bytearray(data)[:1])[0])
-        elif notification_item.plc_datatype == pyads.PLCTYPE_INT:
+        elif notification_item.plc_datatype == PLCTYPE_INT:
             value = struct.unpack('<h', bytearray(data)[:2])[0]
-        elif notification_item.plc_datatype == pyads.PLCTYPE_BYTE:
+        elif notification_item.plc_datatype == PLCTYPE_BYTE:
             value = struct.unpack('<B', bytearray(data)[:1])[0]
-        elif notification_item.plc_datatype == pyads.PLCTYPE_UINT:
+        elif notification_item.plc_datatype == PLCTYPE_UINT:
             value = struct.unpack('<H', bytearray(data)[:2])[0]
         else:
             value = bytearray(data)
diff --git a/homeassistant/components/binary_sensor/ads.py b/homeassistant/components/binary_sensor/ads.py
index a45bdc32a16..21f31e57fe9 100644
--- a/homeassistant/components/binary_sensor/ads.py
+++ b/homeassistant/components/binary_sensor/ads.py
@@ -3,29 +3,31 @@ Support for ADS binary sensors.
 
 """
 import logging
+from datetime import timedelta
 
 import voluptuous as vol
 
 from homeassistant.components.binary_sensor import BinarySensorDevice, \
-    PLATFORM_SCHEMA, DEVICE_CLASSES_SCHEMA, STATE_ON
-from homeassistant.components.ads import DATA_ADS, PLCTYPE_BOOL
+    PLATFORM_SCHEMA, DEVICE_CLASSES_SCHEMA
+from homeassistant.components.ads import DATA_ADS, CONF_ADSVAR, \
+    CONF_ADS_USE_NOTIFY, CONF_ADS_POLL_INTERVAL
 from homeassistant.const import CONF_NAME, CONF_DEVICE_CLASS
+from homeassistant.helpers.event import async_track_time_interval
 import homeassistant.helpers.config_validation as cv
 
 
 _LOGGER = logging.getLogger(__name__)
 
-
 DEPENDENCIES = ['ads']
 DEFAULT_NAME = 'ADS binary sensor'
 
-CONF_ADSVAR = 'adsvar'
-
 
 PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
     vol.Required(CONF_ADSVAR): cv.string,
     vol.Optional(CONF_NAME, default=DEFAULT_NAME): cv.string,
     vol.Optional(CONF_DEVICE_CLASS): DEVICE_CLASSES_SCHEMA,
+    vol.Optional(CONF_ADS_USE_NOTIFY, default=True): cv.boolean,
+    vol.Optional(CONF_ADS_POLL_INTERVAL, default=1000): cv.positive_int,
 })
 
 
@@ -38,22 +40,37 @@ def setup_platform(hass, config, add_devices, discovery_info=None):
     adsvar = config.get(CONF_ADSVAR)
     name = config.get(CONF_NAME)
     device_class = config.get(CONF_DEVICE_CLASS)
+    use_notify = config.get(CONF_ADS_USE_NOTIFY)
+    poll_interval = config.get(CONF_ADS_POLL_INTERVAL)
 
-    add_devices([AdsBinarySensor(ads_hub, name, adsvar, device_class)])
+    ads_sensor = AdsBinarySensor(ads_hub, name, adsvar, device_class,
+                                 use_notify, poll_interval)
+    add_devices([ads_sensor], True)
+
+    if use_notify:
+        ads_hub.add_device_notification(adsvar, ads_hub.PLCTYPE_BOOL,
+                                        ads_sensor.callback)
+    else:
+        dtime = timedelta(0, 0, poll_interval * 1000)
+        async_track_time_interval(hass, ads_sensor.poll, dtime)
 
 
 class AdsBinarySensor(BinarySensorDevice):
     """ Representation of ADS binary sensors. """
 
-    def __init__(self, ads_hub, name, adsvar, device_class):
+    def __init__(self, ads_hub, name, adsvar, device_class, use_notify,
+                 poll_interval):
         self._name = name
         self._state = False
         self._device_class = device_class or 'moving'
         self._ads_hub = ads_hub
         self.adsvar = adsvar
+        self.use_notify = use_notify
+        self.poll_interval = poll_interval
 
-        self._ads_hub.add_device_notification(self.adsvar, PLCTYPE_BOOL,
-                                              self.callback)
+        # make first poll if notifications disabled
+        if not self.use_notify:
+            self.poll(None)
 
     @property
     def name(self):
@@ -78,3 +95,17 @@ class AdsBinarySensor(BinarySensorDevice):
             self.schedule_update_ha_state()
         except AttributeError:
             pass
+
+    def poll(self, now):
+        try:
+            self._state = self._ads_hub.read_by_name(self.adsvar,
+                                                     self._ads_hub.PLCTYPE_BOOL)
+            _LOGGER.debug('Polled value for bool variable {0}: {1}'
+                          .format(self.adsvar, self._state))
+        except self._ads_hub.ADSError as e:
+            _LOGGER.error(e)
+
+        try:
+            self.schedule_update_ha_state()
+        except AttributeError:
+            pass
diff --git a/homeassistant/components/light/ads.py b/homeassistant/components/light/ads.py
index 1bc03973967..a4ce58d7947 100644
--- a/homeassistant/components/light/ads.py
+++ b/homeassistant/components/light/ads.py
@@ -9,13 +9,12 @@ import voluptuous as vol
 from homeassistant.components.light import Light, ATTR_BRIGHTNESS, \
     SUPPORT_BRIGHTNESS, PLATFORM_SCHEMA
 from homeassistant.const import CONF_NAME
-from homeassistant.components.ads import DATA_ADS, PLCTYPE_BOOL, PLCTYPE_UINT
+from homeassistant.components.ads import DATA_ADS, CONF_ADSVAR
 import homeassistant.helpers.config_validation as cv
 
 _LOGGER = logging.getLogger(__name__)
 DEPENDENCIES = ['ads']
 DEFAULT_NAME = 'ADS Light'
-CONF_ADSVAR = 'adsvar'
 CONF_ADSVAR_BRIGHTNESS = 'adsvar_brightness'
 SUPPORT_ADS = SUPPORT_BRIGHTNESS
 PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
@@ -35,7 +34,7 @@ def setup_platform(hass, config, add_devices, discovery_info=None):
     devname = config.get(CONF_NAME)
 
     add_devices([AdsLight(ads_hub, varname_enable, varname_brightness,
-                          devname)])
+                          devname)], True)
 
 
 class AdsLight(Light):
@@ -83,11 +82,12 @@ class AdsLight(Light):
 
         bval = math.floor(self._brightness / 256.0 * 100.0)
 
-        self._ads_hub.write_by_name(self.varname_enable, True, PLCTYPE_BOOL)
+        self._ads_hub.write_by_name(self.varname_enable, True,
+                                    self._ads_hub.PLCTYPE_BOOL)
 
         if self.varname_brightness is not None:
             self._ads_hub.write_by_name(self.varname_brightness, bval,
-                                        PLCTYPE_UINT)
+                                        self._ads_hub.PLCTYPE_UINT)
 
         self._on_state = True
 
@@ -97,13 +97,22 @@ class AdsLight(Light):
         if brightness is not None:
             self._brightness = brightness
         bval = math.floor(self._brightness / 256.0 * 100.0)
-        self._ads_hub.write_by_name(self.varname_brightness, bval,
-                                    PLCTYPE_UINT)
         self._ads_hub.write_by_name(self.varname_enable, False,
-                                    PLCTYPE_BOOL)
+                                    self._ads_hub.PLCTYPE_BOOL)
+        if self.varname_brightness is not None:
+            self._ads_hub.write_by_name(self.varname_brightness, bval,
+                                        self._ads_hub.PLCTYPE_UINT)
         self._on_state = False
 
     def value_changed(self, val):
         self._brightness = math.floor(val / 100.0 * 256.0)
         self._on_state = bool(val != 0)
         self.schedule_update_ha_state()
+
+    def update(self):
+        self._on_state = self._ads_hub.read_by_name(self.varname_enable,
+                                                    self._ads_hub.PLCTYPE_BOOL)
+        if self.varname_brightness is not None:
+            self._brightness = self._ads_hub.read_by_name(
+                self.varname_brightness, self._ads_hub.PLCTYPE_UINT
+            )
diff --git a/homeassistant/components/sensor/ads.py b/homeassistant/components/sensor/ads.py
index 71b8330ae41..ea7ac793048 100644
--- a/homeassistant/components/sensor/ads.py
+++ b/homeassistant/components/sensor/ads.py
@@ -3,21 +3,22 @@ Support for ADS sensors.__init__.py
 
 """
 import logging
+from datetime import timedelta
 import voluptuous as vol
 from homeassistant.components.sensor import PLATFORM_SCHEMA
 from homeassistant.const import CONF_NAME, CONF_UNIT_OF_MEASUREMENT
 from homeassistant.helpers.entity import Entity
 import homeassistant.helpers.config_validation as cv
+from homeassistant.helpers.event import async_track_time_interval
 from homeassistant.components import ads
+from homeassistant.components.ads import CONF_ADSVAR, CONF_ADSTYPE, \
+    CONF_ADS_USE_NOTIFY, CONF_ADS_POLL_INTERVAL, CONF_ADS_FACTOR
 
 _LOGGER = logging.getLogger(__name__)
 
 DEFAULT_NAME = 'ADS sensor'
 DEPENDENCIES = ['ads']
 
-CONF_ADSVAR = 'adsvar'
-CONF_ADSTYPE = 'adstype'
-
 PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
     vol.Required(CONF_ADSVAR): cv.string,
     vol.Optional(CONF_NAME, default=DEFAULT_NAME): cv.string,
@@ -25,6 +26,9 @@ PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
     vol.Optional(CONF_ADSTYPE, default=ads.ADSTYPE_INT): vol.In(
         [ads.ADSTYPE_INT, ads.ADSTYPE_UINT, ads.ADSTYPE_BYTE]
     ),
+    vol.Optional(CONF_ADS_USE_NOTIFY, default=True): cv.boolean,
+    vol.Optional(CONF_ADS_POLL_INTERVAL, default=1000): cv.positive_int,
+    vol.Optional(CONF_ADS_FACTOR): cv.positive_int,
 })
 
 
@@ -38,23 +42,40 @@ def setup_platform(hass, config, add_devices, discovery_info=None):
     adstype = config.get(CONF_ADSTYPE)
     name = config.get(CONF_NAME)
     unit_of_measurement = config.get(CONF_UNIT_OF_MEASUREMENT)
+    use_notify = config.get(CONF_ADS_USE_NOTIFY)
+    poll_interval = config.get(CONF_ADS_POLL_INTERVAL)
+    factor = config.get(CONF_ADS_FACTOR)
 
-    add_devices([AdsSensor(ads_hub, adsvar, adstype, name,
-                           unit_of_measurement)])
+    entity = AdsSensor(ads_hub, adsvar, adstype, name,
+                       unit_of_measurement, use_notify, poll_interval, factor)
+
+    add_devices([entity])
+
+    if use_notify:
+        ads_hub.add_device_notification(adsvar, ads_hub.ADS_TYPEMAP[adstype],
+                                        entity.callback)
+    else:
+        dtime = timedelta(0, 0, poll_interval * 1000)
+        async_track_time_interval(hass, entity.poll, dtime)
 
 
 class AdsSensor(Entity):
 
-    def __init__(self, ads_hub, adsvar, adstype, devname, unit_of_measurement):
+    def __init__(self, ads_hub, adsvar, adstype, devname, unit_of_measurement,
+                 use_notify, poll_interval, factor):
         self._ads_hub = ads_hub
         self._name = devname
         self._value = 0
         self._unit_of_measurement = unit_of_measurement
         self.adsvar = adsvar
         self.adstype = adstype
+        self.use_notify = use_notify
+        self.poll_interval = poll_interval
+        self.factor = factor
 
-        self._ads_hub.add_device_notification(
-            self.adsvar, ads.ADS_TYPEMAP[adstype], self.callback)
+        # make first poll if notifications disabled
+        if not self.use_notify:
+            self.poll(None)
 
     @property
     def name(self):
@@ -72,7 +93,34 @@ class AdsSensor(Entity):
     def callback(self, name, value):
         _LOGGER.debug('Variable "{0}" changed its value to "{1}"'
                       .format(name, value))
-        self._value = value
+
+        # if factor is set use it otherwise not
+        if self.factor is None:
+            self._value = value
+        else:
+            self._value = value / self.factor
+
+        try:
+            self.schedule_update_ha_state()
+        except AttributeError:
+            pass
+
+    def poll(self, now):
+        try:
+            val = self._ads_hub.read_by_name(
+                self.adsvar, self._ads_hub.ADS_TYPEMAP[self.adstype]
+            )
+
+            if self.factor is None:
+                self._value = val
+            else:
+                self._value = val / self.factor
+
+            _LOGGER.debug('Polled value for bool variable {0}: {1}'
+                          .format(self.adsvar, self._value))
+        except self._ads_hub.ADSError as e:
+            _LOGGER.error(e)
+
         try:
             self.schedule_update_ha_state()
         except AttributeError:
diff --git a/homeassistant/components/switch/ads.py b/homeassistant/components/switch/ads.py
index c60b2351b55..56e62ef2155 100644
--- a/homeassistant/components/switch/ads.py
+++ b/homeassistant/components/switch/ads.py
@@ -7,7 +7,7 @@ import logging
 import voluptuous as vol
 from homeassistant.components.switch import PLATFORM_SCHEMA
 from homeassistant.const import CONF_NAME
-from homeassistant.components.ads import DATA_ADS, PLCTYPE_BOOL
+from homeassistant.components.ads import DATA_ADS
 from homeassistant.helpers.entity import ToggleEntity
 import homeassistant.helpers.config_validation as cv
 
@@ -30,7 +30,7 @@ def setup_platform(hass, config, add_devices, discovery_info=None):
     dev_name = config.get(CONF_NAME)
     ads_var = config.get(CONF_ADSVAR)
 
-    add_devices([AdsSwitch(ads_hub, dev_name, ads_var)])
+    add_devices([AdsSwitch(ads_hub, dev_name, ads_var)], True)
 
 
 class AdsSwitch(ToggleEntity):
@@ -51,13 +51,20 @@ class AdsSwitch(ToggleEntity):
         return self.dev_name
 
     def turn_on(self, **kwargs):
-        self._ads_hub.write_by_name(self.ads_var, True, PLCTYPE_BOOL)
+        self._ads_hub.write_by_name(self.ads_var, True,
+                                    self._ads_hub.PLCTYPE_BOOL)
         self._on_state = True
 
     def turn_off(self, **kwargs):
-        self._ads_hub.write_by_name(self.ads_var, False, PLCTYPE_BOOL)
+        self._ads_hub.write_by_name(self.ads_var, False,
+                                    self._ads_hub.PLCTYPE_BOOL)
         self._on_state = False
 
     def value_changed(self, val):
         self._on_state = val
         self.schedule_update_ha_state()
+
+    def update(self):
+        self._on_state = self._ads_hub.read_by_name(
+            self.ads_var, self._ads_hub.PLCTYPE_BOOL
+        )
diff --git a/requirements_all.txt b/requirements_all.txt
index 16cf4dc59c1..8d38b1687f6 100644
--- a/requirements_all.txt
+++ b/requirements_all.txt
@@ -561,6 +561,9 @@ pyTibber==0.1.1
 # homeassistant.components.switch.dlink
 pyW215==0.6.0
 
+# homeassistant.components.ads
+pyads==2.2.6
+
 # homeassistant.components.sensor.airvisual
 pyairvisual==1.0.0