Skip to content

Comments

Suppress reroute announcement on long steps#382

Merged
bsudekum merged 4 commits intomasterfrom
supress-reroute-announcement
Jul 14, 2017
Merged

Suppress reroute announcement on long steps#382
bsudekum merged 4 commits intomasterfrom
supress-reroute-announcement

Conversation

@bsudekum
Copy link
Contributor

Closes: #374

In cases where we reroute the user and the new route has a first step which has very long (1km), there is no need to alert the user about the first step. This is because they have ample time until the upcoming maneuver.

/cc @1ec5 @frederoni @ericrwolfe

@bsudekum bsudekum requested review from 1ec5 and frederoni July 13, 2017 00:16
@1ec5 1ec5 mentioned this pull request Jul 13, 2017
// If the new route has a first step that has a distance greater than 1km,
// surpress the first announcement since they have ample time until the next maneuver
var newAlertLevel: AlertLevel = .none
if let firstLeg = route.legs.first, let firstStep = firstLeg.steps.first, firstStep.distance > 1_000 {
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment above says this heuristic is based on duration, whereas this statement checks only for distance. If the user is traveling along a freeway in the U.S. and the new upcoming maneuver is 1 kilometer ahead, then the user has already passed the standard 1-mile advance guide sign and the next guide sign they’ll see is directly above the exit. Either we’d need to switch to a heuristic based on remaining duration, or we’d need separate logic for freeways versus surface streets.

@1ec5
Copy link
Contributor

1ec5 commented Jul 13, 2017

What if we suppress the announcement when the new upcoming maneuver is farther away or later than the current upcoming maneuver, but preserve the announcement if the new upcoming maneuver is closer or earlier?

@bsudekum
Copy link
Contributor Author

What if we suppress the announcement when the new upcoming maneuver is farther away or later than the current upcoming maneuver, but preserve the announcement if the new upcoming maneuver is closer or earlier?

Sounds good to me.

@ericrwolfe
Copy link
Contributor

What if we suppress the announcement when the new upcoming maneuver is farther away or later than the current upcoming maneuver

If I've missed the maneuver and triggered a reroute, won't the current upcoming maneuver's distance remaining be zero?

@1ec5
Copy link
Contributor

1ec5 commented Jul 13, 2017

What if we suppress the announcement when the new upcoming maneuver is farther away or later than the current upcoming maneuver

If I've missed the maneuver and triggered a reroute, won't the current upcoming maneuver's distance remaining be zero?

#374 calls for silencing the announcement in that case, correct?

- parameter legIndex: Zero-based index indicating the current leg the user is on.
*/
public init(route: Route, legIndex: Int = 0) {
public init(route: Route, legIndex: Int = 0, alertLevel: AlertLevel = .none) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This parameter needs documentation (same for the one on RouteLegProgress).

@bsudekum bsudekum merged commit b45ca0e into master Jul 14, 2017
@bsudekum bsudekum deleted the supress-reroute-announcement branch July 14, 2017 19:05
mappy-mobile pushed a commit to Mappy/mapbox-navigation-ios that referenced this pull request Sep 17, 2020
Remove Objective-C compatibility; adopt Swift language features
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants