Skip to content

Conversation

danesfeder
Copy link
Contributor

@danesfeder danesfeder commented Mar 20, 2018

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:
ezgif com-video-to-gif

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) 🤔

@danesfeder danesfeder added bug Defect to be fixed. navigation-core labels Mar 20, 2018
@danesfeder danesfeder self-assigned this Mar 20, 2018
Copy link
Contributor

@Guardiola31337 Guardiola31337 left a 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;
Copy link
Contributor

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?

@danesfeder danesfeder force-pushed the dan-camera-resume branch 2 times, most recently from 3fa053f to fe59295 Compare March 22, 2018 13:46
Copy link
Contributor

@Guardiola31337 Guardiola31337 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After CI finishes 🚀

@danesfeder danesfeder merged commit 2975aea into master Mar 23, 2018
@danesfeder danesfeder deleted the dan-camera-resume branch March 23, 2018 21:20
@danesfeder danesfeder mentioned this pull request Apr 3, 2018
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Defect to be fixed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants