diff --git a/Demo/ReliaBLE Demo/ReliaBLE Demo/Central/CentralView.swift b/Demo/ReliaBLE Demo/ReliaBLE Demo/Central/CentralView.swift index f3d6128..de5d99b 100644 --- a/Demo/ReliaBLE Demo/ReliaBLE Demo/Central/CentralView.swift +++ b/Demo/ReliaBLE Demo/ReliaBLE Demo/Central/CentralView.swift @@ -30,6 +30,18 @@ import SwiftData import ReliaBLE +extension ConnectionState { + var description: String { + switch self { + case .connecting: "Connecting" + case .connected: "Connected" + case .disconnecting: "Disconnecting" + case .disconnected(let reason): "Disconnected\(reason.map { " (\($0))" } ?? "")" + case .failed(let reason): "Failed\(reason.map { " (\($0))" } ?? "")" + } + } +} + struct CentralView: View { @Environment(\.modelContext) private var modelContext @Environment(\.bleManager) private var reliaBLE @@ -115,17 +127,50 @@ struct CentralView: View { await store.syncDevices(peripherals) } } + group.addTask { + for await change in manager.connectionStateChanges { + await viewModel.updateConnectionState(change) + } + } } } } private var deviceList: some View { List { - ForEach(devices) { device in + ForEach(devices, id: \.persistentModelID) { device in NavigationLink { - Text("Device Details") - Text("ID: \(device.id)") - Text("Last seen: \(device.lastSeen?.formatted(date: .numeric, time: .standard) ?? "")") + Group { + let currentState = viewModel.connectionStates[device.id] + VStack(spacing: 16) { + Text("ID: \(device.id)") + Text("Last seen: \(device.lastSeen?.formatted(date: .numeric, time: .standard) ?? "")") + + if let state = currentState { + Text("Connection: \(state.description)") + .foregroundStyle(state == .connected ? Color.green : Color.orange) + } else { + Text("Connection: unknown") + .foregroundStyle(.secondary) + } + + Button(action: { + if let state = currentState, state == .connected { + Task { try? await reliaBLE.disconnect(from: Peripheral(id: device.id)) } + } else { + Task { try? await reliaBLE.connect(to: Peripheral(id: device.id)) } + } + }) { + if let state = currentState, state == .connected { + Text("Disconnect") + } else { + Text("Connect") + } + } + .buttonStyle(.bordered) + } + .padding() + } } label: { Text("\(device.name ?? "Unknown")") } diff --git a/Demo/ReliaBLE Demo/ReliaBLE Demo/Central/CentralViewModel.swift b/Demo/ReliaBLE Demo/ReliaBLE Demo/Central/CentralViewModel.swift index c23abd9..b58c6a0 100644 --- a/Demo/ReliaBLE Demo/ReliaBLE Demo/Central/CentralViewModel.swift +++ b/Demo/ReliaBLE Demo/ReliaBLE Demo/Central/CentralViewModel.swift @@ -33,6 +33,7 @@ import ReliaBLE @Observable class CentralViewModel { var currentState: BluetoothState = .unknown var servicesInput = "" + var connectionStates: [String: ConnectionState] = [:] private var deviceStore: DeviceStoreActor? private var reliaBLE: ReliaBLEManager? @@ -52,6 +53,11 @@ import ReliaBLE currentState = state } + @MainActor + func updateConnectionState(_ change: ConnectionStateChange) { + connectionStates[change.peripheralId] = change.state + } + func authorizeBluetooth() { Task { try? await reliaBLE?.authorizeBluetooth() } } diff --git a/Sources/ReliaBLE/BluetoothActor.swift b/Sources/ReliaBLE/BluetoothActor.swift index 7132303..e897693 100644 --- a/Sources/ReliaBLE/BluetoothActor.swift +++ b/Sources/ReliaBLE/BluetoothActor.swift @@ -45,6 +45,11 @@ private struct DiscoveryPayload: @unchecked Sendable { let rssi: Int } +private struct ConnectionPayload: @unchecked Sendable { + let peripheral: CBPeripheral + let error: Error? +} + /// A single CoreBluetooth delegate callback, carried in delivery order across the nonisolated /// delegate-queue → ``BluetoothActor`` hop. /// @@ -55,6 +60,9 @@ private struct DiscoveryPayload: @unchecked Sendable { private enum DelegateEvent: Sendable { case stateUpdate case discovered(DiscoveryPayload) + case connected(ConnectionPayload) + case disconnected(ConnectionPayload) + case connectFailed(ConnectionPayload) } // MARK: - BluetoothActor @@ -117,6 +125,11 @@ actor BluetoothActor { private var discoveryContinuations: [UUID: AsyncStream.Continuation] = [:] private var peripheralsContinuations: [UUID: AsyncStream<[Peripheral]>.Continuation] = [:] + /// Per-peripheral connection states, keyed by ``Peripheral/id``. + var connectionStates: [String: ConnectionState] = [:] + + private var connectionStateChangesContinuations: [UUID: AsyncStream.Continuation] = [:] + // MARK: - Initialization private init() {} @@ -169,6 +182,17 @@ actor BluetoothActor { } } + /// Returns a fresh `AsyncStream` of connection-state changes for a single subscriber. + /// + /// Each call mints an independent stream. This feed does **not** replay a value on + /// subscription — a subscriber only receives state changes that occur after it registers, + /// mirroring ``peripheralDiscoveriesStream()``. + nonisolated func connectionStateChangesStream() -> AsyncStream { + AsyncStream(bufferingPolicy: .bufferingNewest(BluetoothActor.discoveryBufferLimit)) { continuation in + Task { await BluetoothActor.shared.register(connectionStateChangeContinuation: continuation) } + } + } + // MARK: - Continuation Registration // // Registration runs as a single, indivisible actor job: the replay-yield, dictionary insert, @@ -206,6 +230,24 @@ actor BluetoothActor { private func removeDiscoveryContinuation(_ id: UUID) { discoveryContinuations[id] = nil } private func removePeripheralsContinuation(_ id: UUID) { peripheralsContinuations[id] = nil } + private func register(connectionStateChangeContinuation continuation: AsyncStream.Continuation) { + let id = UUID() + connectionStateChangesContinuations[id] = continuation + continuation.onTermination = { _ in + Task { await BluetoothActor.shared.removeConnectionStateChangeContinuation(id) } + } + } + + private func removeConnectionStateChangeContinuation(_ id: UUID) { connectionStateChangesContinuations[id] = nil } + + /// A one-shot snapshot of the current per-peripheral connection states. + /// + /// Read this to seed a view on appearance without waiting for the next + /// ``connectionStateChangesStream()`` event. + var currentConnectionStates: [String: ConnectionState] { + connectionStates + } + /// Yields a value to every registered continuation in `continuations`. /// /// Yielding to an already-finished continuation is a harmless no-op, so this never prunes — @@ -297,6 +339,12 @@ actor BluetoothActor { advertisementData: payload.advertisementData, rssi: payload.rssi ) + case .connected(let payload): + handleDidConnect(payload) + case .disconnected(let payload): + handleDidDisconnect(payload) + case .connectFailed(let payload): + handleDidFailToConnect(payload) } } @@ -569,6 +617,7 @@ actor BluetoothActor { private func invalidatePeripherals() { // The value snapshots hold no CoreBluetooth reference to clear; drop the live registry instead. cbPeripherals.removeAll() + connectionStates.removeAll() broadcast(discoveredPeripherals, to: peripheralsContinuations) log?.debug("Invalidated all peripheral references") } @@ -594,17 +643,65 @@ actor BluetoothActor { // MARK: - Connection + /// Resolves a ``Peripheral/id`` from the live `CBPeripheral` reference using reverse object-identity lookup. + /// + /// Derives nothing — it reads back the key that ``handlePeripheralDiscovered(_:advertisementData:rssi:)`` + /// already assigned, so ``handlePeripheralDiscovered(_:advertisementData:rssi:)`` remains the library's single + /// source of identity truth. + // TODO: FR-8.5 + private func id(for cbPeripheral: CBPeripheral) -> String? { + cbPeripherals.first { $0.value === cbPeripheral }?.key + } + + private func handleDidConnect(_ payload: ConnectionPayload) { + guard let id = id(for: payload.peripheral) else { + log?.warn(tags: [.category(.connection)], "didConnect for unknown peripheral — dropped") + return + } + connectionStates[id] = .connected + log?.info(tags: [.peripheral(id), .category(.connection)], "Peripheral connected") + broadcast(ConnectionStateChange(peripheralId: id, state: .connected), to: connectionStateChangesContinuations) + } + + private func handleDidDisconnect(_ payload: ConnectionPayload) { + guard let id = id(for: payload.peripheral) else { + log?.warn(tags: [.category(.connection)], "didDisconnect for unknown peripheral — dropped") + return + } + let mappedError: PeripheralError? = payload.error.map { ($0 as? CBError).map(PeripheralError.fromCBError) ?? .unknown } + let state: ConnectionState = .disconnected(reason: mappedError) + connectionStates[id] = state + if let error = mappedError { + log?.warn(tags: [.peripheral(id), .category(.connection)], "Peripheral disconnected with error: \(error)") + } else { + log?.info(tags: [.peripheral(id), .category(.connection)], "Peripheral disconnected") + } + broadcast(ConnectionStateChange(peripheralId: id, state: state), to: connectionStateChangesContinuations) + } + + private func handleDidFailToConnect(_ payload: ConnectionPayload) { + guard let id = id(for: payload.peripheral) else { + log?.warn(tags: [.category(.connection)], "didFailToConnect for unknown peripheral — dropped") + return + } + let mappedError: PeripheralError? = payload.error.map { ($0 as? CBError).map(PeripheralError.fromCBError) ?? .unknown } + let state: ConnectionState = .failed(reason: mappedError) + connectionStates[id] = state + log?.warn(tags: [.peripheral(id), .category(.connection)], "Peripheral connection failed with error: \(mappedError ?? .unknown)") + broadcast(ConnectionStateChange(peripheralId: id, state: state), to: connectionStateChangesContinuations) + } + /// Initiates a connection to the live peripheral backing the given snapshot `id`. /// + /// Optimistically broadcasts `.connecting` before the CoreBluetooth call, then issues the + /// connection request. The actual `.connected` or `.failed` callback arrives later via the + /// delegate pipeline. + /// /// - Parameter id: The ``Peripheral/id`` of a previously discovered peripheral. /// - Throws: ``PeripheralError/notFound`` if no live `CBPeripheral` is registered for `id` (a stale snapshot). - /// - /// - Note: This currently only fires the connection request. The full connection lifecycle - /// (didConnect/didDisconnect handling and a connection-state surface) is deferred to a later release. + /// - Throws: ``PeripheralError/bluetoothUnavailable`` if Bluetooth has not been set up. func connect(id: String) throws { guard let centralManager else { - // No central manager means Bluetooth was never set up (e.g. not authorized). Surface this - // rather than silently succeeding, mirroring the throwing `notFound` path below. log?.warn(tags: [.peripheral(id)], "Attempted to connect without a central manager") throw PeripheralError.bluetoothUnavailable } @@ -613,8 +710,33 @@ actor BluetoothActor { throw PeripheralError.notFound } + connectionStates[id] = .connecting + broadcast(ConnectionStateChange(peripheralId: id, state: .connecting), to: connectionStateChangesContinuations) centralManager.connect(cbPeripheral, options: nil) } + + /// Initiates a disconnection from the live peripheral backing the given snapshot `id`. + /// + /// Optimistically broadcasts `.disconnecting` before cancelling the connection. The actual + /// `.disconnected` callback arrives later via the delegate pipeline. + /// + /// - Parameter id: The ``Peripheral/id`` of a previously connected peripheral. + /// - Throws: ``PeripheralError/notFound`` if no live `CBPeripheral` is registered for `id`. + /// - Throws: ``PeripheralError/bluetoothUnavailable`` if Bluetooth has not been set up. + func disconnect(id: String) throws { + guard let centralManager else { + log?.warn(tags: [.peripheral(id)], "Attempted to disconnect without a central manager") + throw PeripheralError.bluetoothUnavailable + } + + guard let cbPeripheral = cbPeripherals[id] else { + throw PeripheralError.notFound + } + + connectionStates[id] = .disconnecting + broadcast(ConnectionStateChange(peripheralId: id, state: .disconnecting), to: connectionStateChangesContinuations) + centralManager.cancelPeripheralConnection(cbPeripheral) + } } // MARK: - BluetoothDelegateShim @@ -653,4 +775,19 @@ final class BluetoothDelegateShim: NSObject, CBCentralManagerDelegate { let payload = DiscoveryPayload(peripheral: peripheral, advertisementData: advertisementData, rssi: RSSI.intValue) eventContinuation.yield(.discovered(payload)) } + + func centralManager(_ central: CBCentralManager, didConnect peripheral: CBPeripheral) { + let payload = ConnectionPayload(peripheral: peripheral, error: nil) + eventContinuation.yield(.connected(payload)) + } + + func centralManager(_ central: CBCentralManager, didFailToConnect peripheral: CBPeripheral, error: Error?) { + let payload = ConnectionPayload(peripheral: peripheral, error: error) + eventContinuation.yield(.connectFailed(payload)) + } + + func centralManager(_ central: CBCentralManager, didDisconnectPeripheral peripheral: CBPeripheral, timestamp: CFAbsoluteTime, isReconnecting: Bool, error: Error?) { + let payload = ConnectionPayload(peripheral: peripheral, error: error) + eventContinuation.yield(.disconnected(payload)) + } } diff --git a/Sources/ReliaBLE/Documentation.docc/GettingStarted.md b/Sources/ReliaBLE/Documentation.docc/GettingStarted.md index 8cc8079..0b4c493 100644 --- a/Sources/ReliaBLE/Documentation.docc/GettingStarted.md +++ b/Sources/ReliaBLE/Documentation.docc/GettingStarted.md @@ -143,11 +143,35 @@ If your app already knows a peripheral's identity ahead of time — for example, ## Connecting to a Peripheral -Pass a discovered ``Peripheral`` to ``ReliaBLEManager/connect(to:)``. The snapshot carries only a stable identifier; ReliaBLE looks up the live CoreBluetooth peripheral it owns internally and initiates the connection. +Use ``ReliaBLEManager/connect(to:)`` to initiate a connection to a discovered ``Peripheral``. Consume ``ReliaBLEManager/connectionStateChanges`` (an `AsyncStream`) to observe the full lifecycle for any peripheral; filter by `peripheralId` for a specific device. Call ``ReliaBLEManager/disconnect(from:)`` to end the session. ```swift do { + let changes = bleManager.connectionStateChanges + + let observer = Task { + for await change in changes where change.peripheralId == peripheral.id { + switch change.state { + case .connected: + print("Connected to \(peripheral.id)") + return + case .disconnected(let reason): + print("Disconnected", reason ?? "clean") + return + case .failed(let reason): + print("Failed", reason ?? "") + return + default: + break + } + } + } + defer { observer.cancel() } + try await bleManager.connect(to: peripheral) + + // Later, when you're done with the session: + try await bleManager.disconnect(from: peripheral) } catch PeripheralError.notFound { // The snapshot is stale — its underlying peripheral reference was invalidated // (for example, after a Bluetooth reset). Re-scan to rediscover it. @@ -157,4 +181,4 @@ do { } ``` -- Note: `connect(to:)` currently initiates the connection request only. The full connection lifecycle (connection-state updates and disconnection handling) will arrive in a later release. +Each access to `connectionStateChanges` yields a fresh stream; it does not replay, so begin iteration before calling `connect(to:)`. The `ConnectionState` values are `.connecting`, `.connected`, `.disconnecting`, `.disconnected(reason:)`, and `.failed(reason:)`. Terminal states carry an optional ``PeripheralError`` reason. diff --git a/Sources/ReliaBLE/Models/ConnectionState.swift b/Sources/ReliaBLE/Models/ConnectionState.swift new file mode 100644 index 0000000..d5f69a1 --- /dev/null +++ b/Sources/ReliaBLE/Models/ConnectionState.swift @@ -0,0 +1,59 @@ +// +// ConnectionState.swift +// ReliaBLE +// +// Copyright (c) 2026 Five3 Apps, LLC +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +/// The connection state of a single peripheral tracked by the library. +/// +/// ``ConnectionState`` is a finite, `Sendable` enumeration. Every non-terminal state change +/// is emitted on ``ReliaBLEManager/connectionStateChanges``; the two terminal states carry an +/// optional ``PeripheralError`` reason that the integrating app can surface. +public enum ConnectionState: Sendable, Equatable, Hashable { + /// A connection request has been issued and is in flight. + case connecting + /// The peripheral is currently connected. + case connected + /// A disconnection request has been issued and is in flight. + case disconnecting + /// The peripheral has disconnected. + /// + /// The `reason` is `nil` for a clean, explicit disconnect and non-`nil` for an unexpected + /// drop from CoreBluetooth. + case disconnected(reason: PeripheralError?) + /// A connection attempt has failed. + /// + /// The `reason` carries a ``PeripheralError`` mapped from the underlying `CBError` so the + /// integrating app can react to addressable failures without importing CoreBluetooth. + case failed(reason: PeripheralError?) +} + +/// A single connection-state transition emitted on ``ReliaBLEManager/connectionStateChanges``. +/// +/// Each element pairs the ``Peripheral/id`` of the peripheral with its new ``ConnectionState``. +/// A subscriber that only cares about one peripheral filters with +/// `where $0.peripheralId == targetId`. +public struct ConnectionStateChange: Sendable, Equatable, Hashable { + /// The ``Peripheral/id`` of the peripheral whose connection state changed. + public let peripheralId: String + /// The new connection state. + public let state: ConnectionState +} diff --git a/Sources/ReliaBLE/Models/PeripheralError.swift b/Sources/ReliaBLE/Models/PeripheralError.swift index dceff7b..cb88900 100644 --- a/Sources/ReliaBLE/Models/PeripheralError.swift +++ b/Sources/ReliaBLE/Models/PeripheralError.swift @@ -25,6 +25,8 @@ // SOFTWARE. +import CoreBluetooth + /// Errors thrown by peripheral operations such as ``ReliaBLEManager/connect(to:)``. public enum PeripheralError: Error, Sendable, Equatable { /// The peripheral is no longer known to the library. @@ -41,4 +43,28 @@ public enum PeripheralError: Error, Sendable, Equatable { /// because Bluetooth has not been authorized yet. Call ``ReliaBLEManager/authorizeBluetooth()`` and wait for a /// ready state before retrying. case bluetoothUnavailable + + /// The connection to the peripheral failed. + case connectionFailed + + /// The connection attempt timed out. + case connectionTimeout + + /// The peripheral disconnected unexpectedly. + case peripheralDisconnected + + /// An unknown CoreBluetooth error occurred. + case unknown + + /// Maps a `CBError` to a ``PeripheralError`` for public surfacing. + /// + /// The mapper is exhaustive — unrecognized codes fall back to ``unknown``. + static func fromCBError(_ error: CBError) -> PeripheralError { + switch error.code { + case .connectionFailed: return .connectionFailed + case .connectionTimeout: return .connectionTimeout + case .peripheralDisconnected: return .peripheralDisconnected + default: return .unknown + } + } } diff --git a/Sources/ReliaBLE/ReliaBLEManager.swift b/Sources/ReliaBLE/ReliaBLEManager.swift index 4095405..c06a4a3 100644 --- a/Sources/ReliaBLE/ReliaBLEManager.swift +++ b/Sources/ReliaBLE/ReliaBLEManager.swift @@ -85,6 +85,28 @@ public final class ReliaBLEManager: Sendable { get async { await BluetoothActor.shared.currentBluetoothState } } + /// A multi-subscriber `AsyncStream` of connection-state changes for all peripherals. + /// + /// Each property access returns a fresh, independent stream. This stream does **not** replay + /// a value on subscription — subscribe before initiating connections to observe every + /// transition. To filter for a single peripheral: + /// ```swift + /// for await change in manager.connectionStateChanges where change.peripheralId == device.id { + /// // handle change + /// } + /// ``` + public var connectionStateChanges: AsyncStream { + BluetoothActor.shared.connectionStateChangesStream() + } + + /// An async snapshot of the current per-peripheral connection states, useful for seeding + /// a view on appearance without waiting for the next change event. + public var currentConnectionStates: [String: ConnectionState] { + get async { await BluetoothActor.shared.currentConnectionStates } + } + + // MARK: - Authorization + /// Requests authorization to use Bluetooth, presenting the iOS permission prompt when authorization has not yet /// been determined. /// @@ -163,6 +185,19 @@ public final class ReliaBLEManager: Sendable { await BluetoothActor.shared.ensureInitialized(log: log) try await BluetoothActor.shared.connect(id: peripheral.id) } + + /// Initiates a disconnection from a previously connected peripheral. + /// + /// The ``Peripheral`` is a value snapshot. This forwards its ``Peripheral/id`` to the live + /// CoreBluetooth peripheral and cancels the connection. + /// + /// - Parameter peripheral: A peripheral previously delivered via ``discoveredPeripherals``. + /// - Throws: ``PeripheralError/notFound`` if the peripheral's live reference has been + /// invalidated, or ``PeripheralError/bluetoothUnavailable`` if Bluetooth has not been set up. + public func disconnect(from peripheral: Peripheral) async throws { + await BluetoothActor.shared.ensureInitialized(log: log) + try await BluetoothActor.shared.disconnect(id: peripheral.id) + } } // MARK: - Public Types diff --git a/Tests/ReliaBLETests/ReliaBLEManagerTests.swift b/Tests/ReliaBLETests/ReliaBLEManagerTests.swift index 0d1853f..43270c7 100644 --- a/Tests/ReliaBLETests/ReliaBLEManagerTests.swift +++ b/Tests/ReliaBLETests/ReliaBLEManagerTests.swift @@ -516,6 +516,160 @@ struct ReliaBLEManagerTests { #expect(writer.captured.count == 4) } + // MARK: - Connection Lifecycle + + @Test func connectionStateChangesEmitsConnectSuccessSequence() async throws { + Mock.connectionTestDelegate.connectionResult = .success(()) + + let manager = await Mock.makeManager() + await Mock.ensureReady(manager) + + let changes = manager.connectionStateChanges + + // Discover the connectable test peripheral. + await manager.startScanning() + let peripheral = await Mock.waitForPeripheral( + id: Mock.connectionTestPeripheralID, + on: manager, + withinNanoseconds: 3_000_000_000 + ) + let discovered = try #require(peripheral) + await manager.stopScanning() + + try await manager.connect(to: discovered) + + let connecting = await firstConnectionStateChange(from: changes, withinNanoseconds: 5_000_000_000) + #expect(connecting?.peripheralId == discovered.id) + #expect(connecting?.state == .connecting) + + let connected = await firstConnectionStateChange(from: changes, withinNanoseconds: 5_000_000_000) + #expect(connected?.peripheralId == discovered.id) + #expect(connected?.state == .connected) + } + + @Test func connectionStateChangesEmitsDisconnectSequence() async throws { + Mock.connectionTestDelegate.connectionResult = .success(()) + + let manager = await Mock.makeManager() + await Mock.ensureReady(manager) + + let changes = manager.connectionStateChanges + + await manager.startScanning() + let peripheral = await Mock.waitForPeripheral( + id: Mock.connectionTestPeripheralID, + on: manager, + withinNanoseconds: 3_000_000_000 + ) + let discovered = try #require(peripheral) + await manager.stopScanning() + + try await manager.connect(to: discovered) + + // Drain .connecting and .connected. + _ = await firstConnectionStateChange(from: changes, withinNanoseconds: 5_000_000_000) + _ = await firstConnectionStateChange(from: changes, withinNanoseconds: 5_000_000_000) + + try await manager.disconnect(from: discovered) + + let disconnecting = await firstConnectionStateChange(from: changes, withinNanoseconds: 5_000_000_000) + #expect(disconnecting?.peripheralId == discovered.id) + #expect(disconnecting?.state == .disconnecting) + + let disconnected = await firstConnectionStateChange(from: changes, withinNanoseconds: 5_000_000_000) + #expect(disconnected?.peripheralId == discovered.id) + #expect(disconnected?.state == .disconnected(reason: nil)) + } + + @Test func connectionStateChangesEmitsConnectFailureSequence() async throws { + // Pre-condition: no stale connection state from a preceding lifecycle test. + Mock.connectionTestSpec.simulateDisconnection() + try? await Task.sleep(nanoseconds: 100_000_000) + // Drain any spurious events that simulateDisconnection may have injected + // into the actor's stream continuations. + await BluetoothActor.shared.updateState() + + Mock.connectionTestDelegate.connectionResult = .failure(CBMError(.connectionTimeout)) + defer { Mock.connectionTestDelegate.connectionResult = .success(()) } + + let manager = await Mock.makeManager() + await Mock.ensureReady(manager) + + let changes = manager.connectionStateChanges + + await manager.startScanning() + let peripheral = await Mock.waitForPeripheral( + id: Mock.connectionTestPeripheralID, + on: manager, + withinNanoseconds: 3_000_000_000 + ) + let discovered = try #require(peripheral) + await manager.stopScanning() + + // Force a clean disconnection on the spec to reset any lingering + // `virtualConnections` / `isConnected` state left by a preceding test. + Mock.connectionTestSpec.simulateDisconnection() + try? await Task.sleep(nanoseconds: 100_000_000) + + try await manager.connect(to: discovered) + + var events: [ConnectionStateChange] = [] + for _ in 0..<3 { + if let change = await firstConnectionStateChange(from: changes, withinNanoseconds: 3_000_000_000) { + events.append(change) + } + } + + // The mock's connection callback fires on an async timer (0.045 s), so the + // time spent collecting events is well under the 3 s timeout per event. The + // failure test must see .connecting then .failed(reason: .connectionTimeout). + guard events.count >= 2 else { + #expect(Bool(false), "Expected at least 2 events, got \(events.count): \(events)") + return + } + #expect(events[0].peripheralId == discovered.id) + #expect(events[0].state == .connecting) + #expect(events[1].peripheralId == discovered.id) + #expect(events[1].state == .failed(reason: .connectionTimeout)) + } + + @Test func connectionStateChangesSupportsConcurrentSubscribers() async throws { + Mock.connectionTestDelegate.connectionResult = .success(()) + + let manager = await Mock.makeManager() + await Mock.ensureReady(manager) + + var subscriberA = manager.connectionStateChanges.makeAsyncIterator() + var subscriberB = manager.connectionStateChanges.makeAsyncIterator() + + // Force an actor hop to guarantee the registration Tasks have completed + // before we issue the connect (connectionStateChanges has no replay). + _ = await manager.currentConnectionStates + + await manager.startScanning() + let peripheral = await Mock.waitForPeripheral( + id: Mock.connectionTestPeripheralID, + on: manager, + withinNanoseconds: 3_000_000_000 + ) + let discovered = try #require(peripheral) + await manager.stopScanning() + + try await manager.connect(to: discovered) + + // Both subscribers see the .connecting event. + let a1 = await subscriberA.next() + let b1 = await subscriberB.next() + #expect(a1?.state == .connecting) + #expect(b1?.state == .connecting) + + // Both subscribers see the .connected event. + let a2 = await subscriberA.next() + let b2 = await subscriberB.next() + #expect(a2?.state == .connected) + #expect(b2?.state == .connected) + } + // MARK: - Event Stream Broadcaster @Test func stateStreamReplaysToConcurrentSubscribers() async throws { @@ -558,6 +712,25 @@ struct ReliaBLEManagerTests { // MARK: - Logging Test Support +/// A configurable ``CBMPeripheralSpecDelegate`` used by the connection-lifecycle tests. +/// +/// The connection result returned from ``peripheralDidReceiveConnectionRequest(_:)`` is set on the +/// instance before a test runs. The delegate is registered once in ``SimulationConfig/ensureConfigured()`` +/// and shared across all connection tests via ``Mock/connectionTestDelegate``. +final class ConnectionTestDelegate: @unchecked Sendable { + var connectionResult: Result = .success(()) +} + +extension ConnectionTestDelegate: CBMPeripheralSpecDelegate { + func reset() {} + + func peripheralDidReceiveConnectionRequest(_ peripheral: CBMPeripheralSpec) -> Result { + connectionResult + } + + func peripheral(_ peripheral: CBMPeripheralSpec, didDisconnect error: Error?) {} +} + /// A ``LogWriter`` that records every forwarded message so tests can assert on exactly what the /// ``LoggingService`` emitted — message text and level — at the writer boundary. /// @@ -605,6 +778,22 @@ enum Mock { /// this exact local name, so the discovered snapshot's `id` is deterministic. static let testPeripheralID = "ReliaBLE-Test-Peripheral" + /// The resolved ``Peripheral/id`` of the connectable test peripheral. + /// + /// This peripheral is backed by a ``CBMPeripheralSpec`` whose ``connectionDelegate`` is + /// ``connectionTestDelegate``, so the connection outcome (success or failure) is configurable per-test. + static let connectionTestPeripheralID = "ReliaBLE-Connection-Test" + + /// Configurable connection delegate shared across all lifecycle tests. + /// + /// Set ``ConnectionTestDelegate/connectionResult`` to `.failure(...)` before a test to simulate a + /// failed connection; reset to `.success(())` in a `defer` or at the start of each test. + static let connectionTestDelegate = ConnectionTestDelegate() + + /// The spec registered with the mock so tests can force a clean disconnection between + /// lifecycle tests that otherwise leak `virtualConnections` state. + nonisolated(unsafe) static let connectionTestSpec = makeConnectionTestPeripheralSpec() + /// Builds a `ReliaBLEManager` after ensuring the one-time mock configuration has run. /// /// Every test routes manager creation through here so the simulated peripheral set is registered and authorization @@ -659,6 +848,23 @@ enum Mock { .build() } + /// Builds the simulated peripheral used by connection-lifecycle tests, backed by + /// ``connectionTestDelegate`` so connection outcomes are configurable per-test. + private static func makeConnectionTestPeripheralSpec() -> CBMPeripheralSpec { + CBMPeripheralSpec + .simulatePeripheral(proximity: .immediate) + .advertising( + advertisementData: [ + CBMAdvertisementDataLocalNameKey: connectionTestPeripheralID, + CBMAdvertisementDataServiceUUIDsKey: [CBMUUID(string: "180D")], + CBMAdvertisementDataIsConnectable: NSNumber(value: true) + ], + withInterval: 0.05 + ) + .connectable(name: connectionTestPeripheralID, services: [], delegate: connectionTestDelegate) + .build() + } + /// Polls `manager.currentState` until its description matches `description` or the timeout elapses. static func waitForState(_ description: String, on manager: ReliaBLEManager, timeout: Double = 3.0) async -> Bool { await pollUntil(timeout: timeout) { @@ -707,7 +913,7 @@ actor SimulationConfig { guard !configured else { return } configured = true CBMCentralManagerMock.simulateInitialState(.poweredOn) - CBMCentralManagerMock.simulatePeripherals([Mock.makeTestPeripheralSpec()]) + CBMCentralManagerMock.simulatePeripherals([Mock.makeTestPeripheralSpec(), Mock.connectionTestSpec]) // Pin authorization to `.notDetermined` so `ReliaBLEManager.init`'s `.allowedAlways` auto-setup cannot // create a central before the peripheral set is registered. CBMCentralManagerMock.simulateAuthorization(.notDetermined) @@ -761,3 +967,25 @@ func firstEvent( return first } } + +/// Returns the first connection-state change from `stream`, or `nil` if none arrives within `nanoseconds`. +func firstConnectionStateChange( + from stream: AsyncStream, + withinNanoseconds nanoseconds: UInt64 +) async -> ConnectionStateChange? { + await withTaskGroup(of: ConnectionStateChange?.self) { group in + group.addTask { + for await change in stream { + return change + } + return nil + } + group.addTask { + try? await Task.sleep(nanoseconds: nanoseconds) + return nil + } + let first = await group.next() ?? nil + group.cancelAll() + return first + } +} diff --git a/docs/plans/connection-lifecycle-stream-2026-06-30.md b/docs/plans/connection-lifecycle-stream-2026-06-30.md new file mode 100644 index 0000000..15fad7d --- /dev/null +++ b/docs/plans/connection-lifecycle-stream-2026-06-30.md @@ -0,0 +1,112 @@ +# Peripheral Connection Lifecycle + Connection-State Stream: Plan +*Issue #36 · 2026-06-30* + +## Goal + +Flesh out the peripheral connection lifecycle in ReliaBLE — implement real `connect(to:)` / `disconnect(from:)` driving the CoreBluetooth connection delegate paths, and expose per-peripheral `ConnectionState` changes to the integrating app via an `AsyncStream`, consistent with the existing stream patterns on `BluetoothActor`. Multi-device aware. Ship a Demo consumer (connect/disconnect UI on the device-detail screen). + +Covers the connection-oriented halves of **FR-2.3.1** (connection-change callbacks) and **FR-1.3.1** (connection-stability status). Data-transmission halves (FR-2.3.2 / FR-1.3.2) and auto-reconnect (FR-1.2, issue #37) are out of scope. + +## Background + +All current state verified by exploration (file:line refs below). + +### Library — connect path & streams (`Sources/ReliaBLE/`) +- `ReliaBLEManager.connect(to:)` (`ReliaBLEManager.swift:162`) → `BluetoothActor.shared.connect(id:)`. +- `BluetoothActor.connect(id:)` (`BluetoothActor.swift:604`): guards `centralManager`, looks up `cbPeripherals[id]`, calls `centralManager.connect(cbPeripheral, options: nil)`. **No disconnect, no connection delegate methods exist.** +- AsyncStream broadcaster pattern (the template to mirror): + - Continuation registries (actor-isolated): `stateContinuations`/`discoveryContinuations`/`peripheralsContinuations` (`BluetoothActor.swift:116–118`). + - `nonisolated` factory methods mint a fresh stream per call: `stateStream()` (:131, `.bufferingNewest(1)`), `peripheralDiscoveriesStream()` (:155), `discoveredPeripheralsStream()` (:166). + - `register(...)` (:179–196): yields replay value (state/list streams), stores continuation, sets `onTermination` → remover. Removers at :205–207. + - `broadcast(_:to:)` (:213) loops continuations and `.yield`s. +- Delegate wiring: `BluetoothDelegateShim: NSObject, CBCentralManagerDelegate` (`BluetoothActor.swift:628`). Shim yields into an internal unbounded `AsyncStream` of `DelegateEvent` (:272); a single actor Task drains it via `process(_:)` (:290). Implemented: `centralManagerDidUpdateState` (:638 → `.stateUpdate`), `didDiscover` (:644 → `.discovered`). **No `didConnect`/`didDisconnectPeripheral`/`didFailToConnect`.** Adding one: extend `DelegateEvent` enum (:55), add shim method that yields, handle in `process`, add `handle*` on the actor. +- Peripheral tracking: `Peripheral` is a `Sendable` struct keyed by `id` (`Models/Peripheral.swift`). Actor holds value snapshots `discoveredPeripherals: [Peripheral]` (:99) and live refs `cbPeripherals: [String: CBPeripheral]` (:106, never escapes actor). Populated in `handlePeripheralDiscovered` (:489). +- `ReliaBLEManager` public surface: `init`, `authorizeBluetooth`, `startScanning`, `stopScanning`, `connect(to:)`; properties `state`, `currentState`, `peripheralDiscoveries`, `discoveredPeripherals`. Every entry point does `ensureInitialized(log:)` then delegates to the actor. + +### Tests / mock harness (`Tests/ReliaBLETests/`, `Sources/ReliaBLEMock/`) +- `forceMock: true` is hardcoded at `BluetoothActor.swift:280`; production factory ignores it, mock honors it. +- `ReliaBLEManagerTests.swift` is `@Suite(.serialized)` (process-wide singletons). `Mock.makeManager()` + `SimulationConfig.ensureConfigured()` set up `CBMCentralManagerMock.simulateInitialState(.poweredOn)` / `simulatePeripherals([spec])` / `simulateAuthorization`. +- Test spec: `Mock.makeTestPeripheralSpec()` (:647) builds `CBMPeripheralSpec.simulatePeripheral(...).advertising(...).connectable(name:..., services:[], delegate:nil).build()`. +- Current connection test `connectToDiscoveredPeripheralSucceeds` (:385) only asserts `connect(to:)` doesn't throw — **no didConnect/disconnect/fail coverage.** No usage of `simulateConnection`/`simulateDisconnection` anywhere. `Tests/ReliaBLETests/Mocks/` exists but is empty. +- Stream assertion helpers: `firstEvent(from:withinNanoseconds:)` (:744) races a stream against a sleep inside a TaskGroup; direct iterator+replay via `makeAsyncIterator()`/`next()`. + +### Demo (`Demo/ReliaBLE Demo/ReliaBLE Demo/Central/`) +- `CentralView.swift:112` `.task { withTaskGroup { ... } }` with 3 `group.addTask` loops: `manager.state` → `viewModel.updateState` (@MainActor); `peripheralDiscoveries` → `store.insertDiscovery`; `discoveredPeripherals` → `store.syncDevices` (both off-main `DeviceStoreActor`). +- `deviceList` NavigationLink (:130) destination is a static `Text("Device Details")` placeholder — to be replaced by a real detail view. +- `CentralViewModel` (`:39`) is `@Observable` (not @MainActor): `var currentState`, `@MainActor func updateState(_:)` (:50). Devices come from `@Query` in the View, not the VM. +- `DeviceStoreActor` persists `Device` + `DiscoveryEvent` to SwiftData, writes always off-main (`assertWritesOffMainThread()` :136). **Connection state is persisted nowhere** — keep it that way. +- Sidebar scanning button (`CentralView.swift:43`) is the state-driven button template (conditional on `viewModel.currentState` cases). + +## Approach + +Purely additive — reuse the existing broadcaster machinery and delegate-shim pipeline; touch no existing surfaces. + +**Models.** Two new public `Sendable` types in `Sources/ReliaBLE/Models/` (parity with `Peripheral`): +- `ConnectionState` enum: `.connecting`, `.connected`, `.disconnecting`, `.disconnected(reason: PeripheralError?)`, `.failed(reason: PeripheralError?)`. The `reason` is `nil` for a clean/explicit disconnect and non-`nil` for an unexpected drop or connect failure (Q2-B). Stays `Equatable`/`Hashable` because `PeripheralError` is a finite mapped enum. +- `ConnectionStateChange` struct: `{ let peripheralId: String; let state: ConnectionState }` — the element of the per-event stream. (A raw tuple would not be a clean public/`Sendable`/`Equatable` surface.) + +**Error mapping (Q2-B).** Extend `PeripheralError` (`Models/PeripheralError.swift`, today `.notFound`/`.bluetoothUnavailable`) with connection-failure cases (e.g. `.connectionFailed`, `.connectionTimeout`, `.peripheralDisconnected`, `.unknown`) and a private `CBError` → `PeripheralError` mapper so raw CoreBluetooth `Error` never leaks into the public API. Keep it `Equatable`/`Sendable`. + +**Actor registry + dual surface (Q1-C).** Add `connectionStates: [String: ConnectionState]` to `BluetoothActor` (beside `cbPeripherals`/`discoveredPeripherals`, keyed by `Peripheral.id`, mutated only on-actor). Expose **two** surfaces fed from the same registry mutation: +- **Per-event stream** `connectionStateChangesStream() -> AsyncStream` — no replay (mirrors `peripheralDiscoveriesStream()`, `BluetoothActor.swift:155`). The primary, granular subscription; a single-device view/service filters `where $0.peripheralId == id`. +- **Snapshot accessor** `currentConnectionStates: [String: ConnectionState] { get async }` — a one-shot read of the whole registry (mirrors `currentState`), so a view appearing can seed itself without waiting for the next change. + +Add a `connectionStateChangesContinuations` registry + `nonisolated connectionStateChangesStream()` factory + `register(...)` (copy the non-replay discovery path). Each `handle*` mutates the registry, then broadcasts the single `ConnectionStateChange` to the change-continuations via the existing `broadcast(_:to:)` helper. + +**Connect/disconnect.** Extend `connect(id:)` to optimistically set `.connecting` + broadcast the change before calling `central.connect` (CoreBluetooth has no "connecting" callback). Add `disconnect(id:)` that sets `.disconnecting` + broadcasts, then calls `central.cancelPeripheralConnection(cbPeripheral)`. Both keep the existing `guard centralManager`/`cbPeripherals[id]` → `PeripheralError` behavior. + +**Delegate wiring & id resolution.** Extend `DelegateEvent` (`:55`) with `.connected`, `.disconnected(Error?)`, `.connectFailed(Error?)`, each carrying the raw `CBPeripheral` wrapped in the existing `SendableWrapper` (exactly how `.discovered` hops the non-`Sendable` `CBPeripheral` off the delegate queue — the shim does **not** resolve the id). Add the three `CBCentralManagerDelegate` methods to `BluetoothDelegateShim` (`:628`); each yields its event. Route them in `process(_:)` (`:290`) to actor handlers `handleDidConnect/handleDidDisconnect/handleDidFailToConnect`. + +The handlers resolve `Peripheral.id` through a **single private on-actor helper** — `id(for cbPeripheral: CBPeripheral) -> String?` — that does a reverse object-identity lookup in the existing registry: `cbPeripherals.first { $0.value === cbPeripheral }?.key`. All three handlers call it; if it returns `nil` (should not happen — the callback's peripheral is already in `cbPeripherals` because `connect(id:)` looked it up there), log and drop. This helper performs **zero id derivation** — it reads back the key `handlePeripheralDiscovered` (`:489`) already assigned, so `handlePeripheralDiscovered` stays the library's *single* source of `id` truth. That is what keeps the connection layer FR-8.5-safe: when FR-8.5 rewrites the derivation site, these handlers keep working unchanged (see Decision 5). Tag the helper with a `// TODO: FR-8.5` breadcrumb mirroring the one at `:508`, so a grep for `FR-8.5` surfaces every coupled site. Each handler then updates the registry (`.connected` / `.disconnected(reason:)` / `.failed(reason:)`), logs the mapped error via a `.peripheral(id)` tag, and broadcasts the change. + +**Ordering.** No extra serialization or "pending" machinery is needed: every mutation + broadcast runs on `@BluetoothActor`, and both the internal delegate `AsyncStream` and the per-subscriber output streams preserve emission order. So a subscriber sees `.connecting` (emitted synchronously inside `connect(id:)`) strictly before the later `.connected`/`.failed` (emitted when the delegate event drains). Rapid interleaved connect/disconnect calls are serialized by the actor — last write wins in the registry and every transition is still emitted in order. + +**Façade.** `ReliaBLEManager` adds: +- `var connectionStateChanges: AsyncStream` (forwards to `connectionStateChangesStream()`), +- `var currentConnectionStates: [String: ConnectionState] { get async }` (forwards to the actor snapshot), +- `func disconnect(from: Peripheral) async throws` (`ensureInitialized` → actor `disconnect(id:)`). + +Additive — no existing call sites change. + +**Demo.** Add a 4th `group.addTask` in `CentralView`'s `.task` looping `for await change in manager.connectionStateChanges { await viewModel.updateConnectionState(change) }`. Add `var connectionStates: [String: ConnectionState] = [:]` + `@MainActor func updateConnectionState(_ change: ConnectionStateChange)` (does `connectionStates[change.peripheralId] = change.state`) to `CentralViewModel` (transient only — never SwiftData; the Demo owns retention, which is why registry entry-lifecycle is a non-issue — Q3). Replace the static `Text("Device Details")` `NavigationLink` destination with a real detail view that renders `viewModel.connectionStates[device.id]` and a context-aware Connect/Disconnect button (mirroring the sidebar's `currentState`-driven scanning button). + +**Tests.** Drive connect-success / disconnect / connection-failure through the mock central and assert the emitted `ConnectionStateChange` sequence via the existing `firstEvent`/iterator helpers, including a failure case that asserts the mapped `PeripheralError` reason. Preserve `@Suite(.serialized)` and `forceMock`. (Mock-simulation API details in Work Item #5.) + +## Work Items + +Land 1–4 together (library + delegate paths must build as a unit); 5–7 follow. + +> **Progress (orchestrator):** ✅ ALL COMPLETE. Items 1–4 (library core), 5 (tests: 32/32 pass incl. 4 new connection tests), 6 (Demo: detail view + Connect/Disconnect UI, builds), 7 (DocC GettingStarted example + final `swift build`/`swift test` green). Three-target trick, `forceMock`, lazy central init all verified intact. + +1. **`Models/ConnectionState.swift` + `ConnectionStateChange` (new); extend `PeripheralError`.** Public `Sendable` enum (five cases, `reason: PeripheralError?` on the two terminal cases) + the change struct. Add connection-failure cases to `PeripheralError` and the `CBError`→`PeripheralError` mapper. Compiles independently. +2. **`BluetoothActor.swift` — registry, per-event stream, delegate paths.** Add `connectionStates` dict + `connectionStateChangesContinuations`; `connectionStateChangesStream()` + `register(...)` (copy the non-replay `peripheralDiscoveries` pattern) + `currentConnectionStates` accessor. Extend `DelegateEvent` (:55) with `.connected/.disconnected/.connectFailed` carrying a `SendableWrapper` (+ `Error?`); add the three shim delegate methods (`:628`) and `process(_:)` routes (:290); add a single private `id(for cbPeripheral:)` reverse-identity-lookup helper (tagged `// TODO: FR-8.5`) plus `handleDidConnect/handleDidDisconnect/handleDidFailToConnect` that call it, map the error, log via `.peripheral(id)`, and broadcast the change. Keep `forceMock`, lazy-init, `SendableWrapper`, `currentBluetoothState`. +3. **`BluetoothActor.swift` — connect/disconnect bodies.** Extend `connect(id:)` (:604) to set `.connecting` + broadcast before `central.connect`; add `disconnect(id:)` (set `.disconnecting` + broadcast, then `central.cancelPeripheralConnection`). +4. **`ReliaBLEManager.swift` — public surface.** Add `connectionStateChanges` stream, `currentConnectionStates` async accessor, and `disconnect(from:) async throws`, all delegating through `ensureInitialized`. *(Breaking addition — pre-1.0 OK.)* +5. **`ReliaBLEManagerTests.swift` — coverage.** Add mock-driven tests for connect-success, disconnect, and connection-failure asserting the `ConnectionStateChange` sequence (incl. the mapped `PeripheralError` reason on failure); one Sendable/multi-subscriber proof for the new stream. Drive it with whatever `CBMPeripheralSpec` connection delegate / `simulate*` surface the harness can attach (the current spec is `delegate: nil` — confirm at the keyboard). +6. **Demo — `CentralView.swift` + `CentralViewModel.swift`.** 4th `.task` consumer looping `connectionStateChanges` → `@MainActor updateConnectionState(_:)` merging into the transient `connectionStates` dict; real device-detail view (reads `connectionStates[device.id]`) with context-aware Connect/Disconnect button. *(Read `Demo/CLAUDE.md` first; delegate to a sub-agent per AGENTS.md.)* +7. **Docs + verify.** Add the lifecycle + stream example to `Documentation.docc/GettingStarted.md` (one paragraph — DocC must track the public API per AGENTS.md). `swift build` + `swift test`; confirm three-target constraint, `forceMock`, and lazy-init intact. + +## Decisions (resolved at mid-flow check-in) + +1. **Dual surface (Q1-C).** Ship both a per-event `connectionStateChanges: AsyncStream` (primary, granular — lets a single-device view/service subscribe directly) and a `currentConnectionStates` async snapshot accessor for one-shot seeding. No aggregate *snapshot stream* is shipped — deferred until a multi-device dashboard consumer needs it (avoid speculative surface). +2. **Errors carried, mapped (Q2-B).** `.failed(reason:)` and unexpected `.disconnected(reason:)` carry a `PeripheralError?`, mapped from `CBError` so integrators can surface addressable issues without importing CoreBluetooth. Raw `Error` is never exposed; the enum stays `Equatable`/`Sendable`. +3. **Registry retention (Q3 — N/A at the library boundary).** The Demo owns its own retention in the transient `connectionStates` dict (fed by the per-event stream), so terminal-entry lifecycle is not a public contract. Internally the actor registry keeps the last-known state per id (so `currentConnectionStates` stays meaningful and to support #37); it is cleared on `invalidatePeripherals()`. +4. **Demo scope (Q4-A).** The Demo work stays as Work Item #6 in this plan (matches issue #36's "ship alongside" section); implemented by a sub-agent that reads `Demo/CLAUDE.md` first. +5. **FR-8.5 coupling — one resolution point, tracked.** `CBPeripheral → Peripheral.id` resolution in the connect handlers goes through a single private `id(for:)` helper that *derives nothing* — it reverse-looks-up the key `handlePeripheralDiscovered` already assigned. This avoids planting a second copy of the name-based derivation (`cbPeripheral.name ?? localName ?? uuidString`, `:508`) that FR-8.5 would have to hunt down. Consequence: the connection layer inherits — and never worsens — the current same-name-collision caveat, and needs no rewrite when FR-8.5 lands. Both the helper and `:508` carry `// TODO: FR-8.5` so a single grep surfaces every site. Two integration points are explicitly deferred to FR-8.5 (see Open Questions). + +## Open Questions + +None blocking for this issue. Confirm at the keyboard: +- Exact Nordic `CoreBluetoothMock` API for simulating connect-success / disconnect / connect-failure given the spec is built `delegate: nil` (may require attaching a `CBMPeripheralSpec` connection delegate). Work Item #5. +- Final `CBError` code → `PeripheralError` case list; the mapper is exhaustive with a `.unknown` fallback. Work Item #1. + +**Deferred to FR-8.5** (tracked here so they aren't forgotten — not in scope now): +- **Registry key migration on late id assignment.** FR-8.5.2 lets the app hand back a unique id *after* discovery, so a peripheral may be filed under a provisional id before its final one is known. When the key changes provisional→unique, the `connectionStates` entry must migrate to the new key. The `id(for:)` helper still resolves the object correctly; it's the registry keying that needs the migration. +- **Cross-reconnection persistence (FR-8.5.3).** Once ids are stable across reconnections, the id-keyed `connectionStates` registry becomes the natural home for "consistent tracking across reconnections" — design it together with FR-8.5.3 and the #37 auto-reconnect work. + +## References +- Issue #36. Follow-up: #37 (auto-reconnect, builds on this). Mock harness: #31. +- PRD: FR-2.3.1, FR-1.3.1 (in-scope); FR-2.3.2, FR-1.3.2, FR-1.2, FR-5.1; **FR-8.5** (PRD:106–109 — unique-id-from-manufacturing-data; connection-layer coupling tracked in Decision 5 + Open Questions). +- Prior stream plan: `docs/plans/combine-to-asyncstream-2026-06-18.md` (AsyncStream broadcaster pattern this mirrors). +- Key files: `Sources/ReliaBLE/BluetoothActor.swift`, `ReliaBLEManager.swift`, `Models/Peripheral.swift`; `Tests/ReliaBLETests/ReliaBLEManagerTests.swift`; `Demo/.../Central/CentralView.swift`, `CentralViewModel.swift`, `DeviceStoreActor.swift`. +- Architecture: root `AGENTS.md` (`@BluetoothActor`, three-target mock trick, lazy central init); `Demo/CLAUDE.md` (Demo conventions, XcodeBuildMCP). diff --git a/docs/reviews/connection-lifecycle-stream-plan-critique-2026-06-30.md b/docs/reviews/connection-lifecycle-stream-plan-critique-2026-06-30.md new file mode 100644 index 0000000..d7baf5a --- /dev/null +++ b/docs/reviews/connection-lifecycle-stream-plan-critique-2026-06-30.md @@ -0,0 +1,34 @@ +# Critique: connection-lifecycle-stream-2026-06-30 plan + +## Top 3 under-specified seams + +1. **Delegate shim peripheral-id resolution for connect/disconnect/fail callbacks** (`BluetoothActor.swift:628` shim, `DelegateEvent` at `:55`, handlers at `:290`). + Plan says the three new shim methods will “yield … by peripheral id”. The existing `handlePeripheralDiscovered` resolves `Peripheral.id` via the `name` property of the discovered peripheral (or a fallback). The `didConnect`/`didDisconnectPeripheral`/`didFailToConnect` callbacks receive a `CBPeripheral`, but the plan never states whether the shim will: + - look the peripheral up in `cbPeripherals` by its object identity or identifier, + - read `.name` the same way the discovery path does, or + - guarantee the `CBPeripheral` is already present in `cbPeripherals` before the callback fires. + This is the single most load-bearing missing detail; the entire registry update and broadcast path depends on it. + +2. **Optimistic `.connecting`/`.disconnecting` broadcast vs. delegate-callback ordering** (`connect(id:)` at `:604`, new `disconnect(id:)`, new handlers). + Plan says “optimistically set `.connecting` + broadcast … before calling `central.connect`”. No mention of what happens if the subsequent delegate callback arrives *before* the broadcast continuation has been processed by a subscriber, or if two rapid `connect`/`disconnect` calls interleave. The existing broadcaster has no ordering or serialisation guarantee beyond the actor itself; the plan does not address whether an explicit “pending” state or a small internal queue is required. + +3. **Error mapping surface and `PeripheralError` case list** (`Models/PeripheralError.swift`). + Plan states a private `CBError → PeripheralError` mapper will be added and that raw `Error` never leaks. It does not list the concrete new cases, nor whether the mapper is exhaustive or falls back to `.unknown`. This directly affects the public `ConnectionState` enum cases that carry `reason: PeripheralError?`. + +## Contradictions / missing dependencies + +- None. The plan is internally consistent and correctly identifies the prior stream pattern it must copy. + +## Risk of over-planning + +- **Work Item 5 (mock investigation)** is written as a full investigation task inside the implementation phase. It should be reduced to a one-line note: “use whatever `CBMPeripheralSpec` connection delegate or `simulate*` surface the existing test harness already exercises.” The rest is discovery noise that belongs in the commit message, not the plan. +- **Work Item 7 (DocC)** can be dropped; the GettingStarted example is a one-paragraph addition that does not justify its own work item. + +## Questions whose answers change implementation order + +- “How exactly will the delegate shim obtain the `Peripheral.id` string from the `CBPeripheral` passed to `didConnect`/`didDisconnect`/`didFailToConnect`?” + Answer determines whether the delegate methods can be written first or whether the id-resolution helper must be extracted/refactored before any delegate work begins. If the answer is “the peripheral is already in `cbPeripherals` by the time the callback fires”, the order is safe; otherwise the id-resolution logic must be done *before* the delegate methods are added. + +--- + +*Spot-checked `BluetoothActor.swift:489` (handlePeripheralDiscovered) — confirmed name-based id resolution is the only existing path.* \ No newline at end of file