Allow retries on communication exceptions for Aurora ABB Powerone solar inverter (#104492)

* Allow retries on SerialException, AuroraError

* Add test to verify that retry is occuring

* Fix tests and indents

* Only log to info level for normal on/offline

* Review comment: don't log warning, debug and raise UpdateFailed

* Fix tests

homeassistant/components/aurora_abb_powerone/__init__.py

@@ -11,13 +11,15 @@
 # sudo chmod 777 /dev/ttyUSB0
 
 import logging
+from time import sleep
 
 from aurorapy.client import AuroraError, AuroraSerialClient, AuroraTimeoutError
+from serial import SerialException
 
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import CONF_ADDRESS, CONF_PORT, Platform
 from homeassistant.core import HomeAssistant
-from homeassistant.helpers.update_coordinator import DataUpdateCoordinator
+from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed
 
 from .const import DOMAIN, SCAN_INTERVAL
 
@@ -69,38 +71,49 @@ class AuroraAbbDataUpdateCoordinator(DataUpdateCoordinator[dict[str, float]]):
         """
         data: dict[str, float] = {}
         self.available_prev = self.available
-        try:
+        retries: int = 3
-            self.client.connect()
+        while retries > 0:
+            try:
+                self.client.connect()
 
-            # read ADC channel 3 (grid power output)
+                # read ADC channel 3 (grid power output)
-            power_watts = self.client.measure(3, True)
+                power_watts = self.client.measure(3, True)
-            temperature_c = self.client.measure(21)
+                temperature_c = self.client.measure(21)
-            energy_wh = self.client.cumulated_energy(5)
+                energy_wh = self.client.cumulated_energy(5)
-            [alarm, *_] = self.client.alarms()
+                [alarm, *_] = self.client.alarms()
-        except AuroraTimeoutError:
+            except AuroraTimeoutError:
-            self.available = False
+                self.available = False
-            _LOGGER.debug("No response from inverter (could be dark)")
+                _LOGGER.debug("No response from inverter (could be dark)")
-        except AuroraError as error:
+                retries = 0
-            self.available = False
+            except (SerialException, AuroraError) as error:
-            raise error
+                self.available = False
-        else:
+                retries -= 1
-            data["instantaneouspower"] = round(power_watts, 1)
+                if retries <= 0:
-            data["temp"] = round(temperature_c, 1)
+                    raise UpdateFailed(error) from error
-            data["totalenergy"] = round(energy_wh / 1000, 2)
+                _LOGGER.debug(
-            data["alarm"] = alarm
+                    "Exception: %s occurred, %d retries remaining",
-            self.available = True
+                    repr(error),
-
+                    retries,
-        finally:
+                )
-            if self.available != self.available_prev:
+                sleep(1)
-                if self.available:
+            else:
-                    _LOGGER.info("Communication with %s back online", self.name)
+                data["instantaneouspower"] = round(power_watts, 1)
-                else:
+                data["temp"] = round(temperature_c, 1)
-                    _LOGGER.warning(
+                data["totalenergy"] = round(energy_wh / 1000, 2)
-                        "Communication with %s lost",
+                data["alarm"] = alarm
-                        self.name,
+                self.available = True
-                    )
+                retries = 0
-            if self.client.serline.isOpen():
+            finally:
-                self.client.close()
+                if self.available != self.available_prev:
+                    if self.available:
+                        _LOGGER.info("Communication with %s back online", self.name)
+                    else:
+                        _LOGGER.info(
+                            "Communication with %s lost",
+                            self.name,
+                        )
+                if self.client.serline.isOpen():
+                    self.client.close()
 
         return data
 

tests/components/aurora_abb_powerone/test_sensor.py

@@ -4,6 +4,7 @@ from unittest.mock import patch
 
 from aurorapy.client import AuroraError, AuroraTimeoutError
 from freezegun.api import FrozenDateTimeFactory
+import pytest
 
 from homeassistant.components.aurora_abb_powerone.const import (
     ATTR_DEVICE_NAME,
@@ -171,18 +172,39 @@ async def test_sensor_dark(hass: HomeAssistant, freezer: FrozenDateTimeFactory)
         assert power.state == "unknown"  # should this be 'available'?
 
 
-async def test_sensor_unknown_error(hass: HomeAssistant) -> None:
+async def test_sensor_unknown_error(
+    hass: HomeAssistant,
+    caplog: pytest.LogCaptureFixture,
+    freezer: FrozenDateTimeFactory,
+) -> None:
     """Test other comms error is handled correctly."""
     mock_entry = _mock_config_entry()
 
+    # sun is up
+    with patch("aurorapy.client.AuroraSerialClient.connect", return_value=None), patch(
+        "aurorapy.client.AuroraSerialClient.measure", side_effect=_simulated_returns
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.alarms", return_value=["No alarm"]
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.cumulated_energy",
+        side_effect=_simulated_returns,
+    ):
+        mock_entry.add_to_hass(hass)
+        await hass.config_entries.async_setup(mock_entry.entry_id)
+        await hass.async_block_till_done()
+
     with patch("aurorapy.client.AuroraSerialClient.connect", return_value=None), patch(
         "aurorapy.client.AuroraSerialClient.measure",
         side_effect=AuroraError("another error"),
     ), patch(
         "aurorapy.client.AuroraSerialClient.alarms", return_value=["No alarm"]
     ), patch("serial.Serial.isOpen", return_value=True):
-        mock_entry.add_to_hass(hass)
+        freezer.tick(SCAN_INTERVAL * 2)
-        await hass.config_entries.async_setup(mock_entry.entry_id)
+        async_fire_time_changed(hass)
         await hass.async_block_till_done()
+        assert (
+            "Exception: AuroraError('another error') occurred, 2 retries remaining"
+            in caplog.text
+        )
         power = hass.states.get("sensor.mydevicename_power_output")
-        assert power is None
+        assert power.state == "unavailable"