diff --git a/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceController.java b/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceController.java index 6d2c1a1232d..64d7b1c36b9 100644 --- a/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceController.java +++ b/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceController.java @@ -108,10 +108,6 @@ public class BatteryChartPreferenceController extends AbstractPreferenceControll private View mBatteryChartViewGroup; private PreferenceScreen mPreferenceScreen; private FooterPreference mFooterPreference; - // Daily view model only saves abbreviated day of week texts (e.g. MON). This field saves the - // full day of week texts (e.g. Monday), which is used in category title and battery detail - // page. - private List mDailyTimestampFullTexts; private BatteryChartViewModel mDailyViewModel; private List mHourlyViewModels; @@ -122,6 +118,13 @@ public class BatteryChartPreferenceController extends AbstractPreferenceControll private final MetricsFeatureProvider mMetricsFeatureProvider; private final Handler mHandler = new Handler(Looper.getMainLooper()); + @VisibleForTesting + final DailyChartLabelTextGenerator mDailyChartLabelTextGenerator = + new DailyChartLabelTextGenerator(); + @VisibleForTesting + final HourlyChartLabelTextGenerator mHourlyChartLabelTextGenerator = + new HourlyChartLabelTextGenerator(); + // Preference cache to avoid create new instance each time. @VisibleForTesting final Map mPreferenceCache = new HashMap<>(); @@ -279,29 +282,24 @@ public class BatteryChartPreferenceController extends AbstractPreferenceControll getTotalHours(batteryLevelData)); if (batteryLevelData == null) { - mDailyTimestampFullTexts = null; mDailyViewModel = null; mHourlyViewModels = null; refreshUi(); return; } - mDailyTimestampFullTexts = generateTimestampDayOfWeekTexts( - mContext, batteryLevelData.getDailyBatteryLevels().getTimestamps(), - /* isAbbreviation= */ false); mDailyViewModel = new BatteryChartViewModel( batteryLevelData.getDailyBatteryLevels().getLevels(), - generateTimestampDayOfWeekTexts( - mContext, batteryLevelData.getDailyBatteryLevels().getTimestamps(), - /* isAbbreviation= */ true), - BatteryChartViewModel.AxisLabelPosition.CENTER_OF_TRAPEZOIDS); + batteryLevelData.getDailyBatteryLevels().getTimestamps(), + BatteryChartViewModel.AxisLabelPosition.CENTER_OF_TRAPEZOIDS, + mDailyChartLabelTextGenerator); mHourlyViewModels = new ArrayList<>(); for (BatteryLevelData.PeriodBatteryLevelData hourlyBatteryLevelsPerDay : batteryLevelData.getHourlyBatteryLevelsPerDay()) { mHourlyViewModels.add(new BatteryChartViewModel( hourlyBatteryLevelsPerDay.getLevels(), - generateTimestampHourTexts( - mContext, hourlyBatteryLevelsPerDay.getTimestamps()), - BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS)); + hourlyBatteryLevelsPerDay.getTimestamps(), + BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS, + mHourlyChartLabelTextGenerator)); } refreshUi(); } @@ -543,8 +541,7 @@ public class BatteryChartPreferenceController extends AbstractPreferenceControll @VisibleForTesting String getSlotInformation() { - if (mDailyTimestampFullTexts == null || mDailyViewModel == null - || mHourlyViewModels == null) { + if (mDailyViewModel == null || mHourlyViewModels == null) { // No data return null; } @@ -552,17 +549,13 @@ public class BatteryChartPreferenceController extends AbstractPreferenceControll return null; } - final String selectedDayText = mDailyTimestampFullTexts.get(mDailyChartIndex); + final String selectedDayText = mDailyViewModel.getFullText(mDailyChartIndex); if (mHourlyChartIndex == BatteryChartViewModel.SELECTED_INDEX_ALL) { return selectedDayText; } - final String fromHourText = mHourlyViewModels.get(mDailyChartIndex).texts().get( + final String selectedHourText = mHourlyViewModels.get(mDailyChartIndex).getFullText( mHourlyChartIndex); - final String toHourText = mHourlyViewModels.get(mDailyChartIndex).texts().get( - mHourlyChartIndex + 1); - final String selectedHourText = - String.format("%s%s%s", fromHourText, mIs24HourFormat ? "-" : " - ", toHourText); if (isBatteryLevelDataInOneDay()) { return selectedHourText; } @@ -663,25 +656,6 @@ public class BatteryChartPreferenceController extends AbstractPreferenceControll / DateUtils.HOUR_IN_MILLIS); } - private static List generateTimestampDayOfWeekTexts(@NonNull final Context context, - @NonNull final List timestamps, final boolean isAbbreviation) { - final ArrayList texts = new ArrayList<>(); - for (Long timestamp : timestamps) { - texts.add(ConvertUtils.utcToLocalTimeDayOfWeek(context, timestamp, isAbbreviation)); - } - return texts; - } - - private static List generateTimestampHourTexts( - @NonNull final Context context, @NonNull final List timestamps) { - final boolean is24HourFormat = DateFormat.is24HourFormat(context); - final ArrayList texts = new ArrayList<>(); - for (Long timestamp : timestamps) { - texts.add(ConvertUtils.utcToLocalTimeHour(context, timestamp, is24HourFormat)); - } - return texts; - } - /** Used for {@link AppBatteryPreferenceController}. */ public static List getAppBatteryUsageData(Context context) { final long start = System.currentTimeMillis(); @@ -727,4 +701,36 @@ public class BatteryChartPreferenceController extends AbstractPreferenceControll } return null; } + + private final class DailyChartLabelTextGenerator implements + BatteryChartViewModel.LabelTextGenerator { + @Override + public String generateText(List timestamps, int index) { + return ConvertUtils.utcToLocalTimeDayOfWeek(mContext, + timestamps.get(index), /* isAbbreviation= */ true); + } + + @Override + public String generateFullText(List timestamps, int index) { + return ConvertUtils.utcToLocalTimeDayOfWeek(mContext, + timestamps.get(index), /* isAbbreviation= */ false); + } + } + + private final class HourlyChartLabelTextGenerator implements + BatteryChartViewModel.LabelTextGenerator { + @Override + public String generateText(List timestamps, int index) { + return ConvertUtils.utcToLocalTimeHour(mContext, timestamps.get(index), + mIs24HourFormat); + } + + @Override + public String generateFullText(List timestamps, int index) { + return index == timestamps.size() - 1 + ? generateText(timestamps, index) + : String.format("%s%s%s", generateText(timestamps, index), + mIs24HourFormat ? "-" : " - ", generateText(timestamps, index + 1)); + } + } } diff --git a/src/com/android/settings/fuelgauge/batteryusage/BatteryChartView.java b/src/com/android/settings/fuelgauge/batteryusage/BatteryChartView.java index 40e3167c68b..f84ced76d88 100644 --- a/src/com/android/settings/fuelgauge/batteryusage/BatteryChartView.java +++ b/src/com/android/settings/fuelgauge/batteryusage/BatteryChartView.java @@ -158,7 +158,7 @@ public class BatteryChartView extends AppCompatImageView implements View.OnClick if (mViewModel != null) { int maxTop = 0; for (int index = 0; index < mViewModel.size(); index++) { - final String text = mViewModel.texts().get(index); + final String text = mViewModel.getText(index); mTextPaint.getTextBounds(text, 0, text.length(), mAxisLabelsBounds.get(index)); maxTop = Math.max(maxTop, -mAxisLabelsBounds.get(index).top); } @@ -490,7 +490,7 @@ public class BatteryChartView extends AppCompatImageView implements View.OnClick Canvas canvas, final int index, final Rect displayArea, final float baselineY) { mTextPaint.setTextAlign(Paint.Align.CENTER); canvas.drawText( - mViewModel.texts().get(index), + mViewModel.getText(index), displayArea.centerX(), baselineY, mTextPaint); @@ -524,9 +524,9 @@ public class BatteryChartView extends AppCompatImageView implements View.OnClick mTrapezoidPaint.setColor(isHoverState ? mTrapezoidHoverColor : trapezoidColor); final float leftTop = round( - trapezoidBottom - requireNonNull(mViewModel.levels().get(index)) * unitHeight); + trapezoidBottom - requireNonNull(mViewModel.getLevel(index)) * unitHeight); final float rightTop = round(trapezoidBottom - - requireNonNull(mViewModel.levels().get(index + 1)) * unitHeight); + - requireNonNull(mViewModel.getLevel(index + 1)) * unitHeight); trapezoidPath.reset(); trapezoidPath.moveTo(mTrapezoidSlots[index].mLeft, trapezoidBottom); trapezoidPath.lineTo(mTrapezoidSlots[index].mLeft, leftTop); @@ -564,8 +564,8 @@ public class BatteryChartView extends AppCompatImageView implements View.OnClick private static boolean isTrapezoidValid( @NonNull BatteryChartViewModel viewModel, int trapezoidIndex) { - return viewModel.levels().get(trapezoidIndex) != null - && viewModel.levels().get(trapezoidIndex + 1) != null; + return viewModel.getLevel(trapezoidIndex) != null + && viewModel.getLevel(trapezoidIndex + 1) != null; } private static boolean isTrapezoidIndexValid( @@ -617,8 +617,8 @@ public class BatteryChartView extends AppCompatImageView implements View.OnClick new AccessibilityNodeInfo(BatteryChartView.this, index); onInitializeAccessibilityNodeInfo(childInfo); childInfo.setClickable(isValidToDraw(mViewModel, index)); - childInfo.setText(mViewModel.texts().get(index)); - childInfo.setContentDescription(mViewModel.texts().get(index)); + childInfo.setText(mViewModel.getFullText(index)); + childInfo.setContentDescription(mViewModel.getFullText(index)); final Rect bounds = new Rect(); getBoundsOnScreen(bounds, true); diff --git a/src/com/android/settings/fuelgauge/batteryusage/BatteryChartViewModel.java b/src/com/android/settings/fuelgauge/batteryusage/BatteryChartViewModel.java index ac01bfd645b..f58d2415e19 100644 --- a/src/com/android/settings/fuelgauge/batteryusage/BatteryChartViewModel.java +++ b/src/com/android/settings/fuelgauge/batteryusage/BatteryChartViewModel.java @@ -19,6 +19,7 @@ package com.android.settings.fuelgauge.batteryusage; import androidx.annotation.NonNull; import androidx.core.util.Preconditions; +import java.util.Arrays; import java.util.List; import java.util.Locale; import java.util.Objects; @@ -38,34 +39,59 @@ class BatteryChartViewModel { CENTER_OF_TRAPEZOIDS, } + interface LabelTextGenerator { + /** Generate the label text. The text may be abbreviated to save space. */ + String generateText(List timestamps, int index); + + /** Generate the full text for accessibility. */ + String generateFullText(List timestamps, int index); + } + private final List mLevels; - private final List mTexts; + private final List mTimestamps; private final AxisLabelPosition mAxisLabelPosition; + private final LabelTextGenerator mLabelTextGenerator; + private final String[] mTexts; + private final String[] mFullTexts; + private int mSelectedIndex = SELECTED_INDEX_ALL; - BatteryChartViewModel( - @NonNull List levels, @NonNull List texts, - @NonNull AxisLabelPosition axisLabelPosition) { + BatteryChartViewModel(@NonNull List levels, @NonNull List timestamps, + @NonNull AxisLabelPosition axisLabelPosition, + @NonNull LabelTextGenerator labelTextGenerator) { Preconditions.checkArgument( - levels.size() == texts.size() && levels.size() >= MIN_LEVELS_DATA_SIZE, + levels.size() == timestamps.size() && levels.size() >= MIN_LEVELS_DATA_SIZE, String.format(Locale.ENGLISH, - "Invalid BatteryChartViewModel levels.size: %d, texts.size: %d.", - levels.size(), texts.size())); + "Invalid BatteryChartViewModel levels.size: %d, timestamps.size: %d.", + levels.size(), timestamps.size())); mLevels = levels; - mTexts = texts; + mTimestamps = timestamps; mAxisLabelPosition = axisLabelPosition; + mLabelTextGenerator = labelTextGenerator; + mTexts = new String[size()]; + mFullTexts = new String[size()]; } public int size() { return mLevels.size(); } - public List levels() { - return mLevels; + public Integer getLevel(int index) { + return mLevels.get(index); } - public List texts() { - return mTexts; + public String getText(int index) { + if (mTexts[index] == null) { + mTexts[index] = mLabelTextGenerator.generateText(mTimestamps, index); + } + return mTexts[index]; + } + + public String getFullText(int index) { + if (mFullTexts[index] == null) { + mFullTexts[index] = mLabelTextGenerator.generateFullText(mTimestamps, index); + } + return mFullTexts[index]; } public AxisLabelPosition axisLabelPosition() { @@ -82,7 +108,7 @@ class BatteryChartViewModel { @Override public int hashCode() { - return Objects.hash(mLevels, mTexts, mSelectedIndex, mAxisLabelPosition); + return Objects.hash(mLevels, mTimestamps, mSelectedIndex, mAxisLabelPosition); } @Override @@ -94,16 +120,26 @@ class BatteryChartViewModel { } final BatteryChartViewModel batteryChartViewModel = (BatteryChartViewModel) other; return Objects.equals(mLevels, batteryChartViewModel.mLevels) - && Objects.equals(mTexts, batteryChartViewModel.mTexts) + && Objects.equals(mTimestamps, batteryChartViewModel.mTimestamps) && mAxisLabelPosition == batteryChartViewModel.mAxisLabelPosition && mSelectedIndex == batteryChartViewModel.mSelectedIndex; } @Override public String toString() { - return String.format(Locale.ENGLISH, - "levels: %s,\ntexts: %s,\naxisLabelPosition: %s, selectedIndex: %d", - Objects.toString(mLevels), Objects.toString(mTexts), mAxisLabelPosition, - mSelectedIndex); + // Generate all the texts and full texts. + for (int i = 0; i < size(); i++) { + getText(i); + getFullText(i); + } + + return new StringBuilder() + .append("levels: " + Objects.toString(mLevels)) + .append(", timestamps: " + Objects.toString(mTimestamps)) + .append(", texts: " + Arrays.toString(mTexts)) + .append(", fullTexts: " + Arrays.toString(mFullTexts)) + .append(", axisLabelPosition: " + mAxisLabelPosition) + .append(", selectedIndex: " + mSelectedIndex) + .toString(); } } diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerTest.java index 20af849dcde..26e0f5074ed 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerTest.java @@ -19,6 +19,7 @@ package com.android.settings.fuelgauge.batteryusage; import static com.google.common.truth.Truth.assertThat; import static org.mockito.Mockito.any; +import static org.mockito.Mockito.atLeast; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.never; @@ -173,22 +174,33 @@ public final class BatteryChartPreferenceControllerTest { @Test public void setBatteryChartViewModel_6Hours() { + reset(mHourlyChartView); mBatteryChartPreferenceController.setBatteryHistoryMap(createBatteryHistoryMap(6)); verify(mDailyChartView, atLeastOnce()).setVisibility(View.GONE); verify(mHourlyChartView, atLeastOnce()).setVisibility(View.VISIBLE); - verify(mHourlyChartView).setViewModel(new BatteryChartViewModel( + // Ignore fast refresh ui from the data processor callback. + verify(mHourlyChartView, atLeast(0)).setViewModel(null); + verify(mHourlyChartView, atLeastOnce()).setViewModel(new BatteryChartViewModel( List.of(100, 97, 95), - List.of("8 AM", "10 AM", "12 PM"), - BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS)); + List.of(1619251200000L /* 8 AM */, + 1619258400000L /* 10 AM */, + 1619265600000L /* 12 PM */), + BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS, + mBatteryChartPreferenceController.mHourlyChartLabelTextGenerator)); } @Test public void setBatteryChartViewModel_60Hours() { BatteryChartViewModel expectedDailyViewModel = new BatteryChartViewModel( List.of(100, 83, 59, 41), - List.of("Sat", "Sun", "Mon", "Mon"), - BatteryChartViewModel.AxisLabelPosition.CENTER_OF_TRAPEZOIDS); + // "Sat", "Sun", "Mon", "Mon" + List.of(1619251200000L /* Sat */, + 1619308800000L /* Sun */, + 1619395200000L /* Mon */, + 1619460000000L /* Mon */), + BatteryChartViewModel.AxisLabelPosition.CENTER_OF_TRAPEZOIDS, + mBatteryChartPreferenceController.mDailyChartLabelTextGenerator); mBatteryChartPreferenceController.setBatteryHistoryMap(createBatteryHistoryMap(60)); @@ -208,9 +220,17 @@ public final class BatteryChartPreferenceControllerTest { verify(mDailyChartView).setViewModel(expectedDailyViewModel); verify(mHourlyChartView).setViewModel(new BatteryChartViewModel( List.of(100, 97, 95, 93, 91, 89, 87, 85, 83), - List.of("8 AM", "10 AM", "12 PM", "2 PM", "4 PM", "6 PM", "8 PM", "10 PM", - "12 AM"), - BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS)); + List.of(1619251200000L /* 8 AM */, + 1619258400000L /* 10 AM */, + 1619265600000L /* 12 PM */, + 1619272800000L /* 2 PM */, + 1619280000000L /* 4 PM */, + 1619287200000L /* 6 PM */, + 1619294400000L /* 8 PM */, + 1619301600000L /* 10 PM */, + 1619308800000L /* 12 AM */), + BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS, + mBatteryChartPreferenceController.mHourlyChartLabelTextGenerator)); reset(mDailyChartView); reset(mHourlyChartView); @@ -224,9 +244,21 @@ public final class BatteryChartPreferenceControllerTest { verify(mDailyChartView).setViewModel(expectedDailyViewModel); BatteryChartViewModel expectedHourlyViewModel = new BatteryChartViewModel( List.of(83, 81, 79, 77, 75, 73, 71, 69, 67, 65, 63, 61, 59), - List.of("12 AM", "2 AM", "4 AM", "6 AM", "8 AM", "10 AM", "12 PM", "2 PM", - "4 PM", "6 PM", "8 PM", "10 PM", "12 AM"), - BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS); + List.of(1619308800000L /* 12 AM */, + 1619316000000L /* 2 AM */, + 1619323200000L /* 4 AM */, + 1619330400000L /* 6 AM */, + 1619337600000L /* 8 AM */, + 1619344800000L /* 10 AM */, + 1619352000000L /* 12 PM */, + 1619359200000L /* 2 PM */, + 1619366400000L /* 4 PM */, + 1619373600000L /* 6 PM */, + 1619380800000L /* 8 PM */, + 1619388000000L /* 10 PM */, + 1619395200000L /* 12 AM */), + BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS, + mBatteryChartPreferenceController.mHourlyChartLabelTextGenerator); expectedHourlyViewModel.setSelectedIndex(6); verify(mHourlyChartView).setViewModel(expectedHourlyViewModel); @@ -243,9 +275,18 @@ public final class BatteryChartPreferenceControllerTest { verify(mDailyChartView).setViewModel(expectedDailyViewModel); verify(mHourlyChartView).setViewModel(new BatteryChartViewModel( List.of(59, 57, 55, 53, 51, 49, 47, 45, 43, 41), - List.of("12 AM", "2 AM", "4 AM", "6 AM", "8 AM", "10 AM", "12 PM", "2 PM", - "4 PM", "6 PM"), - BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS)); + List.of(1619395200000L /* 12 AM */, + 1619402400000L /* 2 AM */, + 1619409600000L /* 4 AM */, + 1619416800000L /* 6 AM */, + 1619424000000L /* 8 AM */, + 1619431200000L /* 10 AM */, + 1619438400000L /* 12 PM */, + 1619445600000L /* 2 PM */, + 1619452800000L /* 4 PM */, + 1619460000000L /* 6 PM */), + BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS, + mBatteryChartPreferenceController.mHourlyChartLabelTextGenerator)); } @Test diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartViewTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartViewTest.java index 7e423e0ccab..52131996e5e 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartViewTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartViewTest.java @@ -63,8 +63,8 @@ public final class BatteryChartViewTest { public void onClick_invokesCallback() { final int originalSelectedIndex = 2; BatteryChartViewModel batteryChartViewModel = new BatteryChartViewModel( - List.of(90, 80, 70, 60), List.of("", "", "", ""), - BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS); + List.of(90, 80, 70, 60), List.of(0L, 0L, 0L, 0L), + BatteryChartViewModel.AxisLabelPosition.BETWEEN_TRAPEZOIDS, null); batteryChartViewModel.setSelectedIndex(originalSelectedIndex); mBatteryChartView.setViewModel(batteryChartViewModel); for (int i = 0; i < mBatteryChartView.mTrapezoidSlots.length; i++) {