From c86440538fb156718228299bd44698e24995632e Mon Sep 17 00:00:00 2001 From: Antony Sargent Date: Wed, 9 Aug 2017 15:57:17 -0700 Subject: [PATCH] Stay discoverable in Bluetooth settings and pairing pages There are two problems with the Bluetooth settings and pairing pages that are fixed by this CL: (1) We advertise on the page that the local device is visible to other devices, but that only lasts for the length of the default timeout (120 seconds) for the local adapter being in discoverable mode. (2) Both the BluetoothSettings and BluetoothPairingDetail fragments enter discoverable mode in their onStart handler and exit it in their onStop handler. Unfortunately when doing a fragment navigation the onStart and onStop events interleave in a non-intuitive manner. When you go from BluetoothSettings to BluetoothPairingDetail, we see the onStop event for BluetoothSettings *after* the onStart event for BluetoothPairingDetail, and similarly when going back from BluetoothSettings to BluetoothPairingDetail. What this means in practice is that if you go to the BluetoothSettings page, the device will be discoverable, but once you navigate to BluetoothPairingDetail or back again you won't be discoverable again until you go somewhere else or end the settings activity. This CL adds a new object called AlwaysDiscoverable which can be used to start and stop a mode of "always being discoverable". While started, it will listen for changes to the discoverable state, and return to discoverable mode. This fixes (1) by returning to discoverable mode whenever the normal timeout expires, and (2) similary by returning to discoverable mode when we accidentally exit it due to the onStop/onStart mismatch. A better fix for (2) would be to avoid the "glitch" of briefly exiting discoverable mode only to re-enter it, but the implementation of that is a little more complicated so that's being left as future work in order to keep this CL as small as possible. Bug: 64130265 Test: make RunSettingsRoboTests Change-Id: I559dd8187263ea6a0008be1a8abdfffac97cb87a --- .../bluetooth/AlwaysDiscoverable.java | 87 +++++++++++++ .../bluetooth/BluetoothPairingDetail.java | 9 +- .../settings/bluetooth/BluetoothSettings.java | 12 +- .../bluetooth/AlwaysDiscoverableTest.java | 120 ++++++++++++++++++ .../bluetooth/BluetoothPairingDetailTest.java | 1 + 5 files changed, 223 insertions(+), 6 deletions(-) create mode 100644 src/com/android/settings/bluetooth/AlwaysDiscoverable.java create mode 100644 tests/robotests/src/com/android/settings/bluetooth/AlwaysDiscoverableTest.java diff --git a/src/com/android/settings/bluetooth/AlwaysDiscoverable.java b/src/com/android/settings/bluetooth/AlwaysDiscoverable.java new file mode 100644 index 00000000000..5d7cbd59faa --- /dev/null +++ b/src/com/android/settings/bluetooth/AlwaysDiscoverable.java @@ -0,0 +1,87 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.settings.bluetooth; + +import android.bluetooth.BluetoothAdapter; +import android.content.BroadcastReceiver; +import android.content.Context; +import android.content.Intent; +import android.content.IntentFilter; +import android.os.Handler; +import android.support.annotation.VisibleForTesting; +import android.util.Log; + +import com.android.settingslib.bluetooth.LocalBluetoothAdapter; + +import java.util.Timer; +import java.util.TimerTask; + +/** Helper class, intended to be used by an Activity, to keep the local Bluetooth adapter in + * discoverable mode indefinitely. By default setting the scan mode to + * BluetoothAdapter.SCAN_MODE_CONNECTABLE_DISCOVERABLE will time out after some time, but some + * Bluetooth settings pages would like to keep the device discoverable as long as the page is + * visible. */ +public class AlwaysDiscoverable extends BroadcastReceiver { + private static final String TAG = "AlwaysDiscoverable"; + + private Context mContext; + private LocalBluetoothAdapter mLocalAdapter; + private IntentFilter mIntentFilter; + + @VisibleForTesting + boolean mStarted; + + public AlwaysDiscoverable(Context context, LocalBluetoothAdapter localAdapter) { + mContext = context; + mLocalAdapter = localAdapter; + mIntentFilter = new IntentFilter(); + mIntentFilter.addAction(BluetoothAdapter.ACTION_SCAN_MODE_CHANGED); + } + + /** After calling start(), consumers should make a matching call to stop() when they no longer + * wish to enforce discoverable mode. */ + public void start() { + if (mStarted) { + return; + } + mContext.registerReceiver(this, mIntentFilter); + mStarted = true; + if (mLocalAdapter.getScanMode() != BluetoothAdapter.SCAN_MODE_CONNECTABLE_DISCOVERABLE) { + mLocalAdapter.setScanMode(BluetoothAdapter.SCAN_MODE_CONNECTABLE_DISCOVERABLE); + } + } + + public void stop() { + if (!mStarted) { + return; + } + mContext.unregisterReceiver(this); + mStarted = false; + mLocalAdapter.setScanMode(BluetoothAdapter.SCAN_MODE_CONNECTABLE); + } + + @Override + public void onReceive(Context context, Intent intent) { + String action = intent.getAction(); + if (action != BluetoothAdapter.ACTION_SCAN_MODE_CHANGED) { + return; + } + if (mLocalAdapter.getScanMode() != BluetoothAdapter.SCAN_MODE_CONNECTABLE_DISCOVERABLE) { + mLocalAdapter.setScanMode(BluetoothAdapter.SCAN_MODE_CONNECTABLE_DISCOVERABLE); + } + } +} diff --git a/src/com/android/settings/bluetooth/BluetoothPairingDetail.java b/src/com/android/settings/bluetooth/BluetoothPairingDetail.java index 2e7a7fb7e79..fee37d07e2b 100644 --- a/src/com/android/settings/bluetooth/BluetoothPairingDetail.java +++ b/src/com/android/settings/bluetooth/BluetoothPairingDetail.java @@ -53,6 +53,8 @@ public class BluetoothPairingDetail extends DeviceListPreferenceFragment impleme BluetoothProgressCategory mAvailableDevicesCategory; @VisibleForTesting FooterPreference mFooterPreference; + @VisibleForTesting + AlwaysDiscoverable mAlwaysDiscoverable; private boolean mInitialScanStarted; @@ -64,6 +66,7 @@ public class BluetoothPairingDetail extends DeviceListPreferenceFragment impleme public void onActivityCreated(Bundle savedInstanceState) { super.onActivityCreated(savedInstanceState); mInitialScanStarted = false; + mAlwaysDiscoverable = new AlwaysDiscoverable(getContext(), mLocalAdapter); } @Override @@ -79,7 +82,7 @@ public class BluetoothPairingDetail extends DeviceListPreferenceFragment impleme super.onStop(); // Make the device only visible to connected devices. - mLocalAdapter.setScanMode(BluetoothAdapter.SCAN_MODE_CONNECTABLE); + mAlwaysDiscoverable.stop(); disableScanning(); } @@ -132,9 +135,7 @@ public class BluetoothPairingDetail extends DeviceListPreferenceFragment impleme R.string.bluetooth_preference_found_devices, BluetoothDeviceFilter.UNBONDED_DEVICE_FILTER, mInitialScanStarted); updateFooterPreference(mFooterPreference); - // mLocalAdapter.setScanMode is internally synchronized so it is okay for multiple - // threads to execute. - mLocalAdapter.setScanMode(BluetoothAdapter.SCAN_MODE_CONNECTABLE_DISCOVERABLE); + mAlwaysDiscoverable.start(); enableScanning(); break; diff --git a/src/com/android/settings/bluetooth/BluetoothSettings.java b/src/com/android/settings/bluetooth/BluetoothSettings.java index a5019804fae..aeedc724e7c 100644 --- a/src/com/android/settings/bluetooth/BluetoothSettings.java +++ b/src/com/android/settings/bluetooth/BluetoothSettings.java @@ -84,6 +84,7 @@ public class BluetoothSettings extends DeviceListPreferenceFragment implements I FooterPreference mFooterPreference; private Preference mPairingPreference; private BluetoothEnabler mBluetoothEnabler; + private AlwaysDiscoverable mAlwaysDiscoverable; private SwitchBar mSwitchBar; @@ -115,6 +116,9 @@ public class BluetoothSettings extends DeviceListPreferenceFragment implements I mMetricsFeatureProvider, Utils.getLocalBtManager(activity), MetricsEvent.ACTION_BLUETOOTH_TOGGLE); mBluetoothEnabler.setupSwitchController(); + if (mLocalAdapter != null) { + mAlwaysDiscoverable = new AlwaysDiscoverable(getContext(), mLocalAdapter); + } } @Override @@ -161,7 +165,9 @@ public class BluetoothSettings extends DeviceListPreferenceFragment implements I } // Make the device only visible to connected devices. - mLocalAdapter.setScanMode(BluetoothAdapter.SCAN_MODE_CONNECTABLE); + if (mAlwaysDiscoverable != null) { + mAlwaysDiscoverable.stop(); + } if (isUiRestricted()) { return; @@ -192,7 +198,9 @@ public class BluetoothSettings extends DeviceListPreferenceFragment implements I mPairedDevicesCategory.addPreference(mPairingPreference); updateFooterPreference(mFooterPreference); - mLocalAdapter.setScanMode(BluetoothAdapter.SCAN_MODE_CONNECTABLE_DISCOVERABLE); + if (mAlwaysDiscoverable != null) { + mAlwaysDiscoverable.start(); + } return; // not break case BluetoothAdapter.STATE_TURNING_OFF: diff --git a/tests/robotests/src/com/android/settings/bluetooth/AlwaysDiscoverableTest.java b/tests/robotests/src/com/android/settings/bluetooth/AlwaysDiscoverableTest.java new file mode 100644 index 00000000000..fd46b4b3727 --- /dev/null +++ b/tests/robotests/src/com/android/settings/bluetooth/AlwaysDiscoverableTest.java @@ -0,0 +1,120 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.android.settings.bluetooth; + +import static com.google.common.truth.Truth.assertThat; + +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyInt; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import android.bluetooth.BluetoothAdapter; +import android.content.Context; +import android.content.Intent; + +import com.android.settings.TestConfig; +import com.android.settings.testutils.SettingsRobolectricTestRunner; +import com.android.settingslib.bluetooth.LocalBluetoothAdapter; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.robolectric.annotation.Config; + +@RunWith(SettingsRobolectricTestRunner.class) +@Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION) +public class AlwaysDiscoverableTest { + @Mock + private LocalBluetoothAdapter mLocalAdapter; + + @Mock + private Context mContext; + + private AlwaysDiscoverable mAlwaysDiscoverable; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + mAlwaysDiscoverable = new AlwaysDiscoverable(mContext, mLocalAdapter); + } + + @Test + public void isStartedWithoutStart() { + assertThat(mAlwaysDiscoverable.mStarted).isFalse(); + } + + @Test + public void isStartedWithStart() { + mAlwaysDiscoverable.start(); + assertThat(mAlwaysDiscoverable.mStarted).isTrue(); + } + + @Test + public void isStartedWithStartStop() { + mAlwaysDiscoverable.start(); + mAlwaysDiscoverable.stop(); + assertThat(mAlwaysDiscoverable.mStarted).isFalse(); + } + + @Test + public void stopWithoutStart() { + mAlwaysDiscoverable.stop(); + // expect no crash + verify(mLocalAdapter, never()).setScanMode(anyInt()); + } + + @Test + public void startSetsModeAndRegistersReceiver() { + when(mLocalAdapter.getScanMode()).thenReturn(BluetoothAdapter.SCAN_MODE_NONE); + mAlwaysDiscoverable.start(); + verify(mLocalAdapter).setScanMode(eq(BluetoothAdapter.SCAN_MODE_CONNECTABLE_DISCOVERABLE)); + verify(mContext).registerReceiver(eq(mAlwaysDiscoverable), any()); + } + + @Test + public void stopUnregistersReceiver() { + mAlwaysDiscoverable.start(); + mAlwaysDiscoverable.stop(); + verify(mContext).unregisterReceiver(mAlwaysDiscoverable); + } + + @Test + public void resetsToDiscoverableModeWhenScanModeChanges() { + mAlwaysDiscoverable.start(); + verify(mLocalAdapter, times(1)).setScanMode( + BluetoothAdapter.SCAN_MODE_CONNECTABLE_DISCOVERABLE); + + sendScanModeChangedIntent(BluetoothAdapter.SCAN_MODE_CONNECTABLE, + BluetoothAdapter.SCAN_MODE_CONNECTABLE_DISCOVERABLE); + + verify(mLocalAdapter, times(2)).setScanMode( + BluetoothAdapter.SCAN_MODE_CONNECTABLE_DISCOVERABLE); + } + + private void sendScanModeChangedIntent(int newMode, int previousMode) { + when(mLocalAdapter.getScanMode()).thenReturn(newMode); + Intent intent = new Intent(BluetoothAdapter.ACTION_SCAN_MODE_CHANGED); + intent.putExtra(BluetoothAdapter.EXTRA_SCAN_MODE, newMode); + intent.putExtra(BluetoothAdapter.EXTRA_PREVIOUS_SCAN_MODE, previousMode); + mAlwaysDiscoverable.onReceive(mContext, intent); + } +} diff --git a/tests/robotests/src/com/android/settings/bluetooth/BluetoothPairingDetailTest.java b/tests/robotests/src/com/android/settings/bluetooth/BluetoothPairingDetailTest.java index c4772e5e8e3..d1d49354403 100644 --- a/tests/robotests/src/com/android/settings/bluetooth/BluetoothPairingDetailTest.java +++ b/tests/robotests/src/com/android/settings/bluetooth/BluetoothPairingDetailTest.java @@ -85,6 +85,7 @@ public class BluetoothPairingDetailTest { mFragment.mLocalAdapter = mLocalAdapter; mFragment.mLocalManager = mLocalManager; mFragment.mDeviceListGroup = mPreferenceGroup; + mFragment.mAlwaysDiscoverable = new AlwaysDiscoverable(mContext, mLocalAdapter); } @Test