Vehicle API charger: trigger wakeup when vehicle is asleep#29598
Open
andig wants to merge 1 commit into
Open
Conversation
Closes #28652 The vehicle-api charger propagated api.ErrAsleep from Status(), causing loadpoint Update() to early-return before reaching setLimit/Enable. The existing wake-up logic in setLimit (which calls vv.WakeUp() on ErrAsleep) was therefore never triggered, leaving the vehicle asleep indefinitely even with an active plan. Treat ErrAsleep as connected-not-charging (StatusB) so the loadpoint flow continues. Enable(true) -> ChargeEnable(true) -> StartCharging either implicitly wakes the vehicle (Tesla Fleet API behaviour) or returns ErrAsleep, which the existing setLimit path handles via the rate-limited WakeUp() retry. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
ErrAsleephandling in both theisVehicleAtHomebranch and thev.Status()branch is nearly identical; consider extracting this into a small helper or at least centralizing the logic so future changes to the asleep behavior aren’t duplicated in two places. - When treating
ErrAsleepasStatusByou currently do so silently; adding a debug-level log in these branches would make it much easier to understand why a vehicle is considered connected when it is actually asleep during troubleshooting. - The comment "assume previously known location is still valid" in the
isVehicleAtHomeerror path is a bit ambiguous since the code doesn’t reference any stored location here; consider clarifying how/where that state is maintained or rephrasing to avoid implying behavior that isn’t explicitly visible in this function.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `ErrAsleep` handling in both the `isVehicleAtHome` branch and the `v.Status()` branch is nearly identical; consider extracting this into a small helper or at least centralizing the logic so future changes to the asleep behavior aren’t duplicated in two places.
- When treating `ErrAsleep` as `StatusB` you currently do so silently; adding a debug-level log in these branches would make it much easier to understand why a vehicle is considered connected when it is actually asleep during troubleshooting.
- The comment "assume previously known location is still valid" in the `isVehicleAtHome` error path is a bit ambiguous since the code doesn’t reference any stored location here; consider clarifying how/where that state is maintained or rephrasing to avoid implying behavior that isn’t explicitly visible in this function.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
Closes #28652
Summary
The
vehicle-apicharger propagatedapi.ErrAsleepfromStatus(), causing loadpointUpdate()to early-return atcore/loadpoint.go:1962before reachingsetLimit/Enable. The existing wake-up plumbing insetLimit(loadpoint.go:951— callsvv.WakeUp()onErrAsleep) was therefore never triggered, so a Tesla using the Vehicle-API charger could fall asleep and never wake up again, even with an active plan ormode: now.Fix
Treat
ErrAsleepinVehicleApi.Status()as connected, not charging (StatusB) instead of propagating the error. This lets the rest ofUpdate()run; the existing wake-up path then handles the sleeping vehicle:For Tesla Fleet API,
StartCharging()is one of the calls that wakes the vehicle implicitly, so in many cases the implicit wake is enough; the explicitWakeUp()only fires as the rate-limited retry on the next cycle if needed.Why this approach over patching
Update()The issue body suggests handling
ErrAsleepdirectly inUpdate(). That works but:wakeupAttempts = 6,wakeupTimeout = 30sfromcore/timer.go),planActive()— would still skipmode == now, welcome-charge, SOC-driven charging,setLimit.Re-using the existing path keeps semantics consistent across charger types.
Edge cases
mode == off, no plan, vehicle asleep:Status()reportsStatusB, strategy switch runssetLimit(0)→Enable(false)→ Tesla controller'sStopCharging()swallowsErrAsleep(vehicle/tesla/controller.go:48). No spurious wake-up.isVehicleAtHomewould normally fail becausePosition()requires the vehicle to be awake. With the patch, that path also returnsStatusB. The nextEnable(true)call goes to Tesla, the implicit wake fires, and the nextStatus()call gets a real position — if it's outside the geofence,Status()correctly returnsStatusA. Worst case: one unnecessary wake before the geofence kicks back in.Update, treated as connected. Matches user expectation since they have no other connection signal.Test plan
Updatecycle, plan executesmode = now→ wakes within oneUpdatecyclemode = off, no plan → no wake-up calls, no log spamStatusA, no behaviour changeUpdate(treated as connected)🤖 Generated with Claude Code