Repository navigation
Solis: cap slot currents at the inverter rating and distrust implausible recovery SOC - #5189
Merged
Merged
Conversation
…ble recovery SOC (#5187) CID 7224/7226 are battery limits, not inverter ones. A 3.6kW inverter reading 100A there refused every 100A discharge slot write, leaving its V2 slot at 0A and holding the battery through every export window. Slot currents are now never written above inverterDetail power / nominal pack voltage: on V2 the rating joins the existing 7226/sysCommand.max limit, and on V1 the CID 103 encoder caps each current as it formats it. Most of the fleet reports a recovery SOC (CID 7229) of 0, 1, the over-discharge SOC itself or 65521, which disabled the #4706 cut-off clamp. A reading at or below over-discharge, or above 100, is now taken as over-discharge + 1 and is never written back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Moderate issues remain in V1 battery-limit handling and conservative current-rating truncation.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Adds Solis inverter-rated current caps and fallback handling for implausible recovery SoC values.
Changes:
- Caps V1/V2 slot currents to inverter ratings.
- Validates recovery SoC readings with a safe fallback.
- Adds focused regression tests.
| File | Summary |
|---|---|
apps/predbat/tests/test_solis.py |
Adds coverage for current caps and recovery SoC handling. |
apps/predbat/solis.py |
Implements current limiting and recovery SoC validation. |
💡 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>
This was referenced Sep 24, 2026
springfall2008
pushed a commit
that referenced
this pull request
Sep 26, 2026
Resolve debug-journal conflicts with #5223's fold of the same merge batch: - Solis: keep this PR's #5189 detail and write-skip/test notes, then main's GH#5152 window-scoped hold lever - GE Cloud: main's #5109 wording plus the once-per-episode ems_slot_warned warning and its log text - Teslemetry: this PR's #5188 default-flip consequences plus main's GH#5157 forced re-assert - Manual rates: fold both rows into one (#5173 fix, surviving :59:59 boundary-minute and offset traps, test lesson) - Kraken, Sunsynk, Octopus, Car charging from main; Axle, GivTCP, Compare and the three new rows from this PR Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
CossieRob
pushed a commit
to CossieRob/batpred
that referenced
this pull request
Sep 26, 2026
…pringfall2008#5103/springfall2008#5186/springfall2008#5053-adjacent entries Folded (13 entries from the 15-candidate slice, all re-verified against origin/main 130596b): - Load ML row (GH#5075, PR springfall2008#5112 open draft): is_valid() can report active for a never-trained model; training_timestamp is None is not a sound never-trained test (legacy saved models); train() stamps at the end of every curriculum pass, so an abort leaves fresh stamps that suppress the ml_max_model_age_hours retrain. Normalisation-statistics skew stays suspected. - Octopus row: GH#5144 overlapping DIRECT/NON_DIRECT_DEBIT rows, minute_data last-row-wins and the period-to-period flip - fixed in PR springfall2008#5145 (filter_payment_method, keeps nulls). - Car charging row: GH#5146/GH#5151 the two different Hold-for-car gates (execute.py live block vs prediction.py plan gate), deliberate export-window exemption test-codified (discharge_car_full_bat2), read-only users never see the status sensor's annotation. - Sunsynk/DEYE row: PR springfall2008#5150 control_active persistence + upgrade-cache inference; the in-code press every cycle comment is wrong (both press sites change-gated, post-restart re-commit is is_hm_format-only); GH#5156 TOU padding truncation fixed in tou_schedule.py _padding_segments. - Solis row: GH#5152 the hold lever is the window-scoped timed current (standing battery_discharge_current never driven); PR springfall2008#5189 adds the rated-current cap (GH#5187). - Predheat row: GH#5153 no Fahrenheit conversion in get_weather_data + the optional heating_energy age bug pinning minute_data_age to 0. - Teslemetry row: GH#5157 forced re-assert implemented (FORCED_ASSERT_SECONDS = 2h, dedupe bypassed, tariff excluded); tbc_control default on since PR springfall2008#5188. - Keep section: GH#5162 manual_soc floor penalty is import-rate-weighted, so inert inside 0p windows; the manual_soc_max ceiling is export-rate-weighted; manual_charge lookalike trap. - New Kraken row: GH#5166/PR springfall2008#5167 EDF 400-on-day/night vs Octopus 200-empty; KRAKEN_REST_RATES_UNAVAILABLE_STATUSES; the (None, None) tuple contract and the failures_total double-count, both fixed in the merged PR and kept as mechanisms. - New Manual rates row: GH#5168 day_of_week stamped across the whole rate horizon, fixed in PR springfall2008#5173; end ':59:59' final-minute trap and the metric_future_rate_offset test pin kept. Dropped: 5169-5053-cache-now-on-main (already covered by the redaction entry and the GH#5063 premise bullet; its lock-lazy-init nugget is recorded as a fact of the merged PR springfall2008#5171 mixin). Corrected by the merge scan: GH#5103 fixed in PR springfall2008#5109 (hourly settings refresh + the new check_ems_inverter_slots warning); teslemetry_tbc_control default on (PR springfall2008#5188); the log-redaction cache moved into the LogRedaction mixin (log_secrets.py, PR springfall2008#5171). Co-Authored-By: Claude Code <noreply@anthropic.com>
CossieRob
pushed a commit
to CossieRob/batpred
that referenced
this pull request
Sep 26, 2026
…s); correct entries overtaken by the springfall2008#5109/springfall2008#5173/springfall2008#5185/springfall2008#5188/springfall2008#5189 merge batch Co-Authored-By: Claude Code <noreply@anthropic.com>
This was referenced Sep 29, 2026
springfall2008
pushed a commit
that referenced
this pull request
Sep 29, 2026
…replace the rated estimate The probe started from the rated-current estimate from #5189 (rated power / nominal voltage). So it could only ever confirm or lower that estimate, never find a ceiling above it. An 8kW hybrid on a 51.2V pack reads 180A in CID 7224/7226 and ran slots at 180A before #5189. The rated estimate held it at 156A and capped PV charging below what the inverter takes. - A first probe now starts at the battery-side cap (the register, lowered by any slot metadata) and steps down only on a refusal. - Once an inverter has a measured ceiling, it replaces the rated estimate in slot_current_limits(). The estimate is only a fallback until then. inverter_limit still bounds the AC side in the plan. - A daily re-check starts at the known ceiling. An unchanged one costs two writes (ceiling kept, ceiling + 1 refused) rather than a fresh search, which on a 60A inverter behind a 100A register would be about eight refused writes per direction. - The limits line marks the rated estimate "not used once probed". Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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.

This is an automated draft PR generated from issue #5187 — a maintainer should review it before merging.
Fixes #5187
Summary
Slot current capped at the inverter's rating. CID 7224/7226 are battery limits, not inverter ones. On the affected 3.6 kW unit they read 100 A (4.8 kW at 48 V), and the inverter refused every 100 A discharge-slot write. That left the V2 slot at 0 A, holding the battery through each export window. The same model reading 70 A or 62.5 A accepts its writes.
A new
get_rated_current()works out the rating as inverterDetailpower(theinverter_sizesensor, already bound toinverter_limit) divided byget_nominal_voltage(), floored to whole amps. On a 3.6 kW unit that is 75 A at 48 V and 70 A at 51.2 V. The cap is applied at the point the current is written:min(CID 7226, sysCommand.max)limit inwrite_time_windows_if_changed().Implausible Recovery SoC. 19 of 29 inverters in the fleet report CID 7229 as 0, 1, equal to the over-discharge SoC, or 65521. Any of those disabled the #4706 clamp, so a cut-off at the over-discharge SoC was written, refused, and the stale 40–50 % stayed.
resolve_discharge_soc_floor()now treats a reading at or below the over-discharge SoC, or above 100, as over-discharge + 1, which is where #4702 found the inverter holds it. That fallback is never written back to CID 7229.Testing
./run_pre_commit: every hook passed, and the quick suite it runs passed../run_all --test solis: passes with the fix and fails without it (recovery reads 0: discharge SOC should be 13, got 12).The module run stops at its first failure, so each new test was also run on its own against the unfixed
solis.py:test_implausible_recovery_soc_falls_back_to_inverter_minimumtest_get_rated_currenttest_v2_slot_currents_capped_at_inverter_rating100.0)75.0)test_v1_slot_currents_capped_at_inverter_rating100,100,…)75,75,…)test_slot_currents_uncapped_when_inverter_size_unknownThe last row passes on both on purpose. It checks that an inverter with no reported size behaves exactly as before.
Notes
battery_rate_max/ discharge rate. That is left out deliberately.battery_rate_maxis still CID 7224 × V (4800 W), so Predbat still asks for 4800 W and the slot power entity keeps showing it while 75 A goes to the inverter. This is the same entity/register mismatch the debug journal records for thesysCommand.maxclamp (GH#5068). The plan is not affected, because the prediction already limits battery draw toinverter_limit(prediction.py:1141). If the requested value should be right at the source too, one option is to bindinverter_limit_charge/inverter_limit_dischargetoinverter_sizewithset_arg_auto(..., overwrite=False). Capping insidenumber_event_handlerwould instead makewrite_and_poll_valuefail its read-back and sethad_errorsevery cycle.discharge_rate = 0(execute.py:517), and the discharge slot can be armed ahead of an export window while that is still in force. If the capped current is refused, a V2 slot can still sit at 0 A.solis_nominal_voltagesets the conversion voltage directly.Capping slot currents on <sn> at <n>Aonce per control cycle.impact:resolve_discharge_soc_floorLOW (one caller),encode_time_windowsLOW (one caller, the V1 branch),write_time_windows_if_changedLOW. The index lists no callers for it; by search its only production caller isrun().detect_changes: LOW, no affected processes.mainat 8b9fe02.🤖 Generated with Claude Code