Repository: XKNX/xknx
Pull request #636 (2021-03-17 23:42:39)

Fix Address comparison / ValueReader
<!--
  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.
-->
Comparing GroupAddress to IndividualAddress raised a TypeError. 
```python
GroupAddress(1) == IndividualAddress(1) # raised TypeError because different types
"string" == 4 # doesn't raise anything - and so should BaseAddress
```
In the ValueReader we had a match-all callback for TelegramQueue so when a ValueReader was waiting for a Telegram and eg a `DeviceDescriptorRead` comes in the callback
```python
if telegram.destination_address == self.group_address:
```
raised a TypeError and the TelegramQueue stalls (I'm not sure why - could be HA suppressing the Error)

The ValueReader now registers its GroupAddress in the TelegramQueue.Callback (and checks it again in its `telegram_received` function).

Fixes https://github.com/home-assistant/core/issues/47735

## 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.
-->

- [x] 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:

- [x] I have performed a self-review of my own code
- [x] The documentation has been adjusted accordingly
- [x] The changes generate no new warnings
- [x] Tests have been added that prove the fix is effective or that the feature works
- [x] The changes are documented in the changelog
- [ ] The Homeassistant plugin has been adjusted in case of new config options



Patch:
diff --git a/changelog.md b/changelog.md
index 2be15a7b..eba4d816 100644
--- a/changelog.md
+++ b/changelog.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased changes
+
+### Internals
+
+- Comparing GroupAddress or IndividualAddress to other types don't raise TypeError anymore.
+
 ## 0.17.3 Passive addresses 2021-03-16
 
 ### Devices
diff --git a/xknx/core/telegram_queue.py b/xknx/core/telegram_queue.py
index 581f5273..f870e87a 100644
--- a/xknx/core/telegram_queue.py
+++ b/xknx/core/telegram_queue.py
@@ -119,15 +119,16 @@ class TelegramQueue:
                 self.xknx.telegrams.task_done()
                 break
 
-            try:
-                if telegram.direction == TelegramDirection.INCOMING:
+            if telegram.direction == TelegramDirection.INCOMING:
+                try:
                     await self.process_telegram_incoming(telegram)
+                except XKNXException as ex:
+                    logger.error("Error while processing incoming telegram %s", ex)
+                finally:
                     self.xknx.telegrams.task_done()
-                elif telegram.direction == TelegramDirection.OUTGOING:
-                    self.outgoing_queue.put_nowait(telegram)
-                    # self.xknx.telegrams.task_done() for outgoing is called in _outgoing_rate_limiter.
-            except XKNXException as ex:
-                logger.error("Error while processing telegram %s", ex)
+            elif telegram.direction == TelegramDirection.OUTGOING:
+                self.outgoing_queue.put_nowait(telegram)
+                # self.xknx.telegrams.task_done() for outgoing is called in _outgoing_rate_limiter.
 
     async def _outgoing_rate_limiter(self) -> None:
         """Endless loop for processing outgoing telegrams."""
diff --git a/xknx/core/value_reader.py b/xknx/core/value_reader.py
index b8a61729..d02cc402 100644
--- a/xknx/core/value_reader.py
+++ b/xknx/core/value_reader.py
@@ -37,7 +37,8 @@ class ValueReader:
     async def read(self) -> Optional[Telegram]:
         """Send group read and wait for response."""
         cb_obj = self.xknx.telegram_queue.register_telegram_received_cb(
-            self.telegram_received
+            self.telegram_received,
+            group_addresses=[self.group_address],
         )
         await self.send_group_read()
 
diff --git a/xknx/telegram/address.py b/xknx/telegram/address.py
index 0fe058c4..073a0d73 100644
--- a/xknx/telegram/address.py
+++ b/xknx/telegram/address.py
@@ -12,6 +12,7 @@ The module supports all different writings of group addresses:
 * 2nd level: "1/2"
 * Free format: "123"
 """
+from abc import ABC
 from enum import Enum
 from re import compile as re_compile
 from typing import Optional, Tuple, Union
@@ -41,7 +42,7 @@ def address_tuple_to_int(address: Tuple[int, int]) -> int:
     return int(address[0] * 256 + address[1])
 
 
-class BaseAddress:  # pylint: disable=too-few-public-methods
+class BaseAddress(ABC):
     """Base class for all knx address types."""
 
     def __init__(self) -> None:
@@ -65,11 +66,9 @@ class BaseAddress:  # pylint: disable=too-few-public-methods
 
         Returns `False` if we check against `None`.
         """
-        if other is None:
-            return False
-        if type(self) is not type(other):
-            raise TypeError()
-        return self.__hash__() == other.__hash__()
+        if isinstance(self, type(other)):
+            return self.__hash__() == other.__hash__()
+        return False
 
     def __hash__(self) -> int:
         """Hash Address so it can be used as dict key."""


Test patch:
diff --git a/test/telegram_tests/address_test.py b/test/telegram_tests/address_test.py
index 88338dce..60b9b8af 100644
--- a/test/telegram_tests/address_test.py
+++ b/test/telegram_tests/address_test.py
@@ -82,10 +82,10 @@ class TestIndividualAddress(TestCase):
         self.assertEqual(IndividualAddress("1.0.0"), IndividualAddress(4096))
         self.assertNotEqual(IndividualAddress("1.0.0"), IndividualAddress("1.1.1"))
         self.assertNotEqual(IndividualAddress("1.0.0"), None)
-        with self.assertRaises(TypeError):
-            IndividualAddress(
-                "1.0.0"
-            ) == "example"  # pylint: disable=expression-not-assigned
+        self.assertNotEqual(IndividualAddress("1.0.0"), "example")
+        self.assertNotEqual(IndividualAddress("1.1.1"), GroupAddress("1/1/1"))
+        self.assertNotEqual(IndividualAddress(250), GroupAddress(250))
+        self.assertNotEqual(IndividualAddress(250), 250)
 
     def test_representation(self):
         """Test string representation of address."""
@@ -217,8 +217,9 @@ class TestGroupAddress(TestCase):
         self.assertEqual(GroupAddress("1/0"), GroupAddress(2048))
         self.assertNotEqual(GroupAddress("1/1"), GroupAddress("1/1/0"))
         self.assertNotEqual(GroupAddress("1/0"), None)
-        with self.assertRaises(TypeError):
-            GroupAddress("1/0") == "example"  # pylint: disable=expression-not-assigned
+        self.assertNotEqual(GroupAddress("1/0"), "example")
+        self.assertNotEqual(GroupAddress(1), IndividualAddress(1))
+        self.assertNotEqual(GroupAddress(1), 1)
 
     def test_representation(self):
         """Test string representation of address."""