From c11af01481dd7fb4515e681128340724f2008dc0 Mon Sep 17 00:00:00 2001 From: Jack He Date: Tue, 30 May 2017 23:05:46 -0700 Subject: [PATCH] Bluetooth: Always scan while on pairing or DevicePicker page * Modified DeviceListPreferenceFragment to have enable/disable scanning methods. In ENABLE state, each onScanningStateChanged(false) will restart another round of scanning * Subclasses of DeviceListPreferenceFragment should call enable/disable scanning when scanning is needed for long period of time * Currently, BluetoothPairingDetail and DevicePickerFragment call enableScanning() when Bluetooth is turned ON and call disableScanning when some device is picked in their lists * Both BluetoothPairingDetail and DevicePickerFragment will re-enable scanning if pairing failed for selected device * Added associated unit tests as well Bug: 32172815 Test: make, pair Bluetooth device, send file over Bluetooth Opp Change-Id: I99325e06aadd7b00e7a7ba6d6c282a6831859d8b --- .../bluetooth/BluetoothPairingDetail.java | 39 +++++--- .../DeviceListPreferenceFragment.java | 22 ++++- .../bluetooth/DevicePickerFragment.java | 49 ++++------ .../bluetooth/BluetoothPairingDetailTest.java | 54 ++++++++++- .../DeviceListPreferenceFragmentTest.java | 97 +++++++++++++++++-- 5 files changed, 210 insertions(+), 51 deletions(-) diff --git a/src/com/android/settings/bluetooth/BluetoothPairingDetail.java b/src/com/android/settings/bluetooth/BluetoothPairingDetail.java index 7e2978d8303..987fd3b21c1 100644 --- a/src/com/android/settings/bluetooth/BluetoothPairingDetail.java +++ b/src/com/android/settings/bluetooth/BluetoothPairingDetail.java @@ -82,7 +82,7 @@ public class BluetoothPairingDetail extends DeviceListPreferenceFragment impleme // Make the device only visible to connected devices. mLocalAdapter.setScanMode(BluetoothAdapter.SCAN_MODE_CONNECTABLE); - mLocalAdapter.stopScanning(); + disableScanning(); } @Override @@ -98,25 +98,29 @@ public class BluetoothPairingDetail extends DeviceListPreferenceFragment impleme return MetricsEvent.BLUETOOTH; } - @VisibleForTesting - void startScanning() { - if (mAvailableDevicesCategory != null) { - removeAllDevices(); + @Override + void enableScanning() { + // Clear all device states before first scan + if (!mInitialScanStarted) { + if (mAvailableDevicesCategory != null) { + removeAllDevices(); + } + mLocalManager.getCachedDeviceManager().clearNonBondedDevices(); + mInitialScanStarted = true; } - - mLocalManager.getCachedDeviceManager().clearNonBondedDevices(); - mInitialScanStarted = true; - mLocalAdapter.startScanning(true); + super.enableScanning(); } @Override void onDevicePreferenceClick(BluetoothDevicePreference btPreference) { - mLocalAdapter.stopScanning(); + disableScanning(); super.onDevicePreferenceClick(btPreference); } @Override public void onScanningStateChanged(boolean started) { + super.onScanningStateChanged(started); + started |= mScanEnabled; mAvailableDevicesCategory.setProgress(started); } @@ -131,14 +135,10 @@ public class BluetoothPairingDetail extends DeviceListPreferenceFragment impleme R.string.bluetooth_preference_found_devices, BluetoothDeviceFilter.UNBONDED_DEVICE_FILTER, mInitialScanStarted); updateFooterPreference(mFooterPreference); - - if (!mInitialScanStarted) { - startScanning(); - } - // mLocalAdapter.setScanMode is internally synchronized so it is okay for multiple // threads to execute. mLocalAdapter.setScanMode(BluetoothAdapter.SCAN_MODE_CONNECTABLE_DISCOVERABLE); + enableScanning(); break; case BluetoothAdapter.STATE_OFF: @@ -158,6 +158,15 @@ public class BluetoothPairingDetail extends DeviceListPreferenceFragment impleme if (bondState == BluetoothDevice.BOND_BONDED) { // If one device is connected(bonded), then close this fragment. finish(); + return; + } + if (mSelectedDevice != null && cachedDevice != null) { + BluetoothDevice device = cachedDevice.getDevice(); + if (device != null && mSelectedDevice.equals(device) + && bondState == BluetoothDevice.BOND_NONE) { + // If currently selected device failed to bond, restart scanning + enableScanning(); + } } } diff --git a/src/com/android/settings/bluetooth/DeviceListPreferenceFragment.java b/src/com/android/settings/bluetooth/DeviceListPreferenceFragment.java index 714704e186b..ca06e3c306d 100644 --- a/src/com/android/settings/bluetooth/DeviceListPreferenceFragment.java +++ b/src/com/android/settings/bluetooth/DeviceListPreferenceFragment.java @@ -54,6 +54,9 @@ public abstract class DeviceListPreferenceFragment extends private BluetoothDeviceFilter.Filter mFilter; + @VisibleForTesting + boolean mScanEnabled; + BluetoothDevice mSelectedDevice; LocalBluetoothAdapter mLocalAdapter; @@ -216,8 +219,25 @@ public abstract class DeviceListPreferenceFragment extends } } + @VisibleForTesting + void enableScanning() { + // LocalBluetoothAdapter already handles repeated scan requests + mLocalAdapter.startScanning(true); + mScanEnabled = true; + } + + @VisibleForTesting + void disableScanning() { + mLocalAdapter.stopScanning(); + mScanEnabled = false; + } + @Override - public void onScanningStateChanged(boolean started) {} + public void onScanningStateChanged(boolean started) { + if (!started && mScanEnabled) { + mLocalAdapter.startScanning(true); + } + } @Override public void onBluetoothStateChanged(int bluetoothState) {} diff --git a/src/com/android/settings/bluetooth/DevicePickerFragment.java b/src/com/android/settings/bluetooth/DevicePickerFragment.java index 0cf13a3b5b3..60470c53022 100644 --- a/src/com/android/settings/bluetooth/DevicePickerFragment.java +++ b/src/com/android/settings/bluetooth/DevicePickerFragment.java @@ -25,7 +25,6 @@ import android.os.Bundle; import android.os.UserManager; import android.view.Menu; import android.view.MenuInflater; -import android.view.MenuItem; import com.android.internal.logging.nano.MetricsProto.MetricsEvent; import com.android.settings.R; @@ -41,7 +40,6 @@ import java.util.List; * connection management. */ public final class DevicePickerFragment extends DeviceListPreferenceFragment { - private static final int MENU_ID_REFRESH = Menu.FIRST; private static final String KEY_BT_DEVICE_LIST = "bt_device_list"; private static final String TAG = "DevicePickerFragment"; @@ -52,7 +50,7 @@ public final class DevicePickerFragment extends DeviceListPreferenceFragment { private boolean mNeedAuth; private String mLaunchPackage; private String mLaunchClass; - private boolean mStartScanOnStart; + private boolean mScanAllowed; @Override void initPreferencesFromPreferenceScreen() { @@ -66,22 +64,9 @@ public final class DevicePickerFragment extends DeviceListPreferenceFragment { @Override public void onCreateOptionsMenu(Menu menu, MenuInflater inflater) { - menu.add(Menu.NONE, MENU_ID_REFRESH, 0, R.string.bluetooth_search_for_devices) - .setEnabled(true) - .setShowAsAction(MenuItem.SHOW_AS_ACTION_NEVER); super.onCreateOptionsMenu(menu, inflater); } - @Override - public boolean onOptionsItemSelected(MenuItem item) { - switch (item.getItemId()) { - case MENU_ID_REFRESH: - mLocalAdapter.startScanning(true); - return true; - } - return super.onOptionsItemSelected(item); - } - @Override public int getMetricsCategory() { return MetricsEvent.BLUETOOTH_DEVICE_PICKER; @@ -92,8 +77,7 @@ public final class DevicePickerFragment extends DeviceListPreferenceFragment { super.onCreate(savedInstanceState); getActivity().setTitle(getString(R.string.device_picker)); UserManager um = (UserManager) getSystemService(Context.USER_SERVICE); - mStartScanOnStart = !um.hasUserRestriction(DISALLOW_CONFIG_BLUETOOTH) - && (savedInstanceState == null); // don't start scan after rotation + mScanAllowed = !um.hasUserRestriction(DISALLOW_CONFIG_BLUETOOTH); setHasOptionsMenu(true); } @@ -102,12 +86,18 @@ public final class DevicePickerFragment extends DeviceListPreferenceFragment { super.onStart(); addCachedDevices(); mSelectedDevice = null; - if (mStartScanOnStart) { - mLocalAdapter.startScanning(true); - mStartScanOnStart = false; + if (mScanAllowed) { + enableScanning(); } } + @Override + public void onStop() { + // Try disable scanning no matter what, no effect if enableScanning has not been called + disableScanning(); + super.onStop(); + } + @Override public void onDestroy() { super.onDestroy(); @@ -121,7 +111,7 @@ public final class DevicePickerFragment extends DeviceListPreferenceFragment { @Override void onDevicePreferenceClick(BluetoothDevicePreference btPreference) { - mLocalAdapter.stopScanning(); + disableScanning(); LocalBluetoothPreferences.persistSelectedDeviceInPicker( getActivity(), mSelectedDevice.getAddress()); if ((btPreference.getCachedDevice().getBondState() == @@ -135,12 +125,15 @@ public final class DevicePickerFragment extends DeviceListPreferenceFragment { public void onDeviceBondStateChanged(CachedBluetoothDevice cachedDevice, int bondState) { + BluetoothDevice device = cachedDevice.getDevice(); + if (!device.equals(mSelectedDevice)) { + return; + } if (bondState == BluetoothDevice.BOND_BONDED) { - BluetoothDevice device = cachedDevice.getDevice(); - if (device.equals(mSelectedDevice)) { - sendDevicePickedIntent(device); - finish(); - } + sendDevicePickedIntent(device); + finish(); + } else if (bondState == BluetoothDevice.BOND_NONE) { + enableScanning(); } } @@ -149,7 +142,7 @@ public final class DevicePickerFragment extends DeviceListPreferenceFragment { super.onBluetoothStateChanged(bluetoothState); if (bluetoothState == BluetoothAdapter.STATE_ON) { - mLocalAdapter.startScanning(false); + enableScanning(); } } diff --git a/tests/robotests/src/com/android/settings/bluetooth/BluetoothPairingDetailTest.java b/tests/robotests/src/com/android/settings/bluetooth/BluetoothPairingDetailTest.java index 6774ba9a060..d19666bfca7 100644 --- a/tests/robotests/src/com/android/settings/bluetooth/BluetoothPairingDetailTest.java +++ b/tests/robotests/src/com/android/settings/bluetooth/BluetoothPairingDetailTest.java @@ -23,7 +23,9 @@ import static org.mockito.Matchers.anyBoolean; import static org.mockito.Matchers.anyInt; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import android.bluetooth.BluetoothAdapter; @@ -103,7 +105,7 @@ public class BluetoothPairingDetailTest { mFragment.mAvailableDevicesCategory = mAvailableDevicesCategory; mFragment.mDeviceListGroup = mAvailableDevicesCategory; - mFragment.startScanning(); + mFragment.enableScanning(); verify(mLocalAdapter).startScanning(true); verify(mAvailableDevicesCategory).removeAll(); @@ -130,4 +132,54 @@ public class BluetoothPairingDetailTest { verify(mFragment).finish(); } + @Test + public void testOnScanningStateChanged_restartScanAfterInitialScanning() { + mFragment.mAvailableDevicesCategory = mAvailableDevicesCategory; + mFragment.mFooterPreference = mFooterPreference; + mFragment.mDeviceListGroup = mAvailableDevicesCategory; + doNothing().when(mFragment).addDeviceCategory(any(), anyInt(), any(), anyBoolean()); + + // Initial Bluetooth ON will trigger scan enable, list clear and scan start + mFragment.updateContent(BluetoothAdapter.STATE_ON); + verify(mFragment).enableScanning(); + assertThat(mAvailableDevicesCategory.getPreferenceCount()).isEqualTo(0); + verify(mLocalAdapter).startScanning(true); + + // Subsequent scan started event will not trigger start/stop nor list clear + mFragment.onScanningStateChanged(true); + verify(mLocalAdapter, times(1)).startScanning(anyBoolean()); + verify(mAvailableDevicesCategory, times(1)).setProgress(true); + + // Subsequent scan finished event will trigger scan start without list clean + mFragment.onScanningStateChanged(false); + verify(mLocalAdapter, times(2)).startScanning(true); + verify(mAvailableDevicesCategory, times(2)).setProgress(true); + + // Subsequent scan started event will not trigger any change + mFragment.onScanningStateChanged(true); + verify(mLocalAdapter, times(2)).startScanning(anyBoolean()); + verify(mAvailableDevicesCategory, times(3)).setProgress(true); + verify(mLocalAdapter, never()).stopScanning(); + + // Disable scanning will trigger scan stop + mFragment.disableScanning(); + verify(mLocalAdapter, times(1)).stopScanning(); + + // Subsequent scan start event will not trigger any change besides progress circle + mFragment.onScanningStateChanged(true); + verify(mAvailableDevicesCategory, times(4)).setProgress(true); + + // However, subsequent scan finished event won't trigger new scan start and will stop + // progress circle from spinning + mFragment.onScanningStateChanged(false); + verify(mAvailableDevicesCategory, times(1)).setProgress(false); + verify(mLocalAdapter, times(2)).startScanning(anyBoolean()); + verify(mLocalAdapter, times(1)).stopScanning(); + + // Verify that clean up only happen once at initialization + verify(mAvailableDevicesCategory, times(1)).removeAll(); + } + + + } diff --git a/tests/robotests/src/com/android/settings/bluetooth/DeviceListPreferenceFragmentTest.java b/tests/robotests/src/com/android/settings/bluetooth/DeviceListPreferenceFragmentTest.java index bbb73597e98..d9936d9c478 100644 --- a/tests/robotests/src/com/android/settings/bluetooth/DeviceListPreferenceFragmentTest.java +++ b/tests/robotests/src/com/android/settings/bluetooth/DeviceListPreferenceFragmentTest.java @@ -19,9 +19,12 @@ package com.android.settings.bluetooth; import static com.google.common.truth.Truth.assertThat; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyBoolean; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import android.content.Context; import android.content.res.Resources; @@ -83,6 +86,92 @@ public class DeviceListPreferenceFragmentTest { assertThat(mMyDevicePreference.getTitle()).isEqualTo(FOOTAGE_MAC_STRING); } + @Test + public void testEnableDisableScanning_testStateAfterEanbleDisable() { + mFragment.enableScanning(); + verify(mLocalAdapter).startScanning(true); + assertThat(mFragment.mScanEnabled).isTrue(); + + mFragment.disableScanning(); + verify(mLocalAdapter).stopScanning(); + assertThat(mFragment.mScanEnabled).isFalse(); + } + + @Test + public void testScanningStateChanged_testScanStarted() { + mFragment.enableScanning(); + assertThat(mFragment.mScanEnabled).isTrue(); + verify(mLocalAdapter).startScanning(true); + + mFragment.onScanningStateChanged(true); + verify(mLocalAdapter, times(1)).startScanning(anyBoolean()); + } + + @Test + public void testScanningStateChanged_testScanFinished() { + // Could happen when last scanning not done while current scan gets enabled + mFragment.enableScanning(); + verify(mLocalAdapter).startScanning(true); + assertThat(mFragment.mScanEnabled).isTrue(); + + mFragment.onScanningStateChanged(false); + verify(mLocalAdapter, times(2)).startScanning(true); + } + + @Test + public void testScanningStateChanged_testScanStateMultiple() { + // Could happen when last scanning not done while current scan gets enabled + mFragment.enableScanning(); + assertThat(mFragment.mScanEnabled).isTrue(); + verify(mLocalAdapter).startScanning(true); + + mFragment.onScanningStateChanged(true); + verify(mLocalAdapter, times(1)).startScanning(anyBoolean()); + + mFragment.onScanningStateChanged(false); + verify(mLocalAdapter, times(2)).startScanning(true); + + mFragment.onScanningStateChanged(true); + verify(mLocalAdapter, times(2)).startScanning(anyBoolean()); + + mFragment.disableScanning(); + verify(mLocalAdapter).stopScanning(); + + mFragment.onScanningStateChanged(false); + verify(mLocalAdapter, times(2)).startScanning(anyBoolean()); + + mFragment.onScanningStateChanged(true); + verify(mLocalAdapter, times(2)).startScanning(anyBoolean()); + } + + @Test + public void testScanningStateChanged_testScanFinishedAfterDisable() { + mFragment.enableScanning(); + verify(mLocalAdapter).startScanning(true); + assertThat(mFragment.mScanEnabled).isTrue(); + + mFragment.disableScanning(); + verify(mLocalAdapter).stopScanning(); + assertThat(mFragment.mScanEnabled).isFalse(); + + mFragment.onScanningStateChanged(false); + verify(mLocalAdapter, times(1)).startScanning(anyBoolean()); + } + + @Test + public void testScanningStateChanged_testScanStartedAfterDisable() { + mFragment.enableScanning(); + verify(mLocalAdapter).startScanning(true); + assertThat(mFragment.mScanEnabled).isTrue(); + + mFragment.disableScanning(); + verify(mLocalAdapter).stopScanning(); + assertThat(mFragment.mScanEnabled).isFalse(); + + mFragment.onScanningStateChanged(true); + verify(mLocalAdapter, times(1)).startScanning(anyBoolean()); + } + /** * Fragment to test since {@code DeviceListPreferenceFragment} is abstract */ @@ -98,14 +187,10 @@ public class DeviceListPreferenceFragmentTest { } @Override - public void onDeviceBondStateChanged(CachedBluetoothDevice cachedDevice, int bondState) { - - } + public void onDeviceBondStateChanged(CachedBluetoothDevice cachedDevice, int bondState) {} @Override - void initPreferencesFromPreferenceScreen() { - - } + void initPreferencesFromPreferenceScreen() {} @Override public String getDeviceListKey() {