Digital clock with many functions
Clash Royale CLAN TAG#URR8PPP
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;
up vote
8
down vote
favorite
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]) '
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)
c++ datetime arduino
add a comment |Â
up vote
8
down vote
favorite
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]) '
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)
c++ datetime arduino
1
This is not C code withlcd << labels[0] << ": ";
? Recommend tag change.
â chux
Jan 17 at 1:25
1
How isString
defined? Is it just another name forstd::string
?
â user1118321
Jan 17 at 3:51
This is in fact C code. I use the library Streaming to be able to do lcd << labels[0] < ": ";
â Sebastiaan Speck
Jan 17 at 12:33
String is a type of C
â Sebastiaan Speck
Jan 17 at 12:33
add a comment |Â
up vote
8
down vote
favorite
up vote
8
down vote
favorite
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]) '
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)
c++ datetime arduino
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]) '
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)
c++ datetime arduino
edited Jan 17 at 3:32
user1118321
10.2k11144
10.2k11144
asked Jan 16 at 15:49
Sebastiaan Speck
462
462
1
This is not C code withlcd << labels[0] << ": ";
? Recommend tag change.
â chux
Jan 17 at 1:25
1
How isString
defined? Is it just another name forstd::string
?
â user1118321
Jan 17 at 3:51
This is in fact C code. I use the library Streaming to be able to do lcd << labels[0] < ": ";
â Sebastiaan Speck
Jan 17 at 12:33
String is a type of C
â Sebastiaan Speck
Jan 17 at 12:33
add a comment |Â
1
This is not C code withlcd << labels[0] << ": ";
? Recommend tag change.
â chux
Jan 17 at 1:25
1
How isString
defined? Is it just another name forstd::string
?
â user1118321
Jan 17 at 3:51
This is in fact C code. I use the library Streaming to be able to do lcd << labels[0] < ": ";
â Sebastiaan Speck
Jan 17 at 12:33
String is a type of C
â Sebastiaan Speck
Jan 17 at 12:33
1
1
This is not C code with
lcd << labels[0] << ": ";
? Recommend tag change.â chux
Jan 17 at 1:25
This is not C code with
lcd << labels[0] << ": ";
? Recommend tag change.â chux
Jan 17 at 1:25
1
1
How is
String
defined? Is it just another name for std::string
?â user1118321
Jan 17 at 3:51
How is
String
defined? Is it just another name for std::string
?â user1118321
Jan 17 at 3:51
This is in fact C code. I use the library Streaming to be able to do lcd << labels[0] < ": ";
â Sebastiaan Speck
Jan 17 at 12:33
This is in fact C code. I use the library Streaming to be able to do lcd << labels[0] < ": ";
â Sebastiaan Speck
Jan 17 at 12:33
String is a type of C
â Sebastiaan Speck
Jan 17 at 12:33
String is a type of C
â Sebastiaan Speck
Jan 17 at 12:33
add a comment |Â
2 Answers
2
active
oldest
votes
up vote
4
down vote
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];
One could also usestd::size()
in C++17, or create it, since it shouldn't be hard.
â Incomputable
Jan 17 at 10:03
I changed the if-statement but the numberOfPages replacement didn't work. See user1118321's answer for a better solution.
â Sebastiaan Speck
Jan 17 at 14:26
add a comment |Â
up vote
4
down vote
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)
Also, don't add return
statements to the end of void
functions. It adds nothing, as the function will return without the statement.
I replaced the magic numbers as you said. I simpliefied the types (as far as my knowledge reached. I don't know how I can return multiple values with different types, so I put the main code inside a mainloop but the variables are still global. I simplied the Date.h and I made some functions for common tasks. I will post the new code in a new question
â Sebastiaan Speck
Jan 17 at 14:25
add a comment |Â
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
4
down vote
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];
One could also usestd::size()
in C++17, or create it, since it shouldn't be hard.
â Incomputable
Jan 17 at 10:03
I changed the if-statement but the numberOfPages replacement didn't work. See user1118321's answer for a better solution.
â Sebastiaan Speck
Jan 17 at 14:26
add a comment |Â
up vote
4
down vote
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];
One could also usestd::size()
in C++17, or create it, since it shouldn't be hard.
â Incomputable
Jan 17 at 10:03
I changed the if-statement but the numberOfPages replacement didn't work. See user1118321's answer for a better solution.
â Sebastiaan Speck
Jan 17 at 14:26
add a comment |Â
up vote
4
down vote
up vote
4
down vote
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];
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];
edited Jan 17 at 6:51
answered Jan 17 at 6:45
Roland Illig
10.4k11543
10.4k11543
One could also usestd::size()
in C++17, or create it, since it shouldn't be hard.
â Incomputable
Jan 17 at 10:03
I changed the if-statement but the numberOfPages replacement didn't work. See user1118321's answer for a better solution.
â Sebastiaan Speck
Jan 17 at 14:26
add a comment |Â
One could also usestd::size()
in C++17, or create it, since it shouldn't be hard.
â Incomputable
Jan 17 at 10:03
I changed the if-statement but the numberOfPages replacement didn't work. See user1118321's answer for a better solution.
â Sebastiaan Speck
Jan 17 at 14:26
One could also use
std::size()
in C++17, or create it, since it shouldn't be hard.â Incomputable
Jan 17 at 10:03
One could also use
std::size()
in C++17, or create it, since it shouldn't be hard.â Incomputable
Jan 17 at 10:03
I changed the if-statement but the numberOfPages replacement didn't work. See user1118321's answer for a better solution.
â Sebastiaan Speck
Jan 17 at 14:26
I changed the if-statement but the numberOfPages replacement didn't work. See user1118321's answer for a better solution.
â Sebastiaan Speck
Jan 17 at 14:26
add a comment |Â
up vote
4
down vote
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)
Also, don't add return
statements to the end of void
functions. It adds nothing, as the function will return without the statement.
I replaced the magic numbers as you said. I simpliefied the types (as far as my knowledge reached. I don't know how I can return multiple values with different types, so I put the main code inside a mainloop but the variables are still global. I simplied the Date.h and I made some functions for common tasks. I will post the new code in a new question
â Sebastiaan Speck
Jan 17 at 14:25
add a comment |Â
up vote
4
down vote
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)
Also, don't add return
statements to the end of void
functions. It adds nothing, as the function will return without the statement.
I replaced the magic numbers as you said. I simpliefied the types (as far as my knowledge reached. I don't know how I can return multiple values with different types, so I put the main code inside a mainloop but the variables are still global. I simplied the Date.h and I made some functions for common tasks. I will post the new code in a new question
â Sebastiaan Speck
Jan 17 at 14:25
add a comment |Â
up vote
4
down vote
up vote
4
down vote
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)
Also, don't add return
statements to the end of void
functions. It adds nothing, as the function will return without the statement.
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)
Also, don't add return
statements to the end of void
functions. It adds nothing, as the function will return without the statement.
edited Jan 17 at 12:28
Toby Speight
17.8k13491
17.8k13491
answered Jan 17 at 6:22
user1118321
10.2k11144
10.2k11144
I replaced the magic numbers as you said. I simpliefied the types (as far as my knowledge reached. I don't know how I can return multiple values with different types, so I put the main code inside a mainloop but the variables are still global. I simplied the Date.h and I made some functions for common tasks. I will post the new code in a new question
â Sebastiaan Speck
Jan 17 at 14:25
add a comment |Â
I replaced the magic numbers as you said. I simpliefied the types (as far as my knowledge reached. I don't know how I can return multiple values with different types, so I put the main code inside a mainloop but the variables are still global. I simplied the Date.h and I made some functions for common tasks. I will post the new code in a new question
â Sebastiaan Speck
Jan 17 at 14:25
I replaced the magic numbers as you said. I simpliefied the types (as far as my knowledge reached. I don't know how I can return multiple values with different types, so I put the main code inside a mainloop but the variables are still global. I simplied the Date.h and I made some functions for common tasks. I will post the new code in a new question
â Sebastiaan Speck
Jan 17 at 14:25
I replaced the magic numbers as you said. I simpliefied the types (as far as my knowledge reached. I don't know how I can return multiple values with different types, so I put the main code inside a mainloop but the variables are still global. I simplied the Date.h and I made some functions for common tasks. I will post the new code in a new question
â Sebastiaan Speck
Jan 17 at 14:25
add a comment |Â
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f185229%2fdigital-clock-with-many-functions%23new-answer', 'question_page');
);
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
1
This is not C code with
lcd << labels[0] << ": ";
? Recommend tag change.â chux
Jan 17 at 1:25
1
How is
String
defined? Is it just another name forstd::string
?â user1118321
Jan 17 at 3:51
This is in fact C code. I use the library Streaming to be able to do lcd << labels[0] < ": ";
â Sebastiaan Speck
Jan 17 at 12:33
String is a type of C
â Sebastiaan Speck
Jan 17 at 12:33