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

Upgrade to Swift 4.2 #2058

Merged
merged 10 commits into from Mar 23, 2019
Merged

Upgrade to Swift 4.2 #2058

merged 10 commits into from Mar 23, 2019

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Mar 23, 2019

Ran all the Xcode project’s targets through the Swift 4.2 migrator, simplifying some types. Refreshed the CocoaPods integration test. Removed build setting overrides in the SDK’s podspecs that only affected development pods in the CocoaPods integration test.

Fixes #2057.

/cc @mapbox/navigation-ios @friedbunny

@1ec5 1ec5 added the build Issues related to builds and dependency management. label Mar 23, 2019
@1ec5 1ec5 added this to the v0.31.0 milestone Mar 23, 2019
@1ec5 1ec5 self-assigned this Mar 23, 2019
@1ec5 1ec5 requested a review from frederoni March 23, 2019 16:56
@@ -365,42 +365,18 @@ extension UIApplicationState: Encodable {

extension AVAudioSession {
var audioType: String {
if isOutputBluetooth() {
if currentRoute.outputs.contains(where: { [.bluetoothA2DP, .bluetoothHFP, .bluetoothLE].contains($0.portType) }) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This could just be a simple switch statement inside a for loop, but it looks like the original code was prioritizing Bluetooth types over headphone types over speaker types.

return "speaker"
}
// if currentRoute.outputs.contains(where: { [.builtInMic, .headsetMic, .lineIn].contains($0.portType) }) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added bluetoothHFP to the Bluetooth cases above and carAudio and usbAudio to the headphone cases, but I wasn’t sure what to do about these microphone-related types, so they continue to be classified as “unknown”.

Copy link
Contributor

Choose a reason for hiding this comment

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

Falling back to unknown sounds reasonable. Let's drop this commented code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in #2059.

try audioSession.setCategory(.ambient, mode: audioSession.mode)
} else {
audioSession.perform(Selector("setCategory:error:" as String),
with: AVAudioSession.Category.ambient.rawValue)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is necessary because Swift 4.2 obsoleted setCategory(_:) in favor of setCategory(_:mode:), which isn’t supported on iOS 9. The same workaround was used in AudioKit/AudioKit#1554.

@@ -240,7 +240,7 @@ public class MapboxNavigationService: NSObject, NavigationService, DefaultInterf
locationSource: NavigationLocationManager? = nil,
eventsManagerType: NavigationEventsManager.Type? = nil,
simulating simulationMode: SimulationMode = .onPoorGPS,
routerType: Router.Type? = DefaultRouter.self)
routerType: Router.Type? = nil)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There’s already a fallback further down. This default value was responsible for a warning in client code, because DefaultRouter is internal.

This SWIFT_VERSION override in the podspecs is custom Ruby code that is lost in conversion to JSON when uploading to CocoaPods trunk. So it only affects projects that use the navigation SDK as a development pod, namely the CocoaPods integration test fixture in this repository.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues related to builds and dependency management.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants