Repository: XKNX/xknx
Pull request #1069 (2022-11-10 22:51:15)

Refactor GatewayScanFilter
<!--
  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.
-->
GatewayScanFilter now also matches secure enabled gateways by default. The `secure` argument as been replaced by `secure_tunnelling` and `secure_routing` arguments. When multiple methods are `True` a gateway is matched if one of them is supported. Non-secure methods don't match if secure is required for that gateway.

In "Automatic" connection mode secure-required gateways are skipped.

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:

- [ ] 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)


Patch:
diff --git a/docs/changelog.md b/docs/changelog.md
index d99e2dd1..3cfd56d8 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -8,6 +8,10 @@ nav_order: 2
 
 # Unreleased changes
 
+### Connection
+
+- GatewayScanFilter now also matches secure enabled gateways by default. The `secure` argument as been replaced by `secure_tunnelling` and `secure_routing` arguments. When multiple methods are `True` a gateway is matched if one of them is supported. Non-secure methods don't match if secure is required for that gateway.
+
 ### Features
 
 - Add support for python 3.11
diff --git a/xknx/io/gateway_scanner.py b/xknx/io/gateway_scanner.py
index 5b403cbb..e5ab9d6f 100644
--- a/xknx/io/gateway_scanner.py
+++ b/xknx/io/gateway_scanner.py
@@ -130,51 +130,66 @@ class GatewayDescriptor:
 
 
 class GatewayScanFilter:
-    """Filter to limit gateway scan attempts.
+    """Filter to limit gateway scan results.
 
-    If `tunnelling` and `routing` are set it is treated as AND.
-    KNX/IP devices that don't support `tunnelling` or `routing` aren't matched.
+    If `name` doesn't match the gateway name, the gateway will be ignored.
+
+    Connection methods are treated as OR if `True` is set for multiple methods.
+    Non-secure methods don't match if secure is required.
     """
 
     def __init__(
         self,
         name: str | None = None,
-        tunnelling: bool | None = None,
-        tunnelling_tcp: bool | None = None,
-        routing: bool | None = None,
-        secure: bool | None = False,
+        tunnelling: bool | None = True,
+        tunnelling_tcp: bool | None = True,
+        routing: bool | None = True,
+        secure_tunnelling: bool | None = True,
+        secure_routing: bool | None = True,
     ):
         """Initialize GatewayScanFilter class."""
         self.name = name
         self.tunnelling = tunnelling
         self.tunnelling_tcp = tunnelling_tcp
         self.routing = routing
-        self.secure = secure
+        self.secure_tunnelling = secure_tunnelling
+        self.secure_routing = secure_routing
 
     def match(self, gateway: GatewayDescriptor) -> bool:
         """Check whether the device is a gateway and given GatewayDescriptor matches the filter."""
         if self.name is not None and self.name != gateway.name:
             return False
         if (
-            self.tunnelling is not None
-            and self.tunnelling != gateway.supports_tunnelling
+            self.tunnelling
+            and gateway.supports_tunnelling
+            and not gateway.tunnelling_requires_secure
         ):
-            return False
+            return True
         if (
-            self.tunnelling_tcp is not None
-            and self.tunnelling_tcp != gateway.supports_tunnelling_tcp
+            self.tunnelling_tcp
+            and gateway.supports_tunnelling_tcp
+            and not gateway.tunnelling_requires_secure
         ):
-            return False
-        if self.routing is not None and self.routing != gateway.supports_routing:
-            return False
-        if self.secure is not None:
-            if self.secure is not bool(gateway.tunnelling_requires_secure):
-                return False
-        return (
-            gateway.supports_tunnelling
-            or gateway.supports_tunnelling_tcp
-            or gateway.supports_routing
-        )
+            return True
+        if (
+            self.routing
+            and gateway.supports_routing
+            and not gateway.routing_requires_secure
+        ):
+            return True
+        if (
+            self.secure_tunnelling
+            and gateway.supports_tunnelling_tcp
+            and gateway.tunnelling_requires_secure
+        ):
+            return True
+        if (
+            self.secure_routing
+            and gateway.supports_routing
+            and gateway.routing_requires_secure
+        ):
+            return True
+        return False
 
 
 class GatewayScanner:
diff --git a/xknx/io/knxip_interface.py b/xknx/io/knxip_interface.py
index f58eccb5..58e8a592 100644
--- a/xknx/io/knxip_interface.py
+++ b/xknx/io/knxip_interface.py
@@ -175,20 +175,26 @@ class KNXIPInterface:
         async for gateway in GatewayScanner(
             self.xknx,
             local_ip=self.connection_config.local_ip,
-            scan_filter=self.connection_config.scan_filter,  # secure disabled by default
+            scan_filter=self.connection_config.scan_filter,
         ).async_scan():
             try:
-                if gateway.supports_tunnelling_tcp:
+                if (
+                    gateway.supports_tunnelling_tcp
+                    and not gateway.tunnelling_requires_secure
+                ):
                     await self._start_tunnelling_tcp(
                         gateway_ip=gateway.ip_addr,
                         gateway_port=gateway.port,
                     )
-                elif gateway.supports_tunnelling:
+                elif (
+                    gateway.supports_tunnelling
+                    and not gateway.tunnelling_requires_secure
+                ):
                     await self._start_tunnelling_udp(
                         gateway_ip=gateway.ip_addr,
                         gateway_port=gateway.port,
                     )
-                elif gateway.supports_routing:
+                elif gateway.supports_routing and not gateway.routing_requires_secure:
                     await self._start_routing(
                         local_ip=self.connection_config.local_ip,
                     )


Test patch:
diff --git a/requirements/testing.txt b/requirements/testing.txt
index 48f59e77..57e84bcd 100644
--- a/requirements/testing.txt
+++ b/requirements/testing.txt
@@ -8,6 +8,6 @@ pylint==2.15.5
 pytest==7.2.0
 pytest-asyncio==0.20.1
 pytest-cov==4.0.0
-setuptools==65.5.1
+setuptools==65.5.0
 tox==3.27.0
-mypy==0.990
+mypy==0.982
diff --git a/test/io_tests/gateway_scanner_test.py b/test/io_tests/gateway_scanner_test.py
index 97cfc19e..40a939c3 100644
--- a/test/io_tests/gateway_scanner_test.py
+++ b/test/io_tests/gateway_scanner_test.py
@@ -214,72 +214,99 @@ class TestGatewayScanner:
         supports_secure=True,
     )
     gateway_desc_secure_tunnel.tunnelling_requires_secure = True
+    gateway_desc_secure_router = GatewayDescriptor(
+        name="KNX-Secure-Router",
+        ip_addr="10.1.1.12",
+        port=3761,
+        local_interface="en1",
+        local_ip="10.1.1.100",
+        supports_tunnelling=False,
+        supports_routing=True,
+    )
+    gateway_desc_secure_router.routing_requires_secure = True
 
     def test_gateway_scan_filter_match(self):
         """Test match function of gateway filter."""
         filter_default = GatewayScanFilter()
-        filter_tunnel = GatewayScanFilter(tunnelling=True)
-        filter_tcp_tunnel = GatewayScanFilter(tunnelling_tcp=True, secure=None)
-        filter_secure_tunnel = GatewayScanFilter(tunnelling_tcp=True, secure=True)
-        filter_router = GatewayScanFilter(routing=True)
+        filter_tunnel = GatewayScanFilter(routing=False, secure_routing=False)
+        filter_tcp_tunnel = GatewayScanFilter(
+            tunnelling=False,
+            tunnelling_tcp=True,
+            secure_tunnelling=None,
+            routing=False,
+            secure_routing=False,
+        )
+        filter_secure_tunnel = GatewayScanFilter(
+            tunnelling=False,
+            tunnelling_tcp=False,
+            secure_tunnelling=True,
+            routing=False,
+            secure_routing=False,
+        )
+        filter_router = GatewayScanFilter(
+            tunnelling=False,
+            tunnelling_tcp=False,
+            secure_tunnelling=False,
+            routing=True,
+            secure_routing=False,
+        )
         filter_name = GatewayScanFilter(name="KNX-Router")
-        filter_no_tunnel = GatewayScanFilter(tunnelling=False)
-        filter_no_router = GatewayScanFilter(routing=False)
-        filter_tunnel_and_router = GatewayScanFilter(tunnelling=True, routing=True)
+        filter_secure_router = GatewayScanFilter(
+            tunnelling=False,
+            tunnelling_tcp=False,
+            secure_tunnelling=False,
+            routing=False,
+            secure_routing=True,
+        )
 
         assert filter_default.match(self.gateway_desc_interface)
         assert filter_default.match(self.gateway_desc_router)
         assert filter_default.match(self.gateway_desc_both)
         assert not filter_default.match(self.gateway_desc_neither)
-        assert not filter_default.match(self.gateway_desc_secure_tunnel)
+        assert filter_default.match(self.gateway_desc_secure_tunnel)
+        assert filter_default.match(self.gateway_desc_secure_router)
 
         assert filter_tunnel.match(self.gateway_desc_interface)
         assert not filter_tunnel.match(self.gateway_desc_router)
         assert filter_tunnel.match(self.gateway_desc_both)
         assert not filter_tunnel.match(self.gateway_desc_neither)
-        assert not filter_tunnel.match(self.gateway_desc_secure_tunnel)
+        assert filter_tunnel.match(self.gateway_desc_secure_tunnel)
+        assert not filter_tunnel.match(self.gateway_desc_secure_router)
 
         assert not filter_tcp_tunnel.match(self.gateway_desc_interface)
         assert not filter_tcp_tunnel.match(self.gateway_desc_router)
         assert filter_tcp_tunnel.match(self.gateway_desc_both)
         assert not filter_tcp_tunnel.match(self.gateway_desc_neither)
-        assert filter_tcp_tunnel.match(self.gateway_desc_secure_tunnel)
+        assert not filter_tcp_tunnel.match(self.gateway_desc_secure_tunnel)
+        assert not filter_tcp_tunnel.match(self.gateway_desc_secure_router)
 
         assert not filter_secure_tunnel.match(self.gateway_desc_interface)
         assert not filter_secure_tunnel.match(self.gateway_desc_router)
         assert not filter_secure_tunnel.match(self.gateway_desc_both)
         assert not filter_secure_tunnel.match(self.gateway_desc_neither)
         assert filter_secure_tunnel.match(self.gateway_desc_secure_tunnel)
+        assert not filter_secure_tunnel.match(self.gateway_desc_secure_router)
 
         assert not filter_router.match(self.gateway_desc_interface)
         assert filter_router.match(self.gateway_desc_router)
         assert filter_router.match(self.gateway_desc_both)
         assert not filter_router.match(self.gateway_desc_neither)
         assert not filter_router.match(self.gateway_desc_secure_tunnel)
+        assert not filter_router.match(self.gateway_desc_secure_router)
 
         assert not filter_name.match(self.gateway_desc_interface)
         assert filter_name.match(self.gateway_desc_router)
         assert not filter_name.match(self.gateway_desc_both)
         assert not filter_name.match(self.gateway_desc_neither)
         assert not filter_name.match(self.gateway_desc_secure_tunnel)
-
-        assert not filter_no_tunnel.match(self.gateway_desc_interface)
-        assert filter_no_tunnel.match(self.gateway_desc_router)
-        assert not filter_no_tunnel.match(self.gateway_desc_both)
-        assert not filter_no_tunnel.match(self.gateway_desc_neither)
-        assert not filter_no_tunnel.match(self.gateway_desc_secure_tunnel)
-
-        assert filter_no_router.match(self.gateway_desc_interface)
-        assert not filter_no_router.match(self.gateway_desc_router)
-        assert not filter_no_router.match(self.gateway_desc_both)
-        assert not filter_no_router.match(self.gateway_desc_neither)
-        assert not filter_no_router.match(self.gateway_desc_secure_tunnel)
-
-        assert not filter_tunnel_and_router.match(self.gateway_desc_interface)
-        assert not filter_tunnel_and_router.match(self.gateway_desc_router)
-        assert filter_tunnel_and_router.match(self.gateway_desc_both)
-        assert not filter_tunnel_and_router.match(self.gateway_desc_neither)
-        assert not filter_tunnel_and_router.match(self.gateway_desc_secure_tunnel)
+        assert not filter_name.match(self.gateway_desc_secure_router)
+
+        assert not filter_secure_router.match(self.gateway_desc_interface)
+        assert not filter_secure_router.match(self.gateway_desc_router)
+        assert not filter_secure_router.match(self.gateway_desc_both)
+        assert not filter_secure_router.match(self.gateway_desc_neither)
+        assert not filter_secure_router.match(self.gateway_desc_secure_tunnel)
+        assert filter_secure_router.match(self.gateway_desc_secure_router)
 
     def test_search_response_reception(self):
         """Test function of gateway scanner."""