General outline #3

bsudekum · 2016-11-16T23:16:38Z

@1ec5 okay this is a general working outline. Some notes on how the API will work:

Developers will subscribe to notifications:
alert level changes
progress changes
user reroutes
As a first iteration, we will not expose a voice controller
Because of this, there will be no translations

todo:

expose distance options
allow developer to specify location manager with options
Tests

1ec5 · 2016-11-17T00:53:27Z

MapboxNavigation/MBNavigation.swift

@@ -0,0 +1,155 @@
+//
+//  MBNavigation.swift


Rename this file to MBNavigationController.swift for consistency. (You can drop MB from the file names if you want. The file names don't actually matter.)

1ec5 · 2016-11-17T00:54:35Z

MapboxNavigation/MBNavigation.swift

+//  MapboxNavigation
+//
+//  Created by Bobby Sudekum on 11/16/16.
+//  Copyright © 2016 Mapbox. All rights reserved.


1ec5 · 2016-11-17T00:55:09Z

MapboxNavigation/MBNavigation.swift

+import CoreLocation
+import MapboxDirections
+
+open class NavigationController: NSObject {


Every public symbol will eventually need a documentation comment.

Every public class or enum needs an @objc(…) attribute that prefixes the class name with MB. Otherwise we'll pollute the class namespace in Objective-C.

1ec5 · 2016-11-17T00:57:20Z

MapboxNavigation/MBNavigation.swift

+
+        locationManager.delegate = self
+        if CLLocationManager.authorizationStatus() == .notDetermined {
+            locationManager.requestWhenInUseAuthorization()


Since the user has to manually press Accept in this case, the location manager calls a delegate method asynchronously when permission is given. Implement that method; otherwise, this library will do nothing in the case where the user hasn't already given permissions by the time NavigationController is initialized.

1ec5 · 2016-11-17T00:59:11Z

MapboxNavigation/MBNavigation.swift

+        }
+
+        guard userIsOnRoute(location) else {
+            NotificationCenter.default.post(name: NavigationControllerNotification.rerouted, object: self, userInfo: [NavigationControllerNotification.rerouted : "rerouted"])


What's the point of this "rerouted" key in the user info dictionary? It doesn't provide any additional information, so remove it.

1ec5 · 2016-11-17T01:07:52Z

MapboxNavigation/Constants.swift

+import Foundation
+import CoreLocation
+
+public let NavigationControllerProgressDidChangeNotificationProgressKey = "progress"


These keys should be of type NavigationControllerNotificationUserInfoKey, which would be a new typedef of String.

1ec5 · 2016-11-17T01:08:35Z

MapboxNavigation/Constants.swift

+public let NavigationControllerMaximumAllowedDegreeOffsetForTurnCompletion: Double = 30
+
+// Alert distances
+public let NavigationControllerMediumAlertNumberOfSeconds: Double = 70


These constants should be of type TimeInterval and "NumberOfSeconds" should become "Interval".

1ec5 · 2016-11-17T01:10:12Z

Example/ViewController.swift

+class ViewController: UIViewController, MGLMapViewDelegate, AVSpeechSynthesizerDelegate {
+
+    var destination: CLLocationCoordinate2D?
+    var directions = Directions(accessToken: "pk.eyJ1IjoiYm9iYnlzdWQiLCJhIjoiTi16MElIUSJ9.Clrqck--7WmHeqqvtFdYig")


Replace this string literal with a placeholder token. See the example project in MapboxDirections.swift for an example.

1ec5 · 2016-11-17T01:11:18Z

Example/ViewController.swift

+        options.routeShapeResolution = .full
+
+        _ = directions.calculate(options) { [weak self] (waypoints, routes, error) in
+            if let route = routes?.first {


This should be a guard statement.

1ec5 · 2016-11-17T01:12:09Z

Example/ViewController.swift

+    }
+
+    func getRoute() {
+        let options = RouteOptions(coordinates: [mapView.userLocation!.coordinate, destination!])


Use the automobileAvoidingTraffic profile identifier.

1ec5 · 2016-11-17T01:39:25Z

In order to distribute this framework via CocoaPods, you'll need to add a podspec to the root of the repository. See MapboxDirections.swift for an example. Note that this podspec will directly require MapboxDirections but not Polyline (which would be a transitive dependency).

bsudekum · 2016-11-18T00:31:37Z

Good first step here, going to merge this and begin documentation.

The method is part of the public SDK API, and there are no guarantees that it will be called in a thread-safe manner. The fact current implementation isn't thread-safe was proofed by running a test with Thread Sanitizer (TSan) enabled (see the log at the end). - UnimplementedLogging.logUnimplemented reimplemented so that it is safe to use from multiple threads. - Adds a test to verify that the new implementation is safe. - Adds a test to catch performance regressions in implementation. Below is a log from TSan when run for old implementation: ``` WARNING: ThreadSanitizer: Swift access race (pid=14419) Modifying access of Swift variable at 0x00010a65a7a0 by thread T2: #0 UnimplementedLogging.logUnimplemented(protocolType:level:function:) <null>:12416464 (MapboxCoreNavigationTests:arm64+0x3a7464) #1 func1() in UnimplementedClass #1 in UnimplementedLoggingTests.testUnimplementedLoggingMultithreaded() UnimplementedLoggingTests.swift:9 (MapboxCoreNavigationTests:arm64+0xb26f4) #2 closure #1 in UnimplementedLoggingTests.testUnimplementedLoggingMultithreaded() UnimplementedLoggingTests.swift:24 (MapboxCoreNavigationTests:arm64+0xb1e9c) #3 partial apply for closure #1 in UnimplementedLoggingTests.testUnimplementedLoggingMultithreaded() <compiler-generated> (MapboxCoreNavigationTests:arm64+0xb1f34) #4 partial apply for thunk for @callee_guaranteed (@unowned Int) -> () <null>:12416464 (libswiftDispatch.dylib:arm64+0x5030) #5 _dispatch_client_callout2 <null>:12416464 (libdispatch.dylib:arm64+0x3d80) Previous read of size 8 at 0x00010a65a7a0 by thread T4: #0 UnimplementedLogging.logUnimplemented(protocolType:level:function:) <null>:12416464 (MapboxCoreNavigationTests:arm64+0x3a6f9c) #1 func1() in UnimplementedClass #1 in UnimplementedLoggingTests.testUnimplementedLoggingMultithreaded() UnimplementedLoggingTests.swift:9 (MapboxCoreNavigationTests:arm64+0xb26f4) #2 closure #1 in UnimplementedLoggingTests.testUnimplementedLoggingMultithreaded() UnimplementedLoggingTests.swift:24 (MapboxCoreNavigationTests:arm64+0xb1e9c) #3 partial apply for closure #1 in UnimplementedLoggingTests.testUnimplementedLoggingMultithreaded() <compiler-generated> (MapboxCoreNavigationTests:arm64+0xb1f34) #4 partial apply for thunk for @callee_guaranteed (@unowned Int) -> () <null>:12416464 (libswiftDispatch.dylib:arm64+0x5030) #5 _dispatch_client_callout2 <null>:12416464 (libdispatch.dylib:arm64+0x3d80) Location is global 'warned' at 0x00010a65a7a0 (MapboxCoreNavigationTests+0x000000e3a7a0) Thread T2 (tid=3981887, running) is a GCD worker thread Thread T4 (tid=3981890, running) is a GCD worker thread SUMMARY: ThreadSanitizer: Swift access race (MapboxCoreNavigationTests:arm64+0x3a7464) in UnimplementedLogging.logUnimplemented(protocolType:level:function:)+0x770 ```

commit

ad94893

1ec5 suggested changes Nov 17, 2016

View reviewed changes

bsudekum added 4 commits November 17, 2016 14:59

Updates

05e53bd

More updates

d76bcd9

Use traffic

b87cdf9

Dont use hardcoded access token

892f1e5

bsudekum merged commit 8abc6ca into master Nov 18, 2016

bsudekum deleted the newnewnew branch November 18, 2016 00:31

1ec5 mentioned this pull request Nov 18, 2016

Allow setting access token programmatically #4

Closed

1ec5 mentioned this pull request Sep 16, 2019

Discontinue Objective-C compatibility #2231

Closed

d-prukop mentioned this pull request Sep 8, 2020

Crash when arriving at intermediate waypoint #2611

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

General outline #3

General outline #3

bsudekum commented Nov 16, 2016 •

edited

Loading

1ec5 Nov 17, 2016

1ec5 Nov 17, 2016

1ec5 Nov 17, 2016

1ec5 Nov 17, 2016

1ec5 Nov 17, 2016

1ec5 Nov 17, 2016

1ec5 Nov 17, 2016

1ec5 Nov 17, 2016

1ec5 Nov 17, 2016

1ec5 Nov 17, 2016

1ec5 Nov 17, 2016

1ec5 commented Nov 17, 2016

bsudekum commented Nov 18, 2016

General outline #3

General outline #3

Conversation

bsudekum commented Nov 16, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ec5 commented Nov 17, 2016

bsudekum commented Nov 18, 2016

bsudekum commented Nov 16, 2016 •

edited

Loading