Keep observation data valid for 60 min and retry with no data for nws (#117109)

Co-authored-by: J. Nick Koston <nick@koston.org>

homeassistant/components/nws/__init__.py

@@ -5,10 +5,9 @@ from __future__ import annotations
 from collections.abc import Awaitable, Callable
 from dataclasses import dataclass
 import datetime
-from functools import partial
 import logging
 
-from pynws import SimpleNWS, call_with_retry
+from pynws import NwsNoDataError, SimpleNWS, call_with_retry
 
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import CONF_API_KEY, CONF_LATITUDE, CONF_LONGITUDE, Platform
@@ -16,21 +15,25 @@ from homeassistant.core import HomeAssistant
 from homeassistant.helpers import debounce
 from homeassistant.helpers.aiohttp_client import async_get_clientsession
 from homeassistant.helpers.device_registry import DeviceEntryType, DeviceInfo
-from homeassistant.helpers.update_coordinator import TimestampDataUpdateCoordinator
+from homeassistant.helpers.update_coordinator import (
-from homeassistant.util.dt import utcnow
+    TimestampDataUpdateCoordinator,
+    UpdateFailed,
+)
 
-from .const import CONF_STATION, DOMAIN, UPDATE_TIME_PERIOD
+from .const import (
+    CONF_STATION,
+    DEBOUNCE_TIME,
+    DEFAULT_SCAN_INTERVAL,
+    DOMAIN,
+    RETRY_INTERVAL,
+    RETRY_STOP,
+)
+from .coordinator import NWSObservationDataUpdateCoordinator
 
 _LOGGER = logging.getLogger(__name__)
 
 PLATFORMS = [Platform.SENSOR, Platform.WEATHER]
 
-DEFAULT_SCAN_INTERVAL = datetime.timedelta(minutes=10)
-RETRY_INTERVAL = datetime.timedelta(minutes=1)
-RETRY_STOP = datetime.timedelta(minutes=10)
-
-DEBOUNCE_TIME = 10 * 60  # in seconds
-
 type NWSConfigEntry = ConfigEntry[NWSData]
 
 
@@ -44,7 +47,7 @@ class NWSData:
     """Data for the National Weather Service integration."""
 
     api: SimpleNWS
-    coordinator_observation: TimestampDataUpdateCoordinator[None]
+    coordinator_observation: NWSObservationDataUpdateCoordinator
     coordinator_forecast: TimestampDataUpdateCoordinator[None]
     coordinator_forecast_hourly: TimestampDataUpdateCoordinator[None]
 
@@ -62,55 +65,48 @@ async def async_setup_entry(hass: HomeAssistant, entry: NWSConfigEntry) -> bool:
     nws_data = SimpleNWS(latitude, longitude, api_key, client_session)
     await nws_data.set_station(station)
 
-    def async_setup_update_observation(
-        retry_interval: datetime.timedelta | float,
-        retry_stop: datetime.timedelta | float,
-    ) -> Callable[[], Awaitable[None]]:
-        async def update_observation() -> None:
-            """Retrieve recent observations."""
-            await call_with_retry(
-                nws_data.update_observation,
-                retry_interval,
-                retry_stop,
-                start_time=utcnow() - UPDATE_TIME_PERIOD,
-            )
-
-        return update_observation
-
     def async_setup_update_forecast(
         retry_interval: datetime.timedelta | float,
         retry_stop: datetime.timedelta | float,
     ) -> Callable[[], Awaitable[None]]:
-        return partial(
+        async def update_forecast() -> None:
-            call_with_retry,
+            """Retrieve forecast."""
-            nws_data.update_forecast,
+            try:
-            retry_interval,
+                await call_with_retry(
-            retry_stop,
+                    nws_data.update_forecast,
-        )
+                    retry_interval,
+                    retry_stop,
+                    retry_no_data=True,
+                )
+            except NwsNoDataError as err:
+                raise UpdateFailed("No data returned.") from err
+
+        return update_forecast
 
     def async_setup_update_forecast_hourly(
         retry_interval: datetime.timedelta | float,
         retry_stop: datetime.timedelta | float,
     ) -> Callable[[], Awaitable[None]]:
-        return partial(
+        async def update_forecast_hourly() -> None:
-            call_with_retry,
+            """Retrieve forecast hourly."""
-            nws_data.update_forecast_hourly,
+            try:
-            retry_interval,
+                await call_with_retry(
-            retry_stop,
+                    nws_data.update_forecast_hourly,
-        )
+                    retry_interval,
+                    retry_stop,
+                    retry_no_data=True,
+                )
+            except NwsNoDataError as err:
+                raise UpdateFailed("No data returned.") from err
 
-    # Don't use retries in setup
+        return update_forecast_hourly
-    coordinator_observation = TimestampDataUpdateCoordinator(
+
+    coordinator_observation = NWSObservationDataUpdateCoordinator(
         hass,
-        _LOGGER,
+        nws_data,
-        name=f"NWS observation station {station}",
-        update_method=async_setup_update_observation(0, 0),
-        update_interval=DEFAULT_SCAN_INTERVAL,
-        request_refresh_debouncer=debounce.Debouncer(
-            hass, _LOGGER, cooldown=DEBOUNCE_TIME, immediate=True
-        ),
     )
 
+    # Don't use retries in setup
     coordinator_forecast = TimestampDataUpdateCoordinator(
         hass,
         _LOGGER,
@@ -145,9 +141,6 @@ async def async_setup_entry(hass: HomeAssistant, entry: NWSConfigEntry) -> bool:
     await coordinator_forecast_hourly.async_refresh()
 
     # Use retries
-    coordinator_observation.update_method = async_setup_update_observation(
-        RETRY_INTERVAL, RETRY_STOP
-    )
     coordinator_forecast.update_method = async_setup_update_forecast(
         RETRY_INTERVAL, RETRY_STOP
     )

homeassistant/components/nws/const.py

@@ -76,7 +76,12 @@ CONDITION_CLASSES: dict[str, list[str]] = {
 DAYNIGHT = "daynight"
 HOURLY = "hourly"
 
-OBSERVATION_VALID_TIME = timedelta(minutes=20)
+OBSERVATION_VALID_TIME = timedelta(minutes=60)
 FORECAST_VALID_TIME = timedelta(minutes=45)
 # A lot of stations update once hourly plus some wiggle room
 UPDATE_TIME_PERIOD = timedelta(minutes=70)
+
+DEBOUNCE_TIME = 10 * 60  # in seconds
+DEFAULT_SCAN_INTERVAL = timedelta(minutes=10)
+RETRY_INTERVAL = timedelta(minutes=1)
+RETRY_STOP = timedelta(minutes=10)

homeassistant/components/nws/coordinator.py

@@ -0,0 +1,93 @@
+"""The NWS coordinator."""
+
+from datetime import datetime
+import logging
+
+from aiohttp import ClientResponseError
+from pynws import NwsNoDataError, SimpleNWS, call_with_retry
+
+from homeassistant.core import HomeAssistant
+from homeassistant.helpers import debounce
+from homeassistant.helpers.update_coordinator import (
+    TimestampDataUpdateCoordinator,
+    UpdateFailed,
+)
+from homeassistant.util.dt import utcnow
+
+from .const import (
+    DEBOUNCE_TIME,
+    DEFAULT_SCAN_INTERVAL,
+    OBSERVATION_VALID_TIME,
+    RETRY_INTERVAL,
+    RETRY_STOP,
+    UPDATE_TIME_PERIOD,
+)
+
+_LOGGER = logging.getLogger(__name__)
+
+
+class NWSObservationDataUpdateCoordinator(TimestampDataUpdateCoordinator[None]):
+    """Class to manage fetching NWS observation data."""
+
+    def __init__(
+        self,
+        hass: HomeAssistant,
+        nws: SimpleNWS,
+    ) -> None:
+        """Initialize."""
+        self.nws = nws
+        self.last_api_success_time: datetime | None = None
+        self.initialized: bool = False
+
+        super().__init__(
+            hass,
+            _LOGGER,
+            name=f"NWS observation station {nws.station}",
+            update_interval=DEFAULT_SCAN_INTERVAL,
+            request_refresh_debouncer=debounce.Debouncer(
+                hass, _LOGGER, cooldown=DEBOUNCE_TIME, immediate=True
+            ),
+        )
+
+    async def _async_update_data(self) -> None:
+        """Update data via library."""
+        if not self.initialized:
+            await self._async_first_update_data()
+        else:
+            await self._async_subsequent_update_data()
+
+    async def _async_first_update_data(self):
+        """Update data without retries first."""
+        try:
+            await self.nws.update_observation(
+                raise_no_data=True,
+                start_time=utcnow() - UPDATE_TIME_PERIOD,
+            )
+        except (NwsNoDataError, ClientResponseError) as err:
+            raise UpdateFailed(err) from err
+        else:
+            self.last_api_success_time = utcnow()
+        finally:
+            self.initialized = True
+
+    async def _async_subsequent_update_data(self) -> None:
+        """Update data with retries and caching data over multiple failed rounds."""
+        try:
+            await call_with_retry(
+                self.nws.update_observation,
+                RETRY_INTERVAL,
+                RETRY_STOP,
+                retry_no_data=True,
+                start_time=utcnow() - UPDATE_TIME_PERIOD,
+            )
+        except (NwsNoDataError, ClientResponseError) as err:
+            if not self.last_api_success_time or (
+                utcnow() - self.last_api_success_time > OBSERVATION_VALID_TIME
+            ):
+                raise UpdateFailed(err) from err
+            _LOGGER.debug(
+                "NWS observation update failed, but data still valid. Last success: %s",
+                self.last_api_success_time,
+            )
+        else:
+            self.last_api_success_time = utcnow()

homeassistant/components/nws/sensor.py

@@ -28,7 +28,6 @@ from homeassistant.helpers.update_coordinator import (
     CoordinatorEntity,
     TimestampDataUpdateCoordinator,
 )
-from homeassistant.util.dt import utcnow
 from homeassistant.util.unit_conversion import (
     DistanceConverter,
     PressureConverter,
@@ -37,7 +36,7 @@ from homeassistant.util.unit_conversion import (
 from homeassistant.util.unit_system import US_CUSTOMARY_SYSTEM
 
 from . import NWSConfigEntry, NWSData, base_unique_id, device_info
-from .const import ATTRIBUTION, CONF_STATION, OBSERVATION_VALID_TIME
+from .const import ATTRIBUTION, CONF_STATION
 
 PARALLEL_UPDATES = 0
 
@@ -225,15 +224,3 @@ class NWSSensor(CoordinatorEntity[TimestampDataUpdateCoordinator[None]], SensorE
         if unit_of_measurement == PERCENTAGE:
             return round(value)
         return value
-
-    @property
-    def available(self) -> bool:
-        """Return if state is available."""
-        if self.coordinator.last_update_success_time:
-            last_success_time = (
-                utcnow() - self.coordinator.last_update_success_time
-                < OBSERVATION_VALID_TIME
-            )
-        else:
-            last_success_time = False
-        return self.coordinator.last_update_success or last_success_time

tests/components/nws/test_weather.py

@@ -5,10 +5,15 @@ from unittest.mock import patch
 
 import aiohttp
 from freezegun.api import FrozenDateTimeFactory
+from pynws import NwsNoDataError
 import pytest
 from syrupy.assertion import SnapshotAssertion
 
 from homeassistant.components import nws
+from homeassistant.components.nws.const import (
+    DEFAULT_SCAN_INTERVAL,
+    OBSERVATION_VALID_TIME,
+)
 from homeassistant.components.weather import (
     ATTR_CONDITION_CLEAR_NIGHT,
     ATTR_CONDITION_SUNNY,
@@ -114,6 +119,116 @@ async def test_none_values(hass: HomeAssistant, mock_simple_nws, no_sensor) -> N
         assert data.get(key) is None
 
 
+async def test_data_caching_error_observation(
+    hass: HomeAssistant,
+    freezer: FrozenDateTimeFactory,
+    mock_simple_nws,
+    no_sensor,
+    caplog,
+) -> None:
+    """Test caching of data with errors."""
+    with (
+        patch("homeassistant.components.nws.coordinator.RETRY_STOP", 0),
+        patch("homeassistant.components.nws.coordinator.RETRY_INTERVAL", 0),
+    ):
+        instance = mock_simple_nws.return_value
+
+        entry = MockConfigEntry(
+            domain=nws.DOMAIN,
+            data=NWS_CONFIG,
+        )
+        entry.add_to_hass(hass)
+        await hass.config_entries.async_setup(entry.entry_id)
+        await hass.async_block_till_done()
+
+        state = hass.states.get("weather.abc")
+        assert state.state == "sunny"
+
+        # data is still valid even when update fails
+        instance.update_observation.side_effect = NwsNoDataError("Test")
+
+        freezer.tick(DEFAULT_SCAN_INTERVAL + timedelta(seconds=100))
+        async_fire_time_changed(hass)
+        await hass.async_block_till_done()
+
+        state = hass.states.get("weather.abc")
+        assert state.state == "sunny"
+
+        assert (
+            "NWS observation update failed, but data still valid. Last success: "
+            in caplog.text
+        )
+
+        # data is no longer valid after OBSERVATION_VALID_TIME
+        freezer.tick(OBSERVATION_VALID_TIME + timedelta(seconds=1))
+        async_fire_time_changed(hass)
+        await hass.async_block_till_done()
+
+        state = hass.states.get("weather.abc")
+        assert state.state == STATE_UNAVAILABLE
+
+        assert "Error fetching NWS observation station ABC data: Test" in caplog.text
+
+
+async def test_no_data_error_observation(
+    hass: HomeAssistant, mock_simple_nws, no_sensor, caplog
+) -> None:
+    """Test catching NwsNoDataDrror."""
+    instance = mock_simple_nws.return_value
+    instance.update_observation.side_effect = NwsNoDataError("Test")
+
+    entry = MockConfigEntry(
+        domain=nws.DOMAIN,
+        data=NWS_CONFIG,
+    )
+    entry.add_to_hass(hass)
+    await hass.config_entries.async_setup(entry.entry_id)
+    await hass.async_block_till_done()
+
+    assert "Error fetching NWS observation station ABC data: Test" in caplog.text
+
+
+async def test_no_data_error_forecast(
+    hass: HomeAssistant, mock_simple_nws, no_sensor, caplog
+) -> None:
+    """Test catching NwsNoDataDrror."""
+    instance = mock_simple_nws.return_value
+    instance.update_forecast.side_effect = NwsNoDataError("Test")
+
+    entry = MockConfigEntry(
+        domain=nws.DOMAIN,
+        data=NWS_CONFIG,
+    )
+    entry.add_to_hass(hass)
+    await hass.config_entries.async_setup(entry.entry_id)
+    await hass.async_block_till_done()
+
+    assert (
+        "Error fetching NWS forecast station ABC data: No data returned" in caplog.text
+    )
+
+
+async def test_no_data_error_forecast_hourly(
+    hass: HomeAssistant, mock_simple_nws, no_sensor, caplog
+) -> None:
+    """Test catching NwsNoDataDrror."""
+    instance = mock_simple_nws.return_value
+    instance.update_forecast_hourly.side_effect = NwsNoDataError("Test")
+
+    entry = MockConfigEntry(
+        domain=nws.DOMAIN,
+        data=NWS_CONFIG,
+    )
+    entry.add_to_hass(hass)
+    await hass.config_entries.async_setup(entry.entry_id)
+    await hass.async_block_till_done()
+
+    assert (
+        "Error fetching NWS forecast hourly station ABC data: No data returned"
+        in caplog.text
+    )
+
+
 async def test_none(hass: HomeAssistant, mock_simple_nws, no_sensor) -> None:
     """Test with None as observation and forecast."""
     instance = mock_simple_nws.return_value
@@ -188,7 +303,7 @@ async def test_error_observation(
 ) -> None:
     """Test error during update observation."""
     utc_time = dt_util.utcnow()
-    with patch("homeassistant.components.nws.utcnow") as mock_utc:
+    with patch("homeassistant.components.nws.coordinator.utcnow") as mock_utc:
         mock_utc.return_value = utc_time
         instance = mock_simple_nws.return_value
         # first update fails