Refactor of ADS integration and introduce ADSEntity (#22583)

* Prevent toogle to false at restart

* change to asyncio.run_coroutine_threadsafe

* refactor ADS platforms; introduce AdsEntity

* fix hound findings

* some formatting

* remove redundant def.

* fix useless super delegation

* fix inconsistent ADS data type for brightness

* fix requested changes

* fix comment

homeassistant/components/ads/__init__.py

@@ -4,12 +4,15 @@ import struct
 import logging
 import ctypes
 from collections import namedtuple
+import asyncio
+import async_timeout
 
 import voluptuous as vol
 
 from homeassistant.const import (
     CONF_DEVICE, CONF_IP_ADDRESS, CONF_PORT, EVENT_HOMEASSISTANT_STOP)
 import homeassistant.helpers.config_validation as cv
+from homeassistant.helpers.entity import Entity
 
 REQUIREMENTS = ['pyads==3.0.7']
 
@@ -31,6 +34,9 @@ CONF_ADS_VALUE = 'value'
 CONF_ADS_VAR = 'adsvar'
 CONF_ADS_VAR_BRIGHTNESS = 'adsvar_brightness'
 
+STATE_KEY_STATE = 'state'
+STATE_KEY_BRIGHTNESS = 'brightness'
+
 DOMAIN = 'ads'
 
 SERVICE_WRITE_DATA_BY_NAME = 'write_data_by_name'
@@ -210,3 +216,68 @@ class AdsHub:
             _LOGGER.warning("No callback available for this datatype")
 
         notification_item.callback(notification_item.name, value)
+
+
+class AdsEntity(Entity):
+    """Representation of ADS entity."""
+
+    def __init__(self, ads_hub, name, ads_var):
+        """Initialize ADS binary sensor."""
+        self._name = name
+        self._unique_id = ads_var
+        self._state_dict = {}
+        self._state_dict[STATE_KEY_STATE] = None
+        self._ads_hub = ads_hub
+        self._ads_var = ads_var
+        self._event = None
+
+    async def async_initialize_device(
+            self, ads_var, plctype, state_key=STATE_KEY_STATE, factor=None):
+        """Register device notification."""
+        def update(name, value):
+            """Handle device notifications."""
+            _LOGGER.debug('Variable %s changed its value to %d', name, value)
+
+            if factor is None:
+                self._state_dict[state_key] = value
+            else:
+                self._state_dict[state_key] = value / factor
+
+            asyncio.run_coroutine_threadsafe(async_event_set(), self.hass.loop)
+            self.schedule_update_ha_state()
+
+        async def async_event_set():
+            """Set event in async context."""
+            self._event.set()
+
+        self._event = asyncio.Event()
+
+        await self.hass.async_add_executor_job(
+            self._ads_hub.add_device_notification,
+            ads_var, plctype, update)
+        try:
+            with async_timeout.timeout(10):
+                await self._event.wait()
+        except asyncio.TimeoutError:
+            _LOGGER.debug('Variable %s: Timeout during first update',
+                          ads_var)
+
+    @property
+    def name(self):
+        """Return the default name of the binary sensor."""
+        return self._name
+
+    @property
+    def unique_id(self):
+        """Return an unique identifier for this entity."""
+        return self._unique_id
+
+    @property
+    def should_poll(self):
+        """Return False because entity pushes its state to HA."""
+        return False
+
+    @property
+    def available(self):
+        """Return False if state has not been updated yet."""
+        return self._state_dict[STATE_KEY_STATE] is not None

homeassistant/components/ads/binary_sensor.py

@@ -1,7 +1,5 @@
 """Support for ADS binary sensors."""
 import logging
-import asyncio
-import async_timeout
 
 import voluptuous as vol
 
@@ -10,7 +8,7 @@ from homeassistant.components.binary_sensor import (
 from homeassistant.const import CONF_DEVICE_CLASS, CONF_NAME
 import homeassistant.helpers.config_validation as cv
 
-from . import CONF_ADS_VAR, DATA_ADS
+from . import CONF_ADS_VAR, DATA_ADS, AdsEntity, STATE_KEY_STATE
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -36,70 +34,25 @@ def setup_platform(hass, config, add_entities, discovery_info=None):
     add_entities([ads_sensor])
 
 
-class AdsBinarySensor(BinarySensorDevice):
+class AdsBinarySensor(AdsEntity, BinarySensorDevice):
     """Representation of ADS binary sensors."""
 
     def __init__(self, ads_hub, name, ads_var, device_class):
         """Initialize ADS binary sensor."""
-        self._name = name
+        super().__init__(ads_hub, name, ads_var)
-        self._unique_id = ads_var
-        self._state = None
         self._device_class = device_class or 'moving'
-        self._ads_hub = ads_hub
-        self.ads_var = ads_var
-        self._event = None
 
     async def async_added_to_hass(self):
         """Register device notification."""
-        def update(name, value):
+        await self.async_initialize_device(self._ads_var,
-            """Handle device notifications."""
+                                           self._ads_hub.PLCTYPE_BOOL)
-            _LOGGER.debug('Variable %s changed its value to %d', name, value)
-            self._state = value
-            asyncio.run_coroutine_threadsafe(async_event_set(), self.hass.loop)
-            self.schedule_update_ha_state()
-
-        async def async_event_set():
-            """Set event in async context."""
-            self._event.set()
-
-        self._event = asyncio.Event()
-
-        await self.hass.async_add_executor_job(
-            self._ads_hub.add_device_notification,
-            self.ads_var, self._ads_hub.PLCTYPE_BOOL, update)
-        try:
-            with async_timeout.timeout(10):
-                await self._event.wait()
-        except asyncio.TimeoutError:
-            _LOGGER.debug('Variable %s: Timeout during first update',
-                          self.ads_var)
 
     @property
-    def name(self):
+    def is_on(self):
-        """Return the default name of the binary sensor."""
+        """Return True if the entity is on."""
-        return self._name
+        return self._state_dict[STATE_KEY_STATE]
-
-    @property
-    def unique_id(self):
-        """Return an unique identifier for this entity."""
-        return self._unique_id
 
     @property
     def device_class(self):
         """Return the device class."""
         return self._device_class
-
-    @property
-    def is_on(self):
-        """Return if the binary sensor is on."""
-        return self._state
-
-    @property
-    def should_poll(self):
-        """Return False because entity pushes its state to HA."""
-        return False
-
-    @property
-    def available(self):
-        """Return False if state has not been updated yet."""
-        return self._state is not None

homeassistant/components/ads/light.py

@@ -1,7 +1,5 @@
 """Support for ADS light sources."""
 import logging
-import asyncio
-import async_timeout
 
 import voluptuous as vol
 
@@ -10,12 +8,12 @@ from homeassistant.components.light import (
 from homeassistant.const import CONF_NAME
 import homeassistant.helpers.config_validation as cv
 
-from . import CONF_ADS_VAR, CONF_ADS_VAR_BRIGHTNESS, DATA_ADS
+from . import CONF_ADS_VAR, CONF_ADS_VAR_BRIGHTNESS, DATA_ADS, \
+    AdsEntity, STATE_KEY_BRIGHTNESS, STATE_KEY_STATE
 
 _LOGGER = logging.getLogger(__name__)
 DEPENDENCIES = ['ads']
 DEFAULT_NAME = 'ADS Light'
-CONF_ADSVAR_BRIGHTNESS = 'adsvar_brightness'
 PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
     vol.Required(CONF_ADS_VAR): cv.string,
     vol.Optional(CONF_ADS_VAR_BRIGHTNESS): cv.string,
@@ -35,107 +33,54 @@ def setup_platform(hass, config, add_entities, discovery_info=None):
                            name)])
 
 
-class AdsLight(Light):
+class AdsLight(AdsEntity, Light):
     """Representation of ADS light."""
 
     def __init__(self, ads_hub, ads_var_enable, ads_var_brightness, name):
         """Initialize AdsLight entity."""
-        self._ads_hub = ads_hub
+        super().__init__(ads_hub, name, ads_var_enable)
-        self._on_state = None
+        self._state_dict[STATE_KEY_BRIGHTNESS] = None
-        self._brightness = None
+        self._ads_var_brightness = ads_var_brightness
-        self._name = name
-        self._unique_id = ads_var_enable
-        self.ads_var_enable = ads_var_enable
-        self.ads_var_brightness = ads_var_brightness
-        self._event = None
 
     async def async_added_to_hass(self):
         """Register device notification."""
-        def update_on_state(name, value):
+        await self.async_initialize_device(self._ads_var,
-            """Handle device notifications for state."""
+                                           self._ads_hub.PLCTYPE_BOOL)
-            _LOGGER.debug('Variable %s changed its value to %d', name, value)
-            self._on_state = value
-            asyncio.run_coroutine_threadsafe(async_event_set(), self.hass.loop)
-            self.schedule_update_ha_state()
 
-        async def async_event_set():
+        if self._ads_var_brightness is not None:
-            """Set event in async context."""
+            await self.async_initialize_device(self._ads_var_brightness,
-            self._event.set()
+                                               self._ads_hub.PLCTYPE_UINT,
-
+                                               STATE_KEY_BRIGHTNESS)
-        def update_brightness(name, value):
-            """Handle device notification for brightness."""
-            _LOGGER.debug('Variable %s changed its value to %d', name, value)
-            self._brightness = value
-            self.schedule_update_ha_state()
-
-        self._event = asyncio.Event()
-
-        await self.hass.async_add_executor_job(
-            self._ads_hub.add_device_notification,
-            self.ads_var_enable, self._ads_hub.PLCTYPE_BOOL, update_on_state
-        )
-        if self.ads_var_brightness is not None:
-            await self.hass.async_add_executor_job(
-                self._ads_hub.add_device_notification,
-                self.ads_var_brightness, self._ads_hub.PLCTYPE_INT,
-                update_brightness
-            )
-        try:
-            with async_timeout.timeout(10):
-                await self._event.wait()
-        except asyncio.TimeoutError:
-            _LOGGER.debug('Variable %s: Timeout during first update',
-                          self.ads_var_enable)
-
-    @property
-    def name(self):
-        """Return the name of the device if any."""
-        return self._name
-
-    @property
-    def unique_id(self):
-        """Return an unique identifier for this entity."""
-        return self._unique_id
 
     @property
     def brightness(self):
         """Return the brightness of the light (0..255)."""
-        return self._brightness
+        return self._state_dict[STATE_KEY_BRIGHTNESS]
-
-    @property
-    def is_on(self):
-        """Return if light is on."""
-        return self._on_state
-
-    @property
-    def should_poll(self):
-        """Return False because entity pushes its state to HA."""
-        return False
 
     @property
     def supported_features(self):
         """Flag supported features."""
         support = 0
-        if self.ads_var_brightness is not None:
+        if self._ads_var_brightness is not None:
             support = SUPPORT_BRIGHTNESS
         return support
 
     @property
-    def available(self):
+    def is_on(self):
-        """Return False if state has not been updated yet."""
+        """Return True if the entity is on."""
-        return self._on_state is not None
+        return self._state_dict[STATE_KEY_STATE]
 
     def turn_on(self, **kwargs):
         """Turn the light on or set a specific dimmer value."""
         brightness = kwargs.get(ATTR_BRIGHTNESS)
-        self._ads_hub.write_by_name(self.ads_var_enable, True,
+        self._ads_hub.write_by_name(self._ads_var, True,
                                     self._ads_hub.PLCTYPE_BOOL)
 
-        if self.ads_var_brightness is not None and brightness is not None:
+        if self._ads_var_brightness is not None and brightness is not None:
-            self._ads_hub.write_by_name(self.ads_var_brightness, brightness,
+            self._ads_hub.write_by_name(self._ads_var_brightness, brightness,
                                         self._ads_hub.PLCTYPE_UINT)
 
     def turn_off(self, **kwargs):
         """Turn the light off."""
-        self._ads_hub.write_by_name(self.ads_var_enable, False,
+        self._ads_hub.write_by_name(self._ads_var, False,
                                     self._ads_hub.PLCTYPE_BOOL)

homeassistant/components/ads/sensor.py

@@ -7,9 +7,9 @@ from homeassistant.components import ads
 from homeassistant.components.sensor import PLATFORM_SCHEMA
 from homeassistant.const import CONF_NAME, CONF_UNIT_OF_MEASUREMENT
 import homeassistant.helpers.config_validation as cv
-from homeassistant.helpers.entity import Entity
 
-from . import CONF_ADS_FACTOR, CONF_ADS_TYPE, CONF_ADS_VAR
+from . import CONF_ADS_FACTOR, CONF_ADS_TYPE, CONF_ADS_VAR, \
+    AdsEntity, STATE_KEY_STATE
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -43,60 +43,31 @@ def setup_platform(hass, config, add_entities, discovery_info=None):
     add_entities([entity])
 
 
-class AdsSensor(Entity):
+class AdsSensor(AdsEntity):
     """Representation of an ADS sensor entity."""
 
     def __init__(self, ads_hub, ads_var, ads_type, name, unit_of_measurement,
                  factor):
         """Initialize AdsSensor entity."""
-        self._ads_hub = ads_hub
+        super().__init__(ads_hub, name, ads_var)
-        self._name = name
-        self._unique_id = ads_var
-        self._value = None
         self._unit_of_measurement = unit_of_measurement
-        self.ads_var = ads_var
+        self._ads_type = ads_type
-        self.ads_type = ads_type
+        self._factor = factor
-        self.factor = factor
 
     async def async_added_to_hass(self):
         """Register device notification."""
-        def update(name, value):
+        await self.async_initialize_device(
-            """Handle device notifications."""
+            self._ads_var,
-            _LOGGER.debug("Variable %s changed its value to %d", name, value)
+            self._ads_hub.ADS_TYPEMAP[self._ads_type],
-
+            STATE_KEY_STATE,
-            # If factor is set use it otherwise not
+            self._factor)
-            if self.factor is None:
-                self._value = value
-            else:
-                self._value = value / self.factor
-            self.schedule_update_ha_state()
-
-        self.hass.async_add_job(
-            self._ads_hub.add_device_notification,
-            self.ads_var, self._ads_hub.ADS_TYPEMAP[self.ads_type], update
-        )
-
-    @property
-    def name(self):
-        """Return the name of the entity."""
-        return self._name
-
-    @property
-    def unique_id(self):
-        """Return an unique identifier for this entity."""
-        return self._unique_id
 
     @property
     def state(self):
         """Return the state of the device."""
-        return self._value
+        return self._state_dict[STATE_KEY_STATE]
 
     @property
     def unit_of_measurement(self):
         """Return the unit of measurement."""
         return self._unit_of_measurement
-
-    @property
-    def should_poll(self):
-        """Return False because entity pushes its state."""
-        return False

homeassistant/components/ads/switch.py

@@ -1,16 +1,13 @@
 """Support for ADS switch platform."""
 import logging
-import asyncio
-import async_timeout
 
 import voluptuous as vol
 
-from homeassistant.components.switch import PLATFORM_SCHEMA
+from homeassistant.components.switch import SwitchDevice, PLATFORM_SCHEMA
 from homeassistant.const import CONF_NAME
 import homeassistant.helpers.config_validation as cv
-from homeassistant.helpers.entity import ToggleEntity
 
-from . import CONF_ADS_VAR, DATA_ADS
+from . import CONF_ADS_VAR, DATA_ADS, AdsEntity, STATE_KEY_STATE
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -34,74 +31,25 @@ def setup_platform(hass, config, add_entities, discovery_info=None):
     add_entities([AdsSwitch(ads_hub, name, ads_var)])
 
 
-class AdsSwitch(ToggleEntity):
+class AdsSwitch(AdsEntity, SwitchDevice):
     """Representation of an ADS switch device."""
 
-    def __init__(self, ads_hub, name, ads_var):
-        """Initialize the AdsSwitch entity."""
-        self._ads_hub = ads_hub
-        self._on_state = None
-        self._name = name
-        self._unique_id = ads_var
-        self.ads_var = ads_var
-        self._event = None
-
     async def async_added_to_hass(self):
         """Register device notification."""
-        def update(name, value):
+        await self.async_initialize_device(self._ads_var,
-            """Handle device notification."""
+                                           self._ads_hub.PLCTYPE_BOOL)
-            _LOGGER.debug("Variable %s changed its value to %d", name, value)
-            self._on_state = value
-            asyncio.run_coroutine_threadsafe(async_event_set(), self.hass.loop)
-            self.schedule_update_ha_state()
-
-        async def async_event_set():
-            """Set event in async context."""
-            self._event.set()
-
-        self._event = asyncio.Event()
-
-        await self.hass.async_add_executor_job(
-            self._ads_hub.add_device_notification,
-            self.ads_var, self._ads_hub.PLCTYPE_BOOL, update)
-        try:
-            with async_timeout.timeout(10):
-                await self._event.wait()
-        except asyncio.TimeoutError:
-            _LOGGER.debug('Variable %s: Timeout during first update',
-                          self.ads_var)
 
     @property
     def is_on(self):
-        """Return if the switch is turned on."""
+        """Return True if the entity is on."""
-        return self._on_state
+        return self._state_dict[STATE_KEY_STATE]
-
-    @property
-    def name(self):
-        """Return the name of the entity."""
-        return self._name
-
-    @property
-    def unique_id(self):
-        """Return an unique identifier for this entity."""
-        return self._unique_id
-
-    @property
-    def should_poll(self):
-        """Return False because entity pushes its state to HA."""
-        return False
-
-    @property
-    def available(self):
-        """Return False if state has not been updated yet."""
-        return self._on_state is not None
 
     def turn_on(self, **kwargs):
         """Turn the switch on."""
         self._ads_hub.write_by_name(
-            self.ads_var, True, self._ads_hub.PLCTYPE_BOOL)
+            self._ads_var, True, self._ads_hub.PLCTYPE_BOOL)
 
     def turn_off(self, **kwargs):
         """Turn the switch off."""
         self._ads_hub.write_by_name(
-            self.ads_var, False, self._ads_hub.PLCTYPE_BOOL)
+            self._ads_var, False, self._ads_hub.PLCTYPE_BOOL)