Skip to content
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

MapboxDirections.swift v0.12.0, OSRMTextInstructions v0.5.0 #828

Merged
merged 3 commits into from
Nov 11, 2017

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Nov 10, 2017

Also use the “nationalized current locale” for consistency.

Fixes #821. Depends on Project-OSRM/osrm-text-instructions.swift#51.

/cc @bsudekum @frederoni @JThramer

Also use the “nationalized current locale” for consistency.
@1ec5 1ec5 added bug Something isn’t working build Issues related to builds and dependency management. topic: localization topic: voice labels Nov 10, 2017
@1ec5 1ec5 self-assigned this Nov 10, 2017
@1ec5
Copy link
Contributor Author

1ec5 commented Nov 10, 2017

Tests are crashing on CI but not locally:

Test Case '-[MapboxCoreNavigationTests.MapboxCoreNavigationTests testArrive]' started.
2017-11-09 23:41:34.022631-0800 xctest[5636:22813] This app has attempted to access privacy-sensitive data without a usage description. The app's Info.plist must contain an NSLocationWhenInUseUsageDescription key with a string value explaining to the user how the app uses this data
Fatal error: Unexpectedly found nil while unwrapping an Optional value
2017-11-09 23:41:34.035252-0800 xctest[5636:22813] Fatal error: Unexpectedly found nil while unwrapping an Optional value
2017-11-09 23:41:52.759 xcodebuild[4798:23324]  IDETestOperationsObserverDebug: Writing diagnostic log for test session to:
/var/folders/90/5stft2v13fb_m_gv3c8x9nwc0000gn/T/com.apple.dt.XCTest/IDETestRunSession-9B016A0B-4FE7-4A8E-9EFB-AED9851CC97F/MapboxCoreNavigationTests-35B98E80-5ADF-4288-A8F4-E8C1F22AA196/Session-MapboxCoreNavigationTests-2017-11-09_234152-IJVu9i.log
2017-11-09 23:41:52.759 xcodebuild[4798:18215] [MT] IDETestOperationsObserverDebug: (49563C5D-C219-46D2-825F-344BBF6B4149) Beginning test session MapboxCoreNavigationTests-49563C5D-C219-46D2-825F-344BBF6B4149 at 2017-11-09 23:41:52.760 with Xcode 9B55 on target <DVTiPhoneSimulator: 0x7fce26a5a370> {
		SimDevice: iPhone 6s Plus (014D6C15-FD93-4320-A812-EA3A6001356E, iOS 11.1, Booted)
} (11.1 (15B87))

Restarting after unexpected exit or crash in MapboxCoreNavigationTests/testArrive(); summary will include totals from previous launches.

@bsudekum
Copy link
Contributor

Interesting, I'm actually seeing the failure locally:

image

@bsudekum
Copy link
Contributor

I think this is failing because the encoded tunnel.route object is different now on MapboxDirections.swift v0.12.0.

@bsudekum
Copy link
Contributor

Ok now getting an error here:

image

This line looks incorrect to me:

https://github.com/mapbox/MapboxDirections.swift/blob/master/MapboxDirections/MBRouteLeg.swift#L101

We should not be decoding an array of numbers.

@bsudekum
Copy link
Contributor

Something is not right, even after updating tunnel.route, I'm still getting an error.

@bsudekum bsudekum mentioned this pull request Nov 10, 2017
Copy link
Contributor

@bsudekum bsudekum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seems to be deeper issues here.

@1ec5
Copy link
Contributor Author

1ec5 commented Nov 10, 2017

https://github.com/mapbox/MapboxDirections.swift/blob/master/MapboxDirections/MBRouteLeg.swift#L101

We should not be decoding an array of numbers.

That line is correct. CongestionLevel is an enumeration, so it’s represented in the archive as a number, because archives have a much more limited set of types (only slightly more flexible than JSON).

@JThramer
Copy link
Contributor

@bsudekum @1ec5 the only thing that comes to mind regarding this issue is possibly mapbox/mapbox-directions-swift#204? That bug could cause RouteOptions to return as nil… and if you’re trying to unarchive a Route, I could see where RouteOptions.initWithCoder: returning nil for a property (Route.routeOptions) that is not Optional could be causing the problem…

@1ec5
Copy link
Contributor Author

1ec5 commented Nov 10, 2017

Interesting, I'm actually seeing the failure locally

My bad, I was testing MapboxDirections.swift locally but not the navigation SDK. 😳

the only thing that comes to mind regarding this issue is possibly mapbox/mapbox-directions-swift#204?

Thanks, I can confirm that building with MapboxDirections.swift on master (including mapbox/mapbox-directions-swift#204) does fix the issue. Let’s publish a patch release of MapboxDirections.swift with that fix: mapbox/mapbox-directions-swift#205.

@1ec5 1ec5 requested a review from bsudekum November 10, 2017 19:42
@1ec5
Copy link
Contributor Author

1ec5 commented Nov 10, 2017

The test failures should be resolved now, @bsudekum.

@1ec5 1ec5 merged commit 1e2953b into master Nov 11, 2017
@1ec5 1ec5 deleted the 1ec5-voice-locale-821 branch November 11, 2017 00:47
@bsudekum bsudekum mentioned this pull request Nov 11, 2017
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn’t working build Issues related to builds and dependency management. topic: localization topic: voice
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants