weliem / blessed-android

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

Android 13 Callback Issues #158

Closed prepka-six15 closed 4 months ago

prepka-six15 commented 2 years ago

I'm seeing the onPhyUpdate and onNotificationStateUpdate callbacks aren't being made after calling peripheral.setPreferredPhy(...) or peripheral.setNotify(...).

The onMtuChanged callback is being made after peripheral.requestMtu(...) however.

In the log I see this interesting message: bluetooth: packages/modules/Bluetooth/system/main/shim/acl.cc:758 OnPhyUpdate: Dropping ACL event with no callback Which comes from here: https://android.googlesource.com/platform/packages/modules/Bluetooth/+/refs/heads/master/system/main/shim/acl.cc I'm not really sure whats going on in this file...

I'm running on a Pixel 4XL with Android 13. I don't see this problem on a Pixel 4XL running Android 12.

I'm using Blessed 2.3.1.

weliem commented 2 years ago

I am aware of this. It has nothing to do with this library. I am running Android 13 on a Pixel 5 and I also don't get the callback. It's a bug in Andoird 13.... :-(

prepka-six15 commented 2 years ago

I see. Thanks for the response.

How should I verify that these changes have been made? I suppose I could ask for the MTU last, and assume that once that finishes the others have either worked, or not.

prepka-six15 commented 2 years ago

I did some more tests. It's not that MTU works and the others don't. It simply seems like requesting a PHY change breaks everything. peripheral.setPreferredPhy(PhyType.LE_2M, PhyType.LE_2M, PhyOptions.NO_PREFERRED); Anything I do before a PHY change works, anything after doesn't.

Doing the PHY change last also doesn't work. I still never see onPhyUpdate. If I start talking to my device, I simply get writeCharacteristic timeouts.

Not changing the PHY works, but throughput seems to be MUCH slower as you would expect.

weliem commented 2 years ago

Yes indeed. It is a problem. Because we don't get the callback, the operation queue is stuck. Maybe I can add a timeout or something so that it doesn't stay stuck....need to experiment a bit.

prepka-six15 commented 2 years ago

I see the library uses connectGatt with a transport value. Maybe you also need a phy value now? https://developer.android.com/reference/android/bluetooth/BluetoothDevice#connectGatt(android.content.Context,%20boolean,%20android.bluetooth.BluetoothGattCallback)

weliem commented 2 years ago

Just published a workaround in version 2.3.2

Can you give that a try?

prepka-six15 commented 2 years ago

The workaround works great!

Even though the callback doesn't happen my device is switching to PhyType.LE_2M and I'm getting the expected speedup.

For my own app, I added a simulated call to onPhyUpdate(peripheral, PhyType.UNKNOWN_PHY_TYPE, PhyType.UNKNOWN_PHY_TYPE, GattStatus.SUCCESS) when (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) so my app's logic works just like before.

DjNDB commented 8 months ago

I just spent hours trying to get my app to work on Android 14, until I discovered that commenting out peripheral.setPreferredPhy() fixed it, which lead to me finding this issue on Android 13.

I fixed it by adding Build.VERSION.SDK_INT == Build.VERSION_CODES.UPSIDE_DOWN_CAKE to the timeout in BluetoothPeripheral.

I tested on a Pixel 4a and Samsung Galaxy Tab S5e, both on LineageOS 21, so I can't tell if Android 14 is affected as well in general, or if it's a LineageOS issue.

public boolean setPreferredPhy(@NotNull final PhyType txPhy, @NotNull final PhyType rxPhy, @NotNull final PhyOptions phyOptions) {
    Objects.requireNonNull(txPhy);
    Objects.requireNonNull(rxPhy);
    Objects.requireNonNull(phyOptions);

    if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) {
        Logger.e(TAG, "setPreferredPhy requires Android 8.0 or newer");
        return false;
    }

    return enqueue(new Runnable() {
        @Override
        public void run() {
            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
                currentCommand = SET_PHY_TYPE_COMMAND;
                Logger.i(TAG, "setting preferred Phy: tx = %s, rx = %s, options = %s", txPhy, rxPhy, phyOptions);
                bluetoothGatt.setPreferredPhy(txPhy.mask, rxPhy.mask, phyOptions.value);

                if (Build.VERSION.SDK_INT == Build.VERSION_CODES.TIRAMISU || Build.VERSION.SDK_INT == Build.VERSION_CODES.UPSIDE_DOWN_CAKE) {
                    // There is a bug in Android 13 where onPhyUpdate is not always called
                    // Therefore complete this command after a delay in order not to block the queueu
                    currentCommand = IDLE;
                    callbackHandler.postDelayed(new Runnable() {
                        @Override
                        public void run() {
                            completedCommand();
                        }
                    }, 200);
                }
            }
        }
    });
}
weliem commented 8 months ago

thx for the info, will look into it

weliem commented 8 months ago

I have tested this on a Pixel 8 running Android 14 and the onPhyUpdate is called. So it seems it has been fixed in Android 14. Might be a LineageOs issue or a phone specific issue.

Not sure what do about it from a library perspective...

DjNDB commented 8 months ago

Thanks for looking into it.

The two devices I have are rather old (Pixel 4a, 2020 and Samsung Galaxy Tab S5e, 2019), so it might be worth testing with LineageOS on a recent device to see if it's some old issue that they have in common. If a new device on LineageOS is affected as well, it may be worth testing on more devices to see if it's a LineageOS issue.

I don't have a new device to test on though unfortunately, and I don't know enough about Bluetooth and the LineageOS code base to figure it out analytically with reasonable effort.

If it is a LineageOS issue, it should obviously be fixed in LineageOS.

Blocking the queue is probably never wanted behavior, so having a timeout that throws an exception and logs it may be a good choice in general to draw attention.
That would make it easy to notice and make sure people don't get stuck like I did, and it might lead to more users or developers reporting similar issues on their devices and getting the root cause fixed.