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

Foreground services #1068

Merged
merged 2 commits into from
Oct 9, 2020
Merged

Foreground services #1068

merged 2 commits into from
Oct 9, 2020

Conversation

di72nn
Copy link
Member

@di72nn di72nn commented Oct 1, 2020

I'm not sure if I'll be able to properly deal with #974 anytime soon, so here's a not so elegant but simple fix.

This PR should fix a whole slew of IllegalStateExceptions reported in the Play Console. Fixes #975, #784.

Using foreground services has some merit for some of the tasks, but #974 still stands.

First commit here makes the app use foreground services for any background tasks. This is technically ok, but Android requires to display a visible notification when the service is running, which is quite annoying (especially because in this implementation the notifications don't have any meaningful content). These notifications may be disabled (using notification channels), but only manually by the user.

According to Play Console the errors are relatively rare, so the app should work fine with non-foreground services most of the time. The second commit tries to use foreground services only when starting a normal service fails. This is debatable.

The simplest way to test it is to enable autosync on app start, and start the app with the screen turned off (like with adb shell am start -n fr.gaulupeau.apps.InThePoche.debug/fr.gaulupeau.apps.Poche.ui.MainActivity or directly from AndroidStudio). There's a timeout in MainActivity.updateOnStartup() though.

@tcitworld
Copy link
Member

Thanks for taking time to explain the details, btw!

@Strubbl Strubbl merged commit cb92c51 into master Oct 9, 2020
@Strubbl Strubbl deleted the foreground_services branch October 9, 2020 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto-sync is broken
3 participants