Add availability status to Modbus entities and improve error handling (#31073)

homeassistant/components/modbus/binary_sensor.py

@@ -2,6 +2,8 @@
 import logging
 from typing import Optional
 
+from pymodbus.exceptions import ConnectionException, ModbusException
+from pymodbus.pdu import ExceptionResponse
 import voluptuous as vol
 
 from homeassistant.components.binary_sensor import (
@@ -83,6 +85,7 @@ class ModbusBinarySensor(BinarySensorDevice):
         self._device_class = device_class
         self._input_type = input_type
         self._value = None
+        self._available = True
 
     @property
     def name(self):
@@ -99,18 +102,38 @@ class ModbusBinarySensor(BinarySensorDevice):
         """Return the device class of the sensor."""
         return self._device_class
 
+    @property
+    def available(self) -> bool:
+        """Return True if entity is available."""
+        return self._available
+
     def update(self):
         """Update the state of the sensor."""
-        if self._input_type == INPUT_TYPE_COIL:
-            result = self._hub.read_coils(self._slave, self._address, 1)
-        else:
-            result = self._hub.read_discrete_inputs(self._slave, self._address, 1)
         try:
-            self._value = result.bits[0]
+            if self._input_type == INPUT_TYPE_COIL:
-        except AttributeError:
+                result = self._hub.read_coils(self._slave, self._address, 1)
-            _LOGGER.error(
+            else:
-                "No response from hub %s, slave %s, address %s",
+                result = self._hub.read_discrete_inputs(self._slave, self._address, 1)
-                self._hub.name,
+        except ConnectionException:
-                self._slave,
+            self._set_unavailable()
-                self._address,
+            return
-            )
+
+        if isinstance(result, (ModbusException, ExceptionResponse)):
+            self._set_unavailable()
+            return
+
+        self._value = result.bits[0]
+        self._available = True
+
+    def _set_unavailable(self):
+        """Set unavailable state and log it as an error."""
+        if not self._available:
+            return
+
+        _LOGGER.error(
+            "No response from hub %s, slave %s, address %s",
+            self._hub.name,
+            self._slave,
+            self._address,
+        )
+        self._available = False

homeassistant/components/modbus/climate.py

@@ -1,7 +1,10 @@
 """Support for Generic Modbus Thermostats."""
 import logging
 import struct
+from typing import Optional
 
+from pymodbus.exceptions import ConnectionException, ModbusException
+from pymodbus.pdu import ExceptionResponse
 import voluptuous as vol
 
 from homeassistant.components.climate import PLATFORM_SCHEMA, ClimateDevice
@@ -140,6 +143,7 @@ class ModbusThermostat(ClimateDevice):
         self._min_temp = min_temp
         self._temp_step = temp_step
         self._structure = ">f"
+        self._available = True
 
         data_types = {
             DATA_TYPE_INT: {1: "h", 2: "i", 4: "q"},
@@ -156,8 +160,10 @@ class ModbusThermostat(ClimateDevice):
 
     def update(self):
         """Update Target & Current Temperature."""
-        self._target_temperature = self.read_register(self._target_temperature_register)
+        self._target_temperature = self._read_register(
-        self._current_temperature = self.read_register(
+            self._target_temperature_register
+        )
+        self._current_temperature = self._read_register(
             self._current_temperature_register
         )
 
@@ -215,20 +221,27 @@ class ModbusThermostat(ClimateDevice):
             return
         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)
 
-        try:
+    @property
-            self.write_register(self._target_temperature_register, register_value)
+    def available(self) -> bool:
-        except AttributeError as ex:
+        """Return True if entity is available."""
-            _LOGGER.error(ex)
+        return self._available
 
-    def read_register(self, register):
+    def _read_register(self, register) -> Optional[float]:
         """Read holding register using the Modbus hub slave."""
         try:
             result = self._hub.read_holding_registers(
                 self._slave, register, self._count
             )
-        except AttributeError as ex:
+        except ConnectionException:
-            _LOGGER.error(ex)
+            self._set_unavailable(register)
+            return
+
+        if isinstance(result, (ModbusException, ExceptionResponse)):
+            self._set_unavailable(register)
+            return
+
         byte_string = b"".join(
             [x.to_bytes(2, byteorder="big") for x in result.registers]
         )
@@ -237,8 +250,29 @@ class ModbusThermostat(ClimateDevice):
             (self._scale * val) + self._offset, f".{self._precision}f"
         )
         register_value = float(register_value)
+        self._available = True
+
         return register_value
 
-    def write_register(self, register, value):
+    def _write_register(self, register, value):
-        """Write register using the Modbus hub slave."""
+        """Write holding register using the Modbus hub slave."""
-        self._hub.write_registers(self._slave, register, [value, 0])
+        try:
+            self._hub.write_registers(self._slave, register, [value, 0])
+        except ConnectionException:
+            self._set_unavailable(register)
+            return
+
+        self._available = True
+
+    def _set_unavailable(self, register):
+        """Set unavailable state and log it as an error."""
+        if not self._available:
+            return
+
+        _LOGGER.error(
+            "No response from hub %s, slave %s, register %s",
+            self._hub.name,
+            self._slave,
+            register,
+        )
+        self._available = False

homeassistant/components/modbus/sensor.py

@@ -3,6 +3,8 @@ import logging
 import struct
 from typing import Any, Optional, Union
 
+from pymodbus.exceptions import ConnectionException, ModbusException
+from pymodbus.pdu import ExceptionResponse
 import voluptuous as vol
 
 from homeassistant.components.sensor import DEVICE_CLASSES_SCHEMA, PLATFORM_SCHEMA
@@ -184,6 +186,7 @@ class ModbusRegisterSensor(RestoreEntity):
         self._structure = structure
         self._device_class = device_class
         self._value = None
+        self._available = True
 
     async def async_added_to_hass(self):
         """Handle entity which will be added."""
@@ -212,30 +215,34 @@ class ModbusRegisterSensor(RestoreEntity):
         """Return the device class of the sensor."""
         return self._device_class
 
+    @property
+    def available(self) -> bool:
+        """Return True if entity is available."""
+        return self._available
+
     def update(self):
         """Update the state of the sensor."""
-        if self._register_type == REGISTER_TYPE_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
-            )
-        val = 0
-
         try:
-            registers = result.registers
+            if self._register_type == REGISTER_TYPE_INPUT:
-            if self._reverse_order:
+                result = self._hub.read_input_registers(
-                registers.reverse()
+                    self._slave, self._register, self._count
-        except AttributeError:
+                )
-            _LOGGER.error(
+            else:
-                "No response from hub %s, slave %s, register %s",
+                result = self._hub.read_holding_registers(
-                self._hub.name,
+                    self._slave, self._register, self._count
-                self._slave,
+                )
-                self._register,
+        except ConnectionException:
-            )
+            self._set_unavailable()
             return
+
+        if isinstance(result, (ModbusException, ExceptionResponse)):
+            self._set_unavailable()
+            return
+
+        registers = result.registers
+        if self._reverse_order:
+            registers.reverse()
+
         byte_string = b"".join([x.to_bytes(2, byteorder="big") for x in registers])
         val = struct.unpack(self._structure, byte_string)[0]
         val = self._scale * val + self._offset
@@ -245,3 +252,18 @@ class ModbusRegisterSensor(RestoreEntity):
                 self._value += "." + "0" * self._precision
         else:
             self._value = f"{val:.{self._precision}f}"
+
+        self._available = True
+
+    def _set_unavailable(self):
+        """Set unavailable state and log it as an error."""
+        if not self._available:
+            return
+
+        _LOGGER.error(
+            "No response from hub %s, slave %s, address %s",
+            self._hub.name,
+            self._slave,
+            self._register,
+        )
+        self._available = False

homeassistant/components/modbus/switch.py

@@ -1,6 +1,9 @@
 """Support for Modbus switches."""
 import logging
+from typing import Optional
 
+from pymodbus.exceptions import ConnectionException, ModbusException
+from pymodbus.pdu import ExceptionResponse
 import voluptuous as vol
 
 from homeassistant.components.switch import PLATFORM_SCHEMA
@@ -116,6 +119,7 @@ class ModbusCoilSwitch(ToggleEntity, RestoreEntity):
         self._slave = int(slave) if slave else None
         self._coil = int(coil)
         self._is_on = None
+        self._available = True
 
     async def async_added_to_hass(self):
         """Handle entity which will be added."""
@@ -134,26 +138,62 @@ class ModbusCoilSwitch(ToggleEntity, RestoreEntity):
         """Return the name of the switch."""
         return self._name
 
+    @property
+    def available(self) -> bool:
+        """Return True if entity is available."""
+        return self._available
+
     def turn_on(self, **kwargs):
         """Set switch on."""
-        self._hub.write_coil(self._slave, self._coil, True)
+        self._write_coil(self._coil, True)
 
     def turn_off(self, **kwargs):
         """Set switch off."""
-        self._hub.write_coil(self._slave, self._coil, False)
+        self._write_coil(self._coil, False)
 
     def update(self):
         """Update the state of the switch."""
-        result = self._hub.read_coils(self._slave, self._coil, 1)
+        self._is_on = self._read_coil(self._coil)
+
+    def _read_coil(self, coil) -> Optional[bool]:
+        """Read coil using the Modbus hub slave."""
         try:
-            self._is_on = bool(result.bits[0])
+            result = self._hub.read_coils(self._slave, coil, 1)
-        except AttributeError:
+        except ConnectionException:
-            _LOGGER.error(
+            self._set_unavailable()
-                "No response from hub %s, slave %s, coil %s",
+            return
-                self._hub.name,
+
-                self._slave,
+        if isinstance(result, (ModbusException, ExceptionResponse)):
-                self._coil,
+            self._set_unavailable()
-            )
+            return
+
+        value = bool(result.bits[0])
+        self._available = True
+
+        return value
+
+    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._set_unavailable()
+            return
+
+        self._available = True
+
+    def _set_unavailable(self):
+        """Set unavailable state and log it as an error."""
+        if not self._available:
+            return
+
+        _LOGGER.error(
+            "No response from hub %s, slave %s, coil %s",
+            self._hub.name,
+            self._slave,
+            self._coil,
+        )
+        self._available = False
 
 
 class ModbusRegisterSwitch(ModbusCoilSwitch):
@@ -184,6 +224,7 @@ class ModbusRegisterSwitch(ModbusCoilSwitch):
         self._verify_state = verify_state
         self._verify_register = verify_register if verify_register else self._register
         self._register_type = register_type
+        self._available = True
 
         if state_on is not None:
             self._state_on = state_on
@@ -199,46 +240,86 @@ class ModbusRegisterSwitch(ModbusCoilSwitch):
 
     def turn_on(self, **kwargs):
         """Set switch on."""
-        self._hub.write_register(self._slave, self._register, self._command_on)
+
-        if not self._verify_state:
+        # Only holding register is writable
-            self._is_on = True
+        if self._register_type == REGISTER_TYPE_HOLDING:
+            self._write_register(self._command_on)
+            if not self._verify_state:
+                self._is_on = True
 
     def turn_off(self, **kwargs):
         """Set switch off."""
-        self._hub.write_register(self._slave, self._register, self._command_off)
+
-        if not self._verify_state:
+        # Only holding register is writable
-            self._is_on = False
+        if self._register_type == REGISTER_TYPE_HOLDING:
+            self._write_register(self._command_off)
+            if not self._verify_state:
+                self._is_on = False
+
+    @property
+    def available(self) -> bool:
+        """Return True if entity is available."""
+        return self._available
 
     def update(self):
         """Update the state of the switch."""
         if not self._verify_state:
             return
 
-        value = 0
+        value = self._read_register()
-        if self._register_type == REGISTER_TYPE_INPUT:
-            result = self._hub.read_input_registers(self._slave, self._register, 1)
-        else:
-            result = self._hub.read_holding_registers(self._slave, self._register, 1)
-
-        try:
-            value = int(result.registers[0])
-        except AttributeError:
-            _LOGGER.error(
-                "No response from hub %s, slave %s, register %s",
-                self._hub.name,
-                self._slave,
-                self._verify_register,
-            )
-
         if value == self._state_on:
             self._is_on = True
         elif value == self._state_off:
             self._is_on = False
-        else:
+        elif value is not None:
             _LOGGER.error(
                 "Unexpected response from hub %s, slave %s register %s, got 0x%2x",
                 self._hub.name,
                 self._slave,
-                self._verify_register,
+                self._register,
                 value,
             )
+
+    def _read_register(self) -> Optional[int]:
+        try:
+            if self._register_type == REGISTER_TYPE_INPUT:
+                result = self._hub.read_input_registers(self._slave, self._register, 1)
+            else:
+                result = self._hub.read_holding_registers(
+                    self._slave, self._register, 1
+                )
+        except ConnectionException:
+            self._set_unavailable()
+            return
+
+        if isinstance(result, (ModbusException, ExceptionResponse)):
+            self._set_unavailable()
+            return
+
+        value = int(result.registers[0])
+        self._available = True
+
+        return value
+
+    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._set_unavailable()
+            return
+
+        self._available = True
+
+    def _set_unavailable(self):
+        """Set unavailable state and log it as an error."""
+        if not self._available:
+            return
+
+        _LOGGER.error(
+            "No response from hub %s, slave %s, register %s",
+            self._hub.name,
+            self._slave,
+            self._register,
+        )
+        self._available = False