-
Notifications
You must be signed in to change notification settings - Fork 325
Upgrade to MapboxNavigationNative 14.1.5 #2417
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
Changes from all commits
80e2f9c
b3a3cd7
3d8ffa7
5bbcf1f
d9c62bc
85ae849
09cb015
b6f9085
7cae104
edbc02a
065da2c
e60dae9
d3201ab
d84fb7c
aad1804
53dc0f9
a0b5f6a
e24302e
556c6fc
9499890
76f05f6
4576761
e1fd569
f616d9d
7967aba
449b7ee
ae65b54
57f3d2c
4ac4d35
09973b6
f60cd12
5103d03
30ac0c0
a76de59
71413ee
d4bc153
8c73594
d244aaa
40d9f0f
179d8d5
02d2f26
694eae3
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 |
|---|---|---|
|
|
@@ -74,7 +74,7 @@ step-library: | |
| run: | ||
| name: Build Example | ||
| command: | | ||
| xcodebuild -sdk iphonesimulator -destination 'platform=iOS Simulator,OS=12.2,name=iPhone 6 Plus' -project MapboxNavigation.xcodeproj -scheme Example clean build | xcpretty | ||
| xcodebuild -sdk iphonesimulator -destination 'platform=iOS Simulator,OS=13.4.1,name=iPhone 8 Plus' -project MapboxNavigation.xcodeproj -scheme Example clean build | xcpretty | ||
|
|
||
| jobs: | ||
| pod-job: | ||
|
|
@@ -84,10 +84,10 @@ jobs: | |
| default: false | ||
| iOS: | ||
| type: string | ||
| default: "12.2" | ||
| default: "13.4.1" | ||
| xcode: | ||
| type: string | ||
| default: "10.2.0" | ||
| default: "11.4.1" | ||
| lint: | ||
| type: boolean | ||
| default: false | ||
|
|
@@ -110,7 +110,7 @@ jobs: | |
| condition: << parameters.update >> | ||
| steps: | ||
| - run: cd MapboxCoreNavigationTests/CocoaPodsTest/PodInstall && pod install --repo-update | ||
| - run: cd MapboxCoreNavigationTests/CocoaPodsTest/PodInstall && xcodebuild -workspace PodInstall.xcworkspace -scheme PodInstall -destination 'platform=iOS Simulator,OS=<< parameters.iOS >>,name=iPhone 6 Plus' clean build | xcpretty | ||
| - run: cd MapboxCoreNavigationTests/CocoaPodsTest/PodInstall && xcodebuild -workspace PodInstall.xcworkspace -scheme PodInstall -destination 'platform=iOS Simulator,OS=<< parameters.iOS >>,name=iPhone 8 Plus' clean build | xcpretty | ||
| - when: | ||
| condition: << parameters.lint >> | ||
| steps: | ||
|
|
@@ -123,13 +123,13 @@ jobs: | |
| parameters: | ||
| xcode: | ||
| type: string | ||
| default: "10.1.0" | ||
| default: "11.4.1" | ||
| device: | ||
| type: string | ||
| default: "iPhone 6 Plus" | ||
| default: "iPhone 8 Plus" | ||
| iOS: | ||
| type: string | ||
| default: "12.1" | ||
| default: "13.4.1" | ||
| test: | ||
| type: boolean | ||
| default: true | ||
|
|
@@ -177,9 +177,9 @@ jobs: | |
| steps: | ||
| - run: bash <(curl -s https://codecov.io/bash) | ||
|
|
||
| xcode-10-examples: | ||
| xcode-11-examples: | ||
| macos: | ||
| xcode: "10.2.0" | ||
| xcode: "11.4.1" | ||
| environment: | ||
| HOMEBREW_NO_AUTO_UPDATE: 1 | ||
| steps: | ||
|
|
@@ -201,24 +201,25 @@ workflows: | |
| iOS: "13.5" | ||
| device: "iPhone 8 Plus" | ||
| - build-job: | ||
| name: "Xcode_10.3_iOS_12.1" | ||
| xcode: "10.3.0" | ||
| iOS: "12.1" | ||
| name: "Xcode_11.4.1_iOS_12.2" | ||
| xcode: "11.4.1" | ||
| iOS: "12.2" | ||
| codecoverage: true | ||
| - build-job: | ||
| name: "Xcode_10.2_iOS_10.3.1" | ||
| xcode: "10.2.1" | ||
| name: "Xcode_11.4.1_iOS_10.3.1" | ||
| xcode: "11.4.1" | ||
|
Comment on lines
+209
to
+210
Contributor
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. We need to continue to test against iOS 10.x somehow, because the SDK still supports iOS 10.x. On the other hand, it’s fine to drop Xcode 10 support, but we should update the readme to make that clear.
Contributor
Author
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. I've added step to test against iOS
Contributor
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. Yes, that’s OK. |
||
| iOS: "10.3.1" | ||
| test: false | ||
| device: "iPhone 7 Plus" | ||
| - pod-job: | ||
| name: "Xcode_10.2_iOS_12.2_CP_install" | ||
| name: "Xcode_11.4.1_iOS_12.2_CP_install" | ||
| update: false | ||
| xcode: "10.2.1" | ||
| xcode: "11.4.1" | ||
| iOS: "12.2" | ||
| - pod-job: | ||
| name: "Xcode_10.2_iOS_12.2_CP_update" | ||
| name: "Xcode_11.4.1_iOS_12.2_CP_update" | ||
| update: true | ||
| xcode: "10.2.1" | ||
| xcode: "11.4.1" | ||
| iOS: "12.2" | ||
| lint: true | ||
| - xcode-10-examples | ||
| - xcode-11-examples | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| binary "https://www.mapbox.com/ios-sdk/MapboxAccounts.json" ~> 2.3 | ||
| binary "https://www.mapbox.com/ios-sdk/MapboxAccounts.json" ~> 2.3.0 | ||
|
Contributor
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. Should we pin to a patch version like this? It means a future minor version would be incompatible unless we put out a new version that requires it. MapboxAccounts is post-v1.0, so I don’t think we have to be as concerned about backwards-incompatible changes creeping in. |
||
| binary "https://mapbox-gl-native-ios.s3.amazonaws.com/public/internal/Mapbox-iOS-SDK.json" == 5.9.1000 | ||
| binary "https://www.mapbox.com/ios-sdk/MapboxNavigationNative.json" ~> 9.2.1 | ||
| binary "https://www.mapbox.com/ios-sdk/MapboxNavigationNative.json" ~> 14.1.5 | ||
| github "mapbox/mapbox-directions-swift" ~> 0.32 | ||
| github "mapbox/turf-swift" ~> 0.5 | ||
| github "mapbox/mapbox-events-ios" ~> 0.10 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -117,7 +117,7 @@ public class NavigationDirections: Directions { | |
| public func configureRouter(tilesURL: URL, completionHandler: @escaping NavigationDirectionsCompletionHandler) { | ||
| NavigationDirectionsConstants.offlineSerialQueue.sync { | ||
| let params = RouterParams(tilesPath: tilesURL.path, inMemoryTileCache: nil, mapMatchingSpatialCache: nil, threadsCount: nil, endpointConfig: nil) | ||
| let tileCount = self.navigator.configureRouter(for: params, httpInterface: nil) | ||
| let tileCount = self.navigator.configureRouter(for: params) | ||
| DispatchQueue.main.async { | ||
| completionHandler(tileCount) | ||
| } | ||
|
|
@@ -152,7 +152,12 @@ public class NavigationDirections: Directions { | |
| let tilePath = filePathURL.path | ||
| let outputPath = outputDirectoryURL.path | ||
|
|
||
| let numberOfTiles = Navigator().unpackTiles(forPackedTilesPath: tilePath, outputDirectory: outputPath) | ||
| let navigator: Navigator = { | ||
| let settingsProfile = SettingsProfile(application: ProfileApplication.kMobile, | ||
| platform: ProfilePlatform.KIOS) | ||
| return Navigator(profile: settingsProfile, customConfig: "") | ||
| }() | ||
|
Comment on lines
+155
to
+159
Contributor
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. Can we use
Contributor
Author
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. In this place navigator is used in static context, probably better to leave it as it is. |
||
| let numberOfTiles = navigator.unpackTiles(forPackedTilesPath: tilePath, outputDirectory: outputPath) | ||
|
|
||
| // Report 100% progress | ||
| progressHandler?(totalPackedBytes, totalPackedBytes) | ||
|
|
@@ -235,7 +240,9 @@ public class NavigationDirections: Directions { | |
| "The offline navigator must be accessed from the dedicated serial queue") | ||
|
|
||
| if _navigator == nil { | ||
| self._navigator = Navigator() | ||
| let settingsProfile = SettingsProfile(application: ProfileApplication.kMobile, | ||
| platform: ProfilePlatform.KIOS) | ||
|
Contributor
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. Lovely C++→Objective-C→Swift translation there. 😁 |
||
| self._navigator = Navigator(profile: settingsProfile, customConfig: "") | ||
| } | ||
|
|
||
| return _navigator | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,8 +20,12 @@ open class RouteController: NSObject { | |
| public static let shouldPreventReroutesWhenArrivingAtWaypoint: Bool = true | ||
| public static let shouldDisableBatteryMonitoring: Bool = true | ||
| } | ||
|
|
||
| let navigator = Navigator() | ||
|
|
||
| lazy var navigator: Navigator = { | ||
| let settingsProfile = SettingsProfile(application: ProfileApplication.kMobile, | ||
| platform: ProfilePlatform.KIOS) | ||
| return Navigator(profile: settingsProfile, customConfig: "") | ||
| }() | ||
|
|
||
| public var route: Route { | ||
| get { | ||
|
|
@@ -316,7 +320,7 @@ open class RouteController: NSObject { | |
| NotificationUserInfoKey.routeProgressKey: progress, | ||
| NotificationUserInfoKey.locationKey: location, //guaranteed value | ||
| NotificationUserInfoKey.rawLocationKey: rawLocation, //raw | ||
| ]) | ||
| ]) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -387,7 +391,7 @@ extension RouteController: Router { | |
| } | ||
|
|
||
| let status = status ?? navigator.getStatusForTimestamp(location.timestamp) | ||
| let offRoute = status.routeState == .offRoute | ||
| let offRoute = status.routeState == .offRoute || status.routeState == .invalid | ||
|
Contributor
Author
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. As per discussion with @SiarheiFedartsou in Slack, for now it's better to treat |
||
| return !offRoute | ||
| } | ||
|
|
||
|
|
@@ -431,9 +435,6 @@ extension RouteController: Router { | |
| ]) | ||
| return | ||
| } | ||
|
|
||
|
|
||
|
|
||
| } | ||
| } | ||
| } | ||
|
|
||
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.
Various test fixtures are conditionalized by iOS version, based on their file names. So we’ll need to rename or delete and rerecord a slew of test fixtures, similar to what took place in #2427.
Uh oh!
There was an error while loading. Please reload this page.
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.
Thanks, fixtures were updated. I've also decided to remove iOS 13.4.1 build job since there is one for 13.5 already (this will allow to speed up build time and prevent adding additional fixtures for 13.4.1).