Repository: XKNX/xknx
Pull request #850 (2022-01-17 23:06:21)

Raise if an initial connection can not be established
<!--
  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.
-->
Raise if an initial connection can not be established.

For Tunneling: auto-reconnect only when the connection was successful once.

This enables HA to gracefully fail on `async_setup_entry` with
>[homeassistant.config_entries] Config entry 'Tunneling @ 10.1.0.10' for knx integration not ready yet: Tunnel connection could not be established; Retrying in background

rather than silently running xknx's auto_reconnect when connection parameters were configured wrongly.

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:

- [x] 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


codecov[bot]: # [Codecov](https://codecov.io/gh/XKNX/xknx/pull/850?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) Report
> Merging [#850](https://codecov.io/gh/XKNX/xknx/pull/850?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) (1dc8968) into [main](https://codecov.io/gh/XKNX/xknx/commit/e85342d2b2b0e9416b5ece2f5e4bd06798b92684?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) (e85342d) will **decrease** coverage by `0.03%`.
> The diff coverage is `30.00%`.

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

```diff
@@            Coverage Diff             @@
##             main     #850      +/-   ##
==========================================
- Coverage   95.53%   95.49%   -0.04%     
==========================================
  Files         112      112              
  Lines        6898     6904       +6     
==========================================
+ Hits         6590     6593       +3     
- Misses        308      311       +3     
```


| [Impacted Files](https://codecov.io/gh/XKNX/xknx/pull/850?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) | Coverage Δ | |
|---|---|---|
| [xknx/io/knxip\_interface.py](https://codecov.io/gh/XKNX/xknx/pull/850/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9pby9rbnhpcF9pbnRlcmZhY2UucHk=) | `74.46% <0.00%> (-1.62%)` | :arrow_down: |
| [xknx/io/routing.py](https://codecov.io/gh/XKNX/xknx/pull/850/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9pby9yb3V0aW5nLnB5) | `48.88% <50.00%> (+1.16%)` | :arrow_up: |
| [xknx/io/tunnel.py](https://codecov.io/gh/XKNX/xknx/pull/850/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9pby90dW5uZWwucHk=) | `70.98% <50.00%> (+0.26%)` | :arrow_up: |


Patch:
diff --git a/changelog.md b/changelog.md
index 79048ba0..86e002b5 100644
--- a/changelog.md
+++ b/changelog.md
@@ -8,6 +8,7 @@
 
 ### Connection
 
+- Raise if an initial connection can not be established, auto-reconnect only when the connection was successful once
 - Add support for TCP tunnel connections
 - Optionally run KNXIPInterface in separate thread
 - Handle separate Tunneling control and data endpoints
diff --git a/xknx/io/knxip_interface.py b/xknx/io/knxip_interface.py
index 6060f4e2..03038418 100644
--- a/xknx/io/knxip_interface.py
+++ b/xknx/io/knxip_interface.py
@@ -56,7 +56,7 @@ class KNXIPInterface:
         self.connection_config = connection_config
 
     async def start(self) -> None:
-        """Start KNX/IP interface."""
+        """Start KNX/IP interface. Raise `CommunicationError` if connection fails."""
         await self._start()
 
     async def _start(self) -> None:
@@ -309,7 +309,10 @@ def find_local_ip(gateway_ip: str) -> str:
         logger.warning(
             "No interface on same subnet as gateway found. Falling back to default gateway."
         )
-        default_gateway = _find_default_gateway()
+        try:
+            default_gateway = _find_default_gateway()
+        except KeyError as err:
+            raise CommunicationError(f"No route to {gateway} found") from err
         local_ip = _scan_interfaces(default_gateway)
     assert isinstance(local_ip, str)
     return local_ip
diff --git a/xknx/io/routing.py b/xknx/io/routing.py
index 68979a6b..1ff463cd 100644
--- a/xknx/io/routing.py
+++ b/xknx/io/routing.py
@@ -9,6 +9,7 @@ import logging
 from typing import TYPE_CHECKING, Callable
 
 from xknx.core import XknxConnectionState
+from xknx.exceptions import CommunicationError
 from xknx.knxip import (
     HPAI,
     CEMIFrame,
@@ -107,7 +108,7 @@ class Routing(Interface):
             )
             # close udp transport to prevent open file descriptors
             self.udp_transport.stop()
-            raise ex
+            raise CommunicationError("Routing could not be started") from ex
         await self.xknx.connection_manager.connection_state_changed(
             XknxConnectionState.CONNECTED
         )
diff --git a/xknx/io/tunnel.py b/xknx/io/tunnel.py
index 34b34578..6f8301ba 100644
--- a/xknx/io/tunnel.py
+++ b/xknx/io/tunnel.py
@@ -64,6 +64,7 @@ class _Tunnel(Interface):
         self.sequence_number = 0
         self.telegram_received_callback = telegram_received_callback
         self._data_endpoint_addr: tuple[str, int] | None = None
+        self._initial_connection = True
         self._is_reconnecting = False
         self._reconnect_task: asyncio.Task[None] | None = None
         self._src_address = xknx.own_address
@@ -109,12 +110,14 @@ class _Tunnel(Interface):
             await self.xknx.connection_manager.connection_state_changed(
                 XknxConnectionState.DISCONNECTED
             )
-            if self.auto_reconnect:
+            if not self._initial_connection and self.auto_reconnect:
                 self._reconnect_task = asyncio.create_task(self._reconnect())
                 return False
-            # close udp transport to prevent open file descriptors
+            # close transport to prevent open file descriptors
             self.transport.stop()
-            raise ex
+            raise CommunicationError(
+                "Tunnel connection could not be established"
+            ) from ex
         else:
             self._tunnel_established()
             await self.xknx.connection_manager.connection_state_changed(
@@ -124,6 +127,7 @@ class _Tunnel(Interface):
 
     def _tunnel_established(self) -> None:
         """Set up interface when the tunnel is ready."""
+        self._initial_connection = False
         self.sequence_number = 0
 
     def _tunnel_lost(self) -> None:


Test patch:
diff --git a/test/xknx_test.py b/test/xknx_test.py
index e22835c4..55fe8435 100644
--- a/test/xknx_test.py
+++ b/test/xknx_test.py
@@ -4,6 +4,7 @@ from unittest.mock import AsyncMock, patch
 
 import pytest
 from xknx import XKNX
+from xknx.exceptions import CommunicationError
 from xknx.io import ConnectionConfig, ConnectionType
 
 
@@ -94,3 +95,34 @@ class TestXknxModule:
         assert xknx.knxip_interface is None
         assert xknx.telegram_queue._consumer_task.done()
         assert not xknx.state_updater.started
+
+    @pytest.mark.parametrize(
+        "connection_config",
+        [
+            ConnectionConfig(
+                connection_type=ConnectionType.ROUTING, local_ip="127.0.0.1"
+            ),
+            ConnectionConfig(
+                connection_type=ConnectionType.TUNNELING, gateway_ip="127.0.0.2"
+            ),
+        ],
+    )
+    @patch(
+        "xknx.io.transport.UDPTransport.connect",
+        new_callable=AsyncMock,
+        side_effect=OSError,
+    )
+    async def test_xknx_start_initial_connection_error(
+        self, transport_connect_mock, connection_config
+    ):
+        """Test xknx start raising when socket can't be set up."""
+        xknx = XKNX(
+            state_updater=True,
+            connection_config=connection_config,
+        )
+        with pytest.raises(CommunicationError):
+            await xknx.start()
+        transport_connect_mock.assert_called_once()
+        assert xknx.telegram_queue._consumer_task is None  # not started
+        assert not xknx.state_updater.started
+        assert not xknx.started.is_set()