fix(gateway): don't declare the API started before auto-config completes - #4386
Open
springfall2008 wants to merge 2 commits into
Open
fix(gateway): don't declare the API started before auto-config completes#4386springfall2008 wants to merge 2 commits into
springfall2008 wants to merge 2 commits into
Conversation
ComponentManager.start() gates PredBat startup on wait_api_started(), polled from the main thread while the gateway's MQTT listener runs in its own thread. Two paths set api_started before automatic_config() had wired up the inverter args, so startup raced ahead with num_inverters / soc_percent unset: * _process_telemetry set the flag on the first frame, before _inject_entities() and automatic_config() on that same frame. This dates from the original component commit; the auto-config wait added later to run(first=True) was never what released startup, so it had no effect. * run(first=True) skipped the auto-config wait entirely when the first connection attempt failed, returning True immediately — ComponentBase then set api_started itself. The MQTT loop reconnects 5s later, so the config was usually seconds away. The telemetry handler now sets api_started only once _auto_configured is true, and run() waits for auto-config regardless of whether the first connect succeeded. Both startup waits share a single budget so they cannot stack. Also registers TestGatewayUnitControlBinding in run_gateway_tests(): the 17 regression tests for the 2026-06-04 "Gateway bound as inverter 0" incident were never being run, and had bit-rotted against the EV auto-config changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Load ML ran in phase 1, alongside the cloud components it depends on. Moving it to phase 2 means GECloud and friends have started and initialised pv_today etc. before Load ML reads them — the same reason the solar component was moved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a startup race in the Gateway MQTT component where api_started could become true before automatic_config() had wired up inverter args, letting PredBat continue startup with critical args (e.g. num_inverters, soc_percent) unset. It also updates/extends gateway tests and ensures previously unregistered regression tests are executed.
Changes:
- Delay setting
api_starteduntil after telemetry-driven entity injection and successfulautomatic_config()completion. - Rework
run(first=True)startup waiting so connection and auto-config share a single budget and the auto-config wait is not skipped after a failed first connection attempt. - Add/adjust tests for the above behavior and register previously unregistered gateway regression tests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/predbat/gateway.py | Adds shared startup-wait helper and defers api_started until auto-config completes. |
| apps/predbat/tests/test_gateway.py | Adds coverage for the corrected startup gating and registers TestGatewayUnitControlBinding. |
| _TELEMETRY_STALE_THRESHOLD = 120 | ||
|
|
||
| # Total startup wait budget, in 0.5 s ticks, shared by the connection and auto-config waits | ||
| _STARTUP_WAIT_TICKS = 120 * 2 |
| # PredBat's startup race ahead of auto-config. | ||
| await self._startup_wait(ticks, lambda: self._auto_configured) | ||
| if not self._auto_configured and not self.api_stop: | ||
| self.log(f"Warn: GatewayMQTT: Auto-config not complete after {_STARTUP_WAIT_SECONDS:.0f}s — gateway device may be offline, continuing startup") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The gateway component reported itself started before
automatic_config()had wired up the inverter args, so PredBat's startup raced ahead withnum_inverters/soc_percentunset.ComponentManager.start()gates startup oncomponent.wait_api_started(), which is polled from the main thread while the gateway's MQTT listener runs in its own thread (each component gets its own thread + event loop viahass.create_task).api_started— notrun()returning — is what actually releases startup, and two paths set it too early:1. The telemetry handler set the flag before configuring anything.
_process_telemetryflippedapi_started = Trueon the first frame, before_inject_entities()andautomatic_config()on that same frame. That line dates from the original component commit (9f7fc55); the auto-config wait inrun(first=True)was added later (7eac693, 0dc56fb) without removing it, so the wait never gated anything.2. A failed first connection attempt skipped the auto-config wait. The
if self._mqtt_connectedguard meant a transient broker failure at startup (_first_connection_attemptedis also set from the exception handler) maderun()return True immediately, andComponentBase.start()then setapi_starteditself. The MQTT loop reconnects after 5s, so the config was usually only seconds away.Fix
_process_telemetryinjects entities and runs auto-config first, then setsapi_started, gated on_auto_configured. If auto-config aborts (e.g. agateway_inverter_serialfilter matching nothing) the flag stays down and therun()timeout is the sole release path — which is what the existing "leave_auto_configuredFalse so we retry on the next telemetry" comment already intended.run(first=True)waits for auto-config regardless of whether the first connect attempt succeeded. The connection and auto-config waits now share one_STARTUP_WAIT_TICKSbudget (2 minutes) via a small_startup_wait()helper, so they can't stack.Trade-off: an unreachable broker now costs a startup stall where it previously returned immediately. That's the same stall the code already accepted for a connected-but-offline gateway device, and both warnings are still logged.
Also: Load ML moved to phase 2
Load ML started in phase 1, alongside the cloud components it depends on. It now starts in phase 2 so GECloud and friends have started and initialised
pv_todayetc. before Load ML reads them — the same reason the solar component was moved to phase 2.Tests
Four new gateway tests, all failing before the fix:
test_api_started_only_set_once_auto_config_has_completed— wrapsautomatic_configto captureapi_startedat entry; fails on old code with "api_started was set before auto-config ran".test_api_started_stays_false_when_auto_config_aborts/test_api_started_set_on_later_frame_that_needs_no_reconfigure— the abort and recovery paths.test_auto_config_wait_still_runs_when_first_attempt_failed— replacestest_auto_config_wait_skipped_when_not_connected, which encoded issue 2 as intended behaviour.Timeout expectations now derive from
_STARTUP_WAIT_TICKSrather than being hard-coded, so they can't drift from the constant.Unrelated gap found while doing this
TestGatewayUnitControlBinding— the 17 regression tests for the 2026-06-04 "Gateway bound as inverter 0" incident — was not registered inrun_gateway_tests(), so it had never run, and had bit-rotted (its fixture was missinggateway_evc_automatic, failing since EV auto-config landed). Registered and fixture repaired; all 17 pass.TestSerialFromEntityIdis also unregistered — left alone here, worth a follow-up.Verification
./run_all --test gateway→ 269 OK, exit 0./run_all --quick→ all passed, exit 0pre-commit runon all three changed files → all hooks pass🤖 Generated with Claude Code