-
Notifications
You must be signed in to change notification settings - Fork 21
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
fix: Close gatt when disconnecting #49
base: master
Are you sure you want to change the base?
Conversation
This ensures that the gatt resources are released and we don't reach the maximmum number of gatt connections
Only close the gatt on disconnect when the connection has not been stablished. This issue is explained here https://www.youtube.com/watch\?v\=jDykHjn-4Ng\&t\=2785s
Hey @kbabcockuf! Could you please have a look at this? Thanks |
Just seeing this! Will look and either approve or have feedback today. |
Anyway, we are moving out of RxJava in our apps, so we are just migrating our bluetooth code to another library with coroutines. I've opened the PR to fix this issue we've found anyway 😊 I hope it's something useful to you |
Thanks for the full details and analysis! I had never heard of the callback not firing on disconnection, but nothing surprises me these days. Curious:
Finally, would it be possible to use https://developer.android.com/reference/android/bluetooth/BluetoothManager#getConnectedDevices(int) rather than using an internal bool flag? |
Also curious, are you looking at adopting https://github.com/weliem/blessed-android-coroutines in your app? We feel RxCentral is the best overall API among other libraries we've compared, but we haven't reviewed or compared against other libraries since early 2020. Would love to hear how your integration and experience goes with your switch! |
Hi @kbabcockuf, I've tested on many different devices and it happens the same. I'm actually testing with a single custom peripheral. Anyway, it happens when I don't enable the Peripheral Bluetooth, so it's not related to any peripheral for sure. We've currently migrated our code to Kable as we've migrated our Rx code to Coroutines. Also, we are piloting some libraries with KMM and Kable fits better for our needs. For now, Kable is working pretty well for now 🙂 I won't be able to migrate to use the Thanks! |
Hi 🎉
First of all, thanks for this library 🙂
I've found a bug when connecting to a specific
BluetoothDevice
when we retry the connections too many times. I've fixed the bug in this PR and updated the example to check and illustrate the bug.Description
When connecting to a device through MAC, using a
BluetoothDevice
, retrying the connection crashes due to too many active GATT clients. By closing the GATT client after each direct connection to aBluetoothDevice
, we release the unused connections and clients and we can keep trying to connect to aBluetoothDevice
without reaching the maximum connections limit.I've seen in this video https://www.youtube.com/watch?v=jDykHjn-4Ng&t=2785s that we must close the GATT connection when the connection has not been established and that we cannot rely on the
onConnectionStateChange
callback to be called always. The article recommends closing the GATT with a timeout and canceling the timeout if the callback to close the GATT is called.Reproduce the bug
BluetoothDevice
to1_000
.After some retries, the app crashes with an error like this due to too many connections (see
clientIf
). On my phone, it crashes after 32 connections. It may differ on other devices.Show log and stacktrace
Other issues that may be related in other libraries
Questions
¿Do you think that the GATT connection should be closed too when it's no longer needed instead of just disconnecting?