Make timestamps consumed by Navigator.getStatus consistent#2610
Merged
Conversation
chezzdev
added
the
op-ex
1ec5
approved these changes
Sep 8, 2020
Contributor
1ec5
left a comment
1ec5
left a comment
There was a problem hiding this comment.
In addition to the feedback below, can we have ViewController call Router.advanceLegIndex() and restart the navigator instead of reaching into the route progress and modifying the leg index directly, just to demonstrate that this refactor works correctly?
mapbox-navigation-ios/Example/ViewController.swift
Lines 508 to 511 in ebab7d0
1ec5
added
backwards incompatible
1ec5
approved these changes
Sep 8, 2020
Comment on lines
+79
to
+80
| * Removed the `RouteController.projectedLocation(for:)` method in favor of `RouteController.location`. It is no longer possible to predict the user’s location at an arbitrary time. ([#2583](https://github.com/mapbox/mapbox-navigation-ios/issues/2583)) | ||
| * Renamed the `Router.advanceLegIndex(location:)` method to `Router.advanceLegIndex()`. It is no longer possible to advance to an arbitrary leg using this method. ([#2583](https://github.com/mapbox/mapbox-navigation-ios/issues/2583)) |
Contributor
There was a problem hiding this comment.
Nit: Typically we’re linking to PRs where the problem and solution are laid out in detail. Not essential though.
Suggested change
| * Removed the `RouteController.projectedLocation(for:)` method in favor of `RouteController.location`. It is no longer possible to predict the user’s location at an arbitrary time. ([#2583](https://github.com/mapbox/mapbox-navigation-ios/issues/2583)) | |
| * Renamed the `Router.advanceLegIndex(location:)` method to `Router.advanceLegIndex()`. It is no longer possible to advance to an arbitrary leg using this method. ([#2583](https://github.com/mapbox/mapbox-navigation-ios/issues/2583)) | |
| * Removed the `RouteController.projectedLocation(for:)` method in favor of `RouteController.location`. It is no longer possible to predict the user’s location at an arbitrary time. ([#2610](https://github.com/mapbox/mapbox-navigation-ios/pull/2610)) | |
| * Renamed the `Router.advanceLegIndex(location:)` method to `Router.advanceLegIndex()`. It is no longer possible to advance to an arbitrary leg using this method. ([#2610](https://github.com/mapbox/mapbox-navigation-ios/pull/2610)) |
Contributor
Author
There was a problem hiding this comment.
Let's keep it consistent, thanks for the heads-up.
chezzdev
added a commit
that referenced
this pull request
Sep 8, 2020
* Remove projectedLocation method * Save the timestamp of the last location update and use it for snapped location * Adjust advanceLegIndex method to increment leg index without concidering the location * Update changelog * Bump Nimble patch version * Review fixes * Utilize updated API for leg advancing in the example * Fix changelog links in changelog
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.
The PR addresses 3 points described in #2583 to enhance the consistency across timestamps provided to
Navigator.getStatus.Fixes #2583.