Repository: XKNX/xknx
Pull request #1206 (2023-02-26 22:28:39)

Add CEMIFrame counters and connection time and type
<!--
  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.
-->
Add CEMIFrame counters and timestamp of connection start and connection type to ConnectionManager.
These are resetet on (re)connection.

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)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] 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
- [x] The changes are documented in the changelog (docs/changelog.md)

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

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

```diff
@@           Coverage Diff           @@
##             main    #1206   +/-   ##
=======================================
  Coverage   96.44%   96.44%           
=======================================
  Files         143      143           
  Lines        8993     9012   +19     
=======================================
+ Hits         8673     8692   +19     
  Misses        320      320           
```


| [Impacted Files](https://codecov.io/gh/XKNX/xknx/pull/1206?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) | Coverage Δ | |
|---|---|---|
| [xknx/cemi/cemi\_handler.py](https://codecov.io/gh/XKNX/xknx/pull/1206?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9jZW1pL2NlbWlfaGFuZGxlci5weQ==) | `100.00% <100.00%> (ø)` | |
| [xknx/core/connection\_manager.py](https://codecov.io/gh/XKNX/xknx/pull/1206?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9jb3JlL2Nvbm5lY3Rpb25fbWFuYWdlci5weQ==) | `100.00% <100.00%> (ø)` | |
| [xknx/io/routing.py](https://codecov.io/gh/XKNX/xknx/pull/1206?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9pby9yb3V0aW5nLnB5) | `92.00% <0.00%> (-0.07%)` | :arrow_down: |
| [xknx/io/tunnel.py](https://codecov.io/gh/XKNX/xknx/pull/1206?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9pby90dW5uZWwucHk=) | `85.60% <0.00%> (-0.06%)` | :arrow_down: |


Patch:
diff --git a/docs/changelog.md b/docs/changelog.md
index dd966293..9a602029 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -11,6 +11,7 @@ nav_order: 2
 ### Connection
 
 - When `ConnectionConfig.individual_address` is set and a Keyring is given `ConnectionType.AUTOMATIC` will try to connect to the host of this address. If not found (in keyfile or discovery) it will raise.
+- Add CEMIFrame counters connection type and timestamp of connection start.
 
 ### Internals
 
diff --git a/xknx/cemi/cemi_handler.py b/xknx/cemi/cemi_handler.py
index f2cfbb4a..3a118817 100644
--- a/xknx/cemi/cemi_handler.py
+++ b/xknx/cemi/cemi_handler.py
@@ -69,6 +69,7 @@ class CEMIHandler:
             await self.xknx.knxip_interface.send_cemi(cemi)
         except (ConversionError, CommunicationError) as ex:
             logger.warning("Could not send CEMI frame: %s for %s", ex, cemi)
+            self.xknx.connection_manager.cemi_count_outgoing_error += 1
             raise ex
 
         try:
@@ -77,9 +78,11 @@ class CEMIHandler:
                 timeout=REQUEST_TO_CONFIRMATION_TIMEOUT,
             )
         except asyncio.TimeoutError:
+            self.xknx.connection_manager.cemi_count_outgoing_error += 1
             raise ConfirmationError(
                 f"L_DATA_CON Data Link Layer confirmation timed out for {cemi}"
             )
+        self.xknx.connection_manager.cemi_count_outgoing += 1
 
     def handle_raw_cemi(self, raw_cemi: bytes) -> None:
         """Parse and handle incoming raw CEMI Frames."""
@@ -87,6 +90,7 @@ class CEMIHandler:
             cemi = CEMIFrame.from_knx(raw_cemi)
         except UnsupportedCEMIMessage as unsupported_cemi_err:
             logger.info("CEMI not supported: %s", unsupported_cemi_err)
+            self.xknx.connection_manager.cemi_count_incoming_error += 1
             return
         self.handle_cemi_frame(cemi)
 
@@ -100,10 +104,12 @@ class CEMIHandler:
         if cemi.code is CEMIMessageCode.L_DATA_REQ:
             # L_DATA_REQ frames should only be outgoing.
             logger.warning("Received unexpected L_DATA_REQ frame: %s", cemi)
+            self.xknx.connection_manager.cemi_count_incoming_error += 1
             return
         if cemi.src_addr == self.xknx.current_address:
             # L_DATA_IND frames from our own address should be ignored (may occur form routing)
             logger.debug("Ignoring own CEMI: %s", cemi)
+            self.xknx.connection_manager.cemi_count_incoming_error += 1
             return
         logger.debug("Incoming CEMI: %s", cemi)
 
@@ -117,6 +123,7 @@ class CEMIHandler:
                     err,
                 )
                 return
+        self.xknx.connection_manager.cemi_count_incoming += 1
         # TODO: remove telegram init from CEMIFrame class and move it here?
         telegram = cemi.telegram
         telegram.direction = TelegramDirection.INCOMING
diff --git a/xknx/core/__init__.py b/xknx/core/__init__.py
index 1db2e6c8..2f8ab28c 100644
--- a/xknx/core/__init__.py
+++ b/xknx/core/__init__.py
@@ -1,7 +1,7 @@
 """Module for the automations and business logic of XKNX."""
 # flake8: noqa
 from .connection_manager import ConnectionManager
-from .connection_state import XknxConnectionState
+from .connection_state import XknxConnectionState, XknxConnectionType
 from .state_updater import StateUpdater
 from .task_registry import Task, TaskRegistry
 from .telegram_queue import TelegramQueue
diff --git a/xknx/core/connection_manager.py b/xknx/core/connection_manager.py
index 2606df45..1ac65bc9 100644
--- a/xknx/core/connection_manager.py
+++ b/xknx/core/connection_manager.py
@@ -3,9 +3,10 @@ from __future__ import annotations
 
 import asyncio
 from collections.abc import Awaitable
+from datetime import datetime
 from typing import Callable
 
-from xknx.core.connection_state import XknxConnectionState
+from xknx.core.connection_state import XknxConnectionState, XknxConnectionType
 
 AsyncConnectionStateCallback = Callable[[XknxConnectionState], Awaitable[None]]
 
@@ -21,6 +22,13 @@ class ConnectionManager:
         self._state = XknxConnectionState.DISCONNECTED
         self._connection_state_changed_cbs: list[AsyncConnectionStateCallback] = []
 
+        self.cemi_count_incoming: int = 0
+        self.cemi_count_incoming_error: int = 0
+        self.cemi_count_outgoing: int = 0
+        self.cemi_count_outgoing_error: int = 0
+        self.connected_since: datetime | None = None
+        self.connection_type: XknxConnectionType = XknxConnectionType.NOT_CONNECTED
+
     async def register_loop(self) -> None:
         """Register main loop to enable thread-safe `connection_state_changed` calls."""
         self._main_loop = asyncio.get_running_loop()
@@ -38,25 +46,34 @@ class ConnectionManager:
         if connection_state_changed_cb in self._connection_state_changed_cbs:
             self._connection_state_changed_cbs.remove(connection_state_changed_cb)
 
-    async def connection_state_changed(self, state: XknxConnectionState) -> None:
+    async def connection_state_changed(
+        self,
+        state: XknxConnectionState,
+        connection_type: XknxConnectionType = XknxConnectionType.NOT_CONNECTED,
+    ) -> None:
         """Run registered callbacks in main loop. Set internal state flag."""
         if self._main_loop:
             asyncio.run_coroutine_threadsafe(
-                self._connection_state_changed(state), self._main_loop
+                self._connection_state_changed(state, connection_type), self._main_loop
             )
         else:
-            await self._connection_state_changed(state)
+            await self._connection_state_changed(state, connection_type)
 
-    async def _connection_state_changed(self, state: XknxConnectionState) -> None:
+    async def _connection_state_changed(
+        self, state: XknxConnectionState, connection_type: XknxConnectionType
+    ) -> None:
         """Run registered callbacks. Set internal state flag."""
         if self._state == state:
             return
 
         self._state = state
+        self.connection_type = connection_type
         if state == XknxConnectionState.CONNECTED:
             self.connected.set()
+            self._reset_counters()
         else:
             self.connected.clear()
+            self.connected_since = None
 
         if tasks := [
             connection_state_change_cb(state)
@@ -68,3 +85,11 @@ class ConnectionManager:
     def state(self) -> XknxConnectionState:
         """Get current state."""
         return self._state
+
+    def _reset_counters(self) -> None:
+        """Reset counters."""
+        self.cemi_count_incoming = 0
+        self.cemi_count_incoming_error = 0
+        self.cemi_count_outgoing = 0
+        self.cemi_count_outgoing_error = 0
+        self.connected_since = datetime.now().astimezone()
diff --git a/xknx/core/connection_state.py b/xknx/core/connection_state.py
index 5cb084d7..0d27b141 100644
--- a/xknx/core/connection_state.py
+++ b/xknx/core/connection_state.py
@@ -8,3 +8,14 @@ class XknxConnectionState(Enum):
     CONNECTED = "CONNECTED"
     CONNECTING = "CONNECTING"
     DISCONNECTED = "DISCONNECTED"
+
+
+class XknxConnectionType(Enum):
+    """Possible connection type values."""
+
+    NOT_CONNECTED = None
+    ROUTING = "Routing"
+    ROUTING_SECURE = "Routing IP Secure"
+    TUNNEL_SECURE = "Tunnel IP Secure"
+    TUNNEL_TCP = "Tunnel TCP"
+    TUNNEL_UDP = "Tunnel UDP"
diff --git a/xknx/io/routing.py b/xknx/io/routing.py
index ad5010fa..8de44787 100644
--- a/xknx/io/routing.py
+++ b/xknx/io/routing.py
@@ -13,7 +13,7 @@ import random
 from typing import TYPE_CHECKING, Final
 
 from xknx.cemi import CEMIFrame, CEMIMessageCode
-from xknx.core import XknxConnectionState
+from xknx.core import XknxConnectionState, XknxConnectionType
 from xknx.exceptions import CommunicationError
 from xknx.knxip import (
     HPAI,
@@ -132,6 +132,7 @@ class _RoutingFlowControl:
 class Routing(Interface):
     """Class for handling KNXnet/IP multicast communication."""
 
+    connection_type = XknxConnectionType.ROUTING
     transport: UDPTransport
 
     def __init__(
@@ -180,7 +181,7 @@ class Routing(Interface):
         """Start routing."""
         self.xknx.current_address = self.individual_address
         await self.xknx.connection_manager.connection_state_changed(
-            XknxConnectionState.CONNECTING
+            XknxConnectionState.CONNECTING, self.connection_type
         )
         try:
             await self.transport.connect()
@@ -197,7 +198,7 @@ class Routing(Interface):
             self.transport.stop()
             raise CommunicationError("Routing could not be started") from ex
         await self.xknx.connection_manager.connection_state_changed(
-            XknxConnectionState.CONNECTED
+            XknxConnectionState.CONNECTED, self.connection_type
         )
         return True
 
@@ -262,6 +263,7 @@ class Routing(Interface):
 class SecureRouting(Routing):
     """Class for handling KNXnet/IP secure multicast communication."""
 
+    connection_type = XknxConnectionType.ROUTING_SECURE
     transport: SecureGroup
 
     def __init__(
diff --git a/xknx/io/tunnel.py b/xknx/io/tunnel.py
index 0dcab370..72c37fe1 100644
--- a/xknx/io/tunnel.py
+++ b/xknx/io/tunnel.py
@@ -11,7 +11,7 @@ import logging
 from typing import TYPE_CHECKING
 
 from xknx.cemi import CEMIFrame
-from xknx.core import XknxConnectionState
+from xknx.core import XknxConnectionState, XknxConnectionType
 from xknx.exceptions import CommunicationError, TunnellingAckError
 from xknx.knxip import (
     HPAI,
@@ -43,6 +43,7 @@ logger = logging.getLogger("xknx.log")
 class _Tunnel(Interface):
     """Class for handling KNX/IP tunnels."""
 
+    connection_type: XknxConnectionType
     transport: KNXIPTransport
 
     def __init__(
@@ -99,7 +100,7 @@ class _Tunnel(Interface):
     async def connect(self) -> bool:
         """Connect to a KNX tunneling interface. Returns True on success."""
         await self.xknx.connection_manager.connection_state_changed(
-            XknxConnectionState.CONNECTING
+            XknxConnectionState.CONNECTING, self.connection_type
         )
         try:
             await self.transport.connect()
@@ -125,7 +126,7 @@ class _Tunnel(Interface):
 
         self._tunnel_established()
         await self.xknx.connection_manager.connection_state_changed(
-            XknxConnectionState.CONNECTED
+            XknxConnectionState.CONNECTED, self.connection_type
         )
         return True
 
@@ -373,6 +374,7 @@ class _Tunnel(Interface):
 class UDPTunnel(_Tunnel):
     """Class for handling KNX/IP UDP tunnels."""
 
+    connection_type = XknxConnectionType.TUNNEL_UDP
     transport: UDPTransport
 
     def __init__(
@@ -544,6 +546,7 @@ class UDPTunnel(_Tunnel):
 class TCPTunnel(_Tunnel):
     """Class for handling KNX/IP TCP tunnels."""
 
+    connection_type = XknxConnectionType.TUNNEL_TCP
     transport: TCPTransport
 
     def __init__(
@@ -587,6 +590,7 @@ class TCPTunnel(_Tunnel):
 class SecureTunnel(TCPTunnel):
     """Class for handling KNX/IP secure TCP tunnels."""
 
+    connection_type = XknxConnectionType.TUNNEL_SECURE
     transport: SecureSession
 
     def __init__(


Test patch:
diff --git a/requirements/testing.txt b/requirements/testing.txt
index 26ca4a13..db4b4e77 100644
--- a/requirements/testing.txt
+++ b/requirements/testing.txt
@@ -1,5 +1,5 @@
 -r production.txt
-pre-commit==3.0.4
+pre-commit==3.1.0
 isort==5.12.0
 flake8==6.0.0
 flake8-isort==6.0.0
@@ -8,6 +8,6 @@ pylint==2.16.2
 pytest==7.2.1
 pytest-asyncio==0.20.3
 pytest-cov==4.0.0
-setuptools==67.3.2
-tox==4.4.5
+setuptools==67.4.0
+tox==4.4.6
 mypy==1.0.1
diff --git a/test/cemi_tests/cemi_handler_test.py b/test/cemi_tests/cemi_handler_test.py
index c6426f7e..31bda573 100644
--- a/test/cemi_tests/cemi_handler_test.py
+++ b/test/cemi_tests/cemi_handler_test.py
@@ -31,11 +31,15 @@ async def test_wait_for_l2_confirmation(time_travel):
     task = asyncio.create_task(xknx.cemi_handler.send_telegram(test_telegram))
     await time_travel(0)
     xknx.knxip_interface.send_cemi.assert_called_once_with(test_cemi)
+    assert xknx.connection_manager.cemi_count_outgoing == 0
+
     assert not task.done()
     xknx.cemi_handler.handle_cemi_frame(test_cemi_confirmation)
     await time_travel(0)
     assert task.done()
     await task
+    assert xknx.connection_manager.cemi_count_outgoing == 1
+    assert xknx.connection_manager.cemi_count_outgoing_error == 0
 
     # no L_DATA.con received -> raise ConfirmationError
     xknx.knxip_interface.send_cemi.reset_mock()
@@ -46,6 +50,8 @@ async def test_wait_for_l2_confirmation(time_travel):
         await time_travel(3)
         assert task.done()
         await task
+        assert xknx.connection_manager.cemi_count_outgoing == 1
+        assert xknx.connection_manager.cemi_count_outgoing_error == 1
 
 
 def test_incoming_cemi():
@@ -80,6 +86,7 @@ def test_incoming_cemi():
         )
         xknx.cemi_handler.handle_cemi_frame(test_incoming_l_data_req)
         mock_telegram_received.assert_not_called()
+        assert xknx.connection_manager.cemi_count_incoming == 1
 
 
 @pytest.mark.parametrize(
@@ -111,6 +118,7 @@ def test_incoming_management_telegram(telegram):
         xknx.cemi_handler.handle_cemi_frame(test_cemi)
         mock_management_process.assert_called_once()
         assert xknx.telegrams.qsize() == 0
+        assert xknx.connection_manager.cemi_count_incoming == 1
 
 
 @pytest.mark.parametrize(
@@ -134,6 +142,7 @@ def test_invalid_cemi(raw):
         xknx.cemi_handler.handle_raw_cemi(raw)
         mock_info.assert_called_once()
         mock_handle_cemi_frame.assert_not_called()
+        assert xknx.connection_manager.cemi_count_incoming_error == 1
 
 
 def test_incoming_from_self():
@@ -149,3 +158,4 @@ def test_incoming_from_self():
         xknx.cemi_handler.handle_raw_cemi(raw)
         mock_debug.assert_called_once()
         mock_telegram_received.assert_not_called()
+        assert xknx.connection_manager.cemi_count_incoming_error == 1
diff --git a/test/core_tests/connection_manager_test.py b/test/core_tests/connection_manager_test.py
index 3a4932a6..e48407aa 100644
--- a/test/core_tests/connection_manager_test.py
+++ b/test/core_tests/connection_manager_test.py
@@ -1,10 +1,11 @@
 """Unit test for connection manager."""
 import asyncio
+from datetime import datetime
 import threading
 from unittest.mock import AsyncMock, patch
 
 from xknx import XKNX
-from xknx.core import XknxConnectionState
+from xknx.core import XknxConnectionState, XknxConnectionType
 from xknx.io import ConnectionConfig
 
 
@@ -23,7 +24,7 @@ class TestConnectionManager:
             async_connection_state_changed_cb
         )
         await xknx.connection_manager.connection_state_changed(
-            XknxConnectionState.CONNECTED
+            XknxConnectionState.CONNECTED, XknxConnectionType.ROUTING_SECURE
         )
         async_connection_state_changed_cb.assert_called_once_with(
             XknxConnectionState.CONNECTED
@@ -109,3 +110,44 @@ class TestConnectionManager:
             # wait for side_effect to finish
             await asyncio.wait_for(xknx.connection_manager.connected.wait(), timeout=1)
             await xknx.stop()
+
+    async def test_connection_information(self):
+        """Test connection information."""
+        xknx = XKNX()
+
+        assert xknx.connection_manager.connected_since is None
+        assert (
+            xknx.connection_manager.connection_type is XknxConnectionType.NOT_CONNECTED
+        )
+        xknx.connection_manager.cemi_count_incoming = 5
+        xknx.connection_manager.cemi_count_incoming_error = 5
+        xknx.connection_manager.cemi_count_outgoing = 5
+        xknx.connection_manager.cemi_count_outgoing_error = 5
+
+        # reset counters on new connection
+        await xknx.connection_manager.connection_state_changed(
+            XknxConnectionState.CONNECTED, XknxConnectionType.TUNNEL_TCP
+        )
+        assert xknx.connection_manager.cemi_count_incoming == 0
+        assert xknx.connection_manager.cemi_count_incoming_error == 0
+        assert xknx.connection_manager.cemi_count_outgoing == 0
+        assert xknx.connection_manager.cemi_count_outgoing_error == 0
+        assert isinstance(xknx.connection_manager.connected_since, datetime)
+        assert xknx.connection_manager.connection_type is XknxConnectionType.TUNNEL_TCP
+
+        xknx.connection_manager.cemi_count_incoming = 5
+        xknx.connection_manager.cemi_count_incoming_error = 5
+        xknx.connection_manager.cemi_count_outgoing = 5
+        xknx.connection_manager.cemi_count_outgoing_error = 5
+        # keep values until new connection; set connection timestamp to None
+        await xknx.connection_manager.connection_state_changed(
+            XknxConnectionState.DISCONNECTED
+        )
+        assert xknx.connection_manager.cemi_count_incoming == 5
+        assert xknx.connection_manager.cemi_count_incoming_error == 5
+        assert xknx.connection_manager.cemi_count_outgoing == 5
+        assert xknx.connection_manager.cemi_count_outgoing_error == 5
+        assert xknx.connection_manager.connected_since is None
+        assert (
+            xknx.connection_manager.connection_type is XknxConnectionType.NOT_CONNECTED
+        )