fix(widget): standardize alarm interval to 10 minutes and document updatePeriodMillis limitation#20482
Conversation
|
First PR! 🚀 We sincerely appreciate that you have taken the time to propose a change to AnkiDroid! Please have patience with us as we are all volunteers - we will get to this as soon as possible. |
193be65 to
05c77ea
Compare
| SystemClock.elapsedRealtime() + 10.minutes.inWholeMilliseconds, | ||
| 10.minutes.inWholeMilliseconds, |
There was a problem hiding this comment.
Isn't 1 minute a better duration?
There was a problem hiding this comment.
the real reason for suggesting this approach is that by using 1 minute there are few drawbacks like: as we are firing it every minute then it will drain the battery more faster and it was also mentioned for the 10 mins in the TODO and the code was mismatching and reason behind choosing 10 mins was because it is neutral for it
There was a problem hiding this comment.
Understood, reverting it back in a minute
41c86cf to
3e0c59d
Compare
| <!-- Note: It was suggested to use updatePeriodMillis but it has a minimum of 30 mins enforced by the android | ||
| which is too infrequent for due card count as it can create difficulty for the user, | ||
| and used a 10 minutes alarm as it was intended.--> |
There was a problem hiding this comment.
| <!-- Note: It was suggested to use updatePeriodMillis but it has a minimum of 30 mins enforced by the android | |
| which is too infrequent for due card count as it can create difficulty for the user, | |
| and used a 10 minutes alarm as it was intended.--> |
| <!-- Note: It was suggested to use updatePeriodMillis but it has a minimum of 30 mins enforced by the android | ||
| which is too infrequent for due card count as it can create difficulty for the user, | ||
| and used a 10 minutes alarm as it was intended--> |
There was a problem hiding this comment.
| <!-- Note: It was suggested to use updatePeriodMillis but it has a minimum of 30 mins enforced by the android | |
| which is too infrequent for due card count as it can create difficulty for the user, | |
| and used a 10 minutes alarm as it was intended--> |
|
If the TODOs aren't actionable, just remove them |
Understood, removing the TODOs entirely |
…r XML files `updatePeriodMillis` has a minimum of 30 mins Co-authored-by: David Allison <62114487+david-allison@users.noreply.github.com>
a01ea9b to
f559049
Compare
Purpose / Description:
There was a difference between a comment and the real code and also proposed a new approach
Fixes
There is no issue number since this was TODO and it is located inside the widget_provider_deck_picker.xml at line 11
Approach
So it was mentioned to use updatePeriodMillis and by research i found out it's minimum time is for 30 mins and that time is slower/delayed when the widget is added because it can create inconsistency and confusion for the user and the code was not following the 10 mins thing mentioned in the TODO so updated it to 10 minutes instead of being set at 1 minute.
How Has This Been Tested?
The tests were performed on the emulator device and it was running successfully without any error or crash
##Learning (optional, can help others)
Referred topic for this problem was updatePeriodMillis, source for learning were developer.android.com
Checklist