Fix autosend scheduling #6121
Fix autosend scheduling #6121
Conversation
|
@WKobus could you take another look at this when you get a chance? The biggest change since you last looked is that "cellular only" shouldn't run when connected to a metered wifi (like a hotspot), but should run eventually if you then just connect to cellular (although it might take a few minutes). |
| @@ -81,7 +81,8 @@ interface Scheduler { | |||
|
|
|||
| enum class NetworkType { | |||
| WIFI, | |||
| CELLULAR | |||
| CELLULAR, | |||
| OTHER | |||
There was a problem hiding this comment.
OTHER would just be any other connected network. Android devices can theoretically have network delivered via Ethernet or Bluetooth for example. These kinds of connections would still satisfy WorkManager's (and our won) "connected" constraint.
| * Returns class that can be used to schedule this task using Android's | ||
| * WorkManager framework | ||
| */ | ||
| fun getWorkManagerAdapter(): Class<out WorkerAdapter> |
There was a problem hiding this comment.
I realised we didn't need all this and could just use strings (and no arg constructors) to integrate with WorkManager. It will eventually have to store our class names as strings anyway.
| import android.net.ConnectivityManager | ||
| import org.odk.collect.async.Scheduler | ||
|
|
||
| class ConnectivityProvider(private val context: Context) : NetworkStateProvider { |
There was a problem hiding this comment.
We'll need to replace this implementation at some point, as it's pretty much all deprecated.
| @@ -216,6 +214,14 @@ class InstancesDataService( | |||
| } | |||
| } | |||
|
|
|||
| fun instanceFinalized(projectId: String, form: Form) { | |||
There was a problem hiding this comment.
I think we'd ideally have a method that's more like:
fun finalizeInstance(projectId: String, instance: Instance)
But the code that finalises a form during form entry and finalises during bulk finalisation is very different at the moment (the former happens pre save to disk), so that's something for another day.
Closes #6120
This also:
It's probably a good idea to wait on merging this until it's marked "behaviour verified" as we've seen a lot of churn on auto send recently.
Why is this the best possible solution? Were any other approaches considered?
I would have loved to have a more elegant solution for "cellular only", but couldn't find one that would work within the WorkManager world. I think the compromise of having other settings work optimally within Android's current background job setup is correct though.
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
This should mean that auto send now behaves more like how we'd "expect". One caveat to highlight is that metered wifi (like a hotspot) will not work with "wifi only" or "cellular only". The work around for this (other than using "wifi or cellular") is to change the wifi to "unmetered" in Android's settings.
Before submitting this PR, please make sure you have:
./gradlew connectedAndroidTest(or./gradlew testLab) and confirmed all checks still pass