-
Notifications
You must be signed in to change notification settings - Fork 198
fix(darwin): make Adapter.Enable callable after first invocation #445
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,21 +44,40 @@ var DefaultAdapter = &Adapter{ | |
|
|
||
| // Enable configures the BLE stack. It must be called before any | ||
| // Bluetooth-related calls (unless otherwise indicated). | ||
| // | ||
| // poweredChan doubles as the "Enable currently in flight" sentinel | ||
| // (the upfront non-nil check guards against re-entry) and the | ||
| // channel the central-manager delegate signals on once | ||
| // CoreBluetooth reports powered-on. It is cleared on every exit | ||
| // path — success and timeout — so a subsequent call can run again. | ||
| // Previously the channel was set at the start of Enable and never | ||
| // cleared, so any second Enable on the same Adapter (e.g. from a | ||
| // recovery flow that re-Enables after an error) returned "already | ||
| // calling Enable function" even though no Enable was actually in | ||
| // flight. | ||
| func (a *Adapter) Enable() error { | ||
| if a.poweredChan != nil { | ||
| return errors.New("already calling Enable function") | ||
| } | ||
|
|
||
| // wait until powered | ||
| a.poweredChan = make(chan error, 1) | ||
|
|
||
| // Attach the central-manager delegate BEFORE checking | ||
| // cm.State(): a freshly-constructed CBCentralManager fires | ||
| // DidUpdateState asynchronously, and if SetDelegate happens | ||
| // after the callback runs we miss the powered-on event entirely | ||
| // and wait the full 10s timeout for a callback that already | ||
| // fired. Setting the delegate first guarantees the callback | ||
| // path is wired before the manager has had a chance to update | ||
| // state. | ||
|
retr0h marked this conversation as resolved.
Outdated
|
||
| a.cmd = ¢ralManagerDelegate{a: a} | ||
| a.cm.SetDelegate(a.cmd) | ||
|
|
||
| if a.cm.State() != cbgo.ManagerStatePoweredOn { | ||
| select { | ||
| case <-a.poweredChan: | ||
| case <-time.NewTimer(10 * time.Second).C: | ||
| a.poweredChan = nil | ||
| return errors.New("timeout enabling CentralManager") | ||
| } | ||
| } | ||
|
|
@@ -68,10 +87,11 @@ func (a *Adapter) Enable() error { | |
| <-a.poweredChan | ||
| } | ||
|
|
||
| // wait until powered? | ||
| a.pmd = &peripheralManagerDelegate{a: a} | ||
| a.pm.SetDelegate(a.pmd) | ||
|
|
||
| a.poweredChan = nil | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
|
|
@@ -85,9 +105,20 @@ type centralManagerDelegate struct { | |
|
|
||
| // CentralManagerDidUpdateState when central manager state updated. | ||
| func (cmd *centralManagerDelegate) CentralManagerDidUpdateState(cmgr cbgo.CentralManager) { | ||
| // powered on? | ||
| if cmgr.State() == cbgo.ManagerStatePoweredOn { | ||
| cmd.a.poweredChan <- nil | ||
| // Guard against poweredChan being nil — Enable clears it | ||
| // once the adapter is powered on, so a late or repeated | ||
| // state update (CoreBluetooth occasionally fires multiple | ||
| // times during startup, or after a state-toggle event) | ||
| // would otherwise block forever on a nil-channel send. | ||
| // Non-blocking send so we never park the delegate goroutine | ||
| // even if the channel is set but already buffered full. | ||
| if cmd.a.poweredChan != nil { | ||
| select { | ||
| case cmd.a.poweredChan <- nil: | ||
| default: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we add some logging about discarded events ? I think it is quite important instead of being silently dropped. But we can discuss that in a future evolution.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looked into wiring this up but the package doesn't have a logging convention I can plug into — Punting on it for this PR feels right since adding logging cleanly probably means deciding the shape: a package-level
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can start a new issue about that, but IMO each adapter should support an slog.Logger option. |
||
| } | ||
| } | ||
|
retr0h marked this conversation as resolved.
Outdated
|
||
| } | ||
|
|
||
| // TODO: handle other state changes. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.