From 4cf6a2bd7e338043d99c797d8114377e54ca8129 Mon Sep 17 00:00:00 2001 From: Himanshu Rawat <rwt@google.com> Date: Wed, 29 Nov 2023 22:34:05 +0000 Subject: [PATCH] Test parallel service discovery with pairing cancelation Test that parallel service discovery should not cause the BT stack to lose track of the pairing with the same device. Test: atest BumbleBluetoothTests:PairingTest Flag: EXEMPT test code Bug: 311790408 Change-Id: I8162681ccb4c378d249f3466b7c7dcc8dc605af9 --- flags/Android.bp | 1 + framework/tests/bumble/Android.bp | 2 + .../bluetooth/pairing/PairingTest.java | 368 +++++++++++++++++- 3 files changed, 361 insertions(+), 10 deletions(-) diff --git a/flags/Android.bp b/flags/Android.bp index 61e62bb9023..84b96b3c95c 100644 --- a/flags/Android.bp +++ b/flags/Android.bp @@ -18,6 +18,7 @@ java_aconfig_library { "//cts/tests/tests/bluetooth:__subpackages__", "//packages/modules/Bluetooth/android/app", "//packages/modules/Bluetooth/framework", + "//packages/modules/Bluetooth/framework/tests/bumble", "//packages/modules/Bluetooth/framework/tests/util", "//packages/modules/Bluetooth/service:__subpackages__", ], diff --git a/framework/tests/bumble/Android.bp b/framework/tests/bumble/Android.bp index d1cfa300155..a66ae1e26c3 100644 --- a/framework/tests/bumble/Android.bp +++ b/framework/tests/bumble/Android.bp @@ -25,7 +25,9 @@ android_test_helper_app { "androidx.test.ext.truth", "androidx.test.rules", "bluetooth-test-util-lib", + "bluetooth_flags_java_lib", "compatibility-device-util-axt", + "flag-junit", "grpc-java-lite", "grpc-java-okhttp-client-lite", "mockito-kotlin2", diff --git a/framework/tests/bumble/src/android/bluetooth/pairing/PairingTest.java b/framework/tests/bumble/src/android/bluetooth/pairing/PairingTest.java index f9257ebf699..585c0d893e9 100644 --- a/framework/tests/bumble/src/android/bluetooth/pairing/PairingTest.java +++ b/framework/tests/bumble/src/android/bluetooth/pairing/PairingTest.java @@ -22,8 +22,10 @@ import static androidx.test.espresso.intent.matcher.IntentMatchers.hasExtra; import static com.google.common.truth.Truth.assertThat; import static org.mockito.Mockito.any; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.timeout; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import android.bluetooth.BluetoothAdapter; @@ -36,15 +38,22 @@ import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; import android.content.IntentFilter; +import android.os.ParcelUuid; +import android.platform.test.annotations.RequiresFlagsEnabled; +import android.platform.test.flag.junit.CheckFlagsRule; +import android.platform.test.flag.junit.DeviceFlagsValueProvider; +import android.util.Log; import androidx.test.ext.junit.runners.AndroidJUnit4; import androidx.test.platform.app.InstrumentationRegistry; +import com.android.bluetooth.flags.Flags; import com.android.compatibility.common.util.AdoptShellPermissionsRule; import io.grpc.stub.StreamObserver; import org.hamcrest.Matcher; +import org.hamcrest.Matchers; import org.hamcrest.core.AllOf; import org.junit.After; import org.junit.Before; @@ -57,30 +66,50 @@ import org.mockito.MockitoAnnotations; import org.mockito.hamcrest.MockitoHamcrest; import java.time.Duration; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; +import pandora.GattProto; +import pandora.HostProto.AdvertiseRequest; +import pandora.HostProto.AdvertiseResponse; +import pandora.HostProto.OwnAddressType; +import pandora.SecurityProto.LESecurityLevel; import pandora.SecurityProto.PairingEvent; import pandora.SecurityProto.PairingEventAnswer; +import pandora.SecurityProto.SecureRequest; +import pandora.SecurityProto.SecureResponse; @RunWith(AndroidJUnit4.class) public class PairingTest { + private static final String TAG = "PairingTest"; private static final Duration BOND_INTENT_TIMEOUT = Duration.ofSeconds(10); + private static final ParcelUuid BATTERY_UUID = + ParcelUuid.fromString("0000180F-0000-1000-8000-00805F9B34FB"); + private static final Context sTargetContext = InstrumentationRegistry.getInstrumentation().getTargetContext(); private static final BluetoothAdapter sAdapter = sTargetContext.getSystemService(BluetoothManager.class).getAdapter(); - @Rule public final AdoptShellPermissionsRule mPermissionRule = new AdoptShellPermissionsRule(); + @Rule(order = 0) + public final CheckFlagsRule mCheckFlagsRule = DeviceFlagsValueProvider.createCheckFlagsRule(); + + @Rule(order = 1) + public final AdoptShellPermissionsRule mPermissionRule = new AdoptShellPermissionsRule(); - @Rule public final PandoraDevice mBumble = new PandoraDevice(); + @Rule(order = 2) + public final PandoraDevice mBumble = new PandoraDevice(); - @Rule + @Rule(order = 3) public final EnableBluetoothRule mEnableBluetoothRule = new EnableBluetoothRule(false /* enableTestMode */, true /* toggleBluetooth */); @Mock private BroadcastReceiver mReceiver; + private final Map<String, Integer> mActionRegistrationCounts = new HashMap<>(); private InOrder mInOrder = null; private BluetoothDevice mBumbleDevice; @@ -90,12 +119,30 @@ public class PairingTest { @Before public void setUp() throws Exception { MockitoAnnotations.initMocks(this); - mInOrder = inOrder(mReceiver); - IntentFilter filter = new IntentFilter(); - filter.addAction(BluetoothDevice.ACTION_BOND_STATE_CHANGED); - filter.addAction(BluetoothDevice.ACTION_PAIRING_REQUEST); - sTargetContext.registerReceiver(mReceiver, filter); + doAnswer( + inv -> { + Log.d( + TAG, + "onReceive(): intent=" + Arrays.toString(inv.getArguments())); + Intent intent = inv.getArgument(1); + String action = intent.getAction(); + if (BluetoothDevice.ACTION_UUID.equals(action)) { + ParcelUuid[] uuids = + intent.getParcelableArrayExtra( + BluetoothDevice.EXTRA_UUID, ParcelUuid.class); + Log.d(TAG, "onReceive(): UUID=" + Arrays.toString(uuids)); + } else if (BluetoothDevice.ACTION_BOND_STATE_CHANGED.equals(action)) { + int bondState = + intent.getIntExtra(BluetoothDevice.EXTRA_BOND_STATE, -1); + Log.d(TAG, "onReceive(): bondState=" + bondState); + } + return null; + }) + .when(mReceiver) + .onReceive(any(), any()); + + mInOrder = inOrder(mReceiver); mBumbleDevice = mBumble.getRemoteDevice(); Set<BluetoothDevice> bondedDevices = sAdapter.getBondedDevices(); @@ -111,7 +158,10 @@ public class PairingTest { removeBond(mBumbleDevice); } mBumbleDevice = null; - sTargetContext.unregisterReceiver(mReceiver); + if (getTotalActionRegistrationCounts() > 0) { + sTargetContext.unregisterReceiver(mReceiver); + mActionRegistrationCounts.clear(); + } } /** @@ -129,6 +179,9 @@ public class PairingTest { */ @Test public void testBrEdrPairing_phoneInitiatedBrEdrInquiryOnlyJustWorks() { + registerIntentActions( + BluetoothDevice.ACTION_BOND_STATE_CHANGED, BluetoothDevice.ACTION_PAIRING_REQUEST); + StreamObserver<PairingEventAnswer> pairingEventAnswerObserver = mBumble.security() .withDeadlineAfter(BOND_INTENT_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS) @@ -159,15 +212,219 @@ public class PairingTest { hasExtra(BluetoothDevice.EXTRA_BOND_STATE, BluetoothDevice.BOND_BONDED)); verifyNoMoreInteractions(mReceiver); + + unregisterIntentActions( + BluetoothDevice.ACTION_BOND_STATE_CHANGED, BluetoothDevice.ACTION_PAIRING_REQUEST); + } + + /** + * Test if parallel GATT service discovery interrupts cancelling LE pairing + * + * <p>Prerequisites: + * + * <ol> + * <li>Bumble and Android are not bonded + * <li>Bumble has GATT services in addition to GAP and GATT services + * </ol> + * + * <p>Steps: + * + * <ol> + * <li>Bumble is discoverable and connectable over LE + * <li>Android connects to Bumble over LE + * <li>Android starts GATT service discovery + * <li>Bumble initiates pairing + * <li>Android does not confirm the pairing immediately + * <li>Service discovery completes + * <li>Android cancels the pairing + * </ol> + * + * Expectation: Pairing gets cancelled instead of getting timed out + */ + @Test + @RequiresFlagsEnabled(Flags.FLAG_RESET_PAIRING_ONLY_FOR_RELATED_SERVICE_DISCOVERY) + public void testCancelBondLe_WithGattServiceDiscovery() { + registerIntentActions(BluetoothDevice.ACTION_BOND_STATE_CHANGED); + + // Outgoing GATT service discovery and incoming LE pairing in parallel + StreamObserverSpliterator<SecureResponse> responseObserver = + helper_OutgoingGattServiceDiscoveryWithIncomingLePairing(); + + // Cancel pairing from Android + assertThat(mBumbleDevice.cancelBondProcess()).isTrue(); + + SecureResponse secureResponse = responseObserver.iterator().next(); + assertThat(secureResponse.hasPairingFailure()).isTrue(); + + // Pairing should be cancelled in a moment instead of timing out in 30 + // seconds + verifyIntentReceived( + hasAction(BluetoothDevice.ACTION_BOND_STATE_CHANGED), + hasExtra(BluetoothDevice.EXTRA_DEVICE, mBumbleDevice), + hasExtra(BluetoothDevice.EXTRA_BOND_STATE, BluetoothDevice.BOND_NONE)); + + verifyNoMoreInteractions(mReceiver); + + unregisterIntentActions(BluetoothDevice.ACTION_BOND_STATE_CHANGED); + } + + /** + * Test if parallel GATT service discovery interrupts the LE pairing + * + * <p>Prerequisites: + * + * <ol> + * <li>Bumble and Android are not bonded + * <li>Bumble has GATT services in addition to GAP and GATT services + * </ol> + * + * <p>Steps: + * + * <ol> + * <li>Bumble is discoverable and connectable over LE + * <li>Android connects to Bumble over LE + * <li>Android starts GATT service discovery + * <li>Bumble starts pairing + * <li>Service discovery completes + * <li>Android does confirms the pairing + * <li>Pairing is successful + * </ol> + * + * Expectation: Pairing succeeds + */ + @Test + @RequiresFlagsEnabled(Flags.FLAG_RESET_PAIRING_ONLY_FOR_RELATED_SERVICE_DISCOVERY) + public void testBondLe_WithGattServiceDiscovery() { + registerIntentActions(BluetoothDevice.ACTION_BOND_STATE_CHANGED); + + // Outgoing GATT service discovery and incoming LE pairing in parallel + StreamObserverSpliterator<SecureResponse> responseObserver = + helper_OutgoingGattServiceDiscoveryWithIncomingLePairing(); + + // Approve pairing from Android + assertThat(mBumbleDevice.setPairingConfirmation(true)).isTrue(); + + SecureResponse secureResponse = responseObserver.iterator().next(); + assertThat(secureResponse.hasSuccess()).isTrue(); + + // Ensure that pairing succeeds + verifyIntentReceived( + hasAction(BluetoothDevice.ACTION_BOND_STATE_CHANGED), + hasExtra(BluetoothDevice.EXTRA_DEVICE, mBumbleDevice), + hasExtra(BluetoothDevice.EXTRA_BOND_STATE, BluetoothDevice.BOND_BONDED)); + + verifyNoMoreInteractions(mReceiver); + + unregisterIntentActions(BluetoothDevice.ACTION_BOND_STATE_CHANGED); + } + + /* Starts outgoing GATT service discovery and incoming LE pairing in parallel */ + private StreamObserverSpliterator<SecureResponse> + helper_OutgoingGattServiceDiscoveryWithIncomingLePairing() { + // Setup intent filters + registerIntentActions( + BluetoothDevice.ACTION_BOND_STATE_CHANGED, + BluetoothDevice.ACTION_PAIRING_REQUEST, + BluetoothDevice.ACTION_UUID, + BluetoothDevice.ACTION_ACL_CONNECTED); + + // Register lots of interesting GATT services on Bumble + for (int i = 0; i < 40; i++) { + mBumble.gattBlocking() + .registerService( + GattProto.RegisterServiceRequest.newBuilder() + .setService( + GattProto.GattServiceParams.newBuilder() + .setUuid(BATTERY_UUID.toString()) + .build()) + .build()); + } + + // Start GATT service discovery, this will establish LE ACL + assertThat(mBumbleDevice.fetchUuidsWithSdp(BluetoothDevice.TRANSPORT_LE)).isTrue(); + + // Make Bumble connectable + AdvertiseResponse advertiseResponse = + mBumble.hostBlocking() + .advertise( + AdvertiseRequest.newBuilder() + .setLegacy(true) + .setConnectable(true) + .setOwnAddressType(OwnAddressType.PUBLIC) + .build()) + .next(); + + // Todo: Unexpected empty ACTION_UUID intent is generated + verifyIntentReceived(hasAction(BluetoothDevice.ACTION_UUID)); + + // Wait for connection on Android + verifyIntentReceived( + hasAction(BluetoothDevice.ACTION_ACL_CONNECTED), + hasExtra(BluetoothDevice.EXTRA_TRANSPORT, BluetoothDevice.TRANSPORT_LE)); + + // Start pairing from Bumble + StreamObserverSpliterator<SecureResponse> responseObserver = + new StreamObserverSpliterator<>(); + mBumble.security() + .secure( + SecureRequest.newBuilder() + .setConnection(advertiseResponse.getConnection()) + .setLe(LESecurityLevel.LE_LEVEL3) + .build(), + responseObserver); + + // Wait for incoming pairing notification on Android + // TODO: Order of these events is not deterministic + verifyIntentReceivedUnordered( + hasAction(BluetoothDevice.ACTION_BOND_STATE_CHANGED), + hasExtra(BluetoothDevice.EXTRA_DEVICE, mBumbleDevice), + hasExtra(BluetoothDevice.EXTRA_BOND_STATE, BluetoothDevice.BOND_BONDING)); + verifyIntentReceivedUnordered( + hasAction(BluetoothDevice.ACTION_PAIRING_REQUEST), + hasExtra(BluetoothDevice.EXTRA_DEVICE, mBumbleDevice), + hasExtra( + BluetoothDevice.EXTRA_PAIRING_VARIANT, + BluetoothDevice.PAIRING_VARIANT_CONSENT)); + + // Allow participating in the incoming pairing on Android + assertThat(mBumbleDevice.setPairingConfirmation(true)).isTrue(); + + // Wait for pairing approval notification on Android + verifyIntentReceivedUnordered( + 2, + hasAction(BluetoothDevice.ACTION_PAIRING_REQUEST), + hasExtra(BluetoothDevice.EXTRA_DEVICE, mBumbleDevice), + hasExtra( + BluetoothDevice.EXTRA_PAIRING_VARIANT, + BluetoothDevice.PAIRING_VARIANT_CONSENT)); + + // Wait for GATT service discovery to complete on Android + // so that ACTION_UUID is received here. + verifyIntentReceived( + hasAction(BluetoothDevice.ACTION_UUID), + hasExtra( + BluetoothDevice.EXTRA_UUID, + Matchers.arrayContainingInAnyOrder(BATTERY_UUID))); + + unregisterIntentActions( + BluetoothDevice.ACTION_BOND_STATE_CHANGED, + BluetoothDevice.ACTION_PAIRING_REQUEST, + BluetoothDevice.ACTION_UUID, + BluetoothDevice.ACTION_ACL_CONNECTED); + + return responseObserver; } private void removeBond(BluetoothDevice device) { + registerIntentActions(BluetoothDevice.ACTION_BOND_STATE_CHANGED); + assertThat(device.removeBond()).isTrue(); verifyIntentReceived( hasAction(BluetoothDevice.ACTION_BOND_STATE_CHANGED), hasExtra(BluetoothDevice.EXTRA_DEVICE, mBumbleDevice), hasExtra(BluetoothDevice.EXTRA_BOND_STATE, BluetoothDevice.BOND_NONE)); - verifyNoMoreInteractions(mReceiver); + + unregisterIntentActions(BluetoothDevice.ACTION_BOND_STATE_CHANGED); } @SafeVarargs @@ -175,4 +432,95 @@ public class PairingTest { mInOrder.verify(mReceiver, timeout(BOND_INTENT_TIMEOUT.toMillis())) .onReceive(any(Context.class), MockitoHamcrest.argThat(AllOf.allOf(matchers))); } + + @SafeVarargs + private void verifyIntentReceivedUnordered(int num, Matcher<Intent>... matchers) { + verify(mReceiver, timeout(BOND_INTENT_TIMEOUT.toMillis()).times(num)) + .onReceive(any(Context.class), MockitoHamcrest.argThat(AllOf.allOf(matchers))); + } + + @SafeVarargs + private void verifyIntentReceivedUnordered(Matcher<Intent>... matchers) { + verifyIntentReceivedUnordered(1, matchers); + } + + /** + * Helper function to add reference count to registered intent actions + * + * @param actions new intent actions to add. If the array is empty, it is a no-op. + */ + private void registerIntentActions(String... actions) { + if (actions.length == 0) { + return; + } + if (getTotalActionRegistrationCounts() > 0) { + Log.d(TAG, "registerIntentActions(): unregister ALL intents"); + sTargetContext.unregisterReceiver(mReceiver); + } + for (String action : actions) { + mActionRegistrationCounts.merge(action, 1, Integer::sum); + } + IntentFilter filter = new IntentFilter(); + mActionRegistrationCounts.entrySet().stream() + .filter(entry -> entry.getValue() > 0) + .forEach( + entry -> { + Log.d( + TAG, + "registerIntentActions(): Registering action = " + + entry.getKey()); + filter.addAction(entry.getKey()); + }); + sTargetContext.registerReceiver(mReceiver, filter); + } + + /** + * Helper function to reduce reference count to registered intent actions If total reference + * count is zero after removal, no broadcast receiver will be registered. + * + * @param actions intent actions to be removed. If some action is not registered, it is no-op + * for that action. If the actions array is empty, it is also a no-op. + */ + private void unregisterIntentActions(String... actions) { + if (actions.length == 0) { + return; + } + if (getTotalActionRegistrationCounts() <= 0) { + return; + } + Log.d(TAG, "unregisterIntentActions(): unregister ALL intents"); + sTargetContext.unregisterReceiver(mReceiver); + for (String action : actions) { + if (!mActionRegistrationCounts.containsKey(action)) { + continue; + } + mActionRegistrationCounts.put(action, mActionRegistrationCounts.get(action) - 1); + if (mActionRegistrationCounts.get(action) <= 0) { + mActionRegistrationCounts.remove(action); + } + } + if (getTotalActionRegistrationCounts() > 0) { + IntentFilter filter = new IntentFilter(); + mActionRegistrationCounts.entrySet().stream() + .filter(entry -> entry.getValue() > 0) + .forEach( + entry -> { + Log.d( + TAG, + "unregisterIntentActions(): Registering action = " + + entry.getKey()); + filter.addAction(entry.getKey()); + }); + sTargetContext.registerReceiver(mReceiver, filter); + } + } + + /** + * Get sum of reference count from all registered actions + * + * @return sum of reference count from all registered actions + */ + private int getTotalActionRegistrationCounts() { + return mActionRegistrationCounts.values().stream().reduce(0, Integer::sum); + } } -- GitLab