Subaru: handle transient API failures returning empty payloads#29036
Draft
ummon-v wants to merge 4 commits intoevcc-io:masterfrom
Draft
Subaru: handle transient API failures returning empty payloads#29036ummon-v wants to merge 4 commits intoevcc-io:masterfrom
ummon-v wants to merge 4 commits intoevcc-io:masterfrom
Conversation
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- ValueInKilometers currently truncates the converted float to int64; consider using rounding (e.g. via math.Round) to avoid systematically under-reporting range after unit conversion.
- status() only checks for an empty Unit string to detect transient empty payloads; if the API can return other sentinel values (e.g. zero range with a non-empty unit) for failures, it may be worth broadening the validation so invalid data is consistently mapped to api.ErrMustRetry.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- ValueInKilometers currently truncates the converted float to int64; consider using rounding (e.g. via math.Round) to avoid systematically under-reporting range after unit conversion.
- status() only checks for an empty Unit string to detect transient empty payloads; if the API can return other sentinel values (e.g. zero range with a non-empty unit) for failures, it may be worth broadening the validation so invalid data is consistently mapped to api.ErrMustRetry.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
andig
reviewed
Apr 12, 2026
vehicle/subaru/provider.go
Outdated
| return res, err | ||
| } | ||
| if res.Payload.EvRangeWithAc.Unit == "" || | ||
| res.Payload.BatteryLevel == 0 && res.Payload.EvRangeWithAc.Value == 0 { |
Member
There was a problem hiding this comment.
this won't help as it does not clear the cache. The right thing to do would be not storing the invalid value in the first place.
Contributor
Author
There was a problem hiding this comment.
I'm not 100% sure but it should be addressed with last push so zero can still be returned on a bad call, but it's never returned without the error, and the cache won't sit on it retrying immediately.
Do you think this is a good solution?
andig
reviewed
Apr 12, 2026
| return impl | ||
| } | ||
|
|
||
| func (v *Provider) status() (Status, error) { |
andig
reviewed
Apr 12, 2026
| func (v *Provider) Range() (int64, error) { | ||
| res, err := v.status() | ||
| return int64(res.Payload.EvRangeWithAc.Value), err | ||
| if err != nil { |
Member
There was a problem hiding this comment.
why change this? please don't make arbitrary unrelated changes!
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.
Subaru Connected Services API occasionally returns empty payloads on transient failures. Without validation, these responses surface as 0% SoC / 0 km range, causing evcc to act on bad data (e.g. starting unwanted charging sessions).
Implementation
util.Cached) to avoid redundant requests and to retain last known good valuesapi.ErrMustRetryinstead of propagating invalid data