From d2b295ba36a7b8e237c3792abd177020d1046227 Mon Sep 17 00:00:00 2001 From: Lei Yu Date: Wed, 9 May 2018 17:27:17 -0700 Subject: [PATCH] Update the battery broadcast receiver. Add a type for battery receiver, then in callback client know for which reason it been invoked: 1. battery level change 2. battery saver state change 3. battery plug state change So in this CL, we won't update battery tip for battery level change, then battery tip won't be dismissed by itself. Also note in onResume() we will manually update battery tip. So if user stay in battery settings page and close the screen, once he opens it we will still force update everything. Fixes: 79171742 Test: RunSettingsRoboTests Change-Id: I997844216fd8267e545d74e0d434de9e338f76a1 --- .../fuelgauge/BatteryBroadcastReceiver.java | 35 +++++++++--- .../fuelgauge/PowerUsageAdvanced.java | 7 ++- .../settings/fuelgauge/PowerUsageBase.java | 55 ++++++++++++------- .../settings/fuelgauge/PowerUsageSummary.java | 25 ++++----- .../BatteryBroadcastReceiverTest.java | 13 +++-- .../fuelgauge/PowerUsageAdvancedTest.java | 3 +- .../fuelgauge/PowerUsageBaseTest.java | 2 +- .../fuelgauge/PowerUsageSummaryTest.java | 26 +++++---- 8 files changed, 103 insertions(+), 63 deletions(-) diff --git a/src/com/android/settings/fuelgauge/BatteryBroadcastReceiver.java b/src/com/android/settings/fuelgauge/BatteryBroadcastReceiver.java index c051604d113..bd64dd3d4c7 100644 --- a/src/com/android/settings/fuelgauge/BatteryBroadcastReceiver.java +++ b/src/com/android/settings/fuelgauge/BatteryBroadcastReceiver.java @@ -21,9 +21,15 @@ import android.content.Context; import android.content.Intent; import android.content.IntentFilter; import android.os.PowerManager; + +import androidx.annotation.IntDef; import androidx.annotation.VisibleForTesting; import com.android.settings.Utils; +import com.android.settings.fuelgauge.batterytip.tips.BatteryTip; + +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; /** * Use this broadcastReceiver to listen to the battery change, and it will invoke @@ -43,7 +49,19 @@ public class BatteryBroadcastReceiver extends BroadcastReceiver { * Battery saver(e.g. off->on) */ public interface OnBatteryChangedListener { - void onBatteryChanged(); + void onBatteryChanged(@BatteryUpdateType int type); + } + + @Retention(RetentionPolicy.SOURCE) + @IntDef({BatteryUpdateType.MANUAL, + BatteryUpdateType.BATTERY_LEVEL, + BatteryUpdateType.BATTERY_SAVER, + BatteryUpdateType.BATTERY_STATUS}) + public @interface BatteryUpdateType { + int MANUAL = 0; + int BATTERY_LEVEL = 1; + int BATTERY_SAVER = 2; + int BATTERY_STATUS = 3; } @VisibleForTesting @@ -85,14 +103,17 @@ public class BatteryBroadcastReceiver extends BroadcastReceiver { final String batteryLevel = Utils.getBatteryPercentage(intent); final String batteryStatus = Utils.getBatteryStatus( mContext.getResources(), intent); - if (forceUpdate || !batteryLevel.equals(mBatteryLevel) || !batteryStatus.equals( - mBatteryStatus)) { - mBatteryLevel = batteryLevel; - mBatteryStatus = batteryStatus; - mBatteryListener.onBatteryChanged(); + if (forceUpdate) { + mBatteryListener.onBatteryChanged(BatteryUpdateType.MANUAL); + } else if(!batteryLevel.equals(mBatteryLevel)) { + mBatteryListener.onBatteryChanged(BatteryUpdateType.BATTERY_LEVEL); + } else if (!batteryStatus.equals(mBatteryStatus)) { + mBatteryListener.onBatteryChanged(BatteryUpdateType.BATTERY_STATUS); } + mBatteryLevel = batteryLevel; + mBatteryStatus = batteryStatus; } else if (PowerManager.ACTION_POWER_SAVE_MODE_CHANGED.equals(intent.getAction())) { - mBatteryListener.onBatteryChanged(); + mBatteryListener.onBatteryChanged(BatteryUpdateType.BATTERY_SAVER); } } } diff --git a/src/com/android/settings/fuelgauge/PowerUsageAdvanced.java b/src/com/android/settings/fuelgauge/PowerUsageAdvanced.java index a318e160918..74098ffb9a0 100644 --- a/src/com/android/settings/fuelgauge/PowerUsageAdvanced.java +++ b/src/com/android/settings/fuelgauge/PowerUsageAdvanced.java @@ -13,6 +13,8 @@ */ package com.android.settings.fuelgauge; +import static com.android.settings.fuelgauge.BatteryBroadcastReceiver.BatteryUpdateType; + import android.content.Context; import android.content.Intent; import android.content.IntentFilter; @@ -33,6 +35,7 @@ import com.android.settingslib.core.AbstractPreferenceController; import com.android.settingslib.search.SearchIndexable; import com.android.settingslib.utils.StringUtil; + import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -108,7 +111,7 @@ public class PowerUsageAdvanced extends PowerUsageBase { mMetricsFeatureProvider.action(getContext(), MetricsProto.MetricsEvent.ACTION_SETTINGS_MENU_BATTERY_APPS_TOGGLE, mShowAllApps); - restartBatteryStatsLoader(); + restartBatteryStatsLoader(BatteryUpdateType.MANUAL); return true; default: return super.onOptionsItemSelected(item); @@ -140,7 +143,7 @@ public class PowerUsageAdvanced extends PowerUsageBase { } @Override - protected void refreshUi() { + protected void refreshUi(@BatteryUpdateType int refreshType) { final Context context = getContext(); if (context == null) { return; diff --git a/src/com/android/settings/fuelgauge/PowerUsageBase.java b/src/com/android/settings/fuelgauge/PowerUsageBase.java index f924e46b0f3..aa029191a86 100644 --- a/src/com/android/settings/fuelgauge/PowerUsageBase.java +++ b/src/com/android/settings/fuelgauge/PowerUsageBase.java @@ -15,6 +15,8 @@ */ package com.android.settings.fuelgauge; +import static com.android.settings.fuelgauge.BatteryBroadcastReceiver.*; + import android.app.Activity; import android.app.LoaderManager; import android.content.Context; @@ -26,18 +28,17 @@ import android.view.Menu; import com.android.internal.os.BatteryStatsHelper; import com.android.settings.dashboard.DashboardFragment; -import com.android.settingslib.utils.AsyncLoader; /** * Common base class for things that need to show the battery usage graph. */ -public abstract class PowerUsageBase extends DashboardFragment - implements LoaderManager.LoaderCallbacks { +public abstract class PowerUsageBase extends DashboardFragment { // +1 to allow ordering for PowerUsageSummary. @VisibleForTesting static final int MENU_STATS_REFRESH = Menu.FIRST + 1; private static final String TAG = "PowerUsageBase"; + private static final String KEY_REFRESH_TYPE = "refresh_type"; protected BatteryStatsHelper mStatsHelper; protected UserManager mUm; @@ -57,8 +58,8 @@ public abstract class PowerUsageBase extends DashboardFragment setHasOptionsMenu(true); mBatteryBroadcastReceiver = new BatteryBroadcastReceiver(getContext()); - mBatteryBroadcastReceiver.setBatteryChangedListener(() -> { - restartBatteryStatsLoader(); + mBatteryBroadcastReceiver.setBatteryChangedListener(type -> { + restartBatteryStatsLoader(type); }); } @@ -81,11 +82,14 @@ public abstract class PowerUsageBase extends DashboardFragment mBatteryBroadcastReceiver.unRegister(); } - protected void restartBatteryStatsLoader() { - getLoaderManager().restartLoader(0, Bundle.EMPTY, this); + protected void restartBatteryStatsLoader(int refreshType) { + final Bundle bundle = new Bundle(); + bundle.putInt(KEY_REFRESH_TYPE, refreshType); + + getLoaderManager().restartLoader(0, bundle, new PowerLoaderCallback()); } - protected abstract void refreshUi(); + protected abstract void refreshUi(@BatteryUpdateType int refreshType); protected void updatePreference(BatteryHistoryPreference historyPref) { final long startTime = System.currentTimeMillis(); @@ -93,21 +97,30 @@ public abstract class PowerUsageBase extends DashboardFragment BatteryUtils.logRuntime(TAG, "updatePreference", startTime); } - @Override - public Loader onCreateLoader(int id, - Bundle args) { - return new BatteryStatsHelperLoader(getContext()); - } + /** + * {@link android.app.LoaderManager.LoaderCallbacks} for {@link PowerUsageBase} to load + * the {@link BatteryStatsHelper} + */ + public class PowerLoaderCallback implements LoaderManager.LoaderCallbacks { + private int mRefreshType; - @Override - public void onLoadFinished(Loader loader, - BatteryStatsHelper statsHelper) { - mStatsHelper = statsHelper; - refreshUi(); - } + @Override + public Loader onCreateLoader(int id, + Bundle args) { + mRefreshType = args.getInt(KEY_REFRESH_TYPE); + return new BatteryStatsHelperLoader(getContext()); + } - @Override - public void onLoaderReset(Loader loader) { + @Override + public void onLoadFinished(Loader loader, + BatteryStatsHelper statsHelper) { + mStatsHelper = statsHelper; + refreshUi(mRefreshType); + } + @Override + public void onLoaderReset(Loader loader) { + + } } } diff --git a/src/com/android/settings/fuelgauge/PowerUsageSummary.java b/src/com/android/settings/fuelgauge/PowerUsageSummary.java index af69cee2d0f..82cd16a796b 100644 --- a/src/com/android/settings/fuelgauge/PowerUsageSummary.java +++ b/src/com/android/settings/fuelgauge/PowerUsageSummary.java @@ -16,6 +16,8 @@ package com.android.settings.fuelgauge; +import static com.android.settings.fuelgauge.BatteryBroadcastReceiver.BatteryUpdateType; + import android.app.Activity; import android.app.LoaderManager; import android.app.LoaderManager.LoaderCallbacks; @@ -279,7 +281,7 @@ public class PowerUsageSummary extends PowerUsageBase implements OnLongClickList } else { mStatsType = BatteryStats.STATS_SINCE_CHARGED; } - refreshUi(); + refreshUi(BatteryUpdateType.MANUAL); return true; case MENU_ADVANCED_BATTERY: new SubSettingLauncher(getContext()) @@ -293,14 +295,15 @@ public class PowerUsageSummary extends PowerUsageBase implements OnLongClickList } } - protected void refreshUi() { + protected void refreshUi(@BatteryUpdateType int refreshType) { final Context context = getContext(); if (context == null) { return; } - // Only skip BatteryTipLoader for the first time when device is rotated - if (mNeedUpdateBatteryTip) { + // Skip BatteryTipLoader if device is rotated or only battery level change + if (mNeedUpdateBatteryTip + && refreshType != BatteryUpdateType.BATTERY_LEVEL) { restartBatteryTipLoader(); } else { mNeedUpdateBatteryTip = true; @@ -399,8 +402,9 @@ public class PowerUsageSummary extends PowerUsageBase implements OnLongClickList } @Override - protected void restartBatteryStatsLoader() { - restartBatteryStatsLoader(true /* clearHeader */); + protected void restartBatteryStatsLoader(@BatteryUpdateType int refreshType) { + super.restartBatteryStatsLoader(refreshType); + mBatteryHeaderPreferenceController.quickUpdateHeaderPreference(); } @Override @@ -409,13 +413,6 @@ public class PowerUsageSummary extends PowerUsageBase implements OnLongClickList mBatteryTipPreferenceController.saveInstanceState(outState); } - void restartBatteryStatsLoader(boolean clearHeader) { - super.restartBatteryStatsLoader(); - if (clearHeader) { - mBatteryHeaderPreferenceController.quickUpdateHeaderPreference(); - } - } - @Override public void onBatteryTipHandled(BatteryTip batteryTip) { restartBatteryTipLoader(); @@ -430,7 +427,7 @@ public class PowerUsageSummary extends PowerUsageBase implements OnLongClickList mContext = context; mLoader = loader; mBatteryBroadcastReceiver = new BatteryBroadcastReceiver(mContext); - mBatteryBroadcastReceiver.setBatteryChangedListener(() -> { + mBatteryBroadcastReceiver.setBatteryChangedListener(type -> { BatteryInfo.getBatteryInfo(mContext, new BatteryInfo.Callback() { @Override public void onBatteryInfoLoaded(BatteryInfo info) { diff --git a/tests/robotests/src/com/android/settings/fuelgauge/BatteryBroadcastReceiverTest.java b/tests/robotests/src/com/android/settings/fuelgauge/BatteryBroadcastReceiverTest.java index 3fdbe837425..4583dd1b3b4 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/BatteryBroadcastReceiverTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/BatteryBroadcastReceiverTest.java @@ -15,8 +15,11 @@ */ package com.android.settings.fuelgauge; +import static com.android.settings.fuelgauge.BatteryBroadcastReceiver.BatteryUpdateType; + import static com.google.common.truth.Truth.assertThat; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyInt; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; @@ -70,14 +73,14 @@ public class BatteryBroadcastReceiverTest { } @Test - public void testOnReceive_batteryDataChanged_dataUpdated() { + public void testOnReceive_batteryLevelChanged_dataUpdated() { mBatteryBroadcastReceiver.onReceive(mContext, mChargingIntent); assertThat(mBatteryBroadcastReceiver.mBatteryLevel) .isEqualTo(Utils.getBatteryPercentage(mChargingIntent)); assertThat(mBatteryBroadcastReceiver.mBatteryStatus) .isEqualTo(Utils.getBatteryStatus(mContext.getResources(), mChargingIntent)); - verify(mBatteryListener).onBatteryChanged(); + verify(mBatteryListener).onBatteryChanged(BatteryUpdateType.BATTERY_LEVEL); } @Test @@ -85,7 +88,7 @@ public class BatteryBroadcastReceiverTest { mBatteryBroadcastReceiver.onReceive(mContext, new Intent(PowerManager.ACTION_POWER_SAVE_MODE_CHANGED)); - verify(mBatteryListener).onBatteryChanged(); + verify(mBatteryListener).onBatteryChanged(BatteryUpdateType.BATTERY_SAVER); } @Test @@ -100,7 +103,7 @@ public class BatteryBroadcastReceiverTest { assertThat(mBatteryBroadcastReceiver.mBatteryLevel).isEqualTo(batteryLevel); assertThat(mBatteryBroadcastReceiver.mBatteryStatus).isEqualTo(batteryStatus); - verify(mBatteryListener, never()).onBatteryChanged(); + verify(mBatteryListener, never()).onBatteryChanged(anyInt()); } @Test @@ -115,6 +118,6 @@ public class BatteryBroadcastReceiverTest { assertThat(mBatteryBroadcastReceiver.mBatteryStatus) .isEqualTo(Utils.getBatteryStatus(mContext.getResources(), mChargingIntent)); // 2 times because register will force update the battery - verify(mBatteryListener, times(2)).onBatteryChanged(); + verify(mBatteryListener, times(2)).onBatteryChanged(BatteryUpdateType.MANUAL); } } diff --git a/tests/robotests/src/com/android/settings/fuelgauge/PowerUsageAdvancedTest.java b/tests/robotests/src/com/android/settings/fuelgauge/PowerUsageAdvancedTest.java index 8a7e7a04687..ffa49d4f253 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/PowerUsageAdvancedTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/PowerUsageAdvancedTest.java @@ -18,6 +18,7 @@ package com.android.settings.fuelgauge; import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.nullable; +import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; @@ -86,7 +87,7 @@ public class PowerUsageAdvancedTest { @Test public void testOptionsMenu_menuAppToggle_metricEventInvoked() { mFragment.mShowAllApps = false; - doNothing().when(mFragment).restartBatteryStatsLoader(); + doNothing().when(mFragment).restartBatteryStatsLoader(anyInt()); mFragment.onOptionsItemSelected(mToggleAppsMenu); diff --git a/tests/robotests/src/com/android/settings/fuelgauge/PowerUsageBaseTest.java b/tests/robotests/src/com/android/settings/fuelgauge/PowerUsageBaseTest.java index d9f572deefa..eb683c072e0 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/PowerUsageBaseTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/PowerUsageBaseTest.java @@ -74,7 +74,7 @@ public class PowerUsageBaseTest { } @Override - protected void refreshUi() { + protected void refreshUi(int refreshType) { // Do nothing } diff --git a/tests/robotests/src/com/android/settings/fuelgauge/PowerUsageSummaryTest.java b/tests/robotests/src/com/android/settings/fuelgauge/PowerUsageSummaryTest.java index b48f00e295b..07341a14656 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/PowerUsageSummaryTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/PowerUsageSummaryTest.java @@ -149,7 +149,7 @@ public class PowerUsageSummaryTest { mFragment.initFeatureProvider(); mBatteryMeterView = new BatteryMeterView(mRealContext); mBatteryMeterView.mDrawable = new BatteryMeterView.BatteryMeterDrawable(mRealContext, 0); - doNothing().when(mFragment).restartBatteryStatsLoader(); + doNothing().when(mFragment).restartBatteryStatsLoader(anyInt()); doReturn(mock(LoaderManager.class)).when(mFragment).getLoaderManager(); doReturn(MENU_ADVANCED_BATTERY).when(mAdvancedPageMenu).getItemId(); @@ -316,15 +316,6 @@ public class PowerUsageSummaryTest { verify(mSummary1, times(2)).setOnLongClickListener(any(View.OnLongClickListener.class)); } - @Test - public void restartBatteryStatsLoader_notClearHeader_quickUpdateNotInvoked() { - mFragment.mBatteryHeaderPreferenceController = mBatteryHeaderPreferenceController; - - mFragment.restartBatteryStatsLoader(false /* clearHeader */); - - verify(mBatteryHeaderPreferenceController, never()).quickUpdateHeaderPreference(); - } - @Test public void optionsMenu_advancedPageEnabled() { when(mFeatureFactory.powerUsageFeatureProvider.isPowerAccountingToggleEnabled()) @@ -360,7 +351,18 @@ public class PowerUsageSummaryTest { when(mFragment.mBatteryTipPreferenceController.needUpdate()).thenReturn(false); mFragment.updateBatteryTipFlag(new Bundle()); - mFragment.refreshUi(); + mFragment.refreshUi(BatteryBroadcastReceiver.BatteryUpdateType.MANUAL); + + verify(mFragment, never()).restartBatteryTipLoader(); + } + + @Test + public void refreshUi_batteryLevelChanged_doNotUpdateBatteryTip() { + mFragment.mBatteryTipPreferenceController = mock(BatteryTipPreferenceController.class); + when(mFragment.mBatteryTipPreferenceController.needUpdate()).thenReturn(true); + mFragment.updateBatteryTipFlag(new Bundle()); + + mFragment.refreshUi(BatteryBroadcastReceiver.BatteryUpdateType.BATTERY_LEVEL); verify(mFragment, never()).restartBatteryTipLoader(); } @@ -371,7 +373,7 @@ public class PowerUsageSummaryTest { when(mFragment.mBatteryTipPreferenceController.needUpdate()).thenReturn(true); mFragment.updateBatteryTipFlag(new Bundle()); - mFragment.refreshUi(); + mFragment.refreshUi(BatteryBroadcastReceiver.BatteryUpdateType.MANUAL); verify(mFragment).restartBatteryTipLoader(); }