pbio/drv/bluetooth_btstack: Make scan, connect, and pairing more robust. #445
+481
−305
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We should be explicitly resetting only what needs resetting to avoid introducing subtle bugs every time we add something to the peripheral struct that should not be erased.
At the moment we support only one peripheral, so a lot of code relies on global singletons. In the btstack driver, there is a specific handset instance that we want to generalize, so start migrating from there.
We can properly return errors to the user this way, and write chronological code. Besides catching more operational errors, we can also let the higher level code decide when and where to disconnect, rather than doing that from some but not all places within these tasks. This will also help when we support multiple peripherals in parallel.
As in the last commit, improve error handling and allow for multiple peripherals in the future.
Make them consistent with read and discovery tasks.
This replaces the state machine logic of the scan and connect task with placing logic synchronously in a protothread. At every stage, we can detect, report and return errors, and handle cancellation correctly. This fixes a longstanding issue where the Xbox Controller would sometimes not pair to a SPIKE Prime Hub, even though a second attempt usually worked. Now we can catch the return codes from the security manager and re-try pairing within the task, or retry the whole task a second time. We also allow re-encryption now rather than disconnecting when this is attempted. Extensive USB logging is added to catch issues and flow of the connection process.
Some functions have an early exit because no changes are needed. In these cases, we'll want to set their error state to success so that asserting their error does not raise if an error was set and raised previously.
This was referenced Jan 8, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This makes the scan/connect/pair function more robust. In our other drivers, the
scan_and_connect_funcwas already a protothread that the user could would await until completion. In thebluetooth_btstack.cdriver, this was a little different. Instead, the protothread would kick off a state machine in the btstack event handler, and await until it got into the connected state.This worked well enough for some cases, but there was no mechanism to bubble up intermediate errors so we could not recover from bad states. It was also quite hard to follow, because the btstack event handler does not map to the same states. For example,
GATT_EVENT_QUERY_COMPLETEmeans something different if you're reading, writing, or discovering a characteristic, and then it would have to be mapped to a specific peripheral (once we support more than one, see below.)We can rewrite it so it is more like this, where we drive the Bluetooth process forward on each normal event as well as each btstack event:
Between each step, we can return errors if they occur, and each step is cancellable by the user or by timeout.
Operations that are not cancellable still don't get the hub stuck. If we an irrecoverable Bluetooth state and the user stops the program, the hub will gracefully shut down.
This also further generalizes the btstack driver towards allowing more than one peripheral in the future, a process that has been started in commits prior to this PR. In this step, we further reduce the dependency on hardcoded singletons by moving the btstack peripheral state instance into the peripheral object. The tasks are also written to use the peripheral reference rather than the singleton. Later, we can pass the specific instance to each task.