Fixing issue in current and max duration calculations.
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:
@@ -182,7 +182,7 @@ public abstract class BatteryStats implements Parcelable {
|
|||||||
* New in version 19:
|
* New in version 19:
|
||||||
* - Wakelock data (wl) gets current and max times.
|
* - 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.
|
* Old version, we hit 9 and ran out of room, need to remove.
|
||||||
|
|||||||
@@ -108,7 +108,7 @@ public class BatteryStatsImpl extends BatteryStats {
|
|||||||
private static final int MAGIC = 0xBA757475; // 'BATSTATS'
|
private static final int MAGIC = 0xBA757475; // 'BATSTATS'
|
||||||
|
|
||||||
// Current on-disk Parcel version
|
// 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.
|
// Maximum number of items we will record in the history.
|
||||||
private static final int MAX_HISTORY_ITEMS = 2000;
|
private static final int MAX_HISTORY_ITEMS = 2000;
|
||||||
@@ -1593,7 +1593,7 @@ public class BatteryStatsImpl extends BatteryStats {
|
|||||||
@Override
|
@Override
|
||||||
public void writeToParcel(Parcel out, long elapsedRealtimeUs) {
|
public void writeToParcel(Parcel out, long elapsedRealtimeUs) {
|
||||||
super.writeToParcel(out, elapsedRealtimeUs);
|
super.writeToParcel(out, elapsedRealtimeUs);
|
||||||
out.writeLong(mMaxDurationMs);
|
out.writeLong(getMaxDurationMsLocked(elapsedRealtimeUs / 1000));
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -1606,7 +1606,7 @@ public class BatteryStatsImpl extends BatteryStats {
|
|||||||
@Override
|
@Override
|
||||||
public void writeSummaryFromParcelLocked(Parcel out, long elapsedRealtimeUs) {
|
public void writeSummaryFromParcelLocked(Parcel out, long elapsedRealtimeUs) {
|
||||||
super.writeSummaryFromParcelLocked(out, 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) {
|
public void onTimeStarted(long elapsedRealtimeUs, long baseUptime, long baseRealtime) {
|
||||||
super.onTimeStarted(elapsedRealtimeUs, baseUptime, baseRealtime);
|
super.onTimeStarted(elapsedRealtimeUs, baseUptime, baseRealtime);
|
||||||
if (mNesting > 0) {
|
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.
|
* If the timer is running, add the duration into mCurrentDurationMs.
|
||||||
*/
|
*/
|
||||||
@Override
|
@Override
|
||||||
public void onTimeStopped(long elapsedRealtimeUs, long baseUptime, long baseRealtime) {
|
public void onTimeStopped(long elapsedRealtimeUs, long baseUptime, long baseRealtimeUs) {
|
||||||
super.onTimeStopped(elapsedRealtimeUs, baseUptime, baseRealtime);
|
super.onTimeStopped(elapsedRealtimeUs, baseUptime, baseRealtimeUs);
|
||||||
if (mNesting > 0) {
|
if (mNesting > 0) {
|
||||||
mCurrentDurationMs += (elapsedRealtimeUs / 1000) - mStartTimeMs;
|
// baseRealtimeUs has already been converted to the timebase's realtime.
|
||||||
|
mCurrentDurationMs += (baseRealtimeUs / 1000) - mStartTimeMs;
|
||||||
}
|
}
|
||||||
mStartTimeMs = -1;
|
mStartTimeMs = -1;
|
||||||
}
|
}
|
||||||
@@ -1658,7 +1659,7 @@ public class BatteryStatsImpl extends BatteryStats {
|
|||||||
super.startRunningLocked(elapsedRealtimeMs);
|
super.startRunningLocked(elapsedRealtimeMs);
|
||||||
if (mNesting == 1 && mTimeBase.isRunning()) {
|
if (mNesting == 1 && mTimeBase.isRunning()) {
|
||||||
// Just started
|
// 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
|
@Override
|
||||||
public void stopRunningLocked(long elapsedRealtimeMs) {
|
public void stopRunningLocked(long elapsedRealtimeMs) {
|
||||||
super.stopRunningLocked(elapsedRealtimeMs);
|
if (mNesting == 1) {
|
||||||
if (mNesting == 0) {
|
|
||||||
final long durationMs = getCurrentDurationMsLocked(elapsedRealtimeMs);
|
final long durationMs = getCurrentDurationMsLocked(elapsedRealtimeMs);
|
||||||
if (durationMs > mMaxDurationMs) {
|
if (durationMs > mMaxDurationMs) {
|
||||||
mMaxDurationMs = durationMs;
|
mMaxDurationMs = durationMs;
|
||||||
@@ -1679,6 +1679,9 @@ public class BatteryStatsImpl extends BatteryStats {
|
|||||||
mStartTimeMs = -1;
|
mStartTimeMs = -1;
|
||||||
mCurrentDurationMs = 0;
|
mCurrentDurationMs = 0;
|
||||||
}
|
}
|
||||||
|
// super method decrements mNesting, which getCurrentDurationMsLocked relies on,
|
||||||
|
// so call super.stopRunningLocked after calling getCurrentDurationMsLocked.
|
||||||
|
super.stopRunningLocked(elapsedRealtimeMs);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
@@ -1720,11 +1723,9 @@ public class BatteryStatsImpl extends BatteryStats {
|
|||||||
@Override
|
@Override
|
||||||
public long getCurrentDurationMsLocked(long elapsedRealtimeMs) {
|
public long getCurrentDurationMsLocked(long elapsedRealtimeMs) {
|
||||||
long durationMs = mCurrentDurationMs;
|
long durationMs = mCurrentDurationMs;
|
||||||
if (mNesting > 0) {
|
if (mNesting > 0 && mTimeBase.isRunning()) {
|
||||||
if (mTimeBase.isRunning()) {
|
durationMs += (mTimeBase.getRealtime(elapsedRealtimeMs*1000)/1000)
|
||||||
durationMs += (mTimeBase.getRealtime(elapsedRealtimeMs*1000)/1000)
|
- mStartTimeMs;
|
||||||
- mStartTimeMs;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
return durationMs;
|
return durationMs;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -18,12 +18,9 @@ package com.android.internal.os;
|
|||||||
import android.os.BatteryStats;
|
import android.os.BatteryStats;
|
||||||
import android.os.Parcel;
|
import android.os.Parcel;
|
||||||
import android.support.test.filters.SmallTest;
|
import android.support.test.filters.SmallTest;
|
||||||
import android.util.Log;
|
|
||||||
|
|
||||||
import junit.framework.TestCase;
|
import junit.framework.TestCase;
|
||||||
|
|
||||||
import org.mockito.Mockito;
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Test BatteryStatsImpl.DurationTimer.
|
* Test BatteryStatsImpl.DurationTimer.
|
||||||
*
|
*
|
||||||
@@ -82,15 +79,15 @@ public class BatteryStatsDurationTimerTest extends TestCase {
|
|||||||
// Stop the TimeBase. The values should be frozen.
|
// Stop the TimeBase. The values should be frozen.
|
||||||
timeBase.setRunning(false, /* uptimeUs */ 10, /* realtimeUs */ 55000*1000);
|
timeBase.setRunning(false, /* uptimeUs */ 10, /* realtimeUs */ 55000*1000);
|
||||||
assertTrue(timer.isRunningLocked());
|
assertTrue(timer.isRunningLocked());
|
||||||
assertEquals(28100, timer.getCurrentDurationMsLocked(110100)); // Why 28100 and not 28000?
|
assertEquals(28000, timer.getCurrentDurationMsLocked(110100));
|
||||||
assertEquals(28100, timer.getMaxDurationMsLocked(110101));
|
assertEquals(28000, timer.getMaxDurationMsLocked(110101));
|
||||||
|
|
||||||
// Start the TimeBase. The values should be the old value plus the delta
|
// Start the TimeBase. The values should be the old value plus the delta
|
||||||
// between when the timer restarted and the current time
|
// between when the timer restarted and the current time
|
||||||
timeBase.setRunning(true, /* uptimeUs */ 10, /* realtimeUs */ 220100*1000);
|
timeBase.setRunning(true, /* uptimeUs */ 10, /* realtimeUs */ 220100*1000);
|
||||||
assertTrue(timer.isRunningLocked());
|
assertTrue(timer.isRunningLocked());
|
||||||
assertEquals(28300, timer.getCurrentDurationMsLocked(220300)); // extra 100 from above??
|
assertEquals(28200, timer.getCurrentDurationMsLocked(220300));
|
||||||
assertEquals(28301, timer.getMaxDurationMsLocked(220301));
|
assertEquals(28201, timer.getMaxDurationMsLocked(220301));
|
||||||
}
|
}
|
||||||
|
|
||||||
@SmallTest
|
@SmallTest
|
||||||
|
|||||||
Reference in New Issue
Block a user