Skip to content

RTC Superclass #215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Harrison-O wants to merge 47 commits into
base: master
Choose a base branch
from
Open

RTC Superclass #215

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
cb0bf8d
Wrote `isLeapYear()` function
Jul 30, 2020
0af4be5
Wrote `fixDateTime()` method to fix invalid DateTime objects
Jul 30, 2020
6f49b2b
Write additional comment specifying that if DateTime is already valid,
Jul 30, 2020
bca61c2
Merge pull request #1 from Harrison-O/DateTime_eqaulity
Harrison-O Jul 30, 2020
ba9c0ed
Merge pull request #2 from Harrison-O/fixDateTime
Harrison-O Jul 30, 2020
ae82978
Merge pull request #3 from adafruit/master
Harrison-O Dec 28, 2020
dded178
Merge pull request #4 from Harrison-O/master
Harrison-O Dec 28, 2020
7654480
Wrote RTC superclass
Dec 28, 2020
e3757d3
Modified RTClib.h to use RTC.h
Dec 28, 2020
9dfa49b
Fixed IDE errors
Dec 28, 2020
00075a4
Fixed compile errors
Dec 28, 2020
eaec406
Moved _I2C_WRITE and _I2C_READ to RTClib.cpp
Dec 29, 2020
68bed44
Wrote new keywords
Dec 29, 2020
54fcd63
Fixed usage of millisPerSecond and RTC_Millis::adjustDrift()
Dec 29, 2020
2b98009
Updated doxygen comments for adjustDrift()
Dec 29, 2020
4a25373
Wrote second `begin()` function for RTC classes
Dec 29, 2020
c1001bc
Wrote adjustDraft() functions for RTC classes
Dec 29, 2020
f00f0af
Removed adjustDrift() for future pull request
Dec 29, 2020
58e5432
Made RTC superclass public for subclasses
Dec 29, 2020
1614120
Fixed compile error with begin(const DateTime&)
Dec 29, 2020
de9a42e
Removed fixDateTime() function
Dec 29, 2020
189ddc9
Fixed file placement in utility folder
Dec 29, 2020
0975cc5
Ensured that TimeSpan.h file includes Arduino.h
Dec 29, 2020
3ab7669
Removed COMPILE_DT constant due to compile error
Dec 29, 2020
3721491
Removed adjustDrift() from RTC for future pull request
Dec 29, 2020
290bea2
Fixed build errors
Dec 29, 2020
41b522d
Fixed adjustDrift() for RTC_Millis
Dec 30, 2020
2f99da8
Fixed micro to milli typo
Dec 31, 2020
edc0af4
Fixed clang formatting issues
Dec 31, 2020
fd67d34
Wrote adjustDrift() back in to enforce consistency
Dec 31, 2020
80b5650
:w
Dec 31, 2020
7213586
Fixed clang formatting issues
Dec 31, 2020
5eba8f6
Removed millisPerSecond from RTC_Millis
Dec 31, 2020
db255bf
Fixed compile error involving millisPerSecond
Jan 1, 2021
e2a981c
Renamed RTC_Super to RTC to find RTC macro
Jan 1, 2021
8f7e656
Changed begin(DateTime&) to non-pure virtual
Jan 2, 2021
c13d1c4
Renamed RTC_Super to RTC_Base
Jan 2, 2021
56f3426
Changed begin(void) method for pseudo RTCs
Jan 2, 2021
678f814
Updated doxygen comment
Jan 2, 2021
2e664b4
Renamed RTC_Base to RealTimeClock
Jan 3, 2021
3151cf7
Wrote note for RealTimeClock class
Jan 3, 2021
b8d8b77
Changed ppm to non-const
Jan 3, 2021
266cc68
Made lostPower() virtual in base class
Jan 4, 2021
84fbf61
Commented out adjustDrift() for future versions
Jan 4, 2021
ad85753
Fixed clang formatting issue
Jan 5, 2021
5cee6ba
Fixed clang formatting issue
Jan 5, 2021
a5a037d
Fixed clang formatting issue
Jan 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 115 additions & 25 deletions RTClib.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,18 @@
#include <Wire.h>
#endif

#if (ARDUINO >= 100)
#include <Arduino.h> // capital A so it is error prone on case-sensitive filesystems
// Macro to deal with the difference in I2C write functions from old and new
// Arduino versions.
#define _I2C_WRITE write ///< Modern I2C write
#define _I2C_READ read ///< Modern I2C read
#else
#include <WProgram.h>
#define _I2C_WRITE send ///< Legacy I2C write
#define _I2C_READ receive ///< legacy I2C read
#endif

/**************************************************************************/
/*!
@brief Read a byte from an I2C register
Expand Down Expand Up @@ -105,6 +117,22 @@ static uint8_t bin2bcd(uint8_t val) { return val + 6 * (val / 10); }
/**************************************************************************/
/*!
@brief Start I2C for the DS1307 and test succesful connection
@return True if Wire can find DS1307 or false otherwise.
@note Preserves the date/time on the RTC
*/
/**************************************************************************/
boolean RTC_DS1307::begin(void) {
Wire.begin();
Wire.beginTransmission(DS1307_ADDRESS);
if (Wire.endTransmission() == 0)
return true;

return false;
}

/**************************************************************************/
/*!
@brief Start I2C for the DS1307, test succesful connection, then initialise the date/time
@param dt DateTime object containing desired date/time
@return True if Wire can find DS1307 or false otherwise.
*/
Expand Down Expand Up @@ -166,18 +194,6 @@ void RTC_DS1307::adjust(const DateTime &dt) {
Wire.endTransmission();
}

/**************************************************************************/
/*!
@brief Adjust the RTC clock to compensate for system clock drift
@param drift Adjustment to make in seconds
@note Positive values make the clock go ahead in time and vice-versa
*/
/**************************************************************************/
void RTC_DS1307::adjustDrift(const int drift) {
DateTime newDt = DateTime(now().unixtime() + drift);
adjust(newDt);
}

/**************************************************************************/
/*!
@brief Get the current date and time from the DS1307
Expand Down Expand Up @@ -302,10 +318,21 @@ void RTC_DS1307::writenvram(uint8_t address, uint8_t data) {
rollover issues. Note that lastMillis is **not** the millis() value
of the last call to now(): it's the millis() value corresponding to
the last **full second** of Unix time. */
uint32_t RTC_Millis::millisPerSecond = 1000;
uint32_t RTC_Millis::microsPerSecond = 1000000L;
uint32_t RTC_Millis::lastMillis;
uint32_t RTC_Millis::lastUnix;

/**************************************************************************/
/*!
@brief Start the RTC_Millis date/time
@return true
@note Equivalent to `begin(DateTime(F(__DATE__), F(__TIME__)))`
*/
/**************************************************************************/
boolean RTC_Millis::begin(void) {
adjust(DateTime(F(__DATE__), F(__TIME__)));
return true;
}
/**************************************************************************/
/*!
@brief Start the RTC_Millis date/time
Expand All @@ -331,12 +358,12 @@ void RTC_Millis::adjust(const DateTime &dt) {

/**************************************************************************/
/*!
@brief Adjust the RTC clock to compensate for system clock drift
@param drift Adjustment to make in milliseconds
@note Positive values make the clock go ahead in time and vice-versa
@brief Adjust the RTC clock speed to compensate for system clock drift
@param ppm Parts per million to adjust clock speed by
@note Positive values make the clock faster and vice-versa
*/
/**************************************************************************/
void RTC_Millis::adjustDrift(int drift) { lastMillis = 1000 - drift; }
void RTC_Millis::adjustDrift(const int ppm) { microsPerSecond = 1000000L - ppm; }

/**************************************************************************/
/*!
Expand All @@ -347,20 +374,32 @@ void RTC_Millis::adjustDrift(int drift) { lastMillis = 1000 - drift; }
*/
/**************************************************************************/
DateTime RTC_Millis::now() {
uint32_t elapsedSeconds = (millis() - lastMillis) / millisPerSecond;
lastMillis += elapsedSeconds * millisPerSecond;
uint32_t elapsedSeconds = (millis() - lastMillis) * 1000 / microsPerSecond;
Copy link
Contributor

@edgar-bonet edgar-bonet Dec 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The multiplication will overflow unless now() is called at least once every 71.6 minutes (and not 49.7 days, as stated in the comment). This is the same constraint we already have with RTC_Micros. Given that this constraint is the only drawback of RTC_Micros vs. RTC_Millis, this change makes RTC_Millis redundant: it could be replaced by an alias to RTC_Micros.

Copy link
Author

@Harrison-O Harrison-O Dec 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed by reducing RTC_Millis adjust drift precision to parts per thousand.

Copy link
Contributor

@edgar-bonet edgar-bonet Dec 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parts per thousand is an extremely coarse resolution for tuning a clock. Also note that, because of the way the integer division rounds, if the user types

clock.adjustDrift(+12);  // +12 ppm is about one second per day faster

his clock will speed up by +1,000 ppm, or about 86 seconds per day.

Copy link
Author

@Harrison-O Harrison-O Dec 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't clear enough in my wording, it's changed to a resolution of parts per thousand but the user still inputs it in parts per million. This means that

RTC_Millis rtc;
rtc.adjustDrift(12);

Will have no effect. Please check my latest commit.

Even then, the programmer using adjustDrift() will be responsible for ensuring the drift adjustment is appropriate, not us; if they input some ridiculously high drift then that's their fault.

I plan on implementing ppm resolution to RTC_Millis in a separate pull request.

Copy link
Contributor

@edgar-bonet edgar-bonet Dec 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your wording, and your code, were quite clear.

rtc.adjustDrift(12);

will have the effect of setting

millisPerSecond = (1000000L - 12) / 1000;

The right hand side of this expression, if evaluated as real numbers, would give 999.988. However, this is an integer division, and in C++ integer division rounds towards zero, which results in setting millisPerSecond to 999. This is a +1,000 ppm speed correction, which is both huge and very far from what the user requested.

the programmer using adjustDrift() will be responsible for ensuring the drift adjustment is appropriate, not us

Of course! But if they find, through careful calibration, that +12 ppm is the appropriate correction, they can reasonably expect that rtc.adjustDrift(12); will apply a correction that is not too far from what they requested. The library should avoid breaking such reasonable user expectations.

Copy link
Author

@Harrison-O Harrison-O Dec 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, this is clearly becoming overly-complicated for what was supposed to be a very simple pull request. For now, I've just removed the adjustDrift() function from RTC_Millis. I've wrote adjustDrift() back into RTC_Super to enforce consistency across the RTC classes.

lastMillis += elapsedSeconds * microsPerSecond;
Copy link
Contributor

@edgar-bonet edgar-bonet Dec 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be elapsedSeconds * microsPerSecond / 1000.0. But then, extra bits would be needed in order to also store the fractional part of lastMillis, presumably in a fixed-point format.

Copy link
Author

@Harrison-O Harrison-O Dec 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed by reducing RTC_Millis adjust drift precision to parts per thousand. May be increased without overflow error if modulo operator is used. Will investigate in future pull request for adjustDrift()

lastUnix += elapsedSeconds;
return lastUnix;
}

/** Number of microseconds reported by micros() per "true" (calibrated) second.
*/
uint32_t RTC_Micros::microsPerSecond = 1000000;
uint32_t RTC_Micros::microsPerSecond = 1000000L;

/** The timing logic is identical to RTC_Millis. */
uint32_t RTC_Micros::lastMicros;
uint32_t RTC_Micros::lastUnix;

/**************************************************************************/
/*!
@brief Start the RTC_Micros date/time
@return true
@note Equivalent to `begin(DateTime(F(__DATE__), F(__TIME__)))`
*/
/**************************************************************************/
boolean RTC_Micros::begin(void) {
adjust(DateTime(F(__DATE__), F(__TIME__)));
return true;
}

/**************************************************************************/
/*!
@brief Start the RTC_Micros date/time
Expand All @@ -386,12 +425,12 @@ void RTC_Micros::adjust(const DateTime &dt) {

/**************************************************************************/
/*!
@brief Adjust the RTC_Micros clock to compensate for system clock drift
@param drift Adjustment to make in microseconds
@brief Adjust the RTC_Micros clock speed to compensate for system clock drift
@param ppm Parts per million drift rate adjustment
@note Positive values make the clock faster and vice-versa
*/
/**************************************************************************/
// A positive adjustment makes the clock faster.
void RTC_Micros::adjustDrift(int drift) { microsPerSecond = 1000000 - drift; }
void RTC_Micros::adjustDrift(const int ppm) { microsPerSecond = 1000000L - ppm; }

/**************************************************************************/
/*!
Expand All @@ -410,6 +449,23 @@ DateTime RTC_Micros::now() {
/*!
@brief Start I2C for the PCF8523 and test succesful connection
@return True if Wire can find PCF8523 or false otherwise.
@note Preserves the date/time on the RTC
*/
/**************************************************************************/
boolean RTC_PCF8523::begin(void) {
Wire.begin();
Wire.beginTransmission(PCF8523_ADDRESS);
if (Wire.endTransmission() == 0)
return true;

return false;
}

/**************************************************************************/
/*!
@brief Start I2C for the PCF8523 and test succesful connection
@param dt DateTime object containing desired date/time
@return True if Wire can find PCF8523 or false otherwise.
*/
/**************************************************************************/
boolean RTC_PCF8523::begin(const DateTime &dt) {
Expand Down Expand Up @@ -504,7 +560,7 @@ DateTime RTC_PCF8523::now() {

/**************************************************************************/
/*!
@brief Resets the STOP bit in register Control_1
@brief resets the stop bit in register control_1
*/
/**************************************************************************/
void RTC_PCF8523::start(void) {
Expand Down Expand Up @@ -740,6 +796,23 @@ void RTC_PCF8523::calibrate(Pcf8523OffsetMode mode, int8_t offset) {
/*!
@brief Start I2C for the PCF8563 and test succesful connection
@return True if Wire can find PCF8563 or false otherwise.
@note Preserves the date/time on the RTC
*/
/**************************************************************************/
boolean RTC_PCF8563::begin(void) {
Wire.begin();
Wire.beginTransmission(PCF8563_ADDRESS);
if (Wire.endTransmission() == 0)
return true;

return false;
}

/**************************************************************************/
/*!
@brief Start I2C for the PCF8563 and test succesful connection
@param dt DateTime object containing desired date/time
@return True if Wire can find PCF8563 or false otherwise.
*/
/**************************************************************************/
boolean RTC_PCF8563::begin(const DateTime &dt) {
Expand Down Expand Up @@ -896,6 +969,23 @@ static uint8_t dowToDS3231(uint8_t d) { return d == 0 ? 7 : d; }
/*!
@brief Start I2C for the DS3231 and test succesful connection
@return True if Wire can find DS3231 or false otherwise.
@note Preserves the date/time on the RTC
*/
/**************************************************************************/
boolean RTC_DS3231::begin(void) {
Wire.begin();
Wire.beginTransmission(DS3231_ADDRESS);
if (Wire.endTransmission() == 0)
return true;

return false;
}

/**************************************************************************/
/*!
@brief Start I2C for the DS3231 and test succesful connection
@param dt DateTime object containing desired date/time
@return True if Wire can find DS3231 or false otherwise.
*/
/**************************************************************************/
boolean RTC_DS3231::begin(const DateTime &dt) {
Expand Down
58 changes: 24 additions & 34 deletions RTClib.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
#define _RTCLIB_H_

#include <Arduino.h>
#include "utility/RTC/RTC.h"
#include "utility/RTC.h"
#include "utility/DateTime.h"
#include "utility/TimeSpan.h"

/** Registers */
#define PCF8523_ADDRESS 0x68 ///< I2C address for PCF8523
Expand Down Expand Up @@ -57,18 +59,6 @@
0x11 ///< Temperature register (high byte - low byte is at 0x12), 10-bit
///< temperature value

#if (ARDUINO >= 100)
#include <Arduino.h> // capital A so it is error prone on case-sensitive filesystems
// Macro to deal with the difference in I2C write functions from old and new
// Arduino versions.
#define _I2C_WRITE write ///< Modern I2C write
#define _I2C_READ read ///< Modern I2C read
#else
#include <WProgram.h>
#define _I2C_WRITE send ///< Legacy I2C write
#define _I2C_READ receive ///< legacy I2C read
#endif

/** DS1307 SQW pin mode settings */
enum Ds1307SqwPinMode {
DS1307_OFF = 0x00, // Low
Expand All @@ -84,11 +74,11 @@ enum Ds1307SqwPinMode {
@brief RTC based on the DS1307 chip connected via I2C and the Wire library
*/
/**************************************************************************/
class RTC_DS1307 : RTC {
class RTC_DS1307 : public RTC {
public:
boolean begin(const DateTime &dt = DateTime::COMPILE_DT);
boolean begin(void);
boolean begin(const DateTime &dt);
void adjust(const DateTime &dt);
void adjustDrift(int drift);
bool isrunning(void);
bool lostPower(void);
DateTime now();
Expand Down Expand Up @@ -138,11 +128,11 @@ enum Ds3231Alarm2Mode {
@brief RTC based on the DS3231 chip connected via I2C and the Wire library
*/
/**************************************************************************/
class RTC_DS3231 : RTC {
class RTC_DS3231 : public RTC {
public:
boolean begin(const DateTime &dt = DateTime::COMPILE_DT);
boolean begin(void);
boolean begin(const DateTime &dt);
void adjust(const DateTime &dt);
void adjustDrift(const int drift);
bool isrunning(void);
bool lostPower(void);
DateTime now();
Expand Down Expand Up @@ -205,9 +195,10 @@ enum Pcf8523OffsetMode {
@brief RTC based on the PCF8523 chip connected via I2C and the Wire library
*/
/**************************************************************************/
class RTC_PCF8523 : RTC {
class RTC_PCF8523 : public RTC {
public:
boolean begin(const DateTime &dt = DateTime::COMPILE_DT);
boolean begin(void);
boolean begin(const DateTime &dt);
void adjust(const DateTime &dt);
boolean isrunning(void);
boolean lostPower(void);
Expand Down Expand Up @@ -242,9 +233,10 @@ enum Pcf8563SqwPinMode {
*/
/**************************************************************************/

class RTC_PCF8563 : RTC {
class RTC_PCF8563 : public RTC {
public:
boolean begin(const DateTime &dt = DateTime::COMPILE_DT);
boolean begin(void);
boolean begin(const DateTime &dt);
boolean isrunning(void);
boolean lostPower(void);
void adjust(const DateTime &dt);
Expand All @@ -261,9 +253,10 @@ class RTC_PCF8563 : RTC {
use. NOTE: this is immune to millis() rollover events.
*/
/**************************************************************************/
class RTC_Millis : RTC {
class RTC_Millis : public RTC {
public:
boolean begin(const DateTime &dt = DateTime::COMPILE_DT);
boolean begin(void);
boolean begin(const DateTime &dt);
/*!
@brief Simulate if the RTC is running
@return true
Expand All @@ -275,11 +268,11 @@ class RTC_Millis : RTC {
*/
boolean lostPower(void) { return false; }
void adjust(const DateTime &dt);
void adjustDrift(const int drift);
void adjustDrift(const int ppm);
DateTime now();

protected:
static uint32_t millisPerSecond; ///< Number of milliseconds reported by
static uint32_t microsPerSecond; ///< Number of milliseconds reported by
///< millis() per "true" (calibrated) second
static uint32_t lastUnix; ///< Unix time from the previous call to now() -
///< prevents rollover issues
Expand All @@ -296,13 +289,10 @@ class RTC_Millis : RTC {
approximately 71.6 minutes.
*/
/**************************************************************************/
class RTC_Micros : RTC {
class RTC_Micros : public RTC {
public:
/*!
@brief Start the RTC
@param dt DateTime object with the date/time to set
*/
boolean begin(const DateTime &dt = DateTime::COMPILE_DT);
boolean begin(void);
boolean begin(const DateTime &dt);
/*!
@brief Simulate if the RTC is running
@return true
Expand All @@ -314,7 +304,7 @@ class RTC_Micros : RTC {
*/
boolean lostPower(void) { return false; }
void adjust(const DateTime &dt);
void adjustDrift(int ppm);
void adjustDrift(const int ppm);
DateTime now();

protected:
Expand Down
2 changes: 2 additions & 0 deletions keywords.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

DateTime KEYWORD1
TimeSpan KEYWORD1
RTC KEYWORD1
RTC_DS1307 KEYWORD1
RTC_DS3231 KEYWORD1
RTC_PCF8523 KEYWORD1
Expand Down Expand Up @@ -77,6 +78,7 @@ isEnabled32K KEYWORD2
#######################################
# Constants (LITERAL1)
#######################################
COMPILE_DT LITERAL1
Copy link
Contributor

@edgar-bonet edgar-bonet Dec 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not used anymore.

Copy link
Author

@Harrison-O Harrison-O Dec 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

TIMESTAMP_FULL LITERAL1
TIMESTAMP_DATE LITERAL1
TIMESTAMP_TIME LITERAL1
Expand Down
2 changes: 0 additions & 2 deletions utility/DateTime/DateTime.cpp → utility/DateTime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,6 @@ static uint32_t time2ulong(uint16_t days, uint8_t h, uint8_t m, uint8_t s) {
return ((days * 24UL + h) * 60 + m) * 60 + s;
}

const DateTime DateTime::COMPILE_DT = DateTime(F(__DATE__), F(__TIME__));

/**************************************************************************/
/*!
@brief Constructor from
Expand Down
Loading