[Solved] Inconsistent results for converting UTC to local time with Timezone lib

Hello everyone,

I am rather confused about something related to using the JChristensen Timezone library. I'm trying to do the following:

  • Obtain current UTC time from NTP server. This part works just fine.
  • Then in the setup section I update a real time clock with this timestamp (using RTClib.h). This works fine, too.
  • Now later in the program, whenever I do something with the time obtained from the rtc, I want to convert it first to account for daylight saving time. I want to use the Timezone library for that.

So here are the corresponding code snippets:

time_t UTCtime;
time_t nowUTC;
time_t nowLocal;

// Timezone rules for United Kingdom (London, Belfast)
TimeChangeRule BST = {"BST", Last, Sun, Mar, 1, 60}; // British Summer Time
TimeChangeRule GMT = {"GMT", Last, Sun, Oct, 2, 0}; // Standard Time
Timezone UK(BST, GMT);

// Getting time from NTP server beforehand
UTCtime = epoch;
rtc.adjust(UTCtime + 11);

// At some point later in the code:
nowUTC = rtc.now().unixtime();
nowLocal = UK.toLocal(nowUTC);

Serial.println("Is nowUTC DST?: " + String(UK.utcIsDST(nowUTC)));

Now, what I would expect is that nowUTC will be adjusted so that it gains an hour because currently we have British Summer Time. However, that doesn't seem to happen most of the time. What I obtain is nowUTC and nowLocal being equal, and utcIsDST() returning 0.

However, what is even more weird is that sometimes I get the expected result. Just by re-uploading the code to my Arduino - sometimes the results are as expected but most of the time they are not.

If I replace nowUTC in the toLocal() function by a hard-coded DST timestamp (e.g. 1585684200, provided as unsigned long), I get an unexpected result, too. So I guess I might provide a wrong data type (though I receive no error).

So basically my question is: how do I correctly feed the time received from the rtc to the toLocal() function of the Timezone library to obtain the correct local time?

Why this?

rtc.adjust(UTCtime + 11);

Just for grins, here is the definition from my records:

//British Time Zone (London)
  //UTC+0 uses DST
  {"Britain", {"BDT", Last, dowSunday, Mar, 2, 60},
 {"GMT", Last, dowSunday, Oct, 2, 0}}

is 'Sun' the same as 'dowSunday'?

aarg:
Why this?

rtc.adjust(UTCtime + 11);

Also, please post a complete program that compiles and demonstrates the problem.

rtc.adjust(UTCtime + 11);

This I included because the updated rtc time appeared to be off by about 11 seconds as compared to the time on my computer, which I take as a reference. Removing the + 11 did not resolve my problem, and I didn't expect it to.

In the meanwhile, I have tried to create a minimal piece of code to reproduce the problem. I put a little bit into the setup section with a hard-coded unix timestamp; I gradually added more pieces of my full code. Now I'm more confused than before:

It appears only commenting out the following line affects the result I get from toLocal():

ThingSpeak.writeFields(myChannelNumber, myWriteAPIKey);

This line is used to upload some values to ThingSpeak. Something that I had deliberately not posted here because I wouldn't think in my wildest dreams that this could be related to my problem (And I didn't think it would be sensible to post 1000 lines of code). And, mind you, this line is in the loop section, which, in my testing example, is not even entered as I've put an infinite while loop into the setup section for testing. To me it appears that line should be absolutely unrelated to what I've described, and still, having it present or not reproducibly changes the result I obtain with toLocal(). Right now I'm doubting everything I thought I knew.

Second Update:
No I have found that even just having a String that's printed in the Serial Monitor exceed a certain length results in getting a wrong output from toLocal()...It appears just changing random bits of code that might affect something more globally (e.g. compile time?) have an effect. I've never experienced anything like that...

I'd guess you're overwrite an array bounds somewhere. Or, incorrectly writing to a de-referenced pointer (essentially the same thing).

Thanks for that hint. I have spent many more hours in trying to look into that.

If it's indeed an out of bounds array or writing to a de-referenced pointer, shouldn't then commenting out anything related to the array or pointer in question resolve the problem? Am I wrong in assuming that? Because I have commented out large bits of the code that cover any such instances but to no avail.

Could it be another memory issue of some sort? I'm using an Arduino Mega and upon compiling I get:
"Sketch uses 41526 bytes (16%) of program storage space. Maximum is 253952 bytes.
Global variables use 2026 bytes (24%) of dynamic memory, leaving 6166 bytes for local variables. Maximum is 8192 bytes." That does seem ok to me, but a memory issue might explain what I observe (i.e. the apparent randomness of things affecting if it's working or not), couldn't it?

Anyways, I have created a pastebin with the full code: https://pastebin.com/hhrdwTEA
As you might get from the comments, it's for an automated plant watering system. The bit related to obtaining local time from UTC is in line 285 and subsequent lines. I'd be surprised if the full code is going to be of any help though. I'm sure you'll be able to find a lot of things that might be improved or that should be done differently, but I'm more interested in solving this specific problem right now.

For example, I have tried completely removing the pointer for myWriteAPIKey (line 64) and hard-coding the Thingspeak API key, but that didn't resolve the problem either. I also tried removing any functions that take arrays by reference without luck.

yking:
Second Update:
No I have found that even just having a String that's printed in the Serial Monitor exceed a certain length results in getting a wrong output from toLocal()...It appears just changing random bits of code that might affect something more globally (e.g. compile time?) have an effect. I've never experienced anything like that...

That sounds like memory issues, you are using numerous libraries, possibly they are allocating large buffers of memory and leaving you low on dynamic memory, the compiler summary of memory does not keep track of memory allocated at run-time. Strings are dynamic in size, you probably have some in the code that are overwriting something when they get large enough. If possibly, do not use the String type,

I see a few places in your code where you are using a String for something as simple as comparing a data or time, a very complex and code-intensive method of comparing numbers when you could have just stored the start and end times as integers to begin with.

String CameraStart = "09:00"; // Start of time frame during which to acquire a picture
String CameraEnd = "17:00"; // End of time frame during which to acquire a picture


if ((now.hour() > CameraStart.substring(0, 2).toInt() || (now.hour() == CameraStart.substring(0, 2).toInt() && now.minute() > CameraStart.substring(CameraStart.length() - 2, CameraStart.length()).toInt())) && (now.hour() < CameraEnd.substring(0, 2).toInt() || (now.hour() == CameraEnd.substring(0, 2).toInt() && now.minute() < CameraEnd.substring(CameraEnd.length() - 2, CameraEnd.length()).toInt()))) {

Right. Lots of code allocating dynamic memory combined with lots of nested function calls allocating local variables can result in the stack and heap colliding with each other. That's not to mention sloppy programmers who don't return dynamic memory when it's no longer needed or don't even bother to check if malloc() returns an null pointer.

david_2018:
I see a few places in your code where you are using a String for something as simple as comparing a data or time, a very complex and code-intensive method of comparing numbers...

Ugh....
Almost always, the easiest way to compare times is with Epoch time. You only need hours, minutes, seconds when a human has to read it.

gfvalvo:
Ugh....
Almost always, the easiest way to compare times is with Epoch time. You only need hours, minutes, seconds when a human has to read it.

Not only the easiest, but also the fastest.

Thank you for those additional tips, I'll look into that. I deliberately used Strings in that example you quoted (i.e. "17:00") because this is a setting that I want to be able to change quickly and easily, and be able to see how it's set up at a glance. Which won't be the case if I split it up into integers for minutes and hours, but I guess I'll have to sacrifice convenience for functionality here...

Having said this, I have already tried to have Strings in the Flash memory instead of SRAM where ever possible, by using the F() macro, as you can see in the code. And, as I have mentioned, I also tried commenting out large parts (~ 2 thirds of the code, including the quoted example) but that didn't help either, so I still have my doubts that reducing usage of strings will help for the particular issue at hand. I would argue that by commenting out those parts, that's basically what I've tried already. And everything other than this new bit related to time keeps working just fine.

I have also just tried out the MemoryUsage library to check SRAM usage at various places during run-time. The lowest value I got for free ram size was 5122 B (of 8 KB for my Arduino Mega), which in my opinion does not suggest over-usage of dynamic memory...

A few things the compiler should be showing you:

/home/jdbarney/Arduino/hhrdwTEA/hhrdwTEA.ino:92:39: warning: integer overflow in expression [-Woverflow]
 int MinimumWateringInterval = 60 * 60 * 24; // Minimum time in seconds that must have passed since last watering for next watering to happen
                               ~~~~~~~~^~~~
/home/jdbarney/Arduino/hhrdwTEA/hhrdwTEA.ino:422:72: warning: integer overflow in expression [-Woverflow]
           elapseddays = (wateringtimedif - (wateringtimedif % (60 * 60 * 24))) / (60 * 60 * 24);
                                                                ~~~~~~~~^~~~
/home/jdbarney/Arduino/hhrdwTEA/hhrdwTEA.ino:422:91: warning: integer overflow in expression [-Woverflow]
           elapseddays = (wateringtimedif - (wateringtimedif % (60 * 60 * 24))) / (60 * 60 * 24);
                                                                                   ~~~~~~~~^~~~

/home/jdbarney/Arduino/hhrdwTEA/hhrdwTEA.ino: In function 'int getHue(int, int, int, char)':
/home/jdbarney/Arduino/hhrdwTEA/hhrdwTEA.ino:1324:1: warning: no return statement in function returning non-void [-Wreturn-type]
 }
 ^

There are also a lot of warnings from your getHue() function about the possibility of using a variable that you have not given a value, caused by setting some of these variables inside case statements where the particular case that sets the variable may not be executed.

You wrote in one post: "This I included because the updated rtc time appeared to be off by about 11 seconds as compared to the time on my computer, which I take as a reference. ".

I have to wonder if you are using a PC with Windows. Do you have software running on your PC to keep the time correctly? If not and it has been a long time since you last booted your PC, the clock could be WAY off!

Paul

Thanks again, these are valid points and I'm always looking to improve. I don't know why the compiler did not came up with those warnings, but I see why it should. I have changed MinimumWateringInterval to a long now, changed the calculations to 60UL * 60UL * 24UL, and changed getHue() into a void function.

I am indeed using a PC with Windows which I shut down and boot daily, so I do not expect the clock to be way off.

As much as I appreciate your comments, I'm afraid they haven't brought me closer to the issue (yet).

I have now managed to boil down the code to a much shorter excerpt for testing:
https://pastebin.com/PQSqFwJC

The described issue persists. Now, with this bit I have tested commenting out different parts and I have made the following observations:

I DO get the expected result (i.e., the testing unix timestamp converted using the toLocal() function to account for daylight saving time) when the code remains as posted, i.e. only the following line is commented out:

ThingSpeakResponse = ThingSpeak.writeFields(myChannelNumber, myWriteAPIKey);

I do NOT get the expected result:

  • When the following line is NOT commented out:
ThingSpeakResponse = ThingSpeak.writeFields(myChannelNumber, myWriteAPIKey);
  • When both the following lines are commented out:
ThingSpeak.setStatus("Testing");
ThingSpeakResponse = ThingSpeak.writeFields(myChannelNumber, myWriteAPIKey);
  • When the datetimestring() function and its two calls are commented out, in addition to the following line:
ThingSpeakResponse = ThingSpeak.writeFields(myChannelNumber, myWriteAPIKey);

I still cannot make any sense of it; at least now any arrays are gone and I would guess any memory issues related to using Strings can be eliminated as a possible cause, too.

Have you tried debug printing 'myWriteAPIKey'?

aarg:
Have you tried debug printing 'myWriteAPIKey'?

Yes, I have: Serial.print(myWriteAPIKey); will correctly print the 16-digit API key of my private Thingspeak channel which is taken from secrets.h (I obviously won't share that API key here).

Ah, I think I see your problem. Your datetimestring() function returns a String, but the String it returns is local to the function, so it goes out of scope when the function ends and is being overwritten.

RTClib has a function to convert a DateTime variable to human readable form, the somewhat confusingly named .toString() because it returns a char* , not a String. You can specify a format for the date and time, the details of which are in the toString example sketch included with the library.

You will need to create a buffer to hold the return value from the function. Here is your sketch modified to use that instead of using Strings:

#include <SoftwareSerial.h>
#include "RTClib.h" // For Real Time Clock
#include "ThingSpeak.h"
#include "WiFiEsp.h"
#include "secrets.h"
#include "time.h"
#include <TimeLib.h>
#include <math.h>
#include "WiFiEspUdp.h"
#include <Timezone.h>

// Real time clock
RTC_DS3231 rtc;

char ssid[] = SECRET_SSID;   // network SSID (name)
char pass[] = SECRET_PASS;   // network password
int keyIndex = 0;            // network key Index number (needed only for WEP)
WiFiEspClient  client;

unsigned long myChannelNumber = SECRET_CH_ID; // Thingspeak channel ID
const char * myWriteAPIKey = SECRET_WRITE_APIKEY; // Thingspeak channel API key

int ThingSpeakResponse = 0;

// Timing-related variables
time_t UTCtime;
time_t nowUTC;
time_t nowLocal;

// Timezone rules for United Kingdom (London, Belfast)
TimeChangeRule BST = {"BST", Last, Sun, Mar, 1, 60}; // British Summer Time
TimeChangeRule GMT = {"GMT", Last, Sun, Oct, 2, 0}; // Standard Time
Timezone UK(BST, GMT);

void setup() {
  // Initialize serial for output
  Serial.begin(9600);

  // Initialize DS3231 Real Time Clock
  if (! rtc.begin()) {
    Serial.println(F("Couldn't find RTC"));
    while (1); // can't go further
  }

  // Initialize serial for ESP module
  Serial1.begin(115200);
  // Initialize ESP module
  WiFi.init(&Serial1);

  // Check for the presence of the shield
  if (WiFi.status() == WL_NO_SHIELD) {
    Serial.println(F("WiFi shield not present"));
    while (true);
  }
  // Initialize ThingSpeak
  ThingSpeak.begin(client);

  // Connect or reconnect to WiFi
  if (WiFi.status() != WL_CONNECTED) {
    Serial.print(F("Attempting to connect to SSID: "));
    Serial.println(SECRET_SSID);
    while (WiFi.status() != WL_CONNECTED) {
      WiFi.begin(ssid, pass);  // Connect to WPA/WPA2 network. Change this line if using open or WEP network
      delay(5000);
    }
  }

  // Set RTC time
  rtc.adjust(DateTime(2020, 4, 2, 12, 0, 0)); // Set real time clock manually for testing purposes

  nowUTC = rtc.now().unixtime(); // Get current unix timestamp from rtc
  char buff[20]; //buffer for storing human-readable date and time
  Serial.print(F("nowUTC: "));
  Serial.println(nowUTC); // Correctly is 1585828800
  Serial.print(F("Human readable: "));
  Serial.println(datetimestring(buff, nowUTC)); // Correctly is 2020/04/02 12:00:00
  nowLocal = UK.toLocal(nowUTC); // Convert time according to day light saving time rule
  Serial.print(F("nowLocal: "));
  Serial.println(nowLocal); // Should be 1585832400
  Serial.print(F("Human readable: "));
  Serial.println(datetimestring(buff, nowLocal)); // Should be 2020/04/02 13:00:00
}

void loop() {
  // Set ThingSpeak fields with the values
  ThingSpeak.setField(1, 1);

  // Set ThingSpeak channel status
  ThingSpeak.setStatus("Testing");

  // Write to the ThingSpeak channel
  //ThingSpeakResponse = ThingSpeak.writeFields(myChannelNumber, myWriteAPIKey);
}

// Function to return date and time in human readable form
// format "yyyy/mm/dd hh:mm:ss"
// must declare dest[] as 20 char or larger
char * datetimestring(char * dest, DateTime datetime) {
  static const char datetimeTemplate[] PROGMEM = "YYYY/MM/DD hh:mm:ss"; //stored in PROGMEM to save space in ram
  strcpy_P(dest, datetimeTemplate); //copy template to buffer
  return (datetime.toString(dest)); 
}

david_2018:
I see a few places in your code where you are using a String for something as simple as comparing a data or time, a very complex and code-intensive method of comparing numbers when you could have just stored the start and end times as integers to begin with.

String CameraStart = "09:00"; // Start of time frame during which to acquire a picture

String CameraEnd = "17:00"; // End of time frame during which to acquire a picture

You could use char arrays instead, and compare with strcmp.

Or, if you wish to avoid character strings entirely, you could do something like:

long hmsToSec(byte h, byte m, byte s) {
  return ((h * 3600L) + (m * 60L) + s);
}

and then use hmsToSec(9, 0, 0) for the start time and hmsToSec(17, 0, 0) for the end time.
The purpose of the hmsToSec function is to convert a time of day (given as hours, minutes, seconds) to seconds past midnight. So, for example hmsToSec(0, 1, 5) will give you 65, as the time 00:01:05 is 65 seconds past midnight.
Note that hmsToSec(23, 59, 59) (which means 23:59:59, that is, one second before midnight) gives you 86399, which is too big to fit into an int, which is why you need to use a long instead.

david_2018:
RTClib has a function to convert a DateTime variable to human readable form, the somewhat confusingly named .toString() because it returns a char* , not a String. You can specify a format for the date and time, the details of which are in the toString example sketch included with the library.

You will need to create a buffer to hold the return value from the function. Here is your sketch modified to use that instead of using Strings:

#include <SoftwareSerial.h>

#include "RTClib.h" // For Real Time Clock
#include "ThingSpeak.h"
#include "WiFiEsp.h"
#include "secrets.h"
#include "time.h"
#include <TimeLib.h>
#include <math.h>
#include "WiFiEspUdp.h"
#include <Timezone.h>

// Real time clock
RTC_DS3231 rtc;

char ssid[] = SECRET_SSID;  // network SSID (name)
char pass[] = SECRET_PASS;  // network password
int keyIndex = 0;            // network key Index number (needed only for WEP)
WiFiEspClient  client;

unsigned long myChannelNumber = SECRET_CH_ID; // Thingspeak channel ID
const char * myWriteAPIKey = SECRET_WRITE_APIKEY; // Thingspeak channel API key

int ThingSpeakResponse = 0;

// Timing-related variables
time_t UTCtime;
time_t nowUTC;
time_t nowLocal;

// Timezone rules for United Kingdom (London, Belfast)
TimeChangeRule BST = {"BST", Last, Sun, Mar, 1, 60}; // British Summer Time
TimeChangeRule GMT = {"GMT", Last, Sun, Oct, 2, 0}; // Standard Time
Timezone UK(BST, GMT);

void setup() {
  // Initialize serial for output
  Serial.begin(9600);

// Initialize DS3231 Real Time Clock
  if (! rtc.begin()) {
    Serial.println(F("Couldn't find RTC"));
    while (1); // can't go further
  }

// Initialize serial for ESP module
  Serial1.begin(115200);
  // Initialize ESP module
  WiFi.init(&Serial1);

// Check for the presence of the shield
  if (WiFi.status() == WL_NO_SHIELD) {
    Serial.println(F("WiFi shield not present"));
    while (true);
  }
  // Initialize ThingSpeak
  ThingSpeak.begin(client);

// Connect or reconnect to WiFi
  if (WiFi.status() != WL_CONNECTED) {
    Serial.print(F("Attempting to connect to SSID: "));
    Serial.println(SECRET_SSID);
    while (WiFi.status() != WL_CONNECTED) {
      WiFi.begin(ssid, pass);  // Connect to WPA/WPA2 network. Change this line if using open or WEP network
      delay(5000);
    }
  }

// Set RTC time
  rtc.adjust(DateTime(2020, 4, 2, 12, 0, 0)); // Set real time clock manually for testing purposes

nowUTC = rtc.now().unixtime(); // Get current unix timestamp from rtc
  char buff[20]; //buffer for storing human-readable date and time
  Serial.print(F("nowUTC: "));
  Serial.println(nowUTC); // Correctly is 1585828800
  Serial.print(F("Human readable: "));
  Serial.println(datetimestring(buff, nowUTC)); // Correctly is 2020/04/02 12:00:00
  nowLocal = UK.toLocal(nowUTC); // Convert time according to day light saving time rule
  Serial.print(F("nowLocal: "));
  Serial.println(nowLocal); // Should be 1585832400
  Serial.print(F("Human readable: "));
  Serial.println(datetimestring(buff, nowLocal)); // Should be 2020/04/02 13:00:00
}

void loop() {
  // Set ThingSpeak fields with the values
  ThingSpeak.setField(1, 1);

// Set ThingSpeak channel status
  ThingSpeak.setStatus("Testing");

// Write to the ThingSpeak channel
  //ThingSpeakResponse = ThingSpeak.writeFields(myChannelNumber, myWriteAPIKey);
}

// Function to return date and time in human readable form
// format "yyyy/mm/dd hh:mm:ss"
// must declare dest[] as 20 char or larger
char * datetimestring(char * dest, DateTime datetime) {
  static const char datetimeTemplate[] PROGMEM = "YYYY/MM/DD hh:mm:ss"; //stored in PROGMEM to save space in ram
  strcpy_P(dest, datetimeTemplate); //copy template to buffer
  return (datetime.toString(dest));
}

Thank you so much david_2018, that did it! I did not know that a String goes out of scope if it's local to a function - every day is school day. Thanks odometer, too, I'll implement your suggestion.

I still cannot quite get my head around the observations I have made earlier though. I guess it had to do with the fact that the datetimestring() function was used just before the lines that just 'appeared' to affect the result (when, in fact, it was the memory being screwed up by my datetimestring() function? And maybe it didn't become apparent earlier in the code where it had already been used because the Thingspeak update is more memory-intensive?). If you feel like explaining it in a bit more detail, I'd appreciate that because I like to learn from my mistakes. If not, that's fine too. Thanks again!

yking:
I still cannot quite get my head around the observations I have made earlier though. I guess it had to do with the fact that the datetimestring() function was used just before the lines that just 'appeared' to affect the result (when, in fact, it was the memory being screwed up by my datetimestring() function? And maybe it didn't become apparent earlier in the code where it had already been used because the Thingspeak update is more memory-intensive?). If you feel like explaining it in a bit more detail, I'd appreciate that because I like to learn from my mistakes. If not, that's fine too. Thanks again!

What may have happened was when you removed the line that gets a response from ThingSpeak, the compiler decides that you really don't need any of the code for ThingSpeak, because you are setting it up, but never actually sending or receiving anything. Then the compiler removes all the related code, including the .begin() from before your print statements, any interactions between the WiFi and the arduino that are ThingSpeak related, any variables that are used only for the ThingSpeak code, etc.