NTPClient::begin() doesn't check that the UDP object successfully initialized.

[MHotchin]

The underlying _udp object can fail its own 'begin()' call if no sockets are available.

Have hit this case on Wiznet 5100 hardware with a webserver also running.

[workpage2]

How did you solve the problem?

[WhymustIhaveaname]

The begin function of NTPC looks like

void NTPClient::begin(int port) {
  this->_port = port;

  this->_udp->begin(this->_port);

  this->_udpSetup = true;
}

So you can check UDP afterewards manually.

[MHotchin]

Code change as follows:

void NTPClient::begin(int port) { this->_port = port;

this->_udpSetup = this->_udp->begin(this->_port); }

[workpage2]

Where to change this code?

[WhymustIhaveaname]

Download .h and .cpp file of this repo into your project folder. Change it as MHotchin said. Change the include sentence of your .ino from

#include<NTPClient.h>

to

#include"NTPClient.h"

[WhymustIhaveaname]

But remember, If you use MHotchin's method and if the UDP initialize fails, the this->_udpSetup flag will be set to false. Refering to update of NTPC

bool NTPClient::update() {
  if ((millis() - this->_lastUpdate >= this->_updateInterval)     // Update after _updateInterval
    || this->_lastUpdate == 0) {                                // Update if there was no update yet.
    if (!this->_udpSetup) this->begin();                         // setup the UDP client if needed
    return this->forceUpdate();
  }
  return false;   // return false if update does not occur
}

, we know that udp will only be intialized again. So I recommand you to use a while loop to make sure UDP is indeed inited.

[MHotchin]

No sense using a while() loop, if ‘begin()’ fails you should wait some time then try again.

Your code should always handle the case that ‘update()’ fails, so if ‘begin()’ fails just do nothing – let ‘update()’ try to fix it, and if that fails then wait until your next ‘update()’ time and try again.

Quite frankly, the error handling and design of the code in NTP needs serious work. For example, this whole problem would go away if the UDP client was ephemeral instead of bound to the lifetime of the UDP client. This would also free up a socket for use for the 99.9% of the time that NTP wasn’t actually doing an update. I’d do it, but without some consensus that these changes would be integrated I’m not going to cmmit my time.

From: WhymustIhaveaname [mailto:[email protected]] Sent: Friday, May 08, 2020 11:09 PM To: arduino-libraries/NTPClient Cc: M Hotchin; Author Subject: Re: [arduino-libraries/NTPClient] NTPClient::begin() doesn't check that the UDP object successfully initialized. (#83)

But remember, If you use MHotchin's method and if the UDP initialize fails, the this->_udpSetup flag will be set to false. Refering to update of NTPC

bool NTPClient::update() { if ((millis() - this->_lastUpdate >= this->_updateInterval) // Update after _updateInterval || this->_lastUpdate == 0) { // Update if there was no update yet. if (!this->_udpSetup) this->begin(); // setup the UDP client if needed return this->forceUpdate(); } return false; // return false if update does not occur }

, we know that udp will only be intialized again. So I recommand you to use a while loop to make sure UDP is indeed inited.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/arduino-libraries/NTPClient/issues/83#issuecomment-626113005 , or unsubscribe https://github.com/notifications/unsubscribe-auth/AIM4TL7DSYIDW7HSUSJ3CO3RQTXQ3ANCNFSM4JVDSVIA . https://github.com/notifications/beacon/AIM4TL5ERPGZ2YGT3MFXBBTRQTXQ3A5CNFSM4JVDSVIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEVI3T3I.gif

[WhymustIhaveaname]

Yes, of course he should delay a short time in the while loop if failed. What's more, for his case, he can do something that makes initialize UDP more easily, such as free some port.