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

Adjust implementation for async connect method #8

Merged
merged 7 commits into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -52,7 +52,8 @@ let package = Package(
.product(name: "Spezi", package: "Spezi")
],
swiftSettings: [
swiftConcurrency
swiftConcurrency,
.enableUpcomingFeature("InferSendableFromCaptures")
],
plugins: [] + swiftLintPlugin()
),
Expand All @@ -68,7 +69,8 @@ let package = Package(
.process("Resources")
],
swiftSettings: [
swiftConcurrency
swiftConcurrency,
.enableUpcomingFeature("InferSendableFromCaptures")
],
plugins: [] + swiftLintPlugin()
),
Expand All @@ -83,7 +85,8 @@ let package = Package(
.process("Resources")
],
swiftSettings: [
swiftConcurrency
swiftConcurrency,
.enableUpcomingFeature("InferSendableFromCaptures")
],
plugins: [] + swiftLintPlugin()
),
Expand All @@ -98,7 +101,8 @@ let package = Package(
.product(name: "XCTestExtensions", package: "XCTestExtensions")
],
swiftSettings: [
swiftConcurrency
swiftConcurrency,
.enableUpcomingFeature("InferSendableFromCaptures")
],
plugins: [] + swiftLintPlugin()
),
Expand All @@ -111,7 +115,8 @@ let package = Package(
.product(name: "XCTestExtensions", package: "XCTestExtensions")
],
swiftSettings: [
swiftConcurrency
swiftConcurrency,
.enableUpcomingFeature("InferSendableFromCaptures")
],
plugins: [] + swiftLintPlugin()
)
Expand Down
55 changes: 42 additions & 13 deletions Sources/SpeziDevices/PairedDevices.swift
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,15 @@

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)
}

Check warning on line 347 in Sources/SpeziDevices/PairedDevices.swift

View check run for this annotation

Codecov / codecov/patch

Sources/SpeziDevices/PairedDevices.swift#L339-L347

Added lines #L339 - L347 were not covered by tests
}
}

Expand Down Expand Up @@ -394,27 +402,48 @@
throw DevicePairingError.invalidState
}

await device.connect()

let id = device.id
let timeoutHandler = { @Sendable @MainActor in

// race timeout against the tasks below
async let _ = await withTimeout(of: timeout) { @MainActor in
_ = self.ongoingPairings.removeValue(forKey: id)?.signalTimeout()
}

async let _ = withTimeout(of: timeout, perform: timeoutHandler)
try await withThrowingDiscardingTaskGroup { group in
// connect task
group.addTask { @MainActor in
do {
try await device.connect()
} catch {
if error is CancellationError {
self.ongoingPairings.removeValue(forKey: id)?.signalCancellation()
}

Check warning on line 420 in Sources/SpeziDevices/PairedDevices.swift

View check run for this annotation

Codecov / codecov/patch

Sources/SpeziDevices/PairedDevices.swift#L418-L420

Added lines #L418 - L420 were not covered by tests

try await withTaskCancellationHandler {
try await withCheckedThrowingContinuation { continuation in
ongoingPairings[id] = PairingContinuation(continuation)
throw error

Check warning on line 422 in Sources/SpeziDevices/PairedDevices.swift

View check run for this annotation

Codecov / codecov/patch

Sources/SpeziDevices/PairedDevices.swift#L422

Added line #L422 was not covered by tests
}
}
} 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()

Check warning on line 444 in Sources/SpeziDevices/PairedDevices.swift

View check run for this annotation

Codecov / codecov/patch

Sources/SpeziDevices/PairedDevices.swift#L444

Added line #L444 was not covered by tests
}

await registerPairedDevice(device)
}

Expand Down
4 changes: 2 additions & 2 deletions Tests/SpeziDevicesTests/PairedDevicesTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion Tests/UITests/TestApp/BluetoothViewsTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
11 changes: 7 additions & 4 deletions Tests/UITests/TestApp/DevicesTestView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
Expand All @@ -47,10 +49,10 @@ struct DevicesTestView: View {
device.isInPairingMode = true
device.$advertisementData.inject(AdvertisementData()) // trigger onChange advertisement
}
AsyncButton {
await device.connect()
await weightScale.connect()
await bloodPressureCuff.connect()
AsyncButton(state: $viewState) {
try await device.connect()
try await weightScale.connect()
try await bloodPressureCuff.connect()
} label: {
Label("Connect", systemImage: "cable.connector")
}
Expand All @@ -77,6 +79,7 @@ struct DevicesTestView: View {
}
}
}
.viewStateAlert(state: $viewState)
.onAppear {
guard !didRegister else {
return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,34 @@
ReferencedContainer = "container:UITests.xcodeproj">
</BuildableReference>
</BuildActionEntry>
<BuildActionEntry
buildForTesting = "YES"
buildForRunning = "NO"
buildForProfiling = "NO"
buildForArchiving = "NO"
buildForAnalyzing = "NO">
<BuildableReference
BuildableIdentifier = "primary"
BlueprintIdentifier = "SpeziOmron"
BuildableName = "SpeziOmron"
BlueprintName = "SpeziOmron"
ReferencedContainer = "container:../..">
</BuildableReference>
</BuildActionEntry>
<BuildActionEntry
buildForTesting = "YES"
buildForRunning = "NO"
buildForProfiling = "NO"
buildForArchiving = "NO"
buildForAnalyzing = "NO">
<BuildableReference
BuildableIdentifier = "primary"
BlueprintIdentifier = "SpeziDevicesUI"
BuildableName = "SpeziDevicesUI"
BlueprintName = "SpeziDevicesUI"
ReferencedContainer = "container:../..">
</BuildableReference>
</BuildActionEntry>
</BuildActionEntries>
</BuildAction>
<TestAction
Expand Down
Loading