NordicSemiconductor / Android-BLE-Library

A library that makes working with Bluetooth LE on Android a pleasure. Seriously.
BSD 3-Clause "New" or "Revised" License
2.04k stars 419 forks source link

Cancellation support #423

Closed philips77 closed 2 years ago

philips77 commented 2 years ago

This PR adds support for cancellation of selected requests.

Important

Bluetooth LE requests, which extend SimpleRequest cannot be cancelled. In Android API, each BluetoothGatt method triggers corresponding call in BluetoothGattCallback.

Changes

ble

ble-ktx

corentin-c commented 2 years ago

@philips77 thank you for this feature, I needed it !

Just a question, shouldn't SimpleRequests that uses split be also cancelled when the queue they are in is cancelled ?

Possible use case

For example, let's say I want to write a big file using a splitter, without using a ReliableWriteRequest, but while being able to cancel the remaining split request that were not sent when I call the cancel on the queue.

Possible solution 1

We could make the spliter used in the WriteRequest return null when we want to cancel the remaining write, but that seems like a dirty fix for me.

class BleDataSplitter : DataSplitter {

    private var isCanceled = false

    override fun chunk(
        message: ByteArray,
        @IntRange(from = 0) index: Int,
        @IntRange(from = 20) maxLength: Int
    ): ByteArray? {
        if(isCanceled) return null
        ...
        return data
    }

    fun cancel() {
        isCanceled = true
    }
}

Possible solution 2

I have tested on my side, adding this line to cancelCurrent in BleManagerHandler seems to resolve my issue :

@Override
final void cancelCurrent() {
     final BluetoothDevice device = bluetoothDevice;
     if (device == null)
    return;

     log(Log.WARN, () -> "Request cancelled");
     request.finished = true;  // ------------------------->  LINE ADDED HERE
     if (request instanceof TimeoutableRequest) {
    request.notifyFail(device, FailCallback.REASON_CANCELLED);
     }
     if (awaitingRequest != null) {
    awaitingRequest.notifyFail(device, FailCallback.REASON_CANCELLED);
    awaitingRequest = null;
     }
     if (requestQueue instanceof ReliableWriteRequest) {
    // Cancelling a Reliable Write request requires sending Abort command.
    // Instead of notifying failure, we will remove all enqueued tasks and
    // let the nextRequest to sent Abort command.
    requestQueue.cancelQueue();
     } else if (requestQueue != null) {
    requestQueue.notifyFail(device, FailCallback.REASON_CANCELLED);
    requestQueue = null;
}
nextRequest(request == null || request.finished);
}

Could this interaction with split SimpleRequests be a wanted feature ? Is there a solution I'm not seeing ?

Thanks

philips77 commented 2 years ago

Just a question, shouldn't SimpleRequests that uses split be also cancelled when the queue they are in is cancelled ?

You're right. I did not think about that...

philips77 commented 2 years ago

Thank you for bringing this up. You're right, there should be an way to cancel it. However, it also implies cancellation of receiving a huge packet using notifications, indications, write request (for server case) using DataMerger, which seems to be a bit tricky. I have to sleep with this in mind.