From 7dbacac67cf6e76169dca202df081916e5249e51 Mon Sep 17 00:00:00 2001 From: Thomas Osterried Date: Wed, 9 Aug 2023 17:51:42 +0200 Subject: [PATCH] Fixes for getLocalTime() and configTime() fix #1: getLocalTime() has bugs getLocalTimeTheBetterWay is my reimplementation of the buggy getLocalTime() library call of esp32 which caused 5s delay on each call of getLocalTime(), if no gps- or ntp- time was obtained since boot. getLocalTime() has an optional argument to reduce this delay, but even then each call has a delay of 10ms. Also, posix recommends to call time with nullpointer instead of storing the time to the address of the argument. On linux, see man (2) time. Thanks to Thomas dl3el for locating the cause of the 5s-delay we observed under some cirumstances. fix #2: configTime() resulted in strange DNS requests. configTime() stores the memory address of ntp-server. Thus the variable needs to be static, else ntp will later will see garbage. This forbids the use of ntp_server.c_str(). Now, we use there a static char array, instead of a more inefficient String buffer. configTime() expects a char array anyway. For more fun with bad concepts, see https://github.com/opendata-stuttgart/sensors-software/issues/471 Thanks to Michal SQ2MO for locating the cause: DNS requests for IN A addresses that looked like the content of free()'d memory. Signed-off-by: Thomas Osterried --- src/TTGO_T-Beam_LoRa_APRS.ino | 27 ++++++++++++++++++++++----- src/taskWebServer.cpp | 22 ++++++++++++++++++---- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/src/TTGO_T-Beam_LoRa_APRS.ino b/src/TTGO_T-Beam_LoRa_APRS.ino index 2bcf4c0..6a6f438 100644 --- a/src/TTGO_T-Beam_LoRa_APRS.ino +++ b/src/TTGO_T-Beam_LoRa_APRS.ino @@ -517,6 +517,23 @@ volatile boolean sema_lora_chip = false; // + FUNCTIONS-----------------------------------------------------------+// +// This is a reimplementation of the buggy getLocalTime() library call of esp32 +// which caused 5s delay on each call of getLocalTime(), if no gps- or ntp- +// time was obtained since boot. getLocalTime() has an optional argument to +// reduce this delay, but even then each call has a delay of 10ms. +// Also, posix recommends to call time with nullpointer instead of storing +// the time to the address of the argument. On linux, see man (2) time. +bool getLocalTimeTheBetterWay(struct tm * info) +{ + time_t now = time(0); + if (now != ~0 && now) { + localtime_r(&now, info); + if (info->tm_year > 99) + return true; + } + return false; +} + void do_serial_println(const String &msg) { if (usb_serial_data_type & 1) { @@ -656,7 +673,7 @@ out_relay_path: char *p; // Objects always have a length of 9 and a timestamp - if (getLocalTime(&timeinfo)) { + if (getLocalTimeTheBetterWay(&timeinfo)) { strftime(buf_time, sizeof(buf_time), "%H%M%Sz", &timeinfo); } else { strcpy(buf_time, "000000"); @@ -1198,7 +1215,7 @@ void timer_once_a_second() { t_next_run = millis() + 1000; // update gps time string once a second - if (getLocalTime(&timeinfo)) { + if (getLocalTimeTheBetterWay(&timeinfo)) { strftime(gps_time_s, sizeof(gps_time_s), "%H:%M:%S", &timeinfo); } @@ -1673,7 +1690,7 @@ void sendToWebList(const String& TNC2FormatedFrame, const int RSSI, const int SN receivedPacketData->packet->trim(); receivedPacketData->RSSI = RSSI; receivedPacketData->SNR = SNR; - getLocalTime(&receivedPacketData->rxTime); + getLocalTimeTheBetterWay(&receivedPacketData->rxTime); if (xQueueSend(webListReceivedQueue, &receivedPacketData, (1000 / portTICK_PERIOD_MS)) != pdPASS){ // remove buffer on error @@ -1911,7 +1928,7 @@ void sendTelemetryFrame() { // For convenience, we store in pos 0 the month. If we use letters A-Z, a-z and and numbers 0-9, we could adress 62 days (exactly 2 months), before we start from new. // Pos 1 and 2: encodes the time. -> We get a resolution of min 23s, which is more than we need in typical usecases: 24*60*60.0/((26*2+10)**2) = 22.476s struct tm timeinfo{}; - if (getLocalTime(&timeinfo)) { + if (getLocalTimeTheBetterWay(&timeinfo)) { char buf[4]; int t = (timeinfo.tm_mon % 2) * 31 + ((timeinfo.tm_mday - 1) % 31); buf[0] = encode_int_in_char(t); @@ -1940,7 +1957,7 @@ void sendTelemetryFrame() { // to have the overflow in mid of the week instead of saturday evening. We start our // week at thursday instead of sunday -> (tm_wday += 4). struct tm timeinfo{}; - if (getLocalTime(&timeinfo)) { + if (getLocalTimeTheBetterWay(&timeinfo)) { char buf[4]; // resolution 6 packets in an hour. int t = (24*60*((timeinfo.tm_wday + 4) % 7) + 60*timeinfo.tm_hour + timeinfo.tm_min) / 10.0; diff --git a/src/taskWebServer.cpp b/src/taskWebServer.cpp index 069cb50..bd7be12 100644 --- a/src/taskWebServer.cpp +++ b/src/taskWebServer.cpp @@ -22,6 +22,8 @@ extern const char web_style_css_end[] asm("_binary_data_embed_style_css_out_end" extern const char web_js_js[] asm("_binary_data_embed_js_js_out_start"); extern const char web_js_js_end[] asm("_binary_data_embed_js_js_out_end"); +bool getLocalTimeTheBetterWay(struct tm * info); + // Variable needed to send beacon from html page extern bool manBeacon; @@ -1400,10 +1402,22 @@ void restart_AP_or_STA(void) { ntp_server = "pool.ntp.org"; preferences.putString(PREF_NTP_SERVER, ntp_server); } - configTime(0, 0, ntp_server.c_str()); + + // configTime() stores the memory address of ntp-server. + // Thus the variable needs to be static, else ntp will later will see garbage. + // This forbids the use of ntp_server.c_str(). + // Now, we use there a static char array, instead of a more + // inefficient String buffer. configTime() expects a char array anyway. + // For more fun with bad concepts, see https://github.com/opendata-stuttgart/sensors-software/issues/471 + { static char buf[64]; + *buf = 0; + strncpy(buf, ntp_server.c_str(), sizeof(buf) -1); + buf[sizeof(buf)-1] = 0; + configTime(0, 0, buf); + } #ifdef ENABLE_SYSLOG struct tm timeinfo{}; - if(!getLocalTime(&timeinfo)){ + if(!getLocalTimeTheBetterWay(&timeinfo)){ syslog_log(LOG_WARNING, "NTP: Failed to obtain time"); } else { char buf[64]; @@ -1444,7 +1458,7 @@ void do_send_status_message_about_shutdown_or_reboot_to_aprsis(int why) { String outString = aprsis_callsign + ">" + MY_APRS_DEST_IDENTIFYER + ":>APRSIS-Conn: "; char buf[19];// Room for len(20220917 01:02:03z) + 1 /* \0 */ -> 19 struct tm timeinfo{}; - if (getLocalTime(&timeinfo)) { + if (getLocalTimeTheBetterWay(&timeinfo)) { strftime(buf, sizeof(buf), "%Y%m%d %H:%M:%Sz", &timeinfo); //sprintf(buf, "%X%2.2d %2.2d:%2.2d:%2.2dz", timeinfo.tm_mon+1, timeinfo.tm_mday, timeinfo.tm_hour, timeinfo.tm_min, timeinfo.tm_sec); } else { @@ -1488,7 +1502,7 @@ void do_send_status_message_about_connect_to_aprsis(void) { String outString = aprsis_callsign + ">" + MY_APRS_DEST_IDENTIFYER + ":>APRSIS-Conn: "; char buf[19];// Room for len(20220917 01:02:03z) + 1 /* \0 */ -> 19 struct tm timeinfo{}; - if (getLocalTime(&timeinfo)) { + if (getLocalTimeTheBetterWay(&timeinfo)) { strftime(buf, sizeof(buf), "%Y%m%d %H:%M:%Sz", &timeinfo); //sprintf(buf, "%X%2.2d %2.2d:%2.2d:%2.2dz", timeinfo.tm_mon+1, timeinfo.tm_mday, timeinfo.tm_hour, timeinfo.tm_min, timeinfo.tm_sec); } else {