Digital clock with many functions

Question

I made a digital clock with the use of an RTC (DS3231) and an LCD (16x2).

Current functions:

• Display time
• Display date (different formats)
• Display temperature (Celsius and Fahrenheit)
• Display day of the week (English and Dutch)
• Display week number
• Display number of day in the year
• Humidity (%)
• Dew point (Celsius and Fahrenheit)
• Change summer -> winter time (this will be done automatically, once set correctly)

Future functions:

• Menu (controlled using the buttons on the LCD shield
• Alarm (already possible with predefining it at the start)
• Timer
• Stopwatch
• Local time of different locations

I want to implement these future functions as quickly as possible. Please feel free to give feedback and your opinion about what could be better/more efficient.

Github

Clock.ino:

#include <DS3232RTC.h>      // https://github.com/JChristensen/DS3232RTC
#include <Timezone.h>       // https://github.com/JChristensen/Timezone
#include <TimeLib.h>        // https://github.com/PaulStoffregen/Time
#include <Streaming.h>      // http://arduiniana.org/libraries/streaming/
#include <Wire.h>           // https://www.arduino.cc/en/Reference/Wire
#include <LiquidCrystal.h>  // https://www.arduino.cc/en/Reference/LiquidCrystal
#include <DHT.h>            // https://github.com/adafruit/DHT-sensor-library
#include "Date.h"

#define DHTPIN 2     // what pin we're connected to
#define DHTTYPE DHT22   // DHT 22  (AM2302)

// initialize the library by associating any needed LCD interface pin
// with the arduino pin number it is connected to
LiquidCrystal lcd(8, 9, 4, 5, 6, 7);

DHT dht(DHTPIN, DHTTYPE); //// Initialize DHT sensor for normal 16mhz Arduino

// constants won't change:
const String days[2][7] = {{"Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"}, {"zo", "ma", "di", "wo", "do", "vr", "za"}};
const String months[2][12] = {{"Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"}, {"jan", "feb", "mrt", "mei", "jun", "jul", "aug", "sep", "okt", "nov", "dec"}};
const String labels[5] = {"T", "RH", "DP", "D", "Wk"};

/* EXPLANATION DIFFERENT FUNCTIONS FOR CLOCK (WILL ONLY BE USED ON THE HOMEPAGE)
   TIME
   'h' = hours
   'm' = minutes
   's' = seconds
   'a' = alarm 1       (not atm)
   'A' = alarm 2       (not atm)

   WEATHER
   'T' = temperature
   'H' = humidity      (no sensor atm)
   'P' = dew point     (no sensor atm)

   DATE
   'd' = day of week
   'D' = day
   'M' = month
   'S' = month (short string)
   'Y' = year
   'w' = weeknumber
   'n' = daynumber

   MISCELLANEOUS
   'l' = current location  (need to create a array of enums/strings which can be used in Settings)

   DELIMTERS
   ':' = delimiter
   '-' = delimiter
   '/' = delimiter
   '.' = delimiter
   '|' = delimiter
   ' ' = delimiter
*/
char homepage[][16] = {{"h:m:s"}, {"d D-M-Y"}, {"w n"}, {"S"}, {"T H"}, {"P"}};

enum languages_t {EN, NL};

typedef struct {
  int hourFormat;               // 12 or 24 hour format (AM/PM is not displayed)
  uint8_t language;             // The language for several labels
  char degreesFormat;           // Celcius or Fahrenheit
  boolean labels;               // Display temperature, weeknumber and daynumber with label
  long intervalPage1;           // interval at which to refresh lcd (milliseconds)
  long switchPages;             // interval at which to switchPage 1 to 2 (milliseconds)
} Settings;

Settings default_settings = {24, NL, 'c', true, 1000, 30000};
Settings settings = {24, NL, 'c', true, 1000, 30000};

TimeChangeRule myDST = {"MDT", Last, Sun, Mar, 2, 2 * 60};  //Daylight time/Summertime = UTC + 2 hours
TimeChangeRule mySTD = {"MST", Last, Sun, Oct, 2, 1 * 60};  //Standard time/Wintertime = UTC + 1 hours
Timezone myTZ(myDST, mySTD);
TimeChangeRule *tcr;        //pointer to the time change rule, use to get TZ abbrev

unsigned long previousMillis = 0;        // will store last time lcd was updated (page 1)
unsigned long oldMillis = 0;             // will store last time lcd switched pages
int language_id;
int rowX = 0;
int rowY = 2;
int numberOfPages = (sizeof(homepage) / sizeof(char)) / 32;
float tem;
float hum;
float dew;

void setup() {
  Serial.begin(9600);

  // set up the LCD's number of columns and rows:
  lcd.begin(16, 2);

  // setSyncProvider() causes the Time library to synchronize with the
  // external RTC by calling RTC.get() every five minutes by default.
  setSyncProvider(RTC.get);
  if (timeStatus() != timeSet) lcd << ("RTC SYNC FAILED");


  pinMode(13, OUTPUT);
  digitalWrite(13, LOW);

}

void loop() {
  // check to see if it's time to refresh the lcd; that is, if the difference
  // between the current time and last time you refreshed the lcd is bigger than
  // the interval at which you want to refresh the lcd.
  unsigned long currentMillis = millis();
  defineLanguageId();
  tem = dht.readTemperature();
  hum = dht.readHumidity();
  if (currentMillis - previousMillis >= settings.intervalPage1) {
    // save the last time you refreshed the lcd
    previousMillis = currentMillis;

    // display the date and time according to the specificied order with the specified settings
    displayPage(rowX, rowY);
  }
  if (currentMillis - oldMillis >= settings.switchPages) {
    oldMillis = currentMillis;

    if (rowY == numberOfPages * 2) {
      rowX = 0;
      rowY = 2;
      lcd.clear();
    } else {
      rowX += 2;
      rowY += 2;
      lcd.clear();
    }

  }
}

void displayPage(int rowStart, int rowEnd) {

  time_t utc, local;
  utc = now();

  local = myTZ.toLocal(utc, &tcr);

  // calculate which day and week of the year it is, according to the current local time
  DayWeekNumber(year(local), month(local), day(local), weekday(local));

  lcd.setCursor(0, 0);
  // for-loop which loops through each row
  for (int row = rowStart; row < rowEnd; row++) {
    // if row == odd, then we are on the second line, so move the cursor of the lcd
    if (not(row % 2) == 0) lcd.setCursor(0, 1);
    // for-loop which loops through each char in row
    for (int pos = 0; pos < 15; pos++) {
      displayDesiredFunction(row, pos, local);
    }
  }
  return;
}

void displayDesiredFunction(int row, int pos, time_t l) {
  switch (homepage[row][pos]) {
    case 'h':
      displayHours(l);                  // display hours (use settings.hourFormat)
      break;                            
    case 'm':                           
      printI00(minute(l));              // display minutes
      break;                            
    case 's':                           
      printI00(second(l));              // display seconds
      break;                            
    case 'T':                             
      displayTemperature();             // display temperature (use settings.temperatureFormat)
      break;                            
    case 'H':                           
      displayHumidity();                // display humidity
      break;
    case 'P':
      displayDewPoint();                // display dew point (use settings.temperatureFormat)
      break;
    case 'd':
      displayWeekday(weekday(l));       // display day of week (use settings.lanuague)
      break;
    case 'D':
      printI00(day(l));                 // display day
      break;
    case 'M':
      printI00(month(l));               // display month
      break;
    case 'S':
      displayMonthShortStr(month(l));   // display month as string
      break;
    case 'Y':
      printI00(year(l));                // display year
      break;
    case 'n':
      displayNumber('d');               // display daynumber
      break;
    case 'w':
      displayNumber('w');               // display weeknumber
      break;
    case 'l':
      displayLocation();                // display current location
      break;
    case ':':
      displayDelimiter(':');            // display ':'
      break;
    case '-':
      displayDelimiter('-');            // display '-'
      break;
    case '/':
      displayDelimiter('/');            // display '/'
      break;
    case '.':
      displayDelimiter('.');            // display '.'
      break;
    case '|':
      displayDelimiter('|');            // display '|'
      break;
    case ' ':
      displayDelimiter(' ');            // display ' '
      break;
  }
}

void displayHours(time_t l) {
  (settings.hourFormat == 24) ? printI00(hour(l)) : printI00(hourFormat12(l));
  return;
}

void displayTemperature() {
  if (settings.labels) {
    lcd << labels[0] << ": ";
  }
  (settings.degreesFormat == 'c') ? lcd << int(tem) << (char)223 << "C" : lcd << int(((tem * 9) / 5) + 32) << (char)223 << "F";
}

void displayHumidity() {
  if (settings.labels) {
    lcd << labels[1] << ": ";
  }
  lcd << int(hum) << "%";
}

void displayDewPoint() {
  dew = calculateDewPoint(tem, hum);
  if (settings.labels) {
    lcd << labels[2] << ": ";
  }
  (settings.degreesFormat == 'c') ? lcd << int(dew) << (char)223 << "C" : lcd << int(((dew * 9) / 5) + 32) << (char)223 << "F";
}

float calculateDewPoint(float t, float h) {
  float a = 17.271;
  float b = 237.7;
  float temp = (a * t) / (b + t) + log(h * 0.01);
  float Td = (b * temp) / (a - temp);
  return Td;
}

void displayWeekday(int val)
{
  lcd << days[language_id][val - 1];
  return;
}

void displayNumber(char val) {
  if (val == 'd') {
    if (settings.labels == true) {
      lcd << labels[3] << ": ";
    }
    printI00(DW[0]);
  }
  else {
    if (settings.labels == true) {
      lcd << labels[4] << ": ";
    }
    printI00(DW[1]);
  }
  return;
}

void displayMonthShortStr(int val) {
  lcd << months[language_id][val];
  return;
}

void displayLocation() {
  //lcd << settings.location;
}

void defineLanguageId() {
  switch (settings.language) {
    case EN:
      language_id = 0;
      break;
    case NL:
      language_id = 1;
  }
}

void printI00(int val)
{
  if (val < 10) lcd << '0';
  lcd << _DEC(val);
  return;
}

void displayDelimiter(char delim) {
  lcd << delim;
  return;
}

void setTimeRTC(unsigned int hours, unsigned int minutes, unsigned int seconds, unsigned int d, unsigned int m, unsigned int y) {
  setTime(hours, minutes, seconds, d, m, y);
  RTC.set(now());
  return;
}

Date.h:

short DW[2];

void DayWeekNumber(unsigned int y, unsigned int m, unsigned int d, unsigned int w) {
  int ndays[] = {0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334};  // Number of days at the beginning of the month in a not leap year.
  //Start to calculate the number of day
  if (m == 1 || m == 2) {
    DW[0] = ndays[(m - 1)] + d;                 //for any type of year, it calculate the number of days for January or february
  }                        // Now, try to calculate for the other months
  else if ((y % 4 == 0 && y % 100 != 0) ||  y % 400 == 0) { //those are the conditions to have a leap year
    DW[0] = ndays[(m - 1)] + d + 1; // if leap year, calculate in the same way but increasing one day
  }
  else {                                //if not a leap year, calculate in the normal way, such as January or February
    DW[0] = ndays[(m - 1)] + d;
  }
  // Now start to calculate Week number
  (w == 0) ? DW[1] = (DW[0] - 7 + 10) / 7 : DW[1] = (DW[0] - w + 10) / 7;
  return;
}

Answer 1

Just a single thing I noticed:

if (not(row % 2) == 0)

This code works, but only accidentally. You should have written either of these instead:

if (not(row % 2 == 0))
if (row % 2 != 0)

The code you currently have takes the remainder of the division (0 or 1) and then performs a Boolean operation on it (the not). The result of that negation (true or false) is then converted back to an integer (1 or 0) and compared with zero. That's too much conversion between different types.

This pattern only works for comparing with 0. When you compare with 2, it fails:

not(row % 3) == 2

Since not can only ever return true or false, it can never be equal to 2.

---

And another:

int numberOfPages = (sizeof(homepage) / sizeof(char)) / 32;

This should be:

size_t numberOfPages = sizeof homepage / sizeof homepage[0];

Answer 2

This looks like a really fun project! It's written in a fairly straightforward way, so I think I get most of it. Here are some suggestions for improving it.

Avoid Magic Numbers

You have a lot of bare numbers in your code. Some are obvious, but some are not and there's no explanation of what they are. Furthermore, if you want to change any of them, you'll need to find all instances of them and change each instance. For example, in your constant declarations, you use the numbers 2, 7, 12 and 5. I would make constants for these, like this:

const size_t kNumSupportedLanguages = 2;
const size_t kNumDaysPerWeek = 7;
const size_t kNumMonthsPerYear = 12;
const size_t kNumLabels = 5;

I'd probably do the same for the millisecond values:

const long kOneSecInMS = 1000;
const long kThirtySecsInMS = 30000;

What does the 32 represent in this line:

int numberOfPages = (sizeof(homepage) / sizeof(char)) / 32;

Whatever it is, make a constant for it! Likewise with 9600, 16, 2, and 13 in the setup() function. In fact, if you did that, you wouldn't even need the comment above the call to lcd.begin(). It would just be lcd.begin(kNumLCDCols, kNumLCDRows);. In displayTemperature() you cast the value 233 to a char. Why? What does it represent? If it's the degree symbol, just give it a name.

Simplify With Types

Your Settings struct has several fields that can only have 2 values, but take up more space. I know that embedded applications are often very short on memory, so it's good to keep things small. I'm not sure if that's the case here, but you could easily make a single 8-bit field and use 1 bit for the hour format, 1 bit for language (if you know you won't ever need more languages), 1 bit for the degree format, and 1 bit for whether to show labels or not. That's 7 bytes in your current struct (though probably more due to alignment issues), compressed into a single byte.

Even if you don't do that, I think using an enum similar to the one you have for languages_t would be a good idea for the other 3 fields that only have 2 possible values.

Avoid Global Variables

You have a few dozen global variables in your code. Global variables are a pain to work with because when they get a bad value in them, it can be impossible to track down where it changed, since every part of the code has access to them. I don't see a main() function anywhere in your code, so I'm not sure how this all runs in practice. I would recommend putting all your globals in your main() function (or whatever function eventually calls through to the functions you've written here), and passing them into each function. By passing them into each function you can see the flow of data more easily when debugging.

Don't Declare Variables in Headers

In your Date.h file you declare a global variable DW. This is will cause the variable to be created in every file that #includes that header. And since there are no header guards around the file, if you include the header, and then also include another header that includes Date.h, you'll get a compile error because you'll be redeclaring the variable in the same scope.

Use if When It Would Be More Clear

You've used a ternary operator in a very hard-to-understand way in several places like this:

(w == 0) ? DW[1] = (DW[0] - 7 + 10) / 7 : DW[1] = (DW[0] - w + 10) / 7;

It would be much clearer to just write a simple if statement:

if (w == 0)
{
    DW[1] = (DW[0] - 7 + 10) / 7;
}
else
{
    DW[1] = (DW[0] - w + 10) / 7;
}

If you really really want to use a ternary operator for some reason, at least write it in the expected order:

DW[1] = (w == 0) ? (DW[0] - 7 + 10) / 7 : (DW[0] - w + 10) / 7;

Or even:

int subVal = (w == 0) ? 7 : w;
DW [ 1 ] = (DW[0] - subVal + 10) / 7;

Make Functions for Common Tasks

There are some long lines of code you have repeated in several places. This is another area where you have the potential for a bug in only 1 version that will be hard to track down, or you'll change one and need to update all of them. If you just put it in a function, it's easier to read and doesn't suffer from those problems. For example, your conversion from Fahrenheit to Celsius. Just make a function:

int FahrenheitToCelsius(const float celsius)
{
    return static_cast<int>(celsius * 9 / 5 + 32);
}

That will likely inlined by the compiler, so it shouldn't be any less efficient.

Simplify

The function in Date.h is overly complicated. I would rewrite it like this:

void DayWeekNumber(unsigned int y, unsigned int m, unsigned int d, unsigned int w) {
  int ndays[] = {0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334};  // Number of days at the beginning of the month in a not leap year.
  int numLeapDaysToAdd = 0;
  if ((y % 4 == 0 && y % 100 != 0) ||  y % 400 == 0) {
    numLeapDaysToAdd = 1;
  }

  DW[0] = ndays[(m - 1)] + d + numLeapDaysToAdd;

  // Now start to calculate Week number
  if (w == 0)
  {
     DW[1] = (DW[0] - 7 + 10) / 7;
  } else {
     DW[1] = (DW[0] - w + 10) / 7;
  }
}

Also, don't add return statements to the end of void functions. It adds nothing, as the function will return without the statement.