-
Notifications
You must be signed in to change notification settings - Fork 706
Download rework #2037
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
base: master
Are you sure you want to change the base?
Download rework #2037
Conversation
| updateNotification(context, currentVisualDownloads, currentVisualQueue) | ||
|
|
||
| // Arbitrary delay to prevent hogging the CPU, decrease to make the queue feel slightly more responsive | ||
| delay(500) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be a condition variable. Unfortunately the built in synchronization primatives are built for Java threading, and not the kotlin coroutines.
The fix for this would be something like:
val conditionSignal = CompletableDeferred<Unit>()
// ...
select {
onTimeout(500) {
println("Waited")
}
conditionSignal.onAwait {
println("Skipped waiting")
}
}Select simply awaits the fastest execution. So you can conditionSignal.complete(Unit) to skip the 500ms delay. If you implement this correctly you can do a timeout of 10s or something similar.
| startForeground(DOWNLOAD_QUEUE_NOTIFICATION_ID, baseNotification.build()) | ||
| } | ||
|
|
||
| val context = this.applicationContext |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a context, I do not understand why it needs to be applicationContext
| .data(url) | ||
| .apply { | ||
| headers?.forEach { (key, value) -> | ||
| extras[Extras.Key<String>(key)] = value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
headerBuilder["User-Agent"] = USER_AGENT
Changes the whole download system to:
Remaining issues to fix:
I plan to complete this pull request this week.