fix(solis): stop retries draining the SolisCloud daily API allowance - #5089
Conversation
R0000 (daily allowance used up) was not in SOLIS_API_CODES, so _with_retry treated it as transient and retried it up to 7 times per call, every cycle. B0115 was treated as rate limiting - a 10s sleep then the same retry loop - when it actually means the datalogger is offline, which no retry can fix. Both now get a single attempt, and an R0000 pauses all Solis requests rather than spending more of an allowance that is already gone. Auto-config is also retried on later cycles instead of being a first-cycle only step, so a restart into an exhausted allowance no longer leaves load_today unset and fetch_sensor_data raising ValueError every cycle. Components can now name why they are unhealthy, so the run status says "Solis Cloud daily API limit reached" rather than just "component errors". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Four unresolved moderate findings remain around quota health visibility, startup recovery, offline polling, and repeated PV-only auto-configuration.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR prevents SolisCloud quota exhaustion and offline-datalogger errors from triggering retries, while improving auto-configuration recovery and health reporting.
Changes:
- Handles
R0000andB0115without retries, including quota pausing. - Retries automatic configuration and adds component health diagnostics.
- Adds Solis regression tests.
File summaries
| File | Summary |
|---|---|
apps/predbat/tests/test_solis.py |
Adds regression tests for quota, retries, offline handling, and auto-configuration. |
apps/predbat/solis.py |
Updates Solis error handling and auto-configuration. Moderate findings remain for startup recovery, ongoing offline polling, and repeated configuration attempts for PV-only fleets. |
apps/predbat/predbat.py |
Includes component health details in status. Moderate finding: active quota pauses may not be reported promptly or retained until recovery. |
apps/predbat/component_base.py |
Adds the default health-message hook. |
Review details
Suppressed comments (1)
apps/predbat/solis.py:3508
- When every discovered inverter is explicitly PV-only,
automatic_config()returnsFalseat line 1527, so this condition invokes it on every steady-state cycle forever. That repeatedly emits the existing “No inverters with batteries found” warning and re-scans configuration for a terminal state; distinguish missing/temporary details from an explicitly no-battery fleet and mark the latter as handled.
if self.automatic and self.inverter_sn and not self.automatic_config_done:
self.automatic_config_done = bool(await self.automatic_config())
- Files reviewed: 4/4 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Back off an offline datalogger per inverter rather than only skipping the retry: run() polls every inverter every minute, so leaving it in the rotation still spent a request a minute out of the 200 a day. - Retain the quota diagnostic until a request succeeds instead of tying it to the pause window, which expired at almost exactly the moment is_alive() went stale and anything would have consulted it. - Settle auto-config on an explicitly PV-only fleet. The #5087 retry turned the existing "no inverters with batteries" warning into a once-a-minute log line for anyone running PV-only inverters. - Cache the discovered fleet so a restart while the allowance is exhausted can still bind load_today. Discovery is the first thing a restart does and the first thing R0000 refuses, so the earlier retry was unreachable on that path. - Reuse the aiohttp session across startup retries rather than leaking one per attempt, which the above makes a routine occurrence. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Automated reply from the triage bot — a maintainer should review before merging. Addressing the suppressed finding from the review ( The finding is correct and it was a regression this PR introduced. Fixed by distinguishing the two reasons the battery list can be empty, which is the distinction the retry needed all along:
Covered by All four findings from the review are now addressed; each behavioural change has a test that fails without it. |
This is an automated draft PR generated from issue #5087 — a maintainer should review it before merging.
Fixes #5087
Summary
R0000("no authority too many request 200 times in 1DAYS") was missing fromSOLIS_API_CODES, so_with_retrytreated the exhausted daily allowance as a transient error and retried it — the pre-fix test run measures 5 attempts per call, every call, every cycle.B0115was worse: it was assumed to be rate limiting, so each one slept 10 seconds and then went round the same retry loop, when it actually means the datalogger is offline and no retry can help. Between them they are what drained a 200-request daily budget by mid-afternoon.R0000andB0115are now recognised and never retried (SOLIS_API_CODES_NO_RETRY); transient errors still retry exactly as before.R0000response pauses all Solis requests —_execute_requestfails without sending — until the allowance is expected back. The pause runs to the next UTC midnight but is capped at one hour: SolisCloud doesn't document which timezone the daily reset uses, so a wrong guess must not leave Predbat blind for a whole day, and one probe an hour costs almost nothing of the next day's budget.B0115no longer sleeps, and logs that the datalogger is offline rather than claiming rate limiting.automatic_config()now reports whether it actually bound the args, andrun()retries it on later cycles until it has. Previously it was a first-cycle-only step, so a restart while the allowance was gone leftload_todayand the rest unset andfetch_sensor_dataraising a plainValueErrorevery cycle until the next restart.ComponentBase.health_message()hook, surfaced byrecord_final_run_status(): the run status now readscomponent errors: Solis (Solis Cloud daily API limit reached, paused until 13:00 UTC)instead of just naming the component.Not implemented from the issue's suggestion list: the optional per-day request counter and "warn as usage approaches 200". With retries no longer multiplying each call, the budget is no longer the thing being overspent, and a counter is a larger piece of work than this ticket needs.
Testing
cd coverage && ./run_pre_commit— passes (exit 0, all hooks).Seven new tests in
apps/predbat/tests/test_solis.py, run viatools/triage_test.sh solis. They fail without the fix and pass with it. The red run (source files reverted, test file kept) reproduces every symptom in the ticket:The four behavioural tests are registered first on purpose so their failures are all visible before the run aborts; the remaining three (
note_quota_exhausted,quota_pausedexpiry,health_message) exercise methods that don't exist pre-fix, so they fail onAttributeErrorrather than on an assertion. Those tests deliberately use the wire-level code strings"R0000"/"B0115"rather than the new module constants, so they test behaviour rather than a constant's existence.Also re-run green:
component_health_statusandcomponents(therecord_final_run_statuschange), and the full./run_all --quicksuite (all pass, 4 slow tests skipped).Notes
impact().automatic_config()'s new return value has one production caller (run());SOLIS_API_CODEShas one;health_message()is new, andrecord_final_run_status()calls it behindhasattrbecause tests register component fakes that don't inheritComponentBase.firstblock inrun()creates a newaiohttp.ClientSessionon every startup attempt, so the component-level backoff retry leaks the previous one. It is reached far more often in exactly this scenario, but it is a separate defect from what the ticket describes and fixing it here would widen the diff.tools/debug-journal.mdline 129 describes the general shape of this failure — auto-config only runs after discovery succeeds, so an outage at startup leaves a non-GivEnergy inverter with unset args — which is the same hole the auto-config retry closes for Solis. Worth a journal update once this lands, but journal edits belong in their own PR.🤖 Generated with Claude Code