Countdown Timer - Oddities and Inefficiencies

Hi everyone,

I’m relatively new to programming and Arduino so bear with me! I’m making a project that’s a countdown to Christmas clock for my girlfriend. She likes Christmas (a lot). So I’m using an Arduino, an DS3231 RTC, and right now an Adafruit 1.8" TFT. Code is appended below. There’s a few things I’ve noticed during testing.

  1. My timer loses about 12 seconds per day. I thought this RTC would be more accurate than that, many resources said it’d be within a minute or two per day. Is something in my code causing this?

a) I’ve noticed that the screen sometimes visibly lags between seconds (as in, the screen displays an arbitraty second for like, 1.2 seconds instead of 1.0 seconds

  1. When the hours drops below 1 (it’s past 11pm) the number of hours displayed disappears off the screen rather than displaying 0 hours. This does not happen with the minutes or seconds display. I can’t figure out why this happens.

  2. Feel free to suggest any areas of this code you think I could improve on!

video: - YouTube

code: is attached because I exceeded the character length limit I guess.

Countdown_Clock_10-23.2.ino (9.39 KB)

I doubt that it is a good idea to reset all the prev-variables globally each millisecond, it would be more logical to reset each one in the if clause that tests it.

  1. My timer loses about 12 seconds per day. I thought this RTC would be more accurate than that, many resources said it’d be within a minute or two per day. Is something in my code causing this?

According to my fat-fingered calculations, 12 seconds is less than a minute or two. Do you somehow disagree?

void printcountdowntime() {
  setSyncProvider(RTC.get);

Quit f**king with the sync provider. Set that ONCE in setup.

  daysTilXmas = secondsUntilXmasRaw / 86400;

That 86400 should be 86400UL.

  if(month() == 1 || month() == 2 || month() == 3 && day() < 20) {

Do you KNOW the precedence of all those operators? I’ve been programming in C and C++ for more than 20 years, and I’ve yet to memorize the precedence of all the operators. Instead, I use parentheses to assure that the compiler interprets the statement MY way.

It would be far simpler to print the time until Christmas as a string, letting sprintf() take care of how many characters the string needed to contain.

12 seconds per day is way out of spec for the DS3231. Something is definitely wrong there. Exactly which RTC module do you have?

When this code runs, what do you see on the serial monitor?

 if(timeStatus()!= timeSet)
 Serial.println("Unable to sync with the RTC");
 else
 Serial.println("RTC has set the system time");

Whandall: I doubt that it is a good idea to reset all the prev-variables globally each millisecond, it would be more logical to reset each one in the if clause that tests it.

Ahh, good idea. Will update.

daysTilXmas = secondsUntilXmasRaw / 86400;

You have already included the Time library. You should use:

daysTilXmas = secondsUntilXmasRaw / SECS_PER_DAY ;

and use these predefined constants from Time.h wherever appropriate:

/* Useful Constants */
#define SECS_PER_MIN  (60UL)
#define SECS_PER_HOUR (3600UL)
#define SECS_PER_DAY  (SECS_PER_HOUR * 24UL)
#define DAYS_PER_WEEK (7UL)
#define SECS_PER_WEEK (SECS_PER_DAY * DAYS_PER_WEEK)
#define SECS_PER_YEAR (SECS_PER_WEEK * 52UL)
#define SECS_YR_2000  (946684800UL) // the time at the start of y2k

PaulS:
According to my fat-fingered calculations, 12 seconds is less than a minute or two. Do you somehow disagree?

Oops, I meant to say 1-2 minutes per year. At this rate I’ll lose about 72 minutes per year.

PaulS:
Quit f**king with the sync provider. Set that ONCE in setup.

Alrighty.

PaulS:
That 86400 should be 86400UL.

Could you explain why so I understand?

PaulS:
Do you KNOW the precedence of all those operators? I’ve been programming in C and C++ for more than 20 years, and I’ve yet to memorize the precedence of all the operators. Instead, I use parentheses to assure that the compiler interprets the statement MY way.

Good point. I’ve been programming in C++ for less than a month now, and I would barely call my self “able to program”… which is why I’m posting on here. So this code should look more like this? It should read as: If January, or if February, or before March 19 do this thing.

 if((month() == 1 || month() == 2) || (month() == 3 && day() < 20)) {

PaulS:
It would be far simpler to print the time until Christmas as a string, letting sprintf() take care of how many characters the string needed to contain.

I think I understand what you mean, but I’m not sure how to assemble each variable into a string. Could you provide an example to look at?

aebyers30: Oops, I meant to say 1-2 minutes per year. At this rate I'll lose about 72 minutes per year.

You never responded to my request for information about your RTC.

This is the RTC that I bought:

http://www.amazon.com/gp/product/B00LZCTMJM?psc=1&redirect=true&ref_=oh_aui_detailpage_o09_s00

You don't actually need the UL on the end of 86400. That number is bigger than will fit in 16 bits so the compiler will put it in 32 bits for you. You only need the UL when you need to upgrade a number smaller than int-max to an unsigned long to keep some calculation from overflowing.

If you had written it all with numbers smaller than 32767, then it would become an issue.

24 * 60 * 60 == -11901  // Math done with int and overflowed
24ul * 60 * 60 == 86400   // Math done with unsigned long, no overflow