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

@@ -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),
             ],

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),
+)

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]

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,
+                    }
+                ],
             }
         ]
     }