joeycastillo / Sensor-Watch

A board replacement for the classic Casio F-91W wristwatch
Other
1.03k stars 210 forks source link

Fix for incorrect conversion from `watch_date_time` to Unix time. #262

Closed rieck closed 8 months ago

rieck commented 11 months ago

This pull request aims at fixing an issue with time conversion. In my understanding, the current implementation of watch_utility_date_time_to_unix_time is incorrect. The function converts a watch_date_time struct to a Unix timestamp. However, It does not return the correct timestamp for leap years. I couldn't figure out exactly where the incorrect calculation occurs, but the following test cases help illustrate the problem:

The following code displays both values in the simulator.

    watch_date_time wd;
    wd.reg = 0;

    wd.unit.year = 0; // leap year
    wd.unit.day = wd.unit.month = 1;
    printf("t=%lu\n", watch_utility_date_time_to_unix_time(wd, 0));

    wd.unit.year = 1; // non-leap year
    printf("t=%lu\n", watch_utility_date_time_to_unix_time(wd, 0));    

In the current implementation the first test case returns 1577923200 (incorrect) and the second 1609459200 (correct).

Instead of reverse engineering the current implementation of watch_utility_convert_to_unix_time, I adapted the corresponding code from musl libc, which has already been used in other parts of the project. The code is surprisingly complex and I suspect that the old version misses some edge cases. With this pull request, both Unix times are now calculated correctly.

I am a little nervous about this fix, as it affects a large number of watch faces (16 complications and 3 clocks). If this is a misunderstanding on my part, I apologize for the inconvenience and the false alarm.

Best regards, Konrad

rieck commented 11 months ago

The changes aim at fixing #261.