Skip to content

Commit baa0896

Browse files
itsniperclaude
andcommitted
Address PR #29 review: bound discovery buffer, fix init race, order delegate callbacks, suspend authorize
- peripheralDiscoveriesStream: bound buffer to .bufferingNewest(10_000) (~10MB cap) instead of unbounded, so a stalled subscriber drops oldest events rather than growing memory without limit. - ensureInitialized(log:): idempotent actor setup that every public ReliaBLEManager entry point funnels through, closing the init-vs-immediate-call race. It also (re)creates the central manager on any entry when already .allowedAlways, so operations work after out-of-band authorization while preserving the lazy-permission contract. - Ordered delegate handling: BluetoothDelegateShim yields each callback into a single AsyncStream<DelegateEvent> drained by one consumer task, preserving CoreBluetooth's serial callback order (replaces per-callback Tasks that could reorder). - Suspending authorize: .notDetermined now suspends until centralManagerDidUpdateState resolves the decision; cancellation is wired via withTaskCancellationHandler in the nonisolated ReliaBLEManager facade, keeping the actor method clear of a construct the Swift 6.1 region-isolation checker cannot analyze. - connect(to:) throws PeripheralError.bluetoothUnavailable (new case; enum now Equatable) when no central manager exists, instead of silently returning. - Expanded the FR-8.5 TODO documenting the same-name peripheral identity collision. - Tests updated for the new behaviors (cancellable authorize, broadened connect error). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 42edd51 commit baa0896

4 files changed

Lines changed: 270 additions & 53 deletions

File tree

Sources/ReliaBLE/BluetoothActor.swift

Lines changed: 189 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,18 @@ private struct DiscoveryPayload: @unchecked Sendable {
4545
let rssi: Int
4646
}
4747

48+
/// A single CoreBluetooth delegate callback, carried in delivery order across the nonisolated
49+
/// delegate-queue → ``BluetoothActor`` hop.
50+
///
51+
/// CoreBluetooth invokes delegate methods serially on its dispatch queue. ``BluetoothDelegateShim``
52+
/// yields one of these per callback into a single `AsyncStream`, and ``BluetoothActor`` drains them
53+
/// with a single consumer so the original callback ordering is preserved — independent per-callback
54+
/// `Task`s could be reordered before reaching the actor.
55+
private enum DelegateEvent: Sendable {
56+
case stateUpdate
57+
case discovered(DiscoveryPayload)
58+
}
59+
4860
// MARK: - BluetoothActor
4961

5062
/// Process-wide global actor that serializes all CoreBluetooth interactions.
@@ -68,7 +80,19 @@ actor BluetoothActor {
6880

6981
var centralManager: CBCentralManager?
7082
private var delegateShim: BluetoothDelegateShim?
71-
83+
84+
/// Drains delegate callbacks in order. Lives for the process lifetime of the singleton actor.
85+
private var delegateEventTask: Task<Void, Never>?
86+
87+
/// Tracks one-time actor setup so ``ensureInitialized(log:)`` is idempotent across the many
88+
/// `ReliaBLEManager` façades that may share this process-wide actor.
89+
private var isInitialized = false
90+
91+
/// Continuations for in-flight ``authorize()`` calls awaiting an authorization decision, keyed by a
92+
/// per-call id so a cancelled call can resume just its own continuation. All pending continuations are
93+
/// resumed together once `CBCentralManager.authorization` resolves away from `.notDetermined`.
94+
private var authorizationContinuations: [UUID: CheckedContinuation<Void, Error>] = [:]
95+
7296
/// The current Bluetooth state.
7397
var currentBluetoothState: BluetoothState = .unknown
7498

@@ -110,14 +134,26 @@ actor BluetoothActor {
110134
}
111135
}
112136

137+
/// Upper bound on the number of discovery events buffered for a single subscriber.
138+
///
139+
/// A `PeripheralDiscoveryEvent` is small: a `UUID`, an optional name, an `Int` RSSI, and a typed
140+
/// ``AdvertisementData`` snapshot whose backing advertisement payload is capped by the BLE spec at a few hundred
141+
/// bytes — comfortably under ~1 KB per event including Swift/Foundation overhead. Bounding the buffer at 10,000
142+
/// events caps a stalled or abandoned subscriber at roughly ~10 MB rather than letting it grow without limit,
143+
/// while staying far above any realistic in-flight backlog.
144+
static let discoveryBufferLimit = 10_000
145+
113146
/// Returns a fresh `AsyncStream` of peripheral discovery events for a single subscriber.
114147
///
115148
/// Unlike ``stateStream()`` and ``discoveredPeripheralsStream()`` this feed does **not** replay
116149
/// a value on subscription; a subscriber only receives advertisements observed after it
117150
/// registers. An advertisement that arrives in the narrow window between stream creation and
118151
/// continuation registration is missed — accepted for a lightweight advertisements feed.
152+
///
153+
/// The buffer is bounded with `.bufferingNewest(`` discoveryBufferLimit ``)`: a slow or abandoned subscriber
154+
/// drops the oldest pending advertisements rather than growing memory without bound.
119155
nonisolated func peripheralDiscoveriesStream() -> AsyncStream<PeripheralDiscoveryEvent> {
120-
AsyncStream { continuation in
156+
AsyncStream(bufferingPolicy: .bufferingNewest(BluetoothActor.discoveryBufferLimit)) { continuation in
121157
Task { await BluetoothActor.shared.register(discoveryContinuation: continuation) }
122158
}
123159
}
@@ -189,15 +225,36 @@ actor BluetoothActor {
189225
self.log = log
190226
}
191227

192-
/// Configures the actor with a logger, conditionally sets up the central manager if
193-
/// Bluetooth is already authorized, then broadcasts the initial state. Called once from
194-
/// `ReliaBLEManager.init` via a fire-and-forget `Task`.
195-
func initialize(log: LoggingService) {
196-
configure(log: log)
197-
if CBCentralManager.authorization == .allowedAlways {
228+
/// Performs idempotent actor setup, funneled through by every public ``ReliaBLEManager`` entry point
229+
/// before it acts — so an operation invoked immediately after `init` (whose setup runs in a
230+
/// fire-and-forget `Task`) cannot race ahead of setup and silently no-op.
231+
///
232+
/// The logger is configured exactly once. On *every* call this also creates the central manager if
233+
/// Bluetooth is currently authorized (`.allowedAlways`) and one does not already exist — so an
234+
/// operation issued after authorization is granted out-of-band (via Settings, app lifecycle, or
235+
/// another owner) still finds a live manager instead of being permanently gated by the first call's
236+
/// authorization status.
237+
///
238+
/// Creating the central manager remains gated on existing `.allowedAlways` authorization, preserving
239+
/// the lazy-permission contract: the iOS prompt only appears when the integrating app calls
240+
/// ``ReliaBLEManager/authorizeBluetooth()``. The initial state is broadcast on first setup and
241+
/// whenever the manager is created, but not on every redundant call.
242+
func ensureInitialized(log: LoggingService) {
243+
let firstInitialization = !isInitialized
244+
if firstInitialization {
245+
isInitialized = true
246+
configure(log: log)
247+
}
248+
249+
var createdManager = false
250+
if centralManager == nil, CBCentralManager.authorization == .allowedAlways {
198251
setupCentralManager()
252+
createdManager = true
253+
}
254+
255+
if firstInitialization || createdManager {
256+
updateState()
199257
}
200-
updateState()
201258
}
202259

203260
// MARK: - Central Manager Setup
@@ -207,21 +264,61 @@ actor BluetoothActor {
207264

208265
log?.info("Initializing CBCentralManager")
209266

210-
let shim = BluetoothDelegateShim()
267+
// A single `AsyncStream` carries delegate callbacks in CoreBluetooth's delivery order; the lone
268+
// consumer task below drains them so ordering is preserved end-to-end. The buffer is intentionally
269+
// unbounded: state-change callbacks must never be dropped (unlike the public advertisements feed),
270+
// and `process(_:)` is lightweight, so the actor keeps pace with CoreBluetooth's serial callback
271+
// rate in practice.
272+
let (events, continuation) = AsyncStream.makeStream(
273+
of: DelegateEvent.self,
274+
bufferingPolicy: .unbounded
275+
)
276+
let shim = BluetoothDelegateShim(eventContinuation: continuation)
211277
delegateShim = shim
212278
// Use CBCentralManagerFactory for consistency between normal and test targets.
213279
// `forceMock: true` is load-bearing for the ReliaBLEMock test target — do not remove.
214280
centralManager = CBCentralManagerFactory.instance(delegate: shim, queue: centralManagerQueue, options: nil, forceMock: true)
281+
282+
delegateEventTask = Task { [weak self] in
283+
for await event in events {
284+
await self?.process(event)
285+
}
286+
}
287+
}
288+
289+
/// Drains a single delegate event on the actor, preserving CoreBluetooth's callback order.
290+
private func process(_ event: DelegateEvent) {
291+
switch event {
292+
case .stateUpdate:
293+
handleCentralManagerStateUpdate()
294+
case .discovered(let payload):
295+
handlePeripheralDiscovered(
296+
payload.peripheral,
297+
advertisementData: payload.advertisementData,
298+
rssi: payload.rssi
299+
)
300+
}
215301
}
216302

217303
// MARK: - Authorization
218304

219-
func authorize() throws {
305+
/// Performs the authorization decision for a single ``ReliaBLEManager/authorizeBluetooth()`` call.
306+
///
307+
/// For undetermined authorization this creates the central manager (triggering the iOS prompt) and
308+
/// suspends until the decision arrives via `centralManagerDidUpdateState`, so a successful return
309+
/// means Bluetooth is authorized. The caller-supplied `id` lets ``ReliaBLEManager`` cancel this
310+
/// specific wait via ``cancelAuthorizationContinuation(_:)``.
311+
///
312+
/// The `withTaskCancellationHandler` that wires cancellation lives in the nonisolated
313+
/// ``ReliaBLEManager`` façade, not here, to keep this actor-isolated method free of a construct the
314+
/// region-based isolation checker cannot yet analyze.
315+
func authorize(id: UUID) async throws {
220316
log?.info("Authorizing bluetooth")
221317

222318
switch CBCentralManager.authorization {
223319
case .notDetermined:
224320
setupCentralManager()
321+
try await suspendForAuthorizationDecision(id: id)
225322
case .denied:
226323
throw AuthorizationError.denied
227324
case .restricted:
@@ -233,6 +330,55 @@ actor BluetoothActor {
233330
}
234331
}
235332

333+
/// Suspends until the pending authorization decision resolves (or the calling task is cancelled),
334+
/// storing the continuation under `id`. Kept as its own actor-isolated method so the surrounding
335+
/// `withTaskCancellationHandler` operation closure stays simple for the region-isolation checker.
336+
private func suspendForAuthorizationDecision(id: UUID) async throws {
337+
try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<Void, Error>) in
338+
// The task may already be cancelled by the time this job runs on the actor.
339+
guard !Task.isCancelled else {
340+
continuation.resume(throwing: CancellationError())
341+
return
342+
}
343+
authorizationContinuations[id] = continuation
344+
}
345+
}
346+
347+
/// Resumes a single pending authorization continuation with a `CancellationError`, if still pending.
348+
/// Invoked from ``ReliaBLEManager``'s cancellation handler.
349+
func cancelAuthorizationContinuation(_ id: UUID) {
350+
authorizationContinuations.removeValue(forKey: id)?.resume(throwing: CancellationError())
351+
}
352+
353+
/// Resolves any ``authorize()`` calls suspended on an authorization decision.
354+
///
355+
/// Called after every `centralManagerDidUpdateState`, since CoreBluetooth surfaces an
356+
/// authorization change as a state update. While the decision is still pending
357+
/// (`.notDetermined`) the continuations remain suspended.
358+
private func resolvePendingAuthorization() {
359+
guard !authorizationContinuations.isEmpty else { return }
360+
361+
let result: Result<Void, Error>
362+
switch CBCentralManager.authorization {
363+
case .notDetermined:
364+
return // Still awaiting the user's decision.
365+
case .allowedAlways:
366+
result = .success(())
367+
case .denied:
368+
result = .failure(AuthorizationError.denied)
369+
case .restricted:
370+
result = .failure(AuthorizationError.restricted)
371+
@unknown default:
372+
result = .failure(AuthorizationError.unknown)
373+
}
374+
375+
let pending = authorizationContinuations
376+
authorizationContinuations.removeAll()
377+
for continuation in pending.values {
378+
continuation.resume(with: result)
379+
}
380+
}
381+
236382
// MARK: - Scanning
237383

238384
func startScanning(services: sending [CBUUID]? = nil) {
@@ -337,6 +483,7 @@ actor BluetoothActor {
337483
}
338484

339485
updateState()
486+
resolvePendingAuthorization()
340487
}
341488

342489
func handlePeripheralDiscovered(
@@ -356,7 +503,18 @@ actor BluetoothActor {
356503
to: discoveryContinuations
357504
)
358505

359-
// TODO: FR-8.5: Unique Identifier from Manufacturing Data — connect to id once implemented
506+
// Derive the app-facing `id` from the advertised name, falling back to the local name and
507+
// finally the CoreBluetooth identifier string.
508+
//
509+
// TODO: FR-8.5 — Unique Identifier from Manufacturing Data.
510+
// KNOWN LIMITATION: advertised names are not unique. Two distinct physical devices that
511+
// advertise the same name resolve to the same `identifier` here, so they collapse into a
512+
// single `discoveredPeripherals` entry and a single `cbPeripherals` slot — the later
513+
// discovery overwrites the earlier device's live `CBPeripheral`, so `connect(id:)` may target
514+
// whichever was seen last. FR-8.5 will replace this with a stable identity derived from
515+
// manufacturing data; until then the dedup key is best-effort. The `cbIdentifier` fallback
516+
// below only rescues a *single* device whose advertised name changes, not the same-name
517+
// collision between *different* devices.
360518
let identifier = cbPeripheral.name
361519
?? advertisement.localName
362520
?? cbPeripheral.identifier.uuidString
@@ -444,15 +602,15 @@ actor BluetoothActor {
444602
/// - Note: This currently only fires the connection request. The full connection lifecycle
445603
/// (didConnect/didDisconnect handling and a connection-state surface) is deferred to a later release.
446604
func connect(id: String) throws {
447-
guard let cbPeripheral = cbPeripherals[id] else {
448-
throw PeripheralError.notFound
449-
}
450-
451605
guard let centralManager else {
452-
// Mirrors the start/stopScanning convention: no central manager means there is nothing to act on.
453-
// In practice unreachable — the registry is only populated while a central manager exists.
606+
// No central manager means Bluetooth was never set up (e.g. not authorized). Surface this
607+
// rather than silently succeeding, mirroring the throwing `notFound` path below.
454608
log?.warn(tags: [.peripheral(id)], "Attempted to connect without a central manager")
455-
return
609+
throw PeripheralError.bluetoothUnavailable
610+
}
611+
612+
guard let cbPeripheral = cbPeripherals[id] else {
613+
throw PeripheralError.notFound
456614
}
457615

458616
centralManager.connect(cbPeripheral, options: nil)
@@ -469,8 +627,18 @@ actor BluetoothActor {
469627
/// process-lifetime singleton.
470628
final class BluetoothDelegateShim: NSObject, CBCentralManagerDelegate {
471629

630+
/// Sink for delegate callbacks, drained in order by ``BluetoothActor``'s consumer task.
631+
private let eventContinuation: AsyncStream<DelegateEvent>.Continuation
632+
633+
fileprivate init(eventContinuation: AsyncStream<DelegateEvent>.Continuation) {
634+
self.eventContinuation = eventContinuation
635+
super.init()
636+
}
637+
472638
func centralManagerDidUpdateState(_ central: CBCentralManager) {
473-
Task { @BluetoothActor in await BluetoothActor.shared.handleCentralManagerStateUpdate() }
639+
// Yielding is synchronous and thread-safe; ordering is preserved because CoreBluetooth
640+
// invokes delegate methods serially on its dispatch queue.
641+
eventContinuation.yield(.stateUpdate)
474642
}
475643

476644
func centralManager(
@@ -483,12 +651,6 @@ final class BluetoothDelegateShim: NSObject, CBCentralManagerDelegate {
483651
// single-purpose payload. They are extracted into Sendable types (Peripheral / AdvertisementData) inside
484652
// the actor.
485653
let payload = DiscoveryPayload(peripheral: peripheral, advertisementData: advertisementData, rssi: RSSI.intValue)
486-
Task { @BluetoothActor in
487-
await BluetoothActor.shared.handlePeripheralDiscovered(
488-
payload.peripheral,
489-
advertisementData: payload.advertisementData,
490-
rssi: payload.rssi
491-
)
492-
}
654+
eventContinuation.yield(.discovered(payload))
493655
}
494656
}

Sources/ReliaBLE/Models/PeripheralError.swift

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,19 @@
2626

2727

2828
/// Errors thrown by peripheral operations such as ``ReliaBLEManager/connect(to:)``.
29-
public enum PeripheralError: Error, Sendable {
29+
public enum PeripheralError: Error, Sendable, Equatable {
3030
/// The peripheral is no longer known to the library.
3131
///
3232
/// A ``Peripheral`` is a value snapshot captured at discovery time. The live CoreBluetooth peripheral it refers to
3333
/// is held internally by the library keyed by ``Peripheral/id``. If that reference has since been invalidated (for
3434
/// example, after Bluetooth reset) the snapshot is stale and operations that require the live peripheral throw
3535
/// this error.
3636
case notFound
37+
38+
/// Bluetooth is unavailable, so the operation could not be performed.
39+
///
40+
/// Thrown when a peripheral operation is attempted before the underlying `CBCentralManager` exists — for example,
41+
/// because Bluetooth has not been authorized yet. Call ``ReliaBLEManager/authorizeBluetooth()`` and wait for a
42+
/// ready state before retrying.
43+
case bluetoothUnavailable
3744
}

0 commit comments

Comments
 (0)