Fix: float slot end crash in yesterday_reconstruct_car_slots (issue #3911)#3920
Merged
Conversation
) round(x, 0) returns a Python float (e.g. 4.0 not 4), so when plan_car_charging / plan_iboost_smart clamp a slot length to the remaining kWh, the computed end time becomes a float such as 1570.0. range(start, float_end, step) then raises TypeError in yesterday_reconstruct_car_slots. Fix: wrap the min(...) expression with int() at both sites in plan.py so slot start/end values are always integers. Add regression tests to run_car_charging_smart_tests that verify all slot end values are int when the length-clamping path is triggered (battery_size=11.9 reproduces the exact reporter config).
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a crash in yesterday_reconstruct_car_slots caused by plan_car_charging (and similarly plan_iboost_smart) sometimes producing float start/end minute values after length-clamping, which later breaks range(start, end, step).
Changes:
- Ensure clamped
lengthvalues are converted tointin bothplan_car_chargingandplan_iboost_smartso resulting slot boundaries remain integer minutes. - Add regression tests in
test_car_charging_smart.pyto confirmplan_car_chargingslotstart/endare alwaysintfor configurations that previously triggered the float-end bug.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/predbat/plan.py | Casts clamped slot lengths to int in car-charging and iBoost smart planning to prevent float minute boundaries. |
| apps/predbat/tests/test_car_charging_smart.py | Adds regression coverage to ensure car charging slots always have integer start/end values. |
Comment on lines
4139
to
4144
| # Scale down the number of minutes if the value exceeds the max | ||
| if kwh > iboost_left: | ||
| percent = iboost_left / kwh | ||
| length = min(round(((length * percent) / 5) + 0.5, 0) * 5, end - start) | ||
| length = int(min(round(((length * percent) / 5) + 0.5, 0) * 5, end - start)) | ||
| end = start + length | ||
| hours = length / 60 |
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.
Problem
Fixes #3911 — crash introduced in v8.39.2:
TypeError: 'float' object cannot be interpreted as an integerinyesterday_reconstruct_car_slots.Root Cause
In
plan.py, bothplan_car_chargingandplan_iboost_smartcontain a length-clamping block that fires when the battery is nearly full and the charging window supplies more energy than needed:round(x, 0)in Python 3 returns a float (e.g.4.0, not4), solengthand thereforeend = start + lengthbecome floats (e.g.1570.0). These float values are stored in the slot dict and later passed torange(start, end, PREDICT_STEP)inyesterday_reconstruct_car_slots, which raisesTypeErrorbecauserangerequires integers.Users with fractional
car_charging_battery_size(e.g.11.9) or iBoostiboost_max_energyvalues are most likely to hit the clamping path and trigger this crash.Fix
Wrap the
min(...)result withint()at both sites:This is a minimal, targeted change — only the two clamping expressions are touched.
Tests
Added
run_car_charging_slot_integer_testtotest_car_charging_smart.py. Three new test cases (smart_int1/2/3_issue3911) confirm that:battery_size=11.9, soc=9.9(exact reporter config) — slotendisintbattery_size=11.9, soc=0.0— all slots haveintendsbattery_size=77.0, soc=74.3, loss=0.9— different battery/loss combinationAll existing tests continue to pass (
./run_all --quick).