Repository: XKNX/xknx
Pull request #546 (2020-12-29 22:53:32)

Typecheck devices
<!--
  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.
-->
Typecheck Devices and Device

Fixes # (issue)

## 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)
- [ ] 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:

- [ ] 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
- [ ] The changes are documented in the changelog
- [ ] The Homeassistant plugin has been adjusted in case of new config options


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

Coverage increased (+0.007%) to 94.45% when pulling **7bb19cabaf2b335c69c482c6b15789def9771cfa on farmio:typecheck-devices** into **1f3d51632104872cf1ba8cdd83ffae47af80cb46 on XKNX:master**.


Patch:
diff --git a/setup.cfg b/setup.cfg
index 81b6d598..9d3b91f7 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -51,7 +51,7 @@ warn_unreachable = true
 implicit_reexport = true
 
 # partly typechecked modules (extra block for better overview)
-[mypy-xknx.devices.travelcalculator,xknx.remote_value.remote_value]
+[mypy-xknx.devices.device,xknx.devices.devices,xknx.devices.travelcalculator,xknx.remote_value.remote_value]
 strict = true
 ignore_errors = false
 warn_unreachable = true
diff --git a/xknx/devices/device.py b/xknx/devices/device.py
index 72c44bb5..4e68d8a9 100644
--- a/xknx/devices/device.py
+++ b/xknx/devices/device.py
@@ -3,53 +3,69 @@ Device is the base class for all implemented devices (e.g. Lights/Switches/Senso
 
 It provides basis functionality for reading the state from the KNX bus.
 """
+from abc import ABC, abstractmethod
 import logging
+from typing import TYPE_CHECKING, Awaitable, Callable, Iterator, List, Optional
 
+from xknx.remote_value import RemoteValue
+from xknx.telegram import GroupAddress, Telegram
 from xknx.telegram.apci import GroupValueRead, GroupValueResponse, GroupValueWrite
 
+if TYPE_CHECKING:
+    from xknx.xknx import XKNX
+
+DeviceCallbackType = Callable[["Device"], Awaitable[None]]
+
 logger = logging.getLogger("xknx.log")
 
 
-class Device:
+class Device(ABC):
     """Base class for devices."""
 
-    def __init__(self, xknx, name: str, device_updated_cb=None):
+    def __init__(
+        self,
+        xknx: "XKNX",
+        name: str,
+        device_updated_cb: Optional[DeviceCallbackType] = None,
+    ):
         """Initialize Device class."""
         self.xknx = xknx
         self.name = name
-        self.device_updated_cbs = []
+        self.device_updated_cbs: List[DeviceCallbackType] = []
         if device_updated_cb is not None:
             self.register_device_updated_cb(device_updated_cb)
 
         self.xknx.devices.add(self)
 
-    def _iter_remote_values(self):
+    @abstractmethod
+    def _iter_remote_values(self) -> Iterator[RemoteValue]:
         """Iterate the devices RemoteValue classes."""
-        raise NotImplementedError("_iter_remote_values has to be implemented")
         # yield self.remote_value
         # or
         # yield from (<list all used RemoteValue instances>)
 
-    def register_device_updated_cb(self, device_updated_cb):
+    def register_device_updated_cb(self, device_updated_cb: DeviceCallbackType) -> None:
         """Register device updated callback."""
         self.device_updated_cbs.append(device_updated_cb)
 
-    def unregister_device_updated_cb(self, device_updated_cb):
+    def unregister_device_updated_cb(
+        self, device_updated_cb: DeviceCallbackType
+    ) -> None:
         """Unregister device updated callback."""
         self.device_updated_cbs.remove(device_updated_cb)
 
-    async def after_update(self):
+    async def after_update(self) -> None:
         """Execute callbacks after internal state has been changed."""
         for device_updated_cb in self.device_updated_cbs:
             # pylint: disable=not-callable
             await device_updated_cb(self)
 
-    async def sync(self, wait_for_result=False):
+    async def sync(self, wait_for_result: bool = False) -> None:
         """Read states of device from KNX bus."""
         for remote_value in self._iter_remote_values():
             await remote_value.read_state(wait_for_result=wait_for_result)
 
-    async def process(self, telegram):
+    async def process(self, telegram: Telegram) -> None:
         """Process incoming telegram."""
         if isinstance(telegram.payload, GroupValueWrite):
             await self.process_group_write(telegram)
@@ -58,31 +74,31 @@ class Device:
         elif isinstance(telegram.payload, GroupValueRead):
             await self.process_group_read(telegram)
 
-    async def process_group_read(self, telegram):
+    async def process_group_read(self, telegram: Telegram) -> None:
         """Process incoming GroupValueRead telegrams."""
         # The dafault is, that devices dont answer to group reads
 
-    async def process_group_response(self, telegram):
+    async def process_group_response(self, telegram: Telegram) -> None:
         """Process incoming GroupValueResponse telegrams."""
         # Per default mapped to group write.
         await self.process_group_write(telegram)
 
-    async def process_group_write(self, telegram):
+    async def process_group_write(self, telegram: Telegram) -> None:
         """Process incoming GroupValueWrite telegrams."""
         # The default is, that devices dont process group writes
 
-    def get_name(self):
+    def get_name(self) -> str:
         """Return name of device."""
         return self.name
 
-    def has_group_address(self, group_address):
+    def has_group_address(self, group_address: GroupAddress) -> bool:
         """Test if device has given group address."""
         for remote_value in self._iter_remote_values():
             if remote_value.has_group_address(group_address):
                 return True
         return False
 
-    async def do(self, action):
+    async def do(self, action: str) -> None:
         """Execute 'do' commands."""
         # pylint: disable=invalid-name
         logger.info(
@@ -91,6 +107,6 @@ class Device:
             self.__class__.__name__,
         )
 
-    def __eq__(self, other):
+    def __eq__(self, other: object) -> bool:
         """Compare for quality."""
         return self.__dict__ == other.__dict__
diff --git a/xknx/devices/devices.py b/xknx/devices/devices.py
index 0deab8e0..338fec33 100644
--- a/xknx/devices/devices.py
+++ b/xknx/devices/devices.py
@@ -3,42 +3,44 @@ Module for handling a vector/array of devices.
 
 More or less an array with devices. Adds some search functionality to find devices.
 """
-from typing import Awaitable, Callable
+from typing import Awaitable, Callable, Iterator, List, Union
 
-from xknx.telegram import Telegram
+from xknx.telegram import GroupAddress, Telegram
 
 from .device import Device
 
+DeviceCallbackType = Callable[[Device], Awaitable[None]]
+
 
 class Devices:
     """Class for handling a vector/array of devices."""
 
     def __init__(self) -> None:
         """Initialize Devices class."""
-        self.__devices = []
-        self.device_updated_cbs = []
+        self.__devices: List[Device] = []
+        self.device_updated_cbs: List[DeviceCallbackType] = []
 
-    def register_device_updated_cb(
-        self, device_updated_cb: Callable[[Device], Awaitable[None]]
-    ) -> None:
+    def register_device_updated_cb(self, device_updated_cb: DeviceCallbackType) -> None:
         """Register callback for devices beeing updated."""
         self.device_updated_cbs.append(device_updated_cb)
 
-    def unregister_device_updated_cb(self, device_updated_cb):
+    def unregister_device_updated_cb(
+        self, device_updated_cb: DeviceCallbackType
+    ) -> None:
         """Unregister callback for devices beeing updated."""
         self.device_updated_cbs.remove(device_updated_cb)
 
-    def __iter__(self):
+    def __iter__(self) -> Iterator[Device]:
         """Iterate registered devices."""
         yield from self.__devices
 
-    def devices_by_group_address(self, group_address):
+    def devices_by_group_address(self, group_address: GroupAddress) -> Iterator[Device]:
         """Return device(s) by group address."""
         for device in self.__devices:
             if device.has_group_address(group_address):
                 yield device
 
-    def __getitem__(self, key):
+    def __getitem__(self, key: Union[str, int]) -> Device:
         """Return device by name or by index."""
         for device in self.__devices:
             if device.name == key:
@@ -47,35 +49,36 @@ class Devices:
             return self.__devices[key]
         raise KeyError
 
-    def __len__(self):
+    def __len__(self) -> int:
         """Return number of devices within vector."""
         return len(self.__devices)
 
-    def __contains__(self, key):
+    def __contains__(self, key: str) -> bool:
         """Return if devices with name 'key' is within devices."""
         for device in self.__devices:
             if device.name == key:
                 return True
         return False
 
-    def add(self, device):
+    def add(self, device: Device) -> None:
         """Add device to devices vector."""
         if not isinstance(device, Device):
             raise TypeError()
         device.register_device_updated_cb(self.device_updated)
         self.__devices.append(device)
 
-    async def device_updated(self, 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:
             await device_updated_cb(device)
 
     async def process(self, telegram: Telegram) -> None:
         """Process telegram."""
-        for device in self.devices_by_group_address(telegram.destination_address):
-            await device.process(telegram)
+        if isinstance(telegram.destination_address, GroupAddress):
+            for device in self.devices_by_group_address(telegram.destination_address):
+                await device.process(telegram)
 
-    async def sync(self):
+    async def sync(self) -> None:
         """Read state of devices from KNX bus."""
         for device in self.__devices:
             await device.sync()
diff --git a/xknx/devices/weather.py b/xknx/devices/weather.py
index f6ee1a32..4e888e16 100644
--- a/xknx/devices/weather.py
+++ b/xknx/devices/weather.py
@@ -13,7 +13,7 @@ It provides functionality for
 """
 from datetime import date, datetime
 from enum import Enum
-from typing import Callable, Generator, Optional
+from typing import Callable, Iterator, Optional
 
 from xknx.remote_value import RemoteValue, RemoteValueSensor, RemoteValueSwitch
 
@@ -208,7 +208,7 @@ class Weather(Device):
         if expose_sensors:
             self.expose_sensors()
 
-    def _iter_remote_values(self) -> Generator[RemoteValue, None, None]:
+    def _iter_remote_values(self) -> Iterator[RemoteValue]:
         """Iterate the devices remote values."""
         yield from [
             self._temperature,
diff --git a/xknx/knxip/dib.py b/xknx/knxip/dib.py
index 48d4d346..e7ca2775 100644
--- a/xknx/knxip/dib.py
+++ b/xknx/knxip/dib.py
@@ -11,7 +11,7 @@ A KNX/IP Search Response may contain several DIBs of different types:
                         (fallback for unknown dib type codes)
 """
 from abc import ABC, abstractmethod
-from typing import Generator, List, Union
+from typing import Iterator, List, Union
 
 from xknx.exceptions import CouldNotParseKNXIP
 from xknx.telegram import IndividualAddress
@@ -233,17 +233,17 @@ class DIBDeviceInformation(DIB):
     def to_knx(self) -> List[int]:
         """Serialize to KNX/IP raw data."""
 
-        def hex_notation_to_knx(serial_number: str) -> Generator[int, None, None]:
+        def hex_notation_to_knx(serial_number: str) -> Iterator[int]:
             """Serialize hex notation."""
             for part in serial_number.split(":"):
                 yield int(part, 16)
 
-        def ip_to_knx(ip_addr: str) -> Generator[int, None, None]:
+        def ip_to_knx(ip_addr: str) -> Iterator[int]:
             """Serialize ip."""
             for part in ip_addr.split("."):
                 yield int(part)
 
-        def str_to_knx(string: str, length: int) -> Generator[int, None, None]:
+        def str_to_knx(string: str, length: int) -> Iterator[int]:
             """Serialize string."""
             if len(string) > length - 1:
                 string = string[: length - 1]
diff --git a/xknx/knxip/hpai.py b/xknx/knxip/hpai.py
index 3367603e..3ae3634f 100644
--- a/xknx/knxip/hpai.py
+++ b/xknx/knxip/hpai.py
@@ -3,7 +3,7 @@ Module for serialization and deserialization of KNX HPAI (Host Protocol Address
 
 A HPAI contains an IP address and a port.
 """
-from typing import Generator, List
+from typing import Iterator, List
 
 from xknx.exceptions import ConversionError, CouldNotParseKNXIP
 
@@ -34,7 +34,7 @@ class HPAI:
     def to_knx(self) -> List[int]:
         """Serialize to KNX/IP raw data."""
 
-        def ip_addr_to_bytes(ip_addr: str) -> Generator[int, None, None]:
+        def ip_addr_to_bytes(ip_addr: str) -> Iterator[int]:
             """Serialize ip address to byte array."""
             if not isinstance(ip_addr, str):
                 raise ConversionError("ip_addr is not a string")
diff --git a/xknx/remote_value/remote_value.py b/xknx/remote_value/remote_value.py
index 07603c39..1fffbe9b 100644
--- a/xknx/remote_value/remote_value.py
+++ b/xknx/remote_value/remote_value.py
@@ -13,7 +13,7 @@ from typing import (
     Any,
     Awaitable,
     Callable,
-    Generator,
+    Iterator,
     List,
     Optional,
     Union,
@@ -103,7 +103,7 @@ class RemoteValue(ABC):
     def has_group_address(self, group_address: GroupAddress) -> bool:
         """Test if device has given group address."""
 
-        def _internal_addresses() -> Generator[Optional[GroupAddress], None, None]:
+        def _internal_addresses() -> Iterator[Optional[GroupAddress]]:
             """Yield all group_addresses."""
             yield self.group_address
             yield self.group_address_state


Test patch:
diff --git a/test/devices_tests/device_test.py b/test/devices_tests/device_test.py
index ea607f58..e3609b3b 100644
--- a/test/devices_tests/device_test.py
+++ b/test/devices_tests/device_test.py
@@ -10,6 +10,7 @@ from xknx.telegram import GroupAddress, Telegram
 from xknx.telegram.apci import GroupValueRead, GroupValueResponse, GroupValueWrite
 
 
+@patch.multiple(Device, __abstractmethods__=set())
 class TestDevice(unittest.TestCase):
     """Test class for Switch object."""
 
@@ -39,13 +40,6 @@ class TestDevice(unittest.TestCase):
         self.loop.run_until_complete(device.after_update())
         after_update_callback.assert_called_with(device)
 
-    def test_iter_remote_value_raises_exception(self):
-        """Test _iter_remote_value raises NotImplementedError."""
-        xknx = XKNX()
-        device = Device(xknx, "TestDevice")
-
-        self.assertRaises(NotImplementedError, device._iter_remote_values)
-
     def test_process_callback(self):
         """Test process / reading telegrams from telegram queue. Test if callback was called."""
         xknx = XKNX()
diff --git a/test/devices_tests/devices_test.py b/test/devices_tests/devices_test.py
index 0b84a8bb..89098ed2 100644
--- a/test/devices_tests/devices_test.py
+++ b/test/devices_tests/devices_test.py
@@ -1,13 +1,18 @@
 """Unit test for devices container within XKNX."""
 import asyncio
 import unittest
-from unittest.mock import Mock, patch
+from unittest.mock import MagicMock, patch
 
 from xknx import XKNX
 from xknx.devices import BinarySensor, Device, Devices, Light, Switch
 from xknx.telegram import GroupAddress
 
 
+class AsyncMock(MagicMock):
+    async def __call__(self, *args, **kwargs):
+        return super().__call__(*args, **kwargs)
+
+
 # pylint: disable=too-many-public-methods,invalid-name
 class TestDevices(unittest.TestCase):
     """Test class for devices container within XKNX."""
@@ -167,6 +172,7 @@ class TestDevices(unittest.TestCase):
     #
     # TEST SYNC
     #
+    @patch.multiple(Device, __abstractmethods__=set())
     def test_sync(self):
         """Test sync function."""
         xknx = XKNX()
@@ -182,22 +188,15 @@ class TestDevices(unittest.TestCase):
     #
     # TEST CALLBACK
     #
+    @patch.multiple(Device, __abstractmethods__=set())
     def test_device_updated_callback(self):
         """Test if device updated callback is called correctly if device was updated."""
         xknx = XKNX()
         device1 = Device(xknx, "TestDevice1")
         device2 = Device(xknx, "TestDevice2")
 
-        after_update_callback1 = Mock()
-        after_update_callback2 = Mock()
-
-        async def async_after_update_callback1(device):
-            """Async callback No. 1."""
-            after_update_callback1(device)
-
-        async def async_after_update_callback2(device):
-            """Async callback No. 2."""
-            after_update_callback2(device)
+        async_after_update_callback1 = AsyncMock()
+        async_after_update_callback2 = AsyncMock()
 
         # Registering both callbacks
         xknx.devices.register_device_updated_cb(async_after_update_callback1)
@@ -205,34 +204,34 @@ class TestDevices(unittest.TestCase):
 
         # Triggering first device. Both callbacks to be called
         self.loop.run_until_complete(device1.after_update())
-        after_update_callback1.assert_called_with(device1)
-        after_update_callback2.assert_called_with(device1)
-        after_update_callback1.reset_mock()
-        after_update_callback2.reset_mock()
+        async_after_update_callback1.assert_called_with(device1)
+        async_after_update_callback2.assert_called_with(device1)
+        async_after_update_callback1.reset_mock()
+        async_after_update_callback2.reset_mock()
 
         # Triggering 2nd device. Both callbacks have to be called
         self.loop.run_until_complete(device2.after_update())
-        after_update_callback1.assert_called_with(device2)
-        after_update_callback2.assert_called_with(device2)
-        after_update_callback1.reset_mock()
-        after_update_callback2.reset_mock()
+        async_after_update_callback1.assert_called_with(device2)
+        async_after_update_callback2.assert_called_with(device2)
+        async_after_update_callback1.reset_mock()
+        async_after_update_callback2.reset_mock()
 
         # Unregistering first callback
         xknx.devices.unregister_device_updated_cb(async_after_update_callback1)
 
         # Triggering first device. Only second callback has to be called
         self.loop.run_until_complete(device1.after_update())
-        after_update_callback1.assert_not_called()
-        after_update_callback2.assert_called_with(device1)
-        after_update_callback1.reset_mock()
-        after_update_callback2.reset_mock()
+        async_after_update_callback1.assert_not_called()
+        async_after_update_callback2.assert_called_with(device1)
+        async_after_update_callback1.reset_mock()
+        async_after_update_callback2.reset_mock()
 
         # Unregistering second callback
         xknx.devices.unregister_device_updated_cb(async_after_update_callback2)
 
         # Triggering second device. No callback should be called
         self.loop.run_until_complete(device2.after_update())
-        after_update_callback1.assert_not_called()
-        after_update_callback2.assert_not_called()
-        after_update_callback1.reset_mock()
-        after_update_callback2.reset_mock()
+        async_after_update_callback1.assert_not_called()
+        async_after_update_callback2.assert_not_called()
+        async_after_update_callback1.reset_mock()
+        async_after_update_callback2.reset_mock()