From 3ffaa51d428ebf25e744fdbef8bbeeb02e959d01 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Fri, 31 May 2024 09:52:39 -0700 Subject: [PATCH] Fix NavigationViewController rerouting (#47) * Fix rerouting in NavigationViewController The `reason` parameter was added in c65c9c64c7acd371e73fb7914f95657832aa42fa, 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 --- CHANGELOG.md | 2 ++ .../RouteControllerDelegate.swift | 4 +-- .../SimulatedLocationManager.swift | 35 +++++++++++++++++-- .../RouteControllerTests.swift | 18 +++++++++- .../NavigationViewController.swift | 2 +- 5 files changed, 55 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 67d87417..5239a016 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 . ## v2.0.0 (May 23, 2023) - Upgrade minimum iOS version from 11.0 to 12.0. diff --git a/MapboxCoreNavigation/RouteControllerDelegate.swift b/MapboxCoreNavigation/RouteControllerDelegate.swift index 8bb2f865..6f0685f9 100644 --- a/MapboxCoreNavigation/RouteControllerDelegate.swift +++ b/MapboxCoreNavigation/RouteControllerDelegate.swift @@ -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. */ diff --git a/MapboxCoreNavigation/SimulatedLocationManager.swift b/MapboxCoreNavigation/SimulatedLocationManager.swift index 1b6ef42b..c66a7bfa 100644 --- a/MapboxCoreNavigation/SimulatedLocationManager.swift +++ b/MapboxCoreNavigation/SimulatedLocationManager.swift @@ -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. @@ -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 } @@ -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 diff --git a/MapboxCoreNavigationTests/RouteControllerTests.swift b/MapboxCoreNavigationTests/RouteControllerTests.swift index 8e91a0ee..41c5817b 100644 --- a/MapboxCoreNavigationTests/RouteControllerTests.swift +++ b/MapboxCoreNavigationTests/RouteControllerTests.swift @@ -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() { diff --git a/MapboxNavigation/NavigationViewController.swift b/MapboxNavigation/NavigationViewController.swift index 805dd0c3..5e163ad8 100644 --- a/MapboxNavigation/NavigationViewController.swift +++ b/MapboxNavigation/NavigationViewController.swift @@ -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) }