Skip to content

Commit

Permalink
Correcting setRoutes reason (#4513) (#4514)
Browse files Browse the repository at this point in the history
* vk/NAVIOS-1111-reasons: corrected switching to alternative reason parameter propagation
  • Loading branch information
Udumft authored Jul 27, 2023
1 parent e3b2be2 commit df4b4fe
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 11 deletions.
12 changes: 10 additions & 2 deletions Sources/MapboxCoreNavigation/LegacyRouteController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -148,12 +148,17 @@ open class LegacyRouteController: NSObject, Router, InternalRouter, CLLocationMa
routeOptions: RouteOptions?,
completion: ((Bool) -> Void)?) {
guard !hasFinishedRouting else { return }
updateRoute(with: indexedRouteResponse, routeOptions: routeOptions, isProactive: false, completion: completion)
updateRoute(with: indexedRouteResponse,
routeOptions: routeOptions,
isProactive: false,
isAlternative: false,
completion: completion)
}

func updateRoute(with indexedRouteResponse: IndexedRouteResponse,
routeOptions: RouteOptions?,
isProactive: Bool,
isAlternative: Bool,
completion: ((Bool) -> Void)?) {
guard let route = indexedRouteResponse.currentRoute else {
preconditionFailure("`indexedRouteResponse` does not contain route for index `\(indexedRouteResponse.routeIndex)` when updating route.")
Expand Down Expand Up @@ -502,7 +507,10 @@ open class LegacyRouteController: NSObject, Router, InternalRouter, CLLocationMa
case let .success(indexedResponse):
let response = indexedResponse.routeResponse
guard case let .route(options) = response.options else { return }
self.updateRoute(with: indexedResponse, routeOptions: options, isProactive: false) { success in
self.updateRoute(with: indexedResponse,
routeOptions: options,
isProactive: false,
isAlternative: false) { success in
self.isRerouting = false
}
}
Expand Down
22 changes: 17 additions & 5 deletions Sources/MapboxCoreNavigation/RouteController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -872,7 +872,8 @@ extension RouteController: Router {
}
self.updateRoute(with: indexedResponse,
routeOptions: routeOptions,
isProactive: false) { [weak self] success in
isProactive: false,
isAlternative: false) { [weak self] success in
self?.isRerouting = false
}
case let .failure(error):
Expand All @@ -886,12 +887,17 @@ extension RouteController: Router {
routeOptions: RouteOptions?,
completion: ((Bool) -> Void)?) {
guard !hasFinishedRouting else { return }
updateRoute(with: indexedRouteResponse, routeOptions: routeOptions, isProactive: false, completion: completion)
updateRoute(with: indexedRouteResponse,
routeOptions: routeOptions,
isProactive: false,
isAlternative: false,
completion: completion)
}

func updateRoute(with indexedRouteResponse: IndexedRouteResponse,
routeOptions: RouteOptions?,
isProactive: Bool,
isAlternative: Bool,
completion: ((Bool) -> Void)?) {
guard let route = indexedRouteResponse.currentRoute else {
preconditionFailure("`indexedRouteResponse` does not contain route for index `\(indexedRouteResponse.routeIndex)` when updating route.")
Expand All @@ -905,7 +911,8 @@ extension RouteController: Router {
let routeOptions = routeOptions ?? routeProgress.routeOptions
let routeProgress = RouteProgress(route: route, options: routeOptions)
let reason = routeChangeReason(shouldStartNewBillingSession: shouldStartNewBillingSession,
isProactiveRerouting: isProactive)
isProactiveRerouting: isProactive,
isSwitchingToAlternative: isAlternative)
updateNavigator(with: indexedRouteResponse,
fromLegIndex: routeProgress.legIndex,
reason: reason) { [weak self] result in
Expand All @@ -927,10 +934,13 @@ extension RouteController: Router {
}

private func routeChangeReason(shouldStartNewBillingSession: Bool,
isProactiveRerouting: Bool) -> RouteChangeReason {
isProactiveRerouting: Bool,
isSwitchingToAlternative: Bool) -> RouteChangeReason {
let reason: RouteChangeReason
if isProactiveRerouting {
reason = .fastestRouteAvailable
} else if isSwitchingToAlternative {
reason = .switchToAlternative
} else if shouldStartNewBillingSession {
reason = .startNewRoute
} else {
Expand Down Expand Up @@ -1003,6 +1013,7 @@ extension RouteController: ReroutingControllerDelegate {
updateRoute(with: newRouteResponse,
routeOptions: options,
isProactive: false,
isAlternative: true,
completion: { [weak self] success in
guard let self = self else { return }
var userInfo = [RouteController.NotificationUserInfoKey: Any]()
Expand Down Expand Up @@ -1040,7 +1051,8 @@ extension RouteController: ReroutingControllerDelegate {
responseOrigin: routeOrigin)
updateRoute(with: indexedRouteResponse,
routeOptions: options,
isProactive: false) { [weak self] _ in
isProactive: false,
isAlternative: false) { [weak self] _ in
self?.isRerouting = false
}
}
Expand Down
4 changes: 3 additions & 1 deletion Sources/MapboxCoreNavigation/Router.swift
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ protocol InternalRouter: AnyObject {
func updateRoute(with indexedRouteResponse: IndexedRouteResponse,
routeOptions: RouteOptions?,
isProactive: Bool,
isAlternative: Bool,
completion: ((Bool) -> Void)?)
}

Expand Down Expand Up @@ -457,7 +458,8 @@ extension InternalRouter where Self: Router {
// Prefer the most optimal route (the first one) over the route that matched the original choice.
self.updateRoute(with: indexedResponse,
routeOptions: routeOptions ?? self.routeProgress.routeOptions,
isProactive: true) { [weak self] success in
isProactive: true,
isAlternative: false) { [weak self] success in
self?.isRerouting = false
}
}
Expand Down
18 changes: 15 additions & 3 deletions Tests/MapboxCoreNavigationTests/RouteControllerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,11 @@ class RouteControllerTests: TestCase {
return
}
navigationSessionManagerSpy.reset()
routeController.updateRoute(with: response, routeOptions: routeOptions, isProactive: false, completion: nil)
routeController.updateRoute(with: response,
routeOptions: routeOptions,
isProactive: false,
isAlternative: false,
completion: nil)

XCTAssertTrue(navigatorSpy.setRoutesCalled)
XCTAssertEqual(navigatorSpy.passedUuid, routeController.sessionUUID)
Expand All @@ -725,7 +729,11 @@ class RouteControllerTests: TestCase {
func testUpdateRouteIfShouldNotStartNewBillingSession() {
let response = IndexedRouteResponse(routeResponse: singleRouteResponse, routeIndex: 0)
routingProvider.returnedRoutesResult = .success(response)
routeController.updateRoute(with: response, routeOptions: options, isProactive: false, completion: nil)
routeController.updateRoute(with: response,
routeOptions: options,
isProactive: false,
isAlternative: false,
completion: nil)

XCTAssertTrue(navigatorSpy.setRoutesCalled)
XCTAssertEqual(navigatorSpy.passedUuid, routeController.sessionUUID)
Expand All @@ -743,7 +751,11 @@ class RouteControllerTests: TestCase {
func testUpdateRouteIfFasterRoute() {
let response = IndexedRouteResponse(routeResponse: singleRouteResponse, routeIndex: 0)
routingProvider.returnedRoutesResult = .success(response)
routeController.updateRoute(with: response, routeOptions: options, isProactive: true, completion: nil)
routeController.updateRoute(with: response,
routeOptions: options,
isProactive: true,
isAlternative: false,
completion: nil)

XCTAssertTrue(navigatorSpy.setRoutesCalled)
XCTAssertEqual(navigatorSpy.passedUuid, routeController.sessionUUID)
Expand Down

0 comments on commit df4b4fe

Please sign in to comment.