Skip to content
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

BLE/GATT bindings for Bluedroid #421

Merged
merged 25 commits into from
Jun 12, 2024
Merged

BLE/GATT bindings for Bluedroid #421

merged 25 commits into from
Jun 12, 2024

Conversation

ivmarkov
Copy link
Collaborator

@ivmarkov ivmarkov commented Apr 23, 2024

There is already esp32-nimble which is a binding for the NimBLE stack in ESP IDF (a bit lighter-weight but BLE only), yet I thought it would be good to complete the existing Bluedroid bindings in esp-idf-svc (which are currently only exposing Classic BT) with the BLE/GAP/GATT functionality.

W.r.t. GATT, currently only GATT Server (peripheral) functionality is exposed, but adding support for GATT client (central) would not be hard at all.

Still needs to be tested (and documented a bit) hence in Draft status.

EDIT: Don't pay attention to the changes in Cargo.toml, sdkconfig.defaults and config.toml. These will be reverted later.

@Vollbrecht
Copy link
Collaborator

Cool that you are working on this. Did you try the api only on a esp32? Can i use it on a non esp32 by only enabling the BLE stuff in the sdkconfig? Can you add a quick and dirty example that spawn a BLE gatt server, i can test it out later if it works on my esp's

@ivmarkov
Copy link
Collaborator Author

Cool that you are working on this. Did you try the api only on a esp32? Can i use it on a non esp32 by only enabling the BLE stuff in the sdkconfig?
Yes, that should be possible.

Can you add a quick and dirty example that spawn a BLE gatt server, i can test it out later if it works on my esp's

Yes I'm working on that but it will take some time (hence why the PR is in a draft status).

Once the PR is in a final state, I'll ping you. For now, what is reviewable is the overall approach (which is not much different, if at all, from the Classic BT Bluedroid code path).

Also, FYI some renames would be coming. The names of the GATT server events are suboptimal. I copied them from somewhere but they suck. More human readable names coming, as well as more APIs.

@ivmarkov
Copy link
Collaborator Author

Cool that you are working on this. Did you try the api only on a esp32? Can i use it on a non esp32 by only enabling the BLE stuff in the sdkconfig?

Yes, the BLE part should work fine on other chips.

@Vollbrecht
Copy link
Collaborator

Do you plan on including a example in the examples dir with this PR or as a follow up?

@ivmarkov
Copy link
Collaborator Author

Do you plan on including a example in the examples dir with this PR or as a follow up?

I'll do as this PR. I just can't get there yet.

@Vollbrecht
Copy link
Collaborator

Do you plan on including a example in the examples dir with this PR or as a follow up?

I'll do as this PR. I just can't get there yet.

No pressure !

@ivmarkov ivmarkov force-pushed the gatt branch 2 times, most recently from 77488ac to 31961c9 Compare June 4, 2024 10:14
@Vollbrecht
Copy link
Collaborator

Thanks for pushing the example! i will try it out later. One thing we might want to directly provide in the example header is the specific set of sdkconfig settings so a user can copy paste them when trying to recreate it in a template.

@ivmarkov ivmarkov marked this pull request as ready for review June 4, 2024 16:10
@ivmarkov
Copy link
Collaborator Author

ivmarkov commented Jun 4, 2024

@Vollbrecht Will look into the CI failure tomorrow. The settings are copied verbatim from another project where they work, but here they fail. Only difference is that the other project uses wifi, which might link against the coex driver. While the BT example doesn't.

@ivmarkov
Copy link
Collaborator Author

ivmarkov commented Jun 4, 2024

In any case, the settings should work for anything except the esp32, i.e. you can test on the c3 and/or other riscv whenever you want.

@ivmarkov
Copy link
Collaborator Author

ivmarkov commented Jun 5, 2024

@Vollbrecht Will look into the CI failure tomorrow. The settings are copied verbatim from another project where they work, but here they fail. Only difference is that the other project uses wifi, which might link against the coex driver. While the BT example doesn't.

No I was wrong! The linking behavior is NEW for ESP IDF v5.2.0. With 5.1.X it works just fine!
I'll now check if we have the same error with version > 5.2.0. If not, I guess we can just skip this version. If yes, we need to figure out what is going on...

@Vollbrecht
Copy link
Collaborator

The link issue here is a bit different as to our general linking issue with idf 5.2 and no_std its not missing something out of the compiler provided library's but something from within esp-idf. Still out of curiosity, checking how it would pen out when you run it with "-C default-linker-libraries"

@ivmarkov
Copy link
Collaborator Author

ivmarkov commented Jun 5, 2024

The chance "-C default-linker-libraries" to fix it is likely 0. We are missing a symbol from the Wifi/BT coexist driver. "-C default-linker-libraries" kinda helps with symbols from libc & intrinsics, but that's pretty much all.

@Vollbrecht
Copy link
Collaborator

Vollbrecht commented Jun 5, 2024

We are missing a symbol from the Wifi/BT coexist driver

interesting that it did work on riscv v5.2, Our other linker issues were always present on all platforms

@ivmarkov
Copy link
Collaborator Author

ivmarkov commented Jun 5, 2024

esp32 is the only chip which has classic BT.

@Vollbrecht
Copy link
Collaborator

Vollbrecht commented Jun 5, 2024

ah i missed that btdm_rf_bb_reg_init is a BT classic thing, the rfcoxist.a should be present on all esp variants in some form thats why i had the wrong picture here.

@ivmarkov
Copy link
Collaborator Author

ivmarkov commented Jun 7, 2024

Short update on the (now fixed) build failure:

  • Error was in the wifi example, not in bt_gatt_server which is introduced with this patch
  • Error only with ESP IDF 5.2.0 (ESP IDF 5.2.1+ have this fixed) and esp32 MCU
  • Error is this: d01571b , first found by @MartinBroers (thanks again Martin!). Workaround: either don't enable BT, or if enabling BT, also enable BT Classic

I've tested the example on both esp32 and esp32c3, so IMO this stuff is ready for merging now.

@ivmarkov ivmarkov force-pushed the gatt branch 2 times, most recently from 296376a to e374603 Compare June 7, 2024 12:04
Bluedroid BLE support

Bluedroid BLE support

Bluedroid BLE support

Bluedroid BLE support

Bluedroid BLE support

Bluedroid BLE support
@ivmarkov ivmarkov merged commit 20161ff into master Jun 12, 2024
12 checks passed
@ivmarkov ivmarkov deleted the gatt branch August 26, 2024 08:37
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.

2 participants