Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Persistent High Threshold range verification #3767

Merged
merged 3 commits into from
Nov 26, 2024

Conversation

Navid200
Copy link
Collaborator

@Navid200 Navid200 commented Nov 10, 2024

This PR corrects the value of the preference setting if it is out of range.

The following image shows a log that appears when a correction takes place.
Screenshot_20241110-114505

@Navid200
Copy link
Collaborator Author

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

Done.
We don't use handleUnitsChange() any longer.

@jamorham
Copy link
Collaborator

Hi, can you explain when you would expect this to activate? What are the circumstances that result in an out of range value? thanks.

@Navid200
Copy link
Collaborator Author

Navid200 commented Nov 26, 2024

Hi,

Thanks for the review. There are two scenarios that will trigger this:

1- A user already has xDrip installed and mmol/L selected as their unit of choice.
The version of xDrip installed is prior to when we introduced persistent high threshold.
When the user updates xDrip to a version containing the new persistent high threshold, it will be set to 170 mmol/L.
This will never be corrected to 9.4mmol/L unless the user changes their units to mg/dL and changes it back to mmol/L.

This PR detects this situation and corrects it with no need for the user to do anything. No toast or log or silent notification will be shown.

2- The user has xDrip installed prior to this PR but after we introduced persistent high threshold and by mistake or intentionally set the persistent high threshold to a value outside the 40-400mg/dL range.

After this PR, this situation will be detected and the value will be set to min if the chosen value is less than min or max if the chosen value is greater than max. A silent notification will be shown to notify the user. A corresponding log will also be shown.

@Navid200
Copy link
Collaborator Author

Navid200 commented Nov 26, 2024

I forgot to show the silent notification. here it is on the left. And on the right is the corresponding log.


Screenshot_20241126-082748 Screenshot_20241126-083215

@Navid200
Copy link
Collaborator Author

Navid200 commented Nov 26, 2024

The silent notification only notifies the user that something needs their attention. There is not much detail in it.
But, there is more detail in the corresponding log.

@Navid200
Copy link
Collaborator Author

I'm sorry about that redundancy.
And thanks for removing it. I couldn't think of a way to remove it before one was merged.

Thanks so much for all the support.

@Navid200
Copy link
Collaborator Author

Would it help if I rebase this now that the other one has been merged?
Or, is there anything else I can do to help?

@jamorham
Copy link
Collaborator

I think something has got broken with the imports or similar as it is failing unit test:

JoH.static_toast_long(xdrip.gs(R.string.the_value_must_be_between_min_and_max, unitsConvert2Disp(domgdl, MIN_GLUCOSE_INPUT), unitsConvert2Disp(domgdl, MAX_GLUCOSE_INPUT)));

@Navid200
Copy link
Collaborator Author

Navid200 commented Nov 26, 2024

Is it OK if I rebase this PR and pull it into my Android Studio and test and re-submit?

@jamorham
Copy link
Collaborator

I think it is just missing these imports:

import static com.eveningoutpost.dexdrip.EditAlertActivity.unitsConvert2Disp;
import static com.eveningoutpost.dexdrip.models.JoH.tolerantParseDouble;

@jamorham
Copy link
Collaborator

Likely got broken when I tried to resolve the merge conflicts not realizing they were needed.

@jamorham
Copy link
Collaborator

If you can just pull and add those and push that commit then that should fix it. thanks

@Navid200
Copy link
Collaborator Author

Compiling

@jamorham
Copy link
Collaborator

thanks for the fast response, sorry about the import issue

@jamorham jamorham merged commit d8e96f9 into NightscoutFoundation:master Nov 26, 2024
1 check passed
@Navid200
Copy link
Collaborator Author

Thanks a lot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants