From eedfca6623f8bf46215cb866ae76a92126fed9f7 Mon Sep 17 00:00:00 2001 From: jan iversen Date: Sun, 12 Apr 2020 19:55:03 +0200 Subject: [PATCH] Fix modbus sync/async issues (#34043) * add pyserial to manifest pymodbus is very developer oriented and assumes every developer adapt the requierements.txt to his/hers needs. Our requirements.txt is different it contains all posibilities allowing user to later change configuration without having to install extra packages. As a consequence manifest.json needs to include the pyserial. * modbus: make truly async client creation Make hass call listen_once async. Integrate content of start_modbus into async_setup. Do not use the boiler plate create tcp client function from pymodbus as it is sync, and also does not work well with asyncio, instead call the init_ directly, since that is async. * both component/modbus and component/serial uses pyserial-async but with slighty different version requirements. Combined the 2. * Review 1 * Review 2 * Review @staticmethod is no good, because the function uses class variables. * Review Pytest is sometimes a bit cryptic, lets hope this does it. --- homeassistant/components/modbus/__init__.py | 99 +++++++++++-------- homeassistant/components/modbus/manifest.json | 3 +- requirements_all.txt | 1 + requirements_test_all.txt | 4 + 4 files changed, 62 insertions(+), 45 deletions(-) diff --git a/homeassistant/components/modbus/__init__.py b/homeassistant/components/modbus/__init__.py index 1e889043fae..ad0330b56a0 100644 --- a/homeassistant/components/modbus/__init__.py +++ b/homeassistant/components/modbus/__init__.py @@ -3,13 +3,21 @@ import asyncio import logging from async_timeout import timeout -from pymodbus.client.asynchronous import schedulers -from pymodbus.client.asynchronous.serial import AsyncModbusSerialClient as ClientSerial -from pymodbus.client.asynchronous.tcp import AsyncModbusTCPClient as ClientTCP -from pymodbus.client.asynchronous.udp import AsyncModbusUDPClient as ClientUDP +from pymodbus.client.asynchronous.asyncio import ( + AsyncioModbusSerialClient, + ModbusClientProtocol, + init_tcp_client, + init_udp_client, +) from pymodbus.exceptions import ModbusException +from pymodbus.factory import ClientDecoder from pymodbus.pdu import ExceptionResponse -from pymodbus.transaction import ModbusRtuFramer +from pymodbus.transaction import ( + ModbusAsciiFramer, + ModbusBinaryFramer, + ModbusRtuFramer, + ModbusSocketFramer, +) import voluptuous as vol from homeassistant.const import ( @@ -105,13 +113,6 @@ async def async_setup(hass, config): for client in hub_collect.values(): del client - def start_modbus(): - """Start Modbus service.""" - for client in hub_collect.values(): - client.setup() - - hass.bus.listen_once(EVENT_HOMEASSISTANT_STOP, stop_modbus) - async def write_register(service): """Write Modbus registers.""" unit = int(float(service.data[ATTR_UNIT])) @@ -136,7 +137,11 @@ async def async_setup(hass, config): await hub_collect[client_name].write_coil(unit, address, state) # do not wait for EVENT_HOMEASSISTANT_START, activate pymodbus now - await hass.async_add_executor_job(start_modbus) + for client in hub_collect.values(): + await client.setup(hass) + + # register function to gracefully stop modbus + hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, stop_modbus) # Register services for modbus hass.services.async_register( @@ -189,47 +194,55 @@ class ModbusHub: await asyncio.sleep(self._config_delay) self._config_delay = 0 - def setup(self): - """Set up pymodbus client.""" - # pylint: disable = E0633 - # Client* do deliver loop, client as result but - # pylint does not accept that fact + @staticmethod + def _framer(method): + if method == "ascii": + framer = ModbusAsciiFramer(ClientDecoder()) + elif method == "rtu": + framer = ModbusRtuFramer(ClientDecoder()) + elif method == "binary": + framer = ModbusBinaryFramer(ClientDecoder()) + elif method == "socket": + framer = ModbusSocketFramer(ClientDecoder()) + else: + framer = None + return framer + async def setup(self, hass): + """Set up pymodbus client.""" if self._config_type == "serial": - _, self._client = ClientSerial( - schedulers.ASYNC_IO, - method=self._config_method, - port=self._config_port, + # reconnect ?? + framer = self._framer(self._config_method) + + # just a class creation no IO or other slow items + self._client = AsyncioModbusSerialClient( + self._config_port, + protocol_class=ModbusClientProtocol, + framer=framer, + loop=self._loop, baudrate=self._config_baudrate, - stopbits=self._config_stopbits, bytesize=self._config_bytesize, parity=self._config_parity, - loop=self._loop, + stopbits=self._config_stopbits, ) + await self._client.connect() elif self._config_type == "rtuovertcp": - _, self._client = ClientTCP( - schedulers.ASYNC_IO, - host=self._config_host, - port=self._config_port, - framer=ModbusRtuFramer, - timeout=self._config_timeout, - loop=self._loop, + # framer ModbusRtuFramer ?? + # timeout ?? + self._client = await init_tcp_client( + None, self._loop, self._config_host, self._config_port ) elif self._config_type == "tcp": - _, self._client = ClientTCP( - schedulers.ASYNC_IO, - host=self._config_host, - port=self._config_port, - timeout=self._config_timeout, - loop=self._loop, + # framer ?? + # timeout ?? + self._client = await init_tcp_client( + None, self._loop, self._config_host, self._config_port ) elif self._config_type == "udp": - _, self._client = ClientUDP( - schedulers.ASYNC_IO, - host=self._config_host, - port=self._config_port, - timeout=self._config_timeout, - loop=self._loop, + # framer ?? + # timeout ?? + self._client = await init_udp_client( + None, self._loop, self._config_host, self._config_port ) else: assert False diff --git a/homeassistant/components/modbus/manifest.json b/homeassistant/components/modbus/manifest.json index d1d2a9db550..ec3381577ad 100644 --- a/homeassistant/components/modbus/manifest.json +++ b/homeassistant/components/modbus/manifest.json @@ -2,7 +2,6 @@ "domain": "modbus", "name": "Modbus", "documentation": "https://www.home-assistant.io/integrations/modbus", - "requirements": ["pymodbus==2.3.0"], - "dependencies": [], + "requirements": ["pymodbus==2.3.0", "pyserial-asyncio==0.4"], "codeowners": ["@adamchengtkc", "@janiversen"] } diff --git a/requirements_all.txt b/requirements_all.txt index cd773652112..495e5b78aed 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -1523,6 +1523,7 @@ pysdcp==1 # homeassistant.components.sensibo pysensibo==1.0.3 +# homeassistant.components.modbus # homeassistant.components.serial pyserial-asyncio==0.4 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index ff64c959be8..4434852bccc 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -586,6 +586,10 @@ pyps4-2ndscreen==1.0.7 # homeassistant.components.qwikswitch pyqwikswitch==0.93 +# homeassistant.components.modbus +# homeassistant.components.serial +pyserial-asyncio==0.4 + # homeassistant.components.signal_messenger pysignalclirestapi==0.2.4