Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9f598dcf authored by Grzegorz Kołodziejczyk's avatar Grzegorz Kołodziejczyk Committed by Grzegorz Kolodziejczyk
Browse files

bass_client: Remove pending source operation when stopping receivers 

Some sink devices may not process add source message before stopping
synchronization. Not removing pending operation may lead to block next
add source operation. For external broadcas a timer for source operation
is introduced that would remove pending operation in case of no source
action (add/modify).

Tag: #bug
Bug: 333050419
Bug: 332847049
Test: atest BassClientStateMachineTest BassClientServiceTest
Change-Id: I00967fbcc2b65d17eb23ca8aac6d6c51db1e5bd6
parent dfb1021c

android/app/src/com/android/bluetooth/bass_client/BassClientService.java

+34 −2
Original line number Diff line number Diff line
@@ -1009,7 +1009,13 @@ public class BassClientService extends ProfileService { 
            }

        }

        if (stateMachine.hasPendingSourceOperation()) {

            throw new IllegalStateException("modifySource: source operation already pending");

            Log.w(

                    TAG,

                    "modifySource: source operation already pending, device: "

                            + device

                            + ", broadcastId: "

                            + updatedMetadata.getBroadcastId());

            return BluetoothStatusCodes.ERROR_ALREADY_IN_TARGET_STATE;

        }



        return BluetoothStatusCodes.SUCCESS;
@@ -1560,7 +1566,15 @@ public class BassClientService extends ProfileService { 
                continue;

            }

            if (stateMachine.hasPendingSourceOperation()) {

                throw new IllegalStateException("addSource: source operation already pending");

                Log.w(

                        TAG,

                        "addSource: source operation already pending, device: "

                                + device

                                + ", broadcastId: "

                                + sourceMetadata.getBroadcastId());

                mCallbacks.notifySourceAddFailed(

                        device, sourceMetadata, BluetoothStatusCodes.ERROR_ALREADY_IN_TARGET_STATE);

                continue;

            }

            if (!hasRoomForBroadcastSourceAddition(device)) {

                log("addSource: device has no room");
@@ -1907,6 +1921,21 @@ public class BassClientService extends ProfileService { 
        return list;

    }



    private void cancelPendingSourceOperations(int broadcastId) {

        for (BluetoothDevice device : getConnectedDevices()) {

            synchronized (mStateMachines) {

                BassClientStateMachine sm = getOrCreateStateMachine(device);

                if (sm != null && sm.hasPendingSourceOperation(broadcastId)) {

                    Message message =

                            sm.obtainMessage(

                                    BassClientStateMachine.CANCEL_PENDING_SOURCE_OPERATION);

                    message.arg1 = broadcastId;

                    sm.sendMessage(message);

                }

            }

        }

    }



    private void stopSourceReceivers(int broadcastId) {

        List<Pair<BluetoothLeBroadcastReceiveState, BluetoothDevice>> sourcesToRemove =

                getReceiveStateDevicePairs(broadcastId);
@@ -1914,6 +1943,9 @@ public class BassClientService extends ProfileService { 
        for (Pair<BluetoothLeBroadcastReceiveState, BluetoothDevice> pair : sourcesToRemove) {

            removeSource(pair.second, pair.first.getSourceId());

        }



        /* There may be some pending add/modify source operations */

        cancelPendingSourceOperations(broadcastId);

    }



    private void stopSourceReceivers(int broadcastId, boolean store) {

android/app/src/com/android/bluetooth/bass_client/BassClientStateMachine.java

+54 −4
Original line number Diff line number Diff line
@@ -104,6 +104,7 @@ public class BassClientStateMachine extends StateMachine { 
    static final int CONNECT_TIMEOUT = 15;

    static final int REACHED_MAX_SOURCE_LIMIT = 16;

    static final int SWITCH_BCAST_SOURCE = 17;

    static final int CANCEL_PENDING_SOURCE_OPERATION = 18;



    // NOTE: the value is not "final" - it is modified in the unit tests

    @VisibleForTesting
@@ -265,6 +266,17 @@ public class BassClientStateMachine extends StateMachine { 
        return mPendingMetadata != null;

    }



    Boolean hasPendingSourceOperation(int broadcastId) {

        return mPendingMetadata != null && mPendingMetadata.getBroadcastId() == broadcastId;

    }



    private void cancelPendingSourceOperation(int broadcastId) {

        if ((mPendingMetadata != null) && (mPendingMetadata.getBroadcastId() == broadcastId)) {

            Log.d(TAG, "clearPendingSourceOperation: broadcast ID: " + broadcastId);

            mPendingMetadata = null;

        }

    }



    BluetoothLeBroadcastMetadata getCurrentBroadcastMetadata(Integer sourceId) {

        return mCurrentMetadata.getOrDefault(sourceId, null);

    }
@@ -903,6 +915,7 @@ public class BassClientStateMachine extends StateMachine { 
            if (oldRecvState.getSourceDevice() == null

                    || oldRecvState.getSourceDevice().getAddress().equals(emptyBluetoothDevice)) {

                log("New Source Addition");

                removeMessages(CANCEL_PENDING_SOURCE_OPERATION);

                mService.getCallbacks()

                        .notifySourceAdded(

                                mDevice, recvState, BluetoothStatusCodes.REASON_LOCAL_APP_REQUEST);
@@ -945,6 +958,7 @@ public class BassClientStateMachine extends StateMachine { 
                        setCurrentBroadcastMetadata(recvState.getSourceId(), mPendingMetadata);

                        mPendingMetadata = null;

                    }

                    removeMessages(CANCEL_PENDING_SOURCE_OPERATION);

                    mService.getCallbacks().notifySourceModified(mDevice,

                            recvState.getSourceId(), BluetoothStatusCodes.REASON_LOCAL_APP_REQUEST);

                    checkAndUpdateBroadcastCode(recvState);
@@ -1848,7 +1862,14 @@ public class BassClientStateMachine extends StateMachine { 
                            mSetBroadcastCodePending = true;

                        }

                        transitionTo(mConnectedProcessing);

                        sendMessageDelayed(GATT_TXN_TIMEOUT, BassConstants.GATT_TXN_TIMEOUT_MS);

                        sendMessageDelayed(

                                GATT_TXN_TIMEOUT,

                                ADD_BCAST_SOURCE,

                                BassConstants.GATT_TXN_TIMEOUT_MS);

                        sendMessageDelayed(

                                CANCEL_PENDING_SOURCE_OPERATION,

                                metaData.getBroadcastCode(),

                                BassConstants.SOURCE_OPERATION_TIMEOUT_MS);

                    } else {

                        Log.e(TAG, "ADD_BCAST_SOURCE: no Bluetooth Gatt handle, Fatal");

                        mService.getCallbacks().notifySourceAddFailed(mDevice,
@@ -1878,7 +1899,14 @@ public class BassClientStateMachine extends StateMachine { 
                        }

                        mPendingMetadata = metaData;

                        transitionTo(mConnectedProcessing);

                        sendMessageDelayed(GATT_TXN_TIMEOUT, BassConstants.GATT_TXN_TIMEOUT_MS);

                        sendMessageDelayed(

                                GATT_TXN_TIMEOUT,

                                UPDATE_BCAST_SOURCE,

                                BassConstants.GATT_TXN_TIMEOUT_MS);

                        sendMessageDelayed(

                                CANCEL_PENDING_SOURCE_OPERATION,

                                metaData.getBroadcastCode(),

                                BassConstants.SOURCE_OPERATION_TIMEOUT_MS);

                    } else {

                        Log.e(TAG, "UPDATE_BCAST_SOURCE: no Bluetooth Gatt handle, Fatal");

                        mService.getCallbacks().notifySourceModifyFailed(
@@ -1915,7 +1943,10 @@ public class BassClientStateMachine extends StateMachine { 
                        mPendingOperation = message.what;

                        mPendingSourceId = (byte) recvState.getSourceId();

                        transitionTo(mConnectedProcessing);

                        sendMessageDelayed(GATT_TXN_TIMEOUT, BassConstants.GATT_TXN_TIMEOUT_MS);

                        sendMessageDelayed(

                                GATT_TXN_TIMEOUT,

                                SET_BCAST_CODE,

                                BassConstants.GATT_TXN_TIMEOUT_MS);

                    }

                    break;

                case REMOVE_BCAST_SOURCE:
@@ -1933,7 +1964,10 @@ public class BassClientStateMachine extends StateMachine { 
                        mPendingOperation = message.what;

                        mPendingSourceId = sid;

                        transitionTo(mConnectedProcessing);

                        sendMessageDelayed(GATT_TXN_TIMEOUT, BassConstants.GATT_TXN_TIMEOUT_MS);

                        sendMessageDelayed(

                                GATT_TXN_TIMEOUT,

                                REMOVE_BCAST_SOURCE,

                                BassConstants.GATT_TXN_TIMEOUT_MS);

                    } else {

                        Log.e(TAG, "REMOVE_BCAST_SOURCE: no Bluetooth Gatt handle, Fatal");

                        mService.getCallbacks().notifySourceRemoveFailed(mDevice,
@@ -1953,6 +1987,10 @@ public class BassClientStateMachine extends StateMachine { 
                case PSYNC_ACTIVE_TIMEOUT:

                    cancelActiveSync(null);

                    break;

                case CANCEL_PENDING_SOURCE_OPERATION:

                    int broadcastId = message.arg1;

                    cancelPendingSourceOperation(broadcastId);

                    break;

                default:

                    log("CONNECTED: not handled message:" + message.what);

                    return NOT_HANDLED;
@@ -1995,6 +2033,7 @@ public class BassClientStateMachine extends StateMachine { 
                                .notifySourceAddFailed(mDevice, mPendingMetadata, status);

                        mPendingMetadata = null;

                    }

                    removeMessages(CANCEL_PENDING_SOURCE_OPERATION);

                }

                break;

            case UPDATE_BCAST_SOURCE:
@@ -2003,6 +2042,7 @@ public class BassClientStateMachine extends StateMachine { 
                        mService.getCallbacks().notifySourceModifyFailed(mDevice,

                                mPendingSourceId, status);

                        mPendingMetadata = null;

                        removeMessages(CANCEL_PENDING_SOURCE_OPERATION);

                    }

                } else {

                    mAutoTriggered = false;
@@ -2116,6 +2156,10 @@ public class BassClientStateMachine extends StateMachine { 
                            BluetoothStatusCodes.ERROR_UNKNOWN);

                    mPendingOperation = -1;

                    mPendingSourceId = -1;

                    if ((message.arg1 == UPDATE_BCAST_SOURCE)

                            || (message.arg1 == ADD_BCAST_SOURCE)) {

                        mPendingMetadata = null;

                    }

                    transitionTo(mConnected);

                    break;

                case START_SCAN_OFFLOAD:
@@ -2132,6 +2176,10 @@ public class BassClientStateMachine extends StateMachine { 
                            + ", so that it will be processed later");

                    deferMessage(message);

                    break;

                case CANCEL_PENDING_SOURCE_OPERATION:

                    int broadcastId = message.arg1;

                    cancelPendingSourceOperation(broadcastId);

                    break;

                default:

                    log("CONNECTEDPROCESSING: not handled message:" + message.what);

                    return NOT_HANDLED;
@@ -2224,6 +2272,8 @@ public class BassClientStateMachine extends StateMachine { 
                return "PSYNC_ACTIVE_TIMEOUT";

            case CONNECT_TIMEOUT:

                return "CONNECT_TIMEOUT";

            case CANCEL_PENDING_SOURCE_OPERATION:

                return "CANCEL_PENDING_SOURCE_OPERATION";

            default:

                break;

        }

android/app/src/com/android/bluetooth/bass_client/BassConstants.java

+1 −0
Original line number Diff line number Diff line
@@ -67,6 +67,7 @@ public class BassConstants { 
    public static final int BCAST_RCVR_STATE_BIS_SYNC_SIZE = 4;

    // 30 secs time out for all gatt writes

    public static final int GATT_TXN_TIMEOUT_MS = 30000;

    public static final int SOURCE_OPERATION_TIMEOUT_MS = 3000;

    // 3 min time out for keeping PSYNC active

    public static final int PSYNC_ACTIVE_TIMEOUT_MS = 3 * 60000;

    // 2 secs time out achieving psync

android/app/tests/unit/src/com/android/bluetooth/bass_client/BassClientStateMachineTest.java

+43 −0
Original line number Diff line number Diff line
@@ -38,6 +38,7 @@ import static com.android.bluetooth.bass_client.BassClientStateMachine.START_SCA 
import static com.android.bluetooth.bass_client.BassClientStateMachine.STOP_SCAN_OFFLOAD;

import static com.android.bluetooth.bass_client.BassClientStateMachine.SWITCH_BCAST_SOURCE;

import static com.android.bluetooth.bass_client.BassClientStateMachine.UPDATE_BCAST_SOURCE;

import static com.android.bluetooth.bass_client.BassClientStateMachine.CANCEL_PENDING_SOURCE_OPERATION;

import static com.android.bluetooth.bass_client.BassConstants.CLIENT_CHARACTERISTIC_CONFIG;



import static com.google.common.truth.Truth.assertThat;
@@ -494,6 +495,7 @@ public class BassClientStateMachineTest { 
        assertThat(mBassClientStateMachine.getCurrentBroadcastMetadata(invalidSourceId)).isNull();

        assertThat(mBassClientStateMachine.getDevice()).isEqualTo(mTestDevice);

        assertThat(mBassClientStateMachine.hasPendingSourceOperation()).isFalse();

        assertThat(mBassClientStateMachine.hasPendingSourceOperation(1)).isFalse();

        assertThat(mBassClientStateMachine.isEmpty(new byte[] { 0 })).isTrue();

        assertThat(mBassClientStateMachine.isEmpty(new byte[] { 1 })).isFalse();

        assertThat(mBassClientStateMachine.isPendingRemove(invalidSourceId)).isFalse();
@@ -2136,6 +2138,47 @@ public class BassClientStateMachineTest { 
        Assert.assertEquals(testRssi, metaData.getValue().getRssi());

    }



    @Test

    public void cancelPendingAddBcastSourceMessage_inConnectedState() {

        initToConnectedState();



        BassClientService.Callbacks callbacks = Mockito.mock(BassClientService.Callbacks.class);

        when(mBassClientService.getCallbacks()).thenReturn(callbacks);



        BluetoothLeBroadcastMetadata metadata = createBroadcastMetadata();

        // verify local broadcast doesn't require active synced source

        when(mBassClientService.isLocalBroadcast(any(BluetoothLeBroadcastMetadata.class)))

                .thenReturn(true);



        BassClientStateMachine.BluetoothGattTestableWrapper btGatt =

                Mockito.mock(BassClientStateMachine.BluetoothGattTestableWrapper.class);

        mBassClientStateMachine.mBluetoothGatt = btGatt;

        BluetoothGattCharacteristic scanControlPoint =

                Mockito.mock(BluetoothGattCharacteristic.class);

        mBassClientStateMachine.mBroadcastScanControlPoint = scanControlPoint;



        sendMessageAndVerifyTransition(

                mBassClientStateMachine.obtainMessage(ADD_BCAST_SOURCE, metadata),

                BassClientStateMachine.ConnectedProcessing.class);

        verify(scanControlPoint).setValue(any(byte[].class));

        verify(btGatt).writeCharacteristic(any());



        /* Verify if there is pending add source operation */

        assertThat(mBassClientStateMachine.hasPendingSourceOperation(metadata.getBroadcastId()))

                .isTrue();



        /* Inject a cancel pending source operation event */

        Message msg = mBassClientStateMachine.obtainMessage(CANCEL_PENDING_SOURCE_OPERATION);

        msg.arg1 = metadata.getBroadcastId();

        mBassClientStateMachine.sendMessage(msg);



        TestUtils.waitForLooperToFinishScheduledTask(mHandlerThread.getLooper());



        /* Verify if pending add source operation is canceled */

        assertThat(mBassClientStateMachine.hasPendingSourceOperation(metadata.getBroadcastId()))

                .isFalse();

    }



    private void initToConnectingState() {

        allowConnection(true);

        allowConnectGatt(true);