Skip to content

Update for Swift Concurrency in Swift 6#29

Merged
itsniper merged 35 commits into
masterfrom
10-update-for-swift-concurrency-in-swift-6
Jun 28, 2026
Merged

Update for Swift Concurrency in Swift 6#29
itsniper merged 35 commits into
masterfrom
10-update-for-swift-concurrency-in-swift-6

Conversation

@itsniper

Copy link
Copy Markdown
Member

Summary

Modernizes ReliaBLE for Swift 6 with complete concurrency checking. This is the umbrella PR for issue #10, landing the 5-step migration:

  1. Introduce BluetoothActor to serialize all CoreBluetooth state (replaces ad-hoc queue hopping).
  2. Make Peripheral a Sendable value type (no CBPeripheral reference escapes the actor).
  3. Replace Combine event surfaces with per-subscriber AsyncStream broadcasters (replay-on-subscribe for state & discovered peripherals).
  4. Collapse BluetoothManager + PeripheralManager into ReliaBLEManager (a nonisolated, Sendable façade forwarding to the actor).
  5. Enable strict/complete concurrency checking in Package.swift.

New supporting types: AdvertisementData, PeripheralError, CBUUID+Sendable.

Architecture notes

  • All mutable BLE state (CBCentralManager, continuations, discovered peripherals, live CBPeripheral registry) is owned exclusively by @BluetoothActor.
  • Delegate callbacks arrive on CoreBluetooth's queue and hop into the actor via a nonisolated BluetoothDelegateShim.
  • Lazy authorization is preserved: the central manager is only created when already .allowedAlways or when authorizeBluetooth() is called, so the integrating app controls the permission prompt.
  • CBCentralManagerFactory.instance(..., forceMock: true) is retained (load-bearing for the ReliaBLEMock test target).

Breaking changes

Source-breaking for consumers: Combine publishers → AsyncStream, several operations are now async, the manager split is removed, and Peripheral is now a value snapshot. Should ship as a major version.

🤖 Generated with Claude Code

@itsniper itsniper linked an issue Jun 27, 2026 that may be closed by this pull request

@itsniper itsniper left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review — Swift 6 Concurrency Migration

Summary: A well-executed strict-concurrency migration — the two managers collapse into a single @BluetoothActor, Peripheral/AdvertisementData become pure Sendable value snapshots, Combine is replaced by per-subscriber AsyncStream broadcasters with replay-on-subscribe, and the lazy-authorization + CBCentralManagerFactory(forceMock:) constraints are preserved. The architecture is sound; the items below are reachable correctness/lifecycle edges, not structural flaws.

🔴 Must-fix

  • BluetoothActor.peripheralDiscoveriesStream() — unbounded buffer. It uses the default (.unbounded) buffering policy while stateStream() and discoveredPeripheralsStream() use .bufferingNewest(1). BLE advertisements arrive at high frequency, so a slow or abandoned for await consumer grows memory without bound. Use AsyncStream(bufferingPolicy: .bufferingNewest(N)) or explicitly document the unbounded tradeoff. (The docstring covers no-replay, but not buffering.)

  • BluetoothActor.handlePeripheralDiscovered — name-based identity collapses distinct devices. Identity is keyed on name ?? localName ?? identifier.uuidString, and Peripheral is Hashable/== on id only. Two physical devices advertising the same name collapse into one discoveredPeripherals entry, and cbPeripherals[resolvedId] = cbPeripheral overwrites the live reference — so connect(to:) can target whichever was seen last. The cbIdentifier fallback branch only rescues the same device with a changed name, not this collision. Prefer keying the live registry/dedup on the stable cbPeripheral.identifier. (Partly acknowledged by the FR-8.5 TODO — at minimum surface it as a documented limitation.)

  • ReliaBLEManager.init — fire-and-forget init ordering race. Task { await BluetoothActor.shared.initialize(...) } isn't ordered before a caller's immediate await startScanning(); actors give no cross-Task FIFO guarantee, so an early scan can no-op against centralManager == nil. The inline comment acknowledges this. Consider an actor-side ensureInitialized() that scanning/authorize funnel through (preserving lazy permission) rather than relying on the guard's silent no-op.

🟡 Suggestions

  • authorize() / authorizeBluetooth() returns before resolution. For .notDetermined it calls setupCentralManager() and returns immediately — the user can still deny the system prompt after try await authorizeBluetooth() has succeeded. Either suspend until the authorization callback resolves, or rename/document it as "request authorization" and point callers to the state stream for the result.

  • BluetoothDelegateShim — delegate ordering. Each callback spawns an independent Task { @BluetoothActor in … }. Handlers run atomically (no internal awaits), but a didDiscover task can be scheduled after a resettinginvalidatePeripherals() task, repopulating cbPeripherals post-invalidation. Low probability; consider funneling delegate events through one ordered sequence to guarantee CB-queue order.

  • LoggingService.enabled — mutable var on a Sendable class. Forwards straight to Willow.Logger.enabled. Safe only if Willow synchronizes that property internally — worth confirming; otherwise make enablement configure-once.

  • Tests. Good Sendable/replay coverage, but the highest-risk paths above are untested. Add: init→immediate-scan ordering, stream cancellation pruning continuations, same-name duplicate peripherals, and authorizeBluetooth() from .notDetermined through denial.

  • Breaking API. Combine→AsyncStream, several ops now async, Peripheral now a value snapshot, manager split removed — source-breaking, so release as a major version and confirm README/DocC examples match.

❓ Questions

  • Is collapsing same-named devices an accepted interim behavior pending FR-8.5, or should cbIdentifier become the dedup key now?
  • Should connect(to:) with a nil central manager throw (it currently logs and silently returns, unlike the notFound throw path)?

Authored-By: Claude Opus 4.8 [email protected]

@itsniper

Copy link
Copy Markdown
Member Author

Must-fix

  • BluetoothActor.peripheralDiscoveriesStream() -- unbounded buffer. Implement a very high N so it's not fully unbounded, but unlikely to be hit. I'm open to ideas but some N that would bound it at ~10MB.
  • BluetoothActor.handlePeripheralDiscovered -- name-based identity collapses distinct devices. Since this library is not yet published or used in any production apps, we can leave it and address it with FR8.5. In the meantime expand the TODO comment with a bit more detail on the issue that needs to be addressed.
  • ReliaBLEManager.init -- fire-and-forget init ordering race. I like the idea of an actor-side ensureInitialized() that scanning/authorize funnel through (preserving lazy permission). Please implement this.

Suggestions

  • authorize() / authorizeBluetooth() returns before resolution. Take whichever approach you think provides the best experience for developers of the consuming app. But I personally lean towards suspending until the authorization callback resolves.
  • BluetoothDelegateShim -- delegate ordering. Agreed please implement this.
  • LoggingService.enabled -- mutable var on a Sendable class. This has been confirmed in the Willow library. I want to preserve the ability to enable logs at runtime, which means configure-once is not an acceptable approach.
  • Tests. Please add tests as you see fit.
  • Breaking API. Given this library has not been published and has no consumers, breaking changes are 100% acceptable.

Questions

  • Is collapsing same-named devices an accepted interim behavior pending FR-8.5, or should cbIdentifier become the dedup key now? As mentioned above yes, this is acceptable for now.
  • Should connect(to:) with a nil central manager throw (it currently logs and silently returns, unlike the notFound throw path)? Yes it should throw. Please implement this.

itsniper added a commit that referenced this pull request Jun 28, 2026
…elegate 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 <[email protected]>
itsniper and others added 26 commits June 27, 2026 18:53
…e-object-protocol-to-the-observable-macro

Migrating from the ObservableObject protocol to the @observable macro
Pulled in updated Willow and updated for Swift 6
#13)

Moves CBCentralManager, the discovered-peripherals list, and Combine
subjects into a new @globalActor BluetoothActor, with a
BluetoothDelegateShim bridging CBCentralManagerDelegate callbacks into
actor-isolated handlers. BluetoothManager and ReliaBLEManager become
thin async forwarders, PeripheralManager is folded into the actor, and
docs/demo call sites are updated for the new async API.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
- Restore stored peripheralIdentifier: seed in init, refresh in update,
  don't clear in invalidateCBPeripheral. Fixes refreshPeripherals after
  invalidation (Rec 1, Copilot comments #1/#2/#3).
- Restore dedicated serial DispatchQueue for CBCentralManager instead of
  queue: nil, keeping callbacks off the main thread (Rec 2, comment #9).
- Adopt plan-prescribed @BluetoothActor global-actor annotation in
  BluetoothDelegateShim Task hops (Rec 3, comment #6).
- Remove redundant nonisolated on hash(into:) and fix CBPeriphal typo
  in == doc comment (Rec 4, comments #4 partial / #5).

Co-Authored-By: Claude Opus 4.8 <[email protected]>
…6-safe

Introduce BluetoothActor to serialize CoreBluetooth state (Step 1 of 5)
Convert `Peripheral` from a reference-wrapping type into an immutable,
`Sendable` value snapshot that carries no `CBPeripheral` reference. The
live `CBPeripheral` is now owned exclusively by `BluetoothActor` in an
`id`-keyed registry that never escapes the actor; operations forward the
snapshot's `id` and the actor resolves the live reference, throwing
`PeripheralError.notFound` when a snapshot has gone stale.

- Add `AdvertisementData` to expose typed advertisement fields as values
- Add `PeripheralError` for stale/unresolvable snapshot lookups
- Add `CBUUID+Sendable` retroactive conformance
- Let apps pre-register a known peripheral via `Peripheral(id:)`
- Update `BluetoothActor` discovery to snapshot values and retain the
  non-`Sendable` CoreBluetooth payload only inside the actor
- Update DocC and tests for the new value-type API
I don't want this to be a tempting escape hatch later
I reviewed and all seem reasonable but will need validated with a build and test.

Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Make Peripheral a Sendable value type (Step 2 of 5 for #10)
…f 5, #10)

Replace the three AnyPublisher surfaces on ReliaBLEManager (state,
peripheralDiscoveries, discoveredPeripherals) with per-subscription
AsyncStreams fed by an in-BluetoothActor broadcaster, and remove Combine
from Sources/ReliaBLE and the Demo.

- BluetoothActor: three [UUID: AsyncStream.Continuation] dictionaries replace
  the Combine subjects/publishers; nonisolated stream factories mint a fresh
  stream per call and register on an actor hop. state/discoveredPeripherals
  replay their latest value (.bufferingNewest(1)); peripheralDiscoveries does
  not replay (unbounded). currentBluetoothState retained as the sync
  currentState backing + state replay source.
- BluetoothManager/ReliaBLEManager: retype the three properties to AsyncStream.
- Demo: consume via .task { for await } loops feeding @mainactor handlers;
  drop cancellables/setupSubscriptions.
- Tests: add broadcaster coverage (multi-subscriber replay, fan-out, no-replay).
- DocC: replace .sink examples with for await; document streaming semantics.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
dropping the indirection layer now that the actor work is done directly.

Move BluetoothState/AuthorizationError types and the actor-isolated
state snapshot onto ReliaBLEManager, delete BluetoothManager.swift,
and update docs/tests to match the new public surface.
Replace Combine event surfaces with AsyncStream broadcaster (Step 3 of 5, #10)
Mark ReliaBLEConfig Sendable, and add DocC concurrency guide.

Make Swift 6 language mode and complete strict concurrency explicit in
Package.swift for the ReliaBLE and ReliaBLEMock targets rather than
relying on the swift-tools-version default, mark ReliaBLEConfig as
Sendable, and document the actor isolation contract in a new DocC
Concurrency article. Also record the planning and critique notes for
this step of the Swift 6 migration.
itsniper and others added 7 commits June 27, 2026 18:54
Collapse BluetoothManager into ReliaBLEManager (Step 4 of 5, #10)
Move all SwiftData persistence in the Central tab off the main thread
by introducing `DeviceStoreActor`, a `ModelActor` created off-main via
`create(container:)`. `CentralView` now wires reliaBLE streams and the
store through a single `.task`/`withTaskGroup`, `CentralViewModel`
keeps only UI state and BLE actions, and `Demo/AGENTS.md` documents
the new pattern. Adds the accompanying planning doc.
Added Swift 6 strict concurrency checking (Step 5 of 5, #10)
Use Task.detached in create(container:) so ModelActor construction is
guaranteed off-main even when called from SwiftUI .task. Use
peripheral.lastSeen consistently on new Device inserts. Fix AGENTS.md
to describe manual ModelActor conformance, not the @Modelactor macro.
Demo: route SwiftData writes through background DeviceStoreActor (#20)
…elegate 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 <[email protected]>
…overies

- GettingStarted: authorizeBluetooth() now suspends until the user responds and
  returns only when authorized (and is cancellable) — document the new behavior in
  place of the old "no-op" note.
- GettingStarted: the connect(to:) example now also handles
  PeripheralError.bluetoothUnavailable.
- GettingStarted & Concurrency: note that the peripheralDiscoveries feed is bounded
  (drops the oldest pending advertisements under backpressure).

Co-Authored-By: Claude Opus 4.8 <[email protected]>
@itsniper itsniper force-pushed the 10-update-for-swift-concurrency-in-swift-6 branch from efd72f2 to daa5601 Compare June 28, 2026 01:00
itsniper added 2 commits June 27, 2026 19:26
Route store writes through Task.detached so MainActor-inherited tasks
do not run SwiftData mutations on the main thread. Delete by fetching
in the actor's ModelContext instead of model(for:) with IDs from @query.
Previously, PeripheralManager unconditionally instantiated CBPeripheralManager
in init(), causing the system Bluetooth permission prompt on a fresh app launch.

Move creation into startAdvertising() so the prompt only appears when the
user explicitly chooses to use the peripheral advertising feature. This
prevents the demo from appearing to trigger authorization automatically
(via the library or otherwise).

Also update the Start button enable logic to allow activation while the
manager is still uninitialized.
@itsniper itsniper force-pushed the 10-update-for-swift-concurrency-in-swift-6 branch from 6b50aed to e40bb7d Compare June 28, 2026 04:58
@itsniper itsniper merged commit 8d8bc78 into master Jun 28, 2026
1 check passed
@itsniper itsniper deleted the 10-update-for-swift-concurrency-in-swift-6 branch June 28, 2026 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update for Swift Concurrency in Swift 6

1 participant