From c1e652088be61794c023913e2d96eddd65379e04 Mon Sep 17 00:00:00 2001 From: Al-Mothafar Al-Hasan Date: Sun, 19 Jul 2026 12:17:50 +0300 Subject: [PATCH 1/3] DRY: typed AppPrefs facade owns the critical/warning key + default (#162) Introduce AppPrefs as the single owner of the critical/warning battery-level key + default. The 20/40 literals previously restated in NotificationService, BatteryLevelReceiver, MainActivity and BatteryRangeSliderHelper now live only in AppPrefs.DEFAULT_CRITICAL_LEVEL / DEFAULT_WARNING_LEVEL, so the alert engine, receiver and both sliders can no longer disagree. - Migrate the read sites to AppPrefs.criticalLevel/warningLevel(context). - Route the two slider write paths' shared pair through AppPrefs.setBatteryLevels. - Remove BatteryRangeSliderHelper.DEFAULT_CRITICAL/DEFAULT_WARNING; the slider preference now references the facade constants, and pref_alerts.xml carries a comment tying its XML-declared defaults to them. - Drop MainActivity's now-unused SharedPreferences/PreferenceManager imports. - Add Robolectric coverage for the facade's defaults, read-back and write. Co-Authored-By: Claude Opus 4.8 --- .../receiver/BatteryLevelReceiver.java | 5 +- .../service/NotificationService.java | 5 +- .../ui/MainActivity.java | 19 ++--- .../preference/BatteryRangeSliderHelper.java | 4 - .../BatteryRangeSliderPreference.java | 5 +- .../simplebatterynotifier/util/AppPrefs.java | 78 +++++++++++++++++++ app/src/main/res/xml/pref_alerts.xml | 4 + .../util/AppPrefsTest.java | 69 ++++++++++++++++ 8 files changed, 165 insertions(+), 24 deletions(-) create mode 100644 app/src/main/java/com/almothafar/simplebatterynotifier/util/AppPrefs.java create mode 100644 app/src/test/java/com/almothafar/simplebatterynotifier/util/AppPrefsTest.java diff --git a/app/src/main/java/com/almothafar/simplebatterynotifier/receiver/BatteryLevelReceiver.java b/app/src/main/java/com/almothafar/simplebatterynotifier/receiver/BatteryLevelReceiver.java index 8a71690..4d339fa 100644 --- a/app/src/main/java/com/almothafar/simplebatterynotifier/receiver/BatteryLevelReceiver.java +++ b/app/src/main/java/com/almothafar/simplebatterynotifier/receiver/BatteryLevelReceiver.java @@ -15,6 +15,7 @@ import com.almothafar.simplebatterynotifier.service.NotificationService; import com.almothafar.simplebatterynotifier.service.SlowChargeDetector; import com.almothafar.simplebatterynotifier.service.SystemService; +import com.almothafar.simplebatterynotifier.util.AppPrefs; import com.almothafar.simplebatterynotifier.util.TemperatureUtils; import static java.util.Objects.isNull; @@ -104,8 +105,8 @@ public void onReceive(final Context context, final Intent intent) { final SharedPreferences sharedPref = PreferenceManager.getDefaultSharedPreferences(context); final LevelAlertConfig config = new LevelAlertConfig( - sharedPref.getInt(context.getString(R.string._pref_key_critical_battery_level), 20), - sharedPref.getInt(context.getString(R.string._pref_key_warn_battery_level), 40), + AppPrefs.criticalLevel(context), + AppPrefs.warningLevel(context), sharedPref.getBoolean(context.getString(R.string._pref_key_notify_for_warning_level), true), sharedPref.getBoolean(context.getString(R.string._pref_key_notify_for_full_level), true), sharedPref.getBoolean(context.getString(R.string._pref_key_notify_every_tick), false)); diff --git a/app/src/main/java/com/almothafar/simplebatterynotifier/service/NotificationService.java b/app/src/main/java/com/almothafar/simplebatterynotifier/service/NotificationService.java index 94a9569..6ce3485 100644 --- a/app/src/main/java/com/almothafar/simplebatterynotifier/service/NotificationService.java +++ b/app/src/main/java/com/almothafar/simplebatterynotifier/service/NotificationService.java @@ -31,6 +31,7 @@ import com.almothafar.simplebatterynotifier.model.ChargeSpeed; import com.almothafar.simplebatterynotifier.model.ChargeSpeedTier; import com.almothafar.simplebatterynotifier.ui.MainActivity; +import com.almothafar.simplebatterynotifier.util.AppPrefs; import com.almothafar.simplebatterynotifier.util.BatteryPercentFormatter; import com.almothafar.simplebatterynotifier.util.GeneralHelper; import com.almothafar.simplebatterynotifier.util.TemperatureUtils; @@ -1407,8 +1408,8 @@ private static class NotificationConfig { this.type = type; // Load common preferences - final int warningLevel = prefs.getInt(context.getString(R.string._pref_key_warn_battery_level), 40); - final int criticalLevel = prefs.getInt(context.getString(R.string._pref_key_critical_battery_level), 20); + final int warningLevel = AppPrefs.warningLevel(context); + final int criticalLevel = AppPrefs.criticalLevel(context); this.stickyNotification = prefs.getBoolean(context.getString(R.string._pref_key_notifications_sticky), false); final boolean withinWindow = isWithinNotificationWindow(context, prefs); 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 96064bf..a439ae8 100644 --- a/app/src/main/java/com/almothafar/simplebatterynotifier/ui/MainActivity.java +++ b/app/src/main/java/com/almothafar/simplebatterynotifier/ui/MainActivity.java @@ -3,7 +3,6 @@ import android.Manifest; import android.content.ActivityNotFoundException; import android.content.Intent; -import android.content.SharedPreferences; import android.content.pm.PackageManager; import android.net.Uri; import android.os.BatteryManager; @@ -28,13 +27,13 @@ import androidx.annotation.NonNull; import androidx.appcompat.widget.Toolbar; import androidx.core.content.ContextCompat; -import androidx.preference.PreferenceManager; import com.almothafar.simplebatterynotifier.R; import com.almothafar.simplebatterynotifier.model.BatteryDO; import com.almothafar.simplebatterynotifier.service.BatteryHealthTracker; import com.almothafar.simplebatterynotifier.service.PowerConnectionService; import com.almothafar.simplebatterynotifier.service.SystemService; import com.almothafar.simplebatterynotifier.ui.widget.HorseshoeProgressBar; +import com.almothafar.simplebatterynotifier.util.AppPrefs; import com.almothafar.simplebatterynotifier.util.BatteryPercentFormatter; import java.util.List; @@ -305,10 +304,8 @@ private static HorseshoeProgressBar.Flow flowOf(final int batteryStatus) { */ private void initializeFirstValues() { fillBatteryInfo(); - final SharedPreferences sharedPref = PreferenceManager.getDefaultSharedPreferences(this); - batteryGauge.setThresholds(sharedPref.getInt(getString(R.string._pref_key_critical_battery_level), 20), - sharedPref.getInt(getString(R.string._pref_key_warn_battery_level), 40)); + batteryGauge.setThresholds(AppPrefs.criticalLevel(this), AppPrefs.warningLevel(this)); // Keep the in-fly slider in sync with values that may have changed in Settings. syncThresholdSlider(); @@ -359,10 +356,7 @@ public void onStopTrackingTouch(@NonNull final RangeSlider slider) { final int critical = Math.round(values.get(0)); final int warning = Math.round(values.get(1)); - PreferenceManager.getDefaultSharedPreferences(MainActivity.this).edit() - .putInt(getString(R.string._pref_key_critical_battery_level), critical) - .putInt(getString(R.string._pref_key_warn_battery_level), warning) - .apply(); + AppPrefs.setBatteryLevels(MainActivity.this, critical, warning); batteryGauge.setThresholds(critical, warning); } @@ -379,11 +373,8 @@ private void syncThresholdSlider() { if (isNull(thresholdSlider)) { return; } - final SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(this); - final int critical = prefs.getInt(getString(R.string._pref_key_critical_battery_level), - BatteryRangeSliderHelper.DEFAULT_CRITICAL); - final int warning = prefs.getInt(getString(R.string._pref_key_warn_battery_level), - BatteryRangeSliderHelper.DEFAULT_WARNING); + final int critical = AppPrefs.criticalLevel(this); + final int warning = AppPrefs.warningLevel(this); final int[] pair = BatteryRangeSliderHelper.clampPair(critical, warning, BatteryRangeSliderHelper.LEVEL_FROM, BatteryRangeSliderHelper.LEVEL_TO, BatteryRangeSliderHelper.MIN_SEPARATION); diff --git a/app/src/main/java/com/almothafar/simplebatterynotifier/ui/preference/BatteryRangeSliderHelper.java b/app/src/main/java/com/almothafar/simplebatterynotifier/ui/preference/BatteryRangeSliderHelper.java index 7312e14..85cf9b7 100644 --- a/app/src/main/java/com/almothafar/simplebatterynotifier/ui/preference/BatteryRangeSliderHelper.java +++ b/app/src/main/java/com/almothafar/simplebatterynotifier/ui/preference/BatteryRangeSliderHelper.java @@ -20,10 +20,6 @@ public final class BatteryRangeSliderHelper { public static final int LEVEL_TO = 50; /** Minimum gap kept between the critical and warning thumbs, in percent. */ public static final int MIN_SEPARATION = 5; - /** Default critical level, matching the historical {@code SeekBarPreference} default. */ - public static final int DEFAULT_CRITICAL = 20; - /** Default warning level, matching the historical {@code SeekBarPreference} default. */ - public static final int DEFAULT_WARNING = 40; private BatteryRangeSliderHelper() { // Utility class diff --git a/app/src/main/java/com/almothafar/simplebatterynotifier/ui/preference/BatteryRangeSliderPreference.java b/app/src/main/java/com/almothafar/simplebatterynotifier/ui/preference/BatteryRangeSliderPreference.java index 6139da7..e98eb88 100644 --- a/app/src/main/java/com/almothafar/simplebatterynotifier/ui/preference/BatteryRangeSliderPreference.java +++ b/app/src/main/java/com/almothafar/simplebatterynotifier/ui/preference/BatteryRangeSliderPreference.java @@ -11,6 +11,7 @@ import androidx.preference.PreferenceViewHolder; import com.almothafar.simplebatterynotifier.R; +import com.almothafar.simplebatterynotifier.util.AppPrefs; import com.google.android.material.slider.RangeSlider; import java.util.List; @@ -37,8 +38,8 @@ public class BatteryRangeSliderPreference extends Preference { private int from = BatteryRangeSliderHelper.LEVEL_FROM; private int to = BatteryRangeSliderHelper.LEVEL_TO; private int minSeparation = BatteryRangeSliderHelper.MIN_SEPARATION; - private int criticalDefault = BatteryRangeSliderHelper.DEFAULT_CRITICAL; - private int warningDefault = BatteryRangeSliderHelper.DEFAULT_WARNING; + private int criticalDefault = AppPrefs.DEFAULT_CRITICAL_LEVEL; + private int warningDefault = AppPrefs.DEFAULT_WARNING_LEVEL; public BatteryRangeSliderPreference(final Context context, final AttributeSet attrs, final int defStyleAttr, final int defStyleRes) { diff --git a/app/src/main/java/com/almothafar/simplebatterynotifier/util/AppPrefs.java b/app/src/main/java/com/almothafar/simplebatterynotifier/util/AppPrefs.java new file mode 100644 index 0000000..6c8b279 --- /dev/null +++ b/app/src/main/java/com/almothafar/simplebatterynotifier/util/AppPrefs.java @@ -0,0 +1,78 @@ +package com.almothafar.simplebatterynotifier.util; + +import android.content.Context; +import android.content.SharedPreferences; + +import androidx.preference.PreferenceManager; + +import com.almothafar.simplebatterynotifier.R; + +/** + * Typed facade over the app's default {@link SharedPreferences} (#162): the single owner of each + * migrated setting's key + default (and clamp, as more settings move here). Read sites call the + * typed accessor instead of repeating {@code getInt(getString(R.string._pref_key_…), )}, + * so a default can never silently drift between the alert engine, the receiver and the UI. + *

+ * Migrated so far: the critical/warning battery levels. The same {@code 20}/{@code 40} literals + * previously lived in {@code NotificationService}, {@code BatteryLevelReceiver}, {@code MainActivity} + * and the range slider's helper; they now live only in {@link #DEFAULT_CRITICAL_LEVEL} / + * {@link #DEFAULT_WARNING_LEVEL} here. Remaining settings migrate incrementally. + *

+ * The XML-declared defaults in {@code pref_alerts.xml} ({@code criticalDefault}/{@code warningDefault}) + * must stay in step with these constants — the framework instantiates the slider from XML, so the two + * cannot literally share a constant, but they describe the same value. + */ +public final class AppPrefs { + + /** Default critical battery level in percent — the single owner of this value (#162). */ + public static final int DEFAULT_CRITICAL_LEVEL = 20; + /** Default warning battery level in percent — the single owner of this value (#162). */ + public static final int DEFAULT_WARNING_LEVEL = 40; + + private AppPrefs() { + // Utility class + } + + /** + * The critical battery level in percent: the level at/below which the critical alert fires and the + * home gauge turns red. Falls back to {@link #DEFAULT_CRITICAL_LEVEL} when unset. + * + * @param context Application context + * + * @return the configured critical level + */ + public static int criticalLevel(final Context context) { + return prefs(context).getInt(context.getString(R.string._pref_key_critical_battery_level), DEFAULT_CRITICAL_LEVEL); + } + + /** + * The warning battery level in percent: the level at/below which the warning alert fires and the + * home gauge turns amber. Falls back to {@link #DEFAULT_WARNING_LEVEL} when unset. + * + * @param context Application context + * + * @return the configured warning level + */ + public static int warningLevel(final Context context) { + return prefs(context).getInt(context.getString(R.string._pref_key_warn_battery_level), DEFAULT_WARNING_LEVEL); + } + + /** + * Persist the critical and warning battery levels together. The settings-screen slider and the + * home-screen in-fly slider both write this pair, so the write lives here beside the matching reads. + * + * @param context Application context + * @param critical the critical level to store, in percent + * @param warning the warning level to store, in percent + */ + public static void setBatteryLevels(final Context context, final int critical, final int warning) { + prefs(context).edit() + .putInt(context.getString(R.string._pref_key_critical_battery_level), critical) + .putInt(context.getString(R.string._pref_key_warn_battery_level), warning) + .apply(); + } + + private static SharedPreferences prefs(final Context context) { + return PreferenceManager.getDefaultSharedPreferences(context); + } +} diff --git a/app/src/main/res/xml/pref_alerts.xml b/app/src/main/res/xml/pref_alerts.xml index 5212863..eb02216 100644 --- a/app/src/main/res/xml/pref_alerts.xml +++ b/app/src/main/res/xml/pref_alerts.xml @@ -11,6 +11,10 @@ android:title="@string/pref_cat_title_thresholds" app:iconSpaceReserved="false"> + Date: Sun, 19 Jul 2026 12:29:57 +0300 Subject: [PATCH 2/3] Docs: soften AppPrefs "single owner" wording re: the XML slider default (#162) Code review flagged the class Javadoc's "the literals now live only ... here" as overstating it: pref_alerts.xml still declares the slider's XML default (20/40), which the framework cannot share as a constant. Reword to say every Java read derives its default from the constants, and name the XML restatement as the one remaining (already comment-tied) exception, so the doc matches reality. Co-Authored-By: Claude Opus 4.8 --- .../com/almothafar/simplebatterynotifier/util/AppPrefs.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/almothafar/simplebatterynotifier/util/AppPrefs.java b/app/src/main/java/com/almothafar/simplebatterynotifier/util/AppPrefs.java index 6c8b279..224427b 100644 --- a/app/src/main/java/com/almothafar/simplebatterynotifier/util/AppPrefs.java +++ b/app/src/main/java/com/almothafar/simplebatterynotifier/util/AppPrefs.java @@ -15,8 +15,10 @@ *

* Migrated so far: the critical/warning battery levels. The same {@code 20}/{@code 40} literals * previously lived in {@code NotificationService}, {@code BatteryLevelReceiver}, {@code MainActivity} - * and the range slider's helper; they now live only in {@link #DEFAULT_CRITICAL_LEVEL} / - * {@link #DEFAULT_WARNING_LEVEL} here. Remaining settings migrate incrementally. + * and the range slider's helper; every Java read now derives its default from + * {@link #DEFAULT_CRITICAL_LEVEL} / {@link #DEFAULT_WARNING_LEVEL}. The one restatement that remains is + * the XML-declared slider default in {@code pref_alerts.xml} (see below), which the framework + * instantiates from XML and so cannot share a constant with. Remaining settings migrate incrementally. *

* The XML-declared defaults in {@code pref_alerts.xml} ({@code criticalDefault}/{@code warningDefault}) * must stay in step with these constants — the framework instantiates the slider from XML, so the two From a5798332bc7d642441a6fc1e7cecf8a918512014 Mon Sep 17 00:00:00 2001 From: Al-Mothafar Al-Hasan Date: Sun, 19 Jul 2026 12:43:48 +0300 Subject: [PATCH 3/3] Address review follow-ups: LevelThresholds type, drainLimitPph in facade, XML drift guard (#162) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three items the two-axis review raised as deferrable, now done: 1. Data Clumps — introduce model.LevelThresholds (record) so the critical/warning pair travels as one value instead of a loose (int, int) that could be swapped. AppPrefs.batteryLevels()/setBatteryLevels() and BatteryRangeSliderHelper.clampPair() now take/return it; the sliders and MainActivity unwrap only at the widget edge (HorseshoeProgressBar stays int-based to remain a reusable, app-agnostic widget). 2. Fold the drain limit into the facade — move DEFAULT/MIN/MAX_DRAIN_LIMIT_PPH and the clamp from BatteryRateTracker into AppPrefs as drainLimitPph()/clampDrainLimit(), the "single owner of key + default + clamp" the issue asks for. This also avoids a util->service dependency. Repoint FastDrainDetector and BatteryDetailsFragment; the pure clamp stays unit-tested (test moved alongside it). 3. Drift guard — AppPrefsTest now parses pref_alerts.xml and asserts the slider's XML-declared criticalDefault/warningDefault equal the AppPrefs constants, so the one framework-forced restatement can no longer silently diverge. Full suite: 445 tests, 0 failures. Co-Authored-By: Claude Opus 4.8 --- .../model/LevelThresholds.java | 16 ++ .../service/BatteryRateTracker.java | 33 ---- .../service/FastDrainDetector.java | 5 +- .../ui/MainActivity.java | 19 +- .../ui/fragment/BatteryDetailsFragment.java | 3 +- .../preference/BatteryRangeSliderHelper.java | 26 +-- .../BatteryRangeSliderPreference.java | 28 +-- .../simplebatterynotifier/util/AppPrefs.java | 92 +++++++--- app/src/main/res/xml/pref_alerts.xml | 2 +- .../service/BatteryRateTrackerTest.java | 28 --- .../BatteryRangeSliderHelperTest.java | 22 ++- .../util/AppPrefsTest.java | 166 ++++++++++++++---- 12 files changed, 274 insertions(+), 166 deletions(-) create mode 100644 app/src/main/java/com/almothafar/simplebatterynotifier/model/LevelThresholds.java diff --git a/app/src/main/java/com/almothafar/simplebatterynotifier/model/LevelThresholds.java b/app/src/main/java/com/almothafar/simplebatterynotifier/model/LevelThresholds.java new file mode 100644 index 0000000..9e25b46 --- /dev/null +++ b/app/src/main/java/com/almothafar/simplebatterynotifier/model/LevelThresholds.java @@ -0,0 +1,16 @@ +package com.almothafar.simplebatterynotifier.model; + +/** + * The two battery-level alert thresholds as one value, so critical and warning travel together instead + * of as a loose {@code (int, int)} pair callers must keep in the right order (#162). Critical is the + * lower, more urgent level; warning the higher. + *

+ * No ordering invariant is enforced here: the range slider structurally keeps critical below warning, + * and any persisted or corrupt pair is reconciled by + * {@code BatteryRangeSliderHelper.clampPair(LevelThresholds, int, int, int)} before it is shown. + * + * @param critical the critical level in percent — turns the gauge red and fires the critical alert + * @param warning the warning level in percent — turns the gauge amber and fires the warning alert + */ +public record LevelThresholds(int critical, int warning) { +} diff --git a/app/src/main/java/com/almothafar/simplebatterynotifier/service/BatteryRateTracker.java b/app/src/main/java/com/almothafar/simplebatterynotifier/service/BatteryRateTracker.java index a476284..6a441c8 100644 --- a/app/src/main/java/com/almothafar/simplebatterynotifier/service/BatteryRateTracker.java +++ b/app/src/main/java/com/almothafar/simplebatterynotifier/service/BatteryRateTracker.java @@ -65,12 +65,6 @@ public final class BatteryRateTracker { // Above this the derived %/h is garbage (e.g. a wrong-unit current); reject rather than display it. static final int MAX_PLAUSIBLE_RATE_PPH = 500; - // The "red / high drain" limit shared with the fast-drain alert (#109): default and accepted range. - // MIN/MAX must match the slider bounds (android:min/android:max) in pref_alerts.xml; they are - // enforced when the preference is read, so an out-of-range stored value can't skew the red line. - public static final int DEFAULT_DRAIN_LIMIT_PPH = 20; - public static final int MIN_DRAIN_LIMIT_PPH = 5; - public static final int MAX_DRAIN_LIMIT_PPH = 60; // Amber sits just below the red limit; 0.75x keeps colour reserved for genuinely high drain (#108). private static final float AMBER_RATIO = 0.75f; @@ -386,33 +380,6 @@ static int signedCurrentMilliAmps(final int microAmps, final boolean charging) { return charging ? magnitude : -magnitude; } - /** - * The user's shared "high drain" limit in %/h — the red line in the details table (#108) and the - * fast-drain alert trigger (#109). Defaults to {@link #DEFAULT_DRAIN_LIMIT_PPH}. - * - * @param context Application context - * - * @return the configured limit in %/h - */ - public static int getDrainLimitPercentPerHour(final Context context) { - final int stored = PreferenceManager.getDefaultSharedPreferences(context) - .getInt(context.getString(R.string._pref_key_fast_drain_limit), DEFAULT_DRAIN_LIMIT_PPH); - return clampDrainLimit(stored); - } - - /** - * Clamps a stored drain limit to the accepted range, so a corrupt or out-of-range preference value - * can't skew the red line here or the fast-drain trigger in #109. The bounds mirror the slider's - * {@code android:min}/{@code android:max} in {@code pref_alerts.xml}. Pure so it is unit-testable. - * - * @param stored the raw persisted limit in %/h - * - * @return the limit clamped to [{@link #MIN_DRAIN_LIMIT_PPH}, {@link #MAX_DRAIN_LIMIT_PPH}] - */ - static int clampDrainLimit(final int stored) { - return Math.max(MIN_DRAIN_LIMIT_PPH, Math.min(MAX_DRAIN_LIMIT_PPH, stored)); - } - /** * The amber threshold derived just below a given red limit (#108). Pure so it is unit-testable. * diff --git a/app/src/main/java/com/almothafar/simplebatterynotifier/service/FastDrainDetector.java b/app/src/main/java/com/almothafar/simplebatterynotifier/service/FastDrainDetector.java index b56877e..8979bd9 100644 --- a/app/src/main/java/com/almothafar/simplebatterynotifier/service/FastDrainDetector.java +++ b/app/src/main/java/com/almothafar/simplebatterynotifier/service/FastDrainDetector.java @@ -10,6 +10,7 @@ import com.almothafar.simplebatterynotifier.service.SustainedConditionTracker.Outcome; import com.almothafar.simplebatterynotifier.service.SustainedConditionTracker.Streak; import com.almothafar.simplebatterynotifier.service.SustainedConditionTracker.StreakStore; +import com.almothafar.simplebatterynotifier.util.AppPrefs; import static java.util.Objects.isNull; @@ -80,7 +81,7 @@ public static void evaluate(Context context, BatteryDO batteryDO, BatteryRate ra return; } - final int limit = BatteryRateTracker.getDrainLimitPercentPerHour(context); + final int limit = AppPrefs.drainLimitPph(context); final long sustainedMs = minutesPref(prefs, context, R.string._pref_key_fast_drain_sustained_minutes, DEFAULT_SUSTAINED_MINUTES, MIN_SUSTAINED_MINUTES, MAX_SUSTAINED_MINUTES); final long reminderGapMs = minutesPref(prefs, context, R.string._pref_key_fast_drain_reminder_minutes, @@ -143,7 +144,7 @@ private static long minutesPref(SharedPreferences prefs, /** * Clamps a stored minutes preference to its slider range and converts to millis. Mirrors - * {@link BatteryRateTracker#clampDrainLimit}: the slider constrains UI input, but a corrupt or + * {@link AppPrefs#clampDrainLimit}: the slider constrains UI input, but a corrupt or * out-of-range stored value (e.g. 0 sustained minutes) would otherwise defeat the sustained-window * requirement and fire on a momentary spike. Pure so it is unit-testable. * 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 a439ae8..0ca481e 100644 --- a/app/src/main/java/com/almothafar/simplebatterynotifier/ui/MainActivity.java +++ b/app/src/main/java/com/almothafar/simplebatterynotifier/ui/MainActivity.java @@ -29,6 +29,7 @@ import androidx.core.content.ContextCompat; import com.almothafar.simplebatterynotifier.R; import com.almothafar.simplebatterynotifier.model.BatteryDO; +import com.almothafar.simplebatterynotifier.model.LevelThresholds; import com.almothafar.simplebatterynotifier.service.BatteryHealthTracker; import com.almothafar.simplebatterynotifier.service.PowerConnectionService; import com.almothafar.simplebatterynotifier.service.SystemService; @@ -305,7 +306,8 @@ private static HorseshoeProgressBar.Flow flowOf(final int batteryStatus) { private void initializeFirstValues() { fillBatteryInfo(); - batteryGauge.setThresholds(AppPrefs.criticalLevel(this), AppPrefs.warningLevel(this)); + final LevelThresholds levels = AppPrefs.batteryLevels(this); + batteryGauge.setThresholds(levels.critical(), levels.warning()); // Keep the in-fly slider in sync with values that may have changed in Settings. syncThresholdSlider(); @@ -353,12 +355,11 @@ public void onStartTrackingTouch(@NonNull final RangeSlider slider) { @Override public void onStopTrackingTouch(@NonNull final RangeSlider slider) { final List values = slider.getValues(); - final int critical = Math.round(values.get(0)); - final int warning = Math.round(values.get(1)); + final LevelThresholds levels = new LevelThresholds(Math.round(values.get(0)), Math.round(values.get(1))); - AppPrefs.setBatteryLevels(MainActivity.this, critical, warning); + AppPrefs.setBatteryLevels(MainActivity.this, levels); - batteryGauge.setThresholds(critical, warning); + batteryGauge.setThresholds(levels.critical(), levels.warning()); } }); @@ -373,15 +374,13 @@ private void syncThresholdSlider() { if (isNull(thresholdSlider)) { return; } - final int critical = AppPrefs.criticalLevel(this); - final int warning = AppPrefs.warningLevel(this); - final int[] pair = BatteryRangeSliderHelper.clampPair(critical, warning, + final LevelThresholds pair = BatteryRangeSliderHelper.clampPair(AppPrefs.batteryLevels(this), BatteryRangeSliderHelper.LEVEL_FROM, BatteryRangeSliderHelper.LEVEL_TO, BatteryRangeSliderHelper.MIN_SEPARATION); - thresholdSlider.setValues((float) pair[0], (float) pair[1]); + thresholdSlider.setValues((float) pair.critical(), (float) pair.warning()); updateThresholdCaptions(findViewById(R.id.thresholdCriticalCaption), - findViewById(R.id.thresholdWarningCaption), pair[0], pair[1]); + findViewById(R.id.thresholdWarningCaption), pair.critical(), pair.warning()); } private void updateThresholdCaptions(final TextView criticalCaption, final TextView warningCaption, 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 10f89d4..c35bc11 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 @@ -28,6 +28,7 @@ import com.almothafar.simplebatterynotifier.service.BatteryHealthTracker; import com.almothafar.simplebatterynotifier.service.BatteryRateTracker; import com.almothafar.simplebatterynotifier.service.SystemService; +import com.almothafar.simplebatterynotifier.util.AppPrefs; import com.almothafar.simplebatterynotifier.util.GeneralHelper; import com.almothafar.simplebatterynotifier.util.TemperatureUtils; import com.google.android.material.dialog.MaterialAlertDialogBuilder; @@ -632,7 +633,7 @@ private int rateColor(final Context context, final BatteryRateTracker.BatteryRat if (rate.charging()) { return 0; } - final int limit = BatteryRateTracker.getDrainLimitPercentPerHour(context); + final int limit = AppPrefs.drainLimitPph(context); if (rate.percentPerHour() >= limit) { return GeneralHelper.getColor(getResources(), R.color.battery_rate_high); } diff --git a/app/src/main/java/com/almothafar/simplebatterynotifier/ui/preference/BatteryRangeSliderHelper.java b/app/src/main/java/com/almothafar/simplebatterynotifier/ui/preference/BatteryRangeSliderHelper.java index 85cf9b7..ccba834 100644 --- a/app/src/main/java/com/almothafar/simplebatterynotifier/ui/preference/BatteryRangeSliderHelper.java +++ b/app/src/main/java/com/almothafar/simplebatterynotifier/ui/preference/BatteryRangeSliderHelper.java @@ -1,6 +1,7 @@ package com.almothafar.simplebatterynotifier.ui.preference; import com.almothafar.simplebatterynotifier.R; +import com.almothafar.simplebatterynotifier.model.LevelThresholds; import com.google.android.material.slider.RangeSlider; /** @@ -28,7 +29,7 @@ private BatteryRangeSliderHelper() { /** * Apply the shared bounds, integer step, minimum thumb separation, and a "{@code N%}" label * formatter to a slider. Does not set the thumb values — callers set those after clamping via - * {@link #clampPair(int, int, int, int, int)}. + * {@link #clampPair(LevelThresholds, int, int, int)}. * * @param slider the range slider to configure * @param from combined track start (critical floor) @@ -46,22 +47,27 @@ public static void configure(final RangeSlider slider, final int from, final int } /** - * Clamp a (critical, warning) pair into {@code [from, to]} while keeping them at least - * {@code minSeparation} apart, so persisted or corrupted values can always be shown on the - * slider without tripping its validation. + * Clamp a {@link LevelThresholds} pair into {@code [from, to]} while keeping the two levels at least + * {@code minSeparation} apart, so persisted or corrupted values can always be shown on the slider + * without tripping its validation. * - * @return a two-element array {@code {critical, warning}} + * @param levels the raw (critical, warning) pair to reconcile + * @param from combined track start (critical floor) + * @param to combined track end (warning ceiling) + * @param minSeparation minimum gap between the two thumbs, in value units + * + * @return a clamped {@code (critical, warning)} pair the slider can always accept */ - public static int[] clampPair(final int critical, final int warning, - final int from, final int to, final int minSeparation) { - int low = clamp(critical, from, to); - int high = clamp(warning, from, to); + public static LevelThresholds clampPair(final LevelThresholds levels, + final int from, final int to, final int minSeparation) { + int low = clamp(levels.critical(), from, to); + int high = clamp(levels.warning(), from, to); if (high - low < minSeparation) { // Push the warning thumb up first; if that hits the ceiling, pull critical back down. high = Math.min(to, low + minSeparation); low = Math.max(from, high - minSeparation); } - return new int[]{low, high}; + return new LevelThresholds(low, high); } private static int clamp(final int value, final int lo, final int hi) { diff --git a/app/src/main/java/com/almothafar/simplebatterynotifier/ui/preference/BatteryRangeSliderPreference.java b/app/src/main/java/com/almothafar/simplebatterynotifier/ui/preference/BatteryRangeSliderPreference.java index e98eb88..c4e386c 100644 --- a/app/src/main/java/com/almothafar/simplebatterynotifier/ui/preference/BatteryRangeSliderPreference.java +++ b/app/src/main/java/com/almothafar/simplebatterynotifier/ui/preference/BatteryRangeSliderPreference.java @@ -11,6 +11,7 @@ import androidx.preference.PreferenceViewHolder; import com.almothafar.simplebatterynotifier.R; +import com.almothafar.simplebatterynotifier.model.LevelThresholds; import com.almothafar.simplebatterynotifier.util.AppPrefs; import com.google.android.material.slider.RangeSlider; @@ -103,15 +104,15 @@ public void onBindViewHolder(@NonNull final PreferenceViewHolder holder) { slider.clearOnChangeListeners(); slider.clearOnSliderTouchListeners(); - final int[] pair = readClampedValues(); - slider.setValues((float) pair[0], (float) pair[1]); - updateCaptions(criticalCaption, warningCaption, pair[0], pair[1]); + final LevelThresholds pair = readClampedValues(); + slider.setValues((float) pair.critical(), (float) pair.warning()); + updateCaptions(criticalCaption, warningCaption, pair.critical(), pair.warning()); // Update the captions live while dragging; persist only when the drag ends to avoid a burst // of writes and readers seeing half-applied intermediate values. slider.addOnChangeListener((s, value, fromUser) -> { - final int[] current = currentValues(s); - updateCaptions(criticalCaption, warningCaption, current[0], current[1]); + final LevelThresholds current = currentValues(s); + updateCaptions(criticalCaption, warningCaption, current.critical(), current.warning()); }); slider.addOnSliderTouchListener(new RangeSlider.OnSliderTouchListener() { @@ -122,8 +123,7 @@ public void onStartTrackingTouch(@NonNull final RangeSlider s) { @Override public void onStopTrackingTouch(@NonNull final RangeSlider s) { - final int[] current = currentValues(s); - persist(current[0], current[1]); + persist(currentValues(s)); } }); } @@ -132,19 +132,19 @@ public void onStopTrackingTouch(@NonNull final RangeSlider s) { * Read the persisted critical/warning values (falling back to the defaults) and clamp them into * the slider's bounds and separation so they can always be applied safely. */ - private int[] readClampedValues() { + private LevelThresholds readClampedValues() { final SharedPreferences prefs = getSharedPreferences(); final int critical = nonNull(prefs) ? prefs.getInt(criticalKey, criticalDefault) : criticalDefault; final int warning = nonNull(prefs) ? prefs.getInt(warningKey, warningDefault) : warningDefault; - return BatteryRangeSliderHelper.clampPair(critical, warning, from, to, minSeparation); + return BatteryRangeSliderHelper.clampPair(new LevelThresholds(critical, warning), from, to, minSeparation); } - private void persist(final int critical, final int warning) { + private void persist(final LevelThresholds levels) { final SharedPreferences prefs = getSharedPreferences(); if (nonNull(prefs)) { prefs.edit() - .putInt(criticalKey, critical) - .putInt(warningKey, warning) + .putInt(criticalKey, levels.critical()) + .putInt(warningKey, levels.warning()) .apply(); } } @@ -159,9 +159,9 @@ private void updateCaptions(final TextView criticalCaption, final TextView warni } } - private static int[] currentValues(final RangeSlider slider) { + private static LevelThresholds currentValues(final RangeSlider slider) { final List values = slider.getValues(); - return new int[]{Math.round(values.get(0)), Math.round(values.get(1))}; + return new LevelThresholds(Math.round(values.get(0)), Math.round(values.get(1))); } private static String orDefault(final String value, final String fallback) { diff --git a/app/src/main/java/com/almothafar/simplebatterynotifier/util/AppPrefs.java b/app/src/main/java/com/almothafar/simplebatterynotifier/util/AppPrefs.java index 224427b..a768e08 100644 --- a/app/src/main/java/com/almothafar/simplebatterynotifier/util/AppPrefs.java +++ b/app/src/main/java/com/almothafar/simplebatterynotifier/util/AppPrefs.java @@ -6,23 +6,27 @@ import androidx.preference.PreferenceManager; import com.almothafar.simplebatterynotifier.R; +import com.almothafar.simplebatterynotifier.model.LevelThresholds; /** * Typed facade over the app's default {@link SharedPreferences} (#162): the single owner of each - * migrated setting's key + default (and clamp, as more settings move here). Read sites call the - * typed accessor instead of repeating {@code getInt(getString(R.string._pref_key_…), )}, - * so a default can never silently drift between the alert engine, the receiver and the UI. + * migrated setting's key + default + clamp. Read sites call the typed accessor instead of + * repeating {@code getInt(getString(R.string._pref_key_…), )}, so a default can never + * silently drift between the alert engine, the receiver and the UI. *

- * Migrated so far: the critical/warning battery levels. The same {@code 20}/{@code 40} literals - * previously lived in {@code NotificationService}, {@code BatteryLevelReceiver}, {@code MainActivity} - * and the range slider's helper; every Java read now derives its default from - * {@link #DEFAULT_CRITICAL_LEVEL} / {@link #DEFAULT_WARNING_LEVEL}. The one restatement that remains is - * the XML-declared slider default in {@code pref_alerts.xml} (see below), which the framework - * instantiates from XML and so cannot share a constant with. Remaining settings migrate incrementally. - *

- * The XML-declared defaults in {@code pref_alerts.xml} ({@code criticalDefault}/{@code warningDefault}) - * must stay in step with these constants — the framework instantiates the slider from XML, so the two - * cannot literally share a constant, but they describe the same value. + * Migrated so far: + *

    + *
  • the critical/warning battery levels — the {@code 20}/{@code 40} literals that previously lived + * in {@code NotificationService}, {@code BatteryLevelReceiver}, {@code MainActivity} and the range + * slider's helper now derive from {@link #DEFAULT_CRITICAL_LEVEL} / {@link #DEFAULT_WARNING_LEVEL}, + * and the pair travels as a {@link LevelThresholds};
    • + *
  • the shared "high drain" limit — its default, accepted range and clamp ({@link #drainLimitPph} + * + {@link #clampDrainLimit}) moved here from {@code BatteryRateTracker}, so "a corrupt stored + * value can't defeat the feature" lives in one place.
    • + *
+ * The one restatement that remains is the XML-declared slider default in {@code pref_alerts.xml}, which + * the framework instantiates from XML and so cannot share a constant with — a comment ties the two, and + * {@code AppPrefsTest} asserts they stay equal. Remaining settings migrate incrementally. */ public final class AppPrefs { @@ -31,6 +35,13 @@ public final class AppPrefs { /** Default warning battery level in percent — the single owner of this value (#162). */ public static final int DEFAULT_WARNING_LEVEL = 40; + /** Default "high drain" limit in %/h. */ + public static final int DEFAULT_DRAIN_LIMIT_PPH = 20; + /** Lowest accepted drain limit in %/h; mirrors the slider's {@code android:min} in pref_alerts.xml. */ + public static final int MIN_DRAIN_LIMIT_PPH = 5; + /** Highest accepted drain limit in %/h; mirrors the slider's {@code android:max} in pref_alerts.xml. */ + public static final int MAX_DRAIN_LIMIT_PPH = 60; + private AppPrefs() { // Utility class } @@ -60,20 +71,59 @@ public static int warningLevel(final Context context) { } /** - * Persist the critical and warning battery levels together. The settings-screen slider and the - * home-screen in-fly slider both write this pair, so the write lives here beside the matching reads. + * Both battery-level thresholds as one value, so critical and warning travel together instead of as a + * loose (int, int) pair callers must keep in the right order. + * + * @param context Application context + * + * @return the configured {@code (critical, warning)} thresholds + */ + public static LevelThresholds batteryLevels(final Context context) { + return new LevelThresholds(criticalLevel(context), warningLevel(context)); + } + + /** + * Persist both battery-level thresholds together. The settings-screen slider and the home-screen + * in-fly slider both write this pair, so the write lives here beside the matching reads. * - * @param context Application context - * @param critical the critical level to store, in percent - * @param warning the warning level to store, in percent + * @param context Application context + * @param levels the thresholds to store */ - public static void setBatteryLevels(final Context context, final int critical, final int warning) { + public static void setBatteryLevels(final Context context, final LevelThresholds levels) { prefs(context).edit() - .putInt(context.getString(R.string._pref_key_critical_battery_level), critical) - .putInt(context.getString(R.string._pref_key_warn_battery_level), warning) + .putInt(context.getString(R.string._pref_key_critical_battery_level), levels.critical()) + .putInt(context.getString(R.string._pref_key_warn_battery_level), levels.warning()) .apply(); } + /** + * The user's shared "high drain" limit in %/h — the red line in the details table (#108) and the + * fast-drain alert trigger (#109). Reads {@link #DEFAULT_DRAIN_LIMIT_PPH} when unset and always + * {@link #clampDrainLimit clamps} the stored value, so a corrupt preference can't skew the red line + * or the alert trigger. + * + * @param context Application context + * + * @return the configured limit in %/h + */ + public static int drainLimitPph(final Context context) { + return clampDrainLimit(prefs(context).getInt( + context.getString(R.string._pref_key_fast_drain_limit), DEFAULT_DRAIN_LIMIT_PPH)); + } + + /** + * Clamps a stored drain limit to {@code [MIN_DRAIN_LIMIT_PPH, MAX_DRAIN_LIMIT_PPH]}. The bounds mirror + * the slider's {@code android:min}/{@code android:max} in {@code pref_alerts.xml}. Pure so it is + * unit-testable. + * + * @param stored the raw persisted limit in %/h + * + * @return the limit clamped to {@code [MIN_DRAIN_LIMIT_PPH, MAX_DRAIN_LIMIT_PPH]} + */ + public static int clampDrainLimit(final int stored) { + return Math.max(MIN_DRAIN_LIMIT_PPH, Math.min(MAX_DRAIN_LIMIT_PPH, stored)); + } + private static SharedPreferences prefs(final Context context) { return PreferenceManager.getDefaultSharedPreferences(context); } diff --git a/app/src/main/res/xml/pref_alerts.xml b/app/src/main/res/xml/pref_alerts.xml index eb02216..25367a0 100644 --- a/app/src/main/res/xml/pref_alerts.xml +++ b/app/src/main/res/xml/pref_alerts.xml @@ -143,7 +143,7 @@ app:iconSpaceReserved="false" /> data() { - return Arrays.asList(new Object[][]{ - {20, 20}, // in range: unchanged - {BatteryRateTracker.MIN_DRAIN_LIMIT_PPH, 5}, // boundaries kept - {BatteryRateTracker.MAX_DRAIN_LIMIT_PPH, 60}, - {0, 5}, // below min: clamped up - {-3, 5}, - {999, 60}, // above max: clamped down - }); - } - - @Test - public void matchesExpected() { - assertEquals(expected, BatteryRateTracker.clampDrainLimit(stored)); - } - } - /** * {@link BatteryRateTracker#appendAndTrim}: the min-spacing throttle and the trailing-window age trim. */ diff --git a/app/src/test/java/com/almothafar/simplebatterynotifier/ui/preference/BatteryRangeSliderHelperTest.java b/app/src/test/java/com/almothafar/simplebatterynotifier/ui/preference/BatteryRangeSliderHelperTest.java index 969d9ed..c3d1f66 100644 --- a/app/src/test/java/com/almothafar/simplebatterynotifier/ui/preference/BatteryRangeSliderHelperTest.java +++ b/app/src/test/java/com/almothafar/simplebatterynotifier/ui/preference/BatteryRangeSliderHelperTest.java @@ -1,5 +1,7 @@ package com.almothafar.simplebatterynotifier.ui.preference; +import com.almothafar.simplebatterynotifier.model.LevelThresholds; + import org.junit.Test; import org.junit.experimental.runners.Enclosed; import org.junit.runner.RunWith; @@ -27,7 +29,7 @@ public class BatteryRangeSliderHelperTest { private static final int SEP = BatteryRangeSliderHelper.MIN_SEPARATION; /** - * {@link BatteryRangeSliderHelper#clampPair(int, int, int, int, int)} maps representative + * {@link BatteryRangeSliderHelper#clampPair(LevelThresholds, int, int, int)} maps representative * (critical, warning) inputs — valid, out-of-range, inverted, and too-close — to the expected * clamped pair. */ @@ -56,9 +58,10 @@ public static Collection data() { @Test public void clampsToExpectedPair() { - final int[] result = BatteryRangeSliderHelper.clampPair(critical, warning, FROM, TO, SEP); - assertEquals("critical", expectedCritical, result[0]); - assertEquals("warning", expectedWarning, result[1]); + final LevelThresholds result = BatteryRangeSliderHelper.clampPair( + new LevelThresholds(critical, warning), FROM, TO, SEP); + assertEquals("critical", expectedCritical, result.critical()); + assertEquals("warning", expectedWarning, result.warning()); } } @@ -72,12 +75,13 @@ public static class Invariants { public void everyInputYieldsAValidPair() { for (int critical = -20; critical <= 80; critical++) { for (int warning = -20; warning <= 80; warning++) { - final int[] r = BatteryRangeSliderHelper.clampPair(critical, warning, FROM, TO, SEP); + final LevelThresholds r = BatteryRangeSliderHelper.clampPair( + new LevelThresholds(critical, warning), FROM, TO, SEP); final String at = "(" + critical + "," + warning + ")"; - assertTrue("critical in range at " + at, r[0] >= FROM && r[0] <= TO); - assertTrue("warning in range at " + at, r[1] >= FROM && r[1] <= TO); - assertTrue("separation kept at " + at, r[1] - r[0] >= SEP); - assertTrue("critical below warning at " + at, r[0] < r[1]); + assertTrue("critical in range at " + at, r.critical() >= FROM && r.critical() <= TO); + assertTrue("warning in range at " + at, r.warning() >= FROM && r.warning() <= TO); + assertTrue("separation kept at " + at, r.warning() - r.critical() >= SEP); + assertTrue("critical below warning at " + at, r.critical() < r.warning()); } } } diff --git a/app/src/test/java/com/almothafar/simplebatterynotifier/util/AppPrefsTest.java b/app/src/test/java/com/almothafar/simplebatterynotifier/util/AppPrefsTest.java index de60f42..50b5044 100644 --- a/app/src/test/java/com/almothafar/simplebatterynotifier/util/AppPrefsTest.java +++ b/app/src/test/java/com/almothafar/simplebatterynotifier/util/AppPrefsTest.java @@ -2,68 +2,160 @@ import android.content.Context; import android.content.SharedPreferences; +import android.content.res.XmlResourceParser; import androidx.preference.PreferenceManager; import androidx.test.core.app.ApplicationProvider; import com.almothafar.simplebatterynotifier.R; +import com.almothafar.simplebatterynotifier.model.LevelThresholds; 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 org.xmlpull.v1.XmlPullParser; + +import java.util.Arrays; +import java.util.Collection; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; /** - * Robolectric tests for the {@link AppPrefs} facade (#162): the typed level accessors fall back to the - * single-owner defaults when nothing is stored, read back a stored value, and - * {@link AppPrefs#setBatteryLevels} writes the pair under the exact keys the alert engine and sliders - * read. Each test gets a fresh application (and therefore empty default SharedPreferences). + * Tests for the {@link AppPrefs} facade (#162). The context-backed cases run under Robolectric (typed + * accessors fall back to the single-owned defaults, read stored values, round-trip writes, and — the + * drift guard — the XML slider defaults still equal the constants); the pure drain-limit clamp is a + * plain parameterized test. */ -@RunWith(RobolectricTestRunner.class) -@Config(sdk = 34) +@RunWith(Enclosed.class) public class AppPrefsTest { - private Context context; + @RunWith(RobolectricTestRunner.class) + @Config(sdk = 34) + public static class ContextBacked { - @Before - public void setUp() { - context = ApplicationProvider.getApplicationContext(); - } + private Context context; - @Test - public void levels_fallBackToTheSingleOwnedDefaults() { - assertEquals(AppPrefs.DEFAULT_CRITICAL_LEVEL, AppPrefs.criticalLevel(context)); - assertEquals(AppPrefs.DEFAULT_WARNING_LEVEL, AppPrefs.warningLevel(context)); - // Pin the historically-duplicated 20/40 literals these constants replaced. - assertEquals(20, AppPrefs.DEFAULT_CRITICAL_LEVEL); - assertEquals(40, AppPrefs.DEFAULT_WARNING_LEVEL); - } + @Before + public void setUp() { + context = ApplicationProvider.getApplicationContext(); + } + + @Test + public void levels_fallBackToTheSingleOwnedDefaults() { + assertEquals(AppPrefs.DEFAULT_CRITICAL_LEVEL, AppPrefs.criticalLevel(context)); + assertEquals(AppPrefs.DEFAULT_WARNING_LEVEL, AppPrefs.warningLevel(context)); + // Pin the historically-duplicated 20/40 literals these constants replaced. + assertEquals(20, AppPrefs.DEFAULT_CRITICAL_LEVEL); + assertEquals(40, AppPrefs.DEFAULT_WARNING_LEVEL); + } + + @Test + public void levels_readBackStoredValues() { + PreferenceManager.getDefaultSharedPreferences(context).edit() + .putInt(context.getString(R.string._pref_key_critical_battery_level), 11) + .putInt(context.getString(R.string._pref_key_warn_battery_level), 33) + .apply(); + + assertEquals(11, AppPrefs.criticalLevel(context)); + assertEquals(33, AppPrefs.warningLevel(context)); + } + + @Test + public void batteryLevels_bundlesCriticalAndWarning() { + AppPrefs.setBatteryLevels(context, new LevelThresholds(15, 35)); + + assertEquals(new LevelThresholds(15, 35), AppPrefs.batteryLevels(context)); + } - @Test - public void levels_readBackStoredValues() { - PreferenceManager.getDefaultSharedPreferences(context).edit() - .putInt(context.getString(R.string._pref_key_critical_battery_level), 11) - .putInt(context.getString(R.string._pref_key_warn_battery_level), 33) - .apply(); + @Test + public void setBatteryLevels_persistsBothThresholdsUnderTheSharedKeys() { + AppPrefs.setBatteryLevels(context, new LevelThresholds(15, 35)); - assertEquals(11, AppPrefs.criticalLevel(context)); - assertEquals(33, AppPrefs.warningLevel(context)); + // Round-trips through the typed getters... + assertEquals(15, AppPrefs.criticalLevel(context)); + assertEquals(35, AppPrefs.warningLevel(context)); + + // ...and lands under the exact keys the sliders and alert engine read directly. + final SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(context); + assertEquals(15, prefs.getInt(context.getString(R.string._pref_key_critical_battery_level), -1)); + assertEquals(35, prefs.getInt(context.getString(R.string._pref_key_warn_battery_level), -1)); + } + + @Test + public void drainLimitPph_defaultsWhenUnsetAndClampsStoredValue() { + assertEquals(AppPrefs.DEFAULT_DRAIN_LIMIT_PPH, AppPrefs.drainLimitPph(context)); + + // A corrupt out-of-range stored value is clamped on read, not returned raw. + PreferenceManager.getDefaultSharedPreferences(context).edit() + .putInt(context.getString(R.string._pref_key_fast_drain_limit), 999) + .apply(); + assertEquals(AppPrefs.MAX_DRAIN_LIMIT_PPH, AppPrefs.drainLimitPph(context)); + } + + /** + * The drift guard for the one restatement the facade can't own: the range slider's XML-declared + * defaults in {@code pref_alerts.xml} must equal the {@link AppPrefs} constants, since the + * framework instantiates the slider from XML and its attr wins as that control's default. + */ + @Test + public void xmlSliderDefaults_matchTheFacadeConstants() throws Exception { + final XmlResourceParser parser = context.getResources().getXml(R.xml.pref_alerts); + Integer xmlCritical = null; + Integer xmlWarning = null; + + for (int event = parser.getEventType(); event != XmlPullParser.END_DOCUMENT; event = parser.next()) { + if (event != XmlPullParser.START_TAG || !parser.getName().endsWith("BatteryRangeSliderPreference")) { + continue; + } + for (int i = 0; i < parser.getAttributeCount(); i++) { + final int attrRes = parser.getAttributeNameResource(i); + if (attrRes == R.attr.criticalDefault) { + xmlCritical = parser.getAttributeIntValue(i, Integer.MIN_VALUE); + } else if (attrRes == R.attr.warningDefault) { + xmlWarning = parser.getAttributeIntValue(i, Integer.MIN_VALUE); + } + } + } + + assertNotNull("criticalDefault attr missing from pref_alerts.xml", xmlCritical); + assertNotNull("warningDefault attr missing from pref_alerts.xml", xmlWarning); + assertEquals(AppPrefs.DEFAULT_CRITICAL_LEVEL, (int) xmlCritical); + assertEquals(AppPrefs.DEFAULT_WARNING_LEVEL, (int) xmlWarning); + } } - @Test - public void setBatteryLevels_persistsBothThresholdsUnderTheSharedKeys() { - AppPrefs.setBatteryLevels(context, 15, 35); + /** + * {@link AppPrefs#clampDrainLimit}: a stored limit outside the slider's range is clamped, so a + * corrupt preference can't skew the red line or the #109 trigger. Pure, so no context is needed. + */ + @RunWith(Parameterized.class) + public static class ClampDrainLimit { + + @Parameter(0) public int stored; + @Parameter(1) public int expected; - // Round-trips through the typed getters... - assertEquals(15, AppPrefs.criticalLevel(context)); - assertEquals(35, AppPrefs.warningLevel(context)); + @Parameters(name = "clampDrainLimit({0}) = {1}") + public static Collection data() { + return Arrays.asList(new Object[][]{ + {20, 20}, // in range: unchanged + {AppPrefs.MIN_DRAIN_LIMIT_PPH, 5}, // boundaries kept + {AppPrefs.MAX_DRAIN_LIMIT_PPH, 60}, + {0, 5}, // below min: clamped up + {-3, 5}, + {999, 60}, // above max: clamped down + }); + } - // ...and lands under the exact keys the sliders and alert engine read directly. - final SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(context); - assertEquals(15, prefs.getInt(context.getString(R.string._pref_key_critical_battery_level), -1)); - assertEquals(35, prefs.getInt(context.getString(R.string._pref_key_warn_battery_level), -1)); + @Test + public void matchesExpected() { + assertEquals(expected, AppPrefs.clampDrainLimit(stored)); + } } }