From 4280a7f897da029e79ced0ae43a5332b1fbac169 Mon Sep 17 00:00:00 2001 From: ykhung Date: Fri, 7 May 2021 12:27:38 +0800 Subject: [PATCH] Refine the interpolation rule and clip the usage time data Bug: 184807417 Test: make SettingsRoboTests Change-Id: I6c115beed34abd393e5c615cc59d3c4c323dfa0b --- .../BatteryChartPreferenceController.java | 2 +- .../settings/fuelgauge/ConvertUtils.java | 62 +++++++++++----- .../settings/fuelgauge/ConvertUtilsTest.java | 74 +++++++++++++++++-- 3 files changed, 114 insertions(+), 24 deletions(-) diff --git a/src/com/android/settings/fuelgauge/BatteryChartPreferenceController.java b/src/com/android/settings/fuelgauge/BatteryChartPreferenceController.java index c4efef88ccd..f4c9b0ce775 100644 --- a/src/com/android/settings/fuelgauge/BatteryChartPreferenceController.java +++ b/src/com/android/settings/fuelgauge/BatteryChartPreferenceController.java @@ -223,7 +223,7 @@ public class BatteryChartPreferenceController extends AbstractPreferenceControll ConvertUtils.getIndexedUsageMap( mPrefContext, /*timeSlotSize=*/ CHART_LEVEL_ARRAY_SIZE - 1, mBatteryHistoryKeys, batteryHistoryMap, - /*purgeLowPercentageData=*/ true); + /*purgeLowPercentageAndFakeData=*/ true); forceRefreshUi(); Log.d(TAG, String.format( diff --git a/src/com/android/settings/fuelgauge/ConvertUtils.java b/src/com/android/settings/fuelgauge/ConvertUtils.java index 1f6600bc272..fc58e501af5 100644 --- a/src/com/android/settings/fuelgauge/ConvertUtils.java +++ b/src/com/android/settings/fuelgauge/ConvertUtils.java @@ -18,12 +18,15 @@ import android.content.ContentValues; import android.content.Context; import android.os.BatteryUsageStats; import android.os.UserHandle; +import android.text.format.DateUtils; +import android.util.Log; import androidx.annotation.VisibleForTesting; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.text.SimpleDateFormat; +import java.time.Duration; import java.util.ArrayList; import java.util.Date; import java.util.HashMap; @@ -41,6 +44,8 @@ public final class ConvertUtils { private static final Map EMPTY_BATTERY_MAP = new HashMap<>(); private static final BatteryHistEntry EMPTY_BATTERY_HIST_ENTRY = new BatteryHistEntry(new ContentValues()); + // Maximum total time value for each slot cumulative data at most 2 hours. + private static final float TOTAL_TIME_THRESHOLD = DateUtils.HOUR_IN_MILLIS * 2; @VisibleForTesting static double PERCENTAGE_OF_TOTAL_THRESHOLD = 1f; @@ -145,7 +150,10 @@ public final class ConvertUtils { final int timeSlotSize, final long[] batteryHistoryKeys, final Map> batteryHistoryMap, - final boolean purgeLowPercentageData) { + final boolean purgeLowPercentageAndFakeData) { + if (batteryHistoryMap == null || batteryHistoryMap.isEmpty()) { + return new HashMap<>(); + } final Map> resultMap = new HashMap<>(); // Each time slot usage diff data = // Math.abs(timestamp[i+2] data - timestamp[i+1] data) + @@ -155,16 +163,10 @@ public final class ConvertUtils { for (int index = 0; index < timeSlotSize; index++) { final Long currentTimestamp = Long.valueOf(batteryHistoryKeys[index * timestampStride]); - // Uses empty list if the timestamp is default value. - if (currentTimestamp == 0) { - resultMap.put(Integer.valueOf(index), new ArrayList()); - continue; - } final Long nextTimestamp = Long.valueOf(batteryHistoryKeys[index * timestampStride + 1]); final Long nextTwoTimestamp = Long.valueOf(batteryHistoryKeys[index * timestampStride + 2]); - // Fetches BatteryHistEntry data from corresponding time slot. final Map currentBatteryHistMap = batteryHistoryMap.getOrDefault(currentTimestamp, EMPTY_BATTERY_MAP); @@ -172,8 +174,17 @@ public final class ConvertUtils { batteryHistoryMap.getOrDefault(nextTimestamp, EMPTY_BATTERY_MAP); final Map nextTwoBatteryHistMap = batteryHistoryMap.getOrDefault(nextTwoTimestamp, EMPTY_BATTERY_MAP); + // We should not get the empty list since we have at least one fake data to record + // the battery level and status in each time slot, the empty list is used to + // represent there is no enough data to apply interpolation arithmetic. + if (currentBatteryHistMap.isEmpty() + || nextBatteryHistMap.isEmpty() + || nextTwoBatteryHistMap.isEmpty()) { + resultMap.put(Integer.valueOf(index), new ArrayList()); + continue; + } - // Collects all keys in these three time slot records as population. + // Collects all keys in these three time slot records as all populations. final Set allBatteryHistEntryKeys = new HashSet<>(); allBatteryHistEntryKeys.addAll(currentBatteryHistMap.keySet()); allBatteryHistEntryKeys.addAll(nextBatteryHistMap.keySet()); @@ -193,12 +204,12 @@ public final class ConvertUtils { final BatteryHistEntry nextTwoEntry = nextTwoBatteryHistMap.getOrDefault(key, EMPTY_BATTERY_HIST_ENTRY); // Cumulative values is a specific time slot for a specific app. - final long foregroundUsageTimeInMs = + long foregroundUsageTimeInMs = getDiffValue( currentEntry.mForegroundUsageTimeInMs, nextEntry.mForegroundUsageTimeInMs, nextTwoEntry.mForegroundUsageTimeInMs); - final long backgroundUsageTimeInMs = + long backgroundUsageTimeInMs = getDiffValue( currentEntry.mBackgroundUsageTimeInMs, nextEntry.mBackgroundUsageTimeInMs, @@ -212,8 +223,8 @@ public final class ConvertUtils { // Excludes entry since we don't have enough data to calculate. if (foregroundUsageTimeInMs == 0 - && backgroundUsageTimeInMs == 0 - && consumePower == 0) { + && backgroundUsageTimeInMs == 0 + && consumePower == 0) { continue; } final BatteryHistEntry selectedBatteryEntry = @@ -221,6 +232,21 @@ public final class ConvertUtils { if (selectedBatteryEntry == null) { continue; } + // Force refine the cumulative value since it may introduce deviation + // error since we will apply the interpolation arithmetic. + final float totalUsageTimeInMs = + foregroundUsageTimeInMs + backgroundUsageTimeInMs; + if (totalUsageTimeInMs > TOTAL_TIME_THRESHOLD) { + final float ratio = TOTAL_TIME_THRESHOLD / totalUsageTimeInMs; + Log.w(TAG, String.format("abnormal usage time %d|%d for:\n%s", + Duration.ofMillis(foregroundUsageTimeInMs).getSeconds(), + Duration.ofMillis(backgroundUsageTimeInMs).getSeconds(), + currentEntry)); + foregroundUsageTimeInMs = + Math.round(foregroundUsageTimeInMs * ratio); + backgroundUsageTimeInMs = + Math.round(backgroundUsageTimeInMs * ratio); + } batteryDiffEntryList.add( new BatteryDiffEntry( context, @@ -234,10 +260,10 @@ public final class ConvertUtils { diffEntry.setTotalConsumePower(totalConsumePower); } } - insert24HoursData(BatteryChartView.SELECTED_INDEX_ALL, resultMap); - if (purgeLowPercentageData) { - purgeLowPercentageData(resultMap); + if (purgeLowPercentageAndFakeData) { + purgeLowPercentageAndFakeData(resultMap); } + insert24HoursData(BatteryChartView.SELECTED_INDEX_ALL, resultMap); return resultMap; } @@ -273,13 +299,15 @@ public final class ConvertUtils { indexedUsageMap.put(Integer.valueOf(desiredIndex), resultList); } - private static void purgeLowPercentageData( + // Removes low percentage data and fake usage data, which will be zero value. + private static void purgeLowPercentageAndFakeData( final Map> indexedUsageMap) { for (List entries : indexedUsageMap.values()) { final Iterator iterator = entries.iterator(); while (iterator.hasNext()) { final BatteryDiffEntry entry = iterator.next(); - if (entry.getPercentOfTotal() < PERCENTAGE_OF_TOTAL_THRESHOLD) { + if (entry.getPercentOfTotal() < PERCENTAGE_OF_TOTAL_THRESHOLD + || FAKE_PACKAGE_NAME.equals(entry.getPackageName())) { iterator.remove(); } } diff --git a/tests/robotests/src/com/android/settings/fuelgauge/ConvertUtilsTest.java b/tests/robotests/src/com/android/settings/fuelgauge/ConvertUtilsTest.java index 30df466bee1..c4341ef91f3 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/ConvertUtilsTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/ConvertUtilsTest.java @@ -140,6 +140,21 @@ public final class ConvertUtilsTest { .isEqualTo(ConvertUtils.FAKE_PACKAGE_NAME); } + @Test + public void testGetIndexedUsageMap_nullOrEmptyHistoryMap_returnEmptyCollection() { + final int timeSlotSize = 2; + final long[] batteryHistoryKeys = new long[] {101L, 102L, 103L, 104L, 105L}; + + assertThat(ConvertUtils.getIndexedUsageMap( + mContext, timeSlotSize, batteryHistoryKeys, + /*batteryHistoryMap=*/ null, /*purgeLowPercentageAndFakeData=*/ true)) + .isEmpty(); + assertThat(ConvertUtils.getIndexedUsageMap( + mContext, timeSlotSize, batteryHistoryKeys, + new HashMap>(), + /*purgeLowPercentageAndFakeData=*/ true)) + .isEmpty(); + } @Test public void testGetIndexedUsageMap_returnsExpectedResult() { // Creates the fake testing data. @@ -147,21 +162,25 @@ public final class ConvertUtilsTest { final long[] batteryHistoryKeys = new long[] {101L, 102L, 103L, 104L, 105L}; final Map> batteryHistoryMap = new HashMap<>(); + final BatteryHistEntry fakeEntry = createBatteryHistEntry( + ConvertUtils.FAKE_PACKAGE_NAME, "fake_label", 0, 0L, 0L, 0L); // Adds the index = 0 data. Map entryMap = new HashMap<>(); BatteryHistEntry entry = createBatteryHistEntry( "package1", "label1", 5.0, 1L, 10L, 20L); entryMap.put(entry.getKey(), entry); + entryMap.put(fakeEntry.getKey(), fakeEntry); batteryHistoryMap.put(Long.valueOf(batteryHistoryKeys[0]), entryMap); // Adds the index = 1 data. - batteryHistoryMap.put( - Long.valueOf(batteryHistoryKeys[1]), - new HashMap()); + entryMap = new HashMap<>(); + entryMap.put(fakeEntry.getKey(), fakeEntry); + batteryHistoryMap.put(Long.valueOf(batteryHistoryKeys[1]), entryMap); // Adds the index = 2 data. entryMap = new HashMap<>(); entry = createBatteryHistEntry( "package2", "label2", 10.0, 2L, 15L, 25L); entryMap.put(entry.getKey(), entry); + entryMap.put(fakeEntry.getKey(), fakeEntry); batteryHistoryMap.put(Long.valueOf(batteryHistoryKeys[2]), entryMap); // Adds the index = 3 data. entryMap = new HashMap<>(); @@ -171,6 +190,7 @@ public final class ConvertUtilsTest { entry = createBatteryHistEntry( "package3", "label3", 5.0, 3L, 5L, 5L); entryMap.put(entry.getKey(), entry); + entryMap.put(fakeEntry.getKey(), fakeEntry); batteryHistoryMap.put(Long.valueOf(batteryHistoryKeys[3]), entryMap); // Adds the index = 4 data. entryMap = new HashMap<>(); @@ -183,12 +203,13 @@ public final class ConvertUtilsTest { entry = createBatteryHistEntry( "package3", "label3", 5.0, 3L, 5L, 5L); entryMap.put(entry.getKey(), entry); + entryMap.put(fakeEntry.getKey(), fakeEntry); batteryHistoryMap.put(Long.valueOf(batteryHistoryKeys[4]), entryMap); final Map> resultMap = ConvertUtils.getIndexedUsageMap( mContext, timeSlotSize, batteryHistoryKeys, batteryHistoryMap, - /*purgeLowPercentageData=*/ false); + /*purgeLowPercentageAndFakeData=*/ false); assertThat(resultMap).hasSize(3); // Verifies the first timestamp result. @@ -213,7 +234,7 @@ public final class ConvertUtilsTest { final Map> purgedResultMap = ConvertUtils.getIndexedUsageMap( mContext, timeSlotSize, batteryHistoryKeys, batteryHistoryMap, - /*purgeLowPercentageData=*/ true); + /*purgeLowPercentageAndFakeData=*/ true); assertThat(purgedResultMap).hasSize(3); // Verifies the first timestamp result. @@ -225,8 +246,49 @@ public final class ConvertUtilsTest { assertBatteryDiffEntry(entryList.get(0), 75, 40L, 50L); // Verifies the last 24 hours aggregate result. entryList = purgedResultMap.get(Integer.valueOf(-1)); + assertThat(entryList).hasSize(2); + // Verifies the fake data is cleared out. + assertThat(entryList.get(0).getPackageName()) + .isNotEqualTo(ConvertUtils.FAKE_PACKAGE_NAME); + assertThat(entryList.get(1).getPackageName()) + .isNotEqualTo(ConvertUtils.FAKE_PACKAGE_NAME); + } + + @Test + public void testGetIndexedUsageMap_usageTimeExceed_returnsExpectedResult() { + final int timeSlotSize = 1; + final long[] batteryHistoryKeys = new long[] {101L, 102L, 103L}; + final Map> batteryHistoryMap = + new HashMap<>(); + final BatteryHistEntry fakeEntry = createBatteryHistEntry( + ConvertUtils.FAKE_PACKAGE_NAME, "fake_label", 0, 0L, 0L, 0L); + // Adds the index = 0 data. + Map entryMap = new HashMap<>(); + entryMap.put(fakeEntry.getKey(), fakeEntry); + batteryHistoryMap.put(Long.valueOf(batteryHistoryKeys[0]), entryMap); + // Adds the index = 1 data. + entryMap = new HashMap<>(); + entryMap.put(fakeEntry.getKey(), fakeEntry); + batteryHistoryMap.put(Long.valueOf(batteryHistoryKeys[1]), entryMap); + // Adds the index = 2 data. + entryMap = new HashMap<>(); + final BatteryHistEntry entry = createBatteryHistEntry( + "package3", "label3", 500, 5L, 3600000L, 7200000L); + entryMap.put(entry.getKey(), entry); + batteryHistoryMap.put(Long.valueOf(batteryHistoryKeys[2]), entryMap); + + final Map> purgedResultMap = + ConvertUtils.getIndexedUsageMap( + mContext, timeSlotSize, batteryHistoryKeys, batteryHistoryMap, + /*purgeLowPercentageAndFakeData=*/ true); + + assertThat(purgedResultMap).hasSize(2); + final List entryList = purgedResultMap.get(0); assertThat(entryList).hasSize(1); - assertBatteryDiffEntry(entryList.get(0), 68, 40L, 50L); + // Verifies the clipped usage time. + final BatteryDiffEntry resultEntry = entryList.get(0); + assertThat(resultEntry.mForegroundUsageTimeInMs).isEqualTo(2400000); + assertThat(resultEntry.mBackgroundUsageTimeInMs).isEqualTo(4800000); } @Test