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

Improve how we persist the last manually disconnected device #51

Merged
merged 25 commits into from
Nov 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
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
23 changes: 1 addition & 22 deletions .github/workflows/build-and-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,6 @@ jobs:
scheme: SpeziBluetooth-Package
artifactname: SpeziBluetooth-Package.xcresult
resultBundle: SpeziBluetooth-Package.xcresult
packageios_latest:
name: Build and Test Swift Package iOS Latest
uses: StanfordSpezi/.github/.github/workflows/xcodebuild-or-fastlane.yml@v2
with:
runsonlabels: '["macOS", "self-hosted", "spezi"]'
scheme: SpeziBluetooth-Package
xcodeversion: latest
swiftVersion: 6
artifactname: SpeziBluetooth-Package-Latest.xcresult
resultBundle: SpeziBluetooth-Package-Latest.xcresult
ios:
name: Build and Test iOS
uses: StanfordSpezi/.github/.github/workflows/xcodebuild-or-fastlane.yml@v2
Expand All @@ -43,26 +33,15 @@ jobs:
scheme: TestApp
artifactname: TestApp-iOS.xcresult
resultBundle: TestApp-iOS.xcresult
ios_latest:
name: Build and Test iOS Latest
uses: StanfordSpezi/.github/.github/workflows/xcodebuild-or-fastlane.yml@v2
with:
runsonlabels: '["macOS", "self-hosted", "spezi"]'
path: 'Tests/UITests'
scheme: TestApp
xcodeversion: latest
swiftVersion: 6
artifactname: TestApp-iOS-Latest.xcresult
resultBundle: TestApp-iOS-Latest.xcresult
macos:
name: Build and Test macOS
uses: StanfordSpezi/.github/.github/workflows/xcodebuild-or-fastlane.yml@v2
permissions:
contents: read
with:
runsonlabels: '["macOS", "self-hosted", "bluetooth"]'
setupsigning: true
path: 'Tests/UITests'
setupsigning: true
artifactname: TestApp-macOS.xcresult
resultBundle: TestApp-macOS.xcresult
customcommand: "set -o pipefail && xcodebuild test -scheme 'TestApp' -configuration 'Test' -destination 'platform=macOS,arch=arm64,variant=Mac Catalyst' -derivedDataPath '.derivedData' -resultBundlePath 'TestApp-macOS.xcresult' -skipPackagePluginValidation -skipMacroValidation | xcbeautify"
Expand Down
25 changes: 1 addition & 24 deletions Package.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// swift-tools-version:5.9
// swift-tools-version:6.0

//
// This source file is part of the Stanford Spezi open source project
Expand All @@ -12,13 +12,6 @@ import class Foundation.ProcessInfo
import PackageDescription


#if swift(<6)
let swiftConcurrency: SwiftSetting = .enableExperimentalFeature("StrictConcurrency")
#else
let swiftConcurrency: SwiftSetting = .enableUpcomingFeature("StrictConcurrency")
#endif


let package = Package(
name: "SpeziBluetooth",
defaultLocalization: "en",
Expand Down Expand Up @@ -54,10 +47,6 @@ let package = Package(
resources: [
.process("Resources")
],
swiftSettings: [
swiftConcurrency,
.enableUpcomingFeature("InferSendableFromCaptures")
],
plugins: [] + swiftLintPlugin()
),
.target(
Expand All @@ -67,9 +56,6 @@ let package = Package(
.product(name: "ByteCoding", package: "SpeziNetworking"),
.product(name: "SpeziNumerics", package: "SpeziNetworking")
],
swiftSettings: [
swiftConcurrency
],
plugins: [] + swiftLintPlugin()
),
.executableTarget(
Expand All @@ -79,9 +65,6 @@ let package = Package(
.target(name: "SpeziBluetoothServices"),
.product(name: "ByteCoding", package: "SpeziNetworking")
],
swiftSettings: [
swiftConcurrency
],
plugins: [] + swiftLintPlugin()
),
.testTarget(
Expand All @@ -90,9 +73,6 @@ let package = Package(
.target(name: "SpeziBluetooth"),
.target(name: "SpeziBluetoothServices")
],
swiftSettings: [
swiftConcurrency
],
plugins: [] + swiftLintPlugin()
),
.testTarget(
Expand All @@ -104,9 +84,6 @@ let package = Package(
.product(name: "NIO", package: "swift-nio"),
.product(name: "XCTestExtensions", package: "XCTestExtensions")
],
swiftSettings: [
swiftConcurrency
],
plugins: [] + swiftLintPlugin()
)
]
Expand Down
19 changes: 11 additions & 8 deletions Sources/SpeziBluetooth/Bluetooth.swift
Original file line number Diff line number Diff line change
Expand Up @@ -234,22 +234,24 @@ import Spezi
public final class Bluetooth: Module, EnvironmentAccessible, Sendable {
@Observable
class Storage {
var nearbyDevices: OrderedDictionary<UUID, any BluetoothDevice> = [:]
@MainActor var nearbyDevices: OrderedDictionary<UUID, any BluetoothDevice> = [:]

nonisolated init() {}
}

nonisolated static let logger = Logger(subsystem: "edu.stanford.spezi.bluetooth", category: "Bluetooth")

@SpeziBluetooth private let bluetoothManager = BluetoothManager()
private let bluetoothManager = BluetoothManager()

/// The Bluetooth device configuration.
///
/// Set of configured ``BluetoothDevice`` with their corresponding ``DiscoveryCriteria``.
public nonisolated let configuration: Set<DeviceDiscoveryDescriptor>

// sadly Swifts "lazy var" won't work here with strict concurrency as it doesn't isolate the underlying lazy storage
@SpeziBluetooth private var _lazy_discoveryConfiguration: Set<DiscoveryDescription>?
private var _lazy_discoveryConfiguration: Set<DiscoveryDescription>?
// swiftlint:disable:previous discouraged_optional_collection identifier_name
@SpeziBluetooth private var discoveryConfiguration: Set<DiscoveryDescription> {
private var discoveryConfiguration: Set<DiscoveryDescription> {
guard let discoveryConfiguration = _lazy_discoveryConfiguration else {
let discovery = configuration.parseDiscoveryDescription()
self._lazy_discoveryConfiguration = discovery
Expand Down Expand Up @@ -304,8 +306,10 @@ public final class Bluetooth: Module, EnvironmentAccessible, Sendable {

/// Stores the connected device instance for every configured ``BluetoothDevice`` type.
@Model @MainActor private var connectedDevicesModel = ConnectedDevicesModel()

// we need to manually declare the synthesized property wrapper to avoid https://github.com/swiftlang/swift/issues/76005#issuecomment-2466703851
/// Injects the ``BluetoothDevice`` instances from the `ConnectedDevices` model into the SwiftUI environment.
@Modifier @MainActor private var devicesInjector: ConnectedDevicesEnvironmentModifier
@MainActor private var _devicesInjector: Modifier<ConnectedDevicesEnvironmentModifier>


/// Configure the Bluetooth Module.
Expand All @@ -322,13 +326,12 @@ public final class Bluetooth: Module, EnvironmentAccessible, Sendable {
/// - Parameter devices: The set of configured devices.
@MainActor
public init(
@DiscoveryDescriptorBuilder _ devices: @Sendable () -> Set<DeviceDiscoveryDescriptor>
@DiscoveryDescriptorBuilder _ devices: () -> Set<DeviceDiscoveryDescriptor>
) {
let configuration = devices()
let deviceTypes = configuration.deviceTypes

self.configuration = configuration
self.devicesInjector = ConnectedDevicesEnvironmentModifier(configuredDeviceTypes: deviceTypes)
self._devicesInjector = Modifier(wrappedValue: ConnectedDevicesEnvironmentModifier(from: configuration))

Task { @SpeziBluetooth in
self.observeDiscoveredDevices()
Expand Down
10 changes: 5 additions & 5 deletions Sources/SpeziBluetooth/CoreBluetooth/BluetoothManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ public class BluetoothManager: Observable, Sendable, Identifiable { // swiftlint

/// Currently ongoing discovery session.
private var discoverySession: DiscoverySession?
/// The identifier of the last manually disconnected device.
/// This is to avoid automatically reconnecting to a device that was manually disconnected.
private(set) var lastManuallyDisconnectedDevice: UUID?

/// The list of nearby bluetooth devices.
///
Expand Down Expand Up @@ -197,6 +200,7 @@ public class BluetoothManager: Observable, Sendable, Identifiable { // swiftlint
func cleanupCBCentral() {
_centralManager = nil
isScanningObserver = nil
lastManuallyDisconnectedDevice = nil
logger.debug("Destroyed the underlying CBCentralManager.")
}

Expand Down Expand Up @@ -407,8 +411,6 @@ public class BluetoothManager: Observable, Sendable, Identifiable { // swiftlint

storage.discoveredPeripherals.removeValue(forKey: id)

discoverySession?.clearManuallyDisconnectedDevice(for: id)

checkForCentralDeinit()
}

Expand Down Expand Up @@ -443,8 +445,6 @@ public class BluetoothManager: Observable, Sendable, Identifiable { // swiftlint
func handlePeripheralDeinit(id uuid: UUID) {
storage.retrievedPeripherals.removeValue(forKey: uuid)

discoverySession?.clearManuallyDisconnectedDevice(for: uuid)

checkForCentralDeinit()
}

Expand Down Expand Up @@ -488,7 +488,7 @@ public class BluetoothManager: Observable, Sendable, Identifiable { // swiftlint
// stale timer is handled in the delegate method
centralManager.cancelPeripheralConnection(peripheral.cbPeripheral)

discoverySession?.deviceManuallyDisconnected(id: peripheral.id)
lastManuallyDisconnectedDevice = peripheral.id
}

private func handledConnected(device: BluetoothPeripheral) async {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ import OrderedCollections

@Observable
final class BluetoothManagerStorage: ValueObservable, Sendable {
private let _isScanning = ManagedAtomic<Bool>(false)
private let _state = ManagedAtomic<BluetoothState>(.unknown)
private let _isScanning = ManagedAtomicMainActorBuffered<Bool>(false)
private let _state = ManagedAtomicMainActorBuffered<BluetoothState>(.unknown)

@ObservationIgnored private nonisolated(unsafe) var _discoveredPeripherals: OrderedDictionary<UUID, BluetoothPeripheral> = [:]
private let _discoveredPeripherals: MainActorBuffered<OrderedDictionary<UUID, BluetoothPeripheral>> = .init([:])
private let rwLock = RWLock()

@SpeziBluetooth var retrievedPeripherals: OrderedDictionary<UUID, WeakReference<BluetoothPeripheral>> = [:] {
Expand Down Expand Up @@ -46,20 +46,21 @@ final class BluetoothManagerStorage: ValueObservable, Sendable {

@inlinable var readOnlyDiscoveredPeripherals: OrderedDictionary<UUID, BluetoothPeripheral> {
access(keyPath: \._discoveredPeripherals)
return rwLock.withReadLock {
_discoveredPeripherals
}
return _discoveredPeripherals.load(using: rwLock)
}

@SpeziBluetooth var state: BluetoothState {
get {
readOnlyState
}
set {
withMutation(keyPath: \._state) {
_state.store(newValue, ordering: .relaxed)
let didChange = _state.storeAndCompare(newValue) { @Sendable mutation in
self.withMutation(keyPath: \._state, mutation)
}

if didChange {
_$simpleRegistrar.triggerDidChange(for: \.state, on: self)
}
_$simpleRegistrar.triggerDidChange(for: \.state, on: self)

for continuation in subscribedContinuations.values {
continuation.yield(state)
Expand All @@ -72,10 +73,13 @@ final class BluetoothManagerStorage: ValueObservable, Sendable {
readOnlyIsScanning
}
set {
withMutation(keyPath: \._isScanning) {
_isScanning.store(newValue, ordering: .relaxed)
let didChange = _isScanning.storeAndCompare(newValue) { @Sendable mutation in
self.withMutation(keyPath: \._isScanning, mutation)
}

if didChange {
_$simpleRegistrar.triggerDidChange(for: \.isScanning, on: self) // didSet
}
_$simpleRegistrar.triggerDidChange(for: \.isScanning, on: self) // didSet
}
}

Expand All @@ -84,11 +88,10 @@ final class BluetoothManagerStorage: ValueObservable, Sendable {
readOnlyDiscoveredPeripherals
}
set {
withMutation(keyPath: \._discoveredPeripherals) {
rwLock.withWriteLock {
_discoveredPeripherals = newValue
}
_discoveredPeripherals.store(newValue, using: rwLock) { @Sendable mutation in
self.withMutation(keyPath: \._discoveredPeripherals, mutation)
}

_$simpleRegistrar.triggerDidChange(for: \.discoveredPeripherals, on: self) // didSet
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,6 @@ class DiscoverySession: Sendable {

private var configuration: BluetoothManagerDiscoveryState

/// The identifier of the last manually disconnected device.
/// This is to avoid automatically reconnecting to a device that was manually disconnected.
private(set) var lastManuallyDisconnectedDevice: UUID?

private var autoConnectItem: BluetoothWorkItem?
private(set) var staleTimer: DiscoveryStaleTimer?

Expand Down Expand Up @@ -149,16 +145,6 @@ class DiscoverySession: Sendable {
rssi.intValue >= minimumRSSI && rssi.intValue != 127
}

func deviceManuallyDisconnected(id uuid: UUID) {
lastManuallyDisconnectedDevice = uuid
}

func clearManuallyDisconnectedDevice(for uuid: UUID) {
if lastManuallyDisconnectedDevice == uuid {
lastManuallyDisconnectedDevice = nil
}
}

func deviceDiscoveryPostAction(device: BluetoothPeripheral, newlyDiscovered: Bool) {
if newlyDiscovered {
if staleTimer == nil {
Expand Down Expand Up @@ -204,7 +190,7 @@ extension DiscoverySession {
return nil // auto-connect is disabled
}

guard lastManuallyDisconnectedDevice == nil && !manager.sbHasConnectedDevices else {
guard manager.lastManuallyDisconnectedDevice == nil && !manager.sbHasConnectedDevices else {
return nil
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,17 @@ import CoreBluetooth
import Foundation


struct CharacteristicAccessorCapture: Sendable {
let isNotifying: Bool
let properties: CBCharacteristicProperties

fileprivate init(isNotifying: Bool, properties: CBCharacteristicProperties) {
self.isNotifying = isNotifying
self.properties = properties
}
}


struct GATTCharacteristicCapture: Sendable {
let isNotifying: Bool
let value: Data?
Expand Down Expand Up @@ -69,9 +80,10 @@ public final class GATTCharacteristic {

private let captureLock = RWLock()

var captured: GATTCharacteristicCapture {
captureLock.withReadLock {
GATTCharacteristicCapture(from: self)
var captured: CharacteristicAccessorCapture {
access(keyPath: \.captured)
return captureLock.withReadLock {
CharacteristicAccessorCapture(isNotifying: _isNotifying, properties: properties)
}
}

Expand All @@ -86,7 +98,10 @@ public final class GATTCharacteristic {

@SpeziBluetooth
func synchronizeModel(capture: GATTCharacteristicCapture) {
var shouldNotifyCapture = false

if capture.isNotifying != isNotifying {
shouldNotifyCapture = true
withMutation(keyPath: \.isNotifying) {
captureLock.withWriteLock {
_isNotifying = capture.isNotifying
Expand All @@ -107,6 +122,14 @@ public final class GATTCharacteristic {
}
}
}

if shouldNotifyCapture {
// self is never mutated or even accessed in the withMutation call
nonisolated(unsafe) let this = self
Task { @Sendable @MainActor in
this.withMutation(keyPath: \.captured) {}
}
}
}
}

Expand Down
Loading
Loading