NimBLE-Arduino icon indicating copy to clipboard operation
NimBLE-Arduino copied to clipboard

Upcoming breaking changes.

Open h2zero opened this issue 2 years ago • 5 comments

I have a few changes in mind that will break the current API in a few ways and would like to get feedback on these.

If you're interested please check the pull requests and make any comments/concerns/suggestions of them there. If there is anything you'd like to see changed that I have not already addressed in these PR's please comment below.

Breaking changes merged: #389 #390 #391 #395 #397 #398 #427

h2zero avatar Apr 30 '22 03:04 h2zero

Please adhere to versioning standards and increase major version number to 2.x.x (also in library,properties) on releasing this

Bascy avatar Apr 30 '22 11:04 Bascy

My biggest ask for the next api would be some way of keeping scanning running as much as possible. I understand the underlying ble has a few modes that are mutually exclusive, but the current code is kinda weird about forcing you to stop scanning while connecting, and i'd also LOVE to be able to advertise myself while all this scanning and connecting is happening. Could "scanAsync" be allowed to continue even if we connect, have the client auto resume after switching out of connection?

DTTerastar avatar Apr 30 '22 13:04 DTTerastar

Please adhere to versioning standards and increase major version number to 2.x.x (also in library,properties) on releasing this

Will do

current code is kinda weird about forcing you to stop scanning while connecting

This is a requirement to connect, while forming a connection the stack takes over the receiver and listens only for the device that you are trying to connect to, so the generic scan process needs to stop.

Could "scanAsync" be allowed to continue even if we connect, have the client auto resume after switching out of connection?

This may be better done in the client connect code with a flag that automatically starts the scan after the connection completes/fails.

h2zero avatar Apr 30 '22 13:04 h2zero

Does this pinned issue need updating @h2zero? I went to have a look at the PRs but couldn't see anything that looked to be backward breaking. I could raise the two below as separate Feature Request issues rather than as comments here if you'd like - then you can track (or decline) them separately.

I think the library is really well architected and written. Very flexible and easy to understand - and stable. I only have one backward-breaking change I'd suggest (and it's really minor but stands out like a sore thumb vs all the really clean code!): Remove the bool is_notification parameter from the NimBLECharacteristic::notify() methods. If it's not a notification, we shouldn't be calling the notify method - we should be calling the NimBLECharacteristic::indicate() method! :) It looks like there's a private method that should be called from both notify and indicate to share the common code?

I'm not sure if it would be backward breaking or not, but it would also be nice to be able to register multiple NimBLEServerCallbacks objects. I would like to have a separate class to handle a passkey store under onPassKeyRequest() vs onMTUChange() etc. Again, not a biggie!

j4m3s avatar Sep 25 '22 20:09 j4m3s

Thanks for the suggestions, they make good sense. I have neglected to update this issue, thanks for the reminder.

h2zero avatar Sep 25 '22 22:09 h2zero