NorthernWidget / DS3231

Communicates between Arduino-programmed AVR and Maxim DS3231 RTC: splice of Ayars' (http://hacks.ayars.org/2011/04/ds3231-real-time-clock.html) and Jeelabs/Ladyada's (https://github.com/adafruit/RTClib) libraries
The Unlicense
183 stars 81 forks source link

setEpoch(): unix time (convert internally to y2k) and no 100-yr offset #65

Closed awickert closed 2 years ago

awickert commented 2 years ago

Untested in software or hardware.

This should provide the offset required to make the datum for setEpoch() be 01/01/1970 instead of 01/01/2000, as is expected both from its own documentation and from the way much of the rest of this library works (even if the DS3231 itself zeros out at 01/01/2000).

Tagging relevant folks: @hasenradball @IowaDave @bill-ba @leslieadams57

And issue #53 And this PR: https://github.com/NorthernWidget/DS3231/pull/60

I'm still not sure if tm_wday and tm_mon need their +1s. Going to look at the differences between the C++ library and the DS3231 data sheet at some point after this, and add any changes as needed to this PR.

hasenradball commented 2 years ago

Should we make so defined tests? Feeding some specific values for the Epoch and check the data?

hasenradball commented 2 years ago

@awickert you have done every needed change right?

awickert commented 2 years ago

Defined tests would be great.

I am not sure if I have done every needed change; see my notes on tm_wday and tm_mon.

Unfortunately, I will not have any hardware with me for at least one more week, so I cannot perform a hardware test of this code. I have not yet tried to compile, but can.

hasenradball commented 2 years ago

Hi Andy,

Iam actually not sure was was going wrong with the actual implementation, because I used the actual lib and tried compare the time from ESP8266 and the time from DS3231 and this are the same. see here:

grafik

Or do I anything wrong?

Do we have such a sketch were we see it is going wrong?

hasenradball commented 2 years ago

In the ESP8266 it refers to the following time libs: strftime

Were for example: Weekday as a decimal number with Sunday as 0 (0-6)

hasenradball commented 2 years ago

@awickert During Testing I missed the following member function: dayOfTheWeek()

Was this not implemented i the past?

hasenradball commented 2 years ago

@awickert Hi Andy,

I think I got the issue now correctly:

Let us make these things clear. I will try to figure out a optimal solution.

What do you think?

And waht about the missing member function dayOfWeek()?

hasenradball commented 2 years ago

My idea is to make a simple sketch which runs on ESP8266 and Arduino, feeding a defined UnixTimestamp to the setEpoch function and read out the time from DS3231. The results schouls be the same then.

Thats what I would do.

Should we create an additional example sketch for this?

awickert commented 2 years ago

My thoughts, in summary:

  1. There are two platforms that use different epochs, and so we should find a way to accommodate them. We can do this by:
    • Adding an option that we pass to the function, which should default to the value used on Arduino/AVR
    • Creating a method to detect the hardware internally
  2. dayOfTheWeek is implemented in the header only, and is fundamentally unimportant here.
  3. We should systematically find out which libraries are 0- or 1- indexed and how this compares to the data sheets.
  4. Then, fix these if needed
  5. In the end, we can write a test sketch

Sound good?

hasenradball commented 2 years ago

Hi,

just for clarification. I cannot see the definition of dayOfTheWeek.

grafik

Thus I would check then all data also the output of DayOfTheWeek.

IowaDave commented 2 years ago

@awickert @hasenradball : I really appreciate the attention being given to this issue. I can help with testing. This morning (in Iowa) I will prepare a short test sketch and run it on both AVR and 8266. Perhaps I'll test it with a SAMD Arduino also. My next post in this PR will provide the sketch and the results.


This thread includes mention of the DateTime::dayOfTheWeek() method also. May I offer an observation? These comments are fresh in mind after drafting documentation for the DateTime object.

DateTime objects have no Century property. I suspect the practical range of dates for DateTime objects may be limited to those between 01 January 2000 and 31 December 2099, for two reasons.

  1. Year is implemented as 00-99 even though it is declared as type uint16_t in the class.
  2. The constructor, DateTime::(uint32_t: t), does indeed expect the parameter to represent time since 01 January 1970 GMT; however, it then adjusts the number of seconds to represent time since 01 January 2000.

    • The constructor does this by subtracting the constant, #define SECONDS_FROM_1970_TO_2000 946684800, prior to parsing the value into the various different properties of date and time.
    • Conversely, the DateTime::unixtime() method reverses the process by adding the same constant immediately prior to returning the result.
hasenradball commented 2 years ago

@IowaDave Great I am also working on a preparing an sketch. I created a short gist to share my actual status. The thing I had to rework is the Serial.print because the AVR don 't have the prinf()

https://gist.github.com/hasenradball/4d6a9ea9fa596e9d21bcb93658c6fd4c

I updated the sketch for ESP8266 and AVR see gist.

awickert commented 2 years ago

Thanks, both.

A request: Could we leave the dayOfTheWeek question for another thread, and in general, open new threads for new topics? Feel free to open one and reference the discussion here.

hasenradball commented 2 years ago

Hi Togheter,

with my gist I can check now the output of:

The result using the offset Correction for the ESP8266 shows the following result:

grafik

Next Step is to check the output of Arduino Nano.

hasenradball commented 2 years ago

If I remove:

then I get the following results on the ESP8266.

grafik

hasenradball commented 2 years ago

The Question I have is... why does the function Serial.print(Clock.getYear(), DEC); returns value >100?

hasenradball commented 2 years ago

Thanks, both.

A request: Could we leave the dayOfTheWeek question for another thread, and in general, open new threads for new topics? Feel free to open one and reference the discussion here.

@awickert sure let us first fix the main issue of this PR. Wen can the dayOfWeek issue in an other topic.

hasenradball commented 2 years ago

I want to share an additional information:

grafik grafik

hasenradball commented 2 years ago

So in addition to the point with ESP8266 The function setEpoch is fine with this implementation, as it is in the repo.

grafik

Now the question is whats going wrong? I wil now check this implementation on the Arduino Nano.

IowaDave commented 2 years ago

I feel unsure of where things stand regarding a test sketch. Begging your pardon for possible redundancy, I have one to offer, along with results from testing on AVR (Arduino Nano ATmega328P). Will post results for 8266 and Arduino Zero soon.

GMT Time and Date: 12:25:00 P.M. 08/07/2022 = 1659875100, according to https://unixtime.org/.

DS3231 Result with AVR: 12:25:0 P.M. 8/6/52 The Century Bit is clear. DS3231 Epoch: 2606775900 // = RTClib:now().unixtime(); GMT Epoch: 1659875100 The two epoch values do not agree.

Here is the sketch. It's a great big paste job. Sorry I am not up to speed on all things Github. What is a gist?

/* Worked Example
 * Suppose a web site tells you that the "epoch" time 
 * at 12:25:00 p.m. GMT on August 7, 2022 was 1659875100.
 */

// import the library
 #include <DS3231.h>

// instantiate a DS3231 object
DS3231 myRTC;

// declre some variables for the various time parameters
byte mySecond, myMinute, myHour, myDoW, myDate, myMonth, myYear;
bool hpm, Century, h12;
DateTime myEpoch;

// Initialize a (unsigned long) variable to contain the epoch time.
time_t epochGMT = 1659875100UL;

void setup() {

  Wire.begin();
  Serial.begin(9600);
  while (!Serial);
  Serial.println();

  // Set the hour mode on the DS3231
  // (best to do this before setting the time)
  myRTC.setClockMode(true); // 12-hour mode

  // Set the time on the DS3231
  myRTC.setEpoch(epochGMT);

  // Retrieve the time parameters
  // from the DS3231
  mySecond = myRTC.getSecond();
  myMinute = myRTC.getMinute();
  myHour = myRTC.getHour( h12, hpm );
  myDoW = myRTC.getDoW();
  myDate = myRTC.getDate();
  myMonth = myRTC.getMonth( Century );
  myYear = myRTC.getYear();
  myEpoch = RTClib::now();

  // Display the GMT time and date
  Serial.println("GMT Time and Date:");
  Serial.println("12:25:00 P.M. 08/07/2022\n");

  // Print the values obtained from DS3231
  Serial.println("DS3231 Result:");
  Serial.print(myHour);
  Serial.print(":");
  Serial.print(myMinute);
  Serial.print(":");
  Serial.print(mySecond);
  Serial.print(" ");
  if (h12) {
    if (hpm) {
      Serial.print("P.M. ");
    } else {
      Serial.print("A.M. ");
    }
  }
  Serial.print(myMonth);
  Serial.print("/");
  Serial.print(myDate);
  Serial.print("/");
  Serial.println(myYear);
  Serial.print("The Century Bit is ");
  if (Century) {
    Serial.println("set.");
  } else {
    Serial.println("clear.");
  }
  Serial.print("DS3231 Epoch: ");
  Serial.println(myEpoch.unixtime(), DEC);
  Serial.print("GMT Epoch:    ");
  Serial.println(epochGMT, DEC);
  Serial.print("The two epoch values ");
  if (myEpoch.unixtime() == epochGMT) {
    Serial.println("agree.");
  } else {
    Serial.println("do not agree.");
  }

}

void loop() {
  // put your main code here, to run repeatedly:

}
hasenradball commented 2 years ago

@IowaDave see the link for the gist :-) https://gist.github.com/hasenradball/4d6a9ea9fa596e9d21bcb93658c6fd4c

hasenradball commented 2 years ago

Hi all,

see here my results for the AVR (Aduino Nano): grafik

hasenradball commented 2 years ago

@awickert @IowaDave I could now present a fix for this Issue. It is a Bugfix for the AVR platform.

See my results:

grafik

Remark:

the differences in weekday is fine because the DS3231 has the range [1-7] and the time.h lib ranges from [0-6].

My Proposed Solution:

Additionally this is the corrected setEpoch function:

// setEpoch function gives the epoch as parameter and feeds the RTC
// epoch = UnixTime and starts at 01.01.1970 00:00:00
// HINT: => the AVR time.h Lib is based on the year 2000
void DS3231::setEpoch(time_t epoch, bool flag_localtime) {
#if defined (__AVR__)
    epoch -= SECONDS_FROM_1970_TO_2000;
#endif
    struct tm tmnow;
    if (flag_localtime) {
        localtime_r(&epoch, &tmnow);
    }
    else {
        gmtime_r(&epoch, &tmnow);
    }
    setSecond(tmnow.tm_sec);
    setMinute(tmnow.tm_min);
    setHour(tmnow.tm_hour);
    setDoW(tmnow.tm_wday + 1U);
    setDate(tmnow.tm_mday);
    setMonth(tmnow.tm_mon + 1U);
    setYear(tmnow.tm_year - 100U);
}

Give me some hint how to proceed.

Thanks for all 👍

IowaDave commented 2 years ago

The proposed bugfix obtains a correct result in the DS3231 now, for the test code I offered earlier today, when run on an AVR.

The parameter for "localtime" seems to make no difference in the result of the new function. This does not surprise me, as the DS3231 makes no provision for time zones. Perhaps we do not need that parameter at all?

We still see a discrepancy between the Epoch uploaded to the DS3231 compared with the unixtime() obtained from a DateTime created with the RTClib::now() method.


GMT Time and Date: 12:25:00 P.M. 08/07/2022

Testing with 'localtime' flag = false.

DS3231 Result: 12:25:0 P.M. 8/7/22 The Century Bit is clear. DS3231 Epoch: 1660091100 GMT Epoch: 1659875100 The two epoch values do not agree.


GMT Time and Date: 12:25:00 P.M. 08/07/2022

Testing with 'localtime' flag = true.

DS3231 Result: 12:25:0 P.M. 8/7/22 The Century Bit is clear. DS3231 Epoch: 1660091100 GMT Epoch: 1659875100 The two epoch values do not agree.

IowaDave commented 2 years ago

To clarify labeling in the previous post, "DS3231 Epoch" refers to a derived value. The settings entered into the DS3231 by the setEpoch() method are immediately retrieved into a DateTime by the RTClib::now() method. Then a value is derived from that DateTime using its unixtime() method. The derived value is labeled "DS3231 Epoch".

It's clear that an Epoch timestamp does not survive the voyage into the DS3231 then back out again through a DateTime.

hasenradball commented 2 years ago

@IowaDave on which System did you test this? I assume on the Arduino, right? The arduino has no timezone information too. Yes your post about localtime could lead to such a behaviour on the Arduino. Because the localtime is used to store the „localtime“ directly into the DS3231, which allows you to use your local time on the any device connected to your DS3231. But this works when the microcontoller has info about the timezone (to be able to use localtime) like the esp8266, IF time was synchronized successfully.

This function was not able before I introduced the setEpoch method.

Please test it on the ESP8266, i used it since many month.

best regards Frank

IowaDave commented 2 years ago

The previous test was done with an AVR-based Arduino Nano, having an ATmega328P MPU.

In this post I provide results from testing on an 8266 board, as requested.

The results obtained in the DS3231 appear to be correct.

No difference is seen between the two values of the 'localtime' parameter; (false) and (true) lead to identical results.

The input "GMT Epoch" and the output result ofRTClib::now().unixtime(); remain unequal. This difference was observed with the AVR-based test, also.


Testing with 8266 Lolin Wemos D1 Mini Clone Arduino IDE 1.8.15 August 16, 2022


GMT Time and Date: 12:25:00 P.M. 08/07/2022

Testing with 'localtime' flag = true.

DS3231 Result: 12:25:0 P.M. 8/7/22 The Century Bit is clear. DS3231 Epoch: 1660091100 GMT Epoch: 1659875100 The two epoch values do not agree.


GMT Time and Date: 12:25:00 P.M. 08/07/2022

Testing with 'localtime' flag = false.

DS3231 Result: 12:25:0 P.M. 8/7/22 The Century Bit is clear. DS3231 Epoch: 1660091100 GMT Epoch: 1659875100 The two epoch values do not agree.

IowaDave commented 2 years ago

I will not in a position to work on this issue the remainder of today. I will come back to it tomorrow (Wednesday) morning, Iowa (USA) time, which is GMT - 5 hours. In other words, it will be after Noon in Greenwich, England.

Relax. We will figure this out. Thanks to all for contributing thoughts and solutions!

David

hasenradball commented 2 years ago

Hi David as I tried to mention, the localtime flag will work on the ESp8266 if the internal clock of the ESP8266 was syncronized successfully. You had to use configTime(...) and all that stuff so that the internal RTC ist synched with the NTPserver, Then it has the information about the TimeZone and up to now the localtime flag makes a difference.

hasenradball commented 2 years ago

@IowaDave @awickert I had a look to your second complain -> the Epoch does not fit together. I has to do with the following code.

// UNIX time: IS CORRECT ONLY WHEN SET TO UTC!!!
uint32_t DateTime::unixtime(void) const {
  uint32_t t;
  uint16_t days = date2days(yOff, m, d);
  t = time2long(days, hh, mm, ss);
  t += SECONDS_FROM_1970_TO_2000;  // seconds from 1970 to 2000

  return t;
}

If there is a issue that the epoch does not fit together it must have to do with the following function: time2long() or date2days.

Because the values stored in the DS3231 are then based on 01.01.2000, in all cases now, right? And coming back to UnixTime we had to add the constant SECONDS_FROM_1970_TO_2000, so the issue has to come by these functions.

The differcence of:

DS3231 Epoch: 1660091100 GMT Epoch: 1659875100

is 216. 000 s and we have 86400 s / day so it lead to 2,5 days difference in total.

This does not come from the constant SECONDS_FROM_1970_TO_2000 and not from the DS3231.

So I think we have to review the functions time2long() or date2days.

What do you think?

best regards

Frank

IowaDave commented 2 years ago

You bring good eyes to the aid of our cause. Thank you!

At the very least, it makes sense to reconsider the calculation algorithm going from date+time to an integer number of seconds.

I have an approach in mind to obtain an accurate result using only integer arithmetic.

Let us work on this. I'll share a working example within the next day or two. Will look forward also to seeing what you might choose to share.

With appreciation,

David

On Wed, Aug 17, 2022 at 7:03 AM Hasenradball @.***> wrote:

@awickert https://github.com/awickert @IowaDave https://github.com/IowaDave Maybe thats the point !!!

// number of days since 2000/01/01, valid for 2001..2099 static uint16_t date2days(uint16_t y, uint8_t m, uint8_t d) { if (y >= 2000) y -= 2000; uint16_t days = d; for (uint8_t i = 1; i < m; ++i) days += pgm_read_byte(daysInMonth + i - 1); if (m > 2 && isleapYear(y)) ++days; return days + 365 * y + (y + 3) / 4 - 1; }

Here is the calculation of 365 * y, but in general we calculate it as 365.25F

  • y

— Reply to this email directly, view it on GitHub https://github.com/NorthernWidget/DS3231/pull/65#issuecomment-1217919576, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIOVVUDL4DDSAYFMU3CYUO3VZTIK3ANCNFSM56TMML3A . You are receiving this because you were mentioned.Message ID: @.***>

IowaDave commented 2 years ago

I have a solution for calculating unixtime() correctly. The following code has been tested with success on both AVR and 8266.

Specific date and time values are entered individually, corrresponding to a Unix Timestamp of 1659875100.

A function recombines the individual date and time values into a calculated "unixtime" value.

The result agrees with the Unix Timestamp that we started with.

Comments on the code, offered for discussion:

  1. It uses only integer arithmetic. AVR and ESP8266 CPUs can have very fast and code-efficient integer multiplication opcodes but both of them lack a hardware Floating Point Unit. Using integer math avoids the need to import a software floating-point library into the program flash memory.
  2. The approach is simply to begin with SECONDS_FROM_1970_TO_2000, then progressively add seconds for the years, months, days, hours, minutes and seconds since January 1, 2000.
  3. Comments in the code describe each successive step.
  4. The code uses two arrays of integer constants as lookup tables. One array is for leap years, the other for non-leap years. This approach bypasses calculation of days in months. Instead, the number of days in the year prior to the current month is obtained directly from the relevant array.
  5. I placed the arrays in program (flash) memory. They occupy 52 bytes. My hunch is that the code to implement floating-point calculations, including
  6. Determining leap year is a simple integer division. This is appropriate for "year" values stored in a DateTime because that value is actually a two-digit offset from 2000, that is, 00 to 99, only. Understanding this allows the code to omit a call to the isleapYear() function.
  7. IMHO the DateTime class should be modified as follows:
    • Add a uint32_t property for unixtime to the protected list
    • Implement the calculation of that unixtime property in the constructors. It probably should be defined as a private function that returns a uint32_t, then call it from the constructor.
    • Redefine the DateTime::unixtime() method. Have it simply return the protected unixtime property. Doing this should not break existing code because both the parameter list (empty) and the return value (uint32_t, or time_t) would remain the same.

The code listing follows:


#define SECONDS_FROM_1970_TO_2000 946684800UL
#define SECONDS_IN_YEAR 31536000UL
#define SECONDS_IN_DAY 86400UL

const PROGMEM uint16_t normalYear[] =
  // days in the year prior to the current month
  {
      0, // to be ignored
      0, // January
     31, // February
     59, // March
     90, // April
    120, // May
    151, // June
    181, // July
    212, // August
    243, // September
    273, // October
    304, // November
    334, // December   
  };

const PROGMEM uint16_t leapYear[] =
  // days in the year prior to the current month
  {
      0, // to be ignored
      0, // January
     31, // February
     60, // March
     91, // April
    121, // May
    152, // June
    182, // July
    213, // August
    244, // September
    274, // October
    305, // November
    335, // December   
  };

// set up a struct to mimic a DateTime
struct DateLike{
  uint16_t year;
  uint8_t month;
  uint8_t day;
  uint8_t hour;
  uint8_t minute;
  uint8_t second;
  uint32_t epochTime;
  void calculateEpoch();
};

/* Define a function to Calculate the epoch timestamp. 
 * Note: found it necessary to cast all interim calculations
 * to be type uiint32_t in order to ensure the compiler
 * does not use 16-bit arithmetic.
 */

void DateLike::calculateEpoch() {
  // begin with seconds prior to January 1, 2000
  epochTime = SECONDS_FROM_1970_TO_2000;
  // add seconds for whole years since 2000
  // will add leap days after this
  epochTime += (uint32_t) year * SECONDS_IN_YEAR;
  // Leap days = (year / 4) + 1
  // because 2000 was a leap year
  // add seconds for leap days
  epochTime += (( (uint32_t) year / 4) + 1) * SECONDS_IN_DAY;

  /* The procedure to add seconds for whole months
   *  prior to the current month is as follows:
   *   - ascertain whether current year is a leap year
   *   - fetch number of days from relevant array
   *   - multiply by SECONDS_IN_DAY
   */

   if ( (year % 4) == 0 ) {
     // it is a leap year
     epochTime += (uint32_t)
       pgm_read_word_near(leapYear + (uint32_t) month)
       * SECONDS_IN_DAY;
   } else {
    // it is not a leap year
     epochTime += (uint32_t)
       pgm_read_word_near(normalYear + (uint32_t) month)
       * SECONDS_IN_DAY;
   }

   // add seconds for whole days in month
   epochTime += ( (uint32_t) day - 1) * SECONDS_IN_DAY;

   // add seconds for hour, minute and second
   epochTime +=
      (uint32_t) hour * 3600
     + (uint32_t) minute * 60
     + (uint32_t) second;
}

// declare a datelike container
DateLike myDate;

void setup() {

  Serial.begin(9600);
  while (!Serial);
  Serial.println();

  // initialize the values of myDate
  // corresponding to Unix Timestamp 1659875100
  myDate.year = 22;
  myDate.month = 8;
  myDate.day = 7;
  myDate.hour = 12;
  myDate.minute = 25;
  myDate.second = 0;
  // calculate the unix timestamp
  myDate.calculateEpoch();

  // print the result
  Serial.println(myDate.epochTime, DEC);

  // Correct output would equal 1659875100

}

void loop() {
  // nothing to see here; move along...

}
hasenradball commented 2 years ago

@IowaDave Hi David, great job! :+1: I am a little bit running out of time for this week to test this by myself, but I had some points in mind to bring in for a review. These point are:

What do you think to my points?

best regards Frank

IowaDave commented 2 years ago

Dear Frank,

Thank you for your kind words.

To your questions:

  1. Did I examine the existing functions? Not much. Your investigation gave us a good clue.
  2. uint64_t? I am not smart enough to address that question.
  3. Yes I studied the leap year rules. For DateTime we are concerned with years 2000 through 2099, only. "Evenly-divisible-by-four" is the only relevant rule during this range of years. 2000 was a leap year because evenly divisible by 400. The next time the "400" rule will apply is in the year 2400.
  4. I work with Arduino IDE, only, and almost exclusively with AVR microcontrollers.

I shall work toward a PR proposing changes to the DateTime object.

Appreciatively,

David

IowaDave commented 2 years ago

@awickert , @hasenradball I will soon submit a PR for DateTime addressing a few issues (unsuitable types, possible mis-counting of leap days) that could have bugged results under certain circumstances. Then we can continue discussion of DateTime in that PR. But for now, please allow me to address a question here. Is the design goal for DateTime to focus exclusively on the years 2000 through 2099?

Thanks

hasenradball commented 2 years ago

@IowaDave @awickert Hi Dave, for the Issue with the difference between ESP8266 and AVR Platform I made a PR regarding the setEpoch() function. To answer your question, I am 48 years old now and I don't know if I will reach the year 2099 so devil-may-care. But if we wil spent mor time in this topic I have to dig more in detail into the HW of the DS3231.

Have a nice weekend.

best regards frank

hasenradball commented 2 years ago

@awickert @IowaDave Hi both, I would appreciate if the PR #66 could be accepted and merged, to fix the issue with platform AVR. It was tested on ESP8266 and AVR.

IowaDave commented 2 years ago

Resuming work on this library as of 05 Sept 2022. It is a holiday in the U.S., so I can devote the whole day. Intend to evaluate both this PR #65 by @awickert and PR #66 by @hasenradball with the intent to resolve both PRs today.

IowaDave commented 2 years ago

Oops, closed before recording the reason why. Reopening to do that.

IowaDave commented 2 years ago

Closing this PR #65 because PR #66 incorporates #65 while adding an architecture check to make it work with both AVR and ESP8266. Issues addressed by this #65 are resolved by #66.