Skip to content
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

Create EU DST example #308

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Create EU DST example #308

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7ca75cd
Create EU DST example
J-Brinkman Oct 13, 2024
e9f4efc
Update EU DST example
J-Brinkman Oct 14, 2024
5b98cfd
Update EU DST example
J-Brinkman Oct 14, 2024
5b81a04
Create DST_Europe.ino
J-Brinkman Oct 14, 2024
fc6647c
Delete examples/EU DST example
J-Brinkman Oct 14, 2024
5b55949
Update DST_Europe.ino
J-Brinkman Oct 14, 2024
d88958f
Update DST_Europe.ino
J-Brinkman Oct 21, 2024
56f8a79
Update DST_Europe.ino with enhancd dstclock function
J-Brinkman Oct 23, 2024
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
52 changes: 52 additions & 0 deletions examples/EU DST example
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/* EU DST example */
Copy link
Contributor

Choose a reason for hiding this comment

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

This deserves a few more explanations, stating for instance that:

  1. The RTC is kept in local winter time. This is not obvious at first sight, and is quite non-standard (common practice is to keep the RTC either in UTC or in local time).

  2. This code is for CET only. It can work across Europe if one is willing to accept the DST change to be one hour off.

Copy link
Author

Choose a reason for hiding this comment

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

Very relevant suggestions.
Point 1 appears to be the key to make it a very simple procedure.
Point 2 The hour is now set to 1 and 3. This can be adapted to your local time (for instance at 2 and 4) if applicable. Therefore it is an example.
I would love to add these point to a comment in the proposed code, but have problems to find out how. (This is the very first pull request I ever do).

Copy link
Contributor

Choose a reason for hiding this comment

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

This can be adapted to your local time

I believe a comment explaining how to do so would be of some value.

/* version 1: dd 23-10-2024 */
J-Brinkman marked this conversation as resolved.
Show resolved Hide resolved

#include <RTClib.h> // https://github.com/adafruit/rtclib
RTC_DS3231 rtc;
//RTC_DS1307 rtc;

#define USEDST true // Use DST (true or false)

DateTime now;
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be local to loop().

Copy link
Author

Choose a reason for hiding this comment

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

It is also used in the setup part.

Copy link
Contributor

Choose a reason for hiding this comment

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

I cannot see the variable now used in setup().

Copy link
Author

Choose a reason for hiding this comment

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

I think that is not wise to do.

  1. It is a compiler directive and good practice to declare these directives on the top of the code.
  2. If you have more complex date/time functionality, it is handy to have this boolean available in the complete program

Copy link
Contributor

Choose a reason for hiding this comment

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

Boolean? I am talking about the variable now. I agree that the macro USEDST belongs to the top of the file.


DateTime dstclock(DateTime n) { // Return the given (DST adjusted) date and time according to DST settings (aespecially for date/time checking)

DateTime b, e;

b = DateTime(n.year(), 3, 31, 3, 0, 0); b = DateTime(n.year(), 3, 31 - b.dayOfTheWeek(), 1, 0, 0); // Last sunday in march 1:00
e = DateTime(n.year(), 10, 31, 3, 0, 0); e = DateTime(n.year(), 10, 31 - e.dayOfTheWeek(), 3, 0, 0); // Last sunday in October 3:00
Copy link
Contributor

Choose a reason for hiding this comment

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

You have here two calls to dayOfTheWeek(), which is quite an expensive function. You could easily call this only once, and only in March or October: you don't need all this on any other month.

Copy link
Author

Choose a reason for hiding this comment

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

I don't understand. It is called only twice. For March as well as for October.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is always called twice. What I mean is that, most of the year, you don't need to call dayOfTheWeek() at all. From the 25th to the 31st of October, you do have to call dayOfTheWeek() once, but you do not care about the last day of March. Conversely, in March you do not care about the last day of October.

Here is a version of the function (untested) that never calls dayOfTheWeek() more than once, and only does so 14 days per year:

// Return the given DateTime adjusted for DST.
// The provided DateTime is assumed to be in UTC+01:00 (i.e. CET, with no DST correction).
// The result will be in CET, with the DST correction applied if needed.
DateTime dstclock(DateTime n) {
  bool isDst;

  if (!USEDST) {
    isDst = false;
  } else if (n.month() < 3 || (n.month() == 3 && n.day() < 25)) {
    isDST = false;  // winter time before 25 March
  } else if (n.month() == 3) {
    // DST starts on last Sunday of March at 02:00 UTC+01:00
    uint8_t dow = n.dayOfTheWeek();
    isDst = (dow == 0 && n.hour() >= 2) || (dow > 0 && n.day() - dow >= 25);
  } else if (n.month() < 10 || (n.month == 10 && n.day() < 25)) {
    isDst = true;  // summer time before 25 October
  } else if (n.month() == 10) {
    // DST ends on last Sunday of October at 02:00 UTC+01:00
    uint8_t dow = n.dayOfTheWeek();
    isDst = (dow == 0 && n.hour() < 2) || (dow > 0 && n.day() - dow < 25);
  } else {  // n.month() > 10
    isDst = false;  // winter time after October
  }

  if (isDst)
    n = n + TimeSpan(0, 1, 0, 0);

  return n;
}

This is more source code but, being an if/else chain, only one of the inner blocks is executed.

I just noticed that, given that this function expects its input to always be in UTC+01:00 (CET winter time), the DST switch always happens at 02:00.

Copy link
Author

Choose a reason for hiding this comment

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

I prefer:
b = DateTime(n.year(), 3, 31, 1, 0, 0); // Begin of DST: March 31 1:00 (am) if (month(n) = 3) b = DateTime(n.year(), 3, 31 - b.dayOfTheWeek(), 1, 0, 0); // Last sunday in March 1:00 (am) e = DateTime(n.year(), 10, 31, 3, 0, 0); // End of DST: October 31 3:00 (am) if (month(n) = 10) e = DateTime(n.year(), 10, 31 - e.dayOfTheWeek(), 3, 0, 0); // Last sunday in October 3:00 (am)
Much simpler and - since it is an example - more understandable.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, makes a lot of sense! Note, however, that the time switch should not happen at 01:00 or 03:00, but always at 02:00. This is because, as you have now documented, the RTC is kept in winter time all year long, so this function expects its parameter n to be in CET winter time (UTC+01:00). In Europe, DST transitions always happen at 01:00 UTC.


if (USEDST && (n > b) && (n < e)) n = n + TimeSpan(0,1,0,0); // adjust standard time if within summertime
return n;
};

DateTime getclock() { // Retrieve the (DST adjusted) date and time

DateTime n, b, e;

n = rtc.now();
b = DateTime(n.year(), 3, 31, 3, 0, 0); b = DateTime(n.year(), 3, 31 - b.dayOfTheWeek(), 1, 0, 0); // Last sunday in march 1:00
e = DateTime(n.year(), 10, 31, 3, 0, 0); e = DateTime(n.year(), 10, 31 - e.dayOfTheWeek(), 3, 0, 0); // Last sunday in October 3:00

if (USEDST && (n > b) && (n < e)) n = n + TimeSpan(0,1,0,0); // adjust standard time if within summertime
return n;
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid code duplication:

Suggested change
DateTime n, b, e;
n = rtc.now();
b = DateTime(n.year(), 3, 31, 3, 0, 0); b = DateTime(n.year(), 3, 31 - b.dayOfTheWeek(), 1, 0, 0); // Last sunday in march 1:00
e = DateTime(n.year(), 10, 31, 3, 0, 0); e = DateTime(n.year(), 10, 31 - e.dayOfTheWeek(), 3, 0, 0); // Last sunday in October 3:00
if (USEDST && (n > b) && (n < e)) n = n + TimeSpan(0,1,0,0); // adjust standard time if within summertime
return n;
return dstclock(rtc.now());

Copy link
Author

Choose a reason for hiding this comment

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

It is not duplicated. For both the begin and the end date the last sunday is relevant. So the function should be called twice.

Copy link
Contributor

Choose a reason for hiding this comment

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

What I mean is that getclock() could be reduced to this:

DateTime getclock() {
  return dstclock(rtc.now());
}

};

void setclock(DateTime n) { // if the clock is set during summertime then adjust the clock to standard time

if (USEDST && (rtc.now() != getclock())) n = n - TimeSpan(0,1,0,0); // if summertime then adjust to the standard time or
if (USEDST && (rtc.now() != dstclock(rtc.now()))) n = n - TimeSpan(0,1,0,0); // if summertime then adjust to the standard time or
Copy link
Contributor

Choose a reason for hiding this comment

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

You should not rely on rtc.now() here: if the user is calling this it's because the RTC is not set.

Copy link
Author

Choose a reason for hiding this comment

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

Valid point! rtc.adjust(n); should be added to line 36.
(Stil figgering out how to adjust the propsed code)

Copy link
Author

Choose a reason for hiding this comment

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

DST support v2.txt
I was not able to find out how to incorporate your valueable review comments into the code. I am very sorry about that! So I included a new version with the changes as a result of your review.

Copy link
Contributor

Choose a reason for hiding this comment

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

For amending a pull request, you can simply push extra commits to your “patch-1” branch.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, that helped!

rtc.adjust(n); // Set the clock to standard time
};

void setup() {
// initialise the rtc
rtc.begin();
if (rtc.lostPower()) setclock(DateTime(F(__DATE__), F(__TIME__))); // Set date and time for use with the DS3231 RTC
if (!rtc.isrunning()) setclock(DateTime(F(__DATE__), F(__TIME__))); // Set date and time for use with the DS1307 RTC
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't compile: the class RTC_DS3231 doesn't have a method named isrunning(). You probably want to comment-out this line and uncomment the previous one. Alternatively, comment-out the current declaration of rtc and uncomment RTC_DS1307 rtc;.

}

void loop() {
now = getclock(); // read the time from the RTC and adjust for DST or
now = dstclock(rtc.now()); // read the time from the RTC and adjust for DST and do date/time checking
}
Loading