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

Limit REST API upload failure alert and log to once a day #2858

Merged
merged 4 commits into from
Jun 17, 2023

Conversation

Navid200
Copy link
Collaborator

@Navid200 Navid200 commented May 28, 2023

If you upload to Nightscout and have no internet access, you will get fail log once every 5 minutes until you have access to the internet again.
You will also receive an alert once every 30 minutes.

Many users just disable the alert.

We have Google Cloud Nightscout users who have not logged into their FreeDNS account for 6 months. As a result, their Nightscout has stopped working. But, they have no idea. The reason is that they have disabled this alert.

If this PR is merged, there will only be a single log once every 6 hours and a single alert only once a day if there is no internet access.
However, the uploaders status page still shows the failure immediately as this PR makes no change to that.

If Nightscout upload is failing, it could just be because you have no access to the internet. In that case, the alert has no value.
If your Nightscout server has gone down, you will need to fix it. Receiving an alert after 24 hours of failure, is not going to take the chance of fixing it away.
So, there is no reason to receive so many alerts and logs.

Please let me know if I am overlooking a scenario when a 5-minute log interval and a 30-minute alert interval is necessary.

@Navid200 Navid200 marked this pull request as draft May 28, 2023 22:29
@Navid200 Navid200 marked this pull request as ready for review May 29, 2023 01:06
@Navid200
Copy link
Collaborator Author

This has been tested setting the period to 15 minutes to speed up the test.

I have chosen not to add yet one more setting to xDrip.
There is really no need to a setting. 24 hours should serve every user.

@jamorham
Copy link
Collaborator

jamorham commented Jun 6, 2023

So overall I agree with the problem you're trying to fix, but rather than annotate the PR I think it should be reworked.

For your constants, make them private static final and I prefer to use upper case for those constants to make it clear in the code that they never change. Also a comment to explain the purpose of the sum would be helpful.

I think it should log more frequently than once per day that there is a problem if the uploader is failing, say once per 6 hours.

For the notification once per 24 hours is fine but for the structure of this I would say introduce another rate limit and use pratelimit for anything more than an hour incase android unloads our app. Also I think you should have the preference check for warning outside of the rate limit so that if the switch is off and the user then switches it on they will get an alert sooner and also there is some overhead on pratelimit storage so if the user has warnings disabled then this wont be a problem.

Overall this should reduce alerts to once per 24 hours and logs to once per 6 hours, so that if a user has a problem like "why isn't my nightscout working?" then there is a chance they will see the reason in the logs. If their uploads are failing constantly and their nightscout is dead then they should disable the uploader because its a waste of their phone resources not to so we do need to make sure they know.

@Navid200 Navid200 marked this pull request as draft June 6, 2023 17:03
@Navid200 Navid200 marked this pull request as ready for review June 10, 2023 03:09
@Navid200
Copy link
Collaborator Author

I have made the suggested corrections.
The logs from a test phone are shown below.
Screenshot_20230609-223845

There is a log once every 6 hours as intended. They appear in the screenshot at about 2, 8, 14 and 20.
In additions to those, there is another log, marked with red encircling the logs, that appear once a day.
This is still good and reduces the number of logs significantly compared to what we have now (one every 5 minutes).

The notification appears once a day as intended.

@Navid200
Copy link
Collaborator Author

There is still something about this that needs to be tweaked.
The alert makes a sound. Why?

If someone has Nightscout as a part of their loop, whatever receives data from Nightscout should create a sound alert to inform the user that it is not receiving data; therefore, it cannot calculate how much insulin to inject.

If someone is following someone through Nightscout, it's the follower that can receive a sound alert if they enable their missed readings alert.

But, if I am uploading to Nightscout and have no followers and am not using a loop, why do I need to be woken up in the middle of the night to fix my Nightscout? I can fix it when I am awake. All the readings will be in the queue and as soon as Nightscout is up again, they will all be uploaded. It is not like a low alert that needs action right away.

There are people that still are going to disable the alert I think if we don't make it a silent alert.
So, I would like to make the alert silent.
Would that be acceptable?

@jamorham
Copy link
Collaborator

Look at some of the other signatures for the showNotification() method, you can specify whether or not to make a sound.

@Navid200
Copy link
Collaborator Author

The notification is now silent (no sound and no vibration).

@jamorham jamorham merged commit 800d22b into NightscoutFoundation:master Jun 17, 2023
@Navid200 Navid200 deleted the Navid_2023_05_24c branch June 17, 2023 22:32
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