LibreShift / red-moon

Android screen filter app for night time phone use.
GNU General Public License v3.0
650 stars 81 forks source link

Timer is not active in idle (Doze) mode on newer devices #144

Closed willemw12 closed 7 years ago

willemw12 commented 7 years ago

LineageOS 14.1. Android 7.1.2. Red Moon (F-Droid) 2.10.2.

When the device is idle for a long time, the timer does not work. The filter does not "start" and "stop".

To test this, force the device in idle (Doze) mode and wait for the "start" or "stop" event:

adb shell dumpsys deviceidle force-idle

Possible fix: allow the timer's alarm also to operate in idle mode for newer devices (by calling alarmManager.setExactAndAllowWhileIdle(), SDK API 23):

File TimeToggleChangeReceiver.kt

if (atLeastAPI(23)) {
    alarmManager.setExactAndAllowWhileIdle(AlarmManager.RTC, calendar.timeInMillis, pendingIntent)
} else if (atLeastAPI(19)) {
    alarmManager.setExact(AlarmManager.RTC, calendar.timeInMillis, pendingIntent)

This seems to work after some quick testing.

smichel17 commented 7 years ago

Thanks for the detailed report. It'll be in the next release.

Red Moon is getting closer to feature-complete; at that point I'm going to buckle down and work on stability (like, actually writing some tests :P) & battery usage. Curious how much of an impact this kind of thing has on battery.

In theory at least, we shouldn't need to receive alarms while the device is idle, since we don't actually do anything while the screen is off. But right now that definitely isn't the case (#137).

That does mean we'd need to be smarter about fading in and out (#33), as currently the fade starts when we receive the alarm without checking what time it is.

willemw12 commented 7 years ago

For better battery usage during gradual fading, use the AlarmManager.setInexactRepeating() method.

Note: even when disabling Battery Optimization for the app in the system settings, the timer did not activate.

smichel17 commented 7 years ago

Even when disabling Battery Optimization for the app in the system settings, the timer did not activate.

I noticed that too, troubleshooting now..

willemw12 commented 7 years ago

No, there is no need to troubleshoot that. alarmManager.setExactAndAllowWhileIdle() is the fix (for newer devices).

smichel17 commented 7 years ago

I used alarmManager.setExactAndAllowWhileIdle() in the commit above and there were still situations in which it did not activate (force idle, set alarm for one minute, kill the process). I'm pretty sure it's because even whitelisted apps are rate limited to one alarm per 10 minutes. I'll know for sure in 3 minutes.

smichel17 commented 7 years ago

The emulator I'm testing in is acting really oddly. I'm out of time to spend on this today, I'll run the version with setExactAndAllowWhileIdle on my device for a while and see if it has any effect.

smichel17 commented 7 years ago

There is a new support library, AlarmManagerCompat. We should probably use it.

There's also a permission that will allow us to request battery optimization exemption from within the app; We should use that too.

smichel17 commented 7 years ago

I don't want to spend time fixing this and then throw that all away to use AlarmManagerCompat; I'd rather just use it to begin with. So, I'm going to wait on this until api 26.0.0 reaches a stable release.

smichel17 commented 7 years ago

Actually, I think the timer code will need a few changes as part of implementing root mode, so I may just use the api while it's in beta.

smichel17 commented 7 years ago

I believe the an issue is that we're using AlarmManager.RTC when we should be using AlarmManager.RTC_WAKEUP to force the cpu on at that point.

smichel17 commented 7 years ago

If we use RTC_WAKEUP, I think we can even skip using setExact and just calculate how far into the fade we should be, whenever android decides to deliver the alarm.

willemw12 commented 7 years ago

Note: the solution mentioned in the description (setExactAndAllowWhileIdle) has worked every time on my device.

smichel17 commented 7 years ago

@willemw12 Do you have Times from sun enabled? I believe setExactAndAllowWhileIdle does indeed solve the initial issue as reported, but causes issues if Times from sun is enabled, due to the way #33 is currently implemented*.

*It's just a very long animation, which is actually a handler under the covers. Because it's behind a startService call, there's no guarantee that it will start before the device returns to idle, and the fade-in won't start until the screen is actually turned on.

I believe the proper solution if we wanted to keep the current method (which I don't; it's fragile and incompatible with root mode) would be to grab a wake lock in TimeToggleChangeReciever and then release it once the animation starts.

willemw12 commented 7 years ago

Times from sun is disabled.

grab a wake lock in TimeToggleChangeReciever

This may not allow the device to "doze" when the screen is off.

smichel17 commented 7 years ago

This may not allow the device to "doze" when the screen is off.

I know. We'd only need to hold the wake lock long enough to make sure the animation is started (a couple milliseconds). Then we can release it and I believe the animator / android system will handle the rest (ie, fast forward the animation to the appropriate place when "doze" ends).

smichel17 commented 7 years ago

Android documentation is confusing to me. I asked a question on StackOverflow. If anyone has insight, that would be helpful (feel free to answer here). https://stackoverflow.com/questions/45449127/scheduling-repeating-events-handler-postdelayed-and-doze

smichel17 commented 7 years ago

For gradual fade, I'm going to use a scheduledExecutorService, as it's encouraged not to use AlarmManager for timing operations while an app is running. However, that's tangential to this issue.

smichel17 commented 7 years ago

@willemw12 Sorry it took me so long to get around to this, you can stop running your own fork now :P (or rather, in a week or so when I release v3.2.0 -- in the meantime, what's in master should be pretty stable if you want to test it yourself).

willemw12 commented 7 years ago

The schedule timer still does not work.

Running 3.1.2-e7b79b2 (master branch, f-droid variant). "Sunrise to sunset" is disbled. I am letting the device doze off before the timer goes off.

smichel17 commented 7 years ago

Okay, that's bizzare. The alarmManagerCompat code is the exact same thing you have:

    public static void setExactAndAllowWhileIdle(AlarmManager alarmManager, int type,
            long triggerAtMillis, PendingIntent operation) {
        if (Build.VERSION.SDK_INT >= 23) {
            alarmManager.setExactAndAllowWhileIdle(type, triggerAtMillis, operation);
        } else {
            AlarmManagerCompat.setExact(alarmManager, type, triggerAtMillis, operation);
        }
    }
smichel17 commented 7 years ago

Could you try building from the 'doze' branch (f813367), to see if changing the type to RTC_WAKEUP has any effect?

smichel17 commented 7 years ago

@willemw12 In e7b79b2, if an alarm triggers while in doze mode, it should be postponed until the device wakes from doze mode; we don't need to actually wake the device up, since we only care about what happens when the screen turns on.

willemw12 commented 7 years ago

I did some more testing on the master branch (3.1.2-e7b79b2):

After rebooting the device, the timer does work. I'll keep checking the timer the coming days.

I have seen this possible issue several times before: upgrading a debug apk requires a reboot, to update the app data (under LineageOS).

smichel17 commented 7 years ago

Good to know. Closing again for now, if it pops up let me know

smichel17 commented 7 years ago

@willemw12 Alarms are not rescheduled on update, so that's probably why you need to reboot (AlarmManager alarms do not persist across reboots, so we reschedule them on boot). You can probably achieve the same thing by turning the timer off and on again.

NancyAndroid commented 6 years ago

can anyone explain that how to handle countdown timer in doze mode or active application when timer is running.

smichel17 commented 6 years ago

In general, the simplest/easiest thing to do is use setAlarmClock, which will make your alarm will go off exactly when you set it for. The downside is that you're sacrificing the power savings you'd get by using one of the less exact calls. The reason this has been such a long/confusing issue for Red Moon is that I am trying to get the most power savings I can, instead of just making it work.

Your situation is simpler; if the user set a countdown timer, they expect the alarm to go off when it finishes, not in an arbitrary time range near when it finishes. So, you should use setAlarmClock.

For the countdown itself, I would just refresh its value in onResume (I think you get an onResume call even the screen is turned back on; Red Moon has to catch screen on events because it's using an overlay, not running as the active app. Don't quote me on that, though).