Add pymodbus exception handling and isolate pymodbus to class modbusHub (#49052)

homeassistant/components/modbus/binary_sensor.py

@@ -4,8 +4,6 @@ from __future__ import annotations
 from datetime import timedelta
 import logging
 
-from pymodbus.exceptions import ConnectionException, ModbusException
-from pymodbus.pdu import ExceptionResponse
 import voluptuous as vol
 
 from homeassistant.components.binary_sensor import (
@@ -165,16 +163,11 @@ class ModbusBinarySensor(BinarySensorEntity):
 
     def _update(self):
         """Update the state of the sensor."""
-        try:
-            if self._input_type == CALL_TYPE_COIL:
-                result = self._hub.read_coils(self._slave, self._address, 1)
-            else:
-                result = self._hub.read_discrete_inputs(self._slave, self._address, 1)
-        except ConnectionException:
-            self._available = False
-            return
-
-        if isinstance(result, (ModbusException, ExceptionResponse)):
+        if self._input_type == CALL_TYPE_COIL:
+            result = self._hub.read_coils(self._slave, self._address, 1)
+        else:
+            result = self._hub.read_discrete_inputs(self._slave, self._address, 1)
+        if result is None:
             self._available = False
             return
 

homeassistant/components/modbus/climate.py

@@ -6,9 +6,6 @@ import logging
 import struct
 from typing import Any
 
-from pymodbus.exceptions import ConnectionException, ModbusException
-from pymodbus.pdu import ExceptionResponse
-
 from homeassistant.components.climate import ClimateEntity
 from homeassistant.components.climate.const import (
     HVAC_MODE_AUTO,
@@ -212,7 +209,11 @@ class ModbusThermostat(ClimateEntity):
         )
         byte_string = struct.pack(self._structure, target_temperature)
         register_value = struct.unpack(">h", byte_string[0:2])[0]
-        self._write_register(self._target_temperature_register, register_value)
+        self._available = self._hub.write_registers(
+            self._slave,
+            self._target_temperature_register,
+            register_value,
+        )
         self._update()
 
     @property
@@ -233,20 +234,13 @@ class ModbusThermostat(ClimateEntity):
 
     def _read_register(self, register_type, register) -> float | None:
         """Read register using the Modbus hub slave."""
-        try:
-            if register_type == CALL_TYPE_REGISTER_INPUT:
-                result = self._hub.read_input_registers(
-                    self._slave, register, self._count
-                )
-            else:
-                result = self._hub.read_holding_registers(
-                    self._slave, register, self._count
-                )
-        except ConnectionException:
-            self._available = False
-            return
-
-        if isinstance(result, (ModbusException, ExceptionResponse)):
+        if register_type == CALL_TYPE_REGISTER_INPUT:
+            result = self._hub.read_input_registers(self._slave, register, self._count)
+        else:
+            result = self._hub.read_holding_registers(
+                self._slave, register, self._count
+            )
+        if result is None:
             self._available = False
             return
 
@@ -269,13 +263,3 @@ class ModbusThermostat(ClimateEntity):
         self._available = True
 
         return register_value
-
-    def _write_register(self, register, value):
-        """Write holding register using the Modbus hub slave."""
-        try:
-            self._hub.write_registers(self._slave, register, value)
-        except ConnectionException:
-            self._available = False
-            return
-
-        self._available = True

homeassistant/components/modbus/cover.py

@@ -5,9 +5,6 @@ from datetime import timedelta
 import logging
 from typing import Any
 
-from pymodbus.exceptions import ConnectionException, ModbusException
-from pymodbus.pdu import ExceptionResponse
-
 from homeassistant.components.cover import SUPPORT_CLOSE, SUPPORT_OPEN, CoverEntity
 from homeassistant.const import (
     CONF_COVERS,
@@ -187,22 +184,17 @@ class ModbusCover(CoverEntity, RestoreEntity):
 
     def _read_status_register(self) -> int | None:
         """Read status register using the Modbus hub slave."""
-        try:
-            if self._status_register_type == CALL_TYPE_REGISTER_INPUT:
-                result = self._hub.read_input_registers(
-                    self._slave, self._status_register, 1
-                )
-            else:
-                result = self._hub.read_holding_registers(
-                    self._slave, self._status_register, 1
-                )
-        except ConnectionException:
+        if self._status_register_type == CALL_TYPE_REGISTER_INPUT:
+            result = self._hub.read_input_registers(
+                self._slave, self._status_register, 1
+            )
+        else:
+            result = self._hub.read_holding_registers(
+                self._slave, self._status_register, 1
+            )
+        if result is None:
             self._available = False
-            return
-
-        if isinstance(result, (ModbusException, ExceptionResponse)):
-            self._available = False
-            return
+            return None
 
         value = int(result.registers[0])
         self._available = True
@@ -211,37 +203,18 @@ class ModbusCover(CoverEntity, RestoreEntity):
 
     def _write_register(self, value):
         """Write holding register using the Modbus hub slave."""
-        try:
-            self._hub.write_register(self._slave, self._register, value)
-        except ConnectionException:
-            self._available = False
-            return
-
-        self._available = True
+        self._available = self._hub.write_register(self._slave, self._register, value)
 
     def _read_coil(self) -> bool | None:
         """Read coil using the Modbus hub slave."""
-        try:
-            result = self._hub.read_coils(self._slave, self._coil, 1)
-        except ConnectionException:
+        result = self._hub.read_coils(self._slave, self._coil, 1)
+        if result is None:
             self._available = False
-            return
-
-        if isinstance(result, (ModbusException, ExceptionResponse)):
-            self._available = False
-            return
+            return None
 
         value = bool(result.bits[0] & 1)
-        self._available = True
-
         return value
 
     def _write_coil(self, value):
         """Write coil using the Modbus hub slave."""
-        try:
-            self._hub.write_coil(self._slave, self._coil, value)
-        except ConnectionException:
-            self._available = False
-            return
-
-        self._available = True
+        self._available = self._hub.write_coil(self._slave, self._coil, value)

homeassistant/components/modbus/modbus.py

@@ -3,6 +3,7 @@ import logging
 import threading
 
 from pymodbus.client.sync import ModbusSerialClient, ModbusTcpClient, ModbusUdpClient
+from pymodbus.exceptions import ModbusException
 from pymodbus.transaction import ModbusRtuFramer
 
 from homeassistant.const import (
@@ -37,6 +38,7 @@ from .const import (
     CONF_SENSOR,
     CONF_STOPBITS,
     CONF_SWITCH,
+    DEFAULT_HUB,
     MODBUS_DOMAIN as DOMAIN,
     SERVICE_WRITE_COIL,
     SERVICE_WRITE_REGISTER,
@@ -49,8 +51,8 @@ def modbus_setup(
     hass, config, service_write_register_schema, service_write_coil_schema
 ):
     """Set up Modbus component."""
-    hass.data[DOMAIN] = hub_collect = {}
 
+    hass.data[DOMAIN] = hub_collect = {}
     for conf_hub in config[DOMAIN]:
         hub_collect[conf_hub[CONF_NAME]] = ModbusHub(conf_hub)
 
@@ -71,15 +73,19 @@ def modbus_setup(
 
     def stop_modbus(event):
         """Stop Modbus service."""
+
         for client in hub_collect.values():
             client.close()
+            del client
 
     def write_register(service):
         """Write Modbus registers."""
         unit = int(float(service.data[ATTR_UNIT]))
         address = int(float(service.data[ATTR_ADDRESS]))
         value = service.data[ATTR_VALUE]
-        client_name = service.data[ATTR_HUB]
+        client_name = (
+            service.data[ATTR_HUB] if ATTR_HUB in service.data else DEFAULT_HUB
+        )
         if isinstance(value, list):
             hub_collect[client_name].write_registers(
                 unit, address, [int(float(i)) for i in value]
@@ -92,7 +98,9 @@ def modbus_setup(
         unit = service.data[ATTR_UNIT]
         address = service.data[ATTR_ADDRESS]
         state = service.data[ATTR_STATE]
-        client_name = service.data[ATTR_HUB]
+        client_name = (
+            service.data[ATTR_HUB] if ATTR_HUB in service.data else DEFAULT_HUB
+        )
         if isinstance(state, list):
             hub_collect[client_name].write_coils(unit, address, state)
         else:
@@ -122,6 +130,7 @@ class ModbusHub:
 
         # generic configuration
         self._client = None
+        self._in_error = False
         self._lock = threading.Lock()
         self._config_name = client_config[CONF_NAME]
         self._config_type = client_config[CONF_TYPE]
@@ -140,48 +149,58 @@ class ModbusHub:
             # network configuration
             self._config_host = client_config[CONF_HOST]
             self._config_delay = client_config[CONF_DELAY]
-            if self._config_delay > 0:
-                _LOGGER.warning(
-                    "Parameter delay is accepted but not used in this version"
-                )
+
+        if self._config_delay > 0:
+            _LOGGER.warning("Parameter delay is accepted but not used in this version")
 
     @property
     def name(self):
         """Return the name of this hub."""
         return self._config_name
 
+    def _log_error(self, exception_error: ModbusException, error_state=True):
+        if self._in_error:
+            _LOGGER.debug(str(exception_error))
+        else:
+            _LOGGER.error(str(exception_error))
+            self._in_error = error_state
+
     def setup(self):
         """Set up pymodbus client."""
-        if self._config_type == "serial":
-            self._client = ModbusSerialClient(
-                method=self._config_method,
-                port=self._config_port,
-                baudrate=self._config_baudrate,
-                stopbits=self._config_stopbits,
-                bytesize=self._config_bytesize,
-                parity=self._config_parity,
-                timeout=self._config_timeout,
-                retry_on_empty=True,
-            )
-        elif self._config_type == "rtuovertcp":
-            self._client = ModbusTcpClient(
-                host=self._config_host,
-                port=self._config_port,
-                framer=ModbusRtuFramer,
-                timeout=self._config_timeout,
-            )
-        elif self._config_type == "tcp":
-            self._client = ModbusTcpClient(
-                host=self._config_host,
-                port=self._config_port,
-                timeout=self._config_timeout,
-            )
-        elif self._config_type == "udp":
-            self._client = ModbusUdpClient(
-                host=self._config_host,
-                port=self._config_port,
-                timeout=self._config_timeout,
-            )
+        try:
+            if self._config_type == "serial":
+                self._client = ModbusSerialClient(
+                    method=self._config_method,
+                    port=self._config_port,
+                    baudrate=self._config_baudrate,
+                    stopbits=self._config_stopbits,
+                    bytesize=self._config_bytesize,
+                    parity=self._config_parity,
+                    timeout=self._config_timeout,
+                    retry_on_empty=True,
+                )
+            elif self._config_type == "rtuovertcp":
+                self._client = ModbusTcpClient(
+                    host=self._config_host,
+                    port=self._config_port,
+                    framer=ModbusRtuFramer,
+                    timeout=self._config_timeout,
+                )
+            elif self._config_type == "tcp":
+                self._client = ModbusTcpClient(
+                    host=self._config_host,
+                    port=self._config_port,
+                    timeout=self._config_timeout,
+                )
+            elif self._config_type == "udp":
+                self._client = ModbusUdpClient(
+                    host=self._config_host,
+                    port=self._config_port,
+                    timeout=self._config_timeout,
+                )
+        except ModbusException as exception_error:
+            self._log_error(exception_error, error_state=False)
+            return
 
         # Connect device
         self.connect()
@@ -189,57 +208,115 @@ class ModbusHub:
     def close(self):
         """Disconnect client."""
         with self._lock:
-            self._client.close()
+            try:
+                self._client.close()
+                del self._client
+                self._client = None
+            except ModbusException as exception_error:
+                self._log_error(exception_error, error_state=False)
+                return
 
     def connect(self):
         """Connect client."""
         with self._lock:
-            self._client.connect()
+            try:
+                self._client.connect()
+            except ModbusException as exception_error:
+                self._log_error(exception_error, error_state=False)
+                return
 
     def read_coils(self, unit, address, count):
         """Read coils."""
         with self._lock:
             kwargs = {"unit": unit} if unit else {}
-            return self._client.read_coils(address, count, **kwargs)
+            try:
+                result = self._client.read_coils(address, count, **kwargs)
+            except ModbusException as exception_error:
+                self._log_error(exception_error)
+                return None
+            self._in_error = False
+            return result
 
     def read_discrete_inputs(self, unit, address, count):
         """Read discrete inputs."""
         with self._lock:
             kwargs = {"unit": unit} if unit else {}
-            return self._client.read_discrete_inputs(address, count, **kwargs)
+            try:
+                result = self._client.read_discrete_inputs(address, count, **kwargs)
+            except ModbusException as exception_error:
+                self._log_error(exception_error)
+                return None
+            self._in_error = False
+            return result
 
     def read_input_registers(self, unit, address, count):
         """Read input registers."""
         with self._lock:
             kwargs = {"unit": unit} if unit else {}
-            return self._client.read_input_registers(address, count, **kwargs)
+            try:
+                result = self._client.read_input_registers(address, count, **kwargs)
+            except ModbusException as exception_error:
+                self._log_error(exception_error)
+                return None
+            self._in_error = False
+            return result
 
     def read_holding_registers(self, unit, address, count):
         """Read holding registers."""
         with self._lock:
             kwargs = {"unit": unit} if unit else {}
-            return self._client.read_holding_registers(address, count, **kwargs)
+            try:
+                result = self._client.read_holding_registers(address, count, **kwargs)
+            except ModbusException as exception_error:
+                self._log_error(exception_error)
+                return None
+            self._in_error = False
+            return result
 
-    def write_coil(self, unit, address, value):
+    def write_coil(self, unit, address, value) -> bool:
         """Write coil."""
         with self._lock:
             kwargs = {"unit": unit} if unit else {}
-            self._client.write_coil(address, value, **kwargs)
+            try:
+                self._client.write_coil(address, value, **kwargs)
+            except ModbusException as exception_error:
+                self._log_error(exception_error)
+                return False
+            self._in_error = False
+            return True
 
-    def write_coils(self, unit, address, value):
+    def write_coils(self, unit, address, values) -> bool:
         """Write coil."""
         with self._lock:
             kwargs = {"unit": unit} if unit else {}
-            self._client.write_coils(address, value, **kwargs)
+            try:
+                self._client.write_coils(address, values, **kwargs)
+            except ModbusException as exception_error:
+                self._log_error(exception_error)
+                return False
+            self._in_error = False
+            return True
 
-    def write_register(self, unit, address, value):
+    def write_register(self, unit, address, value) -> bool:
         """Write register."""
         with self._lock:
             kwargs = {"unit": unit} if unit else {}
-            self._client.write_register(address, value, **kwargs)
+            try:
+                self._client.write_register(address, value, **kwargs)
+            except ModbusException as exception_error:
+                self._log_error(exception_error)
+                return False
+            self._in_error = False
+            return True
 
-    def write_registers(self, unit, address, values):
+    def write_registers(self, unit, address, values) -> bool:
         """Write registers."""
         with self._lock:
             kwargs = {"unit": unit} if unit else {}
-            self._client.write_registers(address, values, **kwargs)
+            try:
+                self._client.write_registers(address, values, **kwargs)
+            except ModbusException as exception_error:
+                self._log_error(exception_error)
+                return False
+            self._in_error = False
+            return True

homeassistant/components/modbus/sensor.py

@@ -6,8 +6,6 @@ import logging
 import struct
 from typing import Any
 
-from pymodbus.exceptions import ConnectionException, ModbusException
-from pymodbus.pdu import ExceptionResponse
 import voluptuous as vol
 
 from homeassistant.components.sensor import (
@@ -285,20 +283,15 @@ class ModbusRegisterSensor(RestoreEntity, SensorEntity):
 
     def _update(self):
         """Update the state of the sensor."""
-        try:
-            if self._register_type == CALL_TYPE_REGISTER_INPUT:
-                result = self._hub.read_input_registers(
-                    self._slave, self._register, self._count
-                )
-            else:
-                result = self._hub.read_holding_registers(
-                    self._slave, self._register, self._count
-                )
-        except ConnectionException:
-            self._available = False
-            return
-
-        if isinstance(result, (ModbusException, ExceptionResponse)):
+        if self._register_type == CALL_TYPE_REGISTER_INPUT:
+            result = self._hub.read_input_registers(
+                self._slave, self._register, self._count
+            )
+        else:
+            result = self._hub.read_holding_registers(
+                self._slave, self._register, self._count
+            )
+        if result is None:
             self._available = False
             return
 

homeassistant/components/modbus/switch.py

@@ -6,8 +6,6 @@ from datetime import timedelta
 import logging
 from typing import Any
 
-from pymodbus.exceptions import ConnectionException, ModbusException
-from pymodbus.pdu import ExceptionResponse
 import voluptuous as vol
 
 from homeassistant.components.switch import PLATFORM_SCHEMA, SwitchEntity
@@ -213,13 +211,8 @@ class ModbusCoilSwitch(ModbusBaseSwitch, SwitchEntity):
 
     def _read_coil(self, coil) -> bool:
         """Read coil using the Modbus hub slave."""
-        try:
-            result = self._hub.read_coils(self._slave, coil, 1)
-        except ConnectionException:
-            self._available = False
-            return False
-
-        if isinstance(result, (ModbusException, ExceptionResponse)):
+        result = self._hub.read_coils(self._slave, coil, 1)
+        if result is None:
             self._available = False
             return False
 
@@ -231,13 +224,7 @@ class ModbusCoilSwitch(ModbusBaseSwitch, SwitchEntity):
 
     def _write_coil(self, coil, value):
         """Write coil using the Modbus hub slave."""
-        try:
-            self._hub.write_coil(self._slave, coil, value)
-        except ConnectionException:
-            self._available = False
-            return
-
-        self._available = True
+        self._available = self._hub.write_coil(self._slave, coil, value)
 
 
 class ModbusRegisterSwitch(ModbusBaseSwitch, SwitchEntity):
@@ -301,33 +288,21 @@ class ModbusRegisterSwitch(ModbusBaseSwitch, SwitchEntity):
         self.schedule_update_ha_state()
 
     def _read_register(self) -> int | None:
-        try:
-            if self._register_type == CALL_TYPE_REGISTER_INPUT:
-                result = self._hub.read_input_registers(
-                    self._slave, self._verify_register, 1
-                )
-            else:
-                result = self._hub.read_holding_registers(
-                    self._slave, self._verify_register, 1
-                )
-        except ConnectionException:
+        if self._register_type == CALL_TYPE_REGISTER_INPUT:
+            result = self._hub.read_input_registers(
+                self._slave, self._verify_register, 1
+            )
+        else:
+            result = self._hub.read_holding_registers(
+                self._slave, self._verify_register, 1
+            )
+        if result is None:
             self._available = False
             return
-
-        if isinstance(result, (ModbusException, ExceptionResponse)):
-            self._available = False
-            return
-
         self._available = True
 
         return int(result.registers[0])
 
     def _write_register(self, value):
         """Write holding register using the Modbus hub slave."""
-        try:
-            self._hub.write_register(self._slave, self._register, value)
-        except ConnectionException:
-            self._available = False
-            return
-
-        self._available = True
+        self._available = self._hub.write_register(self._slave, self._register, value)