fix(solis): clamp discharge slot SOC to the inverter's recovery SOC floor, and surface refused control writes

[springfall2008]

Fixes #4702.

The bug

The inverter refuses any discharge slot cut-off SOC below the battery recovery SOC (CID 7229), and will not take a recovery SOC at or below the over-discharge SOC (CID 158). Predbat writes the reserve verbatim to both CID 158 and the slot cut-off, so the cut-off is always exactly one percent below the floor and can never be accepted.

The refusal is silent — the control API answers code: 0 and the register keeps its previous value — so a slot sits on a stale cut-off that can be far above the target, and the battery never force exports, while predbat.status still reports Exporting. On one inverter the stale value was 50%, so no forced export ever ran below half charge.

What this changes

1. Clamp, and lower the recovery SOC where possible. New resolve_discharge_soc_floor() lowers CID 7229 towards the target as far as the inverter allows (over_discharge + 1), then clamps the cut-off to whatever gap remains — usually the unavoidable one percent.

CID 158 is deliberately never written, so the battery protection floor stays where the user set it. Only the V2 path needs this; V1 doesn't write CID 5965 at all (it zeroes the discharge current instead), so it never meets the floor.

Side effect: this also settles the write-every-cycle retry loop. Previously cached 50 never matched the wanted 20, so it rewrote forever; now the cached value matches what we ask for.

2. Poll CID 7229 outside the batch read. atReadBatch reports it as 1 while a single atRead returns the real value. That poisoned the cache and the published recovery_soc entity — and would have made the clamp a silent no-op, since it reads from the cache.

Moving the CID into SOLIS_CID_SINGLE would have been wrong: that list is only polled on the V1 path, so V2 inverters would never read recovery SOC at all. Added SOLIS_CID_INFREQUENT_SINGLE instead, polled with batch=False alongside SOLIS_CID_INFREQUENT, so both paths get a correct value.

3. Surface a control register the inverter keeps refusing. read_and_write_cid() detects the mismatch but only logs Warn:, so the whole failure mode above was invisible to the user — predbat.status carried on reporting Exporting while the battery did nothing.

run() now withholds the success timestamp when a control write fails verification. components.is_alive() already treats a timestamp older than an hour as unhealthy, so a persistent failure ages the component out into Error: Complete run status ... with component errors: Solis, and a transient one heals on the next cycle. No new counters or thresholds.

The run is still reported as successful, deliberately. Routing this through the return value reaches non_fatal_error_occurred(), and the had_errors branch in update_pred() skips record_status() altogether — which would freeze predbat.status on its last value rather than show an error, i.e. worse than the current behaviour.

Scope comes free from reusing what write_time_windows_if_changed() already reports: slot enable, time, SOC and current, plus CID 103 on V1. The storage mode result is excluded, so the CID 636 TOU bit that some inverters clear (see below) cannot age those systems out permanently. That exclusion is currently structural rather than explicit — set_storage_mode_value() keeps its result in a local and returns None — so there is a regression test pinning it down, because one future success &= await self.set_storage_mode_if_needed(...) would otherwise quietly mark every affected system unhealthy.

Hardware validation

The design depends on CID 7229 being writable downwards, so that was checked against a live inverter before writing any of this:

recovery <- 30  read_back='30'  LANDED
recovery <- 25  read_back='25'  LANDED
recovery <- 21  read_back='21'  LANDED
recovery <- 20  read_back='21'  REJECTED    (over_discharge=20, so 21 is the minimum)
over_discharge(158) = '20' (was '20')       (untouched throughout)

The recovery = over_discharge + 1 rule and the cut-off floor were confirmed on three inverters across two model families and both auth methods — full evidence in #4702.

Design notes

resolve_discharge_soc_floor() reads from the cache only, with no live-read fallback. The first=True infrequent poll runs before the first control write, so the value is always populated by the time it is needed; if it somehow isn't, the target is written unclamped, i.e. exactly today's behaviour. That also keeps the helper out of the HTTP path on every slot write.

Tests

Ten new tests in test_solis.py, registered in run_solis_tests:

• the Solis: discharge slot cut-off SoC write silently rejected — PredBat always targets 1% below the inverter's Recovery SoC floor #4702 case — target 20 with recovery 21 clamps to 21 and leaves recovery alone
• recovery at 50 with over-discharge 20 is lowered to 21, target written as 21 rather than a stale 50
• recovery lowered to the target itself when the target is above the inverter minimum
• a target already above recovery is written through untouched, with no recovery write
• unknown recovery SOC leaves the target unclamped
• recovery is never driven below over_discharge + 1
• CID 7229 is in the single-read poll list and not the batched one
• a failed control write withholds the success timestamp without failing the run
• a clean control write still refreshes it
• a refused CID 636 write does not fail the control write (the exclusion guard)

./run_all --test solis and ./run_all --quick both pass, and run_pre_commit is clean.

Note solis.py carries # fmt: off, so black skips it; the new code follows the file's own one-blank-line convention between module-level functions.

Still open from #4702

Not addressed here, happy to follow up separately:

• CID 636 losing bit 1 (TOU) — now tracked separately as Solis: inverter clears the TOU bit on CID 636 while a slot is active, so timed charge/discharge slots are ignored #4707. Confirmed to happen while slot 1 is active, so it is not the known fix(solis): don't force TOU bit on CID 636 when no V2 slot is active #4239 idle case, and it appears on the V1 path too. Deliberately excluded from the health escalation above until it is understood, so it cannot mark affected systems unhealthy.

🤖 Generated with Claude Code

[Copilot]

🟢 Approval recommended

The core fix is well-scoped and backed by targeted tests; only minor test-helper robustness nits were identified.

Pull request overview

This PR fixes a Solis V2 forced-export control edge case where the inverter silently rejects a discharge slot cut-off SoC that is set below the inverter’s Recovery SoC floor, leaving a stale (often too-high) cut-off in place and preventing forced export from running as planned.

Changes:

• Add resolve_discharge_soc_floor() to lower Recovery SoC down to the inverter’s permitted minimum (when possible) and clamp the discharge slot cut-off SoC to the effective floor.
• Move polling of SOLIS_CID_BATTERY_RECOVERY_SOC (CID 7229) out of the batched infrequent reads into a new single-read infrequent poll list to avoid the atReadBatch mis-reporting bug.
• Add targeted Solis tests covering clamping behavior and the polling-list change.

File summaries

File	Description
apps/predbat/solis.py	Implements recovery SoC single polling and clamps discharge slot SoC against the inverter’s enforced floor.
apps/predbat/tests/test_solis.py	Adds regression/unit tests for discharge SoC clamping and the new single-read polling list.

Review details

Suppressed comments (1)

apps/predbat/tests/test_solis.py:2256

• _written_recovery() doesn't filter by inverter serial, so if tests ever exercise multiple inverters its result can be incorrect. Consider taking inverter_sn (defaulting to the existing TEST123) and filtering the recorded calls by it, consistent with _written_soc().

def _written_recovery(api):
    """Return the value written to the battery recovery SOC register, or None.

    Args:
        api: MockSolisAPI whose recorded calls should be searched

    Returns: The written value as a string, or None if the register was not written
    """
    call = next((c for c in api.read_and_write_cid_calls if c["cid"] == SOLIS_CID_BATTERY_RECOVERY_SOC), None)
    return call["value"] if call else None

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

---

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