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 alert threshold #3554

Closed

Conversation

Navid200
Copy link
Collaborator

@Navid200 Navid200 commented Jul 1, 2024

After this change, a user can set a persistent high alert threshold different than the High Value, used for statistics.

Since xDrip persistent high alert has been there for years, we must be careful not to cause any unintended change to its behavior.
If an existing user updates to a release after this PR is merged, their High Value is automatically used as their new Persistent High alert threshold.

This is what the persistent high alert menu looks after this PR:
Screenshot_20240701-144255

This is the log that is shown after an update:
Screenshot_20240701-144207

There are two concerns I have about this PR explained in the next post.

@Navid200
Copy link
Collaborator Author

Navid200 commented Jul 1, 2024

My concerns

1- If a user installs xDrip after this PR is merged and sets a persistent high alert threshold and then, installs an older version of xDrip prior to this PR, their persistent high alert will automatically use their High Value as their alert threshold.
This is a change in the behavior of an alert simply caused by installing an older version of xDrip.

2- If a user installs xDrip after this PR is merged and sets a persistent high alert threshold and then, loads settings they had saved before this PR, their persistent high alert will use the default value, 170 mg/dL.
This could be a change in the behavior of an alert caused by loading old settings.


This first scenario is inevitable. Older versions of xDrip have no idea about a setting that did not exist then.
A user should only install an older version of xDrip when absolutely necessary.

The second scenario is indirectly caused because xDrip is incapable of importing settings. Instead, it restores settings.
This is something I would like to change by adding the ability to import settings. But, that is a major change that is outside the context of this PR.
Hopefully, we can some day accomplish it. But, that day is not today!

I can add a special marker to such releases in the release notes marking a release that the user should be careful going over in time in reverse order.

@Navid200
Copy link
Collaborator Author

Navid200 commented Jul 1, 2024

So, why do we need this?

After I eat, I may go high for a short while. But, I need to come back down shortly after. To ensure about that, I use the persistent high alert. And I set it to 7.5mmol/L (135 mg/dL).
I currently do this, by setting my high value to 7.5mmol/L.
But, this will result in my statistics to be incorrect. My time in range is a very important statistical parameter I need to keep an eye on. The standard value for the high boundary of time in range is 10mml/L (180mg/dL).
Every time I want to get the correct statistical parameters, I will need to change my high value from 7.5 to 10.
I don't think I am the only one.

This change will allow me to set my persistent high alert with no impact on my statistics.

@Navid200
Copy link
Collaborator Author

Navid200 commented Jul 6, 2024

If this is merged, there will be an announcement in the facebook group to highlight that if/when someone changes their high level, their persistent high alert will not be affected as it was before.

If this is not adequate, in order to get this merged, we can add a listener to high level. Whenever there is a change, a log a toast will be issued informing the user that the persistent high alert has a dedicated threshold that is not affected by this change. I hope you won't ask for this.
If we add this, it will need a retirement schedule. Meaning, we will remove it after a certain period, like following the next stable release.
I hope you won't ask for this.

@tzachi-dar
Copy link
Contributor

It seems that this change is also changing the default value of the alert from 170 to 135. I believe that for most people 170 is a better value.

@Navid200
Copy link
Collaborator Author

Navid200 commented Jul 7, 2024

@tzachi-dar How did we come up with that decision?

There is a correlation between the average blood glucose and A1C.
#1653

If someone has their blood glucose at 169 most of the time, which will never hear a persistent high alert if their persistent high alert threshold is set to 170, their A1C, according to that paper, will be 7.5.
Why is that best for everyone?

So, I am curious why you say that.
Nevertheless, changing the threshold for the alert is not the purpose of this PR.
The purpose is to untie the two settings from each other.
I can set the default to 170, and still accomplish the objective of this PR. However, before doing that, I would like to know why this belief exists that 170 is the best value for the majority.

The definition of non-diabetic is someone whose A1C is less than 5.7
Using that formula again, an A1C of 5.7 corresponds to an average blood glucose of 117 mg/dL. 135 is still higher than that.

Of course, we don't just have a persistent high alert. So, it's the combination of all the alerts that help us control our diabetes.
The persistent high alert also has a time threshold. By default, that time threshold is set to 1 hour.

An individual may have 3 main meal a day. If after each one of those meals, the glucose stays above 169 for an hour, they will never hear a persistent high alert.
That means they could have their blood glucose at 169 for 3 hours a day.
Why is that best for everyone?

The persistent high alert is not even enabled by default.

@Navid200
Copy link
Collaborator Author

Navid200 commented Jul 7, 2024

@tzachi-dar Thanks for your comment. After seeing your comment, and thinking about it more, I see that we should keep in mind we should not give users the idea that their blood glucose is too high encouraging them to take too much insulin. This has to be done by a doctor.

I never created this PR in order to change the default. My intent was to separate the two settings from each other and when I created the new setting, it needed a default and I chose the default based on what I was using myself. That is not the right way for choosing a default.
Again, thank you for letting me see this.

I am happy to change the default. My only objective for this PR is to separate the settings.
Anyone can choose a value other than the default including me.
The best default is a value that the least number of people will need to change.
How do we find out that value?

@Navid200
Copy link
Collaborator Author

Navid200 commented Jul 8, 2024

I have set the default of the new setting to 170. Therefore, the default threshold of the persistent high alert remains unchanged if this PR is merged.

@Navid200 Navid200 requested a review from jamorham July 13, 2024 19:47
@Navid200
Copy link
Collaborator Author

Navid200 commented Aug 29, 2024

There are many settings on the Extra Alerts page. I can add a level of hierarchy to it. So, the page would only have the following on it:
Persistent High
Forecast Low
Sensor Expiry
Other

Then, if it helps getting this approved, I will add a checkbox to the Persistent High page that would be: Link to High Value
If that setting is enabled, which will be by default, the "Persistent high threshold" setting will be disabled.
If someone prefers to unlink them, they will uncheck the checkbox and will be able to independently adjust them.

@Navid200 Navid200 marked this pull request as draft September 11, 2024 11:16
@Navid200
Copy link
Collaborator Author

Navid200 commented Oct 1, 2024

I will use a different approach where the default behavior will be identical to what we have now in a new PR that I will open later.

@Navid200 Navid200 closed this Oct 1, 2024
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