Skip to content

fix(fetch): split charge windows at dawn again, unify low-power PV thresholds - #4726

Merged
springfall2008 merged 8 commits into
mainfrom
fix/dawn-detection-fix
Sep 15, 2026
Merged

springfall2008 merged 8 commits into
mainfrom
fix/dawn-detection-fix

Conversation

@chalfontchubby

Copy link
Copy Markdown
Collaborator

Summary

Fixes #4699. self.pv_forecast_minute was reset to {} at the top of fetch_sensor_data(), but calc_pv_light_dark()/calc_dawn() still ran before the line that actually repopulates it - so the dawn split (#4557) always saw an empty forecast and never fired, for any combine_charge_slots user, since the feature first shipped. Fixed by moving the populating fetch_pv_forecast() call earlier.

Separately, the debug-replay test harness's own rate-rescan reimplementation (tests/test_single_debug.py) never passed pv_light_dark through to rate_scan_window() at all - an independent gap that meant no debug.yaml replay could exercise this path even after the production fix. Both are fixed here, and together they let the real fix be verified against the reporter's own debug.yaml: the window now splits at dawn with the pre-dawn slice going to low power instead of full rate.

Threshold unification

While investigating, found the two low-power PV thresholds were independently tuned and could disagree on genuine twilight PV:

  • LOW_POWER_PV_LIGHT_FRACTION - a % of this forecast's own peak, used to decide where to split a window at dawn
  • LOW_POWER_PV_THRESHOLD - a fixed kWh figure, used to decide whether to abandon low-power charging for a window

A %-of-peak threshold is also unreliable on a heavily overcast day, since that day's own peak is low too - the same % ends up being a much lower absolute wattage than on a clear day, and Predbat has no declared panel capacity to normalise against instead (only inverter_limit, which caps the inverter's own output, not the array's rating).

Replaced both constants with a single new config item, low_power_pv_threshold_w (absolute Watts, default 150W), used consistently by:

  • calc_dawn (fetch.py) - the split boundary
  • find_charge_rate (utils.py) - the abandon decision, now comparing average power over the remaining window against the threshold rather than accumulated energy against a fixed kWh figure, so a long window at a low constant trickle no longer creeps past it regardless of window length (the actual twilight-creep failure mode this was hiding)

Also added binary_sensor.predbat_dawn (on past dawn / off before or unclassified) and updated docs for both the sensor and the new config item.

Verification

  • Root cause confirmed via git blame + reading (deterministic, not probabilistic): the reset and the read are in the same function, no path repopulates in between.
  • The average-power fix verified as a genuine regression: temporarily reverted to the old fixed-kWh comparison and confirmed the new low_power_below_threshold_stays_low_power test fails without it (13.62kWh -> 14.2kWh final SoC), passes with it.
  • One random regression scenario (seed 15) shifted as an expected consequence of the behaviour actually changing - baseline regenerated and confirmed it's the only one that moved.
  • Full run_pre_commit clean.

Test plan

  • ./run_all --quick passes
  • ./run_all --test find_charge_window --test find_charge_rate --test model - new/updated scenarios pass
  • ./run_all --test random - only seed 15 changed, as expected
  • ./run_pre_commit passes (ruff, black, cspell, markdownlint, full test suite)
  • Verified against reporter's own debug.yaml that the dawn split now fires and produces low-power pre-dawn charging

🤖 Generated with Claude Code

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Threshold boundary handling, configuration visibility, dawn sensor state, and production-order test coverage need correction.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Fixes dawn-based charge-window splitting and unifies low-power PV thresholds.

Changes:

  • Fetches PV forecasts before calculating dawn splits.
  • Adds a configurable 150 W PV threshold and dawn sensor.
  • Updates low-power behavior, tests, documentation, and regression baseline.
File summaries
File Description
apps/predbat/config.py Adds the PV threshold setting.
apps/predbat/const.py Removes superseded constants.
apps/predbat/execute.py Passes the threshold during execution.
apps/predbat/fetch.py Reorders PV fetching, calculates dawn, and publishes its state.
apps/predbat/output.py Applies the threshold when displaying charge rates.
apps/predbat/predbat.py Initializes dawn classification state.
apps/predbat/prediction.py Propagates the threshold into predictions.
apps/predbat/utils.py Uses average PV power for low-power decisions.
apps/predbat/tests/test_find_charge_rate.py Updates PV overlap tests.
apps/predbat/tests/test_find_charge_window.py Tests absolute-threshold dawn detection.
apps/predbat/tests/test_model.py Adds a low-PV regression scenario.
apps/predbat/tests/test_single_debug.py Includes dawn splitting during debug replay.
coverage/cases/random_results.json Updates the affected random baseline.
docs/customisation.md Documents threshold behavior and configuration.
docs/output-data.md Documents the dawn sensor.
Review details

Suppressed comments (1)

apps/predbat/fetch.py:1643

  • A configured threshold of 0 W makes every all-zero bucket satisfy average >= 0, so an empty-production day is classified as light from midnight and the dawn sensor reports on. Since 0 is an allowed value, require some positive PV before latching light; this preserves the useful meaning “any non-zero PV” without treating darkness as dawn.
        light_threshold = self.low_power_pv_threshold_w / MINUTE_WATT
  • Files reviewed: 14/15 changed files
  • Comments generated: 4
  • Review effort level: Balanced

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

Comment thread apps/predbat/config.py Outdated
Comment thread apps/predbat/utils.py Outdated
Comment thread apps/predbat/fetch.py Outdated
Comment thread apps/predbat/fetch.py Outdated
chalfontchubby and others added 2 commits August 28, 2026 13:54
…resholds

Fixes #4699. self.pv_forecast_minute was reset to {} at the top of
fetch_sensor_data() but calc_pv_light_dark()/calc_dawn() still ran before the
line that actually repopulates it - so the dawn split (#4557) always saw an
empty forecast and never fired, for any combine_charge_slots user, since the
feature first shipped. Fixed by moving the populating fetch_pv_forecast()
call earlier. Separately, the debug-replay test harness's own rate-rescan
reimplementation never passed pv_light_dark through at all, so no debug.yaml
replay could exercise this path even after the fix - both are now fixed.

While investigating, found the two low-power PV thresholds
(LOW_POWER_PV_LIGHT_FRACTION, a % of the forecast's own peak, and
LOW_POWER_PV_THRESHOLD, a fixed kWh figure) were independently tuned and
could disagree on genuine twilight PV. A % of this forecast's own peak is
also unreliable on a heavily overcast day, whose own peak is low too.
Replaced both with a single new config item, low_power_pv_threshold_w (an
absolute Watts figure, default 150), used consistently by calc_dawn (split
boundary) and find_charge_rate (low-power abandon decision, now comparing
average power over the remaining window rather than accumulated energy, so a
long window at a low constant trickle no longer creeps past the threshold).

Added binary_sensor.predbat_dawn, docs updated for both the sensor and the
new config item. Verified the average-power fix as a genuine regression:
temporarily reverted to the old fixed-kWh comparison and confirmed the new
low_power_below_threshold_stays_low_power test fails without it. One random
regression scenario (seed 15) shifted as an expected consequence of the
behaviour actually changing - baseline regenerated, confirmed it's the only
one that moved.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- low_power_pv_threshold_w was hidden behind set_charge_low_power, but
  calc_dawn() also uses it whenever combine_charge_slots is on
  regardless of that setting - ungated so it can always be tuned
- find_charge_rate's low-power abandon check used > against the
  threshold while calc_dawn used >=, so a flat forecast sitting
  exactly at the threshold disagreed with itself; unified to >=,
  guarded so a 0W threshold still never fires on genuinely zero PV
- the dawn binary_sensor only updated inside "if rate_import", so it
  went stale (or was never created) whenever import rates were
  briefly unavailable; extracted fetch_pv_forecast_and_dawn() to fetch
  the PV forecast, compute the split, and publish the sensor as one
  unconditional step, ahead of the rate-dependent window scan
- added a focused test exercising that real extracted method (mocking
  only its one external dependency) rather than reimplementing the
  fetch-then-classify sequence by hand, which would not have caught
  the original #4699 ordering bug - confirmed it fails when the two
  steps are reordered and passes with the fix
@chalfontchubby
chalfontchubby force-pushed the fix/dawn-detection-fix branch from cfa2382 to df1dd0f Compare August 28, 2026 15:32
@chalfontchubby chalfontchubby added the BOT_CLEANUP Trigger: bot should address PR review feedback and CI failures, then commit and push label Aug 31, 2026
@springfall2008

Copy link
Copy Markdown
Owner

No outstanding feedback or CI failures to address: all four review threads from the 2026-08-25 Copilot review were replied to and fixed in df1dd0f (2026-08-28) — the enable gate was removed from low_power_pv_threshold_w, the max-rate/abandon comparison was unified to >= with a pv_window_kwh > 0 guard, and the dawn sensor publish was moved into an unconditional fetch_pv_forecast_and_dawn() with ordering coverage in test_fetch_pv_forecast_and_dawn. Both CI checks (kernel-binaries, pre-commit) passed on that commit; no review activity since.

@springfall2008 springfall2008 removed the BOT_CLEANUP Trigger: bot should address PR review feedback and CI failures, then commit and push label Aug 31, 2026
@springfall2008 springfall2008 added the BOT_CLEANUP Trigger: bot should address PR review feedback and CI failures, then commit and push label Sep 5, 2026
CI and others added 2 commits September 5, 2026 19:23
# Conflicts:
#	apps/predbat/fetch.py
#	coverage/cases/random_results.json
… low-power PV threshold

Co-Authored-By: Claude Code <noreply@anthropic.com>
@springfall2008

Copy link
Copy Markdown
Owner

Automated comment from the triage bot. Routine cleanup check on this PR — no outstanding review feedback or CI failures to address: all four Copilot threads were already replied to and fixed in df1dd0f, and both CI checks were green there.

What this push contains:

  • Merged origin/main into the branch. The only code conflict was the const import block in fetch.py: main added imports of LOW_POWER_PV_LIGHT_FRACTION and the cloud-envelope constants, while this PR deletes LOW_POWER_PV_LIGHT_FRACTION from const.py (its usage was already replaced by the configurable low_power_pv_threshold_w), so the merge keeps main's CLOUD_WINDOW_MINUTES / CLOUD_ARRAY_MARGIN / PV_ARRAY_KWP_UNKNOWN imports and drops the deleted constant. Verified the PR's threshold threading (find_charge_rate in utils.py), the ungated config item, and the extracted fetch_pv_forecast_and_dawn() all survived the merge intact.
  • Regenerated the seed 15 entry in coverage/cases/random_results.json: this PR's threshold change (fraction-of-peak → absolute watts) moves that one scenario slightly, the same seed and fields the PR's own baseline regeneration already moved. All other 19 scenarios match main's baseline bit-identically; the full suite re-ran green afterwards (random, find_charge_window, find_charge_rate, model, debug_cases).

Note: run_pre_commit could not be run locally (blocked by the session's permission mode), but the two edits here are an import-list line removal and baseline JSON values, so no formatting/docstring/spelling surface is touched — CI's pre-commit check will confirm on this push.

@springfall2008 springfall2008 removed the BOT_CLEANUP Trigger: bot should address PR review feedback and CI failures, then commit and push label Sep 5, 2026
@springfall2008
springfall2008 requested a balanced review from Copilot September 7, 2026 07:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The ordering regression, threshold consistency, state publication, tests, and documentation are addressed coherently with no unresolved issues found.

Review details
  • Files reviewed: 14/15 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

chalfontchubby and others added 3 commits September 9, 2026 08:22
Low power charging is abandoned for a charge window whose forecast solar
averages above low_power_pv_threshold_w, the full rate being used instead so
the solar reaches the battery rather than being exported and bought back
later (#4373).

That trade depends on what import costs in the window, which the PV forecast
cannot answer. A user with a free or very cheap daytime import period loses
nothing by throttling: the spilled solar was going to be exported anyway and
the grid fill is free. #4975 is exactly that case, a daily 11:00-14:00 free
window sitting entirely in daylight, where low power can never engage no
matter how the thresholds are tuned.

The two asks are opposites and nothing in the solar data separates them, so
add a switch rather than change the behaviour. set_charge_low_power_solar_full_rate
defaults to True, keeping #4373's behaviour for everyone, and turning it off
lets a solar-overlapping window be throttled on its own charge_left /
minutes_left just as a dark one is.

The dawn split is unaffected and still does the work it was added for: the
dark portion of a sunrise-spanning window is a separate window either way.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
@springfall2008
springfall2008 merged commit 6e73d4e into main Sep 15, 2026
2 checks passed
@springfall2008
springfall2008 deleted the fix/dawn-detection-fix branch September 15, 2026 19:22
@spangingpan

Copy link
Copy Markdown

Has this been added to version 9.0.3?
I am still having the "Low power mode: PV forecast in window 0.78kWh > 0.1kWh, default to max rate",
plus I can't find the entity: binary_sensor.predbat_dawn

@chalfontchubby

Copy link
Copy Markdown
Collaborator Author

https://github.com/springfall2008/batpred/releases

No - it looks like that commit missed 9.0.1

Specifically...
`commit 6e73d4e
Merge: 340205f c45dd6a
Author: Trefor Southwell 48591903+springfall2008@users.noreply.github.com
Date: Tue Sep 15 20:22:46 2026 +0100

Merge pull request #4726 from springfall2008/fix/dawn-detection-fix

fix(fetch): split charge windows at dawn again, unify low-power PV thresholds

commit 340205f (tag: v9.0.3)
Author: Trefor Southwell 48591903+springfall2008@users.noreply.github.com
Date: Tue Sep 15 20:16:48 2026 +0100

Bump version from v9.0.2 to v9.0.3

`
Missed by 6 minutes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Low power mode still an issue after 4577 fix

4 participants