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

Commit b42d0c70 authored by Kweku Adams's avatar Kweku Adams
Browse files

Fix and expand battery saver stats tracking. 

The STATE_CHARGING value had the BatterySaverState bits to resemble
"ON", so dumpsys would incorrectly say battery saver was on whenever the
device was plugged in. It also meant that the enabled count would
incorrectly increase whenever the device was plugged and unplugged. This
change fixes that by explicitly checking for the magic values before
incrementing the counters.

In addition, this adds more logging around adaptive battery saver stats
and removes the STATE_CHARGING magic value in favor of an explicit
PLUGGED/UNPLUGGED state to move away from the assumption that battery
saver is off whenever the device is plugged in.

Bug: 32423528
Bug: 119261320
Bug: 158581564
Test: atest com.android.server.power.batterysaver.BatterySaverPolicyTest
Test: atest com.android.server.power.batterysaver.BatterySaverStateMachineTest
Test: atest com.android.server.power.batterysaver.BatterySavingStatsTest
Test: atest com.android.server.power.PowerManagerServiceTest
Change-Id: I071d47299360a003febd5251c2515c0b55b3cfff
parent 5a6a5740

services/core/java/com/android/server/power/batterysaver/BatterySaverController.java

+3 −5
Original line number Diff line number Diff line
@@ -49,6 +49,7 @@ import com.android.server.power.batterysaver.BatterySaverPolicy.Policy; 
import com.android.server.power.batterysaver.BatterySavingStats.BatterySaverState;

import com.android.server.power.batterysaver.BatterySavingStats.DozeState;

import com.android.server.power.batterysaver.BatterySavingStats.InteractiveState;

import com.android.server.power.batterysaver.BatterySavingStats.PlugState;



import java.util.ArrayList;

import java.util.Objects;
@@ -551,17 +552,14 @@ public class BatterySaverController implements BatterySaverPolicyListener { 
                        : DozeState.NOT_DOZING;



        synchronized (mLock) {

            if (mIsPluggedIn) {

                mBatterySavingStats.startCharging();

                return;

            }

            mBatterySavingStats.transitionState(

                    getFullEnabledLocked() ? BatterySaverState.ON :

                            (getAdaptiveEnabledLocked() ? BatterySaverState.ADAPTIVE :

                            BatterySaverState.OFF),

                            isInteractive ? InteractiveState.INTERACTIVE :

                            InteractiveState.NON_INTERACTIVE,

                            dozeMode);

                            dozeMode,

                    mIsPluggedIn ? PlugState.PLUGGED : PlugState.UNPLUGGED);

        }

    }

services/core/java/com/android/server/power/batterysaver/BatterySavingStats.java

+105 −40
Original line number Diff line number Diff line
@@ -17,8 +17,8 @@ package com.android.server.power.batterysaver; 


import android.os.BatteryManagerInternal;

import android.os.SystemClock;

import android.util.ArrayMap;

import android.util.Slog;

import android.util.SparseArray;

import android.util.TimeUtils;



import com.android.internal.annotations.GuardedBy;
@@ -93,6 +93,20 @@ public class BatterySavingStats { 
        }

    }



    /** Whether the device is plugged in or not. */

    interface PlugState {

        int UNPLUGGED = 0;

        int PLUGGED = 1;



        int SHIFT = DozeState.SHIFT + DozeState.BITS;

        int BITS = 1;

        int MASK = (1 << BITS) - 1;



        static int fromIndex(int index) {

            return (index >> SHIFT) & MASK;

        }

    }



    /**

     * Various stats in each state.

     */
@@ -140,7 +154,6 @@ public class BatterySavingStats { 
    private BatteryManagerInternal mBatteryManagerInternal;



    private static final int STATE_NOT_INITIALIZED = -1;

    private static final int STATE_CHARGING = -2;



    /**

     * Current state, one of STATE_* or values returned by {@link #statesToIndex}.
@@ -153,20 +166,26 @@ public class BatterySavingStats { 
     */

    @VisibleForTesting

    @GuardedBy("mLock")

    final ArrayMap<Integer, Stat> mStats = new ArrayMap<>();

    final SparseArray<Stat> mStats = new SparseArray<>();



    @GuardedBy("mLock")

    private int mBatterySaverEnabledCount = 0;



    @GuardedBy("mLock")

    private boolean mIsBatterySaverEnabled;



    @GuardedBy("mLock")

    private long mLastBatterySaverEnabledTime = 0;



    @GuardedBy("mLock")

    private long mLastBatterySaverDisabledTime = 0;



    @GuardedBy("mLock")

    private int mAdaptiveBatterySaverEnabledCount = 0;



    @GuardedBy("mLock")

    private long mLastAdaptiveBatterySaverEnabledTime = 0;



    @GuardedBy("mLock")

    private long mLastAdaptiveBatterySaverDisabledTime = 0;



    /** Visible for unit tests */

    @VisibleForTesting

    public BatterySavingStats(Object lock) {
@@ -189,10 +208,11 @@ public class BatterySavingStats { 
     */

    @VisibleForTesting

    static int statesToIndex(

            int batterySaverState, int interactiveState, int dozeState) {

            int batterySaverState, int interactiveState, int dozeState, int plugState) {

        int ret = batterySaverState & BatterySaverState.MASK;

        ret |= (interactiveState & InteractiveState.MASK) << InteractiveState.SHIFT;

        ret |= (dozeState & DozeState.MASK) << DozeState.SHIFT;

        ret |= (plugState & PlugState.MASK) << PlugState.SHIFT;

        return ret;

    }
@@ -204,12 +224,11 @@ public class BatterySavingStats { 
        switch (state) {

            case STATE_NOT_INITIALIZED:

                return "NotInitialized";

            case STATE_CHARGING:

                return "Charging";

        }

        return "BS=" + BatterySaverState.fromIndex(state)

                + ",I=" + InteractiveState.fromIndex(state)

                + ",D=" + DozeState.fromIndex(state);

                + ",D=" + DozeState.fromIndex(state)

                + ",P=" + PlugState.fromIndex(state);

    }



    /**
@@ -230,8 +249,9 @@ public class BatterySavingStats { 
    /**

     * @return {@link Stat} fo a given state triplet.

     */

    private Stat getStat(int batterySaverState, int interactiveState, int dozeState) {

        return getStat(statesToIndex(batterySaverState, interactiveState, dozeState));

    private Stat getStat(int batterySaverState, int interactiveState, int dozeState,

            int plugState) {

        return getStat(statesToIndex(batterySaverState, interactiveState, dozeState, plugState));

    }



    @VisibleForTesting
@@ -258,26 +278,17 @@ public class BatterySavingStats { 
    }



    /**

     * Called from the outside whenever any of the states changes, when the device is not plugged

     * in.

     * Called from the outside whenever any of the states change.

     */

    public void transitionState(int batterySaverState, int interactiveState, int dozeState) {

    void transitionState(int batterySaverState, int interactiveState, int dozeState,

            int plugState) {

        synchronized (mLock) {

            final int newState = statesToIndex(

                    batterySaverState, interactiveState, dozeState);

                    batterySaverState, interactiveState, dozeState, plugState);

            transitionStateLocked(newState);

        }

    }



    /**

     * Called from the outside when the device is plugged in.

     */

    public void startCharging() {

        synchronized (mLock) {

            transitionStateLocked(STATE_CHARGING);

        }

    }



    @GuardedBy("mLock")

    private void transitionStateLocked(int newState) {

        if (mCurrentState == newState) {
@@ -287,17 +298,33 @@ public class BatterySavingStats { 
        final int batteryLevel = injectBatteryLevel();

        final int batteryPercent = injectBatteryPercent();



        final boolean oldBatterySaverEnabled =

                BatterySaverState.fromIndex(mCurrentState) != BatterySaverState.OFF;

        final boolean newBatterySaverEnabled =

                BatterySaverState.fromIndex(newState) != BatterySaverState.OFF;

        if (oldBatterySaverEnabled != newBatterySaverEnabled) {

            mIsBatterySaverEnabled = newBatterySaverEnabled;

            if (newBatterySaverEnabled) {

        final int oldBatterySaverState = mCurrentState < 0

                ? BatterySaverState.OFF : BatterySaverState.fromIndex(mCurrentState);

        final int newBatterySaverState = newState < 0

                ? BatterySaverState.OFF : BatterySaverState.fromIndex(newState);

        if (oldBatterySaverState != newBatterySaverState) {

            switch (newBatterySaverState) {

                case BatterySaverState.ON:

                    mBatterySaverEnabledCount++;

                mLastBatterySaverEnabledTime = injectCurrentTime();

                    mLastBatterySaverEnabledTime = now;

                    if (oldBatterySaverState == BatterySaverState.ADAPTIVE) {

                        mLastAdaptiveBatterySaverDisabledTime = now;

                    }

                    break;

                case BatterySaverState.OFF:

                    if (oldBatterySaverState == BatterySaverState.ON) {

                        mLastBatterySaverDisabledTime = now;

                    } else {

                mLastBatterySaverDisabledTime = injectCurrentTime();

                        mLastAdaptiveBatterySaverDisabledTime = now;

                    }

                    break;

                case BatterySaverState.ADAPTIVE:

                    mAdaptiveBatterySaverEnabledCount++;

                    mLastAdaptiveBatterySaverEnabledTime = now;

                    if (oldBatterySaverState == BatterySaverState.ON) {

                        mLastBatterySaverDisabledTime = now;

                    }

                    break;

            }

        }
@@ -377,7 +404,18 @@ public class BatterySavingStats { 


            pw.print(indent);

            pw.print("Battery Saver is currently: ");

            pw.println(mIsBatterySaverEnabled ? "ON" : "OFF");

            switch (BatterySaverState.fromIndex(mCurrentState)) {

                case BatterySaverState.OFF:

                    pw.println("OFF");

                    break;

                case BatterySaverState.ON:

                    pw.println("ON");

                    break;

                case BatterySaverState.ADAPTIVE:

                    pw.println("ADAPTIVE");

                    break;

            }



            if (mLastBatterySaverEnabledTime > 0) {

                pw.print(indent);

                pw.print("  ");
@@ -400,9 +438,34 @@ public class BatterySavingStats { 


            pw.print(indent);

            pw.print("  ");

            pw.print("Times enabled: ");

            pw.print("Times full enabled: ");

            pw.println(mBatterySaverEnabledCount);



            if (mLastAdaptiveBatterySaverEnabledTime > 0) {

                pw.print(indent);

                pw.print("  ");

                pw.print("Last ADAPTIVE ON time: ");

                pw.print(sdf.format(

                        new Date(now - nowElapsed + mLastAdaptiveBatterySaverEnabledTime)));

                pw.print(" ");

                TimeUtils.formatDuration(mLastAdaptiveBatterySaverEnabledTime, nowElapsed, pw);

                pw.println();

            }

            if (mLastAdaptiveBatterySaverDisabledTime > 0) {

                pw.print(indent);

                pw.print("  ");

                pw.print("Last ADAPTIVE OFF time: ");

                pw.print(sdf.format(

                        new Date(now - nowElapsed + mLastAdaptiveBatterySaverDisabledTime)));

                pw.print(" ");

                TimeUtils.formatDuration(mLastAdaptiveBatterySaverDisabledTime, nowElapsed, pw);

                pw.println();

            }

            pw.print(indent);

            pw.print("  ");

            pw.print("Times adaptive enabled: ");

            pw.println(mAdaptiveBatterySaverEnabledCount);



            pw.println();



            pw.print(indent);
@@ -436,8 +499,10 @@ public class BatterySavingStats { 
        pw.print(interactiveLabel);

        pw.print(": ");



        final Stat offStat = getStat(BatterySaverState.OFF, interactiveState, dozeState);

        final Stat onStat = getStat(BatterySaverState.ON, interactiveState, dozeState);

        final Stat offStat = getStat(BatterySaverState.OFF, interactiveState, dozeState,

                PlugState.UNPLUGGED);

        final Stat onStat = getStat(BatterySaverState.ON, interactiveState, dozeState,

                PlugState.UNPLUGGED);



        pw.println(String.format("%6dm %6dmAh(%3d%%) %8.1fmAh/h     %6dm %6dmAh(%3d%%) %8.1fmAh/h",

                offStat.totalMinutes(),

services/tests/servicestests/src/com/android/server/power/batterysaver/BatterySavingStatsTest.java

+68 −33
Original line number Diff line number Diff line
@@ -33,6 +33,7 @@ import com.android.internal.logging.MetricsLogger; 
import com.android.server.power.batterysaver.BatterySavingStats.BatterySaverState;

import com.android.server.power.batterysaver.BatterySavingStats.DozeState;

import com.android.server.power.batterysaver.BatterySavingStats.InteractiveState;

import com.android.server.power.batterysaver.BatterySavingStats.PlugState;



import org.junit.Test;

import org.junit.runner.RunWith;
@@ -118,7 +119,8 @@ public class BatterySavingStatsTest { 
        target.transitionState(

                BatterySaverState.OFF,

                InteractiveState.INTERACTIVE,

                DozeState.NOT_DOZING);

                DozeState.NOT_DOZING,

                PlugState.UNPLUGGED);



        target.advanceClock(4);

        target.drainBattery(100);
@@ -126,7 +128,8 @@ public class BatterySavingStatsTest { 
        target.transitionState(

                BatterySaverState.OFF,

                InteractiveState.NON_INTERACTIVE,

                DozeState.NOT_DOZING);

                DozeState.NOT_DOZING,

                PlugState.UNPLUGGED);



        target.advanceClock(2);

        target.drainBattery(500);
@@ -134,7 +137,8 @@ public class BatterySavingStatsTest { 
        target.transitionState(

                BatterySaverState.OFF,

                InteractiveState.INTERACTIVE,

                DozeState.NOT_DOZING);

                DozeState.NOT_DOZING,

                PlugState.UNPLUGGED);



        target.advanceClock(4);

        target.drainBattery(100);
@@ -142,7 +146,8 @@ public class BatterySavingStatsTest { 
        target.transitionState(

                BatterySaverState.OFF,

                InteractiveState.NON_INTERACTIVE,

                DozeState.NOT_DOZING);

                DozeState.NOT_DOZING,

                PlugState.UNPLUGGED);



        target.advanceClock(2);

        target.drainBattery(500);
@@ -150,7 +155,8 @@ public class BatterySavingStatsTest { 
        target.transitionState(

                BatterySaverState.OFF,

                InteractiveState.INTERACTIVE,

                DozeState.NOT_DOZING);

                DozeState.NOT_DOZING,

                PlugState.UNPLUGGED);



        target.advanceClock(3);

        target.drainBattery(100);
@@ -158,7 +164,8 @@ public class BatterySavingStatsTest { 
        target.transitionState(

                BatterySaverState.OFF,

                InteractiveState.NON_INTERACTIVE,

                DozeState.LIGHT);

                DozeState.LIGHT,

                PlugState.UNPLUGGED);



        target.advanceClock(5);

        target.drainBattery(100);
@@ -166,7 +173,8 @@ public class BatterySavingStatsTest { 
        target.transitionState(

                BatterySaverState.OFF,

                InteractiveState.NON_INTERACTIVE,

                DozeState.DEEP);

                DozeState.DEEP,

                PlugState.UNPLUGGED);



        target.advanceClock(1);

        target.drainBattery(200);
@@ -174,7 +182,8 @@ public class BatterySavingStatsTest { 
        target.transitionState(

                BatterySaverState.ON,

                InteractiveState.INTERACTIVE,

                DozeState.NOT_DOZING);

                DozeState.NOT_DOZING,

                PlugState.UNPLUGGED);



        target.advanceClock(1);

        target.drainBattery(300);
@@ -182,7 +191,8 @@ public class BatterySavingStatsTest { 
        target.transitionState(

                BatterySaverState.OFF,

                InteractiveState.INTERACTIVE,

                DozeState.NOT_DOZING);

                DozeState.NOT_DOZING,

                PlugState.UNPLUGGED);



        target.advanceClock(3);

        target.drainBattery(500);
@@ -190,12 +200,17 @@ public class BatterySavingStatsTest { 
        target.transitionState(

                BatterySaverState.ON,

                InteractiveState.INTERACTIVE,

                DozeState.NOT_DOZING);

                DozeState.NOT_DOZING,

                PlugState.UNPLUGGED);



        target.advanceClock(3);

        target.drainBattery(500);



        target.startCharging();

        target.transitionState(

                BatterySaverState.ON,

                InteractiveState.INTERACTIVE,

                DozeState.NOT_DOZING,

                PlugState.PLUGGED);



        target.advanceClock(5);

        target.drainBattery(1000);
@@ -203,28 +218,34 @@ public class BatterySavingStatsTest { 
        target.transitionState(

                BatterySaverState.ON,

                InteractiveState.INTERACTIVE,

                DozeState.NOT_DOZING);

                DozeState.NOT_DOZING,

                PlugState.UNPLUGGED);



        target.advanceClock(5);

        target.drainBattery(100);



        target.startCharging();

        target.transitionState(

                BatterySaverState.ON,

                InteractiveState.INTERACTIVE,

                DozeState.NOT_DOZING,

                PlugState.PLUGGED);



        target.assertDumpable();



        assertEquals(

                "BS=0,I=0,D=0:{4m,1000,15000.00uA/H,1500.00%}\n" +

                "BS=1,I=0,D=0:{0m,0,0.00uA/H,0.00%}\n" +

                "BS=0,I=1,D=0:{14m,800,3428.57uA/H,342.86%}\n" +

                "BS=1,I=1,D=0:{9m,900,6000.00uA/H,600.00%}\n" +

                "BS=0,I=0,D=1:{5m,100,1200.00uA/H,120.00%}\n" +

                "BS=1,I=0,D=1:{0m,0,0.00uA/H,0.00%}\n" +

                "BS=0,I=1,D=1:{0m,0,0.00uA/H,0.00%}\n" +

                "BS=1,I=1,D=1:{0m,0,0.00uA/H,0.00%}\n" +

                "BS=0,I=0,D=2:{1m,200,12000.00uA/H,1200.00%}\n" +

                "BS=1,I=0,D=2:{0m,0,0.00uA/H,0.00%}\n" +

                "BS=0,I=1,D=2:{0m,0,0.00uA/H,0.00%}\n" +

                "BS=1,I=1,D=2:{0m,0,0.00uA/H,0.00%}",

                "BS=0,I=0,D=0,P=0:{4m,1000,15000.00uA/H,1500.00%}\n"

                        + "BS=1,I=0,D=0,P=0:{0m,0,0.00uA/H,0.00%}\n"

                        + "BS=0,I=1,D=0,P=0:{14m,800,3428.57uA/H,342.86%}\n"

                        + "BS=1,I=1,D=0,P=0:{9m,900,6000.00uA/H,600.00%}\n"

                        + "BS=0,I=0,D=1,P=0:{5m,100,1200.00uA/H,120.00%}\n"

                        + "BS=1,I=0,D=1,P=0:{0m,0,0.00uA/H,0.00%}\n"

                        + "BS=0,I=1,D=1,P=0:{0m,0,0.00uA/H,0.00%}\n"

                        + "BS=1,I=1,D=1,P=0:{0m,0,0.00uA/H,0.00%}\n"

                        + "BS=0,I=0,D=2,P=0:{1m,200,12000.00uA/H,1200.00%}\n"

                        + "BS=1,I=0,D=2,P=0:{0m,0,0.00uA/H,0.00%}\n"

                        + "BS=0,I=1,D=2,P=0:{0m,0,0.00uA/H,0.00%}\n"

                        + "BS=1,I=1,D=2,P=0:{0m,0,0.00uA/H,0.00%}\n"

                        + "BS=1,I=1,D=0,P=1:{5m,1000,12000.00uA/H,1200.00%}",

                target.toDebugString());

    }
@@ -242,7 +263,8 @@ public class BatterySavingStatsTest { 
        target.transitionState(

                BatterySaverState.OFF,

                InteractiveState.INTERACTIVE,

                DozeState.NOT_DOZING);

                DozeState.NOT_DOZING,

                PlugState.UNPLUGGED);



        verify(mMetricsLogger, times(0)).count(anyString(), anyInt());
@@ -253,7 +275,8 @@ public class BatterySavingStatsTest { 
        target.transitionState(

                BatterySaverState.OFF,

                InteractiveState.NON_INTERACTIVE,

                DozeState.NOT_DOZING);

                DozeState.NOT_DOZING,

                PlugState.UNPLUGGED);



        assertLog();
@@ -264,7 +287,8 @@ public class BatterySavingStatsTest { 
        target.transitionState(

                BatterySaverState.OFF,

                InteractiveState.NON_INTERACTIVE,

                DozeState.DEEP);

                DozeState.DEEP,

                PlugState.UNPLUGGED);



        target.advanceClock(1);

        target.drainBattery(2000);
@@ -274,7 +298,8 @@ public class BatterySavingStatsTest { 
        target.transitionState(

                BatterySaverState.OFF,

                InteractiveState.NON_INTERACTIVE,

                DozeState.LIGHT);

                DozeState.LIGHT,

                PlugState.UNPLUGGED);



        target.advanceClock(1);

        target.drainBattery(2000);
@@ -284,7 +309,8 @@ public class BatterySavingStatsTest { 
        target.transitionState(

                BatterySaverState.ON,

                InteractiveState.INTERACTIVE,

                DozeState.NOT_DOZING);

                DozeState.NOT_DOZING,

                PlugState.UNPLUGGED);



        assertLog();
@@ -292,7 +318,11 @@ public class BatterySavingStatsTest { 
        target.drainBattery(10000);



        reset(mMetricsLogger);

        target.startCharging();

        target.transitionState(

                BatterySaverState.ON,

                InteractiveState.INTERACTIVE,

                DozeState.NOT_DOZING,

                PlugState.PLUGGED);



        assertLog();
@@ -303,14 +333,19 @@ public class BatterySavingStatsTest { 
        target.transitionState(

                BatterySaverState.ON,

                InteractiveState.NON_INTERACTIVE,

                DozeState.NOT_DOZING);

                DozeState.NOT_DOZING,

                PlugState.UNPLUGGED);



        verify(mMetricsLogger, times(0)).count(anyString(), anyInt());



        target.advanceClock(1);

        target.drainBattery(2000);



        target.startCharging();

        target.transitionState(

                BatterySaverState.ON,

                InteractiveState.NON_INTERACTIVE,

                DozeState.NOT_DOZING,

                PlugState.PLUGGED);



        assertLog();

    }