From 37bed6460743058deff3b44898b78aa560f85365 Mon Sep 17 00:00:00 2001 From: Matthias Alphart Date: Sun, 19 Dec 2021 11:37:14 +0100 Subject: [PATCH] Silently retry Fronius inverter endpoint 2 times (#61826) --- .../components/fronius/coordinator.py | 24 +++++++++--- tests/components/fronius/test_coordinator.py | 38 ++++++++++++++----- 2 files changed, 47 insertions(+), 15 deletions(-) diff --git a/homeassistant/components/fronius/coordinator.py b/homeassistant/components/fronius/coordinator.py index e89f828f47d208..f5ea498e381301 100644 --- a/homeassistant/components/fronius/coordinator.py +++ b/homeassistant/components/fronius/coordinator.py @@ -5,7 +5,7 @@ from datetime import timedelta from typing import TYPE_CHECKING, Any, Dict, TypeVar -from pyfronius import FroniusError +from pyfronius import BadStatusError, FroniusError from homeassistant.components.sensor import SensorEntityDescription from homeassistant.core import callback @@ -43,6 +43,8 @@ class FroniusCoordinatorBase( error_interval: timedelta valid_descriptions: list[SensorEntityDescription] + MAX_FAILED_UPDATES = 3 + def __init__(self, *args: Any, solar_net: FroniusSolarNet, **kwargs: Any) -> None: """Set up the FroniusCoordinatorBase class.""" self._failed_update_count = 0 @@ -62,7 +64,7 @@ async def _async_update_data(self) -> dict[SolarNetId, Any]: data = await self._update_method() except FroniusError as err: self._failed_update_count += 1 - if self._failed_update_count == 3: + if self._failed_update_count == self.MAX_FAILED_UPDATES: self.update_interval = self.error_interval raise UpdateFailed(err) from err @@ -116,6 +118,8 @@ class FroniusInverterUpdateCoordinator(FroniusCoordinatorBase): error_interval = timedelta(minutes=10) valid_descriptions = INVERTER_ENTITY_DESCRIPTIONS + SILENT_RETRIES = 3 + def __init__( self, *args: Any, inverter_info: FroniusDeviceInfo, **kwargs: Any ) -> None: @@ -125,9 +129,19 @@ def __init__( async def _update_method(self) -> dict[SolarNetId, Any]: """Return data per solar net id from pyfronius.""" - data = await self.solar_net.fronius.current_inverter_data( - self.inverter_info.solar_net_id - ) + # almost 1% of `current_inverter_data` requests on Symo devices result in + # `BadStatusError Code: 8 - LNRequestTimeout` due to flaky internal + # communication between the logger and the inverter. + for silent_retry in range(self.SILENT_RETRIES): + try: + data = await self.solar_net.fronius.current_inverter_data( + self.inverter_info.solar_net_id + ) + except BadStatusError as err: + if silent_retry == (self.SILENT_RETRIES - 1): + raise err + continue + break # wrap a single devices data in a dict with solar_net_id key for # FroniusCoordinatorBase _async_update_data and add_entities_for_seen_keys return {self.inverter_info.solar_net_id: data} diff --git a/tests/components/fronius/test_coordinator.py b/tests/components/fronius/test_coordinator.py index b729c4d97aca88..a2368975128115 100644 --- a/tests/components/fronius/test_coordinator.py +++ b/tests/components/fronius/test_coordinator.py @@ -1,7 +1,7 @@ """Test the Fronius update coordinators.""" from unittest.mock import patch -from pyfronius import FroniusError +from pyfronius import BadStatusError, FroniusError from homeassistant.components.fronius.coordinator import ( FroniusInverterUpdateCoordinator, @@ -18,27 +18,32 @@ async def test_adaptive_update_interval(hass, aioclient_mock): with patch("pyfronius.Fronius.current_inverter_data") as mock_inverter_data: mock_responses(aioclient_mock) await setup_fronius_integration(hass) - assert mock_inverter_data.call_count == 1 + mock_inverter_data.assert_called_once() + mock_inverter_data.reset_mock() async_fire_time_changed( hass, dt.utcnow() + FroniusInverterUpdateCoordinator.default_interval ) await hass.async_block_till_done() - assert mock_inverter_data.call_count == 2 + mock_inverter_data.assert_called_once() + mock_inverter_data.reset_mock() - mock_inverter_data.side_effect = FroniusError - # first 3 requests at default interval - 4th has different interval - for _ in range(4): + mock_inverter_data.side_effect = FroniusError() + # first 3 bad requests at default interval - 4th has different interval + for _ in range(3): async_fire_time_changed( hass, dt.utcnow() + FroniusInverterUpdateCoordinator.default_interval ) await hass.async_block_till_done() - assert mock_inverter_data.call_count == 5 + assert mock_inverter_data.call_count == 3 + mock_inverter_data.reset_mock() + async_fire_time_changed( hass, dt.utcnow() + FroniusInverterUpdateCoordinator.error_interval ) await hass.async_block_till_done() - assert mock_inverter_data.call_count == 6 + assert mock_inverter_data.call_count == 1 + mock_inverter_data.reset_mock() mock_inverter_data.side_effect = None # next successful request resets to default interval @@ -46,10 +51,23 @@ async def test_adaptive_update_interval(hass, aioclient_mock): hass, dt.utcnow() + FroniusInverterUpdateCoordinator.error_interval ) await hass.async_block_till_done() - assert mock_inverter_data.call_count == 7 + mock_inverter_data.assert_called_once() + mock_inverter_data.reset_mock() async_fire_time_changed( hass, dt.utcnow() + FroniusInverterUpdateCoordinator.default_interval ) await hass.async_block_till_done() - assert mock_inverter_data.call_count == 8 + mock_inverter_data.assert_called_once() + mock_inverter_data.reset_mock() + + # BadStatusError on inverter endpoints have special handling + mock_inverter_data.side_effect = BadStatusError("mock_endpoint", 8) + # first 3 requests at default interval - 4th has different interval + for _ in range(3): + async_fire_time_changed( + hass, dt.utcnow() + FroniusInverterUpdateCoordinator.default_interval + ) + await hass.async_block_till_done() + # BadStatusError does 3 silent retries for inverter endpoint * 3 request intervals = 9 + assert mock_inverter_data.call_count == 9