mapbox-navigation-ios icon indicating copy to clipboard operation
mapbox-navigation-ios copied to clipboard

NavigationMatchOptions should not clear out coordinate accuracies

Open 1ec5 opened this issue 7 years ago • 4 comments

NavigationMatchOptions(waypoints:profileIdentifier:) clears out the coordinateAccuracy of any waypoint passed to it:

https://github.com/mapbox/mapbox-navigation-ios/blob/cddb733d99673a48ba442751865c20c6f5a1db4d/MapboxCoreNavigation/NavigationRouteOptions.swift#L78

I think the reason is that we expected these coordinates to come from a Core Location update’s CLLocation.coordinateAccuracy, which can be inaccurate or at least more conservative than what we’d generally need in a Map Matching API request. However, this is a public initializer, and it’s entirely possible for a developer to set Waypoint.coordinateAccuracy explicitly, only to see it silently unset, as in https://github.com/mapbox/MapboxDirections.swift/issues/321#issuecomment-443629473.

NavigationMatchOptions(waypoints:profileIdentifier:) should leave coordinateAccuracy alone. It should be up to the developer to unset coordinateAccuracy when calling NavigationMatchOptions(locations:profileIdentifier:) with a CLLocation from a Core Location update.

/cc @mapbox/navigation-ios

1ec5 avatar Dec 18 '18 21:12 1ec5

Actually, this might’ve just been copy-pasted from NavigationRouteOptions:

https://github.com/mapbox/mapbox-navigation-ios/blob/2a5812fabd84c14e3f858bff9de63f3bc8451acb/MapboxCoreNavigation/NavigationRouteOptions.swift#L20

There are enough callers of NavigationRouteOptions.init(waypoints:profileIdentifier:) in the example application alone that we’ll need to be careful moving this unsetting elsewhere.

1ec5 avatar Dec 20 '18 00:12 1ec5

As a workaround, you can force all the coordinates’ accuracies to be something other than −1:

let options = NavigationMatchOptions(waypoints: waypoints)
options.waypoints = waypoints

1ec5 avatar Dec 20 '18 00:12 1ec5

@1ec5 I have tried your workaround previously but it didn't work. Now, I understand why. This is because the NavigationMatchOptions constructor modifies each waypoints by setting their coordinateAccuracy to -1.

So when you set options.waypoints = waypoints, it does nothing better, because the waypoints' coordinateAccuracy is already set to -1.

The workaround is:

optionsMatch.waypoints = waypoints.map {
    $0.coordinateAccuracy = 49
    return $0
}

It works, now I get a route!

tezqa avatar Feb 05 '19 14:02 tezqa

@1ec5 I have tried your workaround previously but it didn't work. Now, I understand why. This is because the NavigationMatchOptions constructor modifies each waypoints by setting their coordinateAccuracy to -1.

So when you set options.waypoints = waypoints, it does nothing better, because the waypoints' coordinateAccuracy is already set to -1.

The workaround is:

optionsMatch.waypoints = waypoints.map {
    $0.coordinateAccuracy = 49
    return $0
}

It works, now I get a route!

where did you manage to implement this piece?

baukehoeree avatar Sep 24 '19 12:09 baukehoeree