diff --git a/homeassistant/components/rfxtrx/__init__.py b/homeassistant/components/rfxtrx/__init__.py index 8ddd5ffba4c..0f3988442c7 100644 --- a/homeassistant/components/rfxtrx/__init__.py +++ b/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) - except TimeoutError: - # Library currently doesn't support reload - _LOGGER.error( - "Connection timeout: failed to receive response from RFXtrx device" - ) - return False - + await async_setup_internal(hass, entry) 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( - (config[CONF_HOST], config[CONF_PORT]), - None, - transport_protocol=rfxtrxmod.PyNetworkTransport, - modes=modes, - ) + transport = rfxtrxmod.PyNetworkTransport((config[CONF_HOST], config[CONF_PORT])) else: - rfx = rfxtrxmod.Connect( - config[CONF_DEVICE], - None, - modes=modes, - ) + transport = rfxtrxmod.PySerialTransport(config[CONF_DEVICE]) + + rfx = rfxtrxmod.Connect( + 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] diff --git a/homeassistant/components/rfxtrx/config_flow.py b/homeassistant/components/rfxtrx/config_flow.py index fe6aaf07d40..8f6ff45840c 100644 --- a/homeassistant/components/rfxtrx/config_flow.py +++ b/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)) - except OSError: - return False - - conn.close() + conn = rfxtrxmod.PyNetworkTransport((host, port)) else: - try: - conn = rfxtrxmod.PySerialTransport(device) - except serial.SerialException: - return False + conn = rfxtrxmod.PySerialTransport(device) - if conn.serial is None: - return False - - conn.close() + try: + conn.connect() + except (rfxtrxmod.RFXtrxTransportError, TimeoutError): + return False return True diff --git a/homeassistant/components/rfxtrx/manifest.json b/homeassistant/components/rfxtrx/manifest.json index 1e2a3d6da27..ec902855f27 100644 --- a/homeassistant/components/rfxtrx/manifest.json +++ b/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"] } diff --git a/requirements_all.txt b/requirements_all.txt index 7c75cc5fbb5..ce34cc35fe0 100644 --- a/requirements_all.txt +++ b/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 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 94d205526e2..7c546d4dfcb 100644 --- a/requirements_test_all.txt +++ b/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 diff --git a/tests/components/rfxtrx/conftest.py b/tests/components/rfxtrx/conftest.py index 1b63068e73c..6ca7bdd7250 100644 --- a/tests/components/rfxtrx/conftest.py +++ b/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, - "port": None, + "host": host, + "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 = connect.return_value + rfx = Mock(spec=Connect) - async def _signal_event(packet_id): - event = rfxtrx.get_rfx_object(packet_id) - await hass.async_add_executor_job( - rfx.event_callback, - event, - ) + def _init(transport, event_callback=None, modes=None): + rfx.event_callback = event_callback + rfx.transport = transport + return rfx - await hass.async_block_till_done() - await hass.async_block_till_done() - return event + connect_mock.side_effect = _init - 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") diff --git a/tests/components/rfxtrx/test_config_flow.py b/tests/components/rfxtrx/test_config_flow.py index e5a5c73de39..e527b5dfb7b 100644 --- a/tests/components/rfxtrx/test_config_flow.py +++ b/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( - "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: +async def test_setup_serial(com_mock, transport_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( - "homeassistant.components.rfxtrx.rfxtrxmod.PySerialTransport.connect", - serial_connect, -) -@patch( - "homeassistant.components.rfxtrx.rfxtrxmod.PySerialTransport.close", - return_value=None, -) -async def test_setup_serial_manual(com_mock, connect_mock, hass: HomeAssistant) -> None: +async def test_setup_serial_manual( + com_mock, transport_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( - "homeassistant.components.rfxtrx.rfxtrxmod.PySerialTransport.connect", - side_effect=serial.SerialException, -) -async def test_setup_serial_fail(com_mock, connect_mock, hass: HomeAssistant) -> None: +async def test_setup_serial_fail(com_mock, transport_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( - "homeassistant.components.rfxtrx.rfxtrxmod.PySerialTransport.connect", - serial_connect_fail, -) -async def test_setup_serial_manual_fail(com_mock, hass: HomeAssistant) -> None: +async def test_setup_serial_manual_fail( + com_mock, transport_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} ) diff --git a/tests/components/rfxtrx/test_init.py b/tests/components/rfxtrx/test_init.py index 88e5dbfaf31..0fd7150e8fe 100644 --- a/tests/components/rfxtrx/test_init.py +++ b/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: - 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) + assert config_entry.state is ConfigEntryState.LOADED -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) - mock_entry = MockConfigEntry(domain="rfxtrx", unique_id=DOMAIN, data=entry_data) + config_entry = await setup_rfx_test_cfg( + 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