From dec1c3fa848e9e2dcb8876d6057e1fc63f58a622 Mon Sep 17 00:00:00 2001 From: Andreas Bauer Date: Tue, 23 Jul 2024 14:01:26 +0200 Subject: [PATCH 1/7] Adjust implementation for async connect method --- Package.swift | 2 +- Sources/SpeziDevices/PairedDevices.swift | 60 ++++++++++++++----- .../PairedDevicesTests.swift | 4 +- .../UITests/TestApp/BluetoothViewsTest.swift | 2 +- Tests/UITests/TestApp/DevicesTestView.swift | 6 +- 5 files changed, 52 insertions(+), 22 deletions(-) diff --git a/Package.swift b/Package.swift index 20fe7e2..07cc74f 100644 --- a/Package.swift +++ b/Package.swift @@ -36,7 +36,7 @@ let package = Package( .package(url: "https://github.com/StanfordSpezi/SpeziFoundation", from: "1.1.1"), .package(url: "https://github.com/StanfordSpezi/Spezi.git", from: "1.4.0"), .package(url: "https://github.com/StanfordSpezi/SpeziViews.git", from: "1.5.0"), - .package(url: "https://github.com/StanfordSpezi/SpeziBluetooth", exact: "3.0.0-beta.1"), + .package(url: "https://github.com/StanfordSpezi/SpeziBluetooth", exact: "3.0.0-beta.2"), .package(url: "https://github.com/StanfordSpezi/SpeziNetworking", from: "2.1.1"), .package(url: "https://github.com/StanfordBDHG/XCTestExtensions.git", .upToNextMinor(from: "0.4.12")) ] + swiftLintPackage(), diff --git a/Sources/SpeziDevices/PairedDevices.swift b/Sources/SpeziDevices/PairedDevices.swift index 9ad8a90..26e5efb 100644 --- a/Sources/SpeziDevices/PairedDevices.swift +++ b/Sources/SpeziDevices/PairedDevices.swift @@ -336,7 +336,15 @@ public final class PairedDevices: @unchecked Sendable { pendingConnectionAttempts[device.id] = Task { await previousTask?.value // make sure its ordered - await device.connect() + do { + try await device.connect() + } catch { + guard !Task.isCancelled else { + return + } + logger.warning("Failed connection attempt for device \(device.label). Retrying ...") + connectionAttempt(for: device) + } } } @@ -394,27 +402,49 @@ extension PairedDevices { throw DevicePairingError.invalidState } - await device.connect() - let id = device.id - let timeoutHandler = { @Sendable @MainActor in - _ = self.ongoingPairings.removeValue(forKey: id)?.signalTimeout() - } - async let _ = withTimeout(of: timeout, perform: timeoutHandler) + try await withThrowingDiscardingTaskGroup { group in + // timeout task + group.addTask { + await withTimeout(of: timeout) { @MainActor in + _ = self.ongoingPairings.removeValue(forKey: id)?.signalTimeout() + } + } + + // connect task + group.addTask { @MainActor in + do { + try await device.connect() + } catch { + if error is CancellationError { + self.ongoingPairings.removeValue(forKey: id)?.signalCancellation() + } - try await withTaskCancellationHandler { - try await withCheckedThrowingContinuation { continuation in - ongoingPairings[id] = PairingContinuation(continuation) + throw error + } } - } onCancel: { - Task { @MainActor [weak device] in - ongoingPairings.removeValue(forKey: id)?.signalCancellation() - await device?.disconnect() + + // pairing task + group.addTask { @MainActor in + try await withTaskCancellationHandler { + try await withCheckedThrowingContinuation { continuation in + self.ongoingPairings[id] = PairingContinuation(continuation) + } + } onCancel: { + Task { @MainActor [weak device] in + self.ongoingPairings.removeValue(forKey: id)?.signalCancellation() + await device?.disconnect() + } + } } } - // if cancelled the continuation throws an CancellationError + // the task group above should exit with a CancellationError anyways, but safe to double check here + guard !Task.isCancelled else { + throw CancellationError() + } + await registerPairedDevice(device) } diff --git a/Tests/SpeziDevicesTests/PairedDevicesTests.swift b/Tests/SpeziDevicesTests/PairedDevicesTests.swift index 57cc2e5..3e1840c 100644 --- a/Tests/SpeziDevicesTests/PairedDevicesTests.swift +++ b/Tests/SpeziDevicesTests/PairedDevicesTests.swift @@ -83,7 +83,7 @@ final class PairedDevicesTests: XCTestCase { }() - await device.connect() + try await device.connect() try await Task.sleep(for: .seconds(1.1)) XCTAssertEqual(device.state, .connected) @@ -113,7 +113,7 @@ final class PairedDevicesTests: XCTestCase { } device.$nearby.inject(true) - await device.connect() + try await device.connect() try await XCTAssertThrowsErrorAsync(await devices.pair(with: device)) { error in XCTAssertEqual(try XCTUnwrap(error as? DevicePairingError), .invalidState) } diff --git a/Tests/UITests/TestApp/BluetoothViewsTest.swift b/Tests/UITests/TestApp/BluetoothViewsTest.swift index 284b26f..b570fb0 100644 --- a/Tests/UITests/TestApp/BluetoothViewsTest.swift +++ b/Tests/UITests/TestApp/BluetoothViewsTest.swift @@ -41,7 +41,7 @@ struct BluetoothViewsTest: View { Task { switch device.state { case .disconnected, .disconnecting: - await device.connect() + try await device.connect() case .connecting, .connected: await device.disconnect() } diff --git a/Tests/UITests/TestApp/DevicesTestView.swift b/Tests/UITests/TestApp/DevicesTestView.swift index f30828c..29a1a68 100644 --- a/Tests/UITests/TestApp/DevicesTestView.swift +++ b/Tests/UITests/TestApp/DevicesTestView.swift @@ -48,9 +48,9 @@ struct DevicesTestView: View { device.$advertisementData.inject(AdvertisementData()) // trigger onChange advertisement } AsyncButton { - await device.connect() - await weightScale.connect() - await bloodPressureCuff.connect() + try await device.connect() + try await weightScale.connect() + try await bloodPressureCuff.connect() } label: { Label("Connect", systemImage: "cable.connector") } From b91924234483e03e357fdb125b280b9329f79e36 Mon Sep 17 00:00:00 2001 From: Andreas Bauer Date: Tue, 23 Jul 2024 15:18:19 +0200 Subject: [PATCH 2/7] Fix tests --- Tests/UITests/TestApp/DevicesTestView.swift | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Tests/UITests/TestApp/DevicesTestView.swift b/Tests/UITests/TestApp/DevicesTestView.swift index 29a1a68..4d00efa 100644 --- a/Tests/UITests/TestApp/DevicesTestView.swift +++ b/Tests/UITests/TestApp/DevicesTestView.swift @@ -38,6 +38,8 @@ struct DevicesTestView: View { @State private var weightScale = OmronWeightScale.createMockDevice(manufacturerData: .omronManufacturerData(mode: .transferMode)) @State private var bloodPressureCuff = OmronBloodPressureCuff.createMockDevice(manufacturerData: .omronManufacturerData(mode: .transferMode)) + @State private var viewState: ViewState = .idle + var body: some View { NavigationStack { DevicesView(appName: "Example", pairingHint: "Enable pairing mode on the device.") @@ -47,7 +49,7 @@ struct DevicesTestView: View { device.isInPairingMode = true device.$advertisementData.inject(AdvertisementData()) // trigger onChange advertisement } - AsyncButton { + AsyncButton(state: $viewState) { try await device.connect() try await weightScale.connect() try await bloodPressureCuff.connect() @@ -97,6 +99,7 @@ struct DevicesTestView: View { ) didRegister = true } + .viewStateAlert(state: $viewState) } } From 7f0d7fe21dc9ce4030ee34bd0f345d90ecb59a33 Mon Sep 17 00:00:00 2001 From: Andreas Bauer Date: Wed, 24 Jul 2024 08:10:45 +0200 Subject: [PATCH 3/7] Enable InferSendableFromCaptures --- Package.swift | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/Package.swift b/Package.swift index 07cc74f..9999a43 100644 --- a/Package.swift +++ b/Package.swift @@ -52,7 +52,8 @@ let package = Package( .product(name: "Spezi", package: "Spezi") ], swiftSettings: [ - swiftConcurrency + swiftConcurrency, + .enableUpcomingFeature("InferSendableFromCaptures") ], plugins: [] + swiftLintPlugin() ), @@ -68,7 +69,8 @@ let package = Package( .process("Resources") ], swiftSettings: [ - swiftConcurrency + swiftConcurrency, + .enableUpcomingFeature("InferSendableFromCaptures") ], plugins: [] + swiftLintPlugin() ), @@ -83,7 +85,8 @@ let package = Package( .process("Resources") ], swiftSettings: [ - swiftConcurrency + swiftConcurrency, + .enableUpcomingFeature("InferSendableFromCaptures") ], plugins: [] + swiftLintPlugin() ), @@ -98,7 +101,8 @@ let package = Package( .product(name: "XCTestExtensions", package: "XCTestExtensions") ], swiftSettings: [ - swiftConcurrency + swiftConcurrency, + .enableUpcomingFeature("InferSendableFromCaptures") ], plugins: [] + swiftLintPlugin() ), @@ -111,7 +115,8 @@ let package = Package( .product(name: "XCTestExtensions", package: "XCTestExtensions") ], swiftSettings: [ - swiftConcurrency + swiftConcurrency, + .enableUpcomingFeature("InferSendableFromCaptures") ], plugins: [] + swiftLintPlugin() ) From 65254de0c29ca6ccac12b5c1e76903b46883353b Mon Sep 17 00:00:00 2001 From: Andreas Bauer Date: Wed, 24 Jul 2024 08:10:49 +0200 Subject: [PATCH 4/7] Race timeout against task group --- Sources/SpeziDevices/PairedDevices.swift | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/Sources/SpeziDevices/PairedDevices.swift b/Sources/SpeziDevices/PairedDevices.swift index 26e5efb..a1c81ae 100644 --- a/Sources/SpeziDevices/PairedDevices.swift +++ b/Sources/SpeziDevices/PairedDevices.swift @@ -404,14 +404,12 @@ extension PairedDevices { let id = device.id - try await withThrowingDiscardingTaskGroup { group in - // timeout task - group.addTask { - await withTimeout(of: timeout) { @MainActor in - _ = self.ongoingPairings.removeValue(forKey: id)?.signalTimeout() - } - } + // race timeout against the tasks below + async let _ = await withTimeout(of: timeout) { @MainActor in + _ = self.ongoingPairings.removeValue(forKey: id)?.signalTimeout() + } + try await withThrowingDiscardingTaskGroup { group in // connect task group.addTask { @MainActor in do { From d85556d8a1b7e78f1c2139affccfd303c17067f2 Mon Sep 17 00:00:00 2001 From: Andreas Bauer Date: Wed, 24 Jul 2024 08:31:32 +0200 Subject: [PATCH 5/7] Rerun --- Tests/UITests/TestApp/DevicesTestView.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/UITests/TestApp/DevicesTestView.swift b/Tests/UITests/TestApp/DevicesTestView.swift index 4d00efa..a99828f 100644 --- a/Tests/UITests/TestApp/DevicesTestView.swift +++ b/Tests/UITests/TestApp/DevicesTestView.swift @@ -79,6 +79,7 @@ struct DevicesTestView: View { } } } + .viewStateAlert(state: $viewState) .onAppear { guard !didRegister else { return @@ -99,7 +100,6 @@ struct DevicesTestView: View { ) didRegister = true } - .viewStateAlert(state: $viewState) } } From 9ccdc1593945479737a134ddb70063afacfd8fd8 Mon Sep 17 00:00:00 2001 From: Andreas Bauer Date: Wed, 24 Jul 2024 16:21:48 +0200 Subject: [PATCH 6/7] Rerun CI --- Sources/SpeziDevices/PairedDevices.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/SpeziDevices/PairedDevices.swift b/Sources/SpeziDevices/PairedDevices.swift index a1c81ae..ca87720 100644 --- a/Sources/SpeziDevices/PairedDevices.swift +++ b/Sources/SpeziDevices/PairedDevices.swift @@ -438,6 +438,7 @@ extension PairedDevices { } } + // the task group above should exit with a CancellationError anyways, but safe to double check here guard !Task.isCancelled else { throw CancellationError() From bb8097629140d8da2d5ef7d2f26bcda0ca6c83cd Mon Sep 17 00:00:00 2001 From: Andreas Bauer Date: Wed, 24 Jul 2024 16:45:03 +0200 Subject: [PATCH 7/7] Restore coverage collection in UI tests --- .../xcshareddata/xcschemes/TestApp.xcscheme | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/Tests/UITests/UITests.xcodeproj/xcshareddata/xcschemes/TestApp.xcscheme b/Tests/UITests/UITests.xcodeproj/xcshareddata/xcschemes/TestApp.xcscheme index be1e4c0..deb115c 100644 --- a/Tests/UITests/UITests.xcodeproj/xcshareddata/xcschemes/TestApp.xcscheme +++ b/Tests/UITests/UITests.xcodeproj/xcshareddata/xcschemes/TestApp.xcscheme @@ -34,6 +34,34 @@ ReferencedContainer = "container:UITests.xcodeproj"> + + + + + + + +