From cca7da77ad9bb8882efca1b71ebae8058fdca648 Mon Sep 17 00:00:00 2001
From: jan iversen <jancasacondor@gmail.com>
Date: Thu, 21 Oct 2021 00:22:01 +0200
Subject: [PATCH] Report modbus buffer too small or too big to unpack (#57838)

---
 .../components/modbus/base_platform.py        | 10 ++++-
 homeassistant/components/modbus/climate.py    |  3 +-
 homeassistant/components/modbus/sensor.py     |  5 ++-
 tests/components/modbus/test_climate.py       |  2 +
 tests/components/modbus/test_sensor.py        | 38 +++++++++++++++++++
 5 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/homeassistant/components/modbus/base_platform.py b/homeassistant/components/modbus/base_platform.py
index 1c4ebe2237d..c962f298586 100644
--- a/homeassistant/components/modbus/base_platform.py
+++ b/homeassistant/components/modbus/base_platform.py
@@ -160,7 +160,7 @@ class BaseStructPlatform(BasePlatform, RestoreEntity):
             registers.reverse()
         return registers
 
-    def unpack_structure_result(self, registers: list[int]) -> str:
+    def unpack_structure_result(self, registers: list[int]) -> str | None:
         """Convert registers to proper result."""
 
         registers = self._swap_registers(registers)
@@ -168,7 +168,13 @@ class BaseStructPlatform(BasePlatform, RestoreEntity):
         if self._data_type == DataType.STRING:
             return byte_string.decode()
 
-        val = struct.unpack(self._structure, byte_string)
+        try:
+            val = struct.unpack(self._structure, byte_string)
+        except struct.error as err:
+            recv_size = len(registers) * 2
+            msg = f"Received {recv_size} bytes, unpack error {err}"
+            _LOGGER.error(msg)
+            return None
         # 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.
diff --git a/homeassistant/components/modbus/climate.py b/homeassistant/components/modbus/climate.py
index 268d7bfd3df..37ff9504b35 100644
--- a/homeassistant/components/modbus/climate.py
+++ b/homeassistant/components/modbus/climate.py
@@ -166,7 +166,8 @@ class ModbusThermostat(BaseStructPlatform, RestoreEntity, ClimateEntity):
 
         self._lazy_errors = self._lazy_error_count
         self._value = self.unpack_structure_result(result.registers)
-        self._attr_available = True
         if not self._value:
+            self._attr_available = False
             return None
+        self._attr_available = True
         return float(self._value)
diff --git a/homeassistant/components/modbus/sensor.py b/homeassistant/components/modbus/sensor.py
index 4cf642ca44b..6911098dd94 100644
--- a/homeassistant/components/modbus/sensor.py
+++ b/homeassistant/components/modbus/sensor.py
@@ -74,6 +74,9 @@ class ModbusRegisterSensor(BaseStructPlatform, RestoreEntity, SensorEntity):
             return
 
         self._attr_native_value = self.unpack_structure_result(result.registers)
+        if self._attr_native_value is None:
+            self._attr_available = False
+        else:
+            self._attr_available = True
         self._lazy_errors = self._lazy_error_count
-        self._attr_available = True
         self.async_write_ha_state()
diff --git a/tests/components/modbus/test_climate.py b/tests/components/modbus/test_climate.py
index 5fc61fed52d..db471709c10 100644
--- a/tests/components/modbus/test_climate.py
+++ b/tests/components/modbus/test_climate.py
@@ -94,11 +94,13 @@ async def test_temperature_climate(hass, expected, mock_do_cycle):
         {
             CONF_CLIMATES: [
                 {
+                    CONF_COUNT: 2,
                     CONF_NAME: TEST_ENTITY_NAME,
                     CONF_TARGET_TEMP: 117,
                     CONF_ADDRESS: 117,
                     CONF_SLAVE: 10,
                     CONF_SCAN_INTERVAL: 0,
+                    CONF_DATA_TYPE: DataType.INT32,
                 }
             ]
         },
diff --git a/tests/components/modbus/test_sensor.py b/tests/components/modbus/test_sensor.py
index 4f631b279c1..bf3e8711922 100644
--- a/tests/components/modbus/test_sensor.py
+++ b/tests/components/modbus/test_sensor.py
@@ -559,6 +559,44 @@ async def test_all_sensor(hass, mock_do_cycle, expected):
     assert hass.states.get(ENTITY_ID).state == expected
 
 
+@pytest.mark.parametrize(
+    "do_config",
+    [
+        {
+            CONF_SENSORS: [
+                {
+                    CONF_NAME: TEST_ENTITY_NAME,
+                    CONF_ADDRESS: 51,
+                    CONF_SCAN_INTERVAL: 1,
+                },
+            ],
+        },
+    ],
+)
+@pytest.mark.parametrize(
+    "config_addon,register_words",
+    [
+        (
+            {
+                CONF_COUNT: 1,
+                CONF_DATA_TYPE: DataType.INT16,
+            },
+            [7, 9],
+        ),
+        (
+            {
+                CONF_COUNT: 2,
+                CONF_DATA_TYPE: DataType.INT32,
+            },
+            [7],
+        ),
+    ],
+)
+async def test_wrong_unpack(hass, mock_do_cycle):
+    """Run test for sensor."""
+    assert hass.states.get(ENTITY_ID).state == STATE_UNAVAILABLE
+
+
 @pytest.mark.parametrize(
     "do_config",
     [