Repository: XKNX/xknx
Pull request #552 (2021-01-06 23:15:13)

remove DPTComparator
- DPTBinary and DPTArray are not equal - even if their `.value` is
- DPTBinary and DPTArray are never equal to `None`

<!--
  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.
-->


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
- [x] 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/36139994/badge)](https://coveralls.io/builds/36139994)

Coverage decreased (-0.02%) to 94.857% when pulling **a0d7bedaad0c088272a0620b2227a510c0f9e404 on farmio:typecheck-dpt** into **4379417cd6f761362adc9a9dc4406cb095b7b7a5 on XKNX:master**.


Patch:
diff --git a/changelog.md b/changelog.md
index f51c72ef..9f03bf4f 100644
--- a/changelog.md
+++ b/changelog.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased changes
+
+### Internals
+
+- remove DPTComparator: DPTBinary and DPTArray are not equal, even if their .value is, and are never equal to `None`.
+
 ## 0.16.0 APCI possibilities considerably increased 2021-01-01
 
 ### Devices
diff --git a/xknx/dpt/__init__.py b/xknx/dpt/__init__.py
index 13fb10e3..aa0c3182 100644
--- a/xknx/dpt/__init__.py
+++ b/xknx/dpt/__init__.py
@@ -5,7 +5,7 @@ Module for encoding and decoding KNX datatypes.
 * Derived KNX Values like Scaling, Temperature
 """
 # flake8: noqa
-from .dpt import DPTArray, DPTBase, DPTBinary, DPTComparator
+from .dpt import DPTArray, DPTBase, DPTBinary
 from .dpt_1byte_signed import DPTPercentV8, DPTSignedRelativeValue, DPTValue1Count
 from .dpt_1byte_uint import (
     DPTDecimalFactor,
@@ -208,7 +208,6 @@ __all__ = [
     "DPTChargeDensityVolume",
     "DPTColorTemperature",
     "DPTCommonTemperature",
-    "DPTComparator",
     "DPTCompressibility",
     "DPTConductance",
     "DPTControllerStatus",
diff --git a/xknx/dpt/dpt.py b/xknx/dpt/dpt.py
index 4cc3c2b9..f1363dc7 100644
--- a/xknx/dpt/dpt.py
+++ b/xknx/dpt/dpt.py
@@ -128,7 +128,7 @@ class DPTBinary:
     APCI_BITMASK = 0x3F
     APCI_MAX_VALUE = APCI_BITMASK
 
-    def __init__(self, value) -> None:
+    def __init__(self, value: int) -> None:
         """Initialize DPTBinary class."""
         if not isinstance(value, int):
             raise TypeError()
@@ -137,9 +137,11 @@ class DPTBinary:
 
         self.value = value
 
-    def __eq__(self, other):
+    def __eq__(self, other: object) -> bool:
         """Equal operator."""
-        return DPTComparator.compare(self, other)
+        if isinstance(other, DPTBinary):
+            return self.value == other.value
+        return False
 
     def __str__(self) -> str:
         """Return object as readable string."""
@@ -163,49 +165,12 @@ class DPTArray:
         else:
             raise TypeError()
 
-    def __eq__(self, other):
+    def __eq__(self, other: object) -> bool:
         """Equal operator."""
-        return DPTComparator.compare(self, other)
+        if isinstance(other, DPTArray):
+            return self.value == other.value
+        return False
 
     def __str__(self) -> str:
         """Return object as readable string."""
         return '<DPTArray value="[{}]" />'.format(",".join(hex(b) for b in self.value))
-
-
-class DPTComparator:
-    """Helper class to compare different types of DPT objects."""
-
-    # pylint: disable=too-few-public-methods
-
-    @staticmethod
-    def compare(a, b):
-        """Test if 'a' and 'b' are the same."""
-        # pylint: disable=invalid-name,too-many-return-statements,len-as-condition
-        if a is None and b is None:
-            return True
-
-        if a is None:
-            if isinstance(b, DPTBinary):
-                return b.value == 0
-            if isinstance(b, DPTArray):
-                return len(b.value) == 0
-
-        if b is None:
-            if isinstance(a, DPTBinary):
-                return a.value == 0
-            if isinstance(a, DPTArray):
-                return len(a.value) == 0
-
-        if isinstance(a, DPTArray) and isinstance(b, DPTArray):
-            return a.value == b.value
-
-        if isinstance(a, DPTBinary) and isinstance(b, DPTBinary):
-            return a.value == b.value
-
-        if isinstance(a, DPTBinary) and isinstance(b, DPTArray):
-            return a.value == 0 and len(b.value) == 0
-
-        if isinstance(a, DPTArray) and isinstance(b, DPTBinary):
-            return len(a.value) == 0 and b.value == 0
-
-        raise TypeError()


Test patch:
diff --git a/test/dpt_tests/dpt_test.py b/test/dpt_tests/dpt_test.py
index 95a3575c..873356f8 100644
--- a/test/dpt_tests/dpt_test.py
+++ b/test/dpt_tests/dpt_test.py
@@ -1,7 +1,7 @@
 """Unit test for KNX binary/integer objects."""
 import unittest
 
-from xknx.dpt import DPTArray, DPTBase, DPTBinary, DPTComparator
+from xknx.dpt import DPTArray, DPTBase, DPTBinary
 from xknx.exceptions import ConversionError
 
 
@@ -13,6 +13,8 @@ class TestDPT(unittest.TestCase):
     def test_compare_binary(self):
         """Test comparison of DPTBinary objects."""
         self.assertEqual(DPTBinary(0), DPTBinary(0))
+        self.assertEqual(DPTBinary(0), DPTBinary(False))
+        self.assertEqual(DPTBinary(1), DPTBinary(True))
         self.assertEqual(DPTBinary(2), DPTBinary(2))
         self.assertNotEqual(DPTBinary(1), DPTBinary(4))
         self.assertNotEqual(DPTBinary(2), DPTBinary(0))
@@ -30,11 +32,11 @@ class TestDPT(unittest.TestCase):
         self.assertNotEqual(DPTArray((1, 2, 3)), DPTArray([1, 2, 4]))
 
     def test_compare_none(self):
-        """Test comparison of empty DPTArray objects with None."""
-        self.assertEqual(DPTArray(()), None)
-        self.assertEqual(None, DPTArray(()))
-        self.assertEqual(DPTBinary(0), None)
-        self.assertEqual(None, DPTBinary(0))
+        """Test comparison DPTArray objects with None."""
+        self.assertNotEqual(DPTArray(()), None)
+        self.assertNotEqual(None, DPTArray(()))
+        self.assertNotEqual(DPTBinary(0), None)
+        self.assertNotEqual(None, DPTBinary(0))
         self.assertNotEqual(DPTArray((1, 2, 3)), None)
         self.assertNotEqual(None, DPTArray((1, 2, 3)))
         self.assertNotEqual(DPTBinary(1), None)
@@ -42,8 +44,10 @@ class TestDPT(unittest.TestCase):
 
     def test_compare_array_binary(self):
         """Test comparison of empty DPTArray objects with DPTBinary objects."""
-        self.assertEqual(DPTArray(()), DPTBinary(0))
-        self.assertEqual(DPTBinary(0), DPTArray(()))
+        self.assertNotEqual(DPTArray(()), DPTBinary(0))
+        self.assertNotEqual(DPTBinary(0), DPTArray(()))
+        self.assertNotEqual(DPTBinary(0), DPTArray(0))
+        self.assertNotEqual(DPTBinary(1), DPTArray(1))
         self.assertNotEqual(DPTArray((1, 2, 3)), DPTBinary(2))
         self.assertNotEqual(DPTBinary(2), DPTArray((1, 2, 3)))
         self.assertNotEqual(DPTArray((2,)), DPTBinary(2))
@@ -68,17 +72,6 @@ class TestDPT(unittest.TestCase):
         with self.assertRaises(TypeError):
             DPTArray("bla")
 
-    def test_dpt_comparator_none_with_none(self):
-        """Test comperator for DPTBinary and DPTBinary - missing cases."""
-        self.assertTrue(DPTComparator.compare(None, None))
-        self.assertTrue(DPTComparator.compare(None, DPTBinary(0)))
-        self.assertTrue(DPTComparator.compare(None, DPTArray(())))
-
-    def test_dpt_comparator_none_with_wrong_parameter(self):
-        """Test comperator for DPTBinary and DPTBinary - wrong parameter."""
-        with self.assertRaises(TypeError):
-            DPTComparator.compare("bla", DPTBinary(0))
-
 
 class TestDPTBase(unittest.TestCase):
     """Test class for transcoder base object."""
diff --git a/test/knxip_tests/cemi_frame_test.py b/test/knxip_tests/cemi_frame_test.py
index 0dbdc19c..c3fd05ff 100644
--- a/test/knxip_tests/cemi_frame_test.py
+++ b/test/knxip_tests/cemi_frame_test.py
@@ -3,7 +3,7 @@
 from unittest.mock import MagicMock
 
 from pytest import fixture, raises
-from xknx.dpt import DPTBinary, DPTComparator
+from xknx.dpt import DPTBinary
 from xknx.exceptions import ConversionError, CouldNotParseKNXIP, UnsupportedCEMIMessage
 from xknx.knxip.cemi_frame import CEMIFrame
 from xknx.knxip.knxip_enum import CEMIFlags, CEMIMessageCode
@@ -92,7 +92,7 @@ def test_invalid_payload(frame):
     frame.code = CEMIMessageCode.L_DATA_IND
     frame.flags = 0
     frame.mpdu_len = 1
-    frame.payload = DPTComparator()
+    frame.payload = DPTBinary(1)
     frame.src_addr = IndividualAddress(0)
     frame.dst_addr = IndividualAddress(0)