Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit fe25b805 authored by Rahul Arya's avatar Rahul Arya
Browse files

Only restart bluetooth if INIT_FLAGS have changed 

Otherwise we keep restarting it randomly, especially for dogfooders, and
not everything likes it when Bluetooth restarts (especially if they're
doing LE stuff in the background).

Test: atest BluetoothDeviceConfigChangeTrackerTest and manually changing
flags with adb
Tag: #stability
Bug: 242348618

Change-Id: Ia955e5ed92c51ec65e0ac0dc42e0998b9c9845ae
parent d3b3156c

service/java/com/android/server/bluetooth/BluetoothDeviceConfigChangeTracker.java

0 → 100644
+85 −0
Original line number Original line Diff line number Diff line 
/*

 * Copyright 2022 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.server.bluetooth;



import android.provider.DeviceConfig;

import android.provider.DeviceConfig.Properties;

import android.util.Log;



import java.util.ArrayList;

import java.util.HashMap;



/**

 * The BluetoothDeviceConfigChangeTracker receives changes to the DeviceConfig for

 * NAMESPACE_BLUETOOTH, and determines whether we should queue a restart, if any Bluetooth-related

 * INIT_ flags have been changed.

 *

 * <p>The initialProperties should be fetched from the BLUETOOTH namespace in DeviceConfig

 */

public final class BluetoothDeviceConfigChangeTracker {

    private static final String TAG = "BluetoothDeviceConfigChangeTracker";



    private final HashMap<String, String> mCurrFlags;



    public BluetoothDeviceConfigChangeTracker(Properties initialProperties) {

        mCurrFlags = getFlags(initialProperties);

    }



    /**

     * Updates the instance state tracking the latest init flag values, and determines whether an

     * init flag has changed (requiring a restart at some point)

     */

    public boolean shouldRestartWhenPropertiesUpdated(Properties newProperties) {

        if (!newProperties.getNamespace().equals(DeviceConfig.NAMESPACE_BLUETOOTH)) {

            return false;

        }

        ArrayList<String> flags = new ArrayList<>();

        for (String name : newProperties.getKeyset()) {

            flags.add(name + "='" + newProperties.getString(name, "") + "'");

        }

        Log.d(TAG, "shouldRestartWhenPropertiesUpdated: " + String.join(",", flags));

        boolean shouldRestart = false;

        for (String name : newProperties.getKeyset()) {

            if (!isInitFlag(name)) {

                continue;

            }

            var oldValue = mCurrFlags.get(name);

            var newValue = newProperties.getString(name, "");

            if (newValue.equals(oldValue)) {

                continue;

            }

            Log.d(TAG, "Property " + name + " changed from " + oldValue + " -> " + newValue);

            mCurrFlags.put(name, newValue);

            shouldRestart = true;

        }

        return shouldRestart;

    }



    private HashMap<String, String> getFlags(Properties initialProperties) {

        var out = new HashMap();

        for (var name : initialProperties.getKeyset()) {

            if (isInitFlag(name)) {

                out.put(name, initialProperties.getString(name, ""));

            }

        }

        return out;

    }



    private Boolean isInitFlag(String flagName) {

        return flagName.startsWith("INIT_");

    }

}

service/java/com/android/server/bluetooth/BluetoothDeviceConfigListener.java

+16 −28
Original line number Original line Diff line number Diff line
@@ -24,8 +24,6 @@ import android.provider.DeviceConfig; 
import android.provider.Settings;

import android.provider.Settings;

import android.util.Log;

import android.util.Log;





import java.util.ArrayList;



/**

/**

 * The BluetoothDeviceConfigListener handles system device config change callback and checks

 * The BluetoothDeviceConfigListener handles system device config change callback and checks

 * whether we need to inform BluetoothManagerService on this change.

 * whether we need to inform BluetoothManagerService on this change.
@@ -38,11 +36,13 @@ import java.util.ArrayList; 
public class BluetoothDeviceConfigListener {

public class BluetoothDeviceConfigListener {

    private static final String TAG = "BluetoothDeviceConfigListener";

    private static final String TAG = "BluetoothDeviceConfigListener";





    private static final int DEFAULT_APM_ENHANCEMENT = 0;

    private static final int DEFAULT_BT_APM_STATE = 0;



    private final BluetoothManagerService mService;

    private final BluetoothManagerService mService;

    private final boolean mLogDebug;

    private final boolean mLogDebug;

    private final Context mContext;

    private final Context mContext;

    private static final int DEFAULT_APM_ENHANCEMENT = 0;

    private final BluetoothDeviceConfigChangeTracker mConfigChangeTracker;

    private static final int DEFAULT_BT_APM_STATE = 0;





    private boolean mPrevApmEnhancement;

    private boolean mPrevApmEnhancement;

    private boolean mPrevBtApmState;

    private boolean mPrevBtApmState;
@@ -56,6 +56,9 @@ public class BluetoothDeviceConfigListener { 
                APM_ENHANCEMENT, DEFAULT_APM_ENHANCEMENT) == 1;

                APM_ENHANCEMENT, DEFAULT_APM_ENHANCEMENT) == 1;

        mPrevBtApmState = Settings.Global.getInt(mContext.getContentResolver(),

        mPrevBtApmState = Settings.Global.getInt(mContext.getContentResolver(),

                BT_DEFAULT_APM_STATE, DEFAULT_BT_APM_STATE) == 1;

                BT_DEFAULT_APM_STATE, DEFAULT_BT_APM_STATE) == 1;

        mConfigChangeTracker =

                new BluetoothDeviceConfigChangeTracker(

                        DeviceConfig.getProperties(DeviceConfig.NAMESPACE_BLUETOOTH));

        DeviceConfig.addOnPropertiesChangedListener(

        DeviceConfig.addOnPropertiesChangedListener(

                DeviceConfig.NAMESPACE_BLUETOOTH,

                DeviceConfig.NAMESPACE_BLUETOOTH,

                (Runnable r) -> r.run(),

                (Runnable r) -> r.run(),
@@ -65,19 +68,8 @@ public class BluetoothDeviceConfigListener { 
    private final DeviceConfig.OnPropertiesChangedListener mDeviceConfigChangedListener =

    private final DeviceConfig.OnPropertiesChangedListener mDeviceConfigChangedListener =

            new DeviceConfig.OnPropertiesChangedListener() {

            new DeviceConfig.OnPropertiesChangedListener() {

                @Override

                @Override

                public void onPropertiesChanged(DeviceConfig.Properties properties) {

                public void onPropertiesChanged(DeviceConfig.Properties newProperties) {

                    if (!properties.getNamespace().equals(DeviceConfig.NAMESPACE_BLUETOOTH)) {

                    boolean apmEnhancement = newProperties.getBoolean(

                        return;

                    }

                    if (mLogDebug) {

                        ArrayList<String> flags = new ArrayList<>();

                        for (String name : properties.getKeyset()) {

                            flags.add(name + "='" + properties.getString(name, "") + "'");

                        }

                        Log.d(TAG, "onPropertiesChanged: " + String.join(",", flags));

                    }



                    boolean apmEnhancement = properties.getBoolean(

                            APM_ENHANCEMENT, mPrevApmEnhancement);

                            APM_ENHANCEMENT, mPrevApmEnhancement);

                    if (apmEnhancement != mPrevApmEnhancement) {

                    if (apmEnhancement != mPrevApmEnhancement) {

                        mPrevApmEnhancement = apmEnhancement;

                        mPrevApmEnhancement = apmEnhancement;
@@ -85,24 +77,20 @@ public class BluetoothDeviceConfigListener { 
                                APM_ENHANCEMENT, apmEnhancement ? 1 : 0);

                                APM_ENHANCEMENT, apmEnhancement ? 1 : 0);

                    }

                    }





                    boolean btApmState = properties.getBoolean(

                    boolean btApmState = newProperties.getBoolean(

                            BT_DEFAULT_APM_STATE, mPrevBtApmState);

                            BT_DEFAULT_APM_STATE, mPrevBtApmState);

                    if (btApmState != mPrevBtApmState) {

                    if (btApmState != mPrevBtApmState) {

                        mPrevBtApmState = btApmState;

                        mPrevBtApmState = btApmState;

                        Settings.Global.putInt(mContext.getContentResolver(),

                        Settings.Global.putInt(mContext.getContentResolver(),

                                BT_DEFAULT_APM_STATE, btApmState ? 1 : 0);

                                BT_DEFAULT_APM_STATE, btApmState ? 1 : 0);

                    }

                    }

                    boolean foundInit = false;



                    for (String name : properties.getKeyset()) {

                    if (mConfigChangeTracker.shouldRestartWhenPropertiesUpdated(newProperties)) {

                        if (name.startsWith("INIT_")) {

                        Log.d(TAG, "Properties changed, enqueuing restart");

                            foundInit = true;

                            break;

                        }

                    }

                    if (!foundInit) {

                        return;

                    }

                        mService.onInitFlagsChanged();

                        mService.onInitFlagsChanged();

                    } else {

                        Log.d(TAG, "All properties unchanged, skipping restart");

                    }

                }

                }

            };

            };

}
 
}

service/tests/Android.bp

+2 −0
Original line number Original line Diff line number Diff line
@@ -32,6 +32,8 @@ android_test { 
        "androidx.test.rules",

        "androidx.test.rules",

        "mockito-target-extended-minus-junit4",

        "mockito-target-extended-minus-junit4",

        "platform-test-annotations",

        "platform-test-annotations",

        "frameworks-base-testutils",

        "truth-prebuilt",





        // Statically link service-bluetooth-pre-jarjar since we want to test the working copy of

        // Statically link service-bluetooth-pre-jarjar since we want to test the working copy of

        // service-uwb, not the on-device copy.

        // service-uwb, not the on-device copy.

service/tests/src/com/android/server/BluetoothDeviceConfigChangeTrackerTest.java

0 → 100644
+167 −0
Original line number Original line Diff line number Diff line 
/*

 * Copyright 2022 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.server.bluetooth;



import static com.google.common.truth.Truth.assertThat;



import android.provider.DeviceConfig;

import android.provider.DeviceConfig.Properties;



import androidx.test.filters.SmallTest;

import androidx.test.runner.AndroidJUnit4;



import org.junit.Test;

import org.junit.runner.RunWith;



@SmallTest

@RunWith(AndroidJUnit4.class)

public class BluetoothDeviceConfigChangeTrackerTest {

    @Test

    public void testNoProperties() {

        BluetoothDeviceConfigChangeTracker changeTracker =

                new BluetoothDeviceConfigChangeTracker(

                        new Properties.Builder(DeviceConfig.NAMESPACE_BLUETOOTH).build());



        boolean shouldRestart =

                changeTracker.shouldRestartWhenPropertiesUpdated(

                        new Properties.Builder(DeviceConfig.NAMESPACE_BLUETOOTH).build());



        assertThat(shouldRestart).isFalse();

    }



    @Test

    public void testNewFlag() {

        BluetoothDeviceConfigChangeTracker changeTracker =

                new BluetoothDeviceConfigChangeTracker(

                        new Properties.Builder(DeviceConfig.NAMESPACE_BLUETOOTH)

                                .setString("INIT_a", "true")

                                .build());



        boolean shouldRestart =

                changeTracker.shouldRestartWhenPropertiesUpdated(

                        new Properties.Builder(DeviceConfig.NAMESPACE_BLUETOOTH)

                                .setString("INIT_a", "true")

                                .setString("INIT_b", "true")

                                .build());



        assertThat(shouldRestart).isTrue();

    }



    @Test

    public void testChangedFlag() {

        BluetoothDeviceConfigChangeTracker changeTracker =

                new BluetoothDeviceConfigChangeTracker(

                        new Properties.Builder(DeviceConfig.NAMESPACE_BLUETOOTH)

                                .setString("INIT_a", "true")

                                .build());



        boolean shouldRestart =

                changeTracker.shouldRestartWhenPropertiesUpdated(

                        new Properties.Builder("another_namespace")

                                .setString("INIT_a", "false")

                                .build());



        assertThat(shouldRestart).isTrue();

    }



    @Test

    public void testUnchangedInitFlag() {

        BluetoothDeviceConfigChangeTracker changeTracker =

                new BluetoothDeviceConfigChangeTracker(

                        new Properties.Builder(DeviceConfig.NAMESPACE_BLUETOOTH)

                                .setString("INIT_a", "true")

                                .build());



        boolean shouldRestart =

                changeTracker.shouldRestartWhenPropertiesUpdated(

                        new Properties.Builder(DeviceConfig.NAMESPACE_BLUETOOTH)

                                .setString("INIT_a", "true")

                                .build());



        assertThat(shouldRestart).isFalse();

    }



    @Test

    public void testRepeatedChangeInitFlag() {

        BluetoothDeviceConfigChangeTracker changeTracker =

                new BluetoothDeviceConfigChangeTracker(

                        new Properties.Builder(DeviceConfig.NAMESPACE_BLUETOOTH).build());



        changeTracker.shouldRestartWhenPropertiesUpdated(

                new Properties.Builder(DeviceConfig.NAMESPACE_BLUETOOTH)

                        .setString("INIT_a", "true")

                        .build());



        boolean shouldRestart =

                changeTracker.shouldRestartWhenPropertiesUpdated(

                        new Properties.Builder(DeviceConfig.NAMESPACE_BLUETOOTH)

                                .setString("INIT_a", "true")

                                .build());



        assertThat(shouldRestart).isFalse();

    }



    @Test

    public void testWrongNamespace() {

        BluetoothDeviceConfigChangeTracker changeTracker =

                new BluetoothDeviceConfigChangeTracker(

                        new Properties.Builder(DeviceConfig.NAMESPACE_BLUETOOTH).build());



        boolean shouldRestart =

                changeTracker.shouldRestartWhenPropertiesUpdated(

                        new Properties.Builder("another_namespace")

                                .setString("INIT_a", "true")

                                .build());



        assertThat(shouldRestart).isFalse();

    }



    @Test

    public void testSkipProperty() {

        BluetoothDeviceConfigChangeTracker changeTracker =

                new BluetoothDeviceConfigChangeTracker(

                        new Properties.Builder(DeviceConfig.NAMESPACE_BLUETOOTH)

                                .setString("INIT_a", "true")

                                .setString("INIT_b", "false")

                                .build());



        boolean shouldRestart =

                changeTracker.shouldRestartWhenPropertiesUpdated(

                        new Properties.Builder(DeviceConfig.NAMESPACE_BLUETOOTH)

                                .setString("INIT_b", "false")

                                .build());



        assertThat(shouldRestart).isFalse();

    }



    @Test

    public void testNonInitFlag() {

        BluetoothDeviceConfigChangeTracker changeTracker =

                new BluetoothDeviceConfigChangeTracker(

                        new Properties.Builder(DeviceConfig.NAMESPACE_BLUETOOTH)

                                .setString("a", "true")

                                .build());



        boolean shouldRestart =

                changeTracker.shouldRestartWhenPropertiesUpdated(

                        new Properties.Builder(DeviceConfig.NAMESPACE_BLUETOOTH)

                                .setString("a", "false")

                                .build());



        assertThat(shouldRestart).isFalse();

    }

}