2284: refined route refresh checks logic

mapbox · Apr 29, 2020 · 224741e · 224741e
1 parent 51c6a1a
commit 224741e
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 19 deletions.
diff --git a/MapboxCoreNavigation/CoreConstants.swift b/MapboxCoreNavigation/CoreConstants.swift
@@ -74,7 +74,7 @@ public var RouteControllerNumberOfSecondsForRerouteFeedback: TimeInterval = 10
 public var RouteControllerMinimumDurationRemainingForProactiveRerouting: TimeInterval = 600
 
 /**
- The number of seconds between attempts to automatically calculate a more optimal route while traveling. During suc attempt, if Route Refresh is enabled, route will also be refreshed to verify correct ETA and congestion.
+ The number of seconds between attempts to automatically calculate a more optimal route while traveling. During such attempt, if Route Refresh is enabled, route will also be refreshed to verify correct ETA and congestion.
  */
 public var RouteControllerProactiveReroutingInterval: TimeInterval = 120
 

diff --git a/MapboxCoreNavigation/LegacyRouteController.swift b/MapboxCoreNavigation/LegacyRouteController.swift
@@ -35,6 +35,8 @@ open class LegacyRouteController: NSObject, Router, InternalRouter, CLLocationMa
     var didFindFasterRoute = false
 
     var lastProactiveRerouteDate: Date?
+
+    var lastRouteRefresh: Date?
 
     public var routeProgress: RouteProgress {
         get {
@@ -85,7 +87,7 @@ open class LegacyRouteController: NSObject, Router, InternalRouter, CLLocationMa
         self.directions = directions
         self._routeProgress = RouteProgress(route: route, options: options)
         self.dataSource = source
-        refreshesRoute = options.profileIdentifier == .automobileAvoidingTraffic && options.refreshingEnabled // should we?
+        self.refreshesRoute = options.profileIdentifier == .automobileAvoidingTraffic && options.refreshingEnabled
         UIDevice.current.isBatteryMonitoringEnabled = true
 
         super.init()

diff --git a/MapboxCoreNavigation/RouteController.swift b/MapboxCoreNavigation/RouteController.swift
@@ -117,6 +117,8 @@ open class RouteController: NSObject {
 
     var lastProactiveRerouteDate: Date?
 
+    var lastRouteRefresh: Date?
+
     public var refreshesRoute: Bool = true
 
     /**
@@ -146,7 +148,7 @@ open class RouteController: NSObject {
         self.directions = directions
         self._routeProgress = RouteProgress(route: route, options: options)
         self.dataSource = source
-        refreshesRoute = options.profileIdentifier == .automobileAvoidingTraffic && options.refreshingEnabled // should we?
+        self.refreshesRoute = options.profileIdentifier == .automobileAvoidingTraffic && options.refreshingEnabled
         UIDevice.current.isBatteryMonitoringEnabled = true
 
         super.init()

diff --git a/MapboxCoreNavigation/RouteProgress.swift b/MapboxCoreNavigation/RouteProgress.swift
@@ -280,9 +280,7 @@ open class RouteProgress {
         }
     }
 
-    public func refreshRoute(with refreshedRoute: Route) { // seems dangerous.
-        // TODO: check multi-leg routes
-
+    public func refreshRoute(with refreshedRoute: Route) {
         route = refreshedRoute
         currentLegProgress = RouteLegProgress(leg: route.legs[legIndex],
                                               stepIndex: currentLegProgress.stepIndex,

diff --git a/MapboxCoreNavigation/Router.swift b/MapboxCoreNavigation/Router.swift
@@ -64,14 +64,14 @@ public protocol Router: class, CLLocationManagerDelegate {
     var rawLocation: CLLocation? { get }
 
     /**
-     If true, the `RouteController` attempts to calculate a more optimal route for the user on an interval defined by `RouteControllerProactiveReroutingInterval`.
+     If true, the `RouteController` attempts to calculate a more optimal route for the user on an interval defined by `RouteControllerProactiveReroutingInterval`. If `refreshesRoute` is enabled too, reroute attempt will be fired after route refreshing.
      */
     var reroutesProactively: Bool { get set }
 
     /**
      If true, the `RouteController` attempts to update ETA and route congestion on an interval defined by `RouteControllerProactiveReroutingInterval`.
 
-     Refreshing will be used only if route's mode of transportation profile is set to `.automobileAvoidingTraffic`
+     Refreshing will be used only if route's mode of transportation profile is set to `.automobileAvoidingTraffic`. If `reroutesProactively` is enabled too, rerouting will be checked after route is refreshed.
      */
     var refreshesRoute: Bool { get set }
 
@@ -90,6 +90,8 @@ public protocol Router: class, CLLocationManagerDelegate {
 protocol InternalRouter: class {
     var lastProactiveRerouteDate: Date? { get set }
 
+    var lastRouteRefresh: Date? { get set }
+
     var routeTask: URLSessionDataTask? { get set }
 
     var didFindFasterRoute: Bool { get set }
@@ -111,36 +113,33 @@ extension InternalRouter where Self: Router {
 
     func refreshAndCheckForFasterRoute(from location: CLLocation, routeProgress: RouteProgress) {
         if refreshesRoute {
-            refreshRoute(from: location) {
+            refreshRoute(from: location, legIndex: routeProgress.legIndex) {
                 self.checkForFasterRoute(from: location, routeProgress: routeProgress)
             }
         } else {
             checkForFasterRoute(from: location, routeProgress: routeProgress)
         }
-
     }
 
-    func refreshRoute(from location: CLLocation, /* route progress ? */ completion: @escaping ()->()) {
-        let legIndex = routeProgress.legIndex
-
+    func refreshRoute(from location: CLLocation, legIndex: Int, completion: @escaping ()->()) {
         guard refreshesRoute else {
             completion()
             return
         }
 
-        guard let lastProactiveRerouteDate = lastProactiveRerouteDate else {
-            self.lastProactiveRerouteDate = location.timestamp
+        guard let lastRouteRefresh = lastRouteRefresh else {
+            self.lastRouteRefresh = location.timestamp
             completion()
             return
         }
 
-        guard location.timestamp.timeIntervalSince(lastProactiveRerouteDate) >= 30 /*RouteControllerProactiveReroutingInterval*/ else {
+        guard location.timestamp.timeIntervalSince(lastRouteRefresh) >= RouteControllerProactiveReroutingInterval else {
             completion()
             return
         }
 
         if isRefreshing {
-            completion() // should we??
+            completion()
             return
         }
         isRefreshing = true
@@ -150,6 +149,7 @@ extension InternalRouter where Self: Router {
                            completionHandler: { [weak self] (session, result) in
                             defer {
                                 self?.isRefreshing = false
+                                self?.lastRouteRefresh = nil
                                 completion()
                             }
 
@@ -177,13 +177,13 @@ extension InternalRouter where Self: Router {
             return
         }
 
-        guard let lastProactiveRerouteDate = lastProactiveRerouteDate else {
+        guard let lastRouteValidationDate = lastProactiveRerouteDate else {
             self.lastProactiveRerouteDate = location.timestamp
             return
         }
 
         // Only check every so often for a faster route.
-        guard location.timestamp.timeIntervalSince(lastProactiveRerouteDate) >= RouteControllerProactiveReroutingInterval else {
+        guard location.timestamp.timeIntervalSince(lastRouteValidationDate) >= RouteControllerProactiveReroutingInterval else {
             return
         }