fix(car-charging): hold battery discharge during car charging regardless of car_energy_reported_load (#4246)#4264
Merged
Conversation
…rted_load is Off Adds a regression case mirroring no_discharge_car_demand1 but with car_energy_reported_load=False, asserting discharge should still be held (status "Hold for car", pause_discharge=True). Currently fails, reproducing the reported behaviour where the discharge-hold protection is silently skipped for CT-clamp-blind-spot topologies where the battery and car still share a busbar. See issue #4246.
…ess of car_energy_reported_load car_energy_reported_load only controls whether car load is visible to the CT clamp for house-load/export accounting purposes - it says nothing about whether the battery can physically reach the car. Previously the discharge-hold gate for car_charging_from_battery=Off was also conditioned on car_energy_reported_load being On, so CT-clamp-blind-spot topologies (e.g. a Henley block tapped ahead of the clamp) where the battery and car still share a busbar got no discharge protection at all during car charging. Removes that condition in execute.py and prediction.py (with the matching change mirrored in prediction_kernel.cpp to keep C++/Python parity), so discharge is held whenever car_charging_from_battery is Off, independent of CT-clamp visibility. The charge decision itself is unaffected - charge window/rate and discharge-hold are controlled by separate code paths, so a window that's genuinely charging from grid still charges as planned. Updates no_discharge_car_demand1b to assert the corrected behaviour and folds in the reproduction case added previously. See issue #4246.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot review flagged that "the battery shares the same busbar either way" is a claim Predbat can't actually verify from car_energy_reported_load alone - that switch only reflects whether EV energy is counted in the CT-clamp house-load model. Reworded the remaining comments (execute.py, prediction.py, prediction_kernel.cpp) to match the wording already applied to the test_execute.py comment, stating only what the setting controls rather than asserting a specific wiring topology.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes an incorrect discharge-hold gating condition during EV charging windows: battery discharge is now held whenever car_charging_from_battery = Off, even when car_energy_reported_load = Off (shared-busbar / upstream-of-CT topologies). This aligns both the runtime execution path and the Python/C++ prediction engines with the intended semantics of those switches.
Changes:
- Remove
car_energy_reported_loadfrom the discharge-hold gate inexecute.py, so discharge hold is enforced during active car slots whenever charging-from-battery is disabled. - Apply the same logic change to the Python prediction engine (
prediction.py) and the C++ kernel (prediction_kernel.cpp) to maintain parity. - Update the execute regression test (
no_discharge_car_demand1b) to assert “Hold for car” + discharge pause whencar_energy_reported_load=False.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| apps/predbat/execute.py | Enforces discharge-hold during car charging whenever car_charging_from_battery is disabled, independent of CT-clamp visibility. |
| apps/predbat/prediction.py | Mirrors the same discharge-hold behavior in the Python prediction model for correctness and parity. |
| apps/predbat/prediction_kernel.cpp | Mirrors the same change in the C++ kernel to keep compiled-kernel behavior aligned with Python. |
| apps/predbat/tests/test_execute.py | Adjusts regression expectations to validate the corrected discharge-hold behavior when car_energy_reported_load=False. |
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.
Fixes #4246. Carries over the fix from #4247 (by @chalfontchubby) into the main repo so the cross-platform kernel binaries can be rebuilt and committed by CI (the
kernel-binariesauto-commit job can't run on fork PRs, which is why #4247 failed the checked-in-binary parity verify step).The fix
Removes the
and self.car_energy_reported_loadcondition from the discharge-hold gate inexecute.pyandprediction.py(mirrored inprediction_kernel.cppfor C++/Python parity), socar_charging_from_battery = Offalways holds discharge during car-charging windows, regardless of CT-clamp visibility.Why a new PR
#4247 came from a fork, so the
kernel-binariesjob (which needscontents: writeto cross-build and commit theprediction_kernel_lib_*.sobinaries back to the branch) was skipped. That left the checked-in x86_64 binary stale relative to the modifiedprediction_kernel.cpp, failing the parity verify step. Opening from a branch in this repo lets that job run and refresh the binaries.Original commits and authorship from @chalfontchubby are preserved on this branch.
Verification
no_discharge_car_demand1bintest_execute.py) passes against the fix, fails againstmain.kernel_parityandmodel_kernel.run_pre_commitpasses cleanly.Supersedes #4247. Credit to @chalfontchubby for the fix.