diff --git a/app/src/main/java/com/almothafar/simplebatterynotifier/model/BatteryDO.java b/app/src/main/java/com/almothafar/simplebatterynotifier/model/BatteryDO.java index 83ed721..d1f8ffb 100644 --- a/app/src/main/java/com/almothafar/simplebatterynotifier/model/BatteryDO.java +++ b/app/src/main/java/com/almothafar/simplebatterynotifier/model/BatteryDO.java @@ -25,6 +25,9 @@ public final class BatteryDO { // Instantaneous current in µA from BATTERY_PROPERTY_CURRENT_NOW; Integer.MIN_VALUE when the device // doesn't report it. Sign convention varies by OEM, so callers derive direction from the status. private int currentMicroAmps = Integer.MIN_VALUE; + // OS-reported charge cycle count from EXTRA_CYCLE_COUNT (Android 14+); -1 when the device doesn't + // report it. Carried on the snapshot so per-tick consumers don't re-read the sticky broadcast (#159/#161). + private int cycleCount = -1; private String technology; private String powerSource; private String health; @@ -212,6 +215,15 @@ public BatteryDO setCurrentMicroAmps(final int currentMicroAmps) { return this; } + public int getCycleCount() { + return cycleCount; + } + + public BatteryDO setCycleCount(final int cycleCount) { + this.cycleCount = cycleCount; + return this; + } + public String getPowerSource() { return powerSource; } diff --git a/app/src/main/java/com/almothafar/simplebatterynotifier/service/BatteryHealthTracker.java b/app/src/main/java/com/almothafar/simplebatterynotifier/service/BatteryHealthTracker.java index a5709d8..b47d144 100644 --- a/app/src/main/java/com/almothafar/simplebatterynotifier/service/BatteryHealthTracker.java +++ b/app/src/main/java/com/almothafar/simplebatterynotifier/service/BatteryHealthTracker.java @@ -189,7 +189,24 @@ public static int getEffectiveCycleCount(final Context context) { if (isNull(context)) { return 0; } - final int osCycleCount = SystemService.getChargeCycleCount(context); + return getEffectiveCycleCount(context, SystemService.getChargeCycleCount(context)); + } + + /** + * Same as {@link #getEffectiveCycleCount(Context)}, for callers that already hold the OS cycle + * count (e.g. from a {@code BatteryDO} snapshot) — avoids a second sticky-broadcast read on the + * per-tick refresh path (#161). + * + * @param context Application context + * @param osCycleCount the already-read OS cycle count, or -1/0 when the device doesn't report one + * + * @return Charge cycle count (OS-reported if available, else the tracked estimate) plus any + * debug-injected cycles + */ + public static int getEffectiveCycleCount(final Context context, final int osCycleCount) { + if (isNull(context)) { + return 0; + } final int base = osCycleCount > 0 ? osCycleCount : getChargeCycles(context); return base + getDebugChargeCycles(context); } @@ -201,17 +218,15 @@ public static int getEffectiveCycleCount(final Context context) { * The distinction matters for honesty (#114): the OS count covers the battery's whole life, while * the app-tracked estimate only counts charge delivered since the app was installed — on an older * phone it starts at zero and therefore understates real wear. The insights screen labels the - * health basis differently for the two sources. + * health basis differently for the two sources. Takes the already-read count so the caller's + * single sticky read serves both this and {@link #getEffectiveCycleCount(Context, int)} (#161). * - * @param context Application context + * @param osCycleCount the already-read OS cycle count, or -1/0 when the device doesn't report one * * @return true when the OS reports a cycle count for this device */ - public static boolean isCycleCountFromOs(final Context context) { - if (isNull(context)) { - return false; - } - return SystemService.getChargeCycleCount(context) > 0; + public static boolean isCycleCountFromOs(final int osCycleCount) { + return osCycleCount > 0; } /** @@ -345,17 +360,18 @@ public static void setDesignCapacity(final Context context, final int mAh) { * capacity. It is shown regardless of charge level so it stays consistent with the displayed * Capacity (#103); when no design capacity is set, callers fall back to the cycle-based estimate. * - * @param context Application context + * @param context Application context + * @param currentFullMah the already-read current full capacity estimate in mAh (0 when unknown), + * e.g. {@code batteryDO.getCapacity()} — passed in so one capacity estimate + * serves the whole refresh (#161) * * @return Measured health percentage (1-100), or -1 when it cannot be determined */ - public static int getMeasuredHealthPercentage(final Context context) { + public static int getMeasuredHealthPercentage(final Context context, final int currentFullMah) { if (isNull(context)) { return -1; } - return computeMeasuredHealth( - SystemService.getBatteryCapacity(context), - getDesignCapacity(context)); + return computeMeasuredHealth(currentFullMah, getDesignCapacity(context)); } /** @@ -394,18 +410,19 @@ static int computeMeasuredHealth(final int currentFullMah, final int designMah) * keeps the figure but adds a warning — see issue #94. A genuinely worn-out battery can also read * this low, a possibility the UI surfaces to the user. * - * @param context Application context + * @param context Application context + * @param currentFullMah the already-read current full capacity estimate in mAh (0 when unknown), + * e.g. {@code batteryDO.getCapacity()} — passed in so one capacity estimate + * serves the whole refresh (#161) * * @return true when a design capacity is set, a charge-counter estimate exists, and the estimate is * outside the plausible window around the design capacity */ - public static boolean isBatteryReadingUnreliable(final Context context) { + public static boolean isBatteryReadingUnreliable(final Context context, final int currentFullMah) { if (isNull(context)) { return false; } - return isEstimateImplausible( - SystemService.getBatteryCapacity(context), - getDesignCapacity(context)); + return isEstimateImplausible(currentFullMah, getDesignCapacity(context)); } /** @@ -492,8 +509,18 @@ public static int getDaysSinceFirstUse(final Context context) { * @return Estimated battery health percentage (0-100) */ public static int getEstimatedHealthPercentage(final Context context) { - final int cycles = getEffectiveCycleCount(context); + return estimatedHealthForCycles(getEffectiveCycleCount(context)); + } + /** + * Pure core of {@link #getEstimatedHealthPercentage}, taking an already-known cycle count so a + * caller's single read can serve every cycle-derived figure on the screen (#161). + * + * @param cycles effective charge cycle count + * + * @return Estimated battery health percentage (40-100) + */ + public static int estimatedHealthForCycles(final int cycles) { // Excellent health: 0-300 cycles (100% to 95%) if (cycles < EXCELLENT_THRESHOLD) { return 100 - (cycles * 5 / EXCELLENT_THRESHOLD); @@ -527,8 +554,18 @@ public static int getEstimatedHealthPercentage(final Context context) { * @return the matching {@link BatteryHealthGrade} */ public static BatteryHealthGrade getHealthGrade(final Context context) { - final int cycles = getEffectiveCycleCount(context); + return gradeForCycles(getEffectiveCycleCount(context)); + } + /** + * Pure core of {@link #getHealthGrade}, taking an already-known cycle count (#161). Uses the same + * thresholds as {@link #estimatedHealthForCycles} so percentage and grade always agree. + * + * @param cycles effective charge cycle count + * + * @return the matching {@link BatteryHealthGrade} + */ + public static BatteryHealthGrade gradeForCycles(final int cycles) { if (cycles < EXCELLENT_THRESHOLD) { return BatteryHealthGrade.EXCELLENT; } else if (cycles < GOOD_THRESHOLD) { diff --git a/app/src/main/java/com/almothafar/simplebatterynotifier/service/SystemService.java b/app/src/main/java/com/almothafar/simplebatterynotifier/service/SystemService.java index c022cda..bda8063 100644 --- a/app/src/main/java/com/almothafar/simplebatterynotifier/service/SystemService.java +++ b/app/src/main/java/com/almothafar/simplebatterynotifier/service/SystemService.java @@ -150,7 +150,12 @@ private static BatteryExtras extractBatteryExtras(final Intent batteryStatus) { final boolean present = extras != null && extras.getBoolean(BatteryManager.EXTRA_PRESENT); final String technology = extras != null ? extras.getString(BatteryManager.EXTRA_TECHNOLOGY) : ""; - return new BatteryExtras(level, scale, status, health, plugged, temperature, voltage, present, technology); + // Absent below Android 14 (and on devices whose fuel gauge doesn't report it) — normalized to + // -1 like getChargeCycleCount, so snapshot consumers don't need a second sticky read (#161). + final int rawCycleCount = batteryStatus.getIntExtra(BatteryManager.EXTRA_CYCLE_COUNT, -1); + final int cycleCount = rawCycleCount > 0 ? rawCycleCount : -1; + + return new BatteryExtras(level, scale, status, health, plugged, temperature, voltage, present, technology, cycleCount); } /** @@ -210,6 +215,7 @@ private static BatteryDO buildBatteryDataObject(final BatteryExtras extras, .setCapacity(batteryCapacity) .setCurrentMicroAmps(currentMicroAmps) .setChargeCounterMicroAmpHours(chargeCounterUah) + .setCycleCount(extras.cycleCount) .setIntHealth(extras.health); // Determine health status and set it on the battery object @@ -614,6 +620,7 @@ private record BatteryExtras(int level, int temperature, int voltage, boolean present, - String technology) { + String technology, + int cycleCount) { } } diff --git a/app/src/main/java/com/almothafar/simplebatterynotifier/ui/BatteryInsightsActivity.java b/app/src/main/java/com/almothafar/simplebatterynotifier/ui/BatteryInsightsActivity.java index 6eab747..7d0d28e 100644 --- a/app/src/main/java/com/almothafar/simplebatterynotifier/ui/BatteryInsightsActivity.java +++ b/app/src/main/java/com/almothafar/simplebatterynotifier/ui/BatteryInsightsActivity.java @@ -91,16 +91,22 @@ protected void onResume() { * Updates all health data displays with current values from BatteryHealthTracker. */ private void updateHealthData() { + // One sticky read and one capacity estimate per refresh (#161): every cycle- and + // capacity-derived figure below is computed from these two values. + final int osCycles = SystemService.getChargeCycleCount(this); + final int cycles = BatteryHealthTracker.getEffectiveCycleCount(this, osCycles); + final int capacityMah = SystemService.getBatteryCapacity(this); + // Always show the resolved health figure (measured, else cycle-based). - showResolvedHealth(); + showResolvedHealth(cycles, BatteryHealthTracker.isCycleCountFromOs(osCycles), capacityMah); // When the device's charge counter can't be trusted (#94) the figure may be wrong: keep showing // it, but flag it with a tappable warning that explains why (and the failing-battery edge case). - healthWarningIcon.setVisibility(BatteryHealthTracker.isBatteryReadingUnreliable(this) + healthWarningIcon.setVisibility(BatteryHealthTracker.isBatteryReadingUnreliable(this, capacityMah) ? View.VISIBLE : View.GONE); // Metrics and the design-capacity row are shown the same way in every state. - chargeCyclesText.setText(String.valueOf(BatteryHealthTracker.getEffectiveCycleCount(this))); + chargeCyclesText.setText(String.valueOf(cycles)); daysInUseText.setText(String.valueOf(BatteryHealthTracker.getDaysSinceFirstUse(this))); final int designCapacity = BatteryHealthTracker.getDesignCapacity(this); @@ -113,17 +119,22 @@ private void updateHealthData() { /** * Shows the resolved health figure: the measured percentage (current capacity vs. user-entered * design capacity) when available, otherwise the cycle-based estimate. See issue #32 / #7. + * Works entirely from the values the caller already read this refresh (#161). + * + * @param cycles the effective charge cycle count for this refresh + * @param cyclesFromOs whether that count came from the OS (labels the basis honestly, #114) + * @param capacityMah the current full capacity estimate in mAh (0 when unknown) */ - private void showResolvedHealth() { - final int measuredHealth = BatteryHealthTracker.getMeasuredHealthPercentage(this); + private void showResolvedHealth(final int cycles, final boolean cyclesFromOs, final int capacityMah) { + final int measuredHealth = BatteryHealthTracker.getMeasuredHealthPercentage(this, capacityMah); final boolean measured = measuredHealth >= 0; final int healthPercentage = measured ? measuredHealth - : BatteryHealthTracker.getEstimatedHealthPercentage(this); + : BatteryHealthTracker.estimatedHealthForCycles(cycles); final BatteryHealthGrade grade = measured ? BatteryHealthTracker.gradeForPercentage(measuredHealth) - : BatteryHealthTracker.getHealthGrade(this); + : BatteryHealthTracker.gradeForCycles(cycles); // Update health percentage and color it based on grade healthPercentageText.setText(healthPercentage + "%"); @@ -138,7 +149,7 @@ private void showResolvedHealth() { // wear on a phone older than the app (#114). final int basisRes = measured ? R.string.health_basis_measured - : BatteryHealthTracker.isCycleCountFromOs(this) + : cyclesFromOs ? R.string.health_basis_estimated_os : R.string.health_basis_estimated_tracked; healthBasisText.setText(basisRes); diff --git a/app/src/main/java/com/almothafar/simplebatterynotifier/ui/MainActivity.java b/app/src/main/java/com/almothafar/simplebatterynotifier/ui/MainActivity.java index 0b7ae46..96064bf 100644 --- a/app/src/main/java/com/almothafar/simplebatterynotifier/ui/MainActivity.java +++ b/app/src/main/java/com/almothafar/simplebatterynotifier/ui/MainActivity.java @@ -28,7 +28,6 @@ import androidx.annotation.NonNull; import androidx.appcompat.widget.Toolbar; import androidx.core.content.ContextCompat; -import androidx.fragment.app.FragmentManager; import androidx.preference.PreferenceManager; import com.almothafar.simplebatterynotifier.R; import com.almothafar.simplebatterynotifier.model.BatteryDO; @@ -62,9 +61,12 @@ public class MainActivity extends BaseActivity { private ActivityResultLauncher settingsLauncher; private ActivityResultLauncher notificationPermissionLauncher; - // UI elements + // UI elements, looked up once in onCreate — the refresh loop runs every 3 s, so per-tick + // findViewById/findFragmentById traversals are pointless work (#161). private MaterialButton batteryInsightsButton; private RangeSlider thresholdSlider; + private HorseshoeProgressBar batteryGauge; + private BatteryDetailsFragment batteryDetailsFragment; // Self-reposting refresh loop, bound to the foreground lifecycle (started in onPostResume, // stopped in onPause) so it never stacks across resumes or keeps polling in the background. @@ -165,6 +167,9 @@ protected void onCreate(final Bundle savedInstanceState) { // Initialize UI elements batteryInsightsButton = findViewById(R.id.batteryInsightsButton); + batteryGauge = findViewById(R.id.batteryPercentage); + // The details fragment is declared in the layout, so it exists for the activity's lifetime. + batteryDetailsFragment = (BatteryDetailsFragment) getSupportFragmentManager().findFragmentById(R.id.detailsFragmentLayout); // Set up button click listeners batteryInsightsButton.setOnClickListener(v -> openBatteryInsights()); @@ -195,8 +200,7 @@ protected void onPostResume() { // Resume the motion paused in onPause(); restarts only what the battery state still // warrants (charging/discharging wave, full pulse, or critical breathing). - final HorseshoeProgressBar gauge = findViewById(R.id.batteryPercentage); - gauge.resumeAnimations(); + batteryGauge.resumeAnimations(); } /** @@ -210,8 +214,7 @@ protected void onPause() { // Motion is only auto-stopped when the view is destroyed (onDetachedFromWindow), // not on backgrounding, so pause it here for the same reason we stop the timer. - final HorseshoeProgressBar gauge = findViewById(R.id.batteryPercentage); - gauge.pauseAnimations(); + batteryGauge.pauseAnimations(); } /** @@ -267,21 +270,16 @@ private void stopUpdateTimer() { * Runs on the main thread via {@link #handler}. */ private void refreshBatteryUi() { - final HorseshoeProgressBar gauge = findViewById(R.id.batteryPercentage); fillBatteryInfo(); - gauge.setLevel(batteryPercentage); - gauge.setTitle(batteryPercentageText); - gauge.setStatusText(subTitle); + batteryGauge.setLevel(batteryPercentage); + batteryGauge.setTitle(batteryPercentageText); + batteryGauge.setStatusText(subTitle); // Drive the gauge motion: charging wave, full-on-charger idle pulse, or discharge wave. if (nonNull(batteryDO)) { - gauge.setFlow(flowOf(batteryDO.getStatus())); + batteryGauge.setFlow(flowOf(batteryDO.getStatus())); } - final FragmentManager fragmentManager = getSupportFragmentManager(); - final BatteryDetailsFragment batteryDetailsFragment = - (BatteryDetailsFragment) fragmentManager.findFragmentById(R.id.detailsFragmentLayout); - if (nonNull(batteryDetailsFragment) && nonNull(batteryDO)) { batteryDetailsFragment.updateBatteryDetails(batteryDO); } @@ -309,8 +307,7 @@ private void initializeFirstValues() { fillBatteryInfo(); final SharedPreferences sharedPref = PreferenceManager.getDefaultSharedPreferences(this); - final HorseshoeProgressBar gauge = findViewById(R.id.batteryPercentage); - gauge.setThresholds(sharedPref.getInt(getString(R.string._pref_key_critical_battery_level), 20), + batteryGauge.setThresholds(sharedPref.getInt(getString(R.string._pref_key_critical_battery_level), 20), sharedPref.getInt(getString(R.string._pref_key_warn_battery_level), 40)); // Keep the in-fly slider in sync with values that may have changed in Settings. @@ -318,9 +315,9 @@ private void initializeFirstValues() { // The ring still animates whole levels; only the final title carries the decimals (#158). // Intermediate steps count up in whole percent, then the last step lands on the precise text. - gauge.animateLevelTo(batteryPercentage, progress -> { - gauge.setTitle(progress >= batteryPercentage ? batteryPercentageText : BatteryPercentFormatter.formatWhole(progress)); - gauge.setStatusText(subTitle); + batteryGauge.animateLevelTo(batteryPercentage, progress -> { + batteryGauge.setTitle(progress >= batteryPercentage ? batteryPercentageText : BatteryPercentFormatter.formatWhole(progress)); + batteryGauge.setStatusText(subTitle); }); } @@ -367,8 +364,7 @@ public void onStopTrackingTouch(@NonNull final RangeSlider slider) { .putInt(getString(R.string._pref_key_warn_battery_level), warning) .apply(); - final HorseshoeProgressBar gauge = findViewById(R.id.batteryPercentage); - gauge.setThresholds(critical, warning); + batteryGauge.setThresholds(critical, warning); } }); diff --git a/app/src/main/java/com/almothafar/simplebatterynotifier/ui/fragment/BatteryDetailsFragment.java b/app/src/main/java/com/almothafar/simplebatterynotifier/ui/fragment/BatteryDetailsFragment.java index 00d20e3..4229838 100644 --- a/app/src/main/java/com/almothafar/simplebatterynotifier/ui/fragment/BatteryDetailsFragment.java +++ b/app/src/main/java/com/almothafar/simplebatterynotifier/ui/fragment/BatteryDetailsFragment.java @@ -54,19 +54,30 @@ public class BatteryDetailsFragment extends Fragment { // #173: the avg line under the Current value renders at this fraction of the cell's text size. private static final float AVG_LINE_SIZE_RATIO = 0.8f; + // The value TextView is the third cell of every row (label, separator, value). + private static final int VALUE_CELL_INDEX = 2; + private BatteryDO batteryDO; private Map valuesMap; private View viewRef; + // The rows currently in the table, keyed by label (#161): each refresh updates the value cells in + // place and only adds/removes rows whose keys changed, instead of rebuilding the whole table — + // which lost accessibility focus every 3 s and churned allocations. Rebuilt with the view. + private final Map rowViews = new LinkedHashMap<>(); + private TableLayout tableLayout; + private int cellPadding; + private int cellPaddingTop; + // #94: when this device's charge counter can't be trusted, the Capacity row shows "Unknown" with a // tappable info icon instead of a misleading mAh figure. Computed in fillBatteryInfo, applied by - // createDetailsTable to just the capacity row. + // the row binder to just the capacity row. private boolean capacityUnreliable; private String capacityLabel; // #108: the drain-rate row's label, and the colour applied to its value cell (amber near the user's // limit, red at/above it, while discharging; 0 = no special colour). Both set in fillBatteryInfo and - // applied by createDetailsTable to just that row, mirroring the capacity special-case above. + // applied by the row binder to just that row, mirroring the capacity special-case above. private String rateLabel; private int rateValueColor; @@ -94,11 +105,13 @@ public View onCreateView(final LayoutInflater inflater, this.viewRef = view; batteryDO = SystemService.getBatteryInfo(view.getContext()); + setupTable(view); + // CRITICAL: Check for null batteryDO if (isNull(batteryDO)) { Log.w(TAG, "Unable to retrieve battery information"); } else { - createDetailsTable(view); + refreshDetailsTable(view); } maybeShowScrollHint(view); @@ -107,15 +120,14 @@ public View onCreateView(final LayoutInflater inflater, } /** - * Create and populate the battery details table + * One-time table setup for a freshly inflated view: column behaviour, cached paddings, and a clean + * row registry (the old view's rows are gone with it). * * @param view The fragment view containing the table */ - public void createDetailsTable(final View view) { - fillBatteryInfo(view); - - final TableLayout tableLayout = view.findViewById(R.id.batteryDetailsTable); - tableLayout.removeAllViews(); + private void setupTable(final View view) { + tableLayout = view.findViewById(R.id.batteryDetailsTable); + rowViews.clear(); // Colon-aligned rows with the divider near the horizontal centre (#96): stretch BOTH the label (0) // and value (2) columns so they share the width evenly, and the end-aligned labels' colons line up // around the middle in both LTR and RTL (rather than wherever the widest label happens to end). @@ -126,21 +138,96 @@ public void createDetailsTable(final View view) { tableLayout.setColumnStretchable(2, true); tableLayout.setColumnShrinkable(0, true); tableLayout.setColumnShrinkable(2, true); - final int cellPadding = getResources().getDimensionPixelSize(R.dimen.battery_details_cell_padding); - final int cellPaddingTop = getResources().getDimensionPixelSize(R.dimen.battery_details_cell_padding_top); - - int rowIndex = 0; - for (final String key : valuesMap.keySet()) { - // Only the capacity row gets the "unreliable reading" info affordance (#94). - final boolean unreliable = capacityUnreliable && key.equals(capacityLabel); - // Only the drain-rate row is coloured (amber/red near the limit while discharging) — #108. - final int valueColor = (rateValueColor != 0 && key.equals(rateLabel)) ? rateValueColor : 0; - final TableRow row = createTableRow(view, key, valuesMap.get(key), cellPadding, cellPaddingTop, unreliable, valueColor); - tableLayout.addView(row, rowIndex); - rowIndex++; + cellPadding = getResources().getDimensionPixelSize(R.dimen.battery_details_cell_padding); + cellPaddingTop = getResources().getDimensionPixelSize(R.dimen.battery_details_cell_padding_top); + } + + /** + * Refresh the battery details table with the current {@code batteryDO}. + *

+ * Steady-state refreshes (every 3 s while the screen is open) update the existing rows' value + * cells in place; rows are only created or removed when their key genuinely appears or vanishes + * (rate warm-up, time-to-full gating, capacity turning unknown). This keeps accessibility focus, + * scroll position and the view tree stable across refreshes (#161). + * + * @param view The fragment view containing the table + */ + private void refreshDetailsTable(final View view) { + fillBatteryInfo(view); + + syncRows(tableLayout, rowViews, valuesMap, new RowBinder() { + @Override + public TableRow createRow(final String label, final CharSequence value) { + return createTableRow(view, label, value, cellPadding, cellPaddingTop, isUnreliableRow(label), valueColorFor(label)); + } + + @Override + public void bindValue(final TableRow row, final String label, final CharSequence value) { + final TextView valueView = (TextView) row.getChildAt(VALUE_CELL_INDEX); + valueView.setText(value); + applyValueDecorations(valueView, isUnreliableRow(label), valueColorFor(label)); + } + }); + } + + /** Only the capacity row gets the "unreliable reading" info affordance (#94). */ + private boolean isUnreliableRow(final String label) { + return capacityUnreliable && label.equals(capacityLabel); + } + + /** Only the drain-rate row is coloured (amber/red near the limit while discharging) — #108. */ + private int valueColorFor(final String label) { + return (rateValueColor != 0 && label.equals(rateLabel)) ? rateValueColor : 0; + } + + /** + * Bring the table's rows in line with the desired label→value map: bind existing rows' value + * cells in place, remove rows whose label vanished, and insert new rows at their position. + * Relative order of surviving rows never changes (rows only appear/disappear), so inserting at + * the walk index keeps every row where it belongs. Static and binder-driven so the diffing is + * unit-testable without a fragment (#161). + * + * @param tableLayout the table being updated + * @param rowViews the rows currently in the table, keyed by label — updated in place + * @param values the desired rows for this refresh, in display order + * @param binder creates a full row for a new label / rebinds the value cell of an existing one + */ + static void syncRows(final TableLayout tableLayout, final Map rowViews, + final Map values, final RowBinder binder) { + // Drop rows whose label is gone this refresh. + rowViews.entrySet().removeIf(entry -> { + if (values.containsKey(entry.getKey())) { + return false; + } + tableLayout.removeView(entry.getValue()); + return true; + }); + + int index = 0; + for (final Map.Entry entry : values.entrySet()) { + TableRow row = rowViews.get(entry.getKey()); + if (isNull(row)) { + row = binder.createRow(entry.getKey(), entry.getValue()); + tableLayout.addView(row, index); + rowViews.put(entry.getKey(), row); + } else { + binder.bindValue(row, entry.getKey(), entry.getValue()); + } + index++; } } + /** + * How {@link #syncRows} materializes rows: create a complete row for a label that just appeared, + * or rebind the value cell of a row that persists across refreshes. + */ + interface RowBinder { + + TableRow createRow(String label, CharSequence value); + + void bindValue(TableRow row, String label, CharSequence value); + } + /** * Update the battery details with new data * @@ -149,7 +236,7 @@ public void createDetailsTable(final View view) { public void updateBatteryDetails(final BatteryDO batteryDO) { this.batteryDO = batteryDO; if (nonNull(viewRef) && nonNull(batteryDO)) { - this.createDetailsTable(viewRef); + refreshDetailsTable(viewRef); } } @@ -305,14 +392,28 @@ private TextView createValueTextView(final View view, final TextView textView = new TextView(view.getContext()); textView.setTextAppearance(R.style.DefaultTextStyle); textView.setText(text); - textView.setTextColor(valueColor != 0 - ? valueColor - : GeneralHelper.getColor(getResources(), R.color.battery_details_value_color)); // Start-align + relative padding + locale text direction so numeric (Latin) values sit right after // the colon like the Arabic text values do, instead of flying to the far edge in RTL (#96). textView.setGravity(Gravity.START); textView.setTextDirection(View.TEXT_DIRECTION_LOCALE); textView.setPaddingRelative(cellPadding, 0, 0, cellPaddingTop); + applyValueDecorations(textView, unreliable, valueColor); + return textView; + } + + /** + * (Re-)apply the per-refresh decorations of a value cell: the rate colour (#108) and the capacity + * row's tappable "unreliable reading" affordance (#94). Called at creation and on every in-place + * rebind (#161), so it also has to clear both when the row's state changes back. + * + * @param textView the value cell + * @param unreliable when true, decorate with the tappable amber info icon; when false, remove it + * @param valueColor colour for the value text, or 0 for the default value colour + */ + private void applyValueDecorations(final TextView textView, final boolean unreliable, final int valueColor) { + textView.setTextColor(valueColor != 0 + ? valueColor + : GeneralHelper.getColor(getResources(), R.color.battery_details_value_color)); if (unreliable) { // #94: amber warning affordance after the value; tap to learn why the reading can't be trusted. // Same mark as the insights health figure, so the two screens read consistently. @@ -320,8 +421,12 @@ private TextView createValueTextView(final View view, textView.setCompoundDrawablePadding((int) (6 * getResources().getDisplayMetrics().density)); textView.setContentDescription(getString(R.string.battery_reading_unreliable_cd)); textView.setOnClickListener(v -> showCapacityUnreliableDialog()); + } else { + textView.setCompoundDrawablesRelativeWithIntrinsicBounds(0, 0, 0, 0); + textView.setContentDescription(null); + textView.setOnClickListener(null); + textView.setClickable(false); } - return textView; } /** @@ -341,9 +446,10 @@ private void fillBatteryInfo(final View view) { // Capacity is an estimate from BatteryManager. Show "Unknown" rather than "0 mAh" when the device // doesn't report the charge counter, and also when the reported figure can't be trusted (#94) — - // in that case createDetailsTable adds a tappable info icon explaining why. + // in that case the row binder adds a tappable info icon explaining why. The snapshot's capacity + // is passed down so no second capacity estimate is read this tick (#161). final int capacity = batteryDO.getCapacity(); - capacityUnreliable = BatteryHealthTracker.isBatteryReadingUnreliable(view.getContext()); + capacityUnreliable = BatteryHealthTracker.isBatteryReadingUnreliable(view.getContext(), capacity); final String capacityText = (capacity > 0 && !capacityUnreliable) ? capacity + " mAh" : getResources().getString(R.string.unknown); @@ -358,8 +464,9 @@ private void fillBatteryInfo(final View view) { getResources().getString(R.string.design_capacity_value, String.valueOf(designCapacity))); } - // Add charge cycles from the battery health tracker - positioned right after capacity - final int chargeCycles = BatteryHealthTracker.getEffectiveCycleCount(view.getContext()); + // Add charge cycles from the battery health tracker - positioned right after capacity. The + // snapshot already carries the OS cycle count, so this triggers no extra sticky read (#161). + final int chargeCycles = BatteryHealthTracker.getEffectiveCycleCount(view.getContext(), batteryDO.getCycleCount()); valuesMap.put(getResources().getString(R.string.charge_cycles), String.valueOf(chargeCycles)); valuesMap.put(getResources().getString(R.string.voltage), batteryDO.getVoltage() + " mV"); diff --git a/app/src/test/java/com/almothafar/simplebatterynotifier/service/BatteryHealthTrackerStateTest.java b/app/src/test/java/com/almothafar/simplebatterynotifier/service/BatteryHealthTrackerStateTest.java index b82e864..910d2f9 100644 --- a/app/src/test/java/com/almothafar/simplebatterynotifier/service/BatteryHealthTrackerStateTest.java +++ b/app/src/test/java/com/almothafar/simplebatterynotifier/service/BatteryHealthTrackerStateTest.java @@ -104,8 +104,8 @@ public void designCapacity_persistsAndClears() { @Test public void measuredHealth_requiresDesignCapacity() { - // Without a design capacity there's nothing to measure against - assertEquals(-1, BatteryHealthTracker.getMeasuredHealthPercentage(context)); + // Without a design capacity there's nothing to measure against, whatever capacity was read + assertEquals(-1, BatteryHealthTracker.getMeasuredHealthPercentage(context, 4400)); } @Test diff --git a/app/src/test/java/com/almothafar/simplebatterynotifier/service/BatteryHealthTrackerTest.java b/app/src/test/java/com/almothafar/simplebatterynotifier/service/BatteryHealthTrackerTest.java index 1e61a05..96362b1 100644 --- a/app/src/test/java/com/almothafar/simplebatterynotifier/service/BatteryHealthTrackerTest.java +++ b/app/src/test/java/com/almothafar/simplebatterynotifier/service/BatteryHealthTrackerTest.java @@ -20,6 +20,68 @@ @RunWith(Enclosed.class) public class BatteryHealthTrackerTest { + /** + * {@link BatteryHealthTracker#estimatedHealthForCycles(int)} — the cycle-based degradation curve + * (#161 extracted it as a pure core so one cycle read serves every figure on a screen). + */ + @RunWith(Parameterized.class) + public static class EstimatedHealthForCycles { + + @Parameter(0) public String label; + @Parameter(1) public int cycles; + @Parameter(2) public int expected; + + @Parameters(name = "{0}") + public static Collection data() { + return Arrays.asList(new Object[][]{ + {"new battery -> 100%", 0, 100}, + {"mid-excellent 150 -> 98%", 150, 98}, + {"excellent boundary 300 -> 95%", 300, 95}, + {"mid-good 400 -> 90%", 400, 90}, + {"good boundary 500 -> 85%", 500, 85}, + {"mid-fair 650 -> 78%", 650, 78}, + {"fair boundary 800 -> 70%", 800, 70}, + {"deep poor 1300 -> 40%", 1300, 40}, + {"floor holds at 40%", 2000, 40}, + }); + } + + @Test + public void matchesExpected() { + assertEquals(label, expected, BatteryHealthTracker.estimatedHealthForCycles(cycles)); + } + } + + /** + * {@link BatteryHealthTracker#gradeForCycles(int)} — the cycle-count bucket boundaries, which must + * stay consistent with the {@link BatteryHealthTracker#estimatedHealthForCycles(int)} curve. + */ + @RunWith(Parameterized.class) + public static class GradeForCycles { + + @Parameter(0) public int cycles; + @Parameter(1) public BatteryHealthGrade expected; + + @Parameters(name = "{0} cycles -> {1}") + public static Collection data() { + return Arrays.asList(new Object[][]{ + {0, BatteryHealthGrade.EXCELLENT}, + {299, BatteryHealthGrade.EXCELLENT}, + {300, BatteryHealthGrade.GOOD}, + {499, BatteryHealthGrade.GOOD}, + {500, BatteryHealthGrade.FAIR}, + {799, BatteryHealthGrade.FAIR}, + {800, BatteryHealthGrade.POOR}, + {2000, BatteryHealthGrade.POOR}, + }); + } + + @Test + public void matchesExpected() { + assertEquals(expected, BatteryHealthTracker.gradeForCycles(cycles)); + } + } + /** * {@link BatteryHealthTracker#computeMeasuredHealth(int, int)} — measured capacity as a percentage * of design capacity, clamped to 1..100, or -1 when the inputs are unusable. diff --git a/app/src/test/java/com/almothafar/simplebatterynotifier/service/SystemServiceTest.java b/app/src/test/java/com/almothafar/simplebatterynotifier/service/SystemServiceTest.java index e32398e..fbf8e2a 100644 --- a/app/src/test/java/com/almothafar/simplebatterynotifier/service/SystemServiceTest.java +++ b/app/src/test/java/com/almothafar/simplebatterynotifier/service/SystemServiceTest.java @@ -1,11 +1,20 @@ package com.almothafar.simplebatterynotifier.service; +import android.content.Context; +import android.content.Intent; +import android.os.BatteryManager; + +import androidx.test.core.app.ApplicationProvider; + +import org.junit.Before; import org.junit.Test; import org.junit.experimental.runners.Enclosed; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameter; import org.junit.runners.Parameterized.Parameters; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; import java.util.Arrays; import java.util.Collection; @@ -13,11 +22,56 @@ import static org.junit.Assert.assertEquals; /** - * Unit tests for the pure capacity-estimation helpers in {@link SystemService}. + * Unit tests for the pure capacity-estimation helpers in {@link SystemService}, and the snapshot's + * cycle-count extraction (#161). */ @RunWith(Enclosed.class) public class SystemServiceTest { + /** + * The OS cycle count rides on the {@code BatteryDO} snapshot (#161), extracted from the same + * {@code ACTION_BATTERY_CHANGED} intent as everything else — normalized to -1 when absent or + * non-positive, matching {@link SystemService#getChargeCycleCount}, so per-tick consumers never + * need a second sticky-broadcast read. + */ + @RunWith(RobolectricTestRunner.class) + @Config(sdk = 34) + public static class SnapshotCycleCount { + + private Context context; + + @Before + public void setUp() { + context = ApplicationProvider.getApplicationContext(); + } + + @Test + public void reportedCycleCount_carriesOntoTheSnapshot() { + assertEquals(287, SystemService.getBatteryInfo(context, batteryIntent(287)).getCycleCount()); + } + + @Test + public void absentCycleCount_readsAsMinusOne() { + final Intent battery = new Intent(Intent.ACTION_BATTERY_CHANGED); + battery.putExtra(BatteryManager.EXTRA_LEVEL, 50); + battery.putExtra(BatteryManager.EXTRA_SCALE, 100); + assertEquals(-1, SystemService.getBatteryInfo(context, battery).getCycleCount()); + } + + @Test + public void nonPositiveCycleCount_normalizedToMinusOne() { + assertEquals(-1, SystemService.getBatteryInfo(context, batteryIntent(0)).getCycleCount()); + } + + private static Intent batteryIntent(final int cycleCount) { + final Intent battery = new Intent(Intent.ACTION_BATTERY_CHANGED); + battery.putExtra(BatteryManager.EXTRA_LEVEL, 50); + battery.putExtra(BatteryManager.EXTRA_SCALE, 100); + battery.putExtra(BatteryManager.EXTRA_CYCLE_COUNT, cycleCount); + return battery; + } + } + /** * {@link SystemService#estimateFullCapacityMah(int, int)} — derives full capacity (mAh) from a * CHARGE_COUNTER reading (µAh) at a given charge level, rejecting implausible results as 0. diff --git a/app/src/test/java/com/almothafar/simplebatterynotifier/ui/fragment/BatteryDetailsRowSyncTest.java b/app/src/test/java/com/almothafar/simplebatterynotifier/ui/fragment/BatteryDetailsRowSyncTest.java new file mode 100644 index 0000000..7922472 --- /dev/null +++ b/app/src/test/java/com/almothafar/simplebatterynotifier/ui/fragment/BatteryDetailsRowSyncTest.java @@ -0,0 +1,132 @@ +package com.almothafar.simplebatterynotifier.ui.fragment; + +import android.content.Context; +import android.widget.TableLayout; +import android.widget.TableRow; +import android.widget.TextView; + +import androidx.test.core.app.ApplicationProvider; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; + +import java.util.LinkedHashMap; +import java.util.Map; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; + +/** + * Robolectric tests for {@link BatteryDetailsFragment#syncRows} (issue #161): steady-state + * refreshes must update existing rows in place — never recreate them, which reset accessibility + * focus and scroll every 3 s — while rows whose key appears or vanishes (rate warm-up, + * time-to-full gating) are inserted or removed at the right position. + */ +@RunWith(RobolectricTestRunner.class) +@Config(sdk = 34) +public class BatteryDetailsRowSyncTest { + + private Context context; + private TableLayout table; + private Map rowViews; + private BatteryDetailsFragment.RowBinder binder; + + @Before + public void setUp() { + context = ApplicationProvider.getApplicationContext(); + table = new TableLayout(context); + rowViews = new LinkedHashMap<>(); + // Minimal binder: one TextView per row carrying the value, updated in place on rebind. + binder = new BatteryDetailsFragment.RowBinder() { + @Override + public TableRow createRow(final String label, final CharSequence value) { + final TableRow row = new TableRow(context); + final TextView valueView = new TextView(context); + valueView.setText(value); + row.addView(valueView); + return row; + } + + @Override + public void bindValue(final TableRow row, final String label, final CharSequence value) { + ((TextView) row.getChildAt(0)).setText(value); + } + }; + } + + @Test + public void steadyState_updatesValuesWithoutRecreatingRows() { + BatteryDetailsFragment.syncRows(table, rowViews, values("Voltage", "3900 mV", "Temperature", "25 °C"), binder); + final TableRow voltageRow = rowViews.get("Voltage"); + final TableRow temperatureRow = rowViews.get("Temperature"); + + BatteryDetailsFragment.syncRows(table, rowViews, values("Voltage", "3850 mV", "Temperature", "26 °C"), binder); + + // Same view instances (a11y focus survives), fresh values, no row growth. + assertSame(voltageRow, table.getChildAt(0)); + assertSame(temperatureRow, table.getChildAt(1)); + assertEquals(2, table.getChildCount()); + assertEquals("3850 mV", valueAt(0)); + assertEquals("26 °C", valueAt(1)); + } + + @Test + public void vanishedKey_removesOnlyItsRow() { + BatteryDetailsFragment.syncRows(table, rowViews, values("Rate", "9%/h", "Voltage", "3900 mV", "Temperature", "25 °C"), binder); + final TableRow voltageRow = rowViews.get("Voltage"); + final TableRow temperatureRow = rowViews.get("Temperature"); + + // The rate row hides (e.g. static level); its neighbours must survive untouched. + BatteryDetailsFragment.syncRows(table, rowViews, values("Voltage", "3900 mV", "Temperature", "25 °C"), binder); + + assertEquals(2, table.getChildCount()); + assertSame(voltageRow, table.getChildAt(0)); + assertSame(temperatureRow, table.getChildAt(1)); + } + + @Test + public void appearingKey_isInsertedAtItsPosition() { + BatteryDetailsFragment.syncRows(table, rowViews, values("Voltage", "3900 mV", "Temperature", "25 °C"), binder); + final TableRow voltageRow = rowViews.get("Voltage"); + final TableRow temperatureRow = rowViews.get("Temperature"); + + // The rate row appears at the TOP after warm-up; existing rows shift down but stay the same views. + BatteryDetailsFragment.syncRows(table, rowViews, values("Rate", "9%/h", "Voltage", "3900 mV", "Temperature", "25 °C"), binder); + + assertEquals(3, table.getChildCount()); + assertSame(rowViews.get("Rate"), table.getChildAt(0)); + assertSame(voltageRow, table.getChildAt(1)); + assertSame(temperatureRow, table.getChildAt(2)); + assertEquals("9%/h", valueAt(0)); + } + + @Test + public void midTableInsertion_landsBetweenItsNeighbours() { + BatteryDetailsFragment.syncRows(table, rowViews, values("Rate", "9%/h", "Temperature", "25 °C"), binder); + + // Time-to-full gates open mid-table (#124): it must land between rate and temperature. + BatteryDetailsFragment.syncRows(table, rowViews, values("Rate", "9%/h", "Time to full", "~1h 20m", "Temperature", "25 °C"), binder); + + assertEquals(3, table.getChildCount()); + assertSame(rowViews.get("Time to full"), table.getChildAt(1)); + assertEquals("~1h 20m", valueAt(1)); + } + + // --- helpers ------------------------------------------------------------- + + private static Map values(final CharSequence... labelValuePairs) { + final Map map = new LinkedHashMap<>(); + for (int i = 0; i < labelValuePairs.length; i += 2) { + map.put(labelValuePairs[i].toString(), labelValuePairs[i + 1]); + } + return map; + } + + private String valueAt(final int rowIndex) { + final TableRow row = (TableRow) table.getChildAt(rowIndex); + return ((TextView) row.getChildAt(0)).getText().toString(); + } +}