search

gmag11 / ESPNtpClient

High accuracy NTP library for ESP32 and ESP8266
MIT License
118 stars 25 forks source link

NTP.stop() crashes the ESP32 #9

Closed solideblock closed 3 years ago

solideblock commented 3 years ago

Hi,

I tried to implement NTP.stop() command to force a NTP reinitialization. My goal was to take into account a new NTP server. I had the idea to put all parameters in the GUI and let the users chose the NTP address, Time Zone and then immediatly apply the new settings.

But as soon as I call NTP.stop(), I have this error message:

assertion "pbuf_free: sane type" failed: file "/home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/lwip/lwip/src/core/pbuf.c", line 752, function: pbuf_free

I'm using 1.0.4 version of Espressif Systems in arduino IDE.

Any idea if this could be my code or the library ? I've been turning thing around for a while now without luck.

gmag11 commented 3 years ago

Should be fixed in latest release. Please test it and report.

Why do you need to stop time synchronisation?

solideblock commented 3 years ago

Hi,

Thanks a lot for your help. I've just tried a few line of code. It's no longer failling on the same error message. My code might be at falt but i will need to wait few days to debug now (my C skills are still limited and i'm learning).

My intention : when the ESP receives a HTTP GET with parameter http://192.168.0.200/setNtpServer?ntpserverip=216.239.35.0 <-- google IP, it is supposed to replace the original IP and force the library to use the new one. You may ask why am i using String for ntpServer, this is because i'm also saving the info into a file with ArduinoJson library and i found it easier to work with String. You are declaring a const char in your examples and until now, it did the trick. The new ntp value was loaded during boot.

Error message : New NTP server : 216.239.35.0Guru Meditation Error: Core 1 panic'ed (LoadProhibited)

Global var:

#define NTP_TIMEOUT 1500
#define DEFAULT_NTP_IP "192.168.0.115"
String ntpServer = DEFAULT_NTP_IP;

Code in Setup

...
load_conf(); //read conf file and which replaces ntpServer buy the value in conf file. 
init_ntp();
...

Code in the loop:

...
ntp_update();
...

The ntp_update function

void init_ntp() {
  NTP.onNTPSyncEvent ([](NTPEvent_t event) {
    ntpEvent = event;
    syncEventTriggered = true;
  });
  WiFi.onEvent (onWifiEvent);
}

 void ntp_update() {
    if (wifiFirstConnected) {
        wifiFirstConnected = false;
        NTP.setTimeZone (TZ_Europe_Paris);
        NTP.setInterval (1800);
        NTP.setNTPTimeout (NTP_TIMEOUT);
        NTP.begin (ntpServer.c_str());
    }

    if (syncEventTriggered) {
        syncEventTriggered = false;
        processSyncEvent (ntpEvent);
    }
}

Code for the HTTP GET:

void handleSetNtpServer() {
  IPAddress ipCheck;  
  if (ipCheck.fromString(server.arg("ntpserverip"))) {
    Serial.print("\nINFO - NTP - New NTP server : ");
    Serial.print(server.arg("ntpserverip"));
    ntpServer=server.arg("ntpserverip");
    server.send(200, "text/plane", "1");    
    NTP.stop();
    save_conf(); // save all values with ArduinoJson including the new ntpServer value. Working fine to take new value at boot. 
    wifiFirstConnected = true;
    server.send(200, "text/plane", "1");
  }else{
    Serial.println(F("\nERROR - NTP - New NTP server IP is invalid"));    
    server.send(200, "text/plane", "-1");
  }
}

Now to answer your question about the why i need to stop. In fact I don't know if I need to stop, I thought stoping was required to take the new IP into account. Without a stop, it is not working. It is not working anyway with the stop but for a different reason i need to understand the new error message.

Why would I want to change the IP :

Edit : formating and typos.

gmag11 commented 3 years ago

This library is designed to live during the whole program duration. Starting and stoping and restarting it is not a good idea as this is not tested at all, that's why I ask for your use case.

If you want to change NTP server you don't need to stop it. Simply use NTP.setNtpServerName ("newNTPserver"). It may be a FQDN or an ip address inside quotes.

gmag11 commented 3 years ago

The wifiFirstConnected trick may be confusing you. It's actually an ugly solution to avoid running NTP client before wifi is not connected. But actually it's not a problem if it tries to synchronize when WiFi is not available yet.

So. My recommendation is calling all NTP initialization in setup() to avoid multiple calls to NTP.begin()

solideblock commented 3 years ago

Well, I could not got to bed without trying :D

I did not manage to move the following code

 if (wifiFirstConnected) {
    wifiFirstConnected=false;   
    NTP.setTimeZone (TZ_Europe_Paris);
    NTP.setInterval (1800);
    NTP.setNTPTimeout (NTP_TIMEOUT);
    NTP.begin (ntpServer.c_str());
  }

into the setup for a reason I don't understand yet. The ESP keeps on rebooting when calling the init_ntp() function which now looks like this. 

void init_ntp() {
  NTP.onNTPSyncEvent ([](NTPEvent_t event) {
    ntpEvent = event;
    syncEventTriggered = true;
  });
  WiFi.onEvent (onWifiEvent);
    NTP.setTimeZone (TZ_Europe_Paris);
    NTP.setInterval (1800);
    NTP.setNTPTimeout (NTP_TIMEOUT);
    NTP.begin (ntpServer.c_str());
}

I made have missed something.

So I just added 

    NTP.setNtpServerName(ntpServer.c_str());
    NTP.getTime();

to my handleSetNtpServer() function. That did the trick like you said it will. No need to NTP.stop() and i also now understand that the folowing parameters can be updated too

NTP.setTimeZone (TZ_Europe_Paris);
    NTP.setInterval (1800);
    NTP.setNTPTimeout (NTP_TIMEOUT);

Thanks a lot for your help !

gmag11 commented 3 years ago

Please publish your complete code so I can help. I see some mistakes there but having separate pieces makes impossible to suggest corrections

gmag11 commented 3 years ago

Closing as inactive. Feel free to reopen