Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fixing issue in current and max duration calculations.

Browse Source 
It looks like one operation was done out of order and some of the times used in
the calculations were leading to incorrect results.

BUG: 31023263
Test: bit FrameworksCoreTests:com.android.internal.os.BatteryStatsDurationTimerTest

Change-Id: I417cc28c5a55748067b6c7f682a66fe3dbc09f09
(cherry picked from commit 47db5a8bf7)
This commit is contained in:
Kweku Adams
2016-12-09 19:04:50 -08:00
parent 7af83d0e5a
commit c6bd4243a4
3 changed files with 26 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
core/java/android/os/BatteryStats.java
View File

@@ -182,7 +182,7 @@ public abstract class BatteryStats implements Parcelable {
* New in version 19:
* - Wakelock data (wl) gets current and max times.
*/
static final String CHECKIN_VERSION = "19";
static final String CHECKIN_VERSION = "20";
/**
* Old version, we hit 9 and ran out of room, need to remove.

31
core/java/com/android/internal/os/BatteryStatsImpl.java
View File

@@ -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 = 150 + (USE_OLD_HISTORY ? 1000 : 0);
private static final int VERSION = 151 + (USE_OLD_HISTORY ? 1000 : 0);
// Maximum number of items we will record in the history.
private static final int MAX_HISTORY_ITEMS = 2000;
@@ -1593,7 +1593,7 @@ public class BatteryStatsImpl extends BatteryStats {
@Override
public void writeToParcel(Parcel out, long elapsedRealtimeUs) {
super.writeToParcel(out, elapsedRealtimeUs);
out.writeLong(mMaxDurationMs);
out.writeLong(getMaxDurationMsLocked(elapsedRealtimeUs / 1000));
}
/**
@@ -1606,7 +1606,7 @@ public class BatteryStatsImpl extends BatteryStats {
@Override
public void writeSummaryFromParcelLocked(Parcel out, long elapsedRealtimeUs) {
super.writeSummaryFromParcelLocked(out, elapsedRealtimeUs);
out.writeLong(mMaxDurationMs);
out.writeLong(getMaxDurationMsLocked(elapsedRealtimeUs / 1000));
}
/**
@@ -1630,7 +1630,7 @@ public class BatteryStatsImpl extends BatteryStats {
public void onTimeStarted(long elapsedRealtimeUs, long baseUptime, long baseRealtime) {
super.onTimeStarted(elapsedRealtimeUs, baseUptime, baseRealtime);
if (mNesting > 0) {
mStartTimeMs = mTimeBase.getRealtime(mClocks.elapsedRealtime()*1000) / 1000;
mStartTimeMs = baseRealtime / 1000;
}
}
@@ -1640,10 +1640,11 @@ public class BatteryStatsImpl extends BatteryStats {
* If the timer is running, add the duration into mCurrentDurationMs.
*/
@Override
public void onTimeStopped(long elapsedRealtimeUs, long baseUptime, long baseRealtime) {
super.onTimeStopped(elapsedRealtimeUs, baseUptime, baseRealtime);
public void onTimeStopped(long elapsedRealtimeUs, long baseUptime, long baseRealtimeUs) {
super.onTimeStopped(elapsedRealtimeUs, baseUptime, baseRealtimeUs);
if (mNesting > 0) {
mCurrentDurationMs += (elapsedRealtimeUs / 1000) - mStartTimeMs;
// baseRealtimeUs has already been converted to the timebase's realtime.
mCurrentDurationMs += (baseRealtimeUs / 1000) - mStartTimeMs;
}
mStartTimeMs = -1;
}
@@ -1658,7 +1659,7 @@ public class BatteryStatsImpl extends BatteryStats {
super.startRunningLocked(elapsedRealtimeMs);
if (mNesting == 1 && mTimeBase.isRunning()) {
// Just started
mStartTimeMs = mTimeBase.getRealtime(mClocks.elapsedRealtime()*1000) / 1000;
mStartTimeMs = mTimeBase.getRealtime(elapsedRealtimeMs * 1000) / 1000;
}
}
@@ -1670,8 +1671,7 @@ public class BatteryStatsImpl extends BatteryStats {
*/
@Override
public void stopRunningLocked(long elapsedRealtimeMs) {
super.stopRunningLocked(elapsedRealtimeMs);
if (mNesting == 0) {
if (mNesting == 1) {
final long durationMs = getCurrentDurationMsLocked(elapsedRealtimeMs);
if (durationMs > mMaxDurationMs) {
mMaxDurationMs = durationMs;
@@ -1679,6 +1679,9 @@ public class BatteryStatsImpl extends BatteryStats {
mStartTimeMs = -1;
mCurrentDurationMs = 0;
}
// super method decrements mNesting, which getCurrentDurationMsLocked relies on,
// so call super.stopRunningLocked after calling getCurrentDurationMsLocked.
super.stopRunningLocked(elapsedRealtimeMs);
}
@Override
@@ -1720,11 +1723,9 @@ public class BatteryStatsImpl extends BatteryStats {
@Override
public long getCurrentDurationMsLocked(long elapsedRealtimeMs) {
long durationMs = mCurrentDurationMs;
if (mNesting > 0) {
if (mTimeBase.isRunning()) {
durationMs += (mTimeBase.getRealtime(elapsedRealtimeMs*1000)/1000)
- mStartTimeMs;
}
if (mNesting > 0 && mTimeBase.isRunning()) {
durationMs += (mTimeBase.getRealtime(elapsedRealtimeMs*1000)/1000)
- mStartTimeMs;
}
return durationMs;
}

21
core/tests/coretests/src/com/android/internal/os/BatteryStatsDurationTimerTest.java
View File

@@ -18,12 +18,9 @@ package com.android.internal.os;
import android.os.BatteryStats;
import android.os.Parcel;
import android.support.test.filters.SmallTest;
import android.util.Log;
import junit.framework.TestCase;
import org.mockito.Mockito;
/**
* Test BatteryStatsImpl.DurationTimer.
*
@@ -39,7 +36,7 @@ public class BatteryStatsDurationTimerTest extends TestCase {
final BatteryStatsImpl.TimeBase timeBase = new BatteryStatsImpl.TimeBase();
timeBase.init(clocks.uptimeMillis(), clocks.elapsedRealtime());
final BatteryStatsImpl.DurationTimer timer = new BatteryStatsImpl.DurationTimer(clocks,
final BatteryStatsImpl.DurationTimer timer = new BatteryStatsImpl.DurationTimer(clocks,
null, BatteryStats.WAKE_TYPE_PARTIAL, null, timeBase);
// TimeBase running, timer not running: current and max are 0
@@ -82,15 +79,15 @@ public class BatteryStatsDurationTimerTest extends TestCase {
// Stop the TimeBase. The values should be frozen.
timeBase.setRunning(false, /* uptimeUs */ 10, /* realtimeUs */ 55000*1000);
assertTrue(timer.isRunningLocked());
assertEquals(28100, timer.getCurrentDurationMsLocked(110100)); // Why 28100 and not 28000?
assertEquals(28100, timer.getMaxDurationMsLocked(110101));
assertEquals(28000, timer.getCurrentDurationMsLocked(110100));
assertEquals(28000, timer.getMaxDurationMsLocked(110101));
// Start the TimeBase. The values should be the old value plus the delta
// between when the timer restarted and the current time
timeBase.setRunning(true, /* uptimeUs */ 10, /* realtimeUs */ 220100*1000);
assertTrue(timer.isRunningLocked());
assertEquals(28300, timer.getCurrentDurationMsLocked(220300)); // extra 100 from above??
assertEquals(28301, timer.getMaxDurationMsLocked(220301));
assertEquals(28200, timer.getCurrentDurationMsLocked(220300));
assertEquals(28201, timer.getMaxDurationMsLocked(220301));
}
@SmallTest
@@ -104,7 +101,7 @@ public class BatteryStatsDurationTimerTest extends TestCase {
final BatteryStatsImpl.TimeBase timeBase = new BatteryStatsImpl.TimeBase();
timeBase.init(clocks.uptimeMillis(), clocks.elapsedRealtime());
final BatteryStatsImpl.DurationTimer timer = new BatteryStatsImpl.DurationTimer(clocks,
final BatteryStatsImpl.DurationTimer timer = new BatteryStatsImpl.DurationTimer(clocks,
null, BatteryStats.WAKE_TYPE_PARTIAL, null, timeBase);
// Start running on battery.
@@ -124,7 +121,7 @@ public class BatteryStatsDurationTimerTest extends TestCase {
summaryParcel.setDataPosition(0);
// Read summary
final BatteryStatsImpl.DurationTimer summary = new BatteryStatsImpl.DurationTimer(clocks,
final BatteryStatsImpl.DurationTimer summary = new BatteryStatsImpl.DurationTimer(clocks,
null, BatteryStats.WAKE_TYPE_PARTIAL, null, timeBase);
summary.startRunningLocked(3100);
summary.readSummaryFromParcelLocked(summaryParcel);
@@ -138,9 +135,9 @@ public class BatteryStatsDurationTimerTest extends TestCase {
final Parcel fullParcel = Parcel.obtain();
timer.writeToParcel(fullParcel, 1500*1000);
fullParcel.setDataPosition(0);
// Read full - Should be the same as the summary as far as DurationTimer is concerned.
final BatteryStatsImpl.DurationTimer full = new BatteryStatsImpl.DurationTimer(clocks,
final BatteryStatsImpl.DurationTimer full = new BatteryStatsImpl.DurationTimer(clocks,
null, BatteryStats.WAKE_TYPE_PARTIAL, null, timeBase, fullParcel);
// The new one shouldn't be running, and therefore 0 for current time
assertFalse(full.isRunningLocked());