cbonsig / openclock

Open Clock: an open source hardware touchscreen digital clock
http://clock.bonsignore.com
26 stars 6 forks source link

Investigate leap year bug #11

Open cbonsig opened 11 years ago

cbonsig commented 11 years ago

By the way, have you fixed the "leap year" bug in your code? See if you can set the date to Feb. 29, 2016. That will be a Monday, by the way.

(via Robert on Tumblr site)

Robert-L commented 11 years ago

  case STATE_DATE_DAY:
    // upper right digit region: increment day
    if (tapZone == TAP_MINUTEPLUS){
      if (newMonth == 4 || newMonth == 6 || newMonth == 9 || newMonth == 11){
        if (newDay < 30) newDay++;
        else newDay = 1;
      }
      else if (newMonth == 2){
        if (newDay < 28) newDay++;  // See this line.
        else newDay = 1;
      }
      else{
        if (newDay < 31) newDay++;
        else newDay = 1;
      }
      newDisplayState = STATE_DATE_DAY;
    }

    // lower right digit region: decrement day
    else if (tapZone == TAP_MINUTEMINUS){
      if (newMonth == 4 || newMonth == 6 || newMonth == 9 || newMonth == 11){
        if (newDay > 1) newDay--;
        else newDay = 30;
      }
      else if (newMonth == 2){
        if (newDay > 1) newDay--;
        else newDay = 28; // See this line, too.
      }
      else{
        if (newDay > 1) newDay--;
        else newDay = 31;
      }
      newDisplayState = STATE_DATE_DAY;
    }

My (somewhat ugly) fix is:

  case STATE_DATE_DAY:
    // upper right digit region: increment day
    if (tapZone == TAP_MINUTEPLUS){
      if (newMonth == 4 || newMonth == 6 || newMonth == 9 || newMonth == 11){
        if (newDay < 30) newDay++;
        else newDay = 1;
      }
      else if (newMonth == 2){
        if (newDay < ((year % 4) == 0 ? 29 : 28)) newDay++;  // ugly but I expect it will work through A.D. 2099
        else newDay = 1;
      }
      else{
        if (newDay < 31) newDay++;
        else newDay = 1;
      }
      newDisplayState = STATE_DATE_DAY;
    }

    // lower right digit region: decrement day
    else if (tapZone == TAP_MINUTEMINUS){
      if (newMonth == 4 || newMonth == 6 || newMonth == 9 || newMonth == 11){
        if (newDay > 1) newDay--;
        else newDay = 30;
      }
      else if (newMonth == 2){
        if (newDay > 1) newDay--;
        else newDay = ((year % 4) == 0 ? 29 : 28); // again, ugly but I expect it will work through A.D. 2099
      }
      else{
        if (newDay > 1) newDay--;
        else newDay = 31;
      }
      newDisplayState = STATE_DATE_DAY;
    }

NOTE: year 2100 is divisible by 4 but is not a leap year. This won't bother you, though, will it? Possibly related: what happens when you try to set the year to a value greater than 2099?

cbonsig commented 11 years ago

Excellent, thanks!! I will give this a try next time I tinker with the code. I've been in hardware mode for the last few weeks leading up to Maker Faire, so I haven't touched the code at all in a few weeks. I am using the Chronodot 2.1, so the Arduino time is served by the Chronodot -- The specs for the DS3231 state that it accurately manages leap years through 2100. By then, I trust that my grandchildren will have found a solution.

http://datasheets.maximintegrated.com/en/ds/DS3231.pdf

Robert-L commented 11 years ago

No, I mean you do have a leap year bug.

Try setting the date to Feb. 29, 2016. Can you?

(I was trying to let you know about this before Maker Faire. Oh well.)