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

Permit notify event subscription when BLE servers don't require registration #4659

Conversation

xtrasimplicity
Copy link
Contributor

Background
I have an iTag that I'm wanting to use as a BLE button to control an application on my ESP32.

Problem
The iTag doesn't require explicit registration for a client to be able to listen to notify events. Currently, the BLERemoteCharacteristic::registerForNotify function tries to update the remote descriptor at 0x2902 and register/unregister the client as required.

As the iTag doesn't have this descriptor, it causes an unhandled exception error and crashes the application.

Changes
To support devices that don't require you to explicitly register to follow such events, I've added a simple descriptorRequiresRegistration flag to the BLERemoteCharacteristic::registerForNotify function. This defaults to true, for backward compatibility.

Thanks!

@xtrasimplicity xtrasimplicity changed the title Permit notify event registration when BLE servers don't require registration Permit notify event subscription when BLE servers don't require registration Dec 26, 2020
@chegewara
Copy link
Contributor

Even if your iTag does not require to write that descriptor it does not hurt to do it. BLE specification says that characteristic which supports notify/indication shall have descriptor 0x2902, which means you can read/write it.
That change does nothing useful in library.

@xtrasimplicity
Copy link
Contributor Author

xtrasimplicity commented Dec 26, 2020

Writing to that descriptor with my iTag results in an unhandled exception as the descriptor does not exist. This change allows the rest of the event subscription process to take place, without writing to that descriptor.

edit: I’m not the only one who has found that some of these tags don’t expose 0x2902. don/cordova-plugin-ble-central#291 (comment)

@chegewara
Copy link
Contributor

Im still not sure about that change, but i accept explanation and code is backward compatible, so approved.

@xtrasimplicity
Copy link
Contributor Author

Thanks, @chegewara! Are you able to merge this, or is this something that will need to be done by someone else?

@me-no-dev me-no-dev merged commit 434d02c into espressif:master Jan 11, 2021
@xtrasimplicity xtrasimplicity deleted the Feature/OptionalDescriptorRegistrationOnEventSubscription branch January 11, 2021 09:55
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.

3 participants