-
Notifications
You must be signed in to change notification settings - Fork 319
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove last location check from location validation #788
Conversation
665558d
to
76b9dfb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would never be considered valid
Why? Could you clarify?
I'm not even sure if the location apis would ever fire with the getLastLocation twice in a row
Not following here. What do you mean?
return !(location.equals(locationEngine.getLastLocation()) | ||
|| (location.getSpeed() <= 0 && location.hasSpeed()) | ||
|| location.getAccuracy() >= 100); | ||
return location != null && location.hasSpeed() && location.getAccuracy() <= 100; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Magic number 100
. Location#getAccuracy
returns the estimated horizontal accuracy of this location, radial, in meters
what about extracting a constant and give it a name based on what getAccuracy
Javadoc states?
3fa053f
to
fe59295
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After CI finishes 🚀
fe59295
to
ff22ba4
Compare
ff22ba4
to
45dad60
Compare
Found that when starting navigation, the forced location update from
LocationEngine#getLastLocation
would never be considered valid and we would subsequently fall back to to creating the location update from the first point on the route:master
:I removed the check, as this should be an edge case (I'm not even sure if the location apis would ever fire with the
getLastLocation
twice in a row) 🤔