Split charge windows at dawn for any combine_charge_slots user, not just low power mode - #4604
Merged
Merged
Conversation
…s user, not just low power mode The dawn light/dark split (calc_dawn, #4557) only ever mattered when combine_charge_slots could merge a window across sunrise - with it off, find_charge_window already forces a break every charge_slot_split minutes (= plan_interval_minutes, the same granularity calc_dawn buckets at), so the split was already a no-op there. It was gated on set_charge_low_power instead, which meant combine_charge_slots users without low power charging never got it, even though the split also lets the plan optimizer charge just the dark portion of a combined window and skip the daylight portion where solar may cover the load - a benefit independent of low power charging. New calc_pv_light_dark() gates the split on combine_charge_slots directly. Also logs the calculated dawn time each time it runs, so it's visible whether the detected dawn looks sane. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts how Predbat splits charge windows at the PV light/dark (dawn) boundary so the split is applied for any user with combine_charge_slots enabled, rather than only when set_charge_low_power is enabled. This improves the planner’s ability to treat cheap-rate windows spanning sunrise as separable dark/light portions for optimization.
Changes:
- Add
calc_pv_light_dark()to gate dawn-splitting oncombine_charge_slotsand updatefetch_sensor_data()to use it. - Enhance
calc_dawn()documentation and add logging of the detected dawn time (or absence of one). - Add a new unit test
test_calc_pv_light_darkcovering allcombine_charge_slots × set_charge_low_powercombinations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/predbat/fetch.py | Introduces calc_pv_light_dark() and switches dawn boundary computation to be driven by combine_charge_slots; adds dawn-time logging in calc_dawn(). |
| apps/predbat/tests/test_find_charge_window.py | Adds test_calc_pv_light_dark and wires it into the existing test_find_charge_window test flow. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Summary
calc_dawn()(added for Low power charge toggle does nothing #4557) splits a charge window at the light/dark PV boundary so a window spanning sunrise doesn't get treated as one indivisible block. It was only ever computed whenset_charge_low_powerwas on.combine_charge_slotsis on in the first place — with it off,find_charge_windowalready forces a break everycharge_slot_splitminutes (=plan_interval_minutes, the same granularitycalc_dawnbuckets at), so the dawn boundary can never be reached and computing it was a no-op there.combine_charge_slotsusers without low power charging never got the split, even though it has a benefit independent of low power: it lets the plan optimizer charge just the dark portion of a combined window and skip the daylight portion where solar may cover the load.calc_pv_light_dark()decides whether to callcalc_dawn(), gated oncombine_charge_slotsinstead ofset_charge_low_power.calc_dawn()now also logs the calculated dawn time each cycle it runs, so it's visible in the log whether the detected dawn looks sane.Test plan
./run_all --test find_charge_window— newtest_calc_pv_light_darkcovers all fourcombine_charge_slots×set_charge_low_powercombinations; existingtest_calc_dawnand Path J (pv_light_dark boundary) tests unaffected./run_all --quick— full quick suite passes, including the random scenario plan regression against the committed baseline./run_pre_commit— black/ruff/cspell/docstring coverage all pass🤖 Generated with Claude Code