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

Add BLE GAP/GATT support to the dev-esp32 branch #3473

Open
wants to merge 32 commits into
base: dev-esp32
Choose a base branch
from

Conversation

pjsg
Copy link
Member

@pjsg pjsg commented Oct 24, 2021

Fixes #3469

  • This PR is for the dev branch rather than for the release branch.
  • This PR is compliant with the other contributing guidelines as well (if not, please describe why).
  • I have thoroughly tested my contribution.
  • The code changes are reflected in the documentation at docs/*.

This is the first pass at BLE support. There is some more tidyup to do, and also enabling it is slightly tricky as it is built on top
of the NimBLE stack -- and I can't find a way to force enable that stack from the Kconfig.

You can startup and shutdown the BLE stack multiple times and it doesn't obviously leak memory.

It seems to work in my test environment.

Interested in comments.

components/modules/Kconfig Outdated Show resolved Hide resolved
components/modules/Kconfig Outdated Show resolved Hide resolved
components/modules/Kconfig Outdated Show resolved Hide resolved
docs/modules/ble.md Outdated Show resolved Hide resolved
docs/modules/ble.md Outdated Show resolved Hide resolved
docs/modules/ble.md Outdated Show resolved Hide resolved
Fix capitalization of Bluetooth

Co-authored-by: Marcel Stör <[email protected]>
@pjsg pjsg requested a review from marcelstoer January 5, 2022 01:33
@pjsg pjsg dismissed marcelstoer’s stale review January 5, 2022 01:35

Made the requested changes

@pjsg
Copy link
Member Author

pjsg commented Feb 20, 2022

I'd like to get this merged -- any more input?

@marcelstoer marcelstoer requested review from jmattsson and removed request for marcelstoer February 20, 2022 21:38
@marcelstoer
Copy link
Member

As I can't assess C code I added Johny as a reviewer.

Copy link
Member

@jmattsson jmattsson left a comment

Choose a reason for hiding this comment

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

I'm not a BT expert, but on the whole this looks good!
A few comments, and a couple of fixes I'd like to see before merging, but don't wait for me to merge once you've addressed that. Let's get this landed :)

(sorry about the delay, as usual...)

components/modules/Kconfig Show resolved Hide resolved
components/modules/ble.c Show resolved Hide resolved
components/modules/ble.c Outdated Show resolved Hide resolved
components/modules/ble.c Show resolved Hide resolved
components/modules/ble.c Outdated Show resolved Hide resolved
components/modules/ble.c Outdated Show resolved Hide resolved
components/modules/ble.c Outdated Show resolved Hide resolved
components/modules/ledc.c Outdated Show resolved Hide resolved
install.sh Outdated Show resolved Hide resolved
docs/modules/ble.md Show resolved Hide resolved
@pjsg pjsg changed the title First shot at adding BLE GAP/GATT support to the esp32-idf4 branch First shot at adding BLE GAP/GATT support to the dev-esp32 branch Feb 1, 2024
@pjsg pjsg changed the title First shot at adding BLE GAP/GATT support to the dev-esp32 branch Add BLE GAP/GATT support to the dev-esp32 branch Feb 2, 2024
@jmattsson
Copy link
Member

Nice, is there anything else outstanding other than sorting out the merge issues?


### Type conversions

If the `type` value converts a single item, then that will be the value that is placed into the `value` element. If it converts multiple elements, then the elements will be placed into an array that that will be placed into the `value` element.
Copy link
Member

Choose a reason for hiding this comment

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

'that' is repeated twice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants