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.

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

configTime with TZ offset causes incorrect UTC time #6921

Closed
5 of 6 tasks
vlastahajek opened this issue Dec 18, 2019 · 8 comments · Fixed by #6993
Closed
5 of 6 tasks

configTime with TZ offset causes incorrect UTC time #6921

vlastahajek opened this issue Dec 18, 2019 · 8 comments · Fixed by #6993
Assignees
Milestone

Comments

@vlastahajek
Copy link
Contributor

Basic Infos

  • This issue complies with the issue POLICY doc.
  • I have read the documentation at readthedocs and the issue is not addressed there.
  • I have tested that the issue is present in current master branch (aka latest git).
  • I have searched the issue tracker for a similar issue.
  • If there is a stack dump, I have decoded it.
  • I have filled out all fields below.

Platform

  • Hardware: ESP-12
  • Core Version: d40dbb4
  • Development Env: Arduino IDE 1.8.10
  • Operating System: Windows 10

Settings in IDE

  • Module: Wemos D1 mini
  • Flash Size: 4MB
  • lwip Variant: v2 Lower Memory
  • CPU Frequency: 80Mhz
  • Upload Using: SERIAL
  • Upload Speed: 921600

Problem Description

When using the void configTime(int timezone, int daylightOffset_sec, const char* server1, const char* server2 = nullptr, const char* server3 = nullptr) with non-zero timezone offset to synchronize time, device doesn't differentiate between local and UTC time, where UTC time is wrongly set to local time.

This works OK on ESP32 (1.0.4).

I've checked #6678, but it seems it is just about initialization issue.

The workaround is using void configTime(const char* tz, const char* server1, const char* server2 = nullptr, const char* server3 = nullptr).

MCVE Sketch

#if defined(ESP32)
# include <WiFiMulti.h>
  WiFiMulti wifiMulti;
#elif defined(ESP8266)
# include <ESP8266WiFiMulti.h>
  ESP8266WiFiMulti wifiMulti;
#endif

void setup() {
  Serial.begin(115200);

  Serial.println();
  
  // Setup wifi
  WiFi.mode(WIFI_STA);
  wifiMulti.addAP("SSID","PASSWORD");

  Serial.print("Connecting to wifi");
  while (wifiMulti.run() != WL_CONNECTED) {
    Serial.print(".");
    delay(100);
  }
  Serial.println();

  // TZ offset +1h
  configTime(3600, 0 , "pool.ntp.org", "time.nis.gov");
  //configTime("CET-1CEST,M3.5.0,M10.5.0/3" , "pool.ntp.org", "time.nis.gov");
  // Wait till time is synced
  Serial.print("Syncing time");
  int i = 0;
  while (time(nullptr) < 1000000000ul && i<100) {
    Serial.print(".");
    delay(100);
    i++;
  }
  Serial.println();

  time_t tnow = time(nullptr);
  struct tm *timeinfo;
  char buffer [80];


  timeinfo = localtime (&tnow);
  strftime (buffer,80,"Local time: %H:%M.",timeinfo);
  Serial.println(buffer);
  
  timeinfo = gmtime (&tnow);
  strftime (buffer,80,"UTC time: %H:%M.",timeinfo);
  Serial.println(buffer); 
}

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

}

Debug Messages

Connecting to wifi......................
Syncing time.
Local time: 11:17.
UTC time: 11:17.
@d-a-v d-a-v self-assigned this Dec 21, 2019
@d-a-v d-a-v added this to the 2.7.0 milestone Dec 21, 2019
@ElVasquito
Copy link

I am facing the same issue. If I set a timezone as int using ConfigTime(int timezone_sec,...), the local clock gets offset the difference to UTC time. By now I am handling the timezones and clock resyncs by myself, and it works this way, but how do I prevent the auto-resync every hour? It is causing me problems. I tried setting SNTP_UPDATE_DELAY to 0 everywhere but it is still resyncing every hour by itself. Any clues? TIA

@d-a-v
Copy link
Collaborator

d-a-v commented Jan 6, 2020

configTime(timezone_sec, daylightOffset_sec, ...) has been working since the beginning and was based on an espressif patch against time functions. This function stores the user time shift in a variable, and the latter is added to the real source time when setting system time (from sntp). So the Newlib (our "libc") has no knowledge of what is local and UTC, that is because we don't update the newlib's internal variables.

And we can't do that apart from modifying the TZ variable (gmtime() and localtime() depend on tzset(), it's all newlib) which leads me suggesting to use the other configTime() function as shown in the NTP-TZ-DST example that will bring to you all the libc features.

I don't know how esp32 works regarding time management, but the new configTime() based on newlib (which esp32 use too) is more robust than a user defining his own time, because

  • it's newlib
  • we don't need to debug it
  • DST is automagically handled (check the example)

If it's about esp8266/esp32 compatibility, please have a look to our configTime() and try it on esp32. I guess it will be working.

@d-a-v
Copy link
Collaborator

d-a-v commented Jan 6, 2020

#6993 removes espressif legacy time management.
configTime(tz,dst,) will now use configTime(TZSTRING,) with a custom string.
That way, UTC and local time are properly managed by newlib.
The NTP-TZ-DST example is updated with that call (commented by default, the other one is better).

@vlastahajek
Copy link
Contributor Author

Thanks, @d-a-v, for the detailed explanation.
I'm familiar of configTime(TZSTRING,...), but the problem is that I would like to have code maximally compatible for ESP8266 and ESP32, with as least as possible #ifdefs. Unfortunately, ESP32 have different name for the same function, configTzTime.
So, now I use workaround:

configTime(0, 0, "pool.ntp.org", "time.nis.gov");
 // Set timezone
 setenv("TZ", TZ_STRING, 1);

So, I'm looking forward for a release including PR #6993 to have this more simple.

@d-a-v
Copy link
Collaborator

d-a-v commented Jan 9, 2020

I don't see why we wouldn't have the same name.
I'll add that to #6993 .

@vlastahajek
Copy link
Contributor Author

You are very kind, thanks. But, subjectively, I like more the way it is now (same name, overloaded) . ESP32 arduino seems to be less mature and it should be changed to be more compatible with ESP8266 arduino wherever possible.

@d-a-v
Copy link
Collaborator

d-a-v commented Jan 9, 2020

Thanks. I was not intending to remove the overload, but just add an alias.

@engbryan
Copy link

Well, for those who may need it: just call configTime() inside setup(), it can even be called before the wifi begin. It does not matter. It will work. Throw and forget it.
Also, you do not need to call _setEnv_. You can do something like configTime(-3600*3, 0, "pool.ntp.org"); on your setup.
Cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants