NightscoutFoundation / xDrip

Nightscout version of xDrip+
https://jamorham.github.io/#xdrip-plus
GNU General Public License v3.0
1.4k stars 1.14k forks source link

"re-raise every x minutes if unacknowledged"" in alert is not working properly #1159

Closed gregorybel closed 3 years ago

gregorybel commented 4 years ago

I use several alerts on BG value. Some with 1 minute "re-raise" time and some with 5min "re-raise" time.

Current behaviour: Alerts using 5min "re-raise" time are triggered after 1min too, what ever I enter Expected: unacknowledged alerts are retriggered after 5min

using xdrip from 28th of October 2019 (6d4d)

Navid200 commented 3 years ago

Interesting.
May I ask if this is still the case with more recent releases?

sigbjorn88 commented 3 years ago

I have something similar I believe @Navid200 , might be settings though, but it is set like shown in picure on the individual alarm. Are there something else like in the "other alarms" menu that needs to be set to make it work? I had the alert reraise time set to 60 seconds, which is the same time it took for my unacknowledged alarm to raise again. However, this option is only available when I toggle on bad value alerts and then "reraise alerts before snooze time". What is the difference between unacknowledged and reraise setting here? Screenshot_20210303-114549_xDrip+

Screenshot_20210303-114905_xDrip+

Navid200 commented 3 years ago

First of all, what do you mean by "... set to 60 seconds"? Do you mean 1 minute? Why say 60 seconds? I know they are the same. But, you are trying to explain something to people who have not experienced it. Why add unnecessary causes of confusion? Why not say "I set it to 1 minute"?

You say "this option is only available".
That suggests that you have seen a menu that varies as you change a setting. Can you post two screenshots showing the two different versions of the same menu?

gregorybel commented 3 years ago

@Navid200 regarding your question if I retest with a newer version, answer is no. Are there indications in a release notes showing this part of code has been touched ? If yes, then I can retest of course.

Navid200 commented 3 years ago

@gregorybel So, if you are not experiencing this issue with a recent release, I suggest we close this issue.

sigbjorn88 commented 3 years ago

Sorry for the confusion. From the second picture (which I thought was another option on delaying unacknowledged alarms) it said seconds in capital letters so I gave you that. In the "other alerts" menu I have the options from the two pictures here (with and without "bad noisy values" and "reraise alerts before snooze time" toogled ON)

Screenshot_20210303-175814_xDrip+

My normal alarm setting window is like the one above with reraise every 100. minute and snooze every 120. I still get alarm every 1 minute (happy now? 😄) if I dont do anything. Screenshot_20210303-174053_xDrip+

gregorybel commented 3 years ago

@Navid200 no, I did not retest with a newer version! So no idea if issue is fixed. I went through all the commits since one year and could not find one which may fix this issue.

tzachi-dar commented 3 years ago

What profile are you using? Is it asceding?

בתאריך יום ד׳, 3 במרץ 2021, 21:55, מאת Grégory Beloncle ‏< notifications@github.com>:

@Navid200 https://github.com/Navid200 no, I did not retest with a newer version! So no idea if issue is fixed. I went through all the commits since one year and could not find one which may fix this issue.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/NightscoutFoundation/xDrip/issues/1159#issuecomment-790013640, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB4TBZA2FYRLNX52DNZZB23TB2H4HANCNFSM4KDGRHSQ .

Navid200 commented 3 years ago

@tzachi-dar Does it matter what profile he is using?

I tested it with the February 17, 2021 release of xDrip and can confirm the same thing happens.

I created a new high alarm and intentionally set the threshold low to make it go off in a few minutes. I set the "Re-raise every x minutes if unacknowledged" to 2 minutes.

When the alarm went off. I did not snooze it. After 1 minute, it went off again. Again, I didn't snooze. After 1 minutes, it went off again.

It should have waited 2 minutes, as I set it to, not just 1 minute.

gregorybel commented 3 years ago

@tzachi-dar yes I use profile ascending, is this related?

@Navid200 thanks having checked with last version, so indeed not fixed yet.

sigbjorn88 commented 3 years ago

I changed to medium sound (not ascending) and it seems to work, @Navid200. Assume it is not meant to be like that, but at least only a minor bug then and not some strange settings elsewhere.

gregorybel commented 3 years ago

Indeed it helps but ascending sound is quiet usefull, either in a meeting, it just vibrate and at night it goes really loud if one dont wake up.

Navid200 commented 3 years ago

@gregorybel So, are you confirming that the issue occurs only with the ascending volume profile?

tzachi-dar commented 3 years ago

This feature is not working with ascending profile, since ascending profile means that the alert will get stronger every minute.

gregorybel commented 3 years ago

@tzachi-dar ok. Would it maybe then make sence to trigger the stronger alert not every minute but every delay set under "re-raise every x minutes if unackknowledged"?

Navid200 commented 3 years ago

@gregorybel Wouldn't that defeat the purpose of the ascending volume profile? The idea is to start the alarm very quietly, to avoid disturbing the others. But, rapidly raise the volume to avoid delaying the alarm for too long. If it takes 4 volume increments for you to hear it, you are effectively experiencing a 4-minute delay. And, this is before you hear it for the first time. If the increments occur at a slower pace, the alarm may be delayed too long and your glucose may already have dropped dangerously too low before you ever hear it?

This is very different than you hearing an alarm but not taking action, which is the purpose of re-raise every x minutes if unacknowledged.

We can have this as a feature request. In that case, I ask you to close this, and open a new issue with the title stating your new request and adding a link inside the thread to this issue for the records.
But, I strongly advise against such a feature, and will object to its development and pull request and merge. I hope you will find an alternative approach to meet your needs.

sigbjorn88 commented 3 years ago

Support you, @Navid200. In my case I think the ascending was chosen to be similar to the dexcom app where the alarm volume is raised if unacknowledged. But see now the difference between them more clealy. Thanks

gregorybel commented 3 years ago

@Navid200 I don't agree, I think it would be a great feature to be able to setup at which rate volume is ascending. Then it is up to us to define a good rate depending on the alert. For example, I do alerts for several glucose level. When I am at 200, I like to be alerted, because it may go even higher and I need to check everything is normal (is the pump really attached?), but I don't need to be alerted every minute, every 5 or 10 minutes is fine I think. Actually I thought it was exactly the purpose of the "reraise after..." but it seems not.

So strickly looking at the problem and how it has been designed, then I agree, there is no issue, so indeed we can close this ticket.

But I think I will look at developping this feature and pull request it then we can discuss if and how this can be accepted or not.

Definitly thanks to you and @tzachi-dar having look at this issue, we now have an answer :smile:

Navid200 commented 3 years ago

It's good and bad to give too much control to the user. This may sound wrong. But, it is life. We don't have a low volume option. People have asked for it. It has not been added. If anyone submits a pull request to add the ability to set the volume low, I will go out of my way to stop and disapprove it.
Reason: Imagine a construction worker who goes to bed after having changed the volume profile to low because he doesn't want to wake up his partner. In the morning, he forgets to change the volume profile and goes to work with a low volume profile. At work, he gets a low glucose alert but never hears it because of the high noise volume at work. We can blame him for forgetting. But, no, we don't have the option. That is the reason we don't have the option. It's not a oversight on the part of the developers.

Likewise, I will disagree with allowing the user to lower the ascending volume rate. In fact, I prefer the rate to be even higher.

My point is that there are a few things we have to take responsibility about and worry about the consequence of a mistake by the user. Even though everyone is responsible for the their own actions, there are a few things I would not allow if I had a say.

But, that's just my single vote. At the end, it is jamorham who has to make the call.

Thanks for closing this issue and thanks for your contributions.