Add Trouble BLE Support over ESP-IDF VHCI - #677
Conversation
There was a problem hiding this comment.
Thank you for taking the time and effort!
I think exposing the controller might have some small benefits indeed (like running trouble on top of esp-idf-svc) so I'm not opposed to it in general.
We just have to be careful how to do it so that we don't reintroduce a ton of dependencies that might be a bit of a headache to support in future, as the benefit exists but is not that big.
For one, the host in trouble is not certified, and it is unclear when or if Espressif would certify trouble as the BLE host to be used for their esp-hal baremetal portfolio.
... and Espressif would almost never, ever certify trouble on top of the community-maintained esp-idf-* crates.
In fact, one major reason I'm keeping the esp-idf-* crates alive - in the presence of esp-hal - is that they run on top of ESP-IDF and as such inherit the certified status of the Wifi, Thread and BT stacks (both NimBLE host for which APIs were just introduced and Bluedroid host) of ESP-IDF. None of these three are certified in esp-hal yet, which is a bit of a roadblock for production (as opposed to hobby) use-cases. Running trouble-host on top of ESP-IDF would of course not be a certified setup.
| CONFIG_BT_LE_ACL_BUF_SIZE=251 | ||
|
|
||
| # The Trouble runner and host resources exceed ESP-IDF's default main-task stack. | ||
| CONFIG_ESP_MAIN_TASK_STACK_SIZE=32768 |
There was a problem hiding this comment.
HostResources should not be allocated on-stack, but staticlaly in bss or on heap. It is another topic that they regressed it recently, where such an allocation was not possible anymore due to the Controller re-generification. Fortunately, fixed in main: embassy-rs/trouble#645
|
|
||
| env: | ||
| rust_toolchain: nightly | ||
| # Temporary workaround for rust-lang/rust#158168. Remove after the fix reaches nightly: |
There was a problem hiding this comment.
Revert, this script does not run every day.
| rust_toolchain: nightly | ||
| # Temporary workaround for rust-lang/rust#158168. Remove after the fix reaches nightly: | ||
| # https://github.com/rust-lang/rust/pull/160170 | ||
| rust_toolchain: nightly-2026-07-29 |
| type Controller = ExternalController<EspVhciTransport<'static>, 10>; | ||
|
|
||
| pub fn main() -> Result<()> { | ||
| esp_idf_svc::sys::link_patches(); |
There was a problem hiding this comment.
This is no longer necessary, as it is called automatically by the startup code in esp-idf-sys. The other two explicit link calls should also not be necessary, unless you hit an actual linking issue.
| @@ -0,0 +1,335 @@ | |||
| #[cfg(not(esp_idf_bt_controller_only))] | |||
There was a problem hiding this comment.
While the primary driver for implementing the bt-hci traits on top of esp-idf-svc is to run trouble on top of it:
- Naming the bti-hci controller module
troubleis incorrect. In theory there could be other BLE hosts that could use it - Hiding this code behind a feature named
troubleis also not ideal for the same reasons. Furthermore, hiding this code behind a feature in the first place seems unnecessary?
I suggest:
- Remove the
troublefeature altogether. Whether the bt-hci support is compiled-in or compiled-out should only depend on theesp_idf_*flags - Merge
bt_controller/trouble.rsintobt_controller.rs. What is this separation supposed to model anyway?
| feature = "alloc", | ||
| ))] | ||
| pub mod bt; | ||
| #[cfg(all(not(any(esp32s2, esp32p4)), esp_idf_bt_enabled, feature = "trouble",))] |
There was a problem hiding this comment.
add feature alloc, remove feature trouble, etc.
| alloc = ["esp-idf-hal/alloc", "embedded-svc/alloc", "uncased/alloc"] | ||
| nightly = ["embedded-svc/nightly", "esp-idf-hal/nightly"] | ||
| experimental = ["embedded-svc/experimental", "esp-idf-hal/experimental"] | ||
| trouble = [ |
| embassy-futures = "0.1.2" | ||
| embedded-storage = { version = "0.3", optional = true } | ||
| futures-io = { version = "0.3", optional = true } | ||
| bt-hci = { version = "0.9", optional = true, default-features = false } |
There was a problem hiding this comment.
Removing the trouble feature would mean bt-hci becomes non-optional. Oh well. See also my previous comment on bt-hci-drvier (= bt-hci-transport) as a (much better) alternative.
| futures-io = { version = "0.3", optional = true } | ||
| bt-hci = { version = "0.9", optional = true, default-features = false } | ||
| embassy-sync = { version = "0.7", optional = true } | ||
| embedded-io = { version = "0.7", optional = true } |
There was a problem hiding this comment.
not necessary to list explicitly. comes via embedded-svc anyway
| embedded-storage = { version = "0.3", optional = true } | ||
| futures-io = { version = "0.3", optional = true } | ||
| bt-hci = { version = "0.9", optional = true, default-features = false } | ||
| embassy-sync = { version = "0.7", optional = true } |
There was a problem hiding this comment.
As per my earlier comment, let's try not to introduce this.
2 participants
Thank you for your contribution!
We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:
Submission Checklist 📝
cargo fmtcommand to ensure that all changed code is formatted correctly.cargo clippycommand to ensure that all changed code passes latest Clippy nightly lints.CHANGELOG.mdin the proper section.Pull Request Details 📖
Description
This crate already includes support for ESP-IDF’s Bluedroid and NimBLE BLE stacks. However, the Trouble crate makes it much easier to create Bluetooth Low Energy applications.
This PR introduces Trouble support by implementing the
bt-hcitraits. This is also how theesp-radiocrate enables Bluetooth for devices not usingesp-idfIn addition to the changes included here, there was also an issue with nightly Rust not working with some upstream changes in the Rust compiler. To be honest, I'm not sure what it is. To have CI pass, I've capped the nightly release to
nightly-2026-07-29, but I'm sure this is not how you want to fix it. If you could let me know how you want to fix it, I'll do whatever you want.Testing
I added a small Trouble beacon example to exercise the controller and VHCI transport together.