Solis: don't let a refused startup register read abort startup, and back off B0600

[springfall2008]

This is an automated draft PR generated from issue #5177 — a maintainer should review it before merging.

Fixes #5177

Summary

startup_reset_registers() only lowers the over-discharge SoC, but run() called it in the first-cycle block without a try/except. If the datalogger refuses the register read, the exception escapes run() and ends that startup attempt. automatic_config() never runs, so Predbat has no inverter ("no source is configured"). That happens with B0600 after 7 retries, and with B0115 on the first attempt.

This PR:

• Guards the call in run(). A failed startup reset now logs Warn: Solis API: Startup register reset failed for inverter <sn>, continuing without it: ... and startup continues. This is the change that fixes the bug.
• Adds B0600 to SOLIS_API_CODES, so the log no longer says "Unknown code", and to SOLIS_API_CODES_NO_RETRY, so each refusal costs 1 request instead of 8.
• Puts a B0600 inverter into the same per-inverter cooldown B0115 uses (fix(solis): quota-aware retries with exponential backoff #5088). _execute_request now routes both codes through note_datalogger_offline(sn, code).
• Records which code started the cooldown (datalogger_cooldown_code). The log and run status then report B0600 as datalogger returning data abnormally (B0600), not as offline; the owner in the report could see the datalogger online in SolisCloud.
• Keeps the datalogger diagnostic in health_message() until a request succeeds. Before, it lasted only for the 15-minute cooldown. The quota diagnostic already works this way. A component in startup backoff re-probes up to 128 minutes apart, so a diagnostic tied to the cooldown would leave the run status blank most of the time.

Testing

• cd coverage && ./run_pre_commit: all hooks passed and the quick suite passed (4 slow tests skipped).
• tools/triage_test.sh solis fails without the fix and passes with it. I stashed only apps/predbat/solis.py and checked that the file was back to main before the red run.

  • test_run_survives_startup_register_reset_refusal (new) runs the real startup_reset_registers() through run(0, True) against a canned SolisCloud refusal, for both B0600 and B0115. Without the fix both codes abort run(). With the fix it checks:

    • exactly one request is sent
    • the warning is logged
    • automatic_config() and publish_entities() are still reached
    • the inverter is in cooldown
    • the health message names it

  • test_b0600_backs_off_without_calling_the_datalogger_offline (new) covers:

    • B0600 is a known code and puts the inverter in cooldown
    • neither the pause log nor the health message says "offline"
    • two inverters with different codes each get their own reason
    • the diagnostic outlasts the cooldown
    • a successful read clears it

    Without the fix every one of these assertions fails.

  • test_with_retry_does_not_retry_settled_codes now includes B0600. Without the fix it was attempted 5 times.

  • MockSolisAPI gets the new attribute, so the initialize/mock parity test covers it too.

Notes

• Names left wider than they read. datalogger_offline(), note_datalogger_offline(), datalogger_offline_until and SOLIS_DATALOGGER_OFFLINE_SECONDS now cover B0600 too, where the datalogger is online but refuses reads. I updated their docstrings and gave the one new attribute an accurate name (datalogger_cooldown_code). I did not rename the existing symbols, to keep this diff focused; a rename to datalogger_cooldown* would be a clean follow-up. The recovery log line now reads "is answering requests again" instead of "is back online", because that is true for both codes.
• B0600 pauses the whole inverter, as B0115 does. That includes the every-minute inverterDetail poll. On the reporting account inverterDetail failed too (code 1), so nothing is lost there. If B0600 turns out to block only remote register reads while details keep working, a narrower gate on the CID read/control endpoints would keep live data flowing every minute (the "poll-only" outcome the issue hopes for). I did not build that on a single report.
• The component still stays in startup backoff while the inverter can't be polled: poll_success is False, so run() returns False and first stays True. That is Solis: one unreachable inverter keeps the whole component in startup backoff #5093's territory and out of scope here. What this PR changes is that each attempt now completes, reaches automatic_config(), costs one request per refusal, and leaves a named reason in the run status.
• The health message wording for B0115 changed slightly, from datalogger offline for inverter X to datalogger offline or disconnected for inverter X. It now also persists past the cooldown until a request succeeds, as described above.
• startup_reset_registers() is caught with a bare except Exception, like the discovery block and the event-handler drain in the same function, because the step is best-effort.
• Blast radius (GitNexus impact): run, note_datalogger_offline and health_message are LOW. health_message is called dynamically from predbat.py's run-status builder via hasattr. _execute_request is MEDIUM, with its 5 request wrappers as direct callers; the change there only touches the error-code branch. detect_changes reports the single affected flow run → _with_retry, as expected.

🤖 Generated with Claude Code

[springfall2008]

The guard makes the reset best-effort, but the whole startup block above (fetch_inverter_details, the TOU-v2 poll, this reset read — the for sn loop at line 3580) never checks datalogger_offline(sn). During startup backoff each re-probe still sends up to 3 requests to a datalogger its own cooldown says not to poll: the 2/4/8-minute early backoff gaps all fall inside the 15-minute cooldown, so those attempts each spend ~3 refused requests out of the 200/day allowance (the budget pressure issue #5087 was about). The new run-path test asserts 'exactly one request is sent' only because fetch_inverter_details/poll_inverter_data are mocked — production sends ~4 per attempt. Gating the first block's per-inverter calls on datalogger_offline(sn) (they still fire once the cooldown expires, so re-probing is unaffected) would make production match the test's claim.

[springfall2008]

🤖 Automated reply from the triage bot.

Fixed in 2d51c66. The first-cycle loop now skips an inverter whose datalogger is in its cooldown. It also re-checks after the detail read and after the TOU mode read, so it stops at the first refusal that starts a cooldown. A skipped inverter sets poll_success = False, which keeps the component in startup, so the first attempt after the cooldown expires re-probes it.

The new test_startup_leaves_an_inverter_in_datalogger_cooldown_alone drives the real detail, poll and reset code against a session that answers B0600 to everything:

• the first attempt sends 1 request;
• the attempts at 2, 6 and 14 minutes send none;
• the attempt at 30 minutes re-probes with 1.

Against the previous commit, the first attempt sent 3.

[springfall2008]

This rewrite drops the old datalogger_offline(sn) filter, so an entry now survives until a request for that inverter succeeds. For an SN that leaves the polling rotation — removed from the inverter_sn config, or dropped by discovery — no such request ever comes, and the run status reports 'datalogger offline or disconnected / returning data abnormally for inverter X' for the rest of the process lifetime. The old active-cooldown filter was the implicit expiry; nothing replaces it for SNs no longer polled. Worth evicting entries for SNs not in self.inverter_sn, or keeping the filter and handling the outlast-the-cooldown case separately.

[springfall2008]

🤖 Automated reply from the triage bot.

Fixed in 2d51c66. health_message() now reports only inverters in self.inverter_sn. I filtered there rather than evicting, because the run status is the only place an expired entry has any effect: expired entries gate no polling. Test: test_datalogger_diagnostic_dropped_once_the_inverter_leaves_the_rotation.

[springfall2008]

Persistence is keyed to the code recorded when the cooldown started, so once the 15-minute cooldown expires a re-probe failing for an unrelated reason (HTTP 500, timeout, R0000) leaves the run status claiming the datalogger is offline/returning-abnormal while the real cause is e.g. a cloud outage. The old code went quiet once the cooldown expired. Consider clearing the entry when the post-cooldown probe fails with a non-datalogger error.

[springfall2008]

🤖 Automated reply from the triage bot.

Fixed in 2d51c66. The new forget_expired_datalogger_cooldown() drops an entry whose cooldown has expired when a request for that inverter fails for anything other than B0115 or B0600: an HTTP error, a timeout, a network error, or another API code. R0000 counts as another code, and while the quota is exhausted the quota diagnostic is reported first anyway. Inside an active cooldown it does nothing. Test: test_datalogger_diagnostic_cleared_by_an_unrelated_failure_after_its_cooldown, which covers an HTTP 500 and API code 1, the code the detail endpoint returned on the reporting account.

[springfall2008]

The early return also skips updating datalogger_cooldown_code, so if a B0115 cooldown is in force and the refusals turn into B0600 (or vice versa), the pause log and the health message keep reporting the older reason for the rest of the cooldown. Fine when the code never changes, but the reason can then lag the latest refusal until some request succeeds.

[springfall2008]

🤖 Automated reply from the triage bot.

Fixed in 2d51c66. note_datalogger_offline() now records the code before the early return, so the run status follows the latest refusal. The cooldown is still not extended, and the pause is still logged only once. Test: test_datalogger_reason_follows_the_latest_refusal (B0115, then B0600 five minutes in).

[springfall2008]

The try/except only ever catches the initial read: read_and_write_cid() swallows its own failures and returns False, which startup_reset_registers() (line 3524) ignores. A refused/failed write still leaves the over-discharge SoC above the 20% default with only read_and_write_cid's generic 'Failed to read and set ...' warn — this 'Startup register reset failed' warning and the issue-#5177 framing never fire for the write half, so the survivable step is only observable on its read half.

[springfall2008]

🤖 Automated reply from the triage bot.

Not changed, because the write half cannot run today.

startup_reset_registers() does value = await self.read_cid(...), but read_cid() returns a (value, info) tuple. It has done so since #3654, a week before this function arrived in 9f43e43 and c52ae1d. float() of that tuple raises TypeError, the except sets value = 0, and the > 20 write never executes. So there is no refused write to report, and a warning there would be dead code.

Fixing the unpacking would switch on a write that has never happened in production: lowering every inverter's over-discharge SoC to 20% at each startup. That is a behaviour change for a maintainer to decide, not a review fix, so I've kept it out of this PR. If it is wanted, the unpacking fix and a write-failure warning belong together in a follow-up.

[springfall2008]

Comments now trail the code: the request-sn comment above (line 627) still says 'so a B0115 backs off that one', and _with_retry's comment (line 704) still says 'an offline datalogger will answer every retry the same way' — both now cover B0600 too. Worth touching so the next reader doesn't have to re-derive that B0600 settles retries as well.

[springfall2008]

🤖 Automated reply from the triage bot.

Fixed in 2d51c66. The request-sn comment in _execute_request() and the no-retry comment in _with_retry() now name both B0115 and B0600.

[springfall2008]

Simplification: SOLIS_DATALOGGER_COOLDOWN_REASONS.get(code, code) is duplicated here and in health_message (line 617), and the new default code=SOLIS_API_CODE_DATALOGGER_OFFLINE exists only for test callers — the one production caller passes str(code) explicitly. A tiny cooldown_reason(code) helper (or an explicit code everywhere) keeps the two fallbacks from drifting.

[springfall2008]

🤖 Automated reply from the triage bot.

Fixed in 2d51c66. datalogger_cooldown_reason(sn) is now the only place the code-to-wording fallback lives, and both the pause log and health_message() use it. note_datalogger_offline() now takes code explicitly, with no default. The two test callers that relied on the default now pass SOLIS_API_CODE_DATALOGGER_OFFLINE.

[Copilot]

Copilot review overview

🟢 Approval recommended

The changes are targeted, covered by new/updated Solis tests, and the updated error handling/backoff paths appear consistent with existing Solis retry and cooldown mechanisms.

Review effort: Lite
Findings: None

What changed in this PR

This PR fixes Solis component startup robustness when SolisCloud refuses startup register reads (notably API code B0600), ensuring the component can still complete its first-cycle configuration and publish entities rather than aborting startup.

Changes:

• Guard startup_reset_registers() during the first-cycle startup path so a refused read logs a warning and startup continues.
• Treat B0600 as a known, non-retryable Solis API code and route it through the existing per-inverter cooldown/backoff mechanism.
• Improve run-status/health reporting by persisting the datalogger diagnostic until a request succeeds, and by recording which code triggered the cooldown.

File	Description
apps/​predbat/​solis.py	Adds B0600 handling, per-inverter cooldown attribution, persistent health messaging, and guards startup register reset so startup doesn’t abort on refused reads.
apps/​predbat/​tests/​test_solis.py	Extends the Solis test suite with new regressions for #5177 and updates mocks/settled-code retry expectations to include B0600.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[springfall2008]

Automated cleanup check from the triage bot - nothing for a maintainer to act on.

Nothing new to address on this PR at 2d51c66:

• The branch is up to date with main; merging origin/main changed nothing.
• All 7 inline review threads already have a reply, posted with 2d51c66. Six were fixed in that commit, and one (the write half of startup_reset_registers()) was declined with reasoning.
• The Copilot review had no findings, and there are no top-level comments.
• CI passed on 2d51c66 (pre-commit, kernel-binaries).

No commit was pushed.