Repository navigation
fix(sunsynk,deye): persist control_active across a restart - #5150
Merged
Merged
Conversation
…ed_payload save_control only cached applied_payload, so a restart always came back with control_active empty - the gate _reconcile_control checks before it will ever write to an inverter (sunsynk.py:1346). Until a fresh battery_schedule_charge_write event happened to re-arm it, every automatic re-apply was silently skipped, including one meant to stop an export already in progress. alphaess.py's save_control/restore_state already persists control_active for the same reason; this mirrors it, restoring it under the same SUNSYNK_RESTORE_MAX_CONTROL age bound as applied_payload. Related to #5138. Independent of #5142, which addresses a write the cloud acknowledged but the dongle never applied - this addresses a write never attempted at all after a restart. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…payload Believed to be the same bug as the Sunsynk fix in this branch (#5138), found by reading this file alongside that one rather than from a reported deye.py incident. save_control only cached applied_payload/pending_orders/ order_poll_count, so a restart always came back with control_active empty - the gate _reconcile_control checks before it will ever write to an inverter (deye.py:1664). Until a fresh write-button event happened to re-arm it, every automatic re-apply was silently skipped, including one meant to stop an export already in progress. Fixed the same way as sunsynk.py and alphaess.py, which already persist control_active for exactly this reason: restored alongside applied_payload, under the same DEYE_RESTORE_MAX_CONTROL age bound. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The restore paths can still reintroduce the “writes gated off after restart” bug on upgrade when restoring older cache files that lack the new control_active field.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
Open (2)
What changed in this PR
This PR addresses a restart-safety gap in the Sunsynk Cloud and Deye Cloud integrations by persisting the in-memory control_active gate that enables automatic per-cycle plan re-application, so control writes continue after a Predbat restart.
Changes:
- Persist
control_activeinsave_control()for both Sunsynk and Deye, alongside existingapplied_payloadcontrol caches. - Restore
control_activetogether withapplied_payloadinrestore_state(), bounded by the existing cache-age limits. - Add unit tests to ensure
control_activeround-trips through storage and is discarded when the control cache is stale.
| File | Description |
|---|---|
| apps/predbat/sunsynk.py | Persist/restore control_active with the control cache to keep _reconcile_control active after restart. |
| apps/predbat/deye.py | Persist/restore control_active with the control cache for the same restart behavior. |
| apps/predbat/tests/test_sunsynk_storage.py | Add tests validating control_active persistence and staleness-bounded restore behavior. |
| apps/predbat/tests/test_deye_storage.py | Add tests validating control_active persistence and staleness-bounded restore behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
springfall2008
added
the
BOT_CLEANUP
…-upgrade cache A control cache written before control_active was persisted carries applied_payload alone. Restoring that half on its own left _reconcile_control gated off through the one restart that installs the fix, preserving the bug being fixed. Infer the missing half from applied_payload keys, which is a safe lower bound: every key got there via a path that arms control_active first, so this can never arm an inverter Predbat has not driven. An explicitly empty list is honoured rather than inferred. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
springfall2008
removed
the
BOT_CLEANUP
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
deye.py restore_state() currently restores control_active only when applied_payload is non-empty, which can drop a fresh persisted control_active after applied_payload has been intentionally cleared, reintroducing the “no automatic re-apply after restart” failure mode.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (3)
Resolved since last review (2)
Fix typo in docstring for save_control method. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Fix control_active handling after restart to ensure _reconcile_control() functions correctly. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…ed on purpose run() pops a serial from applied_payload once its order stays unconfirmed past DEYE_ORDER_MAX_POLLS and calls save_control() in the same cycle, so an empty payload beside a live control_active is a normal persisted state. Gating the restore on a non-empty payload discarded both and left _reconcile_control gated off after a restart. Also drops the remaining hard-coded source line numbers from the restore_state comments and test docstrings; three of the four were already pointing at the wrong lines.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Deye’s restore bound uses a strict < comparison which likely causes an off-by-one stale-cache decision at exactly the configured age limit.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
| # _reconcile_control gated off after a restart - the exact failure this fix is for, | ||
| # on the one inverter that had just been told to re-write. | ||
| if (isinstance(applied, dict) and applied) or (isinstance(active, list) and active): | ||
| if age is not None and age < DEYE_RESTORE_MAX_CONTROL: |
CossieRob
pushed a commit
to CossieRob/batpred
that referenced
this pull request
Sep 20, 2026
…for the 2026-09-19 merge batch 7 of 15 candidates folded (GH#2033, GH#2444 x2, PR#4756, PR#4972, GH#1406 residual, GH#5066 nuance, teslemetry midnight flake), 8 dropped as already covered. Corrects GH#4165/springfall2008#4641 row for PR springfall2008#5114, GH#5033 row for the springfall2008#5149 balance rework, the debug-snapshot bullet and GH#5070 row for PR springfall2008#5071, the GH#5063 example for PR springfall2008#5053, and adds the myenergi springfall2008#5150 clause. Co-Authored-By: Claude Code <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>
This was referenced Oct 2, 2026
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.
(Written by Claude, posted by @chalfontchubby.)
Problem
Both Sunsynk Cloud and Deye Cloud gate their automatic per-cycle re-apply of the plan (
_reconcile_control) on an in-memorycontrol_activeset: an inverter is only written to once Predbat has pressed its "write" button for it at least once. That set is never persisted, so it resets to empty on every Predbat restart, and nothing re-populates it until a plan change happens to look different enough to trigger a fresh button press. Until then,_reconcile_controlsilently does nothing every cycle - no write, no log line - for every affected inverter, including a write that would have stopped an export or charge already under way.This is believed to be the mechanism behind #5138 (Sunsynk export staying on well past its planned window), based on reading the code rather than a live capture that pins it down directly - the log attached to that issue is too short to catch the moment the write should have happened. Once found in
sunsynk.py, the same pattern was checked againstdeye.pyby reading its equivalent_reconcile_control/save_control/restore_stateand found to have the identical gap - that half is a code-reading finding, not a separately reported bug.alphaess.pyalready avoids this: itssave_control/restore_statepersistcontrol_activealongsideapplied_payload.Fix
Two commits, one per file, each mirroring the
alphaess.pypattern:save_controlnow also persistscontrol_active.restore_staterestores it alongsideapplied_payload, under the same cache-age bound (SUNSYNK_RESTORE_MAX_CONTROL/DEYE_RESTORE_MAX_CONTROL), so a genuinely stale cache still discards both and forces a fresh write-button press to recommit, rather than trusting old control state indefinitely.Scope
This does not address the separate, already-open #5142 (a write the cloud API acknowledges but the inverter's dongle never actually collects, currently only detected by a warning after ~45 minutes with no forced retry). That's a write attempted but not confirmed; this PR is about a write never attempted at all after a restart. The two are independent and can land separately.
Testing
New tests in
test_sunsynk_storage.pyandtest_deye_storage.pycovercontrol_activeround-tripping throughsave_control/restore_state, and being discarded alongside a staleapplied_payloadcache past the age bound. Full quick suite andrun_pre_commitgreen.Related to #5138.