From 7cba34b2e6b53621e6188ff2b473a3553e72bdcd Mon Sep 17 00:00:00 2001 From: jan iversen Date: Thu, 14 Mar 2024 23:19:52 +0100 Subject: [PATCH] Change modbus integration to use async library calls (#113450) --- homeassistant/components/modbus/modbus.py | 46 ++++++++++------------- tests/components/modbus/conftest.py | 14 ++++--- tests/components/modbus/test_cover.py | 2 +- tests/components/modbus/test_init.py | 4 +- 4 files changed, 31 insertions(+), 35 deletions(-) diff --git a/homeassistant/components/modbus/modbus.py b/homeassistant/components/modbus/modbus.py index d08e8b80577..f4947223662 100644 --- a/homeassistant/components/modbus/modbus.py +++ b/homeassistant/components/modbus/modbus.py @@ -8,8 +8,11 @@ from collections.abc import Callable import logging from typing import Any -from pymodbus.client import ModbusSerialClient, ModbusTcpClient, ModbusUdpClient -from pymodbus.client.base import ModbusBaseClient +from pymodbus.client import ( + AsyncModbusSerialClient, + AsyncModbusTcpClient, + AsyncModbusUdpClient, +) from pymodbus.exceptions import ModbusException from pymodbus.pdu import ModbusResponse from pymodbus.transaction import ModbusAsciiFramer, ModbusRtuFramer, ModbusSocketFramer @@ -275,7 +278,7 @@ class ModbusHub: else: client_config[CONF_RETRIES] = 3 # generic configuration - self._client: ModbusBaseClient | None = None + self._client: AsyncModbusSerialClient | AsyncModbusTcpClient | AsyncModbusUdpClient | None = None self._async_cancel_listener: Callable[[], None] | None = None self._in_error = False self._lock = asyncio.Lock() @@ -285,10 +288,10 @@ class ModbusHub: self._config_delay = client_config[CONF_DELAY] self._pb_request: dict[str, RunEntry] = {} self._pb_class = { - SERIAL: ModbusSerialClient, - TCP: ModbusTcpClient, - UDP: ModbusUdpClient, - RTUOVERTCP: ModbusTcpClient, + SERIAL: AsyncModbusSerialClient, + TCP: AsyncModbusTcpClient, + UDP: AsyncModbusUdpClient, + RTUOVERTCP: AsyncModbusTcpClient, } self._pb_params = { "port": client_config[CONF_PORT], @@ -336,9 +339,14 @@ class ModbusHub: async def async_pb_connect(self) -> None: """Connect to device, async.""" async with self._lock: - if not await self.hass.async_add_executor_job(self.pb_connect): - err = f"{self.name} connect failed, retry in pymodbus" + try: + await self._client.connect() # type: ignore[union-attr] + except ModbusException as exception_error: + err = f"{self.name} connect failed, retry in pymodbus ({str(exception_error)})" self._log_error(err, error_state=False) + return + message = f"modbus {self.name} communication open" + _LOGGER.info(message) async def async_setup(self) -> bool: """Set up pymodbus client.""" @@ -392,26 +400,14 @@ class ModbusHub: message = f"modbus {self.name} communication closed" _LOGGER.warning(message) - def pb_connect(self) -> bool: - """Connect client.""" - try: - self._client.connect() # type: ignore[union-attr] - except ModbusException as exception_error: - self._log_error(str(exception_error), error_state=False) - return False - - message = f"modbus {self.name} communication open" - _LOGGER.info(message) - return True - - def pb_call( + async def low_level_pb_call( self, slave: int | None, address: int, value: int | list[int], use_call: str ) -> ModbusResponse | None: """Call sync. pymodbus.""" kwargs = {"slave": slave} if slave else {} entry = self._pb_request[use_call] try: - result: ModbusResponse = entry.func(address, value, **kwargs) + result: ModbusResponse = await entry.func(address, value, **kwargs) except ModbusException as exception_error: error = ( f"Error: device: {slave} address: {address} -> {str(exception_error)}" @@ -448,9 +444,7 @@ class ModbusHub: async with self._lock: if not self._client: return None - result = await self.hass.async_add_executor_job( - self.pb_call, unit, address, value, use_call - ) + result = await self.low_level_pb_call(unit, address, value, use_call) if self._msg_wait: # small delay until next request/response await asyncio.sleep(self._msg_wait) diff --git a/tests/components/modbus/conftest.py b/tests/components/modbus/conftest.py index e470f30fe82..2a81fa3d4fb 100644 --- a/tests/components/modbus/conftest.py +++ b/tests/components/modbus/conftest.py @@ -50,17 +50,18 @@ class ReadResult: @pytest.fixture(name="mock_pymodbus") def mock_pymodbus_fixture(): """Mock pymodbus.""" - mock_pb = mock.MagicMock() + mock_pb = mock.AsyncMock() + mock_pb.close = mock.MagicMock() with mock.patch( - "homeassistant.components.modbus.modbus.ModbusTcpClient", + "homeassistant.components.modbus.modbus.AsyncModbusTcpClient", return_value=mock_pb, autospec=True, ), mock.patch( - "homeassistant.components.modbus.modbus.ModbusSerialClient", + "homeassistant.components.modbus.modbus.AsyncModbusSerialClient", return_value=mock_pb, autospec=True, ), mock.patch( - "homeassistant.components.modbus.modbus.ModbusUdpClient", + "homeassistant.components.modbus.modbus.AsyncModbusUdpClient", return_value=mock_pb, autospec=True, ): @@ -118,9 +119,10 @@ async def mock_modbus_fixture( } ] } - mock_pb = mock.MagicMock() + mock_pb = mock.AsyncMock() + mock_pb.close = mock.MagicMock() with mock.patch( - "homeassistant.components.modbus.modbus.ModbusTcpClient", + "homeassistant.components.modbus.modbus.AsyncModbusTcpClient", return_value=mock_pb, autospec=True, ): diff --git a/tests/components/modbus/test_cover.py b/tests/components/modbus/test_cover.py index 32c79fb4dff..fa9e617d96d 100644 --- a/tests/components/modbus/test_cover.py +++ b/tests/components/modbus/test_cover.py @@ -270,7 +270,7 @@ async def test_service_cover_move(hass: HomeAssistant, mock_modbus, mock_ha) -> ) assert hass.states.get(ENTITY_ID).state == STATE_CLOSED - mock_modbus.reset() + await mock_modbus.reset() mock_modbus.read_holding_registers.side_effect = ModbusException("fail write_") await hass.services.async_call( "cover", "close_cover", {"entity_id": ENTITY_ID}, blocking=True diff --git a/tests/components/modbus/test_init.py b/tests/components/modbus/test_init.py index 3e0e94f4076..e55b9f4232d 100644 --- a/tests/components/modbus/test_init.py +++ b/tests/components/modbus/test_init.py @@ -1329,7 +1329,7 @@ async def test_pymodbus_constructor_fail( ] } with mock.patch( - "homeassistant.components.modbus.modbus.ModbusTcpClient", autospec=True + "homeassistant.components.modbus.modbus.AsyncModbusTcpClient", autospec=True ) as mock_pb: caplog.set_level(logging.ERROR) mock_pb.side_effect = ModbusException("test no class") @@ -1529,7 +1529,7 @@ async def test_stop_restart( async def test_write_no_client(hass: HomeAssistant, mock_modbus) -> None: """Run test for service stop and write without client.""" - mock_modbus.reset() + await mock_modbus.reset() data = { ATTR_HUB: TEST_MODBUS_NAME, }