Skip to content

Commit

Permalink
Add wait-for-state command
Browse files Browse the repository at this point in the history 
Disable and enable are async shell command.
This is creating flakyness in test that expect the bluetooth to be
already enabled / disabled.

Test: atest BluetoothInstrumentationTests
Test: atest BluetoothShellCommandTest
Test: atest 'BluetoothTest#AdapterEnableDisable' // net_test_bluetooth
Fix: 261772749
Tag: #refactor
Change-Id: I3ca29dde23a5182ede28eceaf1ac97e4668ab5d5
(cherry picked from commit ca23daa448294ebea1b5cfd421732e85440fe4b9)
Merged-In: I3ca29dde23a5182ede28eceaf1ac97e4668ab5d5
  • Loading branch information
@wescande
wescande authored and Cherrypicker Worker committed Dec 20, 2022
1 parent c33fdf1 commit 4ffb8eb
Show file tree
Hide file tree
Showing 8 changed files with 202 additions and 75 deletions.
3 changes: 2 additions & 1 deletion AndroidTestTemplate.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@
</target_preparer>
<target_preparer class="com.android.tradefed.targetprep.RunCommandTargetPreparer">
<option name="run-command" value="settings put global ble_scan_always_enabled 0" />
<option name="run-command" value="svc bluetooth disable" />
<option name="run-command" value="cmd bluetooth_manager disable" />
<option name="run-command" value="cmd bluetooth_manager wait-for-state:STATE_OFF" />
</target_preparer>
<target_preparer class="com.android.tradefed.targetprep.FolderSaver">
<option name="device-path" value="/data/vendor/ssrdump" />
Expand Down
2 changes: 2 additions & 0 deletions android/app/tests/unit/AndroidTest.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,12 @@
<option name="throw-if-cmd-fail" value="true" />
<option name="run-command" value="settings put global ble_scan_always_enabled 0" />
<option name="run-command" value="cmd bluetooth_manager disable" />
<option name="run-command" value="cmd bluetooth_manager wait-for-state:STATE_OFF" />
<option name="run-command" value="setprop bluetooth.profile.hfp.hf.enabled true" />
<option name="run-command" value="setprop bluetooth.profile.pbap.client.enabled true" />
<option name="run-command" value="setprop bluetooth.profile.map.client.enabled true" />
<option name="teardown-command" value="cmd bluetooth_manager enable" />
<option name="teardown-command" value="cmd bluetooth_manager wait-for-state:STATE_ON" />
<option name="teardown-command" value="settings put global ble_scan_always_enabled 1" />
<option name="teardown-command" value="setprop bluetooth.profile.hfp.hf.enabled false" />
<option name="teardown-command"
Expand Down
2 changes: 2 additions & 0 deletions android/app/tests/unit/GoogleAndroidTest.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,12 @@
<option name="throw-if-cmd-fail" value="true" />
<option name="run-command" value="settings put global ble_scan_always_enabled 0" />
<option name="run-command" value="cmd bluetooth_manager disable" />
<option name="run-command" value="cmd bluetooth_manager wait-for-state:STATE_OFF" />
<option name="run-command" value="setprop bluetooth.profile.hfp.hf.enabled true" />
<option name="run-command" value="setprop bluetooth.profile.pbap.client.enabled true" />
<option name="run-command" value="setprop bluetooth.profile.map.client.enabled true" />
<option name="teardown-command" value="cmd bluetooth_manager enable" />
<option name="teardown-command" value="cmd bluetooth_manager wait-for-state:STATE_ON" />
<option name="teardown-command" value="settings put global ble_scan_always_enabled 1" />
<option name="teardown-command"
value="setprop bluetooth.profile.pbap.client.enabled false" />
Expand Down
23 changes: 16 additions & 7 deletions service/java/com/android/server/bluetooth/BluetoothManagerService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2861,15 +2861,25 @@ private void bluetoothStateChangeHandler(int prevState, int newState) {
}
}

boolean waitForManagerState(int state) {
return waitForState(Set.of(state), false);
}

private boolean waitForState(Set<Integer> states) {
int i = 0;
while (i < 10) {
return waitForState(states, true);
}
private boolean waitForState(Set<Integer> states, boolean failIfUnbind) {
for (int i = 0; i < 10; i++) {
mBluetoothLock.readLock().lock();
try {
mBluetoothLock.readLock().lock();
if (mBluetooth == null) {
break;
if (mBluetooth == null && failIfUnbind) {
Log.e(TAG, "waitForState " + states + " Bluetooth is not unbind");
return false;
}
if (mBluetooth == null && states.contains(BluetoothAdapter.STATE_OFF)) {
return true; // We are so OFF that the bluetooth is not bind
}
if (states.contains(synchronousGetState())) {
if (mBluetooth != null && states.contains(synchronousGetState())) {
return true;
}
} catch (RemoteException | TimeoutException e) {
Expand All @@ -2879,7 +2889,6 @@ private boolean waitForState(Set<Integer> states) {
mBluetoothLock.readLock().unlock();
}
SystemClock.sleep(300);
i++;
}
Log.e(TAG, "waitForState " + states + " time out");
return false;
Expand Down
163 changes: 119 additions & 44 deletions service/java/com/android/server/bluetooth/BluetoothShellCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@

package com.android.server.bluetooth;

import static java.util.Objects.requireNonNull;

import android.bluetooth.BluetoothAdapter;
import android.content.AttributionSource;
import android.content.Context;
import android.os.Binder;
Expand All @@ -29,7 +32,7 @@
import java.io.PrintWriter;

class BluetoothShellCommand extends BasicShellCommandHandler {
private static final String TAG = "BluetoothShellCommand";
private static final String TAG = BluetoothShellCommand.class.getSimpleName();

private final BluetoothManagerService mManagerService;
private final Context mContext;
Expand All @@ -38,49 +41,111 @@ class BluetoothShellCommand extends BasicShellCommandHandler {
final BluetoothCommand[] mBluetoothCommands = {
new Enable(),
new Disable(),
new WaitForAdapterState(),
};

@VisibleForTesting
abstract class BluetoothCommand {
abstract String getName();
// require root permission by default, can be override in command implementation
final boolean mIsPrivileged;
final String mName;

BluetoothCommand(boolean isPrivileged, String name) {
mIsPrivileged = isPrivileged;
mName = requireNonNull(name, "Command name cannot be null");
}

String getName() {
return mName;
}
boolean isMatch(String cmd) {
return mName.equals(cmd);
}
boolean isPrivileged() {
return true;
return mIsPrivileged;
}
abstract int exec(PrintWriter pw) throws RemoteException;

abstract int exec(String cmd) throws RemoteException;
abstract void onHelp(PrintWriter pw);
}

@VisibleForTesting
class Enable extends BluetoothCommand {
@Override
String getName() {
return "enable";
Enable() {
super(false, "enable");
}
@Override
boolean isPrivileged() {
return false;
public int exec(String cmd) throws RemoteException {
return mManagerService.enable(AttributionSource.myAttributionSource()) ? 0 : -1;
}
@Override
public int exec(PrintWriter pw) throws RemoteException {
pw.println("Enabling Bluetooth");
return mManagerService.enable(AttributionSource.myAttributionSource()) ? 0 : -1;
public void onHelp(PrintWriter pw) {
pw.println(" " + getName());
pw.println(" Enable Bluetooth on this device.");
}
}

@VisibleForTesting
class Disable extends BluetoothCommand {
Disable() {
super(false, "disable");
}
@Override
String getName() {
return "disable";
public int exec(String cmd) throws RemoteException {
return mManagerService.disable(AttributionSource.myAttributionSource(), true) ? 0 : -1;
}
@Override
boolean isPrivileged() {
return false;
public void onHelp(PrintWriter pw) {
pw.println(" " + getName());
pw.println(" Disable Bluetooth on this device.");
}
}

@VisibleForTesting
class WaitForAdapterState extends BluetoothCommand {
WaitForAdapterState() {
super(false, "wait-for-state");
}
private int getWaitingState(String in) {
if (!in.startsWith(getName() + ":")) return -1;
String[] split = in.split(":", 2);
if (split.length != 2 || !getName().equals(split[0])) {
String msg = getName() + ": Invalid state format: " + in;
Log.e(TAG, msg);
PrintWriter pw = getErrPrintWriter();
pw.println(TAG + ": " + msg);
printHelp(pw);
throw new IllegalArgumentException();
}
switch (split[1]) {
case "STATE_OFF":
return BluetoothAdapter.STATE_OFF;
case "STATE_ON":
return BluetoothAdapter.STATE_ON;
default:
String msg = getName() + ": Invalid state value: " + split[1] + ". From: " + in;
Log.e(TAG, msg);
PrintWriter pw = getErrPrintWriter();
pw.println(TAG + ": " + msg);
printHelp(pw);
throw new IllegalArgumentException();
}
}
@Override
public int exec(PrintWriter pw) throws RemoteException {
pw.println("Disabling Bluetooth");
return mManagerService.disable(AttributionSource.myAttributionSource(), true) ? 0 : -1;
boolean isMatch(String cmd) {
return getWaitingState(cmd) != -1;
}
@Override
public int exec(String cmd) throws RemoteException {
int ret = mManagerService.waitForManagerState(getWaitingState(cmd)) ? 0 : -1;
Log.d(TAG, cmd + ": Return value is " + ret); // logging as this method can take time
return ret;
}
@Override
public void onHelp(PrintWriter pw) {
pw.println(" " + getName() + ":<STATE>");
pw.println(" Wait until the adapter state is <STATE>."
+ " <STATE> can be one of STATE_OFF | STATE_ON");
pw.println(" Note: This command can timeout and failed");
}
}

Expand All @@ -91,40 +156,50 @@ public int exec(PrintWriter pw) throws RemoteException {

@Override
public int onCommand(String cmd) {
if (cmd == null) {
return handleDefaultCommands(null);
}
if (cmd == null) return handleDefaultCommands(null);

for (BluetoothCommand bt_cmd : mBluetoothCommands) {
if (cmd.equals(bt_cmd.getName())) {
if (bt_cmd.isPrivileged()) {
final int uid = Binder.getCallingUid();
if (uid != Process.ROOT_UID) {
throw new SecurityException("Uid " + uid + " does not have access to "
+ cmd + " bluetooth command (or such command doesn't exist)");
}
if (!bt_cmd.isMatch(cmd)) continue;
if (bt_cmd.isPrivileged()) {
final int uid = Binder.getCallingUid();
if (uid != Process.ROOT_UID) {
throw new SecurityException("Uid " + uid + " does not have access to "
+ cmd + " bluetooth command");
}
try {
return bt_cmd.exec(getOutPrintWriter());
} catch (RemoteException e) {
Log.w(TAG, cmd + ": error\nException: " + e.getMessage());
getErrPrintWriter().println(cmd + ": error\nException: " + e.getMessage());
e.rethrowFromSystemServer();
}
try {
getOutPrintWriter().println(TAG + ": Exec" + cmd);
Log.d(TAG, "Exec " + cmd);
int ret = bt_cmd.exec(cmd);
if (ret == 0) {
String msg = cmd + ": Success";
Log.d(TAG, msg);
getOutPrintWriter().println(msg);
} else {
String msg = cmd + ": Failed with status=" + ret;
Log.e(TAG, msg);
getErrPrintWriter().println(TAG + ": " + msg);
}
return ret;
} catch (RemoteException e) {
Log.w(TAG, cmd + ": error\nException: " + e.getMessage());
getErrPrintWriter().println(cmd + ": error\nException: " + e.getMessage());
e.rethrowFromSystemServer();
}
}
return handleDefaultCommands(cmd);
}

@Override
public void onHelp() {
PrintWriter pw = getOutPrintWriter();
pw.println("Bluetooth Commands:");
private void printHelp(PrintWriter pw) {
pw.println("Bluetooth Manager Commands:");
pw.println(" help or -h");
pw.println(" Print this help text.");
pw.println(" enable");
pw.println(" Enable Bluetooth on this device.");
pw.println(" disable");
pw.println(" Disable Bluetooth on this device.");
for (BluetoothCommand bt_cmd : mBluetoothCommands) {
bt_cmd.onHelp(pw);
}
}
@Override
public void onHelp() {
printHelp(getOutPrintWriter());
}
}
2 changes: 1 addition & 1 deletion service/tests/Android.bp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ android_test {
"truth-prebuilt",

// Statically link service-bluetooth-pre-jarjar since we want to test the working copy of
// service-uwb, not the on-device copy.
// service-bluetooth, not the on-device copy.
// Use pre-jarjar version so that we can reference symbols before they are renamed.
// Then, the jarjar_rules here will perform the rename for the entire APK
// i.e. service-bluetooth + test code
Expand Down
Loading

0 comments on commit 4ffb8eb

Please sign in to comment.