weliem / blessed-android

BLESSED, a Bluetooth Low Energy (BLE) library for Android
MIT License
557 stars 119 forks source link

BluetoothPerihheral - cancellation forcing the peripheral get disconnected after bonding failure and bonding lost. #101

Closed talatkuyuk closed 2 years ago

talatkuyuk commented 3 years ago

I am facing a strange issue.

Normally, I create a bond after the peripheral is connected.

if (bluetoothPeripheral.bondState == BondState.NONE) {
    bluetoothPeripheral.createBond()
}

It works fine. It appears an android request dialog to get user confirmation. If the user decline to get bonded, or wait for a time, I mean after bonding failed, the peripheral is starting to disconnect. I receive the disconnected callback, it is fine but the strange thing starts.

override fun onDisconnectedPeripheral(peripheral: BluetoothPeripheral, status: HciStatus) {
    val intent = Intent(ACTION_GATT_DISCONNECTED)
    mainHandler.post { context.sendBroadcast(intent) }
}

Normally, on each disconnected callback received, context.sendBroadcast(intent) works fine, but the callback after receive bonding failure does not send broadcast, I mean context.sendBroadcast(intent) does not work. In order to investigate the issue I put some different delays to try it.

override fun onDisconnectedPeripheral(peripheral: BluetoothPeripheral, status: HciStatus) {
    val intent = Intent(ACTION_GATT_DISCONNECTED)
    mainHandler.post { context.sendBroadcast(intent) } // did not work, I mentioned above
    mainHandler.postDelayed({ context.sendBroadcast(intent) }, 100) // did not work
    mainHandler.postDelayed({ context.sendBroadcast(intent) }, 150) // worked
    mainHandler.postDelayed({ context.sendBroadcast(intent) }, 200) // worked
    mainHandler.postDelayed({ context.sendBroadcast(intent) }, 300) // worked, and so on...
}

In order to send a broadcast via context.sendBroadcast(intent) I needed to delay it around 150ms for above issue. I can keep it as delayed 150ms, but it effects my all broadcasting system for other situations. (I suspect that the issue is happening because of android request dialog for user confirmation, some how.)

I investigate the package code in GitHub, I saw that after getting state of BOND_NONE, you are getting the peripheral disconnected in BluetoothPerihheral.java in the function private void handleBondStateChange(final int bondState, final int previousBondState).

Can you put some delay for this disconnection around 200ms in the function ?

case BOND_NONE:
    ...
    final Runnable disconnectRunnable = new Runnable() {
            @Override
            public void run() {
               disconnect()
            }
        };
    mainHandler.postDelayed(disconnectRunnable, 200);
    break;

Actually, bonding is an optional thing. You can extract disconnect() off for the case BOND_NONE. You don't need to force the peripheral get disconnected. You can give us flexibility for choosing disconnect or not via bonding callbacks. For example I can keep the peripheral be connected if the user does not accept the android request for bonding, or I can get the peripheral disconnected via below callbacks.

override fun onBondLost(peripheral: BluetoothPeripheral) {
    peripheral.cancelConnection()
}

override fun onBondingFailed(peripheral: BluetoothPeripheral) {
    peripheral.cancelConnection()
}

If you do that, for the above issue (context.sendBroadcast(intent)) that I am facing currently, I can deal with myself either put delay or not.

If you ask my preference, I prefer you give us flexibility and don't force the peripheral get disconnected if the case BOND_NONE.

weliem commented 3 years ago

Bonding itself is indeed optional in theory. However, in practice all devices I have worked with either demand it or not. So if a device demands a bond, then disconnecting when a bond is lost is the only sensible action to take. Otherwise nothing would work anymore.

Can you explain why in your situation it would be reasonable to make bonding optional?

Nonetheless, I don't really understand why your context.sendBroadcast(intent) would fail. Seems totally unrelated. Also don't understand why it would work with a delay...

Can you try to do the sendBroadcast directly, without posting it to a handler? Sendbroadcast is asynchronous anyway so there is no need to do it on a Handler. And worse even, the Handler is running on the main thread so that would interfere with any UI stuff that you say you are doing.

talatkuyuk commented 3 years ago

I/BluetoothDevice: createBond() for device 80:7D:3A:::** called by pid: 32382 tid: 32382 D/BluetoothPeripheral: manually bonding 80:7D:3A:D4:35:3E D/BluetoothPeripheral: starting bonding with 'ESP32-BLE' (80:7D:3A:D4:35:3E) I/BLUETOOTHHANDLER: onBondingStarted: currentbondState: BONDING for 80:7D:3A:D4:35:3E D/BluetoothPeripheral: pairing request received: PAIRING_VARIANT_CONSENT (3) D/GALIPFRAGMENT: onPause D/ZrHung.AppEyeUiProbe: not watching, wait. E/BluetoothPeripheral: bonding failed for 'ESP32-BLE', disconnecting device I/BluetoothPeripheral: peripheral '80:7D:3A:D4:35:3E' is disconnecting I/BLUETOOTHHANDLER: onBondingFailed: currentbondState: NONE for 80:7D:3A:D4:35:3E I/BLUETOOTHHANDLER: onDisconnectingPeripheral: disconnecting from '80:7D:3A:D4:35:3E' broadcastUpdate(action: com.example.bluetooth.le.ACTION_GATT_DISCONNECTING) I/BluetoothPeripheral: force disconnect 'ESP32-BLE' (80:7D:3A:D4:35:3E) I/BluetoothPeripheral: disconnected 'ESP32-BLE' on request I/HwViewRootImpl: removeInvalidNode all the node in jank list is out of time I/BLUETOOTHHANDLER: onDisconnectedPeripheral: disconnected from 'ESP32-BLE' with status SUCCESS I/BLUETOOTHHANDLER: broadcastUpdate(action: com.example.bluetooth.le.ACTION_GATT_DISCONNECTED) D/ZrHung.AppEyeUiProbe: restart watching D/GALIPFRAGMENT: onResume

talatkuyuk commented 3 years ago

I tried context.sendBroadcast(intent) without handler, the issue is happening again.

After android request dialog for user confirmation for bonding, the fragment goes to onPause, and broadcast is happening before onResume, I suppose the fragment does not receive the broadcast or the broadcast does not send, I don't know, why? If I put the delay enough, then there is no problem.

Bonding itself is indeed optional in theory. However, in practice all devices I have worked with either demand it or not. So if a device demands a bond, then disconnecting when a bond is lost is the only sensible action to take. Otherwise nothing would work anymore. You are right but as a developer we can take this action ourself. We can cancel connection via onBondLost and onBondingFailed callbacks. What is the problem?

weliem commented 3 years ago

I prefer to keep it like this. For most users of the library a disconnect will be exactly the right thing to do. If I leave it to the users of the library, it will be something that people will not know about and hence they won't do it. You are the first one to ask for a different behavior....

If you really want different behavior I think you better fork this library and make your own customizations.

talatkuyuk commented 3 years ago

I don't want to customize this package for my usage. I am not expertise about android ble, but I would like to contribute it. I think you would give developers flexibility and choose what they want to do. The package can be more responsive to the needs.

people will not know about and hence they won't do it. You can easily write down in the documentation (we are reading the documentation), if bonding request is not successful you have to deal with the cancellation connection via callbacks.

I don't know what the other developers who use this package think about this issue?