Update rfxtrx library to handle connection retries (#111366)

Upgrade rfxtrx library to one that leaves reconnection to home assistant and handle loss of connection by reloading the integration.

https://github.com/Danielhiversen/pyRFXtrx/releases/tag/0.31.0

homeassistant/components/rfxtrx/__init__.py

@@ -1,7 +1,6 @@
 """Support for RFXtrx devices."""
 from __future__ import annotations
 
-import asyncio
 import binascii
 from collections.abc import Callable, Mapping
 import copy
@@ -23,6 +22,7 @@ from homeassistant.const import (
     Platform,
 )
 from homeassistant.core import Event, HomeAssistant, ServiceCall, callback
+from homeassistant.exceptions import ConfigEntryNotReady
 from homeassistant.helpers import config_validation as cv, device_registry as dr
 from homeassistant.helpers.device_registry import DeviceInfo
 from homeassistant.helpers.dispatcher import (
@@ -49,6 +49,7 @@ from .const import (
 DEFAULT_OFF_DELAY = 2.0
 
 SIGNAL_EVENT = f"{DOMAIN}_event"
+CONNECT_TIMEOUT = 30.0
 
 _Ts = TypeVarTuple("_Ts")
 
@@ -89,15 +90,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     """Set up the RFXtrx component."""
     hass.data.setdefault(DOMAIN, {})
 
-    try:
+    await async_setup_internal(hass, entry)
-        await async_setup_internal(hass, entry)
-    except TimeoutError:
-        # Library currently doesn't support reload
-        _LOGGER.error(
-            "Connection timeout: failed to receive response from RFXtrx device"
-        )
-        return False
-
     await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS)
 
     return True
@@ -118,7 +111,9 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     return True
 
 
-def _create_rfx(config: Mapping[str, Any]) -> rfxtrxmod.Connect:
+def _create_rfx(
+    config: Mapping[str, Any], event_callback: Callable[[rfxtrxmod.RFXtrxEvent], None]
+) -> rfxtrxmod.Connect:
     """Construct a rfx object based on config."""
 
     modes = config.get(CONF_PROTOCOLS)
@@ -130,18 +125,22 @@ def _create_rfx(config: Mapping[str, Any]) -> rfxtrxmod.Connect:
 
     if config[CONF_PORT] is not None:
         # If port is set then we create a TCP connection
-        rfx = rfxtrxmod.Connect(
+        transport = rfxtrxmod.PyNetworkTransport((config[CONF_HOST], config[CONF_PORT]))
-            (config[CONF_HOST], config[CONF_PORT]),
-            None,
-            transport_protocol=rfxtrxmod.PyNetworkTransport,
-            modes=modes,
-        )
     else:
-        rfx = rfxtrxmod.Connect(
+        transport = rfxtrxmod.PySerialTransport(config[CONF_DEVICE])
-            config[CONF_DEVICE],
+
-            None,
+    rfx = rfxtrxmod.Connect(
-            modes=modes,
+        transport,
-        )
+        event_callback,
+        modes=modes,
+    )
+
+    try:
+        rfx.connect(CONNECT_TIMEOUT)
+    except TimeoutError as exc:
+        raise ConfigEntryNotReady("Timeout on connect") from exc
+    except rfxtrxmod.RFXtrxTransportError as exc:
+        raise ConfigEntryNotReady(str(exc)) from exc
 
     return rfx
 
@@ -165,10 +164,6 @@ async def async_setup_internal(hass: HomeAssistant, entry: ConfigEntry) -> None:
     """Set up the RFXtrx component."""
     config = entry.data
 
-    # Initialize library
-    async with asyncio.timeout(30):
-        rfx_object = await hass.async_add_executor_job(_create_rfx, config)
-
     # Setup some per device config
     devices = _get_device_lookup(config[CONF_DEVICES])
     pt2262_devices: set[str] = set()
@@ -179,8 +174,16 @@ async def async_setup_internal(hass: HomeAssistant, entry: ConfigEntry) -> None:
     @callback
     def async_handle_receive(event: rfxtrxmod.RFXtrxEvent) -> None:
         """Handle received messages from RFXtrx gateway."""
-        # Log RFXCOM event
+
-        if not event.device.id_string:
+        if isinstance(event, rfxtrxmod.ConnectionLost):
+            _LOGGER.warning("Connection was lost, triggering reload")
+            hass.async_create_task(
+                hass.config_entries.async_reload(entry.entry_id),
+                f"config entry reload {entry.title} {entry.domain} {entry.entry_id}",
+            )
+            return
+
+        if not event.device or not event.device.id_string:
             return
 
         event_data = {
@@ -264,6 +267,13 @@ async def async_setup_internal(hass: HomeAssistant, entry: ConfigEntry) -> None:
         if device_id:
             _remove_device(device_id)
 
+    # Initialize library
+    rfx_object = await hass.async_add_executor_job(
+        _create_rfx, config, lambda event: hass.add_job(async_handle_receive, event)
+    )
+
+    hass.data[DOMAIN][DATA_RFXOBJECT] = rfx_object
+
     entry.async_on_unload(
         hass.bus.async_listen(dr.EVENT_DEVICE_REGISTRY_UPDATED, _updated_device)
     )
@@ -275,9 +285,6 @@ async def async_setup_internal(hass: HomeAssistant, entry: ConfigEntry) -> None:
     entry.async_on_unload(
         hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, _shutdown_rfxtrx)
     )
-    hass.data[DOMAIN][DATA_RFXOBJECT] = rfx_object
-
-    rfx_object.event_callback = lambda event: hass.add_job(async_handle_receive, event)
 
     def send(call: ServiceCall) -> None:
         event = call.data[ATTR_EVENT]

homeassistant/components/rfxtrx/config_flow.py

@@ -634,22 +634,14 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
 def _test_transport(host: str | None, port: int | None, device: str | None) -> bool:
     """Construct a rfx object based on config."""
     if port is not None:
-        try:
+        conn = rfxtrxmod.PyNetworkTransport((host, port))
-            conn = rfxtrxmod.PyNetworkTransport((host, port))
-        except OSError:
-            return False
-
-        conn.close()
     else:
-        try:
+        conn = rfxtrxmod.PySerialTransport(device)
-            conn = rfxtrxmod.PySerialTransport(device)
-        except serial.SerialException:
-            return False
 
-        if conn.serial is None:
+    try:
-            return False
+        conn.connect()
-
+    except (rfxtrxmod.RFXtrxTransportError, TimeoutError):
-        conn.close()
+        return False
 
     return True
 

homeassistant/components/rfxtrx/manifest.json

@@ -6,5 +6,5 @@
   "documentation": "https://www.home-assistant.io/integrations/rfxtrx",
   "iot_class": "local_push",
   "loggers": ["RFXtrx"],
-  "requirements": ["pyRFXtrx==0.30.1"]
+  "requirements": ["pyRFXtrx==0.31.0"]
 }

requirements_all.txt

@@ -1658,7 +1658,7 @@ pyEmby==1.9
 pyHik==0.3.2
 
 # homeassistant.components.rfxtrx
-pyRFXtrx==0.30.1
+pyRFXtrx==0.31.0
 
 # homeassistant.components.sony_projector
 pySDCP==1

requirements_test_all.txt

@@ -1296,7 +1296,7 @@ pyDuotecno==2024.1.2
 pyElectra==1.2.0
 
 # homeassistant.components.rfxtrx
-pyRFXtrx==0.30.1
+pyRFXtrx==0.31.0
 
 # homeassistant.components.tibber
 pyTibber==0.28.2

tests/components/rfxtrx/conftest.py

@@ -1,10 +1,11 @@
 """Common test tools."""
 from __future__ import annotations
 
-from unittest.mock import patch
+from unittest.mock import Mock, patch
 
 from freezegun import freeze_time
 import pytest
+from RFXtrx import Connect, RFXtrxTransport
 
 from homeassistant.components import rfxtrx
 from homeassistant.components.rfxtrx import DOMAIN
@@ -15,26 +16,42 @@ from tests.components.light.conftest import mock_light_profiles  # noqa: F401
 
 
 def create_rfx_test_cfg(
-    device="abcd", automatic_add=False, protocols=None, devices=None
+    device="abcd",
+    automatic_add=False,
+    protocols=None,
+    devices=None,
+    host=None,
+    port=None,
 ):
     """Create rfxtrx config entry data."""
     return {
         "device": device,
-        "host": None,
+        "host": host,
-        "port": None,
+        "port": port,
         "automatic_add": automatic_add,
         "protocols": protocols,
         "debug": False,
-        "devices": devices,
+        "devices": devices or {},
     }
 
 
 async def setup_rfx_test_cfg(
-    hass, device="abcd", automatic_add=False, devices: dict[str, dict] | None = None
+    hass,
+    device="abcd",
+    automatic_add=False,
+    devices: dict[str, dict] | None = None,
+    protocols=None,
+    host=None,
+    port=None,
 ):
     """Construct a rfxtrx config entry."""
     entry_data = create_rfx_test_cfg(
-        device=device, automatic_add=automatic_add, devices=devices
+        device=device,
+        automatic_add=automatic_add,
+        devices=devices,
+        protocols=protocols,
+        host=host,
+        port=port,
     )
     mock_entry = MockConfigEntry(domain="rfxtrx", unique_id=DOMAIN, data=entry_data)
     mock_entry.supports_remove_device = True
@@ -46,27 +63,50 @@ async def setup_rfx_test_cfg(
     return mock_entry
 
 
+@pytest.fixture(autouse=True)
+async def transport_mock(hass):
+    """Fixture that make sure all transports are fake."""
+    transport = Mock(spec=RFXtrxTransport)
+    with patch("RFXtrx.PySerialTransport", new=transport), patch(
+        "RFXtrx.PyNetworkTransport", transport
+    ):
+        yield transport
+
+
+@pytest.fixture(autouse=True)
+async def connect_mock(hass):
+    """Fixture that make sure connect class is mocked."""
+    with patch("RFXtrx.Connect") as connect:
+        yield connect
+
+
 @pytest.fixture(autouse=True, name="rfxtrx")
-async def rfxtrx_fixture(hass):
+def rfxtrx_fixture(hass, connect_mock):
     """Fixture that cleans up threads from integration."""
 
-    with patch("RFXtrx.Connect") as connect, patch("RFXtrx.DummyTransport2"):
+    rfx = Mock(spec=Connect)
-        rfx = connect.return_value
 
-        async def _signal_event(packet_id):
+    def _init(transport, event_callback=None, modes=None):
-            event = rfxtrx.get_rfx_object(packet_id)
+        rfx.event_callback = event_callback
-            await hass.async_add_executor_job(
+        rfx.transport = transport
-                rfx.event_callback,
+        return rfx
-                event,
-            )
 
-            await hass.async_block_till_done()
+    connect_mock.side_effect = _init
-            await hass.async_block_till_done()
-            return event
 
-        rfx.signal = _signal_event
+    async def _signal_event(packet_id):
+        event = rfxtrx.get_rfx_object(packet_id)
+        await hass.async_add_executor_job(
+            rfx.event_callback,
+            event,
+        )
 
-        yield rfx
+        await hass.async_block_till_done()
+        await hass.async_block_till_done()
+        return event
+
+    rfx.signal = _signal_event
+
+    return rfx
 
 
 @pytest.fixture(name="rfxtrx_automatic")

tests/components/rfxtrx/test_config_flow.py

@@ -2,6 +2,7 @@
 import os
 from unittest.mock import MagicMock, patch, sentinel
 
+from RFXtrx import RFXtrxTransportError
 import serial.tools.list_ports
 
 from homeassistant import config_entries, data_entry_flow
@@ -15,16 +16,6 @@ from tests.common import MockConfigEntry
 SOME_PROTOCOLS = ["ac", "arc"]
 
 
-def serial_connect(self):
-    """Mock a serial connection."""
-    self.serial = True
-
-
-def serial_connect_fail(self):
-    """Mock a failed serial connection."""
-    self.serial = None
-
-
 def com_port():
     """Mock of a serial port."""
     port = serial.tools.list_ports_common.ListPortInfo("/dev/ttyUSB1234")
@@ -46,7 +37,6 @@ async def start_options_flow(hass, entry):
     return await hass.config_entries.options.async_init(entry.entry_id)
 
 
-@patch("homeassistant.components.rfxtrx.rfxtrxmod.PyNetworkTransport", autospec=True)
 async def test_setup_network(transport_mock, hass: HomeAssistant) -> None:
     """Test we can setup network."""
     result = await hass.config_entries.flow.async_init(
@@ -83,15 +73,7 @@ async def test_setup_network(transport_mock, hass: HomeAssistant) -> None:
 
 
 @patch("serial.tools.list_ports.comports", return_value=[com_port()])
-@patch(
+async def test_setup_serial(com_mock, transport_mock, hass: HomeAssistant) -> None:
-    "homeassistant.components.rfxtrx.rfxtrxmod.PySerialTransport.connect",
-    serial_connect,
-)
-@patch(
-    "homeassistant.components.rfxtrx.rfxtrxmod.PySerialTransport.close",
-    return_value=None,
-)
-async def test_setup_serial(com_mock, connect_mock, hass: HomeAssistant) -> None:
     """Test we can setup serial."""
     port = com_port()
 
@@ -129,15 +111,9 @@ async def test_setup_serial(com_mock, connect_mock, hass: HomeAssistant) -> None
 
 
 @patch("serial.tools.list_ports.comports", return_value=[com_port()])
-@patch(
+async def test_setup_serial_manual(
-    "homeassistant.components.rfxtrx.rfxtrxmod.PySerialTransport.connect",
+    com_mock, transport_mock, hass: HomeAssistant
-    serial_connect,
+) -> None:
-)
-@patch(
-    "homeassistant.components.rfxtrx.rfxtrxmod.PySerialTransport.close",
-    return_value=None,
-)
-async def test_setup_serial_manual(com_mock, connect_mock, hass: HomeAssistant) -> None:
     """Test we can setup serial with manual entry."""
     result = await hass.config_entries.flow.async_init(
         DOMAIN, context={"source": config_entries.SOURCE_USER}
@@ -180,13 +156,9 @@ async def test_setup_serial_manual(com_mock, connect_mock, hass: HomeAssistant)
     }
 
 
-@patch(
-    "homeassistant.components.rfxtrx.rfxtrxmod.PyNetworkTransport",
-    autospec=True,
-    side_effect=OSError,
-)
 async def test_setup_network_fail(transport_mock, hass: HomeAssistant) -> None:
     """Test we can setup network."""
+    transport_mock.return_value.connect.side_effect = RFXtrxTransportError
     result = await hass.config_entries.flow.async_init(
         DOMAIN, context={"source": config_entries.SOURCE_USER}
     )
@@ -214,12 +186,9 @@ async def test_setup_network_fail(transport_mock, hass: HomeAssistant) -> None:
 
 
 @patch("serial.tools.list_ports.comports", return_value=[com_port()])
-@patch(
+async def test_setup_serial_fail(com_mock, transport_mock, hass: HomeAssistant) -> None:
-    "homeassistant.components.rfxtrx.rfxtrxmod.PySerialTransport.connect",
-    side_effect=serial.SerialException,
-)
-async def test_setup_serial_fail(com_mock, connect_mock, hass: HomeAssistant) -> None:
     """Test setup serial failed connection."""
+    transport_mock.return_value.connect.side_effect = RFXtrxTransportError
     port = com_port()
 
     result = await hass.config_entries.flow.async_init(
@@ -249,12 +218,11 @@ async def test_setup_serial_fail(com_mock, connect_mock, hass: HomeAssistant) ->
 
 
 @patch("serial.tools.list_ports.comports", return_value=[com_port()])
-@patch(
+async def test_setup_serial_manual_fail(
-    "homeassistant.components.rfxtrx.rfxtrxmod.PySerialTransport.connect",
+    com_mock, transport_mock, hass: HomeAssistant
-    serial_connect_fail,
+) -> None:
-)
-async def test_setup_serial_manual_fail(com_mock, hass: HomeAssistant) -> None:
     """Test setup serial failed connection."""
+    transport_mock.return_value.connect.side_effect = RFXtrxTransportError
     result = await hass.config_entries.flow.async_init(
         DOMAIN, context={"source": config_entries.SOURCE_USER}
     )

tests/components/rfxtrx/test_init.py

@@ -1,19 +1,18 @@
 """The tests for the Rfxtrx component."""
 from __future__ import annotations
 
-from unittest.mock import ANY, call, patch
+from unittest.mock import ANY, call
 
 import RFXtrx as rfxtrxmod
 
-from homeassistant.components.rfxtrx import DOMAIN
 from homeassistant.components.rfxtrx.const import EVENT_RFXTRX_EVENT
+from homeassistant.config_entries import ConfigEntryState
 from homeassistant.core import HomeAssistant, callback
 from homeassistant.helpers import device_registry as dr
 from homeassistant.setup import async_setup_component
 
-from .conftest import create_rfx_test_cfg, setup_rfx_test_cfg
+from .conftest import setup_rfx_test_cfg
 
-from tests.common import MockConfigEntry
 from tests.typing import WebSocketGenerator
 
 SOME_PROTOCOLS = ["ac", "arc"]
@@ -130,29 +129,89 @@ async def test_ws_device_remove(
     assert mock_entry.data["devices"] == {}
 
 
-async def test_connect(hass: HomeAssistant) -> None:
+async def test_connect(
+    rfxtrx, connect_mock, transport_mock, hass: HomeAssistant
+) -> None:
     """Test that we attempt to connect to the device."""
-    entry_data = create_rfx_test_cfg(device="/dev/ttyUSBfake")
-    mock_entry = MockConfigEntry(domain="rfxtrx", unique_id=DOMAIN, data=entry_data)
 
-    mock_entry.add_to_hass(hass)
+    config_entry = await setup_rfx_test_cfg(hass, device="/dev/ttyUSBfake")
+    transport_mock.assert_called_once_with("/dev/ttyUSBfake")
+    connect_mock.assert_called_once_with(transport_mock.return_value, ANY, modes=ANY)
+    rfxtrx.connect.assert_called_once_with(ANY)
 
-    with patch.object(rfxtrxmod, "Connect") as connect:
+    assert config_entry.state is ConfigEntryState.LOADED
-        await hass.config_entries.async_setup(mock_entry.entry_id)
-        await hass.async_block_till_done()
-
-    connect.assert_called_once_with("/dev/ttyUSBfake", ANY, modes=ANY)
 
 
-async def test_connect_with_protocols(hass: HomeAssistant) -> None:
+async def test_connect_network(
+    rfxtrx, connect_mock, transport_mock, hass: HomeAssistant
+) -> None:
+    """Test that we attempt to connect to the device."""
+
+    config_entry = await setup_rfx_test_cfg(hass, host="localhost", port=1234)
+    transport_mock.assert_called_once_with(("localhost", 1234))
+    connect_mock.assert_called_once_with(transport_mock.return_value, ANY, modes=ANY)
+    rfxtrx.connect.assert_called_once_with(ANY)
+
+    assert config_entry.state is ConfigEntryState.LOADED
+
+
+async def test_connect_with_protocols(
+    rfxtrx, connect_mock, transport_mock, hass: HomeAssistant
+) -> None:
     """Test that we attempt to set protocols."""
-    entry_data = create_rfx_test_cfg(device="/dev/ttyUSBfake", protocols=SOME_PROTOCOLS)
+    config_entry = await setup_rfx_test_cfg(
-    mock_entry = MockConfigEntry(domain="rfxtrx", unique_id=DOMAIN, data=entry_data)
+        hass, device="/dev/ttyUSBfake", protocols=SOME_PROTOCOLS
+    )
+    transport_mock.assert_called_once_with("/dev/ttyUSBfake")
+    connect_mock.assert_called_once_with(
+        transport_mock.return_value, ANY, modes=SOME_PROTOCOLS
+    )
+    rfxtrx.connect.assert_called_once_with(ANY)
 
-    mock_entry.add_to_hass(hass)
+    assert config_entry.state is ConfigEntryState.LOADED
 
-    with patch.object(rfxtrxmod, "Connect") as connect:
-        await hass.config_entries.async_setup(mock_entry.entry_id)
-        await hass.async_block_till_done()
 
-    connect.assert_called_once_with("/dev/ttyUSBfake", ANY, modes=SOME_PROTOCOLS)
+async def test_connect_timeout(
+    rfxtrx, connect_mock, transport_mock, hass: HomeAssistant
+) -> None:
+    """Test that we attempt to connect to the device."""
+
+    rfxtrx.connect.side_effect = TimeoutError
+
+    config_entry = await setup_rfx_test_cfg(hass, device="/dev/ttyUSBfake")
+    transport_mock.assert_called_once_with("/dev/ttyUSBfake")
+    connect_mock.assert_called_once_with(transport_mock.return_value, ANY, modes=ANY)
+    rfxtrx.connect.assert_called_once_with(ANY)
+
+    assert config_entry.state is ConfigEntryState.SETUP_RETRY
+
+
+async def test_connect_failed(
+    rfxtrx, connect_mock, transport_mock, hass: HomeAssistant
+) -> None:
+    """Test that we attempt to connect to the device."""
+
+    rfxtrx.connect.side_effect = rfxtrxmod.RFXtrxTransportError
+
+    config_entry = await setup_rfx_test_cfg(hass, device="/dev/ttyUSBfake")
+    transport_mock.assert_called_once_with("/dev/ttyUSBfake")
+    connect_mock.assert_called_once_with(transport_mock.return_value, ANY, modes=ANY)
+    rfxtrx.connect.assert_called_once_with(ANY)
+
+    assert config_entry.state is ConfigEntryState.SETUP_RETRY
+
+
+async def test_reconnect(rfxtrx, hass: HomeAssistant) -> None:
+    """Test that we reconnect on connection loss."""
+    config_entry = await setup_rfx_test_cfg(hass, device="/dev/ttyUSBfake")
+
+    assert config_entry.state is ConfigEntryState.LOADED
+    rfxtrx.connect.call_count = 1
+
+    await hass.async_add_executor_job(
+        rfxtrx.event_callback,
+        rfxtrxmod.ConnectionLost(),
+    )
+
+    assert config_entry.state is ConfigEntryState.LOADED
+    rfxtrx.connect.call_count = 2