diff options
| -rw-r--r-- | service/Android.bp | 2 | ||||
| -rwxr-xr-x | service/generate_local_coverage.sh | 3 | ||||
| -rw-r--r-- | service/src/AutoOnFeature.kt | 2 | ||||
| -rw-r--r-- | service/src/AutoOnFeatureTest.kt | 2 | ||||
| -rw-r--r-- | service/src/airplane/ModeListener.kt | 39 | ||||
| -rw-r--r-- | service/src/airplane/ModeListenerTest.kt | 86 | ||||
| -rw-r--r-- | service/src/com/android/server/bluetooth/BluetoothManagerService.java | 42 | ||||
| -rw-r--r-- | service/src/com/android/server/bluetooth/BluetoothShellCommand.java | 52 |
8 files changed, 174 insertions, 54 deletions
diff --git a/service/Android.bp b/service/Android.bp index 9770b368bd..bd4867aa93 100644 --- a/service/Android.bp +++ b/service/Android.bp @@ -185,6 +185,8 @@ android_robolectric_test { static_libs: [ "androidx.test.core", "androidx.test.ext.truth", + "bluetooth_flags_java_lib", + "flag-junit", "kotlin-test", "kotlinx_coroutines", "kotlinx_coroutines_test", diff --git a/service/generate_local_coverage.sh b/service/generate_local_coverage.sh index 83e1138cbd..b60a8463b2 100755 --- a/service/generate_local_coverage.sh +++ b/service/generate_local_coverage.sh @@ -27,7 +27,8 @@ rm ${COVERAGE_TMP_FOLDER}/ServiceBluetoothRobo_unzip/com/android/server/bluetoot rm ${COVERAGE_TMP_FOLDER}/ServiceBluetoothRobo_unzip/com/android/server/bluetooth/R.class # Generate report: -java -jar "${ANDROID_BUILD_TOP}"/out/dist/jacoco-cli.jar report "${COVERAGE_COLLECTED}"/invocation_*/inv_*/coverage_*.ec --classfiles ${COVERAGE_TMP_FOLDER}/ServiceBluetoothRobo_unzip --html ${COVERAGE_TMP_FOLDER}/coverage --name coverage.html --sourcefiles ${COVERAGE_TMP_FOLDER} +# You may want to run "m jacoco-cli" if above command failed +java -jar "${ANDROID_BUILD_TOP}"/out/host/linux-x86/framework/jacoco-cli.jar report "${COVERAGE_COLLECTED}"/invocation_*/inv_*/coverage_*.ec --classfiles ${COVERAGE_TMP_FOLDER}/ServiceBluetoothRobo_unzip --html ${COVERAGE_TMP_FOLDER}/coverage --name coverage.html --sourcefiles ${COVERAGE_TMP_FOLDER} # Start python server and expose URL printf "Url to connect to the coverage \033[32m http://%s:8000 \033[0m\n" "$(hostname)" diff --git a/service/src/AutoOnFeature.kt b/service/src/AutoOnFeature.kt index e8c96c589c..6181e3df6c 100644 --- a/service/src/AutoOnFeature.kt +++ b/service/src/AutoOnFeature.kt @@ -39,7 +39,7 @@ import androidx.annotation.RequiresApi import androidx.annotation.VisibleForTesting import com.android.modules.expresslog.Counter import com.android.server.bluetooth.airplane.hasUserToggledApm as hasUserToggledApm -import com.android.server.bluetooth.airplane.isOn as isAirplaneModeOn +import com.android.server.bluetooth.airplane.isOnOverrode as isAirplaneModeOn import com.android.server.bluetooth.satellite.isOn as isSatelliteModeOn import java.time.LocalDateTime import java.time.LocalTime diff --git a/service/src/AutoOnFeatureTest.kt b/service/src/AutoOnFeatureTest.kt index 4a490c2119..13febe228b 100644 --- a/service/src/AutoOnFeatureTest.kt +++ b/service/src/AutoOnFeatureTest.kt @@ -28,7 +28,7 @@ import com.android.server.bluetooth.BluetoothAdapterState import com.android.server.bluetooth.Log import com.android.server.bluetooth.Timer import com.android.server.bluetooth.USER_SETTINGS_KEY -import com.android.server.bluetooth.airplane.isOn as isAirplaneModeOn +import com.android.server.bluetooth.airplane.isOnOverrode as isAirplaneModeOn import com.android.server.bluetooth.airplane.test.ModeListenerTest as AirplaneListener import com.android.server.bluetooth.isUserEnabled import com.android.server.bluetooth.isUserSupported diff --git a/service/src/airplane/ModeListener.kt b/service/src/airplane/ModeListener.kt index 2d92e6c1c6..ded7738de3 100644 --- a/service/src/airplane/ModeListener.kt +++ b/service/src/airplane/ModeListener.kt @@ -27,6 +27,7 @@ import android.os.Looper import android.provider.Settings import android.widget.Toast import com.android.bluetooth.BluetoothStatsLog +import com.android.bluetooth.flags.Flags import com.android.server.bluetooth.BluetoothAdapterState import com.android.server.bluetooth.Log import com.android.server.bluetooth.initializeRadioModeListener @@ -36,7 +37,16 @@ import kotlin.time.TimeSource private const val TAG = "AirplaneModeListener" -/** @return true if Bluetooth state is impacted by airplane mode */ +/** @return true if Bluetooth state is currently impacted by airplane mode */ +public var isOnOverrode = false + private set + +/** + * @return true if airplane is ON on the device. + * + * This need to be used instead of reading the settings properties to avoid race condition from + * within the BluetoothManagerService thread + */ public var isOn = false private set @@ -80,11 +90,12 @@ public fun initialize( Settings.Global.AIRPLANE_MODE_RADIOS, Settings.Global.AIRPLANE_MODE_ON, fun(newMode: Boolean) { - val previousMode = isOn + isOn = newMode + val previousMode = isOnOverrode val isBluetoothOn = state.oneOf(STATE_ON, STATE_TURNING_ON, STATE_TURNING_OFF) val isMediaConnected = isBluetoothOn && mediaCallback() - isOn = + isOnOverrode = airplaneModeValueOverride( systemResolver, newMode, @@ -103,17 +114,25 @@ public fun initialize( timeSource.markNow(), ) - if (previousMode == isOn) { - Log.d(TAG, "Ignore airplane mode change because is already: $isOn") + val description = "isOn=$isOn, isOnOverrode=$isOnOverrode" + + if (previousMode == isOnOverrode) { + Log.d(TAG, "Ignore mode change to same state. $description") + return + } else if ( + Flags.airplaneModeXBleOn() && isOnOverrode == false && state.oneOf(STATE_ON) + ) { + Log.d(TAG, "Ignore mode change as Bluetooth is ON. $description") return } - Log.i(TAG, "Trigger callback with state: $isOn") - modeCallback(isOn) + Log.i(TAG, "Trigger callback. $description") + modeCallback(isOnOverrode) } ) - isOn = + isOn = airplaneModeAtBoot + isOnOverrode = airplaneModeValueOverride( systemResolver, airplaneModeAtBoot, @@ -132,7 +151,7 @@ public fun initialize( false, timeSource.markNow(), ) - Log.i(TAG, "Initialized successfully with state: $isOn") + Log.i(TAG, "Init completed. isOn=$isOn, isOnOverrode=$isOnOverrode") } @kotlin.time.ExperimentalTime @@ -263,7 +282,7 @@ private class AirplaneMetricSession( } } - private val isBluetoothOnAfterApmToggle = !isOn + private val isBluetoothOnAfterApmToggle = !isOnOverrode private var userToggledBluetoothDuringApm = false private var userToggledBluetoothDuringApmWithinMinute = false diff --git a/service/src/airplane/ModeListenerTest.kt b/service/src/airplane/ModeListenerTest.kt index d0b8a31245..da11d5e2ed 100644 --- a/service/src/airplane/ModeListenerTest.kt +++ b/service/src/airplane/ModeListenerTest.kt @@ -22,8 +22,10 @@ import android.content.Context import android.content.res.Resources import android.os.Looper import android.os.UserHandle +import android.platform.test.flag.junit.SetFlagsRule import android.provider.Settings import androidx.test.core.app.ApplicationProvider +import com.android.bluetooth.flags.Flags import com.android.server.bluetooth.BluetoothAdapterState import com.android.server.bluetooth.Log import com.android.server.bluetooth.airplane.APM_BT_ENABLED_NOTIFICATION @@ -35,6 +37,7 @@ import com.android.server.bluetooth.airplane.BLUETOOTH_APM_STATE import com.android.server.bluetooth.airplane.WIFI_APM_STATE import com.android.server.bluetooth.airplane.initialize import com.android.server.bluetooth.airplane.isOn +import com.android.server.bluetooth.airplane.isOnOverrode import com.android.server.bluetooth.airplane.notifyUserToggledBluetooth import com.android.server.bluetooth.test.disableMode import com.android.server.bluetooth.test.disableSensitive @@ -93,7 +96,9 @@ class ModeListenerTest { private val state = BluetoothAdapterState() private val mContext = ApplicationProvider.getApplicationContext<Context>() private val resolver: ContentResolver = mContext.contentResolver + @JvmField @Rule val testName = TestName() + @JvmField @Rule val setFlagsRule = SetFlagsRule() private val userContext = mContext.createContextAsUser(UserHandle.of(ActivityManager.getCurrentUser()), 0) @@ -160,6 +165,7 @@ class ModeListenerTest { initializeAirplane() assertThat(isOn).isFalse() + assertThat(isOnOverrode).isFalse() assertThat(mode).isEmpty() } @@ -171,6 +177,7 @@ class ModeListenerTest { initializeAirplane() assertThat(isOn).isFalse() + assertThat(isOnOverrode).isFalse() assertThat(mode).isEmpty() } @@ -184,6 +191,7 @@ class ModeListenerTest { enableMode() assertThat(isOn).isFalse() + assertThat(isOnOverrode).isFalse() assertThat(mode).isEmpty() } @@ -192,22 +200,24 @@ class ModeListenerTest { initializeAirplane() assertThat(isOn).isFalse() + assertThat(isOnOverrode).isFalse() assertThat(mode).isEmpty() } @Test - fun initialize_whenSensitive_isOn() { + fun initialize_whenSensitive_isOnOverrode() { enableSensitive() enableMode() initializeAirplane() assertThat(isOn).isTrue() + assertThat(isOnOverrode).isTrue() assertThat(mode).isEmpty() } @Test - fun initialize_whenApmToggled_isOn() { + fun initialize_whenApmToggled_isOnOverrode() { enableSensitive() enableMode() Settings.Secure.putInt(userContext.contentResolver, APM_USER_TOGGLED_BLUETOOTH, 1) @@ -215,12 +225,13 @@ class ModeListenerTest { initializeAirplane() - assertThat(isOn).isFalse() + assertThat(isOn).isTrue() + assertThat(isOnOverrode).isFalse() assertThat(mode).isEmpty() } @Test - fun toggleSensitive_whenEnabled_isOnOffOn() { + fun toggleSensitive_whenEnabled_isOnOverrode() { enableSensitive() enableMode() @@ -229,7 +240,7 @@ class ModeListenerTest { disableSensitive() enableSensitive() - assertThat(isOn).isTrue() + assertThat(isOnOverrode).isTrue() assertThat(mode).containsExactly(false, true) } @@ -240,7 +251,7 @@ class ModeListenerTest { enableMode() disableMode() - assertThat(isOn).isFalse() + assertThat(isOnOverrode).isFalse() assertThat(mode).containsExactly(true, false) } @@ -250,11 +261,38 @@ class ModeListenerTest { disableMode() - assertThat(isOn).isFalse() + assertThat(isOnOverrode).isFalse() assertThat(mode).isEmpty() } @Test + fun disable_whenBluetoothOn_discardUpdate() { + setFlagsRule.enableFlags(Flags.FLAG_AIRPLANE_MODE_X_BLE_ON) + initializeAirplane() + enableMode() + + state.set(BluetoothAdapter.STATE_ON) + disableMode() + + assertThat(isOnOverrode).isFalse() + assertThat(mode).containsExactly(true) + } + + // Test to remove once AIRPLANE_MODE_X_BLE_ON has shipped + @Test + fun disable_whenBluetoothOn_notDiscardUpdate() { + setFlagsRule.disableFlags(Flags.FLAG_AIRPLANE_MODE_X_BLE_ON) + initializeAirplane() + enableMode() + + state.set(BluetoothAdapter.STATE_ON) + disableMode() + + assertThat(isOnOverrode).isFalse() + assertThat(mode).containsExactly(true, false) + } + + @Test fun enabled_whenEnabled_discardOnChange() { enableSensitive() enableMode() @@ -263,7 +301,7 @@ class ModeListenerTest { enableMode() - assertThat(isOn).isTrue() + assertThat(isOnOverrode).isTrue() assertThat(mode).isEmpty() } @@ -274,7 +312,7 @@ class ModeListenerTest { disableSensitive() enableMode() - assertThat(isOn).isFalse() + assertThat(isOnOverrode).isFalse() // As opposed to the bare RadioModeListener, similar consecutive event are discarded assertThat(mode).isEmpty() } @@ -287,7 +325,7 @@ class ModeListenerTest { enableMode() - assertThat(isOn).isTrue() + assertThat(isOnOverrode).isTrue() assertThat(mode).containsExactly(true) assertThat(ShadowToast.shownToastCount()).isEqualTo(0) } @@ -301,7 +339,7 @@ class ModeListenerTest { enableMode() - assertThat(isOn).isFalse() + assertThat(isOnOverrode).isFalse() assertThat(mode).isEmpty() assertThat(ShadowToast.shownToastCount()).isEqualTo(1) @@ -323,6 +361,7 @@ class ModeListenerTest { enableMode() + assertThat(isOnOverrode).isTrue() assertThat(isOn).isTrue() assertThat(mode).containsExactly(true) } @@ -337,7 +376,8 @@ class ModeListenerTest { enableMode() - assertThat(isOn).isFalse() + assertThat(isOnOverrode).isFalse() + assertThat(isOn).isTrue() assertThat(mode).isEmpty() } @@ -350,6 +390,7 @@ class ModeListenerTest { enableMode() + assertThat(isOnOverrode).isTrue() assertThat(isOn).isTrue() assertThat(mode).containsExactly(true) } @@ -364,7 +405,8 @@ class ModeListenerTest { enableMode() - assertThat(isOn).isFalse() + assertThat(isOnOverrode).isFalse() + assertThat(isOn).isTrue() assertThat(mode).isEmpty() assertThat(notification).containsExactly(APM_BT_NOTIFICATION) } @@ -382,7 +424,7 @@ class ModeListenerTest { enableMode() - assertThat(isOn).isFalse() + assertThat(isOnOverrode).isFalse() assertThat(mode).isEmpty() assertThat(notification).containsExactly(APM_WIFI_BT_NOTIFICATION) } @@ -399,7 +441,7 @@ class ModeListenerTest { enableMode() - assertThat(isOn).isFalse() + assertThat(isOnOverrode).isFalse() assertThat(mode).isEmpty() assertThat(notification).containsExactly(APM_BT_NOTIFICATION) } @@ -416,7 +458,7 @@ class ModeListenerTest { disableMode() } - assertThat(isOn).isFalse() + assertThat(isOnOverrode).isFalse() assertThat(mode).isEmpty() assertThat(notification).isEmpty() @@ -430,7 +472,7 @@ class ModeListenerTest { notifyUserToggledBluetooth(resolver, userContext, false) - assertThat(isOn).isFalse() + assertThat(isOnOverrode).isFalse() assertThat(mode).isEmpty() assertThat(notification).isEmpty() assertThat(ShadowToast.shownToastCount()).isEqualTo(0) @@ -444,7 +486,7 @@ class ModeListenerTest { enableMode() notifyUserToggledBluetooth(resolver, userContext, true) - assertThat(isOn).isTrue() + assertThat(isOnOverrode).isTrue() assertThat(mode).containsExactly(true) assertThat(notification).isEmpty() assertThat(ShadowToast.shownToastCount()).isEqualTo(0) @@ -463,7 +505,7 @@ class ModeListenerTest { enableMode() notifyUserToggledBluetooth(resolver, userContext, false) - assertThat(isOn).isTrue() + assertThat(isOnOverrode).isTrue() assertThat(mode).containsExactly(true) assertThat(notification).isEmpty() assertThat(ShadowToast.shownToastCount()).isEqualTo(0) @@ -482,7 +524,7 @@ class ModeListenerTest { enableMode() notifyUserToggledBluetooth(resolver, userContext, true) - assertThat(isOn).isTrue() + assertThat(isOnOverrode).isTrue() assertThat(mode).containsExactly(true) assertThat(notification).containsExactly(APM_BT_ENABLED_NOTIFICATION) assertThat(ShadowToast.shownToastCount()).isEqualTo(0) @@ -502,7 +544,7 @@ class ModeListenerTest { notifyUserToggledBluetooth(resolver, userContext, true) notifyUserToggledBluetooth(resolver, userContext, false) - assertThat(isOn).isTrue() + assertThat(isOnOverrode).isTrue() assertThat(mode).containsExactly(true) assertThat(notification).containsExactly(APM_BT_ENABLED_NOTIFICATION) assertThat(ShadowToast.shownToastCount()).isEqualTo(0) @@ -533,7 +575,7 @@ class ModeListenerTest { timesource += 2.minutes notifyUserToggledBluetooth(resolver, userContext, true) - assertThat(isOn).isTrue() + assertThat(isOnOverrode).isTrue() assertThat(mode).containsExactly(true) assertThat(notification).isEmpty() assertThat(ShadowToast.shownToastCount()).isEqualTo(0) diff --git a/service/src/com/android/server/bluetooth/BluetoothManagerService.java b/service/src/com/android/server/bluetooth/BluetoothManagerService.java index 7b978b143a..1c2f0c60b8 100644 --- a/service/src/com/android/server/bluetooth/BluetoothManagerService.java +++ b/service/src/com/android/server/bluetooth/BluetoothManagerService.java @@ -390,7 +390,8 @@ class BluetoothManagerService { TAG, ("delayModeChangedIfNeeded(" + modechanged + "):") + (" state=" + BluetoothAdapter.nameForState(state)) - + (" isAirplaneModeOn()=" + isAirplaneModeOn()) + + (" Airplane.isOnOverrode=" + AirplaneModeListener.isOnOverrode()) + + (" Airplane.isOn=" + AirplaneModeListener.isOn()) + (" isSatelliteModeOn()=" + isSatelliteModeOn()) + (" delayed=" + delayMs + "ms")); @@ -537,7 +538,7 @@ class BluetoothManagerService { return false; } - if (isAirplaneModeOn() && isBluetoothPersistedStateOnAirplane()) { + if (AirplaneModeListener.isOnOverrode() && isBluetoothPersistedStateOnAirplane()) { Log.d(TAG, "shouldBluetoothBeOn: BT should be off as airplaneMode is on."); return false; } @@ -705,11 +706,6 @@ class BluetoothManagerService { return mBinder; } - /** Returns true if airplane mode is currently on */ - private boolean isAirplaneModeOn() { - return AirplaneModeListener.isOn(); - } - /** Returns true if satellite mode is turned on. */ private boolean isSatelliteModeOn() { return SatelliteModeListener.isOn(); @@ -879,8 +875,14 @@ class BluetoothManagerService { } boolean isBleScanAlwaysAvailable() { - if (isAirplaneModeOn() && !mEnable) { - return false; + if (Flags.airplaneModeXBleOn()) { + if (AirplaneModeListener.isOn() && !mEnable) { + return false; + } + } else { + if (AirplaneModeListener.isOnOverrode() && !mEnable) { + return false; + } } try { return Settings.Global.getInt( @@ -983,9 +985,16 @@ class BluetoothManagerService { + (" isBinding=" + isBinding()) + (" mState=" + mState)); - if (isAirplaneModeOn()) { - Log.d(TAG, "enableBle: not enabling - Airplane mode is on"); - return false; + if (Flags.airplaneModeXBleOn()) { + if (AirplaneModeListener.isOn() && !mEnable) { + Log.d(TAG, "enableBle: not enabling - Airplane mode is ON on system"); + return false; + } + } else { + if (AirplaneModeListener.isOnOverrode()) { + Log.d(TAG, "enableBle: not enabling - Airplane mode is ON"); + return false; + } } if (isSatelliteModeOn()) { @@ -1099,7 +1108,9 @@ class BluetoothManagerService { Log.w(TAG, "sendBrEdrDownCallback: mAdapter is null"); return; } - if (isBleAppPresent()) { + boolean airplaneDoesNotAllowBleOn = + Flags.airplaneModeXBleOn() && AirplaneModeListener.isOn(); + if (!airplaneDoesNotAllowBleOn && isBleAppPresent()) { // Need to stay at BLE ON. Disconnect all Gatt connections Log.i(TAG, "sendBrEdrDownCallback: Staying in BLE_ON"); mAdapter.unregAllGattClient(mContext.getAttributionSource()); @@ -2195,7 +2206,7 @@ class BluetoothManagerService { mHandler.sendEmptyMessageDelayed(MESSAGE_RESTART_BLUETOOTH_SERVICE, ERROR_RESTART_TIME_MS); if (repeatAirplaneRunnable) { - onAirplaneModeChanged(isAirplaneModeOn()); + onAirplaneModeChanged(AirplaneModeListener.isOnOverrode()); } } @@ -2396,8 +2407,7 @@ class BluetoothManagerService { for (Method m : Flags.class.getDeclaredMethods()) { String flagStatus = ((Boolean) m.invoke(null)) ? "[■]" : "[ ]"; String name = m.getName(); - String snakeCaseName = - name.replaceAll("([a-z])([A-Z]+)", "$1_$2").toLowerCase(Locale.US); + String snakeCaseName = name.replaceAll("([A-Z])", "_$1").toLowerCase(Locale.US); writer.println(String.format(fmt, flagStatus, name, snakeCaseName)); } writer.println(""); diff --git a/service/src/com/android/server/bluetooth/BluetoothShellCommand.java b/service/src/com/android/server/bluetooth/BluetoothShellCommand.java index d4970b7b63..a6eb010cfa 100644 --- a/service/src/com/android/server/bluetooth/BluetoothShellCommand.java +++ b/service/src/com/android/server/bluetooth/BluetoothShellCommand.java @@ -36,9 +36,7 @@ class BluetoothShellCommand extends BasicShellCommandHandler { @VisibleForTesting final BluetoothCommand[] mBluetoothCommands = { - new Enable(), - new Disable(), - new WaitForAdapterState(), + new Enable(), new EnableBle(), new Disable(), new DisableBle(), new WaitForAdapterState(), }; @VisibleForTesting @@ -66,6 +64,54 @@ class BluetoothShellCommand extends BasicShellCommandHandler { } @VisibleForTesting + class EnableBle extends BluetoothCommand { + EnableBle() { + super(true, "enableBle"); + } + + @Override + public int exec(String cmd) throws RemoteException { + return mManagerService + .getBinder() + .enableBle( + AttributionSource.myAttributionSource(), + mManagerService.getBinder()) + ? 0 + : -1; + } + + @Override + public void onHelp(PrintWriter pw) { + pw.println(" " + getName()); + pw.println(" Call enableBle to activate ble only mode on this device."); + } + } + + @VisibleForTesting + class DisableBle extends BluetoothCommand { + DisableBle() { + super(true, "disableBle"); + } + + @Override + public int exec(String cmd) throws RemoteException { + return mManagerService + .getBinder() + .disableBle( + AttributionSource.myAttributionSource(), + mManagerService.getBinder()) + ? 0 + : -1; + } + + @Override + public void onHelp(PrintWriter pw) { + pw.println(" " + getName()); + pw.println(" revoke the call to enableBle. No-op if enableBle wasn't call before"); + } + } + + @VisibleForTesting class Enable extends BluetoothCommand { Enable() { super(false, "enable"); |