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

Idle mode awareness #47

Merged
merged 5 commits into from Nov 7, 2015
Merged

Idle mode awareness #47

merged 5 commits into from Nov 7, 2015

Conversation

coltin
Copy link
Contributor

@coltin coltin commented Oct 29, 2015

Android M adds Doze mode and App Standby. This change aims to make the Job Manager work with Doze mode.

Doze mode disables access to network while active, but isConnectedOrConnecting() says there is network. This results in the Job Manager thinking there is network, scheduling jobs to run that require the network, those jobs failing and then being rescheduled (just to fail again).

Additionally when Doze exits a network broadcast (CONNECTIVITY_ACTION) is NOT sent. So until another job is entered into the queue or the device loses internet and gains it back, networking jobs will be idle even after Doze is finished. By listening to ACTION_DEVICE_IDLE_MODE_CHANGED the Job Manager can update the network queue accordingly.

This is important especially for IDLE_MAINTENANCE which is entered periodically during doze (granting network access if available). If we don't listen to this broadcast it's unlikely we'll wake up to do work during this maintenance window.

task wrapper(type: Wrapper) {
gradleVersion = '2.2.1'
gradleVersion = '2.8.0'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2.8.0 does not work, I'll have to fix this to 2.8.

@yigit
Copy link
Owner

yigit commented Nov 2, 2015

Sorry i missed this over the weekend, i'll check tonight or tomorrow. Thanks.

@@ -19,7 +20,7 @@ buildscript {
}

dependencies {
classpath 'com.android.tools.build:gradle:1.1.3'
classpath 'com.android.tools.build:gradle:1.3.1'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I shouldn't go around trying to upgrade everything. This for some reason breaks jacoco. ./gradlew tasks --stacktrace produces:

Could not determine the dependencies of task ':jacocoTestReport'.
...
Task with path 'testDebug' not found in root project 'jobqueue'.

Since this build tool bump is not really necessary I'm going to discard it.

Also removes gradle version bump since it breaks jacoco.
@coltin
Copy link
Contributor Author

coltin commented Nov 2, 2015

screen shot 2015-11-02 at 5 49 01 pm

Apparently checking the api version the line before you use things is not good enough for lint. I'll fix these tomorrow morning >_>

@yigit
Copy link
Owner

yigit commented Nov 3, 2015

I'm not sure if you can add it to the constructor (you should) but just adding TargetAPI can fix it. A better/nicer way to do it would be moving it to another method which has the TargetAPI check.

@yigit
Copy link
Owner

yigit commented Nov 3, 2015

overall looks good to me except the gradle 2.8 update.

@coltin
Copy link
Contributor Author

coltin commented Nov 3, 2015

Lint errors are all fixed, let me know if you're fine with how I pulled out the code into separate methods.

Running tests ./gradlew clean assembleDebug assembleDebugUnitTest test or ./gradlew check has 100 errors. All of this nature:


  symbol:   class JobHolder
  location: package com.path.android.jobqueue
../android-priority-jobqueue/jobqueue/src/test/java/com/path/android/jobqueue/test/jobmanager/GroupingTest.java:4: error: cannot find symbol
import com.path.android.jobqueue.JobManager;
                                ^
  symbol:   class JobManager
  location: package com.path.android.jobqueue
../android-priority-jobqueue/jobqueue/src/test/java/com/path/android/jobqueue/test/jobmanager/GroupingTest.java:5: error: cannot find symbol
import com.path.android.jobqueue.Params;
                                ^
  symbol:   class Params
  location: package com.path.android.jobqueue
../android-priority-jobqueue/jobqueue/src/test/java/com/path/android/jobqueue/test/jobmanager/GroupingTest.java:6: error: package com.path.android.jobqueue.config does not exist
import com.path.android.jobqueue.config.Configuration;
                                       ^
../android-priority-jobqueue/jobqueue/src/test/java/com/path/android/jobqueue/test/jobmanager/InjectorTest.java:3: error: cannot find symbol
import com.path.android.jobqueue.Job;

@yigit
Copy link
Owner

yigit commented Nov 4, 2015

hmm i did just run gradlew clean check and works fine. I'll try to checkout this and try there.
Looks like jacoco dependency is broken, you can change that line as follows:

task jacocoTestReport(type:JacocoReport, dependsOn: "testDebugUnitTest") {

@coltin
Copy link
Contributor Author

coltin commented Nov 4, 2015

If I change the dependency to "testDebugUnitTest" I get

:tasks FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':tasks'.
> Could not determine the dependencies of task ':jacocoTestReport'.

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output.

BUILD FAILED

The stack trace shows that it can't find Caused by: org.gradle.api.UnknownTaskException: Task with path 'testDebugUnitTest' not found in root project 'jobqueue'.

Did you have any luck running it? Could be that my setup is foobar in some way.

@yigit
Copy link
Owner

yigit commented Nov 7, 2015

I'll just merge it and fix whatever is wrong in tests.
Thanks a lot!

yigit added a commit that referenced this pull request Nov 7, 2015
@yigit yigit merged commit acee7df into yigit:master Nov 7, 2015
@yigit
Copy link
Owner

yigit commented Nov 8, 2015

i rebased my delay group branch with this and running tests works fine. So everything should be OK. Thanks again!

@coltin
Copy link
Contributor Author

coltin commented Nov 8, 2015

Awesome, thanks a lot! Cheers :)

private static IntentFilter getNetworkIntentFilter() {
IntentFilter networkIntentFilter = new IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION);
if (VERSION.SDK_INT >= 23) {
networkIntentFilter.addAction(PowerManager.ACTION_DEVICE_IDLE_MODE_CHANGED);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. (y)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:)

@malisetti
Copy link
Contributor

@coltin #57

@coltin
Copy link
Contributor Author

coltin commented Nov 30, 2015

@mseshachalam +1.

I don't have time to work on this for a few days, but if you make a pull request with your fix maybe @yigit can take a look before I can.

@TargetApi(23)
private static IntentFilter getNetworkIntentFilter() {
IntentFilter networkIntentFilter = new IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION);
if (VERSION.SDK_INT >= 23) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be Build.VERSION_CODES.M instead of a magic number? It would be easier to read in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with this.

I believe I had Lint issues or Android Studio yelling at me about VERSION_CODES.M because of the compileSdkVersion or targetSdkVersion being lower than 23; could be making that up haha.

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.

None yet

4 participants