From cb9c53dd7c6846655021d827beaaf76563c980b6 Mon Sep 17 00:00:00 2001 From: Salvador Martinez Date: Mon, 19 Jun 2017 16:04:17 -0700 Subject: [PATCH] Fix BatteryInfo using enhanced estimate for charge time Due to a typo in BatteryEstimates the enhanced estimate we use for discharging would also be used for charging. This CL corrects the typo and adds a simple test to catch a similar mistake in the future. We were also always loading the enhanced estimate even when we were not using it. As a ~*bonus*~ it should also help improve the speed of the charging use case for this screen and fixes an issue with the battery not switching to the charging icon in the main battery page. Test: robotest Bug: 62738028 Bug: 62873396 Bug: 62784575 Bug: 62736144 Change-Id: Ib2cbdeea22afb7da590b701b84f526bdac243410 --- .../android/settings/fuelgauge/BatteryInfo.java | 13 +++++++++---- .../settings/fuelgauge/BatteryInfoLoader.java | 11 ++++++++--- .../settings/fuelgauge/PowerUsageSummary.java | 7 ++----- .../settings/fuelgauge/BatteryInfoTest.java | 17 +++++++++++++++++ 4 files changed, 36 insertions(+), 12 deletions(-) diff --git a/src/com/android/settings/fuelgauge/BatteryInfo.java b/src/com/android/settings/fuelgauge/BatteryInfo.java index 48aa0b35901..e9863b312be 100644 --- a/src/com/android/settings/fuelgauge/BatteryInfo.java +++ b/src/com/android/settings/fuelgauge/BatteryInfo.java @@ -137,11 +137,15 @@ public class BatteryInfo { final BatteryUtils batteryUtils = BatteryUtils.getInstance(context); final long elapsedRealtimeUs = batteryUtils.convertMsToUs(SystemClock.elapsedRealtime()); + Intent batteryBroadcast = context.registerReceiver(null, new IntentFilter(Intent.ACTION_BATTERY_CHANGED)); BatteryUtils utils = BatteryUtils.getInstance(context); - - if (provider != null && provider.isEnhancedBatteryPredictionEnabled(context)) { + // 0 means we are discharging, anything else means charging + boolean discharging = + batteryBroadcast.getIntExtra(BatteryManager.EXTRA_PLUGGED, -1) == 0; + if (discharging && provider != null + && provider.isEnhancedBatteryPredictionEnabled(context)) { return BatteryInfo.getBatteryInfo(context, batteryBroadcast, stats, elapsedRealtimeUs, shortString, utils.convertMsToUs(provider.getEnhancedBatteryPrediction(context)), @@ -149,7 +153,8 @@ public class BatteryInfo { } else { return BatteryInfo.getBatteryInfo(context, batteryBroadcast, stats, elapsedRealtimeUs, shortString, - stats.computeBatteryTimeRemaining(elapsedRealtimeUs), false); + discharging ? stats.computeBatteryTimeRemaining(elapsedRealtimeUs) : 0, + false); } } @@ -211,7 +216,7 @@ public class BatteryInfo { if (chargeTime > 0 && status != BatteryManager.BATTERY_STATUS_FULL) { info.remainingTimeUs = chargeTime; CharSequence timeString = Utils.formatElapsedTime(context, - batteryUtils.convertUsToMs(drainTimeUs), false /* withSeconds */); + batteryUtils.convertUsToMs(chargeTime), false /* withSeconds */); int resId = shortString ? R.string.power_charging_duration_short : R.string.power_charging_duration; info.remainingLabel = TextUtils.expandTemplate(context.getText( diff --git a/src/com/android/settings/fuelgauge/BatteryInfoLoader.java b/src/com/android/settings/fuelgauge/BatteryInfoLoader.java index f1c25471218..e85acc582eb 100644 --- a/src/com/android/settings/fuelgauge/BatteryInfoLoader.java +++ b/src/com/android/settings/fuelgauge/BatteryInfoLoader.java @@ -20,6 +20,7 @@ import android.content.Intent; import android.content.IntentFilter; import android.database.Cursor; import android.net.Uri; +import android.os.BatteryManager; import android.os.BatteryStats; import android.os.SystemClock; import com.android.internal.os.BatteryStatsHelper; @@ -57,9 +58,12 @@ public class BatteryInfoLoader extends AsyncLoader{ final long elapsedRealtimeUs = batteryUtils.convertMsToUs(SystemClock.elapsedRealtime()); BatteryInfo batteryInfo; - // Get enhanced prediction if available, otherwise use the old prediction code + // 0 means we are discharging, anything else means charging + boolean discharging = batteryBroadcast.getIntExtra(BatteryManager.EXTRA_PLUGGED, -1) == 0; + // Get enhanced prediction if available and discharging, otherwise use the old code Cursor cursor = null; - if (powerUsageFeatureProvider.isEnhancedBatteryPredictionEnabled(context)) { + if (discharging && powerUsageFeatureProvider != null && + powerUsageFeatureProvider.isEnhancedBatteryPredictionEnabled(context)) { final Uri queryUri = powerUsageFeatureProvider.getEnhancedBatteryPredictionUri(); cursor = context.getContentResolver().query(queryUri, null, null, null, null); } @@ -72,7 +76,8 @@ public class BatteryInfoLoader extends AsyncLoader{ BatteryStats stats = mStatsHelper.getStats(); batteryInfo = BatteryInfo.getBatteryInfo(context, batteryBroadcast, stats, elapsedRealtimeUs, false /* shortString */, - stats.computeBatteryTimeRemaining(elapsedRealtimeUs), false /* basedOnUsage */); + discharging ? 0 : stats.computeBatteryTimeRemaining(elapsedRealtimeUs), + false /* basedOnUsage */); } return batteryInfo; diff --git a/src/com/android/settings/fuelgauge/PowerUsageSummary.java b/src/com/android/settings/fuelgauge/PowerUsageSummary.java index 707246a8de7..26dac41197e 100644 --- a/src/com/android/settings/fuelgauge/PowerUsageSummary.java +++ b/src/com/android/settings/fuelgauge/PowerUsageSummary.java @@ -769,11 +769,8 @@ public class PowerUsageSummary extends PowerUsageBase implements @VisibleForTesting void restartBatteryInfoLoader() { - if (mPowerFeatureProvider != null - && mPowerFeatureProvider.isEnhancedBatteryPredictionEnabled(getContext())) { - getLoaderManager().restartLoader(BATTERY_INFO_LOADER, Bundle.EMPTY, - mBatteryInfoLoaderCallbacks); - } + getLoaderManager().restartLoader(BATTERY_INFO_LOADER, Bundle.EMPTY, + mBatteryInfoLoaderCallbacks); } private static List getFakeStats() { diff --git a/tests/robotests/src/com/android/settings/fuelgauge/BatteryInfoTest.java b/tests/robotests/src/com/android/settings/fuelgauge/BatteryInfoTest.java index c1b6174e61b..cf1146e111c 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/BatteryInfoTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/BatteryInfoTest.java @@ -27,6 +27,7 @@ import com.android.settings.testutils.SettingsRobolectricTestRunner; import com.android.settings.TestConfig; import com.android.settings.testutils.FakeFeatureFactory; +import java.util.concurrent.TimeUnit; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -47,6 +48,7 @@ import static org.mockito.Mockito.spy; @RunWith(SettingsRobolectricTestRunner.class) @Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION) public class BatteryInfoTest { + private static final String STATUS_FULL = "Full"; private static final String STATUS_CHARGING_NO_TIME = "50% - charging"; private static final String STATUS_CHARGING_TIME = "50% - 0m until fully charged"; @@ -54,6 +56,9 @@ public class BatteryInfoTest { private static final long REMAINING_TIME_NULL = -1; private static final long REMAINING_TIME = 2; public static final String ENHANCED_STRING_SUFFIX = "left based on your usage"; + public static final long TEST_CHARGE_TIME_REMAINING = TimeUnit.MINUTES.toMicros(1); + public static final String TEST_CHARGE_TIME_REMAINING_STRINGIFIED = + "1m left until fully charged"; private Intent mDisChargingBatteryBroadcast; private Intent mChargingBatteryBroadcast; private Context mContext; @@ -147,4 +152,16 @@ public class BatteryInfoTest { assertThat(info.remainingLabel.toString()).doesNotContain(ENHANCED_STRING_SUFFIX); assertThat(info2.remainingLabel.toString()).doesNotContain(ENHANCED_STRING_SUFFIX); } + + @Test + public void testGetBatteryInfo_charging_usesChargeTime() { + doReturn(TEST_CHARGE_TIME_REMAINING) + .when(mBatteryStats) + .computeChargeTimeRemaining(anyLong()); + BatteryInfo info = BatteryInfo.getBatteryInfo(mContext, mChargingBatteryBroadcast, + mBatteryStats, SystemClock.elapsedRealtime() * 1000, false, 1000, false); + assertThat(info.remainingTimeUs = TEST_CHARGE_TIME_REMAINING); + assertThat(info.remainingLabel.toString()) + .isEqualTo(TEST_CHARGE_TIME_REMAINING_STRINGIFIED); + } }