Repository: XKNX/xknx
Pull request #563 (2021-01-15 19:03:47)

HA integration: Dynamic expose
<!--
  You are awesome! Thanks for contributing to our project!
  Please, DO NOT DELETE ANY TEXT from this template!.
-->
## Description
<!--
Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context.
List any dependencies that are required for this change.
-->
- HA integration: add `knx.exposure_register` service allowing to add and remove ExposeSensor at runtime
- HA integration: move Expose classes to own module and create exposures after xknx.start() was called
- XKNX: add Device.shutdown
  - delete its callbacks
  - remove RemoteValues from StateUpdater
  - remove from Devices
- mitigate warnings in tests when calling `__del__` when the loop is already closed

the intention behind this is the same as in #535

## Type of change
<!--
Please tick the applicable options.
NOTE: Ticking multiple options most likely indicates
that your change is to big and it is suggested to split it into several smaller PRs.
-->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] This change requires a documentation update

## Checklist:

- [x] I have performed a self-review of my own code
- [ ] The documentation has been adjusted accordingly
- [ ] The changes generate no new warnings
- [ ] Tests have been added that prove the fix is effective or that the feature works
- [x] The changes are documented in the changelog
- [x] The Homeassistant plugin has been adjusted in case of new config options


coveralls: 
[![Coverage Status](https://coveralls.io/builds/36363139/badge)](https://coveralls.io/builds/36363139)

Coverage decreased (-0.01%) to 94.851% when pulling **6849c3ca8f8620f7ef170d486ceef08193c1d409 on farmio:dynamic-expose** into **cc40d0c695846d5e87c83286ea6305da7c3284fb on XKNX:master**.


Patch:
diff --git a/changelog.md b/changelog.md
index 65c96329..42415d90 100644
--- a/changelog.md
+++ b/changelog.md
@@ -2,9 +2,14 @@
 
 ## Unreleased changes
 
+### HA integration
+
+- added `knx.exposure_register` service allowing to add and remove ExposeSensor at runtime
+
 ### Internals
 
 - remove DPTComparator: DPTBinary and DPTArray are not equal, even if their .value is, and are never equal to `None`.
+- add Device.shutdown() method (used eg. when removing ExposeSensor)
 
 ## 0.16.0 APCI possibilities considerably increased 2021-01-01
 
diff --git a/home-assistant-plugin/custom_components/xknx/__init__.py b/home-assistant-plugin/custom_components/xknx/__init__.py
index 308dcc8c..fab7ec68 100644
--- a/home-assistant-plugin/custom_components/xknx/__init__.py
+++ b/home-assistant-plugin/custom_components/xknx/__init__.py
@@ -5,7 +5,6 @@ import logging
 import voluptuous as vol
 from xknx import XKNX
 from xknx.core.telegram_queue import TelegramQueue
-from xknx.devices import DateTime, ExposeSensor
 from xknx.dpt import DPTArray, DPTBase, DPTBinary
 from xknx.exceptions import XKNXException
 from xknx.io import (
@@ -18,26 +17,20 @@ from xknx.telegram import AddressFilter, GroupAddress, Telegram
 from xknx.telegram.apci import GroupValueResponse, GroupValueWrite
 
 from homeassistant.const import (
-    CONF_ENTITY_ID,
     CONF_HOST,
     CONF_PORT,
     EVENT_HOMEASSISTANT_STOP,
     SERVICE_RELOAD,
-    STATE_OFF,
-    STATE_ON,
-    STATE_UNAVAILABLE,
-    STATE_UNKNOWN,
 )
-from homeassistant.core import callback
 from homeassistant.helpers import discovery
 import homeassistant.helpers.config_validation as cv
 from homeassistant.helpers.entity_platform import async_get_platforms
-from homeassistant.helpers.event import async_track_state_change_event
 from homeassistant.helpers.reload import async_integration_yaml_config
 from homeassistant.helpers.service import async_register_admin_service
 from homeassistant.helpers.typing import ServiceCallType
 
 from .const import DOMAIN, SupportedPlatforms
+from .expose import create_knx_exposure
 from .factory import create_knx_device
 from .schema import (
     BinarySensorSchema,
@@ -74,6 +67,7 @@ SERVICE_XKNX_ATTR_PAYLOAD = "payload"
 SERVICE_XKNX_ATTR_TYPE = "type"
 SERVICE_XKNX_ATTR_REMOVE = "remove"
 SERVICE_XKNX_EVENT_REGISTER = "event_register"
+SERVICE_XKNX_EXPOSURE_REGISTER = "exposure_register"
 
 CONFIG_SCHEMA = vol.Schema(
     {
@@ -169,12 +163,27 @@ SERVICE_XKNX_EVENT_REGISTER_SCHEMA = vol.Schema(
     }
 )
 
+SERVICE_XKNX_EXPOSURE_REGISTER_SCHEMA = vol.Any(
+    ExposeSchema.SCHEMA.extend(
+        {
+            vol.Optional(SERVICE_XKNX_ATTR_REMOVE, default=False): cv.boolean,
+        }
+    ),
+    vol.Schema(
+        # for removing only `address` is required
+        {
+            vol.Required(SERVICE_XKNX_ATTR_ADDRESS): cv.string,
+            vol.Required(SERVICE_XKNX_ATTR_REMOVE): vol.All(cv.boolean, True),
+        },
+        extra=vol.ALLOW_EXTRA,
+    ),
+)
+
 
 async def async_setup(hass, config):
     """Set up the KNX component."""
     try:
         hass.data[DOMAIN] = KNXModule(hass, config)
-        hass.data[DOMAIN].async_create_exposures()
         await hass.data[DOMAIN].start()
     except XKNXException as ex:
         _LOGGER.warning("Could not connect to KNX interface: %s", ex)
@@ -182,6 +191,12 @@ async def async_setup(hass, config):
             f"Could not connect to KNX interface: <br><b>{ex}</b>", title="KNX"
         )
 
+    if CONF_XKNX_EXPOSE in config[DOMAIN]:
+        for expose_config in config[DOMAIN][CONF_XKNX_EXPOSE]:
+            hass.data[DOMAIN].exposures.append(
+                create_knx_exposure(hass, hass.data[DOMAIN].xknx, expose_config)
+            )
+
     for platform in SupportedPlatforms:
         if platform.value in config[DOMAIN]:
             for device_config in config[DOMAIN][platform.value]:
@@ -214,6 +229,14 @@ async def async_setup(hass, config):
         schema=SERVICE_XKNX_EVENT_REGISTER_SCHEMA,
     )
 
+    async_register_admin_service(
+        hass,
+        DOMAIN,
+        SERVICE_XKNX_EXPOSURE_REGISTER,
+        hass.data[DOMAIN].service_exposure_register_modify,
+        schema=SERVICE_XKNX_EXPOSURE_REGISTER_SCHEMA,
+    )
+
     async def reload_service_handler(service_call: ServiceCallType) -> None:
         """Remove all KNX components and load new ones from config."""
 
@@ -248,6 +271,7 @@ class KNXModule:
         self.config = config
         self.connected = False
         self.exposures = []
+        self.service_exposures = {}
 
         self.init_xknx()
         self._knx_event_callback: TelegramQueue.Callback = self.register_callback()
@@ -316,34 +340,6 @@ class KNXModule:
             auto_reconnect=True,
         )
 
-    @callback
-    def async_create_exposures(self):
-        """Create exposures."""
-        if CONF_XKNX_EXPOSE not in self.config[DOMAIN]:
-            return
-        for to_expose in self.config[DOMAIN][CONF_XKNX_EXPOSE]:
-            expose_type = to_expose.get(ExposeSchema.CONF_XKNX_EXPOSE_TYPE)
-            entity_id = to_expose.get(CONF_ENTITY_ID)
-            attribute = to_expose.get(ExposeSchema.CONF_XKNX_EXPOSE_ATTRIBUTE)
-            default = to_expose.get(ExposeSchema.CONF_XKNX_EXPOSE_DEFAULT)
-            address = to_expose.get(ExposeSchema.CONF_XKNX_EXPOSE_ADDRESS)
-            if expose_type.lower() in ["time", "date", "datetime"]:
-                exposure = KNXExposeTime(self.xknx, expose_type, address)
-                exposure.async_register()
-                self.exposures.append(exposure)
-            else:
-                exposure = KNXExposeSensor(
-                    self.hass,
-                    self.xknx,
-                    expose_type,
-                    entity_id,
-                    attribute,
-                    default,
-                    address,
-                )
-                exposure.async_register()
-                self.exposures.append(exposure)
-
     async def telegram_received_cb(self, telegram):
         """Call invoked after a KNX telegram was received."""
         data = None
@@ -378,9 +374,51 @@ class KNXModule:
         """Service for adding or removing a GroupAddress to the knx_event filter."""
         group_address = GroupAddress(call.data.get(SERVICE_XKNX_ATTR_ADDRESS))
         if call.data.get(SERVICE_XKNX_ATTR_REMOVE):
-            self._knx_event_callback.group_addresses.remove(group_address)
+            try:
+                self._knx_event_callback.group_addresses.remove(group_address)
+            except ValueError:
+                _LOGGER.warning(
+                    "Service event_register could not remove event for '%s'",
+                    group_address,
+                )
         elif group_address not in self._knx_event_callback.group_addresses:
             self._knx_event_callback.group_addresses.append(group_address)
+            _LOGGER.debug(
+                "Service event_register registered event for '%s'",
+                group_address,
+            )
+
+    async def service_exposure_register_modify(self, call):
+        """Service for adding or removing an exposure to KNX bus."""
+        group_address = call.data.get(SERVICE_XKNX_ATTR_ADDRESS)
+
+        if call.data.get(SERVICE_XKNX_ATTR_REMOVE):
+            try:
+                removed_exposure = self.service_exposures.pop(group_address)
+            except KeyError:
+                _LOGGER.warning(
+                    "Service exposure_register could not remove exposure for '%s'",
+                    group_address,
+                )
+            else:
+                removed_exposure.shutdown()
+            return
+
+        if group_address in self.service_exposures:
+            replaced_exposure = self.service_exposures.pop(group_address)
+            _LOGGER.warning(
+                "Service exposure_register replacing already registered exposure for '%s' - %s",
+                group_address,
+                replaced_exposure.device.name,
+            )
+            replaced_exposure.shutdown()
+        exposure = create_knx_exposure(self.hass, self.xknx, call.data)
+        self.service_exposures[group_address] = exposure
+        _LOGGER.debug(
+            "Service exposure_register registered exposure for '%s' - %s",
+            group_address,
+            exposure.device.name,
+        )
 
     async def service_send_to_knx_bus(self, call):
         """Service for sending an arbitrary KNX message to the KNX bus."""
@@ -404,93 +442,3 @@ class KNXModule:
             payload=GroupValueWrite(calculate_payload(attr_payload)),
         )
         await self.xknx.telegrams.put(telegram)
-
-
-class KNXExposeTime:
-    """Object to Expose Time/Date object to KNX bus."""
-
-    def __init__(self, xknx: XKNX, expose_type: str, address: str):
-        """Initialize of Expose class."""
-        self.xknx = xknx
-        self.expose_type = expose_type
-        self.address = address
-        self.device = None
-
-    @callback
-    def async_register(self):
-        """Register listener."""
-        self.device = DateTime(
-            self.xknx,
-            name=self.expose_type.capitalize(),
-            broadcast_type=self.expose_type.upper(),
-            localtime=True,
-            group_address=self.address,
-        )
-
-
-class KNXExposeSensor:
-    """Object to Expose Home Assistant entity to KNX bus."""
-
-    def __init__(self, hass, xknx, expose_type, entity_id, attribute, default, address):
-        """Initialize of Expose class."""
-        self.hass = hass
-        self.xknx = xknx
-        self.type = expose_type
-        self.entity_id = entity_id
-        self.expose_attribute = attribute
-        self.expose_default = default
-        self.address = address
-        self.device = None
-
-    @callback
-    def async_register(self):
-        """Register listener."""
-        if self.expose_attribute is not None:
-            _name = self.entity_id + "__" + self.expose_attribute
-        else:
-            _name = self.entity_id
-        self.device = ExposeSensor(
-            self.xknx,
-            name=_name,
-            group_address=self.address,
-            value_type=self.type,
-        )
-        async_track_state_change_event(
-            self.hass, [self.entity_id], self._async_entity_changed
-        )
-
-    async def _async_entity_changed(self, event):
-        """Handle entity change."""
-        new_state = event.data.get("new_state")
-        if new_state is None:
-            return
-        if new_state.state in (STATE_UNKNOWN, STATE_UNAVAILABLE):
-            return
-
-        if self.expose_attribute is not None:
-            new_attribute = new_state.attributes.get(self.expose_attribute)
-            old_state = event.data.get("old_state")
-
-            if old_state is not None:
-                old_attribute = old_state.attributes.get(self.expose_attribute)
-                if old_attribute == new_attribute:
-                    # don't send same value sequentially
-                    return
-            await self._async_set_knx_value(new_attribute)
-        else:
-            await self._async_set_knx_value(new_state.state)
-
-    async def _async_set_knx_value(self, value):
-        """Set new value on xknx ExposeSensor."""
-        if value is None:
-            if self.expose_default is None:
-                return
-            value = self.expose_default
-
-        if self.type == "binary":
-            if value == STATE_ON:
-                value = True
-            elif value == STATE_OFF:
-                value = False
-
-        await self.device.set(value)
diff --git a/home-assistant-plugin/custom_components/xknx/expose.py b/home-assistant-plugin/custom_components/xknx/expose.py
new file mode 100644
index 00000000..14c7997d
--- /dev/null
+++ b/home-assistant-plugin/custom_components/xknx/expose.py
@@ -0,0 +1,147 @@
+"""Exposures to KNX bus."""
+from typing import Union
+
+from xknx import XKNX
+from xknx.devices import DateTime, ExposeSensor
+
+from homeassistant.const import (
+    CONF_ENTITY_ID,
+    STATE_OFF,
+    STATE_ON,
+    STATE_UNAVAILABLE,
+    STATE_UNKNOWN,
+)
+from homeassistant.core import HomeAssistant, callback
+from homeassistant.helpers.event import async_track_state_change_event
+from homeassistant.helpers.typing import ConfigType
+
+from .schema import ExposeSchema
+
+
+def create_knx_exposure(
+    hass: HomeAssistant, xknx: XKNX, config: ConfigType
+) -> Union["KNXExposeSensor", "KNXExposeTime"]:
+    """Create exposures from config."""
+    expose_type = config.get(ExposeSchema.CONF_XKNX_EXPOSE_TYPE)
+    entity_id = config.get(CONF_ENTITY_ID)
+    attribute = config.get(ExposeSchema.CONF_XKNX_EXPOSE_ATTRIBUTE)
+    default = config.get(ExposeSchema.CONF_XKNX_EXPOSE_DEFAULT)
+    address = config.get(ExposeSchema.CONF_XKNX_EXPOSE_ADDRESS)
+    if expose_type.lower() in ["time", "date", "datetime"]:
+        exposure = KNXExposeTime(xknx, expose_type, address)
+        exposure.async_register()
+    else:
+        exposure = KNXExposeSensor(
+            hass,
+            xknx,
+            expose_type,
+            entity_id,
+            attribute,
+            default,
+            address,
+        )
+        exposure.async_register()
+    return exposure
+
+
+class KNXExposeSensor:
+    """Object to Expose Home Assistant entity to KNX bus."""
+
+    def __init__(self, hass, xknx, expose_type, entity_id, attribute, default, address):
+        """Initialize of Expose class."""
+        self.hass = hass
+        self.xknx = xknx
+        self.type = expose_type
+        self.entity_id = entity_id
+        self.expose_attribute = attribute
+        self.expose_default = default
+        self.address = address
+        self.device = None
+        self._remove_listener = None
+
+    @callback
+    def async_register(self):
+        """Register listener."""
+        if self.expose_attribute is not None:
+            _name = self.entity_id + "__" + self.expose_attribute
+        else:
+            _name = self.entity_id
+        self.device = ExposeSensor(
+            self.xknx,
+            name=_name,
+            group_address=self.address,
+            value_type=self.type,
+        )
+        self._remove_listener = async_track_state_change_event(
+            self.hass, [self.entity_id], self._async_entity_changed
+        )
+
+    def shutdown(self) -> None:
+        """Prepare for deletion."""
+        if self._remove_listener is not None:
+            self._remove_listener()
+        if self.device is not None:
+            self.device.shutdown()
+
+    async def _async_entity_changed(self, event):
+        """Handle entity change."""
+        new_state = event.data.get("new_state")
+        if new_state is None:
+            return
+        if new_state.state in (STATE_UNKNOWN, STATE_UNAVAILABLE):
+            return
+
+        if self.expose_attribute is not None:
+            new_attribute = new_state.attributes.get(self.expose_attribute)
+            old_state = event.data.get("old_state")
+
+            if old_state is not None:
+                old_attribute = old_state.attributes.get(self.expose_attribute)
+                if old_attribute == new_attribute:
+                    # don't send same value sequentially
+                    return
+            await self._async_set_knx_value(new_attribute)
+        else:
+            await self._async_set_knx_value(new_state.state)
+
+    async def _async_set_knx_value(self, value):
+        """Set new value on xknx ExposeSensor."""
+        if value is None:
+            if self.expose_default is None:
+                return
+            value = self.expose_default
+
+        if self.type == "binary":
+            if value == STATE_ON:
+                value = True
+            elif value == STATE_OFF:
+                value = False
+
+        await self.device.set(value)
+
+
+class KNXExposeTime:
+    """Object to Expose Time/Date object to KNX bus."""
+
+    def __init__(self, xknx: XKNX, expose_type: str, address: str):
+        """Initialize of Expose class."""
+        self.xknx = xknx
+        self.expose_type = expose_type
+        self.address = address
+        self.device = None
+
+    @callback
+    def async_register(self):
+        """Register listener."""
+        self.device = DateTime(
+            self.xknx,
+            name=self.expose_type.capitalize(),
+            broadcast_type=self.expose_type.upper(),
+            localtime=True,
+            group_address=self.address,
+        )
+
+    def shutdown(self):
+        """Prepare for deletion."""
+        if self.device is not None:
+            self.device.shutdown()
diff --git a/home-assistant-plugin/custom_components/xknx/services.yaml b/home-assistant-plugin/custom_components/xknx/services.yaml
index cab8c100..142baca2 100644
--- a/home-assistant-plugin/custom_components/xknx/services.yaml
+++ b/home-assistant-plugin/custom_components/xknx/services.yaml
@@ -18,3 +18,25 @@ event_register:
       example: "1/1/0"
     remove:
       description: "Optional. If `True` the group address will be removed. Defaults to `False`."
+exposure_register:
+  description: "Add or remove exposures to KNX bus. Only exposures added with this service can be removed."
+  fields:
+    address:
+      description: "Required. Group address state or attribute updates will be sent to. GroupValueRead requests will be answered. Per address only one exposure can be registered."
+      example: "1/1/0"
+    type:
+      description: "Required. Telegrams will be encoded as given DPT. 'binary' and all Knx sensor types are valid values (see https://www.home-assistant.io/integrations/sensor.knx)"
+      example: "percentU8"
+    entity_id:
+      description: "Required. Entity id to be exposed."
+      example: "light.kitchen"
+      required: true
+    attribute:
+      description: "Optional. Attribute of the entity that shall be sent to the KNX bus. If not set the state will be sent. Eg. for a light the state is eigther “on” or “off” - with attribute you can expose its “brightness”."
+      example: "brightness"
+      required: false
+    default:
+      description: "Optional. Default value to send to the bus if the state or attribute value is None. Eg. a light with state “off” has no brightness attribute so a default value of 0 could be used. If not set (or None) no value would be sent to the bus and a GroupReadRequest to the address would return the last known value."
+      example: "0"
+    remove:
+      description: "Optional. If `True` the exposure will be removed. Only `address` is required for removal."
diff --git a/xknx/devices/binary_sensor.py b/xknx/devices/binary_sensor.py
index 80b28352..b9bfcc32 100644
--- a/xknx/devices/binary_sensor.py
+++ b/xknx/devices/binary_sensor.py
@@ -76,11 +76,14 @@ class BinarySensor(Device):
 
     def __del__(self) -> None:
         """Destructor. Cleaning up if this was not done before."""
-        if self._reset_task:
-            self._reset_task.cancel()
-
-        if self._context_task:
-            self._context_task.cancel()
+        try:
+            if self._reset_task:
+                self._reset_task.cancel()
+            if self._context_task:
+                self._context_task.cancel()
+        except RuntimeError:
+            pass
+        super().__del__()
 
     @classmethod
     def from_config(cls, xknx: "XKNX", name: str, config: Any) -> "BinarySensor":
diff --git a/xknx/devices/datetime.py b/xknx/devices/datetime.py
index dcf55246..6c2bc598 100644
--- a/xknx/devices/datetime.py
+++ b/xknx/devices/datetime.py
@@ -49,7 +49,11 @@ class DateTime(Device):
     def __del__(self) -> None:
         """Destructor. Cleaning up if this was not done before."""
         if self._broadcast_task:
-            self._broadcast_task.cancel()
+            try:
+                self._broadcast_task.cancel()
+            except RuntimeError:
+                pass
+        super().__del__()
 
     def _iter_remote_values(self) -> Iterator[RemoteValueDateTime]:
         """Iterate the devices RemoteValue classes."""
diff --git a/xknx/devices/device.py b/xknx/devices/device.py
index 4e68d8a9..ed09f333 100644
--- a/xknx/devices/device.py
+++ b/xknx/devices/device.py
@@ -37,12 +37,26 @@ class Device(ABC):
 
         self.xknx.devices.add(self)
 
+    def __del__(self) -> None:
+        """Remove Device form Devices."""
+        try:
+            self.shutdown()
+        except ValueError:
+            pass
+
+    def shutdown(self) -> None:
+        """Prepare for deletion. Remove callbacks and device form Devices vector."""
+        self.xknx.devices.remove(self)
+        self.device_updated_cbs = []
+        for remote_value in self._iter_remote_values():
+            remote_value.__del__()
+
     @abstractmethod
     def _iter_remote_values(self) -> Iterator[RemoteValue]:
         """Iterate the devices RemoteValue classes."""
         # yield self.remote_value
-        # or
         # yield from (<list all used RemoteValue instances>)
+        yield from ()
 
     def register_device_updated_cb(self, device_updated_cb: DeviceCallbackType) -> None:
         """Register device updated callback."""
diff --git a/xknx/devices/devices.py b/xknx/devices/devices.py
index 338fec33..5dae9e3f 100644
--- a/xknx/devices/devices.py
+++ b/xknx/devices/devices.py
@@ -67,6 +67,10 @@ class Devices:
         device.register_device_updated_cb(self.device_updated)
         self.__devices.append(device)
 
+    def remove(self, device: Device) -> None:
+        """Remove device from devices vector."""
+        self.__devices.remove(device)
+
     async def device_updated(self, device: Device) -> None:
         """Call all registered device updated callbacks of device."""
         for device_updated_cb in self.device_updated_cbs:
diff --git a/xknx/devices/switch.py b/xknx/devices/switch.py
index ca89b108..28eccf43 100644
--- a/xknx/devices/switch.py
+++ b/xknx/devices/switch.py
@@ -58,7 +58,11 @@ class Switch(Device):
     def __del__(self) -> None:
         """Destructor. Cleaning up if this was not done before."""
         if self._reset_task:
-            self._reset_task.cancel()
+            try:
+                self._reset_task.cancel()
+            except RuntimeError:
+                pass
+        super().__del__()
 
     @classmethod
     def from_config(cls, xknx: "XKNX", name: str, config: Any) -> "Switch":
diff --git a/xknx/remote_value/remote_value.py b/xknx/remote_value/remote_value.py
index 319f3615..73b1f75f 100644
--- a/xknx/remote_value/remote_value.py
+++ b/xknx/remote_value/remote_value.py
@@ -78,7 +78,9 @@ class RemoteValue(ABC):
         """Destructor. Removing self from StateUpdater if was registered."""
         try:
             self.xknx.state_updater.unregister_remote_value(self)
-        except KeyError:
+        except (KeyError, AttributeError):
+            # KeyError if it was never added to StateUpdater
+            # AttributeError if instantiation failed (tests mostly)
             pass
 
     @property


Test patch:
diff --git a/test/devices_tests/devices_test.py b/test/devices_tests/devices_test.py
index 89098ed2..f6dc49d0 100644
--- a/test/devices_tests/devices_test.py
+++ b/test/devices_tests/devices_test.py
@@ -142,6 +142,19 @@ class TestDevices(unittest.TestCase):
         self.assertTrue("Living-Room.Light_2" in xknx.devices)
         self.assertFalse("Living-Room.Light_3" in xknx.devices)
 
+    @patch.multiple(Device, __abstractmethods__=set())
+    def test_add_remove(self):
+        """Tesst add and remove functions."""
+        xknx = XKNX()
+        device1 = Device(xknx, "TestDevice1")
+        device2 = Device(xknx, "TestDevice2")
+        self.assertEqual(len(xknx.devices), 2)
+        device1.shutdown()
+        self.assertEqual(len(xknx.devices), 1)
+        self.assertFalse("TestDevice1" in xknx.devices)
+        device2.shutdown()
+        self.assertEqual(len(xknx.devices), 0)
+
     def test_modification_of_device(self):
         """Test if devices object does store references and not copies of objects."""
         xknx = XKNX()