Repository: XKNX/xknx
Pull request #981 (2022-06-22 06:43:06)

GatewayDescriptor parsing independent of DIB order
<!--
  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.
-->
Fix GatewayDescriptor parsing when SearchResponseExtended DIBs are in unexpected order

When `DIBSecuredServiceFamilies` was parsed after `DIBSuppSVCFamilies` it overwrote its previously parsed fields because the `isinstance()` check matched as one derived from the other.

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.
-->

- [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:

- [ ] The documentation has been adjusted accordingly
- [x] Tests have been added that prove the fix is effective or that the feature works
- [x] The changes are documented in the changelog (docs/changelog.md)

codecov[bot]: # [Codecov](https://codecov.io/gh/XKNX/xknx/pull/981?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) Report
> Merging [#981](https://codecov.io/gh/XKNX/xknx/pull/981?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) (8824563) into [main](https://codecov.io/gh/XKNX/xknx/commit/30edee57435ba55e4bf635cc35d2a12e9d4f0b02?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) (30edee5) will **increase** coverage by `0.10%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/XKNX/xknx/pull/981/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://codecov.io/gh/XKNX/xknx/pull/981?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX)

```diff
@@            Coverage Diff             @@
##             main     #981      +/-   ##
==========================================
+ Coverage   96.00%   96.11%   +0.10%     
==========================================
  Files         131      131              
  Lines        7942     7948       +6     
==========================================
+ Hits         7625     7639      +14     
+ Misses        317      309       -8     
```


| [Impacted Files](https://codecov.io/gh/XKNX/xknx/pull/981?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) | Coverage Δ | |
|---|---|---|
| [xknx/knxip/dib.py](https://codecov.io/gh/XKNX/xknx/pull/981/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9rbnhpcC9kaWIucHk=) | `89.82% <100.00%> (+1.00%)` | :arrow_up: |
| [xknx/io/gateway\_scanner.py](https://codecov.io/gh/XKNX/xknx/pull/981/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9pby9nYXRld2F5X3NjYW5uZXIucHk=) | `97.77% <0.00%> (+5.18%)` | :arrow_up: |


Patch:
diff --git a/docs/changelog.md b/docs/changelog.md
index 95c80354..99750f99 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -8,6 +8,10 @@ nav_order: 2
 
 ## Unreleased changes
 
+### Bugfix
+
+- Fix GatewayDescriptor parsing when SearchResponseExtended DIBs are in unexpected order
+
 ## 0.21.4 Fan out 2022-06-07
 
 ### Devices
diff --git a/xknx/knxip/dib.py b/xknx/knxip/dib.py
index df1867a0..b01ff749 100644
--- a/xknx/knxip/dib.py
+++ b/xknx/knxip/dib.py
@@ -14,7 +14,7 @@ from __future__ import annotations
 
 from abc import ABC, abstractmethod
 import socket
-from typing import NamedTuple
+from typing import NamedTuple, final
 
 from xknx.exceptions import CouldNotParseKNXIP
 from xknx.telegram import IndividualAddress
@@ -117,6 +117,7 @@ class DIBGeneric(DIB):
         return f'<DIB dtc="{self.dtc}" data="{", ".join(f"0x{i:02x}" for i in self.data)}" />'
 
 
+@final
 class DIBDeviceInformation(DIB):
     """Class for serialization and deserialization of KNX DIB Device Information Block."""
 
@@ -213,10 +214,10 @@ class DIBDeviceInformation(DIB):
         )
 
 
-class DIBSuppSVCFamilies(DIB):
-    """Class for serialization and deserialization of KNX DIB Supported Services."""
+class _DIBServiceFamilies(DIB):
+    """Base class for serialization and deserialization of KNX DIB Service Families."""
 
-    type_code = DIBTypeCode.SUPP_SVC_FAMILIES
+    type_code: DIBTypeCode
 
     class Family:
         """Class for storing a supported device family."""
@@ -289,7 +290,15 @@ class DIBSuppSVCFamilies(DIB):
         return f'<{self.__class__.__name__} families="[{_families_str}]" />'
 
 
-class DIBSecuredServiceFamilies(DIBSuppSVCFamilies):
+@final
+class DIBSuppSVCFamilies(_DIBServiceFamilies):
+    """Class for serialization and deserialization of KNX DIB Supported Services."""
+
+    type_code = DIBTypeCode.SUPP_SVC_FAMILIES
+
+
+@final
+class DIBSecuredServiceFamilies(_DIBServiceFamilies):
     """Class for serialization and deserialization of KNX DIB Secured Service Families."""
 
     type_code = DIBTypeCode.SECURED_SERVICE_FAMILIES
@@ -312,6 +321,7 @@ class TunnelingSlotStatus(NamedTuple):
         )
 
 
+@final
 class DIBTunnelingInfo(DIB):
     """Class for serialization and deserialization of KNX DIB Tunneling Info."""
 


Test patch:
diff --git a/test/io_tests/gateway_scanner_test.py b/test/io_tests/gateway_scanner_test.py
index e98f3ec8..97cfc19e 100644
--- a/test/io_tests/gateway_scanner_test.py
+++ b/test/io_tests/gateway_scanner_test.py
@@ -18,10 +18,148 @@ from xknx.knxip import (
     SearchRequest,
     SearchRequestExtended,
     SearchResponse,
+    SearchResponseExtended,
 )
 from xknx.telegram import IndividualAddress
 
 
+class TestGatewayDescriptor:
+    """Test GatewayDescriptor object."""
+
+    @pytest.mark.parametrize(
+        "raw,expected",
+        [
+            (
+                # MDT SCN-IP000.03 IP Interface (IP-Secure enabled)
+                bytes.fromhex(
+                    "0610020c00a20801c0a800ba0e57360102001101000000837b40054500000000"
+                    "cc1be080b80153434e2d49503030302e303320495020496e7465726661636520"
+                    "6d6974200c02020203020402070209010808000000f000121003000000004005"
+                    "45000000000001041404c0a800ba40054500c0a800010000000004000c051101"
+                    "1102110311041105060603010401140700f01102000011030000110400001105"
+                    "0000"
+                ),
+                {
+                    "supports_routing": False,
+                    "supports_tunnelling": True,
+                    "supports_tunnelling_tcp": True,
+                    "supports_secure": True,
+                    "routing_requires_secure": False,
+                    "tunnelling_requires_secure": True,
+                },
+            ),
+            (
+                # MDT SCN-IP100.02 IP Router (no IP-Secure support, CoreV1)
+                bytes.fromhex(
+                    "06100202004e08010a0102280e5736010200200000000083477f0124e000170c"
+                    "cc1be08004c44d4454204b4e5820495020526f75746572000000000000000000"
+                    "000000000a020201030104010501"
+                ),
+                {
+                    "supports_routing": True,
+                    "supports_tunnelling": True,
+                    "supports_tunnelling_tcp": False,
+                    "supports_secure": False,
+                    "routing_requires_secure": None,
+                    "tunnelling_requires_secure": None,
+                },
+            ),
+            (
+                # Gira X1 (no IP-Secure support but CoreV2)
+                bytes.fromhex(
+                    "0610020c006408010a0100290e57360102001001000000081171012600000000"
+                    "000ab3290b134769726120583100000000000000000000000000000000000000"
+                    "000000000c0202020302040207010901140700dc10fbfff810fcffff10fdffff"
+                    "10feffff"
+                ),
+                {
+                    "supports_routing": False,
+                    "supports_tunnelling": True,
+                    "supports_tunnelling_tcp": True,
+                    "supports_secure": True,
+                    "routing_requires_secure": None,
+                    "tunnelling_requires_secure": None,
+                },
+            ),
+            (
+                # Gira IP-Router I12 (no IP-Secure support but CoreV2)
+                bytes.fromhex(
+                    "0610020c006608010a0100280e57360102001000000000082d40834de000170c"
+                    "000ab3274a3247697261204b4e582f49502d526f757465720000000000000000"
+                    "000000000e02020203020402050207010901140700dc10f1fffe10f2ffff10f3"
+                    "ffff10f4ffff"
+                ),
+                {
+                    "supports_routing": True,
+                    "supports_tunnelling": True,
+                    "supports_tunnelling_tcp": True,
+                    "supports_secure": True,
+                    "routing_requires_secure": None,
+                    "tunnelling_requires_secure": None,
+                },
+            ),
+            (
+                # Jung IPR 300 SREG (IP-Secure disabled)
+                bytes.fromhex(
+                    "0610020c007408010a0101550e57360102004000000000a615000037e000170c"
+                    "0022d10400374a756e67204b4e582049502d526f757465720000000000000000"
+                    "000000000c0202020302040205020901240700f8400100054002000540030005"
+                    "4004000540050005400600054007000540080005"
+                ),
+                {
+                    "supports_routing": True,
+                    "supports_tunnelling": True,
+                    "supports_tunnelling_tcp": True,
+                    "supports_secure": True,
+                    "routing_requires_secure": None,
+                    "tunnelling_requires_secure": None,
+                },
+            ),
+            (
+                # Jung IPR 300 SREG (IP-Secure enabled)
+                bytes.fromhex(
+                    "0610020c008408010a0101550e57360102004000000000a615000037e000170c"
+                    "0022d10400374b4e582049502d526f7574657200000000000000000000000000"
+                    "000000000c02020203020402050209010808000004b0091a0806030104010501"
+                    "240700f840010005400200054003000540040005400500054006000540070005"
+                    "40080005"
+                ),
+                {
+                    "supports_routing": True,
+                    "supports_tunnelling": True,
+                    "supports_tunnelling_tcp": True,
+                    "supports_secure": True,
+                    "routing_requires_secure": True,
+                    "tunnelling_requires_secure": True,
+                },
+            ),
+        ],
+    )
+    def test_parser(self, raw, expected):
+        """Test parsing GatewayDescriptor objects from real-world responses."""
+        response = KNXIPFrame()
+        response.from_knx(raw)
+        assert isinstance(response.body, (SearchResponse, SearchResponseExtended))
+
+        descriptor = GatewayDescriptor(
+            ip_addr=response.body.control_endpoint.ip_addr,
+            port=response.body.control_endpoint.port,
+        )
+        descriptor.parse_dibs(response.body.dibs)
+
+        assert descriptor.supports_routing is expected["supports_routing"]
+        assert descriptor.supports_tunnelling is expected["supports_tunnelling"]
+        assert descriptor.supports_tunnelling_tcp is expected["supports_tunnelling_tcp"]
+
+        assert descriptor.supports_secure is expected["supports_secure"]
+
+        assert descriptor.routing_requires_secure is expected["routing_requires_secure"]
+        assert (
+            descriptor.tunnelling_requires_secure
+            is expected["tunnelling_requires_secure"]
+        )
+
+
 class TestGatewayScanner:
     """Test class for xknx/io/GatewayScanner objects."""