diff options
4 files changed, 212 insertions, 30 deletions
diff --git a/core/java/com/android/internal/os/BatteryStatsHistory.java b/core/java/com/android/internal/os/BatteryStatsHistory.java index 036faef7aa65..85794d43df3d 100644 --- a/core/java/com/android/internal/os/BatteryStatsHistory.java +++ b/core/java/com/android/internal/os/BatteryStatsHistory.java @@ -77,7 +77,7 @@ public class BatteryStatsHistory { private static final String TAG = "BatteryStatsHistory"; // Current on-disk Parcel version. Must be updated when the format of the parcelable changes - private static final int VERSION = 212; + private static final int VERSION = 213; // Part of initial delta int that specifies the time delta. static final int DELTA_TIME_MASK = 0x7ffff; @@ -109,10 +109,27 @@ public class BatteryStatsHistory { static final int STATE_BATTERY_PLUG_MASK = 0x00000003; static final int STATE_BATTERY_PLUG_SHIFT = 24; + // Pieces of data that are packed into the battery level int + static final int BATTERY_LEVEL_LEVEL_MASK = 0xFF000000; + static final int BATTERY_LEVEL_LEVEL_SHIFT = 24; + static final int BATTERY_LEVEL_TEMP_MASK = 0x00FF8000; + static final int BATTERY_LEVEL_TEMP_SHIFT = 15; + static final int BATTERY_LEVEL_VOLT_MASK = 0x00007FFC; + static final int BATTERY_LEVEL_VOLT_SHIFT = 2; + // Flag indicating that the voltage and temperature deltas are too large to + // store in the battery level int and full volt/temp values are instead + // stored in a following int. + static final int BATTERY_LEVEL_OVERFLOW_FLAG = 0x00000002; // We use the low bit of the battery state int to indicate that we have full details // from a battery level change. static final int BATTERY_LEVEL_DETAILS_FLAG = 0x00000001; + // Pieces of data that are packed into the extended battery level int + static final int BATTERY_LEVEL2_TEMP_MASK = 0xFFFF0000; + static final int BATTERY_LEVEL2_TEMP_SHIFT = 16; + static final int BATTERY_LEVEL2_VOLT_MASK = 0x0000FFFF; + static final int BATTERY_LEVEL2_VOLT_SHIFT = 0; + // Flag in history tag index: indicates that this is the first occurrence of this tag, // therefore the tag value is written in the parcel static final int TAG_FIRST_OCCURRENCE_FLAG = 0x8000; @@ -1808,12 +1825,22 @@ public class BatteryStatsHistory { Battery level int: if A in the first token is set, 31 23 15 7 0 - █L|L|L|L|L|L|L|T█T|T|T|T|T|T|T|T█T|V|V|V|V|V|V|V█V|V|V|V|V|V|V|D█ + █L|L|L|L|L|L|L|L█T|T|T|T|T|T|T|T█T|V|V|V|V|V|V|V█V|V|V|V|V|V|E|D█ D: indicates that extra history details follow. - V: the battery voltage. - T: the battery temperature. - L: the battery level (out of 100). + E: indicates that the voltage delta or temperature delta is too large to fit in the + respective V or T field of this int. If this flag is set, an extended battery level + int containing the complete voltage and temperature values immediately follows. + V: the signed battery voltage delta in millivolts. + T: the signed battery temperature delta in tenths of a degree Celsius. + L: the signed battery level delta (out of 100). + + Extended battery level int: if E in the battery level int is set, + 31 23 15 7 0 + █T|T|T|T|T|T|T|T█T|T|T|T|T|T|T|T█V|V|V|V|V|V|V|V█V|V|V|V|V|V|V|V█ + + V: the current battery voltage (complete 16-bit value, not a delta). + T: the current battery temperature (complete 16-bit value, not a delta). State change int: if B in the first token is set, 31 23 15 7 0 @@ -1869,7 +1896,7 @@ public class BatteryStatsHistory { int extensionFlags = 0; final long deltaTime = cur.time - last.time; - final int lastBatteryLevelInt = buildBatteryLevelInt(last); + int batteryLevelInt = buildBatteryLevelInt(cur, last); final int lastStateInt = buildStateInt(last); int deltaTimeToken; @@ -1881,7 +1908,6 @@ public class BatteryStatsHistory { deltaTimeToken = (int) deltaTime; } int firstToken = deltaTimeToken | (cur.states & BatteryStatsHistory.DELTA_STATE_MASK); - int batteryLevelInt = buildBatteryLevelInt(cur); if (cur.batteryLevel < mLastHistoryStepLevel || mLastHistoryStepLevel == 0) { cur.stepDetails = mStepDetailsCalculator.getHistoryStepDetails(); @@ -1894,7 +1920,7 @@ public class BatteryStatsHistory { mLastHistoryStepLevel = cur.batteryLevel; } - final boolean batteryLevelIntChanged = batteryLevelInt != lastBatteryLevelInt; + final boolean batteryLevelIntChanged = batteryLevelInt != 0; if (batteryLevelIntChanged) { firstToken |= BatteryStatsHistory.DELTA_BATTERY_LEVEL_FLAG; } @@ -1948,10 +1974,21 @@ public class BatteryStatsHistory { } } if (batteryLevelIntChanged) { + boolean overflow = (batteryLevelInt & BATTERY_LEVEL_OVERFLOW_FLAG) != 0; + int extendedBatteryLevelInt = 0; + dest.writeInt(batteryLevelInt); + if (overflow) { + extendedBatteryLevelInt = buildExtendedBatteryLevelInt(cur); + dest.writeInt(extendedBatteryLevelInt); + } + if (DEBUG) { Slog.i(TAG, "WRITE DELTA: batteryToken=0x" + Integer.toHexString(batteryLevelInt) + + (overflow + ? " batteryToken2=0x" + Integer.toHexString(extendedBatteryLevelInt) + : "") + " batteryLevel=" + cur.batteryLevel + " batteryTemp=" + cur.batteryTemperature + " batteryVolt=" + (int) cur.batteryVoltage); @@ -2049,16 +2086,43 @@ public class BatteryStatsHistory { } } - private int buildBatteryLevelInt(HistoryItem h) { - int bits = 0; - bits = setBitField(bits, h.batteryLevel, 25, 0xfe000000 /* 7F << 25 */); - bits = setBitField(bits, h.batteryTemperature, 15, 0x01ff8000 /* 3FF << 15 */); - short voltage = (short) h.batteryVoltage; - if (voltage == -1) { - voltage = 0x3FFF; + private boolean signedValueFits(int value, int mask, int shift) { + mask >>>= shift; + // The original value can only be restored if all of the lost + // high-order bits match the MSB of the packed value. Extract both the + // MSB and the lost bits, and check if they match (i.e. they are all + // zeros or all ones). + int msbAndLostBitsMask = ~(mask >>> 1); + int msbAndLostBits = value & msbAndLostBitsMask; + + return msbAndLostBits == 0 || msbAndLostBits == msbAndLostBitsMask; + } + + private int buildBatteryLevelInt(HistoryItem cur, HistoryItem prev) { + final int levelDelta = (int) cur.batteryLevel - (int) prev.batteryLevel; + final int tempDelta = (int) cur.batteryTemperature - (int) prev.batteryTemperature; + final int voltDelta = (int) cur.batteryVoltage - (int) prev.batteryVoltage; + final boolean overflow = + !signedValueFits(tempDelta, BATTERY_LEVEL_TEMP_MASK, BATTERY_LEVEL_VOLT_SHIFT) + || !signedValueFits(voltDelta, BATTERY_LEVEL_VOLT_MASK, BATTERY_LEVEL_TEMP_SHIFT); + + int batt = 0; + batt |= (levelDelta << BATTERY_LEVEL_LEVEL_SHIFT) & BATTERY_LEVEL_LEVEL_MASK; + if (overflow) { + batt |= BATTERY_LEVEL_OVERFLOW_FLAG; + } else { + batt |= (tempDelta << BATTERY_LEVEL_TEMP_SHIFT) & BATTERY_LEVEL_TEMP_MASK; + batt |= (voltDelta << BATTERY_LEVEL_VOLT_SHIFT) & BATTERY_LEVEL_VOLT_MASK; } - bits = setBitField(bits, voltage, 1, 0x00007ffe /* 3FFF << 1 */); - return bits; + + return batt; + } + + private int buildExtendedBatteryLevelInt(HistoryItem cur) { + int battExt = 0; + battExt |= (cur.batteryTemperature << BATTERY_LEVEL2_TEMP_SHIFT) & BATTERY_LEVEL2_TEMP_MASK; + battExt |= (cur.batteryVoltage << BATTERY_LEVEL2_VOLT_SHIFT) & BATTERY_LEVEL2_VOLT_MASK; + return battExt; } private int buildStateInt(HistoryItem h) { diff --git a/core/java/com/android/internal/os/BatteryStatsHistoryIterator.java b/core/java/com/android/internal/os/BatteryStatsHistoryIterator.java index ee897cd37cd3..0d5d8761d00d 100644 --- a/core/java/com/android/internal/os/BatteryStatsHistoryIterator.java +++ b/core/java/com/android/internal/os/BatteryStatsHistoryIterator.java @@ -150,11 +150,21 @@ public class BatteryStatsHistoryIterator implements Iterator<BatteryStats.Histor final int batteryLevelInt; if ((firstToken & BatteryStatsHistory.DELTA_BATTERY_LEVEL_FLAG) != 0) { batteryLevelInt = src.readInt(); - readBatteryLevelInt(batteryLevelInt, cur); cur.numReadInts += 1; + final boolean overflow = + (batteryLevelInt & BatteryStatsHistory.BATTERY_LEVEL_OVERFLOW_FLAG) != 0; + int extendedBatteryLevelInt = 0; + if (overflow) { + extendedBatteryLevelInt = src.readInt(); + cur.numReadInts += 1; + } + readBatteryLevelInts(batteryLevelInt, extendedBatteryLevelInt, cur); if (DEBUG) { Slog.i(TAG, "READ DELTA: batteryToken=0x" + Integer.toHexString(batteryLevelInt) + + (overflow + ? " batteryToken2=0x" + Integer.toHexString(extendedBatteryLevelInt) + : "") + " batteryLevel=" + cur.batteryLevel + " batteryTemp=" + cur.batteryTemperature + " batteryVolt=" + (int) cur.batteryVoltage); @@ -312,15 +322,43 @@ public class BatteryStatsHistoryIterator implements Iterator<BatteryStats.Histor return true; } - private static void readBatteryLevelInt(int batteryLevelInt, BatteryStats.HistoryItem out) { - out.batteryLevel = (byte) ((batteryLevelInt & 0xfe000000) >>> 25); - out.batteryTemperature = (short) ((batteryLevelInt & 0x01ff8000) >>> 15); - int voltage = ((batteryLevelInt & 0x00007ffe) >>> 1); - if (voltage == 0x3FFF) { - voltage = -1; - } + private static int extractSignedBitField(int bits, int mask, int shift) { + mask >>>= shift; + bits >>>= shift; + int value = bits & mask; + int msbMask = mask ^ (mask >>> 1); + // Sign extend with MSB + if ((value & msbMask) != 0) value |= ~mask; + return value; + } + + private static void readBatteryLevelInts(int batteryInt, int extendedBatteryInt, + BatteryStats.HistoryItem out) { - out.batteryVoltage = (short) voltage; + out.batteryLevel += extractSignedBitField( + batteryInt, + BatteryStatsHistory.BATTERY_LEVEL_LEVEL_MASK, + BatteryStatsHistory.BATTERY_LEVEL_LEVEL_SHIFT); + + if ((batteryInt & BatteryStatsHistory.BATTERY_LEVEL_OVERFLOW_FLAG) == 0) { + out.batteryTemperature += extractSignedBitField( + batteryInt, + BatteryStatsHistory.BATTERY_LEVEL_TEMP_MASK, + BatteryStatsHistory.BATTERY_LEVEL_TEMP_SHIFT); + out.batteryVoltage += extractSignedBitField( + batteryInt, + BatteryStatsHistory.BATTERY_LEVEL_VOLT_MASK, + BatteryStatsHistory.BATTERY_LEVEL_VOLT_SHIFT); + } else { + out.batteryTemperature = (short) extractSignedBitField( + extendedBatteryInt, + BatteryStatsHistory.BATTERY_LEVEL2_TEMP_MASK, + BatteryStatsHistory.BATTERY_LEVEL2_TEMP_SHIFT); + out.batteryVoltage = (short) extractSignedBitField( + extendedBatteryInt, + BatteryStatsHistory.BATTERY_LEVEL2_VOLT_MASK, + BatteryStatsHistory.BATTERY_LEVEL2_VOLT_SHIFT); + } } /** diff --git a/services/core/java/com/android/server/power/stats/BatteryStatsImpl.java b/services/core/java/com/android/server/power/stats/BatteryStatsImpl.java index 90bc54b06c0a..1d62087428d8 100644 --- a/services/core/java/com/android/server/power/stats/BatteryStatsImpl.java +++ b/services/core/java/com/android/server/power/stats/BatteryStatsImpl.java @@ -15276,13 +15276,10 @@ public class BatteryStatsImpl extends BatteryStats { @GuardedBy("this") public void setBatteryStateLocked(final int status, final int health, final int plugType, - final int level, /* not final */ int temp, final int voltageMv, final int chargeUah, + final int level, final int temp, final int voltageMv, final int chargeUah, final int chargeFullUah, final long chargeTimeToFullSeconds, final long elapsedRealtimeMs, final long uptimeMs, final long currentTimeMs) { - // Temperature is encoded without the signed bit, so clamp any negative temperatures to 0. - temp = Math.max(0, temp); - reportChangesToStatsLog(status, plugType, level); final boolean onBattery = isOnBattery(plugType, status); diff --git a/services/tests/powerstatstests/src/com/android/server/power/stats/BatteryStatsHistoryIteratorTest.java b/services/tests/powerstatstests/src/com/android/server/power/stats/BatteryStatsHistoryIteratorTest.java index d6f50368df84..7e40e6b4f71f 100644 --- a/services/tests/powerstatstests/src/com/android/server/power/stats/BatteryStatsHistoryIteratorTest.java +++ b/services/tests/powerstatstests/src/com/android/server/power/stats/BatteryStatsHistoryIteratorTest.java @@ -17,6 +17,7 @@ package com.android.server.power.stats; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth.assertWithMessage; import android.os.BatteryManager; import android.os.BatteryStats; @@ -301,6 +302,88 @@ public class BatteryStatsHistoryIteratorTest { assertThat(item = iterator.next()).isNull(); } + @Test + public void batteryLevelAccuracy() { + // Check that battery values are recorded correctly throughout their valid ranges: + // - Battery level range: [0, 100] + // - Voltage and temperature range: [-2**15, 2**15-1] (note: exceeds physical requirements) + + int i16Min = Short.MIN_VALUE; + int i16Max = Short.MAX_VALUE; + int[][] testValues = { + // { level, temperature, voltage } + + // Min/max values + { 0, 0, 0 }, + { 0, i16Min, i16Min }, + { 100, i16Max, i16Max }, + + // Level changes + { 0, 0, 0 }, + { 12, 0, 0 }, + { 90, 0, 0 }, + { 34, 0, 0 }, + { 78, 0, 0 }, + { 56, 0, 0 }, + + // Temperature changes + { 0, 0, 0 }, + { 0, i16Max, 0 }, // Large change to max + { 0, i16Max - 12, 0 }, // Small change near max + { 0, i16Max - 56, 0 }, // Small change near max + { 0, i16Max - 34, 0 }, // Small change near max + { 0, 0, 0 }, // Large change to 0 + { 0, 12, 0 }, // Small change near 0 + { 0, -34, 0 }, // Small change near 0 + { 0, 56, 0 }, // Small change near 0 + { 0, -78, 0 }, // Small change near 0 + { 0, i16Min, 0 }, // Large change to min + { 0, i16Min + 12, 0 }, // Small change near min + { 0, i16Min + 56, 0 }, // Small change near min + { 0, i16Min + 34, 0 }, // Small change near min + + // Voltage changes + { 0, 0, 0 }, + { 0, 0, i16Max }, // Large change to max + { 0, 0, i16Max - 120 }, // Small change near max + { 0, 0, i16Max - 560 }, // Small change near max + { 0, 0, i16Max - 340 }, // Small change near max + { 0, 0, 0 }, // Large change to 0 + { 0, 0, 120 }, // Small change near 0 + { 0, 0, -340 }, // Small change near 0 + { 0, 0, 560 }, // Small change near 0 + { 0, 0, -780 }, // Small change near 0 + { 0, 0, i16Min }, // Large change to min + { 0, 0, i16Min + 120 }, // Small change near min + { 0, 0, i16Min + 560 }, // Small change near min + { 0, 0, i16Min + 340 }, // Small change near min + }; + + for (int[] val : testValues) { + mBatteryStats.setBatteryStateLocked( + BatteryManager.BATTERY_STATUS_DISCHARGING, + 100, 0, val[0], val[1], val[2], 1000_000, 1000_000, 0, 0, 0, 0); + } + + final BatteryStatsHistoryIterator iterator = + mBatteryStats.iterateBatteryStatsHistory(0, MonotonicClock.UNDEFINED); + + BatteryStats.HistoryItem item; + assertThat(item = iterator.next()).isNotNull(); + assertThat(item.cmd).isEqualTo((int) BatteryStats.HistoryItem.CMD_RESET); + + for (int i = 0; i < testValues.length; i++) { + int[] val = testValues[i]; + String msg = String.format("Incorrect battery value returned at index %d:", i); + assertThat(item = iterator.next()).isNotNull(); + assertWithMessage(msg).that(item.batteryLevel).isEqualTo(val[0]); + assertWithMessage(msg).that(item.batteryTemperature).isEqualTo(val[1]); + assertWithMessage(msg).that(item.batteryVoltage).isEqualTo(val[2]); + } + + assertThat(item = iterator.next()).isNull(); + } + private void assertHistoryItem(BatteryStats.HistoryItem item, int command, int eventCode, String tag, int uid, int voltageMv, int batteryChargeUah, int batteryLevel, long elapsedTimeMs) { |