From 9edd6be5424a7104d841703f929821a5a77d6c04 Mon Sep 17 00:00:00 2001 From: Adam Lesinski Date: Fri, 12 Aug 2016 15:49:44 -0700 Subject: SamplingTimer: Fix issue with summary recording too much The summary is supposed to just hold enough data to continue counting once the device has reset. Since kernel stats reset when the device resets, and the first update is ignored to account for soft resets where the kernel continues running, SamplingTimer should not be recording the last value it saw from /proc/wakelocks in the summary. Bug:30575302 Change-Id: Ic193bc5af9a0ede514e3abc8146523d7316c47d3 --- .../com/android/internal/os/BatteryStatsImpl.java | 18 +--------- .../internal/os/BatteryStatsSamplingTimerTest.java | 39 +++++++++++++++++----- 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/core/java/com/android/internal/os/BatteryStatsImpl.java b/core/java/com/android/internal/os/BatteryStatsImpl.java index 7fb92aef30bd..2538d60f1924 100644 --- a/core/java/com/android/internal/os/BatteryStatsImpl.java +++ b/core/java/com/android/internal/os/BatteryStatsImpl.java @@ -108,7 +108,7 @@ public class BatteryStatsImpl extends BatteryStats { private static final int MAGIC = 0xBA757475; // 'BATSTATS' // Current on-disk Parcel version - private static final int VERSION = 149 + (USE_OLD_HISTORY ? 1000 : 0); + private static final int VERSION = 150 + (USE_OLD_HISTORY ? 1000 : 0); // Maximum number of items we will record in the history. private static final int MAX_HISTORY_ITEMS = 2000; @@ -1415,22 +1415,6 @@ public class BatteryStatsImpl extends BatteryStats { mUnpluggedReportedCount = 0; return true; } - - @Override - public void writeSummaryFromParcelLocked(Parcel out, long batteryRealtime) { - super.writeSummaryFromParcelLocked(out, batteryRealtime); - out.writeLong(mCurrentReportedTotalTime); - out.writeInt(mCurrentReportedCount); - out.writeInt(mTrackingReportedValues ? 1 : 0); - } - - @Override - public void readSummaryFromParcelLocked(Parcel in) { - super.readSummaryFromParcelLocked(in); - mUnpluggedReportedTotalTime = mCurrentReportedTotalTime = in.readLong(); - mUnpluggedReportedCount = mCurrentReportedCount = in.readInt(); - mTrackingReportedValues = in.readInt() == 1; - } } /** diff --git a/core/tests/coretests/src/com/android/internal/os/BatteryStatsSamplingTimerTest.java b/core/tests/coretests/src/com/android/internal/os/BatteryStatsSamplingTimerTest.java index ce6879d0ef4e..b4afddab97c9 100644 --- a/core/tests/coretests/src/com/android/internal/os/BatteryStatsSamplingTimerTest.java +++ b/core/tests/coretests/src/com/android/internal/os/BatteryStatsSamplingTimerTest.java @@ -178,19 +178,40 @@ public class BatteryStatsSamplingTimerTest extends TestCase { clocks.elapsedRealtime() * 1000); offBatterySummaryParcel.setDataPosition(0); + // Set the timebase running again. That way any issues with tracking reported values + // get tested when we unparcel the timers below. + timeBase.setRunning(true, clocks.uptimeMillis(), clocks.elapsedRealtime()); + // Read the on battery summary from the parcel. - BatteryStatsImpl.SamplingTimer unparceledTimer = new BatteryStatsImpl.SamplingTimer( - clocks, timeBase); - unparceledTimer.readSummaryFromParcelLocked(onBatterySummaryParcel); + BatteryStatsImpl.SamplingTimer unparceledOnBatteryTimer = + new BatteryStatsImpl.SamplingTimer(clocks, timeBase); + unparceledOnBatteryTimer.readSummaryFromParcelLocked(onBatterySummaryParcel); - assertEquals(10, unparceledTimer.getTotalTimeLocked(0, BatteryStats.STATS_SINCE_CHARGED)); - assertEquals(1, unparceledTimer.getCountLocked(BatteryStats.STATS_SINCE_CHARGED)); + assertEquals(10, unparceledOnBatteryTimer.getTotalTimeLocked(0, + BatteryStats.STATS_SINCE_CHARGED)); + assertEquals(1, unparceledOnBatteryTimer.getCountLocked(BatteryStats.STATS_SINCE_CHARGED)); // Read the off battery summary from the parcel. - unparceledTimer = new BatteryStatsImpl.SamplingTimer(clocks, timeBase); - unparceledTimer.readSummaryFromParcelLocked(offBatterySummaryParcel); + BatteryStatsImpl.SamplingTimer unparceledOffBatteryTimer = + new BatteryStatsImpl.SamplingTimer(clocks, timeBase); + unparceledOffBatteryTimer.readSummaryFromParcelLocked(offBatterySummaryParcel); + + assertEquals(10, unparceledOffBatteryTimer.getTotalTimeLocked(0, + BatteryStats.STATS_SINCE_CHARGED)); + assertEquals(1, unparceledOffBatteryTimer.getCountLocked(BatteryStats.STATS_SINCE_CHARGED)); - assertEquals(10, unparceledTimer.getTotalTimeLocked(0, BatteryStats.STATS_SINCE_CHARGED)); - assertEquals(1, unparceledTimer.getCountLocked(BatteryStats.STATS_SINCE_CHARGED)); + // Now, just like with a fresh timer, the first update should be absorbed to account for + // data being collected when we weren't recording. + unparceledOnBatteryTimer.update(10, 10); + + assertEquals(10, unparceledOnBatteryTimer.getTotalTimeLocked(0, + BatteryStats.STATS_SINCE_CHARGED)); + assertEquals(1, unparceledOnBatteryTimer.getCountLocked(BatteryStats.STATS_SINCE_CHARGED)); + + unparceledOffBatteryTimer.update(10, 10); + + assertEquals(10, unparceledOffBatteryTimer.getTotalTimeLocked(0, + BatteryStats.STATS_SINCE_CHARGED)); + assertEquals(1, unparceledOffBatteryTimer.getCountLocked(BatteryStats.STATS_SINCE_CHARGED)); } } -- cgit v1.2.3-59-g8ed1b