aws / amazon-freertos-ble-android-sdk

Android SDK for FreeRTOS Bluetooth Devices.
Apache License 2.0
49 stars 41 forks source link

Recommended way to disconnect during pairing/bonding #13

Closed roberthartman closed 4 years ago

roberthartman commented 4 years ago

Describe the bug Exception occurs in AmazonFreeRTOSDevice disconnect() mContext.unregisterReceiver(mBondStateBroadcastReceiver); when using AmazonFreeRTOSManager disconnectFromDevice() while waiting for a connection. The line mBluetoothGatt.close() never gets closed due to the exception, but it's not clear whether the close() call would adequately clean things up.

Build info What is the branch and commit on the device side? Not sure; device is using Pin pairing. What is the branch and tag the SDK side? v1.1.0 Does pulling the latest code help resolve the issue? Using Gradle, did not pull latest version.

System info What is the Android version? 10 What is the bluetooth version? 4.2

To Reproduce What is the frequency of this issue? Every time Steps to reproduce the behavior: Disconnect before connection is fully established.

Log Could you provide log on both the device side and SDK side? Device had not output anything pertinent to the connection when the exception occurred in the app. Here is the app stacktrace:

    java.lang.IllegalArgumentException: Receiver not registered: software.amazon.freertos.amazonfreertossdk.AmazonFreeRTOSDevice$1@3572ecd
        at android.app.LoadedApk.forgetReceiverDispatcher(LoadedApk.java:1429)
        at android.app.ContextImpl.unregisterReceiver(ContextImpl.java:1543)
        at android.content.ContextWrapper.unregisterReceiver(ContextWrapper.java:664)
        at software.amazon.freertos.amazonfreertossdk.AmazonFreeRTOSDevice.disconnect(AmazonFreeRTOSDevice.java:170)
        at software.amazon.freertos.amazonfreertossdk.AmazonFreeRTOSManager.disconnectFromDevice(AmazonFreeRTOSManager.java:216)
        at com.ghsp.uvc.managers.UvcProvisioningManager.disconnect(UvcProvisioningManager.kt:79)
        at com.ghsp.uvc.ui.MainActivity.onSupportNavigateUp(MainActivity.kt:45)
        at androidx.appcompat.app.AppCompatActivity.onMenuItemSelected(AppCompatActivity.java:226)
        at androidx.appcompat.view.WindowCallbackWrapper.onMenuItemSelected(WindowCallbackWrapper.java:109)
        at androidx.appcompat.widget.ToolbarWidgetWrapper$1.onClick(ToolbarWidgetWrapper.java:188)
        at android.view.View.performClick(View.java:7140)
        at android.view.View.performClickInternal(View.java:7117)
        at android.view.View.access$3500(View.java:801)
        at android.view.View$PerformClick.run(View.java:27351)
        at android.os.Handler.handleCallback(Handler.java:883)
        at android.os.Handler.dispatchMessage(Handler.java:100)
        at android.os.Looper.loop(Looper.java:214)
        at android.app.ActivityThread.main(ActivityThread.java:7356)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:492)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:930)

Additional context Add any other context about the problem here.

bxpan commented 4 years ago

Hi roberthartman,

Thank you for reporting the issue. The exception is due to unregistering mBondStateBroadcastReceiver before it is registered, since the disconnectFromDevice was called before the device was actually connected. When you call connectToDevice, you are passing a BleConnectionStatusCallback. I would suggest waiting for the callback and check the connection status to be "connected" before calling disconnectFromDevice. That being said, I agree that we should handle this case in the disconnect logic to avoid such exception, so that even if disconnect is called accidentally before a device is successfully connected, it would not cause a crash.

roberthartman commented 4 years ago

@bxpan thanks for the response. I am seeing the BLE connection process sometimes take 10s of seconds, so waiting for the connection to go through before disconnecting would involve either a bad user experience or me jumping through hoops in the mobile app code. It's not clear why connecting takes so long, but that problem could go away when I eventually start working with the production HW instead of the esp32 devkit running one of the demo apps that I'm using currently.

I can catch the exception in my code, preventing the crash, however Android OS still has the connection going through in the background, and I see the OS-generated pairing dialogs still appear even though from the user's perspective the connection has been canceled. So if a fix is made, if possible, please make sure that the BLE connection is fully canceled (by calling gatt.close() or whatever, if that would be sufficient). Thanks again.

bxpan commented 4 years ago

@roberthartman Thanks for the details. In the disconnect function, it is actually calling close(). The problem is that, unregisterReceiver happened before it was able to call close. That is why it failed to cancel the pending connection. If we move close() ahead and catch the exception thrown by unregisterReceiver in the disconnect call itself, it should solve the issue. We'll make a fix in the next release. In the meantime, you may give this fix a try on your end.

roberthartman commented 4 years ago

@bxpan that's what I was thinking. If I get some time I'll give it a run.