From f941a684e1c0eb18eac9950991e67bc4222fb60b Mon Sep 17 00:00:00 2001 From: Dmitri Plotnikov Date: Fri, 12 Mar 2021 15:01:13 -0800 Subject: [PATCH] Remove smearing of hidden BatterySipper power Bug: 182598424 Test: make RunSettingsRoboTests ROBOTEST_FILTER=com.android.settings.applications.appinfo.AppBatteryPreferenceControllerTest Test: make RunSettingsRoboTests ROBOTEST_FILTER=com.android.settings.fuelgauge.BatteryUtilsTest Change-Id: I78b8d7c4faafa83de198005617e99a7f54bcd174 --- .../AppBatteryPreferenceController.java | 7 +- .../BatteryAppListPreferenceController.java | 7 +- .../settings/fuelgauge/BatteryUtils.java | 68 ++---------------- .../detectors/HighUsageDetector.java | 2 +- .../AppBatteryPreferenceControllerTest.java | 2 +- .../settings/fuelgauge/BatteryUtilsTest.java | 72 +------------------ 6 files changed, 14 insertions(+), 144 deletions(-) diff --git a/src/com/android/settings/applications/appinfo/AppBatteryPreferenceController.java b/src/com/android/settings/applications/appinfo/AppBatteryPreferenceController.java index cc20d4b9771..19187ba9930 100644 --- a/src/com/android/settings/applications/appinfo/AppBatteryPreferenceController.java +++ b/src/com/android/settings/applications/appinfo/AppBatteryPreferenceController.java @@ -46,7 +46,6 @@ import com.android.settingslib.core.lifecycle.LifecycleObserver; import com.android.settingslib.core.lifecycle.events.OnPause; import com.android.settingslib.core.lifecycle.events.OnResume; -import java.util.ArrayList; import java.util.List; public class AppBatteryPreferenceController extends BasePreferenceController @@ -162,13 +161,9 @@ public class AppBatteryPreferenceController extends BasePreferenceController void updateBattery() { mPreference.setEnabled(true); if (isBatteryStatsAvailable()) { - final int dischargePercentage = mBatteryUsageStats.getDischargePercentage(); - - final List usageList = new ArrayList<>(mBatteryHelper.getUsageList()); - final double hiddenAmount = mBatteryUtils.removeHiddenBatterySippers(usageList); final int percentOfMax = (int) mBatteryUtils.calculateBatteryPercent( mUidBatteryConsumer.getConsumedPower(), mBatteryUsageStats.getConsumedPower(), - hiddenAmount, dischargePercentage); + mBatteryUsageStats.getDischargePercentage()); mBatteryPercent = Utils.formatPercentage(percentOfMax); mPreference.setSummary(mContext.getString(R.string.battery_summary, mBatteryPercent)); } else { diff --git a/src/com/android/settings/fuelgauge/BatteryAppListPreferenceController.java b/src/com/android/settings/fuelgauge/BatteryAppListPreferenceController.java index 56a2b01c263..cfa9e23a076 100644 --- a/src/com/android/settings/fuelgauge/BatteryAppListPreferenceController.java +++ b/src/com/android/settings/fuelgauge/BatteryAppListPreferenceController.java @@ -189,8 +189,9 @@ public class BatteryAppListPreferenceController extends AbstractPreferenceContro if (averagePower >= MIN_AVERAGE_POWER_THRESHOLD_MILLI_AMP || USE_FAKE_DATA) { final List usageList = getCoalescedUsageList( USE_FAKE_DATA ? getFakeStats() : statsHelper.getUsageList()); - double hiddenPowerMah = showAllApps ? 0 : - mBatteryUtils.removeHiddenBatterySippers(usageList); + if (!showAllApps) { + mBatteryUtils.removeHiddenBatterySippers(usageList); + } mBatteryUtils.sortUsageList(usageList); final int numSippers = usageList.size(); @@ -199,7 +200,7 @@ public class BatteryAppListPreferenceController extends AbstractPreferenceContro double totalPower = USE_FAKE_DATA ? 4000 : statsHelper.getTotalPower(); final double percentOfTotal = mBatteryUtils.calculateBatteryPercent( - sipper.totalPowerMah, totalPower, hiddenPowerMah, dischargeAmount); + sipper.totalPowerMah, totalPower, dischargeAmount); if (((int) (percentOfTotal + .5)) < 1) { continue; diff --git a/src/com/android/settings/fuelgauge/BatteryUtils.java b/src/com/android/settings/fuelgauge/BatteryUtils.java index 8c0ab46a347..0174cfa5a84 100644 --- a/src/com/android/settings/fuelgauge/BatteryUtils.java +++ b/src/com/android/settings/fuelgauge/BatteryUtils.java @@ -33,9 +33,7 @@ import android.os.Process; import android.os.SystemClock; import android.os.UserHandle; import android.os.UserManager; -import android.text.format.DateUtils; import android.util.Log; -import android.util.SparseLongArray; import androidx.annotation.IntDef; import androidx.annotation.Nullable; @@ -174,72 +172,16 @@ public class BatteryUtils { } /** - * Remove the {@link BatterySipper} that we should hide and smear the screen usage based on - * foreground activity time. + * Remove the {@link BatterySipper} that we should hide. * * @param sippers sipper list that need to check and remove - * @return the total power of the hidden items of {@link BatterySipper} * for proportional smearing */ - public double removeHiddenBatterySippers(List sippers) { - double proportionalSmearPowerMah = 0; - BatterySipper screenSipper = null; + public void removeHiddenBatterySippers(List sippers) { for (int i = sippers.size() - 1; i >= 0; i--) { final BatterySipper sipper = sippers.get(i); if (shouldHideSipper(sipper)) { sippers.remove(i); - if (sipper.drainType != BatterySipper.DrainType.OVERCOUNTED - && sipper.drainType != BatterySipper.DrainType.SCREEN - && sipper.drainType != BatterySipper.DrainType.UNACCOUNTED - && sipper.drainType != BatterySipper.DrainType.BLUETOOTH - && sipper.drainType != BatterySipper.DrainType.WIFI - && sipper.drainType != BatterySipper.DrainType.IDLE - && !isHiddenSystemModule(sipper)) { - // Don't add it if it is overcounted, unaccounted, wifi, bluetooth, screen - // or hidden system modules - proportionalSmearPowerMah += sipper.totalPowerMah; - } - } - - if (sipper.drainType == BatterySipper.DrainType.SCREEN) { - screenSipper = sipper; - } - } - - smearScreenBatterySipper(sippers, screenSipper); - - return proportionalSmearPowerMah; - } - - /** - * Smear the screen on power usage among {@code sippers}, based on ratio of foreground activity - * time. - */ - @VisibleForTesting - void smearScreenBatterySipper(List sippers, BatterySipper screenSipper) { - long totalActivityTimeMs = 0; - final SparseLongArray activityTimeArray = new SparseLongArray(); - for (int i = 0, size = sippers.size(); i < size; i++) { - final BatteryStats.Uid uid = sippers.get(i).uidObj; - if (uid != null) { - final long timeMs = getProcessTimeMs(StatusType.SCREEN_USAGE, uid, - BatteryStats.STATS_SINCE_CHARGED); - activityTimeArray.put(uid.getUid(), timeMs); - totalActivityTimeMs += timeMs; - } - } - - if (totalActivityTimeMs >= 10 * DateUtils.MINUTE_IN_MILLIS) { - if (screenSipper == null) { - Log.e(TAG, "screen sipper is null even when app screen time is not zero"); - return; - } - - final double screenPowerMah = screenSipper.totalPowerMah; - for (int i = 0, size = sippers.size(); i < size; i++) { - final BatterySipper sipper = sippers.get(i); - sipper.totalPowerMah += screenPowerMah * activityTimeArray.get(sipper.getUid(), 0) - / totalActivityTimeMs; } } } @@ -287,19 +229,17 @@ public class BatteryUtils { * * @param powerUsageMah power used by the app * @param totalPowerMah total power used in the system - * @param hiddenPowerMah power used by no-actionable app that we want to hide, i.e. Screen, - * Android OS. * @param dischargeAmount The discharge amount calculated by {@link BatteryStats} * @return A percentage value scaled by {@paramref dischargeAmount} * @see BatteryStats#getDischargeAmount(int) */ public double calculateBatteryPercent(double powerUsageMah, double totalPowerMah, - double hiddenPowerMah, int dischargeAmount) { + int dischargeAmount) { if (totalPowerMah == 0) { return 0; } - return (powerUsageMah / (totalPowerMah - hiddenPowerMah)) * dischargeAmount; + return (powerUsageMah / totalPowerMah) * dischargeAmount; } /** diff --git a/src/com/android/settings/fuelgauge/batterytip/detectors/HighUsageDetector.java b/src/com/android/settings/fuelgauge/batterytip/detectors/HighUsageDetector.java index 928ae52532c..6c102fb5d4d 100644 --- a/src/com/android/settings/fuelgauge/batterytip/detectors/HighUsageDetector.java +++ b/src/com/android/settings/fuelgauge/batterytip/detectors/HighUsageDetector.java @@ -86,7 +86,7 @@ public class HighUsageDetector implements BatteryTipDetector { sipper1.totalSmearedPowerMah)); for (BatterySipper batterySipper : batterySippers) { final double percent = mBatteryUtils.calculateBatteryPercent( - batterySipper.totalSmearedPowerMah, totalPower, 0, dischargeAmount); + batterySipper.totalSmearedPowerMah, totalPower, dischargeAmount); if ((percent + 0.5f < 1f) || mBatteryUtils.shouldHideSipper(batterySipper)) { // Don't show it if we should hide or usage percentage is lower than 1% continue; diff --git a/tests/robotests/src/com/android/settings/applications/appinfo/AppBatteryPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/applications/appinfo/AppBatteryPreferenceControllerTest.java index c735452732a..440ad044087 100644 --- a/tests/robotests/src/com/android/settings/applications/appinfo/AppBatteryPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/applications/appinfo/AppBatteryPreferenceControllerTest.java @@ -152,7 +152,7 @@ public class AppBatteryPreferenceControllerTest { mController.mBatteryUsageStats = mBatteryUsageStats; mController.mUidBatteryConsumer = mUidBatteryConsumer; doReturn(BATTERY_LEVEL).when(mBatteryUtils).calculateBatteryPercent(anyDouble(), - anyDouble(), anyDouble(), anyInt()); + anyDouble(), anyInt()); doReturn(new ArrayList<>()).when(mBatteryStatsHelper).getUsageList(); mController.displayPreference(mScreen); diff --git a/tests/robotests/src/com/android/settings/fuelgauge/BatteryUtilsTest.java b/tests/robotests/src/com/android/settings/fuelgauge/BatteryUtilsTest.java index c8fdf8c9562..17fee4aa2c3 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/BatteryUtilsTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/BatteryUtilsTest.java @@ -28,11 +28,8 @@ import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.nullable; -import static org.mockito.Mockito.RETURNS_DEEP_STUBS; -import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; @@ -54,7 +51,6 @@ import android.os.Bundle; import android.os.Process; import android.os.SystemClock; import android.os.UserManager; -import android.text.format.DateUtils; import com.android.internal.os.BatterySipper; import com.android.internal.os.BatteryStatsHelper; @@ -91,8 +87,6 @@ public class BatteryUtilsTest { private static final long TIME_STATE_TOP_SLEEPING = 2500 * UNIT; private static final long TIME_STATE_FOREGROUND = 3000 * UNIT; private static final long TIME_STATE_BACKGROUND = 6000 * UNIT; - private static final long TIME_FOREGROUND_ZERO = 0; - private static final long TIME_FOREGROUND = 100 * DateUtils.MINUTE_IN_MILLIS; private static final long TIME_SINCE_LAST_FULL_CHARGE_MS = 120 * 60 * 1000; private static final long TIME_SINCE_LAST_FULL_CHARGE_US = TIME_SINCE_LAST_FULL_CHARGE_MS * 1000; @@ -106,13 +100,11 @@ public class BatteryUtilsTest { private static final double BATTERY_SYSTEM_USAGE = 600; private static final double BATTERY_OVERACCOUNTED_USAGE = 500; private static final double BATTERY_UNACCOUNTED_USAGE = 700; - private static final double BATTERY_APP_USAGE = 100; private static final double BATTERY_WIFI_USAGE = 200; private static final double BATTERY_BLUETOOTH_USAGE = 300; private static final double TOTAL_BATTERY_USAGE = 1000; - private static final double HIDDEN_USAGE = 200; private static final int DISCHARGE_AMOUNT = 80; - private static final double PERCENT_SYSTEM_USAGE = 60; + private static final double PERCENT_SYSTEM_USAGE = 48; private static final double PRECISION = 0.001; private static final int SDK_VERSION = Build.VERSION_CODES.L; private static final String PACKAGE_NAME = "com.android.app"; @@ -302,12 +294,10 @@ public class BatteryUtilsTest { sippers.add(mBluetoothBatterySipper); sippers.add(mIdleBatterySipper); when(mProvider.isTypeSystem(mSystemBatterySipper)).thenReturn(true); - doNothing().when(mBatteryUtils).smearScreenBatterySipper(any(), any()); - final double totalUsage = mBatteryUtils.removeHiddenBatterySippers(sippers); + mBatteryUtils.removeHiddenBatterySippers(sippers); assertThat(sippers).containsExactly(mNormalBatterySipper); - assertThat(totalUsage).isWithin(PRECISION).of(BATTERY_SYSTEM_USAGE); } @Test @@ -388,49 +378,10 @@ public class BatteryUtilsTest { @Test public void testCalculateBatteryPercent() { assertThat(mBatteryUtils.calculateBatteryPercent(BATTERY_SYSTEM_USAGE, TOTAL_BATTERY_USAGE, - HIDDEN_USAGE, DISCHARGE_AMOUNT)) + DISCHARGE_AMOUNT)) .isWithin(PRECISION).of(PERCENT_SYSTEM_USAGE); } - @Test - public void testSmearScreenBatterySipper() { - final BatterySipper sipperNull = createTestSmearBatterySipper(TIME_FOREGROUND_ZERO, - BATTERY_APP_USAGE, 0 /* uid */, true /* isUidNull */); - final BatterySipper sipperBg = createTestSmearBatterySipper(TIME_FOREGROUND_ZERO, - BATTERY_APP_USAGE, 1 /* uid */, false /* isUidNull */); - final BatterySipper sipperFg = createTestSmearBatterySipper(TIME_FOREGROUND, - BATTERY_APP_USAGE, 2 /* uid */, false /* isUidNull */); - final BatterySipper sipperFg2 = createTestSmearBatterySipper(TIME_FOREGROUND, - BATTERY_APP_USAGE, 3 /* uid */, false /* isUidNull */); - - final List sippers = new ArrayList<>(); - sippers.add(sipperNull); - sippers.add(sipperBg); - sippers.add(sipperFg); - sippers.add(sipperFg2); - - mBatteryUtils.smearScreenBatterySipper(sippers, mScreenBatterySipper); - - assertThat(sipperNull.totalPowerMah).isWithin(PRECISION).of(BATTERY_APP_USAGE); - assertThat(sipperBg.totalPowerMah).isWithin(PRECISION).of(BATTERY_APP_USAGE); - assertThat(sipperFg.totalPowerMah).isWithin(PRECISION).of( - BATTERY_APP_USAGE + BATTERY_SCREEN_USAGE / 2); - assertThat(sipperFg2.totalPowerMah).isWithin(PRECISION).of( - BATTERY_APP_USAGE + BATTERY_SCREEN_USAGE / 2); - } - - @Test - public void testSmearScreenBatterySipper_screenSipperNull_shouldNotCrash() { - final BatterySipper sipperFg = createTestSmearBatterySipper(TIME_FOREGROUND, - BATTERY_APP_USAGE, 2 /* uid */, false /* isUidNull */); - - final List sippers = new ArrayList<>(); - sippers.add(sipperFg); - - // Shouldn't crash - mBatteryUtils.smearScreenBatterySipper(sippers, null /* screenSipper */); - } - @Test public void testCalculateRunningTimeBasedOnStatsType() { assertThat(mBatteryUtils.calculateRunningTimeBasedOnStatsType(mBatteryStatsHelper, @@ -509,23 +460,6 @@ public class BatteryUtilsTest { .isFalse(); } - private BatterySipper createTestSmearBatterySipper( - long topTime, double totalPowerMah, int uidCode, boolean isUidNull) { - final BatterySipper sipper = mock(BatterySipper.class); - sipper.drainType = BatterySipper.DrainType.APP; - sipper.totalPowerMah = totalPowerMah; - doReturn(uidCode).when(sipper).getUid(); - if (!isUidNull) { - final BatteryStats.Uid uid = mock(BatteryStats.Uid.class, RETURNS_DEEP_STUBS); - doReturn(topTime).when(mBatteryUtils).getProcessTimeMs( - eq(BatteryUtils.StatusType.SCREEN_USAGE), eq(uid), anyInt()); - doReturn(uidCode).when(uid).getUid(); - sipper.uidObj = uid; - } - - return sipper; - } - @Test public void testInitBatteryStatsHelper_init() { mBatteryUtils.initBatteryStatsHelper(mBatteryStatsHelper, mBundle, mUserManager);