From 70b09ed9a1e9f0b4db4502f5f1304d428ad25515 Mon Sep 17 00:00:00 2001 From: jan iversen Date: Mon, 10 May 2021 19:28:38 +0200 Subject: [PATCH] Handle relation between scan_interval and pymodbus timeout in modbus (#50363) * Control scan_interval compared to pymodbus timeout. add MINIMUM_SCAN_INTERVAL=5 seconds and validata with Voluptous. Keep modbus.py 100% coverage. * Please pylint. * Review comments. * pylint. --- homeassistant/components/modbus/__init__.py | 38 +++++++++++++++++++++ homeassistant/components/modbus/const.py | 26 +++++++++++--- homeassistant/components/modbus/modbus.py | 22 ++---------- tests/components/modbus/test_init.py | 13 ++++--- 4 files changed, 70 insertions(+), 29 deletions(-) diff --git a/homeassistant/components/modbus/__init__.py b/homeassistant/components/modbus/__init__.py index 2ebab4635b3..8e0d9220718 100644 --- a/homeassistant/components/modbus/__init__.py +++ b/homeassistant/components/modbus/__init__.py @@ -1,6 +1,7 @@ """Support for Modbus.""" from __future__ import annotations +import logging from typing import Any import voluptuous as vol @@ -95,10 +96,14 @@ from .const import ( DEFAULT_SCAN_INTERVAL, DEFAULT_STRUCTURE_PREFIX, DEFAULT_TEMP_UNIT, + MINIMUM_SCAN_INTERVAL, MODBUS_DOMAIN as DOMAIN, + PLATFORMS, ) from .modbus import modbus_setup +_LOGGER = logging.getLogger(__name__) + BASE_SCHEMA = vol.Schema({vol.Optional(CONF_NAME, default=DEFAULT_HUB): cv.string}) @@ -121,6 +126,38 @@ def number(value: Any) -> int | float: raise vol.Invalid(f"invalid number {value}") from err +def control_scan_interval(config: dict) -> dict: + """Control scan_interval.""" + for hub in config: + minimum_scan_interval = DEFAULT_SCAN_INTERVAL + for component, conf_key in PLATFORMS: + if conf_key not in hub: + continue + + for entry in hub[conf_key]: + scan_interval = entry.get(CONF_SCAN_INTERVAL, DEFAULT_SCAN_INTERVAL) + if scan_interval < MINIMUM_SCAN_INTERVAL: + _LOGGER.warning( + "%s %s scan_interval(%d) is adjusted to minimum(%d)", + component, + entry.get(CONF_NAME), + scan_interval, + MINIMUM_SCAN_INTERVAL, + ) + scan_interval = MINIMUM_SCAN_INTERVAL + entry[CONF_SCAN_INTERVAL] = scan_interval + minimum_scan_interval = min(scan_interval, minimum_scan_interval) + if CONF_TIMEOUT in hub and hub[CONF_TIMEOUT] > minimum_scan_interval - 1: + _LOGGER.warning( + "Modbus %s timeout(%d) is adjusted(%d) due to scan_interval", + hub.get(CONF_NAME, ""), + hub[CONF_TIMEOUT], + minimum_scan_interval - 1, + ) + hub[CONF_TIMEOUT] = minimum_scan_interval - 1 + return config + + BASE_COMPONENT_SCHEMA = vol.Schema( { vol.Required(CONF_NAME): cv.string, @@ -279,6 +316,7 @@ CONFIG_SCHEMA = vol.Schema( { DOMAIN: vol.All( cv.ensure_list, + control_scan_interval, [ vol.Any(SERIAL_SCHEMA, ETHERNET_SCHEMA), ], diff --git a/homeassistant/components/modbus/const.py b/homeassistant/components/modbus/const.py index bddaf4b789a..d817108b4d3 100644 --- a/homeassistant/components/modbus/const.py +++ b/homeassistant/components/modbus/const.py @@ -1,13 +1,22 @@ """Constants used in modbus integration.""" +from homeassistant.components.binary_sensor import DOMAIN as BINARY_SENSOR_DOMAIN +from homeassistant.components.climate.const import DOMAIN as CLIMATE_DOMAIN +from homeassistant.components.cover import DOMAIN as COVER_DOMAIN +from homeassistant.components.sensor import DOMAIN as SENSOR_DOMAIN +from homeassistant.components.switch import DOMAIN as SWITCH_DOMAIN +from homeassistant.const import ( + CONF_BINARY_SENSORS, + CONF_COVERS, + CONF_SENSORS, + CONF_SWITCHES, +) + # configuration names CONF_BAUDRATE = "baudrate" -CONF_BINARY_SENSOR = "binary_sensor" CONF_BYTESIZE = "bytesize" -CONF_CLIMATE = "climate" CONF_CLIMATES = "climates" CONF_COILS = "coils" -CONF_COVER = "cover" CONF_CURRENT_TEMP = "current_temp_register" CONF_CURRENT_TEMP_REGISTER_TYPE = "current_temp_register_type" CONF_DATA_COUNT = "data_count" @@ -24,7 +33,6 @@ CONF_REGISTERS = "registers" CONF_REVERSE_ORDER = "reverse_order" CONF_PRECISION = "precision" CONF_SCALE = "scale" -CONF_SENSOR = "sensor" CONF_STATE_CLOSED = "state_closed" CONF_STATE_CLOSING = "state_closing" CONF_STATE_OFF = "state_off" @@ -40,7 +48,6 @@ CONF_SWAP_BYTE = "byte" CONF_SWAP_NONE = "none" CONF_SWAP_WORD = "word" CONF_SWAP_WORD_BYTE = "word_byte" -CONF_SWITCH = "switch" CONF_TARGET_TEMP = "target_temp_register" CONF_VERIFY = "verify" CONF_VERIFY_REGISTER = "verify_register" @@ -74,6 +81,7 @@ SERVICE_WRITE_REGISTER = "write_register" # integration names DEFAULT_HUB = "modbus_hub" +MINIMUM_SCAN_INTERVAL = 5 # seconds DEFAULT_SCAN_INTERVAL = 15 # seconds DEFAULT_SLAVE = 1 DEFAULT_STRUCTURE_PREFIX = ">f" @@ -84,3 +92,11 @@ DEFAULT_STRUCT_FORMAT = { } DEFAULT_TEMP_UNIT = "C" MODBUS_DOMAIN = "modbus" + +PLATFORMS = ( + (CLIMATE_DOMAIN, CONF_CLIMATES), + (COVER_DOMAIN, CONF_COVERS), + (BINARY_SENSOR_DOMAIN, CONF_BINARY_SENSORS), + (SENSOR_DOMAIN, CONF_SENSORS), + (SWITCH_DOMAIN, CONF_SWITCHES), +) diff --git a/homeassistant/components/modbus/modbus.py b/homeassistant/components/modbus/modbus.py index 3c3eff9b5b0..d9db6cf980c 100644 --- a/homeassistant/components/modbus/modbus.py +++ b/homeassistant/components/modbus/modbus.py @@ -8,15 +8,11 @@ from pymodbus.exceptions import ModbusException from pymodbus.transaction import ModbusRtuFramer from homeassistant.const import ( - CONF_BINARY_SENSORS, - CONF_COVERS, CONF_DELAY, CONF_HOST, CONF_METHOD, CONF_NAME, CONF_PORT, - CONF_SENSORS, - CONF_SWITCHES, CONF_TIMEOUT, CONF_TYPE, EVENT_HOMEASSISTANT_STOP, @@ -31,17 +27,12 @@ from .const import ( ATTR_UNIT, ATTR_VALUE, CONF_BAUDRATE, - CONF_BINARY_SENSOR, CONF_BYTESIZE, - CONF_CLIMATE, - CONF_CLIMATES, - CONF_COVER, CONF_PARITY, - CONF_SENSOR, CONF_STOPBITS, - CONF_SWITCH, DEFAULT_HUB, MODBUS_DOMAIN as DOMAIN, + PLATFORMS, SERVICE_WRITE_COIL, SERVICE_WRITE_REGISTER, ) @@ -63,13 +54,7 @@ def modbus_setup( hub_collect[conf_hub[CONF_NAME]].setup(hass) # load platforms - for component, conf_key in ( - (CONF_CLIMATE, CONF_CLIMATES), - (CONF_COVER, CONF_COVERS), - (CONF_BINARY_SENSOR, CONF_BINARY_SENSORS), - (CONF_SENSOR, CONF_SENSORS), - (CONF_SWITCH, CONF_SWITCHES), - ): + for component, conf_key in PLATFORMS: if conf_key in conf_hub: load_platform(hass, component, DOMAIN, conf_hub, config) @@ -140,8 +125,7 @@ class ModbusHub: self._config_port = client_config[CONF_PORT] self._config_timeout = client_config[CONF_TIMEOUT] self._config_delay = client_config[CONF_DELAY] - - Defaults.Timeout = 10 + Defaults.Timeout = client_config[CONF_TIMEOUT] if self._config_type == "serial": # serial configuration self._config_method = client_config[CONF_METHOD] diff --git a/tests/components/modbus/test_init.py b/tests/components/modbus/test_init.py index 99a8ba467f6..7c0c7453abb 100644 --- a/tests/components/modbus/test_init.py +++ b/tests/components/modbus/test_init.py @@ -343,11 +343,14 @@ async def _read_helper(hass, do_group, do_type, do_return, do_exception, mock_py CONF_HOST: "modbusTestHost", CONF_PORT: 5501, CONF_NAME: TEST_MODBUS_NAME, - do_group: { - CONF_INPUT_TYPE: do_type, - CONF_NAME: TEST_SENSOR_NAME, - CONF_ADDRESS: 51, - }, + do_group: [ + { + CONF_INPUT_TYPE: do_type, + CONF_NAME: TEST_SENSOR_NAME, + CONF_ADDRESS: 51, + CONF_SCAN_INTERVAL: 1, + } + ], } ] }