Add support for periodically downloading puzzles in the background.#111
Add support for periodically downloading puzzles in the background.#111kebernet merged 1 commit intokebernet:masterfrom
Conversation
| BackgroundDownloadService.class.getName())); | ||
|
|
||
| builder.setPeriodic(TimeUnit.HOURS.toMillis(1)) | ||
| .setRequiredNetworkType(JobInfo.NETWORK_TYPE_UNMETERED) |
There was a problem hiding this comment.
This would likely be better done at something like 07:00UTC. That generally means that the puzzles that update at Midnight pacific time will be there. Not a lot of reason to keep running all the DLs.
There was a problem hiding this comment.
I did consider this, but the JobScheduler API doesn't support scheduling at a specific time of day (only periodically or after a given deadline) so scheduling the job consistently at 07:00UTC is non-trivial (the delay needs to change if the user changes time zones, DST, etc.). We'll also have to implement some handling of failed downloads, whereas now we just converge on the next hourly run.
If your main concern is device battery usage, I've been running this for the past few months and haven't seen Shortyz noticeable in my battery stats (obviously a pretty small sample size). My guess is JobScheduler batches pretty efficiently. There's also an option to download only while charging, which we could change to a default.
I also have a personal preference to make this work with PR #88 which would require also scheduling at 04:00UTC to get the NYT puzzle the night before. I could just hard code two daily runs to make this work for now if we go this route, so not a big deal.
Let me know how strongly you feel about this. I can spend some time figuring out a way to cut down the number of download attempts if you'd like.
There was a problem hiding this comment.
Turns out that this is a bit more complicated than planned. It's possible to listen for timezone (and it seems DST) changes on the user's device via the TIME_CHANGED and TIMEZONE_CHANGED intents and reschedule your job/alarm accordingly.
However, we're actually trying to set an alarm in a different time zone (12AM Pacific as you mentioned). Since not everyone changes DST at the same time (or at all, e.g. Arizona), we need to reschedule the job when the PDT<->PST transition happens to keep the alarm at the right time of day.
If we just ditch periodic jobs and reschedule after a successful completion, it might work alright since the consequences of a miss by an hour or so in either direction aren't too bad (deliver late a couple times a year, or schedule an extra job). We'd have to make this work with handling transient download failures though.
That said, the Downloaders class will already prevent re-downloading if the puzzle exists. I just updated the PR with a fix for a bug around notifications here (a notification still got posted even if all downloads were skipped).
Given that making this work for users in all time zones is as bit complex, and we already avoid unnecessary network access, would you be alright leaving this as is for now?
| // user turns on the preference for the first time, and we want to ensure the UI is re-rendered | ||
| // when the exit the settings dialog. | ||
| private boolean checkBackgroundDownload() { | ||
| if (Build.VERSION.SDK_INT < 21) { |
There was a problem hiding this comment.
This kind of thing can be abstracted into the AndroidVersionUtils class.
|
BTW thanks for this. I have been going back and forth on a service for this, but using JobScheduler is the correct way, and, you know, fuck old versions. I have a couple of nits picked, but for the most part this is solid. |
|
Thanks for the review! I should have some time to take care of these points this weekend. It's definitely possible to backport this to older versions at some point. Evernote has a library that mostly abstracts this (evernote/android-job), but I figured it would be simpler to start with this for now. |
jonzolla
force-pushed
the
background_download
branch
from
6b506e1 to
c3672f5
Compare
This is implemented by using JobScheduler to periodically download puzzles that the user has selected. JobScheduler is only available on API version >=21, so this functionality is disabled for lower API versions. Using the JobScheduler API allows us to easily provide options for downloading on wifi only, when charging, etc. Currently just attempt to download every hour. There are more intelligent things we can do here, but this has been working well on my device. The updated NYT authentication mechanism is supported by posting a notification requesting login when a background download fails instead of immediately launching the login activity. A slight refactor of Downloaders was required to allow for downloading the most recent the most recent puzzle for each source instead of just the puzzle for the current (or specified) date.
jonzolla
force-pushed
the
background_download
branch
from
c3672f5 to
a418b0a
Compare
2 participants
This is implemented by using JobScheduler to periodically download puzzles that the user has selected. JobScheduler is only available on API version >=21, so this functionality is disabled for lower API versions.
Using the JobScheduler API allows us to easily provide options for downloading on wifi only, when charging, etc.
Currently just attempt to download every hour. There are more intelligent things we can do here, but this has been working well on my device.
The updated NYT authentication mechanism is supported by posting a notification requesting login when a background download fails instead of immediately launching the login activity.
A slight refactor of Downloaders was required to allow for downloading the most recent the most recent puzzle for each source instead of just the puzzle for the current (or specified) date. This is intended to work in concert with PR #88 which allows the NYT puzzles to be available before the puzzle date.
I'm not too familiar with Android development, and was having a hard time coming up with a way to unit test these changes. I'm open to adding some if you have some suggestions for how to do so.