Skip to content

Commit

Permalink
Fix NavigationViewController rerouting (#47)
Browse files Browse the repository at this point in the history
* Fix rerouting in NavigationViewController

The `reason` parameter was added in
c65c9c6, but it was not added to
existing callers.

Since it's an `optional` delegate method, no compiler error was
triggered, the old method just sat around not being run.

* Make it easy to test re-routing

* changelog

* Simplify example with Turf's haversine offset

* test for re-route
  • Loading branch information
michaelkirk authored May 31, 2024
1 parent b4fc365 commit 3ffaa51
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 6 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
- Added `Day/NightStyle(demoStyle: ())` as an explicit alternative when the user doesn't have a tileserver handy. This uses MapLibre's demo style and is intended for testing and demonstration use only.
- Deprecated `DayStyle()`/`NightStyle()` initializers because they were backed by an implicit tile service. If these default styles *are* still used, they'll now use the MapLibre demo style.
- `NavigationViewController` now expects explicit style URLs with `NavigationViewController(route:dayStyleURL:nightStyleURL:...)` or NavigationViewController(route:dayStyle:nightStyle:...)` and the existing initializer, which allowed "default" styles, is deprecated and uses the MapLibre demo styles.
- Fix: NavigationViewController was not re-routing when the user went off route.
- Merged in <https://github.com/maplibre/maplibre-navigation-ios/pull/47>.

## v2.0.0 (May 23, 2023)
- Upgrade minimum iOS version from 11.0 to 12.0.
Expand Down
4 changes: 2 additions & 2 deletions MapboxCoreNavigation/RouteControllerDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ public protocol RouteControllerDelegate: AnyObject {
/**
Called immediately before the route controller calculates a new route.
This method is called after `routeController(_:shouldRerouteFrom:)` is called, simultaneously with the `RouteControllerWillReroute` notification being posted, and before `routeController(_:didRerouteAlong:)` is called.
This method is called after `routeController(_:shouldRerouteFrom:)` is called, simultaneously with the `RouteControllerWillReroute` notification being posted, and before `routeController(_:didRerouteAlong:reason:)` is called.
- parameter routeController: The route controller that will calculate a new route.
- parameter location: The user’s current location.
*/
Expand Down
35 changes: 33 additions & 2 deletions MapboxCoreNavigation/SimulatedLocationManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,29 @@ private class SimulatedLocation: CLLocation {
}
}

/// Provides methods for modifying the locations along the `SimulatedLocationManager`'s route.
@objc public protocol SimulatedLocationManagerDelegate: AnyObject {
/// Offers the delegate an opportunity to modify the next location along the route. This can be useful when testing re-routing.
///
/// - Parameters:
/// - simulatedLocationManager: The location manager simulating locations along a given route.
/// - originalLocation: The next coordinate along the route.
/// - Returns: The next coordinate that the location manager will return as the "current" location. Return the `originalLocation` to follow the route, or modify it to go off route.
///
/// ## Examples
/// ```
/// extension MyDelegate: SimulatedLocationManagerDelegate {
/// func simulatedLocationManager(_ simulatedLocationManager: SimulatedLocationManager, locationFor originalLocation: CLLocation) -> CLLocation {
/// // go off track 100 meters East of the original route
/// let offsetCoordinate = originalLocation.coordinate.coordinate(at: 100, facing: 90)
/// return CLLocation(latitude: offsetCoordinate.latitude, longitude: offsetCoordinate.longitude)
/// }
/// }
/// ```
@objc
func simulatedLocationManager(_ simulatedLocationManager: SimulatedLocationManager, locationFor originalLocation: CLLocation) -> CLLocation
}

/**
The `SimulatedLocationManager` class simulates location updates along a given route.
Expand All @@ -40,7 +63,10 @@ open class SimulatedLocationManager: NavigationLocationManager {
Specify the multiplier to use when calculating speed based on the RouteLeg’s `expectedSegmentTravelTimes`.
*/
@objc public var speedMultiplier: Double = 1


/// Instead of following the given route, go slightly off route. Useful for testing rerouting.
@objc public weak var simulatedLocationManagerDelegate: SimulatedLocationManagerDelegate?

@objc override open var location: CLLocation? {
self.currentLocation
}
Expand Down Expand Up @@ -165,13 +191,18 @@ open class SimulatedLocationManager: NavigationLocationManager {
self.currentSpeed = self.calculateCurrentSpeed(distance: distance, coordinatesNearby: coordinatesNearby, closestLocation: closestLocation)
}

let location = CLLocation(coordinate: newCoordinate,
var location = CLLocation(coordinate: newCoordinate,
altitude: 0,
horizontalAccuracy: horizontalAccuracy,
verticalAccuracy: verticalAccuracy,
course: newCoordinate.direction(to: lookAheadCoordinate).wrap(min: 0, max: 360),
speed: self.currentSpeed,
timestamp: Date())

if let delegate = self.simulatedLocationManagerDelegate {
location = delegate.simulatedLocationManager(self, locationFor: location)
}

self.currentLocation = location
lastKnownLocation = location

Expand Down
18 changes: 17 additions & 1 deletion MapboxCoreNavigationTests/RouteControllerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,23 @@ class RouteControllerTests: XCTestCase {
XCTAssertTrue(self.delegate.recentMessages.contains("routeController(_:shouldPreventReroutesWhenArrivingAt:)"))

// We should not reroute here because the user has arrived.
XCTAssertFalse(self.delegate.recentMessages.contains("routeController(_:didRerouteAlong:)"))
XCTAssertFalse(self.delegate.recentMessages.contains("routeController(_:didRerouteAlong:reason:)"))
}

func testReroutesWhenOffCourse() {
let routeController = self.dependencies.routeController
let firstLocation = self.dependencies.routeLocations.firstLocation

// Start the route
routeController.locationManager(routeController.locationManager, didUpdateLocations: [firstLocation])

// Go very far off route
let locationBeyondRoute = routeController.location!.coordinate.coordinate(at: 2000, facing: 0)
routeController.locationManager(routeController.locationManager, didUpdateLocations: [CLLocation(latitude: locationBeyondRoute.latitude, longitude: locationBeyondRoute.latitude)])

// We should reroute because the user was off course.
self.directionsClientSpy.fireLastCalculateCompletion(with: nil, routes: [self.alternateRoute], error: nil)
XCTAssertTrue(self.delegate.recentMessages.contains("routeController(_:didRerouteAlong:reason:)"))
}

func testRouteControllerDoesNotHaveRetainCycle() {
Expand Down
2 changes: 1 addition & 1 deletion MapboxNavigation/NavigationViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,7 @@ extension NavigationViewController: RouteControllerDelegate {
self.delegate?.navigationViewController?(self, willRerouteFrom: location)
}

@objc public func routeController(_ routeController: RouteController, didRerouteAlong route: Route) {
@objc public func routeController(_ routeController: RouteController, didRerouteAlong route: Route, reason: RouteController.RerouteReason) {
self.mapViewController?.notifyDidReroute(route: route)
self.delegate?.navigationViewController?(self, didRerouteAlong: route)
}
Expand Down

0 comments on commit 3ffaa51

Please sign in to comment.