Skip to content

Bug fix: Cancel tasks when they are removed from the task queue #580

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.

Sign up for GitHub

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

Merged
philips77 merged 3 commits into from
Aug 29, 2024
Merged

Bug fix: Cancel tasks when they are removed from the task queue #580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cb3e407
Notifying cancelled tasks when on cancellation
philips77 Aug 29, 2024
5aca649
Notifying cancelled tasks of a request queue when it fails
philips77 Aug 29, 2024
74dc3cb
Indentation fixed
philips77 Aug 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 44 additions & 20 deletions ble/src/main/java/no/nordicsemi/android/ble/BleManagerHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,7 @@ public void onReceive(final Context context, final Intent intent) {
&& previousState != BluetoothAdapter.STATE_OFF) {
// No more calls are possible
operationInProgress = true;
taskQueue.clear();
initQueue = null;
emptyTasks(FailCallback.REASON_BLUETOOTH_DISABLED);
ready = false;

final BluetoothDevice device = bluetoothDevice;
Expand Down Expand Up @@ -546,8 +545,7 @@ void close() {
// Setting this flag to false would allow to enqueue a new request before the
// current one ends processing. The following line should not be uncommented.
// mGattCallback.operationInProgress = false;
taskQueue.clear();
initQueue = null;
emptyTasks(FailCallback.REASON_DEVICE_DISCONNECTED);
initialization = false;
bluetoothDevice = null;
connected = false;
Expand All @@ -561,6 +559,37 @@ void close() {
}
}

/**
* This method clears the task queues and notifies removed requests of cancellation.
* @param status the reason of cancellation.
*/
private void emptyTasks(final int status) {
final BluetoothDevice oldBluetoothDevice = bluetoothDevice;
if (initQueue != null) {
for (final Request task : initQueue) {
if (oldBluetoothDevice != null)
task.notifyFail(oldBluetoothDevice, status);
else
task.notifyInvalidRequest();
}
initQueue = null;
}
for (final Request task : taskQueue) {
if (oldBluetoothDevice != null) {
if (status == FailCallback.REASON_BLUETOOTH_DISABLED ||
task.characteristic != null ||
task.descriptor != null) {
task.notifyFail(oldBluetoothDevice, status);
} else {
task.notifyFail(oldBluetoothDevice, FailCallback.REASON_CANCELLED);
}
} else {
task.notifyInvalidRequest();
}
}
taskQueue.clear();
}

public BluetoothDevice getBluetoothDevice() {
return bluetoothDevice;
}
Expand Down Expand Up @@ -776,7 +805,7 @@ private boolean internalDisconnect(final int reason) {
log(Log.DEBUG, () -> "gatt.disconnect()");
gatt.disconnect();
if (wasConnected)
return true;
return;

// If the device wasn't connected, there will be no callback after calling
// gatt.disconnect(), the connection attempt will be stopped.
Expand Down Expand Up @@ -1618,8 +1647,7 @@ final void enqueue(@NonNull final Request request) {

@Override
final void cancelQueue() {
taskQueue.clear();
initQueue = null;
emptyTasks(FailCallback.REASON_CANCELLED);
initialization = false;

final BluetoothDevice device = bluetoothDevice;
Expand Down Expand Up @@ -1655,7 +1683,7 @@ final void cancelCurrent() {
// 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();
requestQueue.notifyAndCancelQueue(device);
} else if (requestQueue != null) {
requestQueue.notifyFail(device, FailCallback.REASON_CANCELLED);
requestQueue = null;
Expand Down Expand Up @@ -2251,8 +2279,7 @@ public void onConnectionStateChange(@NonNull final BluetoothGatt gatt,
}

operationInProgress = true; // no more calls are possible
taskQueue.clear();
initQueue = null;
emptyTasks(FailCallback.REASON_DEVICE_DISCONNECTED);
ready = false;

// Store the current value of the connected and deviceNotSupported flags...
Expand Down Expand Up @@ -2447,8 +2474,7 @@ public void onServiceChanged(@NonNull final BluetoothGatt gatt) {
manager.onServicesInvalidated();
onDeviceDisconnected();
// Clear queues, services are no longer valid.
taskQueue.clear();
initQueue = null;
emptyTasks(FailCallback.REASON_NULL_ATTRIBUTE);
// And discover services again
serviceDiscoveryRequested = true;
servicesDiscovered = false;
Expand Down Expand Up @@ -2525,7 +2551,7 @@ public void onCharacteristicWrite(final BluetoothGatt gatt,
final boolean valid = wr.notifyPacketSent(gatt.getDevice(), characteristic.getValue());
if (!valid && requestQueue instanceof ReliableWriteRequest) {
wr.notifyFail(gatt.getDevice(), FailCallback.REASON_VALIDATION);
requestQueue.cancelQueue();
requestQueue.notifyAndCancelQueue(gatt.getDevice());
} else if (wr.hasMore()) {
enqueueFirst(wr);
} else {
Expand All @@ -2549,7 +2575,7 @@ public void onCharacteristicWrite(final BluetoothGatt gatt,
request.notifyFail(gatt.getDevice(), status);
// Automatically abort Reliable Write when write error happen
if (requestQueue instanceof ReliableWriteRequest)
requestQueue.cancelQueue();
requestQueue.notifyAndCancelQueue(gatt.getDevice());
}
awaitingRequest = null;
onError(gatt.getDevice(), ERROR_WRITE_CHARACTERISTIC, status);
Expand Down Expand Up @@ -2649,7 +2675,7 @@ public void onDescriptorWrite(final BluetoothGatt gatt,
final boolean valid = wr.notifyPacketSent(gatt.getDevice(), data);
if (!valid && requestQueue instanceof ReliableWriteRequest) {
wr.notifyFail(gatt.getDevice(), FailCallback.REASON_VALIDATION);
requestQueue.cancelQueue();
requestQueue.notifyAndCancelQueue(gatt.getDevice());
} else if (wr.hasMore()) {
enqueueFirst(wr);
} else {
Expand All @@ -2673,7 +2699,7 @@ public void onDescriptorWrite(final BluetoothGatt gatt,
request.notifyFail(gatt.getDevice(), status);
// Automatically abort Reliable Write when write error happen
if (requestQueue instanceof ReliableWriteRequest)
requestQueue.cancelQueue();
requestQueue.notifyAndCancelQueue(gatt.getDevice());
}
awaitingRequest = null;
onError(gatt.getDevice(), ERROR_WRITE_DESCRIPTOR, status);
Expand Down Expand Up @@ -2707,8 +2733,7 @@ public void onCharacteristicChanged(
manager.onServicesInvalidated();
onDeviceDisconnected();
// Clear queues, services are no longer valid.
taskQueue.clear();
initQueue = null;
emptyTasks(FailCallback.REASON_NULL_ATTRIBUTE);
serviceDiscoveryRequested = true;
log(Log.VERBOSE, () -> "Discovering Services...");
log(Log.DEBUG, () -> "gatt.discoverServices()");
Expand Down Expand Up @@ -3826,8 +3851,7 @@ private synchronized void nextRequest(final boolean force) {
awaitingRequest.notifyFail(bluetoothDevice, FailCallback.REASON_NULL_ATTRIBUTE);
awaitingRequest = null;
}
taskQueue.clear();
initQueue = null;
emptyTasks(FailCallback.REASON_NULL_ATTRIBUTE);
final BluetoothGatt bluetoothGatt = this.bluetoothGatt;
if (connected && bluetoothGatt != null) {
// Invalidate all services and characteristics
Expand Down
21 changes: 21 additions & 0 deletions ble/src/main/java/no/nordicsemi/android/ble/RequestQueue.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

package no.nordicsemi.android.ble;

import android.bluetooth.BluetoothDevice;
import android.os.Handler;

import androidx.annotation.IntRange;
Expand Down Expand Up @@ -171,6 +172,9 @@ public boolean isEmpty() {
* Cancels all the enqueued operations that were not executed yet.
* The one currently executed will be cancelled, if it is {@link TimeoutableRequest}.
* <p>
* Cancelled operations will NOT be notified with {@link FailCallback#REASON_CANCELLED},
* they will be just removed from the queue.
* <p>
* It is safe to call this method in {@link Request#done(SuccessCallback)} or
* {@link Request#fail(FailCallback)} callback;
*/
Expand Down Expand Up @@ -210,4 +214,21 @@ boolean hasMore() {
void cancelQueue() {
requests.clear();
}

/**
* Clears the queue, but first notifies all remaining tasks about cancellation.
* @param device the connected device.
*/
void notifyAndCancelQueue(final @NonNull BluetoothDevice device) {
for (final Request request : requests) {
request.notifyFail(device, FailCallback.REASON_CANCELLED);
}
cancelQueue();
}

@Override
void notifyFail(@NonNull BluetoothDevice device, int status) {
super.notifyFail(device, status);
notifyAndCancelQueue(device);
}
}