Repository: XKNX/xknx
Pull request #1499 (2024-06-25 21:26:36)

Fix DPTEnum subclass lookup
<!--
  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.
-->
DPTEnum should not be treated as concrete class - thus not being listed in `DPTBase.dpt_class_tree()`.

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)
- [x] Code quality improvements to existing code or addition of tests

## Checklist

- [ ] The documentation has been adjusted accordingly
- [x] Tests have been added that prove the fix is effective or that the feature works
- [ ] The changes are documented in the changelog (docs/changelog.md)

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/XKNX/xknx/pull/1499?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 96.65%. Comparing base [(`b528345`)](https://app.codecov.io/gh/XKNX/xknx/commit/b528345a6120890db1daff5c75409b1f6820c2ca?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) to head [(`2048164`)](https://app.codecov.io/gh/XKNX/xknx/commit/2048164184c5a9300487b99f1b4e2a218701dd5e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX).

<details><summary>Additional details and impacted files</summary>


[![Impacted file tree graph](https://app.codecov.io/gh/XKNX/xknx/pull/1499/graphs/tree.svg?width=650&height=150&src=pr&token=irWbIygS84&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX)](https://app.codecov.io/gh/XKNX/xknx/pull/1499?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX)

```diff
@@           Coverage Diff           @@
##             main    #1499   +/-   ##
=======================================
  Coverage   96.65%   96.65%           
=======================================
  Files         156      156           
  Lines       10060    10070   +10     
=======================================
+ Hits         9723     9733   +10     
  Misses        337      337           
```

| [Files](https://app.codecov.io/gh/XKNX/xknx/pull/1499?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) | Coverage Δ | |
|---|---|---|
| [xknx/dpt/\_\_init\_\_.py](https://app.codecov.io/gh/XKNX/xknx/pull/1499?src=pr&el=tree&filepath=xknx%2Fdpt%2F__init__.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9kcHQvX19pbml0X18ucHk=) | `100.00% <100.00%> (ø)` | |
| [xknx/dpt/dpt.py](https://app.codecov.io/gh/XKNX/xknx/pull/1499?src=pr&el=tree&filepath=xknx%2Fdpt%2Fdpt.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9kcHQvZHB0LnB5) | `100.00% <100.00%> (ø)` | |
| [xknx/dpt/dpt\_20.py](https://app.codecov.io/gh/XKNX/xknx/pull/1499?src=pr&el=tree&filepath=xknx%2Fdpt%2Fdpt_20.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9kcHQvZHB0XzIwLnB5) | `100.00% <100.00%> (ø)` | |

</details>

Patch:
diff --git a/xknx/dpt/__init__.py b/xknx/dpt/__init__.py
index b955f6bc..dc1f4a18 100644
--- a/xknx/dpt/__init__.py
+++ b/xknx/dpt/__init__.py
@@ -6,7 +6,7 @@ Module for encoding and decoding KNX datatypes.
 """
 
 # ruff: noqa: F401
-from .dpt import DPTBase, DPTComplex, DPTComplexData, DPTNumeric
+from .dpt import DPTBase, DPTComplex, DPTComplexData, DPTEnum, DPTNumeric
 from .dpt_3 import (
     DPTControlStartStop,
     DPTControlStartStopBlinds,
diff --git a/xknx/dpt/dpt.py b/xknx/dpt/dpt.py
index 4057217f..435d5ff5 100644
--- a/xknx/dpt/dpt.py
+++ b/xknx/dpt/dpt.py
@@ -7,7 +7,7 @@ from collections.abc import Iterator, Mapping
 from dataclasses import dataclass
 from enum import Enum
 from inspect import isabstract
-from typing import Any, Generic, TypedDict, TypeVar, cast
+from typing import Any, Generic, TypedDict, TypeVar, cast, final
 
 from xknx.exceptions import ConversionError, CouldNotParseTelegram
 
@@ -231,14 +231,21 @@ class DPTEnum(DPTBase, Generic[EnumT]):
     payload_length = 1
 
     data_type: type[EnumT]
-    VALUE_MAP: Mapping[int, EnumT]
+
+    @classmethod
+    @abstractmethod
+    def get_value_map(cls) -> Mapping[int, EnumT]:
+        """Return mapping of raw KNX values to Enum members."""
+        # At least one abstract method is needed for our parse_transcoder lookup to
+        # ignore the DPTEnum base class and only find concrete base classes.
+        # Therefore this is not a ClassVar (which wouldn't support TypeVar parameters).
 
     @classmethod
     def from_knx(cls, payload: DPTArray | DPTBinary) -> EnumT:
         """Parse/deserialize from KNX/IP raw data."""
         raw = cls.validate_payload(payload)
         try:
-            return cls.VALUE_MAP[raw[0]]  # type: ignore[no-any-return]  # without ClassVar this is Any, ClassVar doesn't work with bound TypeVar though
+            return cls.get_value_map()[raw[0]]
         except KeyError:
             raise ConversionError(
                 f"Payload not supported for {cls.__name__}", raw=raw
@@ -260,7 +267,7 @@ class DPTEnum(DPTBase, Generic[EnumT]):
                         value=value,
                         valid_values=cls.get_valid_values(),
                     ) from None
-        for knx_raw_value, enum_member in cls.VALUE_MAP.items():
+        for knx_raw_value, enum_member in cls.get_value_map().items():
             if enum_member is value:
                 return DPTArray(knx_raw_value)
         raise ConversionError(
@@ -272,7 +279,7 @@ class DPTEnum(DPTBase, Generic[EnumT]):
     @classmethod
     def get_valid_values(cls) -> list[EnumT]:
         """Return list of valid values."""
-        return list(cls.VALUE_MAP.values())
+        return list(cls.get_value_map().values())
 
 
 @dataclass(slots=True)
@@ -301,6 +308,7 @@ class DPTComplex(DPTBase, Generic[TComplexData]):
     def from_knx(cls, payload: DPTArray | DPTBinary) -> TComplexData:
         """Parse/deserialize from KNX/IP payload data."""
 
+    @final
     @classmethod
     def to_knx(cls, value: TComplexData | Mapping[str, Any]) -> DPTArray:
         """Serialize to KNX/IP raw data."""
diff --git a/xknx/dpt/dpt_20.py b/xknx/dpt/dpt_20.py
index c5e82d5c..abf27299 100644
--- a/xknx/dpt/dpt_20.py
+++ b/xknx/dpt/dpt_20.py
@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+from collections.abc import Mapping
 from enum import Enum
 
 from xknx.exceptions import ConversionError
@@ -51,22 +52,26 @@ class DPTHVACContrMode(DPTEnum[HVACControllerMode]):
     dpt_sub_number = 105
     value_type = "hvac_controller_mode"
     data_type = HVACControllerMode
-    VALUE_MAP = {
-        0: HVACControllerMode.AUTO,
-        1: HVACControllerMode.HEAT,
-        2: HVACControllerMode.MORNING_WARMUP,
-        3: HVACControllerMode.COOL,
-        4: HVACControllerMode.NIGHT_PURGE,
-        5: HVACControllerMode.PRECOOL,
-        6: HVACControllerMode.OFF,
-        7: HVACControllerMode.TEST,
-        8: HVACControllerMode.EMERGENCY_HEAT,
-        9: HVACControllerMode.FAN_ONLY,
-        10: HVACControllerMode.FREE_COOL,
-        11: HVACControllerMode.ICE,
-        14: HVACControllerMode.DRY,
-        20: HVACControllerMode.NODEM,
-    }
+
+    @classmethod
+    def get_value_map(cls) -> Mapping[int, HVACControllerMode]:
+        """Return mapping of raw KNX values to Enum members."""
+        return {
+            0: HVACControllerMode.AUTO,
+            1: HVACControllerMode.HEAT,
+            2: HVACControllerMode.MORNING_WARMUP,
+            3: HVACControllerMode.COOL,
+            4: HVACControllerMode.NIGHT_PURGE,
+            5: HVACControllerMode.PRECOOL,
+            6: HVACControllerMode.OFF,
+            7: HVACControllerMode.TEST,
+            8: HVACControllerMode.EMERGENCY_HEAT,
+            9: HVACControllerMode.FAN_ONLY,
+            10: HVACControllerMode.FREE_COOL,
+            11: HVACControllerMode.ICE,
+            14: HVACControllerMode.DRY,
+            20: HVACControllerMode.NODEM,
+        }
 
 
 class DPTHVACMode(DPTEnum[HVACOperationMode]):
@@ -80,13 +85,17 @@ class DPTHVACMode(DPTEnum[HVACOperationMode]):
     dpt_sub_number = 102
     value_type = "hvac_mode"
     data_type = HVACOperationMode
-    VALUE_MAP = {
-        0: HVACOperationMode.AUTO,
-        1: HVACOperationMode.COMFORT,
-        2: HVACOperationMode.STANDBY,
-        3: HVACOperationMode.NIGHT,
-        4: HVACOperationMode.FROST_PROTECTION,
-    }
+
+    @classmethod
+    def get_value_map(cls) -> Mapping[int, HVACOperationMode]:
+        """Return mapping of raw KNX values to Enum members."""
+        return {
+            0: HVACOperationMode.AUTO,
+            1: HVACOperationMode.COMFORT,
+            2: HVACOperationMode.STANDBY,
+            3: HVACOperationMode.NIGHT,
+            4: HVACOperationMode.FROST_PROTECTION,
+        }
 
 
 class DPTControllerStatus(DPTEnum[HVACOperationMode]):
@@ -101,12 +110,16 @@ class DPTControllerStatus(DPTEnum[HVACOperationMode]):
     """
 
     data_type = HVACOperationMode
-    VALUE_MAP = {
-        0x21: HVACOperationMode.COMFORT,
-        0x22: HVACOperationMode.STANDBY,
-        0x24: HVACOperationMode.NIGHT,
-        0x28: HVACOperationMode.FROST_PROTECTION,
-    }
+
+    @classmethod
+    def get_value_map(cls) -> Mapping[int, HVACOperationMode]:
+        """Return mapping of raw KNX values to Enum members."""
+        return {
+            0x21: HVACOperationMode.COMFORT,
+            0x22: HVACOperationMode.STANDBY,
+            0x24: HVACOperationMode.NIGHT,
+            0x28: HVACOperationMode.FROST_PROTECTION,
+        }
 
     @classmethod
     def from_knx(cls, payload: DPTArray | DPTBinary) -> HVACOperationMode:


Test patch:
diff --git a/test/dpt_tests/dpt_test.py b/test/dpt_tests/dpt_test.py
index a9dc7153..3351649f 100644
--- a/test/dpt_tests/dpt_test.py
+++ b/test/dpt_tests/dpt_test.py
@@ -9,6 +9,10 @@ from xknx.dpt import (
     DPTArray,
     DPTBase,
     DPTBinary,
+    DPTColorRGBW,
+    DPTComplex,
+    DPTEnum,
+    DPTHVACContrMode,
     DPTNumeric,
     DPTScaling,
     DPTString,
@@ -23,7 +27,19 @@ class TestDPTBase:
     def test_dpt_abstract_subclasses_ignored(self):
         """Test if abstract base classes are ignored by dpt_class_tree and __recursive_subclasses__."""
         for dpt in DPTBase.dpt_class_tree():
-            assert dpt not in (DPTBase, DPTNumeric)
+            assert dpt not in (DPTBase, DPTNumeric, DPTEnum, DPTComplex)
+
+    def test_dpt_concrete_subclasses_included(self):
+        """Test if concrete subclasses are included by dpt_class_tree."""
+        for dpt in (
+            DPT2ByteFloat,
+            DPTString,
+            DPTTemperature,
+            DPTScaling,
+            DPTHVACContrMode,
+            DPTColorRGBW,
+        ):
+            assert dpt in DPTBase.dpt_class_tree()
 
     @pytest.mark.parametrize("dpt_class", [DPTString, DPT2ByteFloat])
     def test_dpt_non_abstract_baseclass_included(self, dpt_class):