Add location manager for free drive #2410
Conversation
IodaMikeMapbox
force-pushed
the
ima-free-drive
branch
2 times, most recently
from
7600900
to
a7fc221
Compare
This comment has been minimized.
This comment has been minimized.
IodaMikeMapbox
force-pushed
the
ima-free-drive
branch
from
dfa359b
to
805c9b5
Compare
|
The pr uses @dersim-davaod's commit from temp-route-tiles-logic branch, as I see the work from that branch was utilised in #2533 |
|
@IodaMikeMapbox thanks for notifying. it will be slightly changed in #2533, but that's fine. If the current changes will be merged first, that's also fine, I'll handle a proper rebase. |
Decoupled tile directory creation and navigator configuration from location manager initialization. Client code needs to explicitly start location updates after creating the location manager, just like with an ordinary CLLocationManager. Starting location updates causes a completion handler to be called. The completion handler accepts any error that occurs during tile directory creation, so that the application can catch and handle the error instead of crashing.
Inverted FreeDriveLocationManager to be a delegate of a CLLocationManager that is responsible for talking to the navigator in much the same way that RouteController does. Removed FreeDriveDebugInfoListener and FreeDriveLocationManager.ProxyDelegate in favor of FreeDriveLocationManagerDelegate. Moved debugging utilities from MapboxNavigation to the example application targets and out of the project root. Removed remaining vestiges of MapboxNavigationNative from the MapboxNavigation public API.
…ive debugger Send a notification alongside the delegate method whenever the free drive location manager updates. Draw the raw and idealized user location paths using mutable polyline annotations instead of runtime styling. Polyline annotations are easier and more performant to update synchronously. Deleted the table view for matches.
Update the title bar to display the current road name.
There’s no good way to mock a route tile request at the moment, so disable the test instead of requiring network access.
The distinction between the MapboxCoreNavigation and MapboxNavigation free driving classes is easier to make if we call one a location data source, borrowing from RouteDataSource terminology, and the other a location manager, for consistency with MGLLocationManager. This naming convention also avoids the awkwardness of beginning class names with a noun phrase that could easily be misread as a verb phrase. Added a changelog entry.
|
|
||
| trackLocations(mapView: mapView) | ||
| mapView.showsUserLocation = true | ||
| mapView.userTrackingMode = .followWithHeading |
There was a problem hiding this comment.
The map view isn’t showing the heading indicator for some reason. I think it has to do with the location manager only delivering location updates and not heading updates. In any case, I don’t think we want the map view to rotate with the user’s heading or course, because frequent rotation makes the map much less useful for choosing a destination.
| // Assumption: MapboxNavigation.framework includes NavigationViewController and exposes it to the Objective-C runtime as MapboxNavigation.NavigationViewController. | ||
| guard let NavigationViewController = NSClassFromString("MapboxNavigation.NavigationViewController") else { |
There was a problem hiding this comment.
We’ve been making this assumption since #2221. This change just moves it somewhere more accessible to non-events-related code.
|
|
||
| The user agent string for any HTTP requests performed directly within MapboxCoreNavigation or MapboxNavigation. | ||
| */ | ||
| public static let userAgent: String = { |
There was a problem hiding this comment.
Is there any specific reason we use custom User-Agent? (NSURLSessionsets User-Agent by default). I can also see that Directions.swift uses pretty similar approach when creating userAgent so might make sense to have some common way of creating it.
There was a problem hiding this comment.
In general, we should always customize the user agent string to provide more context in the server logs than the application and Core Foundation versions, which is what URLSession provides by default. This particular user agent string is specific to requests made by MapboxCoreNavigation or MapboxNavigation.
| locationManager.updateLocation(CLLocation(latitude: 47.208674, longitude: 9.524650)) | ||
| } | ||
|
|
||
| DispatchQueue.main.asyncAfter(deadline: .now() + .seconds(25)) { |
There was a problem hiding this comment.
Probably makes sense to review this test in future since it's time consuming.
There was a problem hiding this comment.
Yep, in fact, I disabled this test because it requires network access and a valid Mapbox access token to function at all. We’ll figure out a better integration test as tail work. I just rewrote history in this PR to undo the change that deleted the offline routing tests; I think we could possibly leverage that test’s test fixture to avoid reinventing the wheel.
| self.directions = directions | ||
|
|
||
| let settingsProfile = SettingsProfile(application: ProfileApplication.kMobile, platform: ProfilePlatform.KIOS) | ||
| self.navigator = Navigator(profile: settingsProfile, config: NavigatorConfig() , customConfig: "") |
There was a problem hiding this comment.
I remember that @SiarheiFedartsou was mentioning that it can be beneficial to use one global MBNavigator and feed it with data whenever it's available. Since we now have 4 (?) places where MBNavigator is created it might make sense to consider one global instance approach.
There was a problem hiding this comment.
Right, for better or worse, the navigator is intended to be used as something of a singleton if I remember correctly.
#2509 will remove two of these initializations, leaving just the ones in RouteController and PassiveLocationDataSource, so I didn’t bother to centralize the logic just yet.
The long-term vision is indeed to unify PassiveLocationDataSource and RouteController, so that the latter is just a special case of setting a route on the former and keeping track of route progress. For now, we would probably recommend that the developer stop location updates on PassiveLocationDataSource when beginning turn-by-turn navigation with RouteController. This happens to be the case in the example application, because it demonstrates temporarily taking the main map view out of the view hierarchy as a performance optimization.
Reuse the better-tested, more strongly typed implementation of route tiles API integration in MapboxDirections. Removed the user default and property for the current version, which appears to be unused.
|
Based on the feedback in #2410 (review), I removed the RouteTilesVersion implementation outright in favor of the preexisting routing tiles API support in MapboxDirections, which is better tested and slightly more strongly typed. |
The pr adds implementation of
NavigationLocationManagerwhich processes raw gps values with theNavigatorin order to map them to the road graph.Fixes #1627, fixes #2456.
/cc @mapbox/navigation-ios