Remove duplicate functions in modbus climate/sensor. (#53141)

Convert all data types correctly for climate.

homeassistant/components/modbus/base_platform.py

@@ -4,17 +4,21 @@ from __future__ import annotations
 from abc import abstractmethod
 from datetime import timedelta
 import logging
+import struct
 from typing import Any
 
 from homeassistant.const import (
     CONF_ADDRESS,
     CONF_COMMAND_OFF,
     CONF_COMMAND_ON,
+    CONF_COUNT,
     CONF_DELAY,
     CONF_DEVICE_CLASS,
     CONF_NAME,
+    CONF_OFFSET,
     CONF_SCAN_INTERVAL,
     CONF_SLAVE,
+    CONF_STRUCTURE,
     STATE_ON,
 )
 from homeassistant.helpers.entity import Entity
@@ -30,11 +34,19 @@ from .const import (
     CALL_TYPE_WRITE_REGISTERS,
     CALL_TYPE_X_COILS,
     CALL_TYPE_X_REGISTER_HOLDINGS,
+    CONF_DATA_TYPE,
     CONF_INPUT_TYPE,
+    CONF_PRECISION,
+    CONF_SCALE,
     CONF_STATE_OFF,
     CONF_STATE_ON,
+    CONF_SWAP,
+    CONF_SWAP_BYTE,
+    CONF_SWAP_WORD,
+    CONF_SWAP_WORD_BYTE,
     CONF_VERIFY,
     CONF_WRITE_TYPE,
+    DATA_TYPE_STRING,
 )
 from .modbus import ModbusHub
 
@@ -90,6 +102,75 @@ class BasePlatform(Entity):
         return self._available
 
 
+class BaseStructPlatform(BasePlatform, RestoreEntity):
+    """Base class representing a sensor/climate."""
+
+    def __init__(self, hub: ModbusHub, config: dict) -> None:
+        """Initialize the switch."""
+        super().__init__(hub, config)
+        self._swap = config[CONF_SWAP]
+        self._data_type = config[CONF_DATA_TYPE]
+        self._structure = config.get(CONF_STRUCTURE)
+        self._precision = config[CONF_PRECISION]
+        self._scale = config[CONF_SCALE]
+        self._offset = config[CONF_OFFSET]
+        self._count = config[CONF_COUNT]
+
+    def _swap_registers(self, registers):
+        """Do swap as needed."""
+        if self._swap in [CONF_SWAP_BYTE, CONF_SWAP_WORD_BYTE]:
+            # convert [12][34] --> [21][43]
+            for i, register in enumerate(registers):
+                registers[i] = int.from_bytes(
+                    register.to_bytes(2, byteorder="little"),
+                    byteorder="big",
+                    signed=False,
+                )
+        if self._swap in [CONF_SWAP_WORD, CONF_SWAP_WORD_BYTE]:
+            # convert [12][34] ==> [34][12]
+            registers.reverse()
+        return registers
+
+    def unpack_structure_result(self, registers):
+        """Convert registers to proper result."""
+
+        registers = self._swap_registers(registers)
+        byte_string = b"".join([x.to_bytes(2, byteorder="big") for x in registers])
+        if self._data_type == DATA_TYPE_STRING:
+            self._value = byte_string.decode()
+        else:
+            val = struct.unpack(self._structure, byte_string)
+
+            # Issue: https://github.com/home-assistant/core/issues/41944
+            # If unpack() returns a tuple greater than 1, don't try to process the value.
+            # Instead, return the values of unpack(...) separated by commas.
+            if len(val) > 1:
+                # Apply scale and precision to floats and ints
+                v_result = []
+                for entry in val:
+                    v_temp = self._scale * entry + self._offset
+
+                    # We could convert int to float, and the code would still work; however
+                    # we lose some precision, and unit tests will fail. Therefore, we do
+                    # the conversion only when it's absolutely necessary.
+                    if isinstance(v_temp, int) and self._precision == 0:
+                        v_result.append(str(v_temp))
+                    else:
+                        v_result.append(f"{float(v_temp):.{self._precision}f}")
+                self._value = ",".join(map(str, v_result))
+            else:
+                # Apply scale and precision to floats and ints
+                val = self._scale * val[0] + self._offset
+
+                # We could convert int to float, and the code would still work; however
+                # we lose some precision, and unit tests will fail. Therefore, we do
+                # the conversion only when it's absolutely necessary.
+                if isinstance(val, int) and self._precision == 0:
+                    self._value = str(val)
+                else:
+                    self._value = f"{float(val):.{self._precision}f}"
+
+
 class BaseSwitch(BasePlatform, RestoreEntity):
     """Base class representing a Modbus switch."""
 

homeassistant/components/modbus/climate.py

@@ -11,9 +11,7 @@ from homeassistant.components.climate.const import (
     SUPPORT_TARGET_TEMPERATURE,
 )
 from homeassistant.const import (
-    CONF_COUNT,
     CONF_NAME,
-    CONF_OFFSET,
     CONF_STRUCTURE,
     CONF_TEMPERATURE_UNIT,
     PRECISION_TENTHS,
@@ -25,22 +23,15 @@ from homeassistant.core import HomeAssistant
 from homeassistant.helpers.restore_state import RestoreEntity
 from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType
 
-from .base_platform import BasePlatform
+from .base_platform import BaseStructPlatform
 from .const import (
     ATTR_TEMPERATURE,
     CALL_TYPE_REGISTER_HOLDING,
     CALL_TYPE_WRITE_REGISTERS,
     CONF_CLIMATES,
-    CONF_DATA_TYPE,
     CONF_MAX_TEMP,
     CONF_MIN_TEMP,
-    CONF_PRECISION,
-    CONF_SCALE,
     CONF_STEP,
-    CONF_SWAP,
-    CONF_SWAP_BYTE,
-    CONF_SWAP_WORD,
-    CONF_SWAP_WORD_BYTE,
     CONF_TARGET_TEMP,
     DEFAULT_STRUCT_FORMAT,
     MODBUS_DOMAIN,
@@ -69,7 +60,7 @@ async def async_setup_platform(
     async_add_entities(entities)
 
 
-class ModbusThermostat(BasePlatform, RestoreEntity, ClimateEntity):
+class ModbusThermostat(BaseStructPlatform, RestoreEntity, ClimateEntity):
     """Representation of a Modbus Thermostat."""
 
     def __init__(
@@ -82,17 +73,11 @@ class ModbusThermostat(BasePlatform, RestoreEntity, ClimateEntity):
         self._target_temperature_register = config[CONF_TARGET_TEMP]
         self._target_temperature = None
         self._current_temperature = None
-        self._data_type = config[CONF_DATA_TYPE]
         self._structure = config[CONF_STRUCTURE]
-        self._count = config[CONF_COUNT]
-        self._precision = config[CONF_PRECISION]
-        self._scale = config[CONF_SCALE]
-        self._offset = config[CONF_OFFSET]
         self._unit = config[CONF_TEMPERATURE_UNIT]
         self._max_temp = config[CONF_MAX_TEMP]
         self._min_temp = config[CONF_MIN_TEMP]
         self._temp_step = config[CONF_STEP]
-        self._swap = config[CONF_SWAP]
 
     async def async_added_to_hass(self):
         """Handle entity which will be added."""
@@ -175,21 +160,6 @@ class ModbusThermostat(BasePlatform, RestoreEntity, ClimateEntity):
         self._available = result is not None
         await self.async_update()
 
-    def _swap_registers(self, registers):
-        """Do swap as needed."""
-        if self._swap in [CONF_SWAP_BYTE, CONF_SWAP_WORD_BYTE]:
-            # convert [12][34] --> [21][43]
-            for i, register in enumerate(registers):
-                registers[i] = int.from_bytes(
-                    register.to_bytes(2, byteorder="little"),
-                    byteorder="big",
-                    signed=False,
-                )
-        if self._swap in [CONF_SWAP_WORD, CONF_SWAP_WORD_BYTE]:
-            # convert [12][34] ==> [34][12]
-            registers.reverse()
-        return registers
-
     async def async_update(self, now=None):
         """Update Target & Current Temperature."""
         # remark "now" is a dummy parameter to avoid problems with
@@ -217,21 +187,7 @@ class ModbusThermostat(BasePlatform, RestoreEntity, ClimateEntity):
             self._available = False
             return -1
 
-        registers = self._swap_registers(result.registers)
+        self.unpack_structure_result(result.registers)
-        byte_string = b"".join([x.to_bytes(2, byteorder="big") for x in registers])
-        val = struct.unpack(self._structure, byte_string)
-        if len(val) != 1 or not isinstance(val[0], (float, int)):
-            _LOGGER.error(
-                "Unable to parse result as a single int or float value; adjust your configuration. Result: %s",
-                str(val),
-            )
-            return -1
 
-        val2 = val[0]
-        register_value = format(
-            (self._scale * val2) + self._offset, f".{self._precision}f"
-        )
-        register_value2 = float(register_value)
         self._available = True
-
+        return self._value
-        return register_value2

homeassistant/components/modbus/sensor.py

@@ -2,34 +2,16 @@
 from __future__ import annotations
 
 import logging
-import struct
 from typing import Any
 
 from homeassistant.components.sensor import SensorEntity
-from homeassistant.const import (
+from homeassistant.const import CONF_NAME, CONF_SENSORS, CONF_UNIT_OF_MEASUREMENT
-    CONF_COUNT,
-    CONF_NAME,
-    CONF_OFFSET,
-    CONF_SENSORS,
-    CONF_STRUCTURE,
-    CONF_UNIT_OF_MEASUREMENT,
-)
 from homeassistant.core import HomeAssistant
 from homeassistant.helpers.restore_state import RestoreEntity
 from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType
 
-from .base_platform import BasePlatform
+from .base_platform import BaseStructPlatform
-from .const import (
+from .const import MODBUS_DOMAIN
-    CONF_DATA_TYPE,
-    CONF_PRECISION,
-    CONF_SCALE,
-    CONF_SWAP,
-    CONF_SWAP_BYTE,
-    CONF_SWAP_WORD,
-    CONF_SWAP_WORD_BYTE,
-    DATA_TYPE_STRING,
-    MODBUS_DOMAIN,
-)
 from .modbus import ModbusHub
 
 PARALLEL_UPDATES = 1
@@ -55,7 +37,7 @@ async def async_setup_platform(
     async_add_entities(sensors)
 
 
-class ModbusRegisterSensor(BasePlatform, RestoreEntity, SensorEntity):
+class ModbusRegisterSensor(BaseStructPlatform, RestoreEntity, SensorEntity):
     """Modbus register sensor."""
 
     def __init__(
@@ -66,13 +48,6 @@ class ModbusRegisterSensor(BasePlatform, RestoreEntity, SensorEntity):
         """Initialize the modbus register sensor."""
         super().__init__(hub, entry)
         self._unit_of_measurement = entry.get(CONF_UNIT_OF_MEASUREMENT)
-        self._count = int(entry[CONF_COUNT])
-        self._swap = entry[CONF_SWAP]
-        self._scale = entry[CONF_SCALE]
-        self._offset = entry[CONF_OFFSET]
-        self._precision = entry[CONF_PRECISION]
-        self._structure = entry.get(CONF_STRUCTURE)
-        self._data_type = entry[CONF_DATA_TYPE]
 
     async def async_added_to_hass(self):
         """Handle entity which will be added."""
@@ -91,21 +66,6 @@ class ModbusRegisterSensor(BasePlatform, RestoreEntity, SensorEntity):
         """Return the unit of measurement."""
         return self._unit_of_measurement
 
-    def _swap_registers(self, registers):
-        """Do swap as needed."""
-        if self._swap in [CONF_SWAP_BYTE, CONF_SWAP_WORD_BYTE]:
-            # convert [12][34] --> [21][43]
-            for i, register in enumerate(registers):
-                registers[i] = int.from_bytes(
-                    register.to_bytes(2, byteorder="little"),
-                    byteorder="big",
-                    signed=False,
-                )
-        if self._swap in [CONF_SWAP_WORD, CONF_SWAP_WORD_BYTE]:
-            # convert [12][34] ==> [34][12]
-            registers.reverse()
-        return registers
-
     async def async_update(self, now=None):
         """Update the state of the sensor."""
         # remark "now" is a dummy parameter to avoid problems with
@@ -118,41 +78,6 @@ class ModbusRegisterSensor(BasePlatform, RestoreEntity, SensorEntity):
             self.async_write_ha_state()
             return
 
-        registers = self._swap_registers(result.registers)
+        self.unpack_structure_result(result.registers)
-        byte_string = b"".join([x.to_bytes(2, byteorder="big") for x in registers])
-        if self._data_type == DATA_TYPE_STRING:
-            self._value = byte_string.decode()
-        else:
-            val = struct.unpack(self._structure, byte_string)
-
-            # Issue: https://github.com/home-assistant/core/issues/41944
-            # If unpack() returns a tuple greater than 1, don't try to process the value.
-            # Instead, return the values of unpack(...) separated by commas.
-            if len(val) > 1:
-                # Apply scale and precision to floats and ints
-                v_result = []
-                for entry in val:
-                    v_temp = self._scale * entry + self._offset
-
-                    # We could convert int to float, and the code would still work; however
-                    # we lose some precision, and unit tests will fail. Therefore, we do
-                    # the conversion only when it's absolutely necessary.
-                    if isinstance(v_temp, int) and self._precision == 0:
-                        v_result.append(str(v_temp))
-                    else:
-                        v_result.append(f"{float(v_temp):.{self._precision}f}")
-                self._value = ",".join(map(str, v_result))
-            else:
-                # Apply scale and precision to floats and ints
-                val = self._scale * val[0] + self._offset
-
-                # We could convert int to float, and the code would still work; however
-                # we lose some precision, and unit tests will fail. Therefore, we do
-                # the conversion only when it's absolutely necessary.
-                if isinstance(val, int) and self._precision == 0:
-                    self._value = str(val)
-                else:
-                    self._value = f"{float(val):.{self._precision}f}"
-
         self._available = True
         self.async_write_ha_state()