Skip to content

Added callbacks to BLERemoteCharacteristic #4737

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

Merged
merged 1 commit into from
Feb 16, 2021

Conversation

morsisko
Copy link
Contributor

Hello,

currently it is not possible to subscribe for the remote characteristic's notification with additional context. This way it's really hard to distinguish the source of callback in case if you are connected to multiple devices that shares the same callback function. Thanks to this pull request one can implement the BLERemoteCharacteristicCallbacks interface and then they can easily determine the device connected to particular notification. It is very useful for my case.

Let me know what do you think about it

@kind3r
Copy link
Contributor

kind3r commented Jan 25, 2021

After #4735 was merged you can now access characteristic's service and from there the device so you know which device and service the notification was send from.

@morsisko
Copy link
Contributor Author

Yeah, this is definitely some sort of solution, but this way you need to keep global array of pairs like (device_id, associated_object_pointer) (unless I'm missing something) which is not the cleanest solution I would say

@kind3r
Copy link
Contributor

kind3r commented Jan 25, 2021

Oh, I think I see what you mean now. The callback is now a function and you want to be able to provide a method (part of an object). Wouldn't std::bind work in this case ?

@morsisko
Copy link
Contributor Author

morsisko commented Jan 25, 2021

Yes, that's what I want to do. Unfortunately std::bind won't work here, because the notification callback parameter is raw function pointer, std::bind need std::function to work, but yeah this could be another way to implement this functionality. Additionally a lot of BLE classes use the interface solution, for example you got BLEClientCallbacks or BLEServerCallbacks

@me-no-dev
Copy link
Member

What I do not like in the situation proposed is that there are two methods with different names that seem to do the same/very similar thing. On top of that it's possible to have both defined and if you first registerForNotify(cb) and then setCallbacks(nullptr), then notifications will be disabled for both.

Please find some common ground and propose a solution that will work in both cases and will have a common internal exit route. Lambdas come into mind :) You can have a look at AsyncUDP::onPacket as example.

@morsisko
Copy link
Contributor Author

Ok, thanks for the feedback. I've just pushed new commit. This time I've changed the typedef to std::function. This change is backward compatible, as the function pointers may be implicitly casted to std::function. To add additional context variable to the notification call, user may just bind the variable using std::bind.

@me-no-dev
Copy link
Member

Thanks :)

@me-no-dev me-no-dev merged commit 1ab550f into espressif:master Feb 16, 2021
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