-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Make sensor expiry notifications customizable to the minute #3675
base: master
Are you sure you want to change the base?
Make sensor expiry notifications customizable to the minute #3675
Conversation
4aea550
to
c9384a1
Compare
@Navid200 Kindly requesting your review. 😊 |
I can only give you my opinion. Only the approval of the repository owner matters as far as what is merged and what is not. In general, I would never include translations in a PR. It makes your PR look bigger than it really is. And translations should be added by translators in Crowdin, not be developers in a PR. But, my main concern with this is that I don't think we should have alerts for sensor expiry. We should instead have reminders. To expand on that, imagine if something comes up for you and you don't get to start your sensor at the time you would have liked. Let's say by the time you take care of the unexpected, you end up starting your sensor 4 hours later than you would have liked. That's why reminders, multiple of them, instead of one alert will let you plan your day such that you get to start the sensor exactly when you need to. I agree that if a sensor expires, that requires an alert. If I forgot that my sensor was expiring and I let it expire, I need an alert. That alert (notification) already exists. In my opinion, we can modify the existing sensor alert (not the existing sensor expiry notification) such that in addition to the timings that it currently has, it would have an additional notification at 15 minutes to expiry and we can set only that one not to be silent. We will still need to get the lead developer to review and approve. I can not promise approval. But, that change makes sense to me at least. |
"To expand on that, imagine if something comes up for you and you don't get to start your sensor at the time you would have liked. Let's say by the time you take care of the unexpected, you end up starting your sensor 4 hours later than you would have liked." That's why i would like to see that the 12hr/24hr sensor expiry time would be more clearly understandable for non-english users. They're always confused by the AM versus PM difference (which one of these is before or after lunch..), instead of 24hrs as they use. That would add up to an 12hr+ issue in case of misunderstanding. |
In this case, the user can just change the minutes before sensor expiry alert to go off 4 hours earlier than previously.
I don't find this functionality desirable, but my change preserves it for those who do. My belief is that users should have the freedom to maximally customize their diabetes management, with minimal developer input. @jamorham May you please review my PR? |
As a diabetic, I am with you 100%. I want to have absolute freedom. As a developer, I am aware that 90% of those who have made changes to xDrip have disappeared and are no where to be found. Look at this: #3641 Any change made to xDrip, in my opinion, should have a very good reason addressing a serious shortcoming. We have had a request to allow the 12-hour alert to become non silent as an option. |
Implements idea introduced in #3666.
Note that the current way of hard coding notifications at 24, 12, 6, and 2 hours before sensory expiry is preserved if a custom value isn't supplied.
Tested locally using phone and smartwatch.