Merge "Fix temperature handler to always run regardless of sample from throttling callback" into main

2 files changed, 93 insertions, 16 deletions

diff --git a/services/core/java/com/android/server/power/ThermalManagerService.java b/services/core/java/com/android/server/power/ThermalManagerService.java
index 5ee9b7d09fdd..46c497d04f9e 100644
--- a/services/core/java/com/android/server/power/ThermalManagerService.java
+++ b/services/core/java/com/android/server/power/ThermalManagerService.java
@@ -176,7 +176,9 @@ public class ThermalManagerService extends SystemService {
                     try {
                         final HeadroomCallbackData data;
                         synchronized (mTemperatureWatcher.mSamples) {
-                            Slog.d(TAG, "Updating skin threshold: " + threshold);
+                            if (DEBUG) {
+                                Slog.d(TAG, "Updating skin threshold: " + threshold);
+                            }
                             mTemperatureWatcher.updateTemperatureThresholdLocked(threshold, true);
                             data = mTemperatureWatcher.getHeadroomCallbackDataLocked();
                         }
@@ -454,7 +456,9 @@ public class ThermalManagerService extends SystemService {
                 && temperature.getType() == Temperature.TYPE_SKIN) {
             final HeadroomCallbackData data;
             synchronized (mTemperatureWatcher.mSamples) {
-                Slog.d(TAG, "Updating new temperature: " + temperature);
+                if (DEBUG) {
+                    Slog.d(TAG, "Updating new temperature: " + temperature);
+                }
                 mTemperatureWatcher.updateTemperatureSampleLocked(System.currentTimeMillis(),
                         temperature);
                 mTemperatureWatcher.mCachedHeadrooms.clear();
@@ -1878,6 +1882,7 @@ public class ThermalManagerService extends SystemService {
         @VisibleForTesting
         long mInactivityThresholdMillis = INACTIVITY_THRESHOLD_MILLIS;
 
+        @GuardedBy("mSamples")
         private final Handler mHandler = BackgroundThread.getHandler();
 
         /**
@@ -1900,6 +1905,9 @@ public class ThermalManagerService extends SystemService {
         @GuardedBy("mSamples")
         private long mLastForecastCallTimeMillis = 0;
 
+        private final Runnable mGetAndUpdateTemperatureSamplesRunnable =
+                this::getAndUpdateTemperatureSamples;
+
         void getAndUpdateThresholds() {
             List<TemperatureThreshold> thresholds =
                     mHalWrapper.getTemperatureThresholds(true, Temperature.TYPE_SKIN);
@@ -1930,7 +1938,9 @@ public class ThermalManagerService extends SystemService {
                 return;
             }
             if (override) {
-                Slog.d(TAG, "Headroom cache cleared on threshold update " + threshold);
+                if (DEBUG) {
+                    Slog.d(TAG, "Headroom cache cleared on threshold update " + threshold);
+                }
                 mCachedHeadrooms.clear();
                 Arrays.fill(mHeadroomThresholds, Float.NaN);
             }
@@ -1962,7 +1972,7 @@ public class ThermalManagerService extends SystemService {
                         < mInactivityThresholdMillis) {
                     // Trigger this again after a second as long as forecast has been called more
                     // recently than the inactivity timeout
-                    mHandler.postDelayed(this::getAndUpdateTemperatureSamples, 1000);
+                    mHandler.postDelayed(mGetAndUpdateTemperatureSamplesRunnable, 1000);
                 } else {
                     // Otherwise, we've been idle for at least 10 seconds, so we should
                     // shut down
@@ -1974,6 +1984,9 @@ public class ThermalManagerService extends SystemService {
                 long now = SystemClock.elapsedRealtime();
                 final List<Temperature> temperatures = mHalWrapper.getCurrentTemperatures(true,
                         Temperature.TYPE_SKIN);
+                if (DEBUG) {
+                    Slog.d(TAG, "Thermal HAL getCurrentTemperatures result: " + temperatures);
+                }
                 for (Temperature temperature : temperatures) {
                     updateTemperatureSampleLocked(now, temperature);
                 }
@@ -2080,10 +2093,16 @@ public class ThermalManagerService extends SystemService {
             }
             synchronized (mSamples) {
                 mLastForecastCallTimeMillis = SystemClock.elapsedRealtime();
-                if (mSamples.isEmpty()) {
+                if (!mHandler.hasCallbacks(mGetAndUpdateTemperatureSamplesRunnable)) {
+                    if (DEBUG) {
+                        Slog.d(TAG, "No temperature update callback, scheduling one");
+                    }
                     getAndUpdateTemperatureSamples();
+                } else {
+                    if (DEBUG) {
+                        Slog.d(TAG, "Temperature update callback already exists");
+                    }
                 }
-
                 // If somehow things take much longer than expected or there are no temperatures
                 // to sample, return early
                 if (mSamples.isEmpty()) {
@@ -2103,8 +2122,11 @@ public class ThermalManagerService extends SystemService {
                             Binder.getCallingUid(),
                             FrameworkStatsLog.THERMAL_HEADROOM_CALLED__API_STATUS__SUCCESS,
                             headroom, forecastSeconds);
-                    Slog.d(TAG, "Headroom forecast in " + forecastSeconds + "s served from cache: "
-                            + headroom);
+                    if (DEBUG) {
+                        Slog.d(TAG,
+                                "Headroom forecast in " + forecastSeconds + "s served from cache: "
+                                        + headroom);
+                    }
                     return headroom;
                 }
 
@@ -2133,7 +2155,10 @@ public class ThermalManagerService extends SystemService {
                                     Binder.getCallingUid(),
                                     FrameworkStatsLog.THERMAL_HEADROOM_CALLED__API_STATUS__SUCCESS,
                                     headroom, 0);
-                            Slog.d(TAG, "Headroom forecast in 0s served from cache: " + headroom);
+                            if (DEBUG) {
+                                Slog.d(TAG,
+                                        "Headroom forecast in 0s served from cache: " + headroom);
+                            }
                             return headroom;
                         }
                         // Don't try to forecast, just use the latest one we have
@@ -2182,7 +2207,9 @@ public class ThermalManagerService extends SystemService {
                     getForecast(DEFAULT_FORECAST_SECONDS),
                     DEFAULT_FORECAST_SECONDS,
                     Arrays.copyOf(mHeadroomThresholds, mHeadroomThresholds.length));
-            Slog.d(TAG, "New headroom callback data: " + data);
+            if (DEBUG) {
+                Slog.d(TAG, "New headroom callback data: " + data);
+            }
             return data;
         }
 
diff --git a/services/tests/servicestests/src/com/android/server/power/ThermalManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/power/ThermalManagerServiceTest.java
index 952d8fa47a34..09acfddacf03 100644
--- a/services/tests/servicestests/src/com/android/server/power/ThermalManagerServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/power/ThermalManagerServiceTest.java
@@ -51,6 +51,7 @@ import android.os.IThermalStatusListener;
 import android.os.PowerManager;
 import android.os.RemoteException;
 import android.os.Temperature;
+import android.platform.test.annotations.DisableFlags;
 import android.platform.test.annotations.EnableFlags;
 import android.platform.test.flag.junit.SetFlagsRule;
 
@@ -78,6 +79,7 @@ import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
 
 /**
  * atest $ANDROID_BUILD_TOP/frameworks/base/services/tests/servicestests/src/com/android/server
@@ -117,7 +119,8 @@ public class ThermalManagerServiceTest {
      */
     private class ThermalHalFake extends ThermalHalWrapper {
         private static final int INIT_STATUS = Temperature.THROTTLING_NONE;
-        private List<Temperature> mTemperatureList = new ArrayList<>();
+        private final List<Temperature> mTemperatureList = new ArrayList<>();
+        private AtomicInteger mGetCurrentTemperaturesCalled = new AtomicInteger();
         private List<CoolingDevice> mCoolingDeviceList = new ArrayList<>();
         private List<TemperatureThreshold> mTemperatureThresholdList = initializeThresholds();
 
@@ -173,6 +176,7 @@ public class ThermalManagerServiceTest {
             mTemperatureList.add(mUsbPort);
             mCoolingDeviceList.add(mCpu);
             mCoolingDeviceList.add(mGpu);
+            mGetCurrentTemperaturesCalled.set(0);
         }
 
         void enableForecastSkinTemperature() {
@@ -188,14 +192,24 @@ public class ThermalManagerServiceTest {
             mForecastSkinTemperaturesError = true;
         }
 
+        void updateTemperatureList(Temperature... temperatures) {
+            synchronized (mTemperatureList) {
+                mTemperatureList.clear();
+                mTemperatureList.addAll(Arrays.asList(temperatures));
+            }
+        }
+
         @Override
         protected List<Temperature> getCurrentTemperatures(boolean shouldFilter, int type) {
             List<Temperature> ret = new ArrayList<>();
-            for (Temperature temperature : mTemperatureList) {
-                if (shouldFilter && type != temperature.getType()) {
-                    continue;
+            synchronized (mTemperatureList) {
+                mGetCurrentTemperaturesCalled.incrementAndGet();
+                for (Temperature temperature : mTemperatureList) {
+                    if (shouldFilter && type != temperature.getType()) {
+                        continue;
+                    }
+                    ret.add(temperature);
                 }
-                ret.add(temperature);
             }
             return ret;
         }
@@ -407,7 +421,7 @@ public class ThermalManagerServiceTest {
         Thread.sleep(CALLBACK_TIMEOUT_MILLI_SEC);
         resetListenerMock();
         int status = Temperature.THROTTLING_SEVERE;
-        mFakeHal.mTemperatureList = new ArrayList<>();
+        mFakeHal.updateTemperatureList();
 
         // Should not notify on non-skin type
         Temperature newBattery = new Temperature(37, Temperature.TYPE_BATTERY, "batt", status);
@@ -537,6 +551,42 @@ public class ThermalManagerServiceTest {
     }
 
     @Test
+    @DisableFlags({Flags.FLAG_ALLOW_THERMAL_HAL_SKIN_FORECAST})
+    public void testGetThermalHeadroom_handlerUpdateTemperatures()
+            throws RemoteException, InterruptedException {
+        // test that handler will at least enqueue one message to periodically read temperatures
+        // even if there is sample seeded from HAL temperature callback
+        String temperatureName = "skin1";
+        Temperature temperature = new Temperature(100, Temperature.TYPE_SKIN, temperatureName,
+                Temperature.THROTTLING_NONE);
+        mFakeHal.mCallback.onTemperatureChanged(temperature);
+        float headroom = mService.mService.getThermalHeadroom(0);
+        // the callback temperature 100C (headroom > 1.0f) sample should have been appended by the
+        // immediately scheduled fake HAL current temperatures read (mSkin1, mSkin2), and because
+        // there are less samples for prediction, the latest temperature mSkin1 is used to calculate
+        // headroom (mSkin2 has no threshold), which is 0.6f (28C vs threshold 40C).
+        assertEquals(0.6f, headroom, 0.01f);
+        // one called by service onActivityManagerReady, one called by handler on headroom call
+        assertEquals(2, mFakeHal.mGetCurrentTemperaturesCalled.get());
+        // periodic read should update the samples history, so the headroom should increase 0.1f
+        // as current temperature goes up by 3C every 1100ms.
+        for (int i = 1; i < 5; i++) {
+            Temperature newTemperature = new Temperature(mFakeHal.mSkin1.getValue() + 3 * i,
+                    Temperature.TYPE_SKIN,
+                    temperatureName,
+                    Temperature.THROTTLING_NONE);
+            mFakeHal.updateTemperatureList(newTemperature);
+            // wait for handler to update temperature
+            Thread.sleep(1100);
+            // assert that only one callback was scheduled to query HAL when making multiple
+            // headroom calls
+            assertEquals(2 + i, mFakeHal.mGetCurrentTemperaturesCalled.get());
+            headroom = mService.mService.getThermalHeadroom(0);
+            assertEquals(0.6f + 0.1f * i, headroom, 0.01f);
+        }
+    }
+
+    @Test
     @EnableFlags({Flags.FLAG_ALLOW_THERMAL_HAL_SKIN_FORECAST})
     public void testGetThermalHeadroom_halForecast() throws RemoteException {
         mFakeHal.mForecastSkinTemperaturesCalled = 0;