Persistent High threshold fix #3734

Navid200 · 2024-10-26T00:16:48Z

This PR fixes two issues I caused adding Persistent High threshold (#3704).

1- You can enter any value as persistent high threshold.
This PR rejects all values that are outside the 40-400 mg/dL range.

2- If xDrip is already installed and mmol/L is selected and xDrip is updated to our latest Nightly (2024.10.11), the threshold will be set to 170 mmol/L.
The user will need to change units to mg/dL and back to mmol/L to correct that to 9.4 mmol/L.
This PR does the correction automatically.

Navid200 · 2024-10-27T11:55:23Z

This is ready for a review.

jamorham · 2024-10-29T11:15:01Z

app/src/main/java/com/eveningoutpost/dexdrip/Home.java

@@ -385,6 +386,9 @@ protected void onCreate(Bundle savedInstanceState) {
        setVolumeControlStream(AudioManager.STREAM_MUSIC);

        checkedeula = checkEula();
+        if (!Pref.getString("units", "mgdl").equals("mgdl")) { // Only if the selected unit is mmol/L
+            handleUnitsChange(null, "mmol", null); // Trigger the correction of values (defaults) if needed based on the selected unit


I don't think this should be here, we should only need to call handleUnitsChange() when the units are changed.

I will create a video clip and add here showing why we need this.

Install a previous version of xDrip on a test phone. Change the unit to mmol/L.
Then, update xDrip to our current Nightly. Go to the Persistent High Alert page. You will see this as the threshold:

170 mmol/L

Change the unit to mg/dL and back to mmol/L. You will see it change to 9.4 mmol/L.

Triggering the handle unit change right after xDrip start guarantees that will never happen.

ok, but the onCreate method is not once on application start it is whenever the Home activity is created. It should be in IdempotentMigrations.performAll() but you need to check that there are no side effects as well because that handleUnitsChange() method does a few different things.

Thanks a lot. I will fix all of this in a few hours.

Thanks

jamorham · 2024-10-29T11:15:45Z

app/src/main/java/com/eveningoutpost/dexdrip/utils/Preferences.java

@@ -755,6 +758,11 @@ public boolean onPreferenceChange(Preference preference, Object value) {
            String stringValue = value.toString();
            if (isNumeric(stringValue)) {
                final boolean domgdl = Pref.getString("units", "mgdl").equals("mgdl"); // Identify which unit is chosen
+                double submissionMgdl = domgdl ? Double.parseDouble(stringValue) : Double.parseDouble(stringValue) * Constants.MMOLL_TO_MGDL;
+                if (submissionMgdl > MAX_GLUCOSE_INPUT || submissionMgdl < MIN_GLUCOSE_INPUT) {
+                    JoH.static_toast_long("The value must be between " + unitsConvert2Disp(domgdl, MIN_GLUCOSE_INPUT) + " and " + unitsConvert2Disp(domgdl, MAX_GLUCOSE_INPUT));


If we want to continue improving localization this should be a string resource format string that has these values formatted in to it.

I left everything in English intentionally to make sure it all looks good before adding any of it to strings.

jamorham · 2024-10-29T11:16:52Z

app/src/main/java/com/eveningoutpost/dexdrip/utils/Preferences.java

@@ -755,6 +758,11 @@ public boolean onPreferenceChange(Preference preference, Object value) {
            String stringValue = value.toString();
            if (isNumeric(stringValue)) {
                final boolean domgdl = Pref.getString("units", "mgdl").equals("mgdl"); // Identify which unit is chosen
+                double submissionMgdl = domgdl ? Double.parseDouble(stringValue) : Double.parseDouble(stringValue) * Constants.MMOLL_TO_MGDL;


tolerantParseDouble() is typically much safer to use and supports localized number input where , can be the decimal point.

Thanks
Will change it

Navid200 · 2024-10-29T14:44:04Z

Thanks for the catch.

I have changed to tolerantParseDouble().
I have moved the call to handleUnitChaange to IdempotentMigrations.

Thanks for the guidance.

I have added the strings for translation as well.

Navid200 · 2024-10-31T03:53:40Z

I don't know why/how the file "output-metadata.json" was added to the pull request.
Should I remove it somehow? Or, can you exclude it from merge assuming you want to approve?

jamorham · 2024-11-03T13:57:12Z

app/prod/release/output-metadata.json

@@ -0,0 +1,20 @@
+{


This file should not be in the PR

Removing it right now

jamorham · 2024-11-03T13:59:44Z

app/src/main/java/com/eveningoutpost/dexdrip/utils/Preferences.java

@@ -755,6 +759,11 @@ public boolean onPreferenceChange(Preference preference, Object value) {
            String stringValue = value.toString();
            if (isNumeric(stringValue)) {
                final boolean domgdl = Pref.getString("units", "mgdl").equals("mgdl"); // Identify which unit is chosen
+                double submissionMgdl = domgdl ? tolerantParseDouble(stringValue) : tolerantParseDouble(stringValue) * Constants.MMOLL_TO_MGDL;
+                if (submissionMgdl > MAX_GLUCOSE_INPUT || submissionMgdl < MIN_GLUCOSE_INPUT) {
+                    JoH.static_toast_long(xdrip.gs(R.string.the_value_must_be_between_space) + unitsConvert2Disp(domgdl, MIN_GLUCOSE_INPUT) + xdrip.gs(R.string.space_and_space) + unitsConvert2Disp(domgdl, MAX_GLUCOSE_INPUT));


For good localization you shouldn't be building the string yourself with concatenation. Instead use a format string. See how xdrip_software_changed_format is handled for an example.

Navid200 · 2024-11-03T14:05:27Z

Thanks for the multiple reviews. I don't know how a compilation file got added to the PR.

I hope you don't mind I will have to close this PR yet again and open a new clean PR without that file and with the string set up as you have advised.

Navid200 added 3 commits October 25, 2024 20:11

Persistent High threshold fix

a90f486

Cleanup

d69b318

Cleanup 2

d00302d

jamorham reviewed Oct 29, 2024

View reviewed changes

Fixed

11710eb

Translate

0eec76c

jamorham reviewed Nov 3, 2024

View reviewed changes

Navid200 closed this Nov 3, 2024

Navid200 mentioned this pull request Nov 3, 2024

Persistent High threshold fix #3752

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Persistent High threshold fix #3734

Persistent High threshold fix #3734

Navid200 commented Oct 26, 2024 •

edited

Loading

Navid200 commented Oct 27, 2024

jamorham Oct 29, 2024

Navid200 Oct 29, 2024

Navid200 Oct 29, 2024

jamorham Oct 29, 2024

Navid200 Oct 29, 2024

jamorham Oct 29, 2024

Navid200 Oct 29, 2024

jamorham Oct 29, 2024

Navid200 Oct 29, 2024

Navid200 commented Oct 29, 2024 •

edited

Loading

Navid200 commented Oct 31, 2024

jamorham Nov 3, 2024

Navid200 Nov 3, 2024

jamorham Nov 3, 2024

Navid200 commented Nov 3, 2024

Persistent High threshold fix #3734

Persistent High threshold fix #3734

Conversation

Navid200 commented Oct 26, 2024 • edited Loading

Navid200 commented Oct 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Navid200 commented Oct 29, 2024 • edited Loading

Navid200 commented Oct 31, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Navid200 commented Nov 3, 2024

Navid200 commented Oct 26, 2024 •

edited

Loading

Navid200 commented Oct 29, 2024 •

edited

Loading