you-apps / ClockYou

Privacy focused clock app built with MD3
https://you-apps.net
GNU General Public License v3.0
480 stars 33 forks source link

Alarm time wrong after switch to winter time #371

Closed hmkemppainen closed 3 weeks ago

hmkemppainen commented 3 weeks ago

Steps to reproduce

  1. Wait for winter
  2. Set an alarm for 10:00 the morning after daylight saving changes

Expected behavior

I expect an alarm at 10:00.

Actual behavior

I got woken up at 9:00.

This is reproducible: now I have to configure any alarm one hour later than I want it to ring.

Clock You version

8.1

Android version

14

Other details

alarm-1

alarm-2

Acknowledgements

JanicePolito commented 3 weeks ago

Same here

FeliksMarksen commented 3 weeks ago

Same here

Bnyro commented 3 weeks ago

I was able to reproduce this yesterday, but not today anymore. We're using Android's timezone database, possibly it has the wrong date for the time shift set?

(The time of your phone is updated via network commonly, not via Android's timezone database).

hmkemppainen commented 3 weeks ago

Regardless of where the device got its time from, I don't see how database having the wrong date could cause an alarm set at 10:42 to ring at 11:46 could make the software think that 11:46 is 3 minutes from now. Clearly both times are on the same date, it is not confused about that.

Let me propose this explanation:

Local time is 9:00 EET after the night when we switched from EEST to EET. You try to set an alarm for 10:00, which should be one hour from now.

getAlarmTime first initializes a time for today, 00:00. At 00:00, we are still in summer time:

    fun getAlarmTime(alarm: Alarm): Long {
        val calendar = GregorianCalendar()
        calendar.time = TimeHelper.currentDateTime

        // reset the calendar time to the start of the day
        calendar.set(Calendar.HOUR_OF_DAY, 0)
        calendar.set(Calendar.MINUTE, 0)
        calendar.set(Calendar.SECOND, 0)
        calendar.set(Calendar.MILLISECOND, 0)

Then it adds milliseconds to offset the alarm from 00:00:

        // add the milliseconds from the new alarm
        calendar.add(Calendar.MILLISECOND, alarm.time.toInt())

This is 10 hours' worth of milliseconds. But since the time from 00:00 EEST to 10:00 EET is actually 11 hours, we should be adding one more hour. After postponing the alarm by what should be 0 days (no-op), we invoke fixDaylightTime() to account for the DST change:

        calendar.add(Calendar.DATE, getPostponeDays(alarm, calendar))

        fixDaylightTime(calendar)

In fixDaylightTime() there are two relevant conditions:

            if (calendar.timeZone.inDaylightTime(now) && !calendar.timeZone.inDaylightTime(calendar.time)) {
                calendar.timeInMillis += calendar.timeZone.dstSavings
            } else if (!calendar.timeZone.inDaylightTime(now) && calendar.timeZone.inDaylightTime(calendar.time)) {
                calendar.timeInMillis -= calendar.timeZone.dstSavings
            }                          

Which of the conditions is true? Neither.

I'm setting the alarm at 9AM EET, and the alarm should ring at 10AM EET. Clearly both the desired time and the current time are in the same zone as far as DST is concerned, so we don't do anything. But we're still missing one hour because the reference time for the day's alarm was 00:00 EEST. Therefore, the alarm will sound one hour too early, or I have to schedule it one hour later, which is exactly what I observed.

Bnyro commented 3 weeks ago

Based on your thoughts, I created https://github.com/you-apps/ClockYou/commit/da56788fafbae888b462708c60870c673280a7ed to specifically set hour and minute instead of adding the offset to the date instance and moved getPostponeDays before setting the current time.

In theory the GregorianCalendar should now automatically use the right offset when we use calendar.timeInMillis.

If I manually change the date in my system settings now to Oct 26 and schedule an alarm for the next day, it properly schedules it in 1 day + 1 hour (so daylight savings still work), and we don't manually change the offset which fixes this issue.