Turns on when it shouldn't on Android Nougat

[jat255]

After upgrading to Android N on my 6p, red moon randomly turns on during the day when I have it set to automatic enabling. Never happened on Marshmallow.

Not sure how to debug this more, but it just kicked on at 3pm, even though the sun is very much still up! Had it set to automatic (sun) setting. When this happens, going into the app and pressing pause works for a second, but then the filter turns back on. I have to set the app to manual mode, and them switch it off in order to get it back to normal.

[smichel17]

When automatic (sun) mode is enabled, although the time preferences are grayed out, their value gets updated to show what Red Moon thinks the sunset time is. Could you check to see what those values are so we can figure out whether it's a bug in the location part or the filter part of Red Moon?

[jat255]

They seem to be correct (at least right now). 18:38 for sunset and 6:36 for sunrise.

On Sat, Sep 24, 2016, 3:28 PM smichel17 notifications@github.com wrote:

When automatic (sun) mode is enabled, although the time preferences are grayed out, their value gets updated to show what Red Moon thinks the sunset time is. Could you check to see what those values are so we can figure out whether it's a bug in the location part or the filter part of Red Moon?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/raatmarien/red-moon/issues/87#issuecomment-249382940, or mute the thread https://github.com/notifications/unsubscribe-auth/ABOBXc0Z7ObUnRrLALf2IOBSLMiBBqHGks5qtXnygaJpZM4KFuFc .

[raatmarien]

This is very weird, this seems to be happening to more users who have upgraded to Android 7. Sorry for the inconvenience! However, I can't seem to replicate it in the emulator. I will take a look in the source to see if I can find anything which may be causing it, but it is quite hard without being able to replicate it.

[smichel17]

Maybe we could push an update with more extensive logging so users experiencing the bug can send detailed logs?

[jat255]

Hmm, so today, everything worked as expected. Didn't come on when it wasn't supposed to, and came on at 6:39pm as expected. I have no idea what changed, though.

On Sun, Sep 25, 2016 at 11:55 AM, smichel17 notifications@github.com wrote:

Maybe we could push an update with more extensive logging so users experiencing the bug can send detailed logs?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/raatmarien/red-moon/issues/87#issuecomment-249429411, or mute the thread https://github.com/notifications/unsubscribe-auth/ABOBXS_oiY_aif4NPmbHRewmwINYG9s9ks5qtpmOgaJpZM4KFuFc .

Joshua Taillon Materials Science and Engineering University of Maryland, College Park www.joshuataillon.com

[WillRubin]

I'm having a similar issue as well under Nougat. If I enable turn on/off for particular times (for example enable at 21:30 and disable at 6:00) red moon enables immediately during the non bounded times. (So all the time.) I also tried reversing the times to see if they were "backwards" but that had no effect: the system dimmed immediately after pausing. My only solution was going back to manual activation.

[smichel17]

Maybe Nougat changed the contract of whatever method we use to get the time?

[WillRubin]

Possibly. Anything I can try to help determine the culprit?

[smichel17]

I don't have any more time to look into this, but it looks like we don't interact with Android until the following function in AutomaticFilterChangeReciever.java, so the bug has to be in here somewhere:

public static void scheduleNextAlarm(Context context, String time, Intent operation, boolean timeInUtc) {
    GregorianCalendar calendar = new GregorianCalendar();
    calendar.set(Calendar.HOUR_OF_DAY, Integer.parseInt(time.split(":")[0]));
    calendar.set(Calendar.MINUTE, Integer.parseInt(time.split(":")[1]));

    GregorianCalendar now = new GregorianCalendar();
    now.add(Calendar.SECOND, 1);
    if (calendar.before(now)) {
        calendar.add(Calendar.DATE, 1);
    }
    if (!timeInUtc)
        calendar.setTimeZone(TimeZone.getTimeZone("UTC"));

    if (DEBUG) Log.i(TAG, "Scheduling alarm for " + calendar.toString());

    AlarmManager alarmManager = (AlarmManager) context.getSystemService(Context.ALARM_SERVICE);

    PendingIntent pendingIntent = PendingIntent.getBroadcast(context, 0, operation, 0);

    if (android.os.Build.VERSION.SDK_INT >= 19) {
        alarmManager.setExact(AlarmManager.RTC, calendar.getTimeInMillis(), pendingIntent);
    } else {
        alarmManager.set(AlarmManager.RTC, calendar.getTimeInMillis(), pendingIntent);
    }
}

[boconnell]

Took some time to look into this because it was affecting me. To me, the bug is that in the method above, we are always changing the time zone to UTC. It doesn't make sense to imply the user's time zone. At least, as a user myself, I assume that when I'm using the date picker, it is in my local time zone. Using the "automatic (sun)" option would seem to corroborate that. Perhaps this was left over from

Now you may be wondering as I was why this bug doesn't manifest on Marshmallow. Well it so happens that the "Calendar.before" method as implemented in Marshmallow's SDK has a side effect that triggers a re-calculation of the "time in millis" based on the hour, minute, and time zone fields[0][1], but this happens before we've changed the time zone. Whereas in Nougat's SDK, the "Calendar.before" method doesn't[2] have such a side effect. So in Nougat, the re-calculation of the "time in millis" based on the individual fields isn't (correctly) done until after we've set the time zone, which is why the bug manifests in Nougat: The app is interpreting the user's time at if they are in the UTC time zone.

All that to say: I'll be submitting a PR to remove setting the time to UTC. Can confirm this fixes the bug on a Nexus 6P running Nougat and there is no regression on a Nexus 5 running Marshmallow. If we want to be safe, I can update the PR to only do the time zone change if the SDK version is <= 23.

[0] Calendar class data flow: https://android.googlesource.com/platform/libcore/+/android-7.0.0_r1/ojluni/src/main/java/java/util/Calendar.java#311 [1] Downstream from the before method - calls computeTime(): https://android.googlesource.com/platform/libcore/+/android-6.0.0_r1/luni/src/main/java/java/util/Calendar.java#1046 [2] Downstream from Calendar.before - calls computeTime() on a clone: https://android.googlesource.com/platform/libcore/+/android-7.0.0_r1/ojluni/src/main/java/java/util/Calendar.java#2600

[raatmarien]

At least, as a user myself, I assume that when I'm using the date picker, it is in my local time zone.

This is indeed the intention, I changed the time zone of the calendar to UTC, because the AlarmManager wants the time in UTC, however Calendar.getTimeInMillis() always returns the milliseconds in UTC, regardless of the time zone of the Calendar, so this was indeed an error.

Now you may be wondering as I was why this bug doesn't manifest on Marshmallow. Well it so happens that the "Calendar.before" method as implemented in Marshmallow's SDK has a side effect that triggers a re-calculation of the "time in millis" based on the hour, minute, and time zone fields[0][1], but this happens before we've changed the time zone. Whereas in Nougat's SDK, the "Calendar.before" method doesn't[2] have such a side effect. So in Nougat, the re-calculation of the "time in millis" based on the individual fields isn't (correctly) done until after we've set the time zone, which is why the bug manifests in Nougat: The app is interpreting the user's time at if they are in the UTC time zone.

This seems like quite an intricate bug, thanks for taking the time to find and fix it!