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

Commit fce16dc5 authored by Xiang Wang's avatar Xiang Wang
Browse files

Skip updating the samples history on temperature change 

The max headroom staleness of ignoring such adhoc temperature update is
within 1 second, so we don't need to seed the value but let the looper update samples by itself. This will prevent other problems caused by
race between callback update and looper update when two temperature
samples updated within a short window can result in high error variance
when calculating slope during linear regression, even if they have small
value difference. If needed we can revisit this if a temperature can
change drastically within a second on any device.

Bug: 408509573
Test: atest ThermalManagerServiceTest
Flag: EXEMPT bugfix
Change-Id: Ide671ae13b52d5b291e20aab52e9db41058d44c1
parent cddc2fdd

services/core/java/com/android/server/power/ThermalManagerService.java

+14 −13
Original line number Diff line number Diff line
@@ -484,16 +484,11 @@ public class ThermalManagerService extends SystemService { 
                onTemperatureMapChangedLocked();

            }

        }

        if (sendCallback && Flags.allowThermalThresholdsCallback()

                && temperature.getType() == Temperature.TYPE_SKIN) {

            synchronized (mTemperatureWatcher.mSamples) {

        if (DEBUG) {

                    Slog.d(TAG, "Updating new temperature: " + temperature);

                }

                mTemperatureWatcher.updateTemperatureSampleLocked(System.currentTimeMillis(),

                        temperature);

                mTemperatureWatcher.mCachedHeadrooms.clear();

            Slog.d(TAG, "Temperature changed: " + temperature);

        }

        if (sendCallback && Flags.allowThermalThresholdsCallback()

                && temperature.getType() == Temperature.TYPE_SKIN) {

            synchronized (mLock) {

                if (mThermalHeadroomListeners.getRegisteredCallbackCount() == 0) {

                    return;
@@ -2095,19 +2090,24 @@ public class ThermalManagerService extends SystemService { 
                if (DEBUG) {

                    Slog.d(TAG, "Thermal HAL getCurrentTemperatures result: " + temperatures);

                }

                boolean samplesUpdated = false;

                for (Temperature temperature : temperatures) {

                    updateTemperatureSampleLocked(now, temperature);

                    samplesUpdated |= updateTemperatureSampleLocked(now, temperature);

                }

                if (samplesUpdated) {

                    mCachedHeadrooms.clear();

                }

            }

        }



        // returns whether the cache of samples was modified for the provided temperature, which

        // can be used as the signal of whether the headroom cache should be cleared.

        @GuardedBy("mSamples")

        private void updateTemperatureSampleLocked(long timeNow, Temperature temperature) {

        private boolean updateTemperatureSampleLocked(long timeNow, Temperature temperature) {

            // Filter out invalid temperatures. If this results in no values being stored at

            // all, the mSamples.empty() check in getForecast() will catch it.

            // all, the mSamples.isEmpty() check in getForecast() will catch it.

            if (Float.isNaN(temperature.getValue())) {

                return;

                return false;

            }

            ArrayList<Sample> samples = mSamples.computeIfAbsent(temperature.getName(),

                    k -> new ArrayList<>(RING_BUFFER_SIZE));
@@ -2115,6 +2115,7 @@ public class ThermalManagerService extends SystemService { 
                samples.removeFirst();

            }

            samples.add(new Sample(timeNow, temperature.getValue()));

            return true;

        }



        /**

services/tests/servicestests/src/com/android/server/power/ThermalManagerServiceTest.java

+19 −5
Original line number Diff line number Diff line
@@ -442,6 +442,7 @@ public class ThermalManagerServiceTest { 


        // Should not notify on non-skin type should not trigger headroom polling nor notification

        Temperature newBattery = new Temperature(37, Temperature.TYPE_BATTERY, "batt", status);

        mFakeHal.updateTemperatureList(newBattery);

        mFakeHal.mGetCurrentTemperaturesCalled.set(0);

        mFakeHal.mCallback.onTemperatureChanged(newBattery);

        verify(mHeadroomListener, timeout(CALLBACK_TIMEOUT_MILLI_SEC)
@@ -451,6 +452,7 @@ public class ThermalManagerServiceTest { 


        // Notify headroom on skin temperature change

        newSkin = new Temperature(37, Temperature.TYPE_SKIN, "skin1", status);

        mFakeHal.updateTemperatureList(newSkin);

        mFakeHal.mGetCurrentTemperaturesCalled.set(0);

        mFakeHal.mCallback.onTemperatureChanged(newSkin);

        verify(mHeadroomListener, timeout(CALLBACK_TIMEOUT_MILLI_SEC)
@@ -464,23 +466,38 @@ public class ThermalManagerServiceTest { 
        mFakeHal.mCallback.onTemperatureChanged(newSkin);

        Thread.sleep(1000);

        newSkin = new Temperature(36.99f, Temperature.TYPE_SKIN, "skin1", status);

        mFakeHal.updateTemperatureList(newSkin);

        mFakeHal.mCallback.onTemperatureChanged(newSkin);

        Thread.sleep(1000);

        newSkin = new Temperature(37.01f, Temperature.TYPE_SKIN, "skin1", status);

        mFakeHal.updateTemperatureList(newSkin);

        mFakeHal.mCallback.onTemperatureChanged(newSkin);

        verify(mHeadroomListener, timeout(CALLBACK_TIMEOUT_MILLI_SEC)

                .times(0)).onHeadroomChange(anyFloat(), anyFloat(), anyInt(), any());

        resetListenerMock();

        Thread.sleep(1000);



        // Significant temperature should trigger in a short period

        // Significant temperature change should trigger in a short period

        newSkin = new Temperature(34f, Temperature.TYPE_SKIN, "skin1", status);

        mFakeHal.updateTemperatureList(newSkin);

        mFakeHal.mCallback.onTemperatureChanged(newSkin);

        verify(mHeadroomListener, timeout(CALLBACK_TIMEOUT_MILLI_SEC)

                .times(1)).onHeadroomChange(eq(0.8f), anyFloat(), anyInt(),

                eq(new float[]{Float.NaN, 0.6666667f, 0.8333333f, 1.0f, 1.1666666f, 1.3333334f,

                        1.5f}));

        resetListenerMock();

        Thread.sleep(1000);

        newSkin = new Temperature(40f, Temperature.TYPE_SKIN, "skin1", status);

        mFakeHal.updateTemperatureList(newSkin);

        mFakeHal.mCallback.onTemperatureChanged(newSkin);

        verify(mHeadroomListener, timeout(CALLBACK_TIMEOUT_MILLI_SEC)

                .times(1)).onHeadroomChange(eq(1.0f), anyFloat(), anyInt(),

                eq(new float[]{Float.NaN, 0.6666667f, 0.8333333f, 1.0f, 1.1666666f, 1.3333334f,

                        1.5f}));

        resetListenerMock();

        Thread.sleep(ThermalManagerService.HEADROOM_CALLBACK_MIN_INTERVAL_MILLIS);



        // Non-significant temperature change but after long time can also trigger

        mFakeHal.mCallback.onTemperatureChanged(newSkin);

        verify(mHeadroomListener, timeout(CALLBACK_TIMEOUT_MILLI_SEC)

                .times(1)).onHeadroomChange(eq(1.0f), anyFloat(), anyInt(),
@@ -577,15 +594,12 @@ public class ThermalManagerServiceTest { 
    @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

        // callback temperature will not update the samples used for headroom calculation. Since

        // 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);