Fox cloud: Fix stale cache settings (#4196)

Author: springfall2008

apps/predbat/fox.py

@@ -58,8 +58,14 @@
     "minsoc": {"value": 10},
 }
 
+# Bump this whenever the shape/derivation of device_settings entries changes (e.g. adding
+# range/unit/precision to schedule-derived settings) so a persisted cache from before that
+# change is detected as stale and forces one settings/scheduler refresh regardless of age,
+# instead of being reused as-is - potentially forever, since nothing else would ever correct it.
+FOX_SETTINGS_CACHE_VERSION = 2
+
 # Storage cache keys for device data persisted between reboots
-FOX_CACHE_KEYS = ["device_list", "device_detail", "battery_charging_time", "device_settings", "device_settings_unavailable", "scheduler_state", "device_values", "device_production_month"]
+FOX_CACHE_KEYS = ["device_list", "device_detail", "battery_charging_time", "device_settings", "device_settings_unavailable", "device_settings_version", "scheduler_state", "device_values", "device_production_month"]
 
 # Maximum age (minutes) of cached data before an API refresh is triggered
 FOX_REFRESH_STATIC = 24 * 60  # Device list, detail and battery charge times rarely change
@@ -344,6 +350,10 @@ def initialize(self, key, automatic, automatic_ignore_pv=False, inverter_sn=None
         # {deviceSN: [key_lower, ...]} of settings the device has reported as unsupported (errno
         # 42015/44096), so they are never polled or written to again
         self.device_settings_unavailable = {}
+        # Version of the persisted device_settings cache actually on disk; 0 (never matches
+        # FOX_SETTINGS_CACHE_VERSION) until load_cached_data() loads a real value, so a fresh
+        # install/first-ever poll is treated the same as a stale cache - both force one refresh
+        self.device_settings_version = 0
         # Set within request_get_func for the duration of a single request_get() call, so callers
         # can tell an "unsupported" failure (permanent) apart from a transient one
         self.last_unsupported = False
@@ -473,8 +483,14 @@ async def run(self, seconds, first):
                 if sn:
                     await self.get_device_history(sn)
 
-        # Device settings and scheduler - refresh based on age
-        settings_refresh = self._needs_refresh("device_settings", FOX_REFRESH_SETTINGS)
+        # Device settings and scheduler - refresh based on age. Also force a refresh, regardless
+        # of age, when the persisted cache predates FOX_SETTINGS_CACHE_VERSION - a one-time
+        # self-heal after a code update changes how settings are derived/shaped (e.g. adding
+        # range/unit/precision), so a customer isn't stuck reusing a stale-shaped cached value
+        # for up to FOX_REFRESH_SETTINGS. Once refreshed, the version is saved and this stops
+        # firing - restarts do not otherwise force a refresh, to avoid hammering the API.
+        stale_cache_version = self.device_settings_version != FOX_SETTINGS_CACHE_VERSION
+        settings_refresh = stale_cache_version or self._needs_refresh("device_settings", FOX_REFRESH_SETTINGS)
         if settings_refresh:
             settings_updated = False
             scheduler_updated = False
@@ -486,8 +502,17 @@ async def run(self, seconds, first):
                         settings_updated = True
                     if await self.get_scheduler(sn) is not None:
                         scheduler_updated = True
-            if settings_updated:
+            if settings_updated or scheduler_updated:
+                # update_settings_from_schedule() (called from get_scheduler()) mutates
+                # device_settings too, so this must save on scheduler_updated as well - not
+                # just settings_updated - or a schedule-derived upgrade is lost on restart
                 await self._save_cache("device_settings", self.device_settings)
+            if stale_cache_version and scheduler_updated:
+                # Only get_scheduler() -> update_settings_from_schedule() can actually produce
+                # the upgraded (range/unit/precision) shape this version tracks, so the version
+                # must not be marked current on settings_updated alone
+                self.device_settings_version = FOX_SETTINGS_CACHE_VERSION
+                await self._save_cache("device_settings_version", FOX_SETTINGS_CACHE_VERSION)
             if self.device_settings_unavailable:
                 await self._save_cache("device_settings_unavailable", self.device_settings_unavailable)
             if scheduler_updated:
@@ -621,6 +646,10 @@ async def load_cached_data(self):
         if device_settings_unavailable is not None:
             self.device_settings_unavailable = device_settings_unavailable
 
+        device_settings_version = await self._load_cache("device_settings_version")
+        if device_settings_version is not None:
+            self.device_settings_version = device_settings_version
+
         scheduler_state = await self._load_cache("scheduler_state")
         if isinstance(scheduler_state, dict):
             self.device_scheduler = scheduler_state.get("scheduler", {})

apps/predbat/predbat.py

@@ -35,7 +35,7 @@
 import pytz
 import asyncio
 
-THIS_VERSION = "v8.44.3"
+THIS_VERSION = "v8.44.4"
 
 from download import predbat_update_move, predbat_update_download, check_install, DEFAULT_PREDBAT_REPOSITORY
 from const import MINUTE_WATT

apps/predbat/tests/test_fox_api.py

@@ -12,7 +12,7 @@
 import aiohttp
 import json
 from unittest.mock import MagicMock, patch, AsyncMock
-from fox import validate_schedule, minutes_to_schedule_time, end_minute_inclusive_to_exclusive, FoxAPI, schedules_are_equal, FOX_CACHE_KEYS, FOX_REFRESH_SETTINGS, FOX_REFRESH_REALTIME, OPTIONS_WORK_MODE
+from fox import validate_schedule, minutes_to_schedule_time, end_minute_inclusive_to_exclusive, FoxAPI, schedules_are_equal, FOX_CACHE_KEYS, FOX_REFRESH_SETTINGS, FOX_REFRESH_REALTIME, OPTIONS_WORK_MODE, FOX_SETTINGS_CACHE_VERSION
 from tests.test_infra import run_async, create_aiohttp_mock_response, create_aiohttp_mock_session
 
 
@@ -67,6 +67,9 @@ def __init__(self):
         self.device_values = {}
         self.device_settings = {}
         self.device_settings_unavailable = {}
+        # Default to "current" so existing tests aren't affected by the stale-cache-version
+        # self-heal check; tests exercising that mechanism explicitly set an older value
+        self.device_settings_version = FOX_SETTINGS_CACHE_VERSION
         self.last_unsupported = False
         self.device_production_month = {}
         self.device_production_year = {}
@@ -4392,6 +4395,94 @@ def test_run_settings_refresh_on_age(my_predbat):
     return False
 
 
+def test_run_settings_refresh_forced_by_stale_cache_version(my_predbat):
+    """
+    Test run() forces a settings/scheduler refresh, regardless of cache age, when the
+    persisted device_settings_version predates FOX_SETTINGS_CACHE_VERSION - a one-time
+    self-heal so a customer isn't stuck reusing a stale-shaped cached setting (e.g. one
+    missing range/unit/precision from before that shape existed) for up to FOX_REFRESH_SETTINGS
+    after every restart. Also verifies the version is persisted afterwards so a subsequent,
+    still-fresh run does NOT force another refresh.
+    """
+    print("  - test_run_settings_refresh_forced_by_stale_cache_version")
+
+    from datetime import datetime, timezone
+
+    fox = MockFoxAPIWithRunTracking()
+    fox.device_list = [{"deviceSN": "TEST123"}]
+    fox.device_settings_version = 0  # Simulates a cache persisted before this version existed
+
+    # Everything else, including device_settings itself, is freshly updated
+    now = datetime.now(timezone.utc)
+    for key in FOX_CACHE_KEYS:
+        fox.data_age[key] = now
+
+    result = run_async(fox.run(0, first=False))
+
+    assert result == True
+    # Forced despite a fresh device_settings cache age, because the version is stale
+    assert "get_device_settings:TEST123" in fox.method_calls
+    assert "get_scheduler:TEST123" in fox.method_calls
+    # The version must be bumped to current so it stops forcing a refresh going forward
+    assert fox.device_settings_version == FOX_SETTINGS_CACHE_VERSION
+
+    # A second run, still with a fresh cache age, must NOT force another refresh now that the
+    # version matches - otherwise every restart would keep hammering the API forever
+    fox.method_calls = []
+    fox.data_age["device_settings"] = now
+    result2 = run_async(fox.run(0, first=False))
+    assert result2 == True
+    assert "get_device_settings:TEST123" not in fox.method_calls
+
+    return False
+
+
+def test_run_stale_cache_version_not_cleared_without_scheduler_success(my_predbat):
+    """
+    Regression guard: the stale cache version must only clear once get_scheduler() (via
+    update_settings_from_schedule()) has actually had a chance to produce the upgraded shape -
+    not merely because get_device_settings() succeeded. Otherwise a cycle where settings
+    succeed but the scheduler read fails would wrongly mark the migration "done", permanently
+    losing the retry and leaving the customer stuck with the stale-shaped cached value.
+    """
+    print("  - test_run_stale_cache_version_not_cleared_without_scheduler_success")
+
+    from datetime import datetime, timezone
+
+    class MockFoxAPISchedulerFails(MockFoxAPIWithRunTracking):
+        async def get_scheduler(self, deviceSN):
+            self.method_calls.append(f"get_scheduler:{deviceSN}")
+            return None  # Simulates a failed scheduler read this cycle
+
+    fox = MockFoxAPISchedulerFails()
+    fox.device_list = [{"deviceSN": "TEST123"}]
+    fox.device_settings_version = 0
+
+    now = datetime.now(timezone.utc)
+    for key in FOX_CACHE_KEYS:
+        fox.data_age[key] = now
+
+    result = run_async(fox.run(0, first=False))
+
+    assert result == True
+    # Settings still succeeded and get attempted every cycle while the version stays stale
+    assert "get_device_settings:TEST123" in fox.method_calls
+    assert "get_scheduler:TEST123" in fox.method_calls
+    # Version must remain stale - the scheduler read (and any range upgrade it would apply)
+    # never actually succeeded
+    assert fox.device_settings_version == 0
+
+    # A second run, still with a fresh device_settings cache age, must keep retrying rather
+    # than silently giving up because the version never cleared
+    fox.method_calls = []
+    result2 = run_async(fox.run(0, first=False))
+    assert result2 == True
+    assert "get_device_settings:TEST123" in fox.method_calls
+    assert "get_scheduler:TEST123" in fox.method_calls
+
+    return False
+
+
 def test_run_realtime_refresh_after_cache_expires(my_predbat):
     """
     Test run() leaves fresh real-time data alone but re-fetches it once the cache expires,
@@ -4458,15 +4549,17 @@ def test_run_device_list_failure_does_not_mark_cache_fresh(my_predbat):
 def test_run_first_refreshes_device_list_despite_fresh_cache(my_predbat):
     """
     Test run() always re-fetches the device list on first start, even when the cached data
-    is still fresh, so a new inverter or changed serial number is picked up.  Device detail
-    and all other age-gated categories are skipped while the cache is fresh.
+    is still fresh, so a new inverter or changed serial number is picked up. Device detail
+    and all other age-gated categories are skipped while the cache is fresh and the settings
+    cache version is current.
     """
     print("  - test_run_first_refreshes_device_list_despite_fresh_cache")
 
     from datetime import datetime, timezone
 
     fox = MockFoxAPIWithRunTracking()
     fox.device_list = [{"deviceSN": "TEST123"}]
+    fox.device_settings_version = FOX_SETTINGS_CACHE_VERSION
 
     # Mark every cache category as freshly updated
     now = datetime.now(timezone.utc)
@@ -4478,7 +4571,8 @@ def test_run_first_refreshes_device_list_despite_fresh_cache(my_predbat):
     assert result == True
     # Device list must always refresh on first start regardless of cache age
     assert "get_device_list" in fox.method_calls
-    # Age-based categories with fresh data should NOT be re-fetched on first start
+    # Age-based categories with fresh data and a current settings cache version should NOT be
+    # re-fetched on first start
     assert "get_device_detail:TEST123" not in fox.method_calls
     assert "get_real_time_data:TEST123" not in fox.method_calls
     assert "get_device_settings:TEST123" not in fox.method_calls
@@ -4536,6 +4630,7 @@ def test_run_unchanged_device_list_preserves_cache(my_predbat):
 
     fox = MockFoxAPIWithRunTracking()
     fox.device_list = [{"deviceSN": "TEST123"}]
+    fox.device_settings_version = FOX_SETTINGS_CACHE_VERSION
     now = datetime.now(timezone.utc)
     for key in FOX_CACHE_KEYS:
         fox.data_age[key] = now
@@ -6735,6 +6830,8 @@ def run_fox_api_tests(my_predbat):
         failed |= test_run_first_call_no_devices(my_predbat)
         failed |= test_run_subsequent_call(my_predbat)
         failed |= test_run_settings_refresh_on_age(my_predbat)
+        failed |= test_run_settings_refresh_forced_by_stale_cache_version(my_predbat)
+        failed |= test_run_stale_cache_version_not_cleared_without_scheduler_success(my_predbat)
         failed |= test_run_realtime_refresh_after_cache_expires(my_predbat)
         failed |= test_run_device_list_failure_does_not_mark_cache_fresh(my_predbat)
         failed |= test_run_first_refreshes_device_list_despite_fresh_cache(my_predbat)