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

Commit b1d63874 authored by Sal Savage's avatar Sal Savage
Browse files

Retry SDP when we receive an SDP_BUSY response 

Sometimes the stack can return an SDP_BUSY response, meaning we sent a
valid request to the stack but it couldn't be handled at this time. The
native stack doesn't enqueue or handle this event.

To fix this, we can simply retry SDP when we get an SDP_BUSY (2)
response from the native stack.

Tag: #stability
Bug: 293911750
Test: atest BluetoothInstrumentationTests
Change-Id: I9e8afd94f4e26705fdb062c49034f6851f11e098
parent 2dcade2c

android/app/src/com/android/bluetooth/mapclient/MapClientService.java

+4 −9
Original line number Diff line number Diff line
@@ -768,19 +768,14 @@ public class MapClientService extends ProfileService { 
                }



                if (uuid.equals(BluetoothUuid.MAS)) {

                    // Check if we have a valid SDP record.

                    // Check if we have a successful status with a valid SDP record.

                    int status = intent.getIntExtra(BluetoothDevice.EXTRA_SDP_SEARCH_STATUS, -1);

                    SdpMasRecord masRecord =

                            intent.getParcelableExtra(BluetoothDevice.EXTRA_SDP_RECORD);

                    if (DBG) {

                        Log.d(TAG, "SDP = " + masRecord);

                    }

                    int status = intent.getIntExtra(BluetoothDevice.EXTRA_SDP_SEARCH_STATUS, -1);

                    if (masRecord == null) {

                        Log.w(TAG, "SDP search ended with no MAS record. Status: " + status);

                        return;

                        Log.d(TAG, "SDP complete, status: " + status + ", record:" + masRecord);

                    }

                    stateMachine.obtainMessage(MceStateMachine.MSG_MAS_SDP_DONE,

                            masRecord).sendToTarget();

                    stateMachine.sendSdpResult(status, masRecord);

                }

            }

        }

android/app/src/com/android/bluetooth/mapclient/MceStateMachine.java

+47 −4
Original line number Diff line number Diff line
@@ -94,7 +94,7 @@ class MceStateMachine extends StateMachine { 
    static final int MSG_MAS_REQUEST_COMPLETED = 1003;

    static final int MSG_MAS_REQUEST_FAILED = 1004;

    static final int MSG_MAS_SDP_DONE = 1005;

    static final int MSG_MAS_SDP_FAILED = 1006;

    static final int MSG_MAS_SDP_UNSUCCESSFUL = 1006;

    static final int MSG_OUTBOUND_MESSAGE = 2001;

    static final int MSG_INBOUND_MESSAGE = 2002;

    static final int MSG_NOTIFICATION = 2003;
@@ -116,9 +116,15 @@ class MceStateMachine extends StateMachine { 
    private static final int MAX_MESSAGES = 20;

    private static final int MSG_CONNECT = 1;

    private static final int MSG_DISCONNECT = 2;

    private static final int MSG_CONNECTING_TIMEOUT = 3;

    static final int MSG_CONNECTING_TIMEOUT = 3;

    private static final int MSG_DISCONNECTING_TIMEOUT = 4;



    // Constants for SDP. Note that these values come from the native stack, but no centralized

    // constants exist for them as part of the various SDP APIs.

    public static final int SDP_SUCCESS = 0;

    public static final int SDP_FAILED = 1;

    public static final int SDP_BUSY = 2;



    private static final boolean MESSAGE_SEEN = true;

    private static final boolean MESSAGE_NOT_SEEN = false;
@@ -293,6 +299,21 @@ class MceStateMachine extends StateMachine { 
        return mMostRecentState;

    }



    /**

     * Notify of SDP completion.

     */

    public void sendSdpResult(int status, SdpMasRecord record) {

        if (DBG) {

            Log.d(TAG, "Received SDP Result, status=" + status + ", record=" + record);

        }

        if (status != SDP_SUCCESS || record == null) {

            Log.w(TAG, "SDP unsuccessful, status: " + status + ", record: " + record);

            sendMessage(MceStateMachine.MSG_MAS_SDP_UNSUCCESSFUL, status);

            return;

        }

        sendMessage(MceStateMachine.MSG_MAS_SDP_DONE, record);

    }



    public boolean disconnect() {

        if (DBG) {

            Log.d(TAG, "Disconnect Request " + mDevice);
@@ -533,6 +554,28 @@ class MceStateMachine extends StateMachine { 
                    }

                    break;



                case MSG_MAS_SDP_UNSUCCESSFUL:

                    int sdpStatus = message.arg1;

                    Log.i(TAG, Utils.getLoggableAddress(mDevice)

                            + " [Connecting]: SDP unsuccessful, status=" + sdpStatus);

                    if (sdpStatus == SDP_BUSY) {

                        if (DBG) {

                            Log.d(TAG, Utils.getLoggableAddress(mDevice)

                                    + " [Connecting]: SDP was busy, try again");

                        }

                        mDevice.sdpSearch(BluetoothUuid.MAS);

                    } else {

                        // This means the status is 0 (success, but no record) or 1 (organic

                        // failure). We historically have never retried SDP in failure cases, so we

                        // don't need to wait for the timeout anymore.

                        if (DBG) {

                            Log.d(TAG, Utils.getLoggableAddress(mDevice)

                                    + " [Connecting]: SDP failed completely, disconnecting");

                        }

                        transitionTo(mDisconnecting);

                    }

                    break;



                case MSG_MAS_CONNECTED:

                    transitionTo(mConnected);

                    break;
@@ -1178,8 +1221,8 @@ class MceStateMachine extends StateMachine { 
                return "MSG_MAS_REQUEST_FAILED";

            case MSG_MAS_SDP_DONE:

                return "MSG_MAS_SDP_DONE";

            case MSG_MAS_SDP_FAILED:

                return "MSG_MAS_SDP_FAILED";

            case MSG_MAS_SDP_UNSUCCESSFUL:

                return "MSG_MAS_SDP_UNSUCCESSFUL";

            case MSG_OUTBOUND_MESSAGE:

                return "MSG_OUTBOUND_MESSAGE";

            case MSG_INBOUND_MESSAGE:

android/app/tests/unit/src/com/android/bluetooth/mapclient/MapClientServiceTest.java

+54 −3
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ import static com.google.common.truth.Truth.assertThat; 


import static org.mockito.Mockito.anyString;

import static org.mockito.Mockito.doReturn;

import static org.mockito.Mockito.eq;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.never;

import static org.mockito.Mockito.verify;
@@ -28,6 +29,7 @@ import android.bluetooth.BluetoothAdapter; 
import android.bluetooth.BluetoothDevice;

import android.bluetooth.BluetoothProfile;

import android.bluetooth.BluetoothUuid;

import android.bluetooth.SdpMasRecord;

import android.content.Intent;



import androidx.test.filters.MediumTest;
@@ -322,15 +324,64 @@ public class MapClientServiceTest { 
    }



    @Test

    public void broadcastReceiver_withActionSdpRecord_withoutMasRecord_doesNothing() {

    public void broadcastReceiver_withActionSdpRecord_receivedMasRecord_sdpSuccess() {

        MceStateMachine sm = mock(MceStateMachine.class);

        mService.getInstanceMap().put(mRemoteDevice, sm);

        SdpMasRecord mockSdpRecord = mock(SdpMasRecord.class);



        Intent intent = new Intent(BluetoothDevice.ACTION_SDP_RECORD);

        intent.putExtra(BluetoothDevice.EXTRA_DEVICE, mRemoteDevice);

        intent.putExtra(BluetoothProfile.EXTRA_STATE, BluetoothProfile.STATE_DISCONNECTED);

        intent.putExtra(BluetoothDevice.EXTRA_UUID, BluetoothUuid.MAS);

        // No MasRecord / searchStatus is included in this intent

        intent.putExtra(BluetoothDevice.EXTRA_SDP_SEARCH_STATUS, MceStateMachine.SDP_SUCCESS);

        intent.putExtra(BluetoothDevice.EXTRA_SDP_RECORD, mockSdpRecord);

        mService.mMapReceiver.onReceive(mService, intent);



        verify(sm).sendSdpResult(eq(MceStateMachine.SDP_SUCCESS), eq(mockSdpRecord));

    }



    @Test

    public void broadcastReceiver_withActionSdpRecord_withoutMasRecord_sdpFailed() {

        MceStateMachine sm = mock(MceStateMachine.class);

        mService.getInstanceMap().put(mRemoteDevice, sm);



        Intent intent = new Intent(BluetoothDevice.ACTION_SDP_RECORD);

        intent.putExtra(BluetoothDevice.EXTRA_DEVICE, mRemoteDevice);

        intent.putExtra(BluetoothDevice.EXTRA_UUID, BluetoothUuid.MAS);

        intent.putExtra(BluetoothDevice.EXTRA_SDP_SEARCH_STATUS, MceStateMachine.SDP_SUCCESS);

        // No MasRecord is included in this intent

        mService.mMapReceiver.onReceive(mService, intent);



        // Verify message: SDP was successfully complete, but no record was returned

        verify(sm).sendSdpResult(eq(MceStateMachine.SDP_SUCCESS), eq(null));

    }



    @Test

    public void broadcastReceiver_withActionSdpRecord_withSdpBusy_sdpFailed() {

        MceStateMachine sm = mock(MceStateMachine.class);

        mService.getInstanceMap().put(mRemoteDevice, sm);



        Intent intent = new Intent(BluetoothDevice.ACTION_SDP_RECORD);

        intent.putExtra(BluetoothDevice.EXTRA_DEVICE, mRemoteDevice);

        intent.putExtra(BluetoothDevice.EXTRA_UUID, BluetoothUuid.MAS);

        intent.putExtra(BluetoothDevice.EXTRA_SDP_SEARCH_STATUS, MceStateMachine.SDP_BUSY);

        mService.mMapReceiver.onReceive(mService, intent);



        // Verify message: SDP was busy and no record was returned

        verify(sm).sendSdpResult(eq(MceStateMachine.SDP_BUSY), eq(null));

    }



    @Test

    public void broadcastReceiver_withActionSdpRecord_withSdpFailed_sdpFailed() {

        MceStateMachine sm = mock(MceStateMachine.class);

        mService.getInstanceMap().put(mRemoteDevice, sm);



        Intent intent = new Intent(BluetoothDevice.ACTION_SDP_RECORD);

        intent.putExtra(BluetoothDevice.EXTRA_DEVICE, mRemoteDevice);

        intent.putExtra(BluetoothDevice.EXTRA_UUID, BluetoothUuid.MAS);

        intent.putExtra(BluetoothDevice.EXTRA_SDP_SEARCH_STATUS, MceStateMachine.SDP_FAILED);

        mService.mMapReceiver.onReceive(mService, intent);



        // Verify message: SDP was failed for some reason and no record was returned

        verify(sm).sendSdpResult(eq(MceStateMachine.SDP_FAILED), eq(null));

    }

}

android/app/tests/unit/src/com/android/bluetooth/mapclient/MapClientStateMachineTest.java

+100 −2
Original line number Diff line number Diff line
@@ -847,6 +847,105 @@ public class MapClientStateMachineTest { 
        Assert.assertNull(mIntentArgument.getValue().getPackage());

    }



    @Test

    public void testSdpBusyWhileConnecting_sdpRetried() {

        // Perform first part of MAP connection logic.

        verify(mMockMapClientService,

                timeout(ASYNC_CALL_TIMEOUT_MILLIS).times(1)).sendBroadcastMultiplePermissions(

                mIntentArgument.capture(), any(String[].class),

                any(BroadcastOptions.class));

        Assert.assertEquals(BluetoothProfile.STATE_CONNECTING, mMceStateMachine.getState());



        // Send SDP Failed with status "busy"

        // Note: There's no way to validate the BluetoothDevice#sdpSearch call

        mMceStateMachine.sendSdpResult(MceStateMachine.SDP_BUSY, null);



        // Send successful SDP record, then send MAS Client connected

        SdpMasRecord record = new SdpMasRecord(1, 1, 1, 1, 1, 1, "MasRecord");

        mMceStateMachine.sendSdpResult(MceStateMachine.SDP_SUCCESS, record);

        Message msg = Message.obtain(mHandler, MceStateMachine.MSG_MAS_CONNECTED);

        mMceStateMachine.sendMessage(msg);



        // Verify we move into the connected state

        verify(mMockMapClientService,

                timeout(ASYNC_CALL_TIMEOUT_MILLIS).times(2)).sendBroadcastMultiplePermissions(

                mIntentArgument.capture(), any(String[].class),

                any(BroadcastOptions.class));

        assertThat(mMceStateMachine.getState()).isEqualTo(BluetoothProfile.STATE_CONNECTED);

    }



    @Test

    public void testSdpBusyWhileConnectingAndRetryResultsReceivedAfterTimeout_resultsIgnored() {

        // Perform first part of MAP connection logic.

        verify(mMockMapClientService,

                timeout(ASYNC_CALL_TIMEOUT_MILLIS).times(1)).sendBroadcastMultiplePermissions(

                mIntentArgument.capture(), any(String[].class),

                any(BroadcastOptions.class));

        Assert.assertEquals(BluetoothProfile.STATE_CONNECTING, mMceStateMachine.getState());



        // Send SDP Failed with status "busy"

        // Note: There's no way to validate the BluetoothDevice#sdpSearch call

        mMceStateMachine.sendSdpResult(MceStateMachine.SDP_BUSY, null);



        // Timeout waiting for record

        Message msg = Message.obtain(mHandler, MceStateMachine.MSG_CONNECTING_TIMEOUT);

        mMceStateMachine.sendMessage(msg);



        // Verify we move into the disconnecting state

        verify(mMockMapClientService,

                timeout(ASYNC_CALL_TIMEOUT_MILLIS).times(2)).sendBroadcastMultiplePermissions(

                mIntentArgument.capture(), any(String[].class),

                any(BroadcastOptions.class));

        assertThat(mMceStateMachine.getState()).isEqualTo(BluetoothProfile.STATE_DISCONNECTING);





        // Send successful SDP record, then send MAS Client connected

        SdpMasRecord record = new SdpMasRecord(1, 1, 1, 1, 1, 1, "MasRecord");

        mMceStateMachine.sendSdpResult(MceStateMachine.SDP_SUCCESS, record);



        // Verify nothing happens

        verifyNoMoreInteractions(mMockMapClientService);

    }



    @Test

    public void testSdpFailedWithNoRecordWhileConnecting_deviceDisconnected() {

        // Perform first part of MAP connection logic.

        verify(mMockMapClientService,

                timeout(ASYNC_CALL_TIMEOUT_MILLIS).times(1)).sendBroadcastMultiplePermissions(

                mIntentArgument.capture(), any(String[].class),

                any(BroadcastOptions.class));

        Assert.assertEquals(BluetoothProfile.STATE_CONNECTING, mMceStateMachine.getState());



        // Send SDP process success with no record found

        mMceStateMachine.sendSdpResult(MceStateMachine.SDP_SUCCESS, null);



        // Verify we move into the disconnecting state

        verify(mMockMapClientService,

                timeout(ASYNC_CALL_TIMEOUT_MILLIS).times(2)).sendBroadcastMultiplePermissions(

                mIntentArgument.capture(), any(String[].class),

                any(BroadcastOptions.class));

        assertThat(mMceStateMachine.getState()).isEqualTo(BluetoothProfile.STATE_DISCONNECTING);

    }



    @Test

    public void testSdpOrganicFailure_deviceDisconnected() {

        // Perform first part of MAP connection logic.

        verify(mMockMapClientService,

                timeout(ASYNC_CALL_TIMEOUT_MILLIS).times(1)).sendBroadcastMultiplePermissions(

                mIntentArgument.capture(), any(String[].class),

                any(BroadcastOptions.class));

        Assert.assertEquals(BluetoothProfile.STATE_CONNECTING, mMceStateMachine.getState());



        // Send SDP Failed entirely

        mMceStateMachine.sendSdpResult(MceStateMachine.SDP_FAILED, null);



        verify(mMockMapClientService,

                timeout(ASYNC_CALL_TIMEOUT_MILLIS).times(2)).sendBroadcastMultiplePermissions(

                mIntentArgument.capture(), any(String[].class),

                any(BroadcastOptions.class));

        assertThat(mMceStateMachine.getState()).isEqualTo(BluetoothProfile.STATE_DISCONNECTING);

    }



    private void setupSdpRecordReceipt() {

        // Perform first part of MAP connection logic.

        verify(mMockMapClientService,
@@ -857,8 +956,7 @@ public class MapClientStateMachineTest { 


        // Setup receipt of SDP record

        SdpMasRecord record = new SdpMasRecord(1, 1, 1, 1, 1, 1, "MasRecord");

        Message msg = Message.obtain(mHandler, MceStateMachine.MSG_MAS_SDP_DONE, record);

        mMceStateMachine.sendMessage(msg);

        mMceStateMachine.sendSdpResult(MceStateMachine.SDP_SUCCESS, record);

    }



    private class MockSmsContentProvider extends MockContentProvider {