modbus: Repair swap for slaves (#97960)

homeassistant/components/modbus/base_platform.py

@@ -50,10 +50,12 @@ from .const import (
     CONF_NAN_VALUE,
     CONF_PRECISION,
     CONF_SCALE,
+    CONF_SLAVE_COUNT,
     CONF_STATE_OFF,
     CONF_STATE_ON,
     CONF_SWAP,
     CONF_SWAP_BYTE,
+    CONF_SWAP_NONE,
     CONF_SWAP_WORD,
     CONF_SWAP_WORD_BYTE,
     CONF_VERIFY,
@@ -154,15 +156,25 @@ class BaseStructPlatform(BasePlatform, RestoreEntity):
         """Initialize the switch."""
         super().__init__(hub, config)
         self._swap = config[CONF_SWAP]
+        if self._swap == CONF_SWAP_NONE:
+            self._swap = None
         self._data_type = config[CONF_DATA_TYPE]
         self._structure: str = config[CONF_STRUCTURE]
         self._precision = config[CONF_PRECISION]
         self._scale = config[CONF_SCALE]
         self._offset = config[CONF_OFFSET]
-        self._count = config[CONF_COUNT]
+        self._slave_count = config.get(CONF_SLAVE_COUNT, 0)
+        self._slave_size = self._count = config[CONF_COUNT]
 
-    def _swap_registers(self, registers: list[int]) -> list[int]:
+    def _swap_registers(self, registers: list[int], slave_count: int) -> list[int]:
         """Do swap as needed."""
+        if slave_count:
+            swapped = []
+            for i in range(0, self._slave_count + 1):
+                inx = i * self._slave_size
+                inx2 = inx + self._slave_size
+                swapped.extend(self._swap_registers(registers[inx:inx2], 0))
+            return swapped
         if self._swap in (CONF_SWAP_BYTE, CONF_SWAP_WORD_BYTE):
             # convert [12][34] --> [21][43]
             for i, register in enumerate(registers):
@@ -192,7 +204,8 @@ class BaseStructPlatform(BasePlatform, RestoreEntity):
     def unpack_structure_result(self, registers: list[int]) -> str | None:
         """Convert registers to proper result."""
 
-        registers = self._swap_registers(registers)
+        if self._swap:
+            registers = self._swap_registers(registers, self._slave_count)
         byte_string = b"".join([x.to_bytes(2, byteorder="big") for x in registers])
         if self._data_type == DataType.STRING:
             return byte_string.decode()

homeassistant/components/modbus/climate.py

@@ -210,7 +210,7 @@ class ModbusThermostat(BaseStructPlatform, RestoreEntity, ClimateEntity):
             int.from_bytes(as_bytes[i : i + 2], "big")
             for i in range(0, len(as_bytes), 2)
         ]
-        registers = self._swap_registers(raw_regs)
+        registers = self._swap_registers(raw_regs, 0)
 
         if self._data_type in (
             DataType.INT16,

homeassistant/components/modbus/sensor.py

@@ -134,10 +134,7 @@ class ModbusRegisterSensor(BaseStructPlatform, RestoreSensor, SensorEntity):
                 self._coordinator.async_set_updated_data(None)
         else:
             self._attr_native_value = result
-        if self._attr_native_value is None:
+        self._attr_available = self._attr_native_value is not None
-            self._attr_available = False
-        else:
-            self._attr_available = True
         self._lazy_errors = self._lazy_error_count
         self.async_write_ha_state()
 

tests/components/modbus/test_sensor.py

@@ -615,9 +615,7 @@ async def test_all_sensor(hass: HomeAssistant, mock_do_cycle, expected) -> None:
                     CONF_ADDRESS: 51,
                     CONF_INPUT_TYPE: CALL_TYPE_REGISTER_HOLDING,
                     CONF_DATA_TYPE: DataType.UINT32,
-                    CONF_SCALE: 1,
+                    CONF_SCAN_INTERVAL: 1,
-                    CONF_OFFSET: 0,
-                    CONF_PRECISION: 0,
                 },
             ],
         },
@@ -689,17 +687,184 @@ async def test_all_sensor(hass: HomeAssistant, mock_do_cycle, expected) -> None:
 )
 async def test_slave_sensor(hass: HomeAssistant, mock_do_cycle, expected) -> None:
     """Run test for sensor."""
-    assert hass.states.get(ENTITY_ID).state == expected[0]
     entity_registry = er.async_get(hass)
-
+    for i in range(0, len(expected)):
-    for i in range(1, len(expected)):
+        entity_id = f"{SENSOR_DOMAIN}.{TEST_ENTITY_NAME}".replace(" ", "_")
-        entity_id = f"{SENSOR_DOMAIN}.{TEST_ENTITY_NAME}_{i}".replace(" ", "_")
+        unique_id = f"{SLAVE_UNIQUE_ID}"
-        assert hass.states.get(entity_id).state == expected[i]
+        if i:
-        unique_id = f"{SLAVE_UNIQUE_ID}_{i}"
+            entity_id = f"{entity_id}_{i}"
+            unique_id = f"{unique_id}_{i}"
         entry = entity_registry.async_get(entity_id)
+        state = hass.states.get(entity_id).state
+        assert state == expected[i]
         assert entry.unique_id == unique_id
 
 
+@pytest.mark.parametrize(
+    "do_config",
+    [
+        {
+            CONF_SENSORS: [
+                {
+                    CONF_NAME: TEST_ENTITY_NAME,
+                    CONF_ADDRESS: 51,
+                    CONF_INPUT_TYPE: CALL_TYPE_REGISTER_HOLDING,
+                    CONF_SCAN_INTERVAL: 1,
+                },
+            ],
+        },
+    ],
+)
+@pytest.mark.parametrize(
+    ("config_addon", "register_words", "do_exception", "expected"),
+    [
+        (
+            {
+                CONF_SLAVE_COUNT: 0,
+                CONF_UNIQUE_ID: SLAVE_UNIQUE_ID,
+                CONF_SWAP: CONF_SWAP_BYTE,
+                CONF_DATA_TYPE: DataType.UINT16,
+            },
+            [0x0102],
+            False,
+            [str(int(0x0201))],
+        ),
+        (
+            {
+                CONF_SLAVE_COUNT: 0,
+                CONF_UNIQUE_ID: SLAVE_UNIQUE_ID,
+                CONF_SWAP: CONF_SWAP_WORD,
+                CONF_DATA_TYPE: DataType.UINT32,
+            },
+            [0x0102, 0x0304],
+            False,
+            [str(int(0x03040102))],
+        ),
+        (
+            {
+                CONF_SLAVE_COUNT: 0,
+                CONF_UNIQUE_ID: SLAVE_UNIQUE_ID,
+                CONF_SWAP: CONF_SWAP_WORD,
+                CONF_DATA_TYPE: DataType.UINT64,
+            },
+            [0x0102, 0x0304, 0x0506, 0x0708],
+            False,
+            [str(int(0x0708050603040102))],
+        ),
+        (
+            {
+                CONF_SLAVE_COUNT: 1,
+                CONF_UNIQUE_ID: SLAVE_UNIQUE_ID,
+                CONF_DATA_TYPE: DataType.UINT16,
+                CONF_SWAP: CONF_SWAP_BYTE,
+            },
+            [0x0102, 0x0304],
+            False,
+            [str(int(0x0201)), str(int(0x0403))],
+        ),
+        (
+            {
+                CONF_SLAVE_COUNT: 1,
+                CONF_UNIQUE_ID: SLAVE_UNIQUE_ID,
+                CONF_DATA_TYPE: DataType.UINT32,
+                CONF_SWAP: CONF_SWAP_WORD,
+            },
+            [0x0102, 0x0304, 0x0506, 0x0708],
+            False,
+            [str(int(0x03040102)), str(int(0x07080506))],
+        ),
+        (
+            {
+                CONF_SLAVE_COUNT: 1,
+                CONF_UNIQUE_ID: SLAVE_UNIQUE_ID,
+                CONF_DATA_TYPE: DataType.UINT64,
+                CONF_SWAP: CONF_SWAP_WORD,
+            },
+            [0x0102, 0x0304, 0x0506, 0x0708, 0x0901, 0x0902, 0x0903, 0x0904],
+            False,
+            [str(int(0x0708050603040102)), str(int(0x0904090309020901))],
+        ),
+        (
+            {
+                CONF_SLAVE_COUNT: 3,
+                CONF_UNIQUE_ID: SLAVE_UNIQUE_ID,
+                CONF_DATA_TYPE: DataType.UINT16,
+                CONF_SWAP: CONF_SWAP_BYTE,
+            },
+            [0x0102, 0x0304, 0x0506, 0x0708],
+            False,
+            [str(int(0x0201)), str(int(0x0403)), str(int(0x0605)), str(int(0x0807))],
+        ),
+        (
+            {
+                CONF_SLAVE_COUNT: 3,
+                CONF_UNIQUE_ID: SLAVE_UNIQUE_ID,
+                CONF_DATA_TYPE: DataType.UINT32,
+                CONF_SWAP: CONF_SWAP_WORD,
+            },
+            [
+                0x0102,
+                0x0304,
+                0x0506,
+                0x0708,
+                0x090A,
+                0x0B0C,
+                0x0D0E,
+                0x0F00,
+            ],
+            False,
+            [
+                str(int(0x03040102)),
+                str(int(0x07080506)),
+                str(int(0x0B0C090A)),
+                str(int(0x0F000D0E)),
+            ],
+        ),
+        (
+            {
+                CONF_SLAVE_COUNT: 3,
+                CONF_UNIQUE_ID: SLAVE_UNIQUE_ID,
+                CONF_DATA_TYPE: DataType.UINT64,
+                CONF_SWAP: CONF_SWAP_WORD,
+            },
+            [
+                0x0601,
+                0x0602,
+                0x0603,
+                0x0604,
+                0x0701,
+                0x0702,
+                0x0703,
+                0x0704,
+                0x0801,
+                0x0802,
+                0x0803,
+                0x0804,
+                0x0901,
+                0x0902,
+                0x0903,
+                0x0904,
+            ],
+            False,
+            [
+                str(int(0x0604060306020601)),
+                str(int(0x0704070307020701)),
+                str(int(0x0804080308020801)),
+                str(int(0x0904090309020901)),
+            ],
+        ),
+    ],
+)
+async def test_slave_swap_sensor(hass: HomeAssistant, mock_do_cycle, expected) -> None:
+    """Run test for sensor."""
+    for i in range(0, len(expected)):
+        entity_id = f"{SENSOR_DOMAIN}.{TEST_ENTITY_NAME}".replace(" ", "_")
+        if i:
+            entity_id = f"{entity_id}_{i}"
+        state = hass.states.get(entity_id).state
+        assert state == expected[i]
+
+
 @pytest.mark.parametrize(
     "do_config",
     [