fix(car): stop the car charge start time walking forward (#269) - #5120
Open
chalfontchubby wants to merge 1 commit into
Open
chalfontchubby wants to merge 1 commit into
chalfontchubby wants to merge 1 commit into
Conversation
plan_car_charging() clamps an in-progress window's start to minutes_now so the kWh maths only counts time the car can still charge for. The plan is rebuilt from scratch every cycle, so that clamped start was also what got published as predbat.car_charging_start - making the displayed start time advance by five minutes on every cycle while the car was charging. Record the window's real start as start_orig and publish that for both car_charging_start and the slot sensor's planned attribute. All the kWh and cost arithmetic still runs on the clamped start, so no planning behaviour changes. The four components that drive a charger off the published plan (Ohme, myenergi, GE Cloud, gateway) only test start <= now < end against a freshly-read plan, so their answers are unchanged and their lower bound gains the margin the clamp had removed. Octopus Intelligent slots never go through plan_car_charging(), carry no start_orig, and are left exactly as they were. Addresses #269 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
This PR was written by Claude, working with me.
Fixes the oldest outstanding issue in the tracker, from November 2023.
The problem
Once a car charging window is underway,
predbat.car_charging_startadvances by five minutes on every cycle, so the displayed "charging starts at" never settles — it just tracks the clock. The actual charge session is unaffected, which is why it went unnoticed for so long.Root cause
plan_car_charging()clamps a window's start tominutes_now:This is correct, and load-bearing. Everything below it treats
start→endas deliverable charging time —length = end - start, thenkwh = rate * hours. The plan is rebuilt from scratch every cycle against the car's current SoC, with no memory that a window already began, so without the clamp a 23:30–05:30 window still on the books at 03:00 would be credited with six hours of charging when only 2.5 remain, and the planner would stop allocating later windows it actually needs.The bug is that
publish_car_plan()then reused that planning-internal value as the user-facing timestamp.Fix
plan_car_charging()records the window's real start asstart_orig;publish_car_plan()displays that for bothcar_charging_startand the slot sensor'splannedattribute. All kWh and cost arithmetic still uses the clamped start, so no planning behaviour changes.Effect on charger control
plannedis not just a display — Ohme, myenergi, GE Cloud and the gateway all read it back out of HA and drive real chargers from it. All four usestartfor one thing only, a containment test:With the clamp this degenerates to
now <= now < end— right, but with zero margin on the lower bound. Each re-reads the attribute and rebuilds its window list wholesale every cycle, so none holds state the drift could corrupt. Their answers are unchanged, and the lower bound regains its margin.One genuine change worth a reviewer's eye: all four parse
startand repair the year when it looks too far in the past (start < now - 23h). Un-clamping means a start can now legitimately be hours old rather than pinned to now. That stays inside the 23-hour threshold for any car-charging window, which is bounded by the ready time, but it is the one place where the old and new values are not interchangeable.Octopus Intelligent slots never go through
plan_car_charging()—fetch_sensor_data_car_planning()takes a separate branch for IOG cars — so they carry nostart_origand the.get()fallback leaves them exactly as they are.Testing
New regression test
car_charging_start_stable_issue269stepsminutes_nowforward a cycle at a time while staying inside one planned window, and asserts the published start does not move.Validated as a negative control: reverting only the publisher makes it print
['12:00:00', '12:05:00', '12:10:00']— the exact drift from the issue — and it holds at12:00:00with the fix. The test also asserts the first window is at least 15 minutes long, so it cannot silently go vacuous if the fixture's rate windows ever shorten.Full suite green, pre-commit clean. No docs change — the entity keeps its meaning, it just stops being wrong.