fix(drives): keep mileage when a position has no odometer - #5637
Open
mews-se wants to merge 1 commit into
Open
Conversation
close_drive/2 reads the mileage of a drive with first_value/last_value over its positions. Neither skips NULLs, so a drive whose first or last position carried no odometer was closed with a NULL start_km, end_km and distance, permanently. That happens on Fleet Telemetry setups, where a record can be built from a single partial payload. min and max ignore NULLs, and since the odometer never decreases within a drive they return the same values as before whenever every position carries one. A migration recomputes the three columns for the drives that are already affected and still have positions to compute from.
✅ Deploy Preview for teslamate ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Member
|
Thanks for your contribution. That being said: if an issue is assigned (like in this case), the PR will come form the assignee. So I tend to close this PR, you can convince me otherwise. |
JakobLichterfeld
added
the
note:discussion
Contributor
Author
|
My bad! I mistook your thumbs up for a go earlier. I have no objection to you closing it - and feel free to use anything you find useful! :) |
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
2 participants
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.
Addresses case 1 of #5583 — drives losing
start_km— and stays inside the Log layer as direction 3 suggests, so no state machine changes.The bug
close_drive/2reads a drive's mileage withfirst_value(p.odometer)/last_value(p.odometer)over the drive's positions. Neither skips NULLs, so a drive whose first or last position carries no odometer is closed withstart_km,end_kmanddistanceall NULL — permanently, since a closed drive is never recomputed. That is the reported symptom on Fleet Telemetry setups, where a record can be built from a single partial payload.minandmaxignore NULLs, and since the odometer never decreases within a drive they return exactly the same values asfirst_value/last_valuewhenever every position carries one. The Owner API path is therefore unchanged.Changes
close_drive/2:start_km,end_kmanddistancecome frommin/maxover the same window.0.Verification
mix testagainstpostgres:18-trixieonelixir:1.20.2-otp-29. The new test fails on current main and passes with the change; the rest of the suite is unaffected.Not covered
A drive where no position carries an odometer still ends up with NULLs.
distanceis nil there, and thedistance >= 0.01guard accepts nil because atoms sort above numbers in Elixir, so the drive is kept rather than deleted. That behaviour is unchanged by this PR and seemed like a separate call — happy to fold it in if you would rather have those drives dropped.