From 1d28daf8a07078ee6bd29fb752aeba5702f35308 Mon Sep 17 00:00:00 2001 From: glenn schrooyen Date: Mon, 5 Oct 2026 23:24:02 +0200 Subject: [PATCH] MAINT-10 review: warn when HA did not answer and TZ is UTC; test the 3 s budget HA can take minutes to start (2026-10-05, 502 from the proxy). Then TZ holds, which the Supervisor sets from HA's zone, so the hour stays right; only a TZ of UTC there now logs a WARNING asking for a restart. A test pins the 3 s budget on the zone request (a mutant without it survived). Co-Authored-By: Claude Opus 5.5 --- .../DECISIONS/MAINT-10-local-start-hour.md | 10 ++++++++-- goodwe_controller/DOCS.md | 3 ++- goodwe_controller/app/main.py | 10 ++++++++++ goodwe_controller/test_maint10.py | 15 +++++++++++++++ 4 files changed, 35 insertions(+), 3 deletions(-) diff --git a/goodwe_controller/DECISIONS/MAINT-10-local-start-hour.md b/goodwe_controller/DECISIONS/MAINT-10-local-start-hour.md index a1b0a8e..f0b0821 100644 --- a/goodwe_controller/DECISIONS/MAINT-10-local-start-hour.md +++ b/goodwe_controller/DECISIONS/MAINT-10-local-start-hour.md @@ -13,8 +13,14 @@ peak guard outrank the drain. Home Assistant's `time_zone` (`GET /core/api/config`, 3 s budget) at startup, else the container's `TZ`, else UTC. One log line says which (WARNING only for UTC). Resolved once per start: a zone change in HA takes effect on the next -add-on restart. The add-on image has tzdata (checked in `amd64-base:3.20`); the -CI image does not, so the tests grade DST on `tariff._EUTimezone` too. +add-on restart. HA can still be starting when the add-on starts (502 from the +proxy, 2026-10-05); then `TZ` holds. HA pushes its zone to the Supervisor +(hassio `push_config`), which stores it and sets it as the add-on's `TZ` +(`docker/app.py`, Supervisor 2026.08.0), so the hour stays right. Only a `TZ` of +UTC there logs a WARNING asking for a restart. No later re-resolve: the fallback +already is HA's zone, so the rare UTC case gets a warning rather than more code. +The add-on image has tzdata (checked in `amd64-base:3.20`); the CI image does +not, so the tests grade DST on `tariff._EUTimezone` too. ## 2. DST diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md index 2b907c5..04d74d2 100644 --- a/goodwe_controller/DOCS.md +++ b/goodwe_controller/DOCS.md @@ -1653,7 +1653,8 @@ help - see "Why the tuning is what it is". **The start hour is local time.** The add-on reads Home Assistant's time zone (Settings → System → General) at startup. If Home Assistant does not answer, it uses the add-on's own time zone, and failing that UTC. The log says which -one it used. On the day the clocks go forward, a start hour that does not exist +one it used, and warns if it is UTC: then restart the add-on once Home Assistant +is up. On the day the clocks go forward, a start hour that does not exist starts at the first hour after the jump. On the day they go back, a repeated hour starts only once, in its first pass. With two inverters both batteries use the same local hour; one cycles at a time, and the other starts at that hour on diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index 883bb50..da986db 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -7267,6 +7267,10 @@ async def ha_time_zone(session, token: str) -> tuple: return None, f"{type(err).__name__}: {text}" if text else type(err).__name__ +_UTC_NAMES = {"UTC", "ETC/UTC", "GMT", "ETC/GMT", "UCT", "ETC/UCT", + "UNIVERSAL", "ETC/UNIVERSAL", "ZULU", "ETC/ZULU"} + + async def maint_site_tz(session, token: str): """MAINT-10: the zone the maintenance start hour is read in, logged once.""" zone, why = await ha_time_zone(session, token) @@ -7274,6 +7278,12 @@ async def maint_site_tz(session, token: str): if source == "ha": _LOG.info("Maintenance start hour is local time in %s, from Home " "Assistant.", name) + elif source == "tz" and name.strip().upper() in _UTC_NAMES: + # The Supervisor sets TZ to its copy of HA's zone, or to UTC if it has + # none: HA did not answer here, and UTC may not be the site's clock. + _LOG.warning("Maintenance start hour uses UTC, because Home Assistant " + "did not answer. Restart the add-on when it is up.") + _LOG.debug("maintenance time zone: %s", why) elif source == "tz": _LOG.info("Maintenance start hour is local time in %s, from the " "add-on's time zone. Home Assistant gave none: %s.", name, why) diff --git a/goodwe_controller/test_maint10.py b/goodwe_controller/test_maint10.py index c88eac0..6025c00 100644 --- a/goodwe_controller/test_maint10.py +++ b/goodwe_controller/test_maint10.py @@ -316,6 +316,7 @@ class _Session: def get(self, url, **kw): self.asked.append(url) + self.timeouts = getattr(self, "timeouts", []) + [kw.get("timeout")] if url.endswith("/core/api/config"): return _Resp(self.zone_body) return _Resp({"data": {}}) @@ -325,6 +326,9 @@ s = _Session({"time_zone": "Europe/Brussels"}) check("ha_time_zone() reads /core/api/config's time_zone", asyncio.run(M.ha_time_zone(s, "tok")) == ("Europe/Brussels", "") and s.asked == [M.CORE_API + "/config"]) +# Review: HA can take minutes to start (2026-10-05); the ask must not wait on it. +check("...with a time budget of at most 3 s, so a slow HA cannot hold up start", + s.timeouts == [M.DOCS_LINK_TIMEOUT_S] and 0 < M.DOCS_LINK_TIMEOUT_S <= 3) check("...and a failure is None plus the reason, never a raise", asyncio.run(M.ha_time_zone(_Session(OSError("down")), "tok")) == (None, "OSError: down") @@ -408,6 +412,17 @@ try: check("amain(), neither: UTC, with a WARNING line", tz is UTC and any(lvl == "WARNING" and "uses UTC" in m for lvl, m in logs)) + # Review: HA still starting (502) and the Supervisor's TZ is UTC. + tz, lines, logs = amain_zone(OSError("502"), "UTC") + check("amain(), Home Assistant unavailable and TZ is UTC: a WARNING " + "that says to restart", tz is UTC + and [(lvl, m) for lvl, m in logs if "Maintenance start" in m] + == [("WARNING", "Maintenance start hour uses UTC, because Home " + "Assistant did not answer. Restart the add-on when it is up.")]) + tz, lines, logs = amain_zone(OSError("502"), "America/New_York") + check("...but a real TZ is used without a warning", + not any(lvl == "WARNING" for lvl, m in logs + if "Maintenance start" in m)) finally: MT._load_zone = _real_load