-
Notifications
You must be signed in to change notification settings - Fork 307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Offline routing #1768
Offline routing #1768
Changes from 17 commits
b9785d4
af69d03
1c4151b
233d335
a282566
80f9548
300e775
a8942c7
cae9ef3
43a3afb
5352c5c
584228c
b911dfb
17f2763
ae4232d
cebe19a
a3a0d33
8c2a4a9
3682e9f
d61dc25
6543704
1e67a08
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
[submodule "thirdparty/osrm-text-instructions"] | ||
path = thirdparty/osrm-text-instructions | ||
url = https://github.com/Project-OSRM/osrm-text-instructions.git |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,119 @@ | ||
import Foundation | ||
import MapboxDirections | ||
import MapboxNavigationNative | ||
|
||
public typealias OfflineDirectionsCompletionHandler = (_ numberOfTiles: UInt64) -> Void | ||
|
||
public enum OfflineRoutingError: Error { | ||
case unexpectedRouteResult | ||
case corruptRouteData | ||
case responseError(String) | ||
} | ||
|
||
struct OfflineDirectionsConstants { | ||
static let offlineSerialQueueLabel = Bundle.mapboxCoreNavigation.bundleIdentifier!.appending(".offline") | ||
static let serialQueue = DispatchQueue(label: OfflineDirectionsConstants.offlineSerialQueueLabel) | ||
} | ||
|
||
/** | ||
Defines additional functionality similar to `Directions` with support for offline routing. | ||
*/ | ||
@objc(MBOfflineDirectionsProtocol) | ||
This comment was marked as resolved.
Sorry, something went wrong. |
||
public protocol OfflineRoutingProtocol { | ||
|
||
/** | ||
Initializes a newly created directions object with an optional access token and host. | ||
|
||
- parameter tilesPath: The location where the tiles has been sideloaded to. | ||
- parameter translationsPath: The location where the translations has been sideloaded to. | ||
- parameter accessToken: A Mapbox [access token](https://www.mapbox.com/help/define-access-token/). If an access token is not specified when initializing the directions object, it should be specified in the `MGLMapboxAccessToken` key in the main application bundle’s Info.plist. | ||
- parameter host: An optional hostname to the server API. The [Mapbox Directions API](https://www.mapbox.com/api-documentation/?language=Swift#directions) endpoint is used by default. | ||
*/ | ||
init(tilesPath: URL, translationsPath: URL, accessToken: String?, host: String?, completionHandler: @escaping OfflineDirectionsCompletionHandler) | ||
|
||
/** | ||
Begins asynchronously calculating the route or routes using the given options and delivers the results to a closure. | ||
|
||
This method retrieves the routes asynchronously via MapboxNavigationNative. | ||
|
||
Routes may be displayed atop a [Mapbox map](https://www.mapbox.com/maps/). They may be cached but may not be stored permanently. To use the results in other contexts or store them permanently, [upgrade to a Mapbox enterprise plan](https://www.mapbox.com/directions/#pricing). | ||
|
||
- parameter options: A `RouteOptions` object specifying the requirements for the resulting routes. | ||
- parameter completionHandler: The closure (block) to call with the resulting routes. This closure is executed on the application’s main thread. | ||
*/ | ||
func calculateOffline(_ options: RouteOptions, completionHandler: @escaping Directions.RouteCompletionHandler) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method name doesn’t quite sound grammatical, since RouteOptions is not an offline. Would it be possible to combine this functionality with the existing method provided by MapboxDirections.swift using a method signature like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Combining sounds good but I think we can be sparse with the prepositions since it doesn't read fluently anyway. |
||
} | ||
|
||
@objc(MBMapboxOfflineDirections) | ||
public class MapboxOfflineDirections: Directions, OfflineRoutingProtocol { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure about the name of this class. How about There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That’s as good a name as I can think of. Hopefully developers don’t end up thinking they can only use NavigationRouteOptions with this class. |
||
|
||
public required init(tilesPath: URL, translationsPath: URL, accessToken: String?, host: String? = nil, completionHandler: @escaping OfflineDirectionsCompletionHandler) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rename |
||
|
||
super.init(accessToken: accessToken, host: host) | ||
|
||
OfflineDirectionsConstants.serialQueue.sync { | ||
let tilesPath = tilesPath.absoluteString.replacingOccurrences(of: "file://", with: "") | ||
let translationsPath = translationsPath.absoluteString.replacingOccurrences(of: "file://", with: "") | ||
let tileCount = self.navigator.configureRouter(forTilesPath: tilesPath, translationsPath: translationsPath) | ||
|
||
DispatchQueue.main.async { | ||
completionHandler(tileCount) | ||
} | ||
} | ||
} | ||
|
||
public func calculateOffline(_ options: RouteOptions, completionHandler: @escaping Directions.RouteCompletionHandler) { | ||
|
||
let url = self.url(forCalculating: options) | ||
|
||
OfflineDirectionsConstants.serialQueue.sync { [weak self] in | ||
|
||
guard let result = self?.navigator.getRouteForDirectionsUri(url.absoluteString) else { | ||
return completionHandler(nil, nil, OfflineRoutingError.unexpectedRouteResult as NSError) | ||
} | ||
|
||
guard let data = result.json.data(using: .utf8) else { | ||
return completionHandler(nil, nil, OfflineRoutingError.corruptRouteData as NSError) | ||
} | ||
|
||
do { | ||
let json = try JSONSerialization.jsonObject(with: data, options: []) as! [String: Any] | ||
if let errorValue = json["error"] as? String { | ||
DispatchQueue.main.async { | ||
let error = NSError(domain: "..", code: 102, userInfo: [NSLocalizedDescriptionKey: errorValue]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an unusual domain. Can it be replaced with a constant that reliably identifies this SDK? |
||
return completionHandler(nil, nil, error) | ||
} | ||
} else { | ||
let response = options.response(from: json) | ||
|
||
DispatchQueue.main.async { | ||
return completionHandler(response.0, response.1, nil) | ||
} | ||
} | ||
|
||
} catch { | ||
DispatchQueue.main.async { | ||
return completionHandler(nil, nil, error as NSError) | ||
} | ||
} | ||
} | ||
} | ||
|
||
var _navigator: MBNavigator! | ||
var navigator: MBNavigator { | ||
|
||
assert(currentQueueName() == OfflineDirectionsConstants.offlineSerialQueueLabel, | ||
"The offline navigator must be accessed from the dedicated serial queue") | ||
|
||
if _navigator == nil { | ||
self._navigator = MBNavigator() | ||
} | ||
|
||
return _navigator | ||
} | ||
} | ||
|
||
fileprivate func currentQueueName() -> String? { | ||
let name = __dispatch_queue_get_label(nil) | ||
return String(cString: name, encoding: .utf8) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
import XCTest | ||
import MapboxDirections | ||
@testable import MapboxCoreNavigation | ||
|
||
class OfflineRoutingTests: XCTestCase { | ||
|
||
func testOfflineDirections() { | ||
let bundle = Bundle(for: OfflineRoutingTests.self) | ||
let tilesPath = URL(fileURLWithPath: bundle.bundlePath.appending("/routing/liechtenstein")) | ||
let translationsPath = URL(fileURLWithPath: bundle.bundlePath.appending("/translations")) | ||
|
||
let setupExpectation = expectation(description: "Set up offline routing") | ||
|
||
let directions = MapboxOfflineDirections(tilesPath: tilesPath, translationsPath: translationsPath, accessToken: "foo") { (numberOfTiles) in | ||
XCTAssertEqual(numberOfTiles, 5) | ||
setupExpectation.fulfill() | ||
} | ||
|
||
wait(for: [setupExpectation], timeout: 2) | ||
|
||
// Coordinates within Liechtenstein | ||
let coordinates = [CLLocationCoordinate2D(latitude: 47.1192, longitude: 9.5412), | ||
CLLocationCoordinate2D(latitude: 47.1153, longitude: 9.5531)] | ||
|
||
let options = NavigationRouteOptions(coordinates: coordinates, profileIdentifier: .automobile) | ||
let calculateRouteExpectation = expectation(description: "Calculate route offline") | ||
var route: Route? | ||
|
||
directions.calculateOffline(options) { (waypoints, routes, error) in | ||
XCTAssertNil(error) | ||
XCTAssertNotNil(waypoints) | ||
XCTAssertNotNil(routes) | ||
route = routes!.first! | ||
calculateRouteExpectation.fulfill() | ||
} | ||
|
||
wait(for: [calculateRouteExpectation], timeout: 2) | ||
|
||
XCTAssertNotNil(route) | ||
XCTAssertEqual(route!.coordinates!.count, 239) | ||
} | ||
|
||
func testOfflineDirectionsError() { | ||
let bundle = Bundle(for: OfflineRoutingTests.self) | ||
let tilesPath = URL(fileURLWithPath: bundle.bundlePath).appendingPathComponent("/routing/liechtenstein") | ||
let translationsPath = URL(fileURLWithPath: bundle.bundlePath).appendingPathComponent("/translations") | ||
|
||
let setupExpectation = expectation(description: "Set up offline routing") | ||
|
||
let directions = MapboxOfflineDirections(tilesPath: tilesPath, translationsPath: translationsPath, accessToken: "foo") { (numberOfTiles) in | ||
XCTAssertEqual(numberOfTiles, 5) | ||
setupExpectation.fulfill() | ||
} | ||
|
||
wait(for: [setupExpectation], timeout: 2) | ||
|
||
// Coordinates in SF | ||
let coordinates = [CLLocationCoordinate2D(latitude: 37.7870, longitude: -122.4261), | ||
CLLocationCoordinate2D(latitude: 37.7805, longitude: -122.4073)] | ||
|
||
let options = NavigationRouteOptions(coordinates: coordinates, profileIdentifier: .automobile) | ||
let calculateRouteExpectation = expectation(description: "Calculate route offline") | ||
|
||
directions.calculateOffline(options) { (waypoints, routes, error) in | ||
XCTAssertNotNil(error) | ||
XCTAssertNil(routes) | ||
XCTAssertNil(waypoints) | ||
calculateRouteExpectation.fulfill() | ||
} | ||
|
||
wait(for: [calculateRouteExpectation], timeout: 2) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this enumeration bridge to Objective-C? If so, it needs a class prefix in Objective-C (and documentation).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These errors are casted to NSErrors but I guess it needs a localized description key? Or does Error bridge fine to Obj-C since Swift 4.0? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, yeah, the localized description keys would need to be provided somehow, though I’m not sure exactly how that works when relying on bridging from Error in Swift.