Skip to content

Commit 3f9957d

Browse files
mlesniewmlesniew
committed
Avoid blocking in NTPClient::update()
When a NTP request is sent, it may take several milliseconds to retrieve the response. This commit changes the NTPClient::update() behaviour to asynchronous allowing a NTP request to be sent with one update() call and handle the response when it's available, in another call eliminating active waiting. This commit also changes the NTPClient::forceUpdate() implementation to rely on the logic in NTPClient::update(). However, the behaviour of this function does not change from the API user's perspective. It is still synchronous, it only returns when all processing is complete.
1 parent 531eff3 commit 3f9957d

File tree

2 files changed

+116
-45
lines changed

2 files changed

+116
-45
lines changed

Diff for: ‎NTPClient.cpp

+107-44
Original file line numberDiff line numberDiff line change
@@ -75,55 +75,116 @@ void NTPClient::begin() {
7575

7676
void NTPClient::begin(unsigned int port) {
7777
this->_port = port;
78+
this->_state = State::uninitialized;
79+
}
7880

79-
this->_udp->begin(this->_port);
81+
bool NTPClient::update() {
82+
switch (this->_state) {
83+
case State::uninitialized:
84+
this->_udp->begin(this->_port);
85+
this->_state = State::idle;
86+
87+
// fall through -- we're all initialized now
88+
89+
case State::idle:
90+
if ((millis() - this->_lastUpdate < this->_updateInterval) // Update after _updateInterval
91+
&& this->_lastUpdate != 0) // Update if there was no update yet.
92+
return false;
93+
94+
this->_state = State::send_request;
95+
96+
// fall through -- ready to send request now
97+
98+
case State::send_request:
99+
#ifdef DEBUG_NTPClient
100+
Serial.println(F("Sending NTP request"));
101+
#endif
102+
103+
// flush any existing packets
104+
while(this->_udp->parsePacket() != 0)
105+
this->_udp->flush();
106+
107+
this->sendNTPPacket();
108+
109+
this->_lastRequest = millis();
110+
this->_state = State::wait_response;
111+
112+
// fall through -- if we're lucky we may already receive a response
113+
114+
case State::wait_response:
115+
if (!this->_udp->parsePacket()) {
116+
// no reply yet
117+
if (millis() - this->_lastRequest >= 1000) {
118+
// time out
119+
#ifdef DEBUG_NTPClient
120+
Serial.println(F("NTP reply timeout"));
121+
#endif
122+
this->_state = State::idle;
123+
}
124+
return false;
125+
}
126+
127+
#ifdef DEBUG_NTPClient
128+
Serial.println(F("NTP reply received"));
129+
#endif
130+
131+
// got a reply!
132+
this->_lastUpdate = this->_lastRequest;
133+
this->_udp->read(this->_packetBuffer, NTP_PACKET_SIZE);
134+
135+
{
136+
unsigned long highWord = word(this->_packetBuffer[40], this->_packetBuffer[41]);
137+
unsigned long lowWord = word(this->_packetBuffer[42], this->_packetBuffer[43]);
138+
// combine the four bytes (two words) into a long integer
139+
// this is NTP time (seconds since Jan 1 1900):
140+
unsigned long secsSince1900 = highWord << 16 | lowWord;
141+
this->_currentEpoc = secsSince1900 - SEVENZYYEARS;
142+
}
143+
144+
return true; // return true after successful update
145+
146+
default:
147+
this->_state = State::uninitialized;
148+
}
80149

81-
this->_udpSetup = true;
150+
return false;
82151
}
83152

84153
bool NTPClient::forceUpdate() {
85-
#ifdef DEBUG_NTPClient
86-
Serial.println("Update from NTP Server");
87-
#endif
88-
89-
// flush any existing packets
90-
while(this->_udp->parsePacket() != 0)
91-
this->_udp->flush();
92-
93-
this->sendNTPPacket();
94-
95-
// Wait till data is there or timeout...
96-
byte timeout = 0;
97-
int cb = 0;
98-
do {
99-
delay ( 10 );
100-
cb = this->_udp->parsePacket();
101-
if (timeout > 100) return false; // timeout after 1000 ms
102-
timeout++;
103-
} while (cb == 0);
104-
105-
this->_lastUpdate = millis() - (10 * (timeout + 1)); // Account for delay in reading the time
106-
107-
this->_udp->read(this->_packetBuffer, NTP_PACKET_SIZE);
108-
109-
unsigned long highWord = word(this->_packetBuffer[40], this->_packetBuffer[41]);
110-
unsigned long lowWord = word(this->_packetBuffer[42], this->_packetBuffer[43]);
111-
// combine the four bytes (two words) into a long integer
112-
// this is NTP time (seconds since Jan 1 1900):
113-
unsigned long secsSince1900 = highWord << 16 | lowWord;
114-
115-
this->_currentEpoc = secsSince1900 - SEVENZYYEARS;
116-
117-
return true; // return true after successful update
118-
}
154+
// In contrast to NTPClient::update(), this function always sends a NTP
155+
// request and only returns when the whole operation completes (no matter
156+
// if it's a success or a failure because of a timeout). In other words
157+
// this function is fully synchronous. It will block until the whole
158+
// NTP operation completes.
159+
//
160+
// We could only make this function switch the state to State::send_request
161+
// to ensure a NTP request would happen with the next call to
162+
// NTPClient::update(). However, this would be an API change, users could
163+
// expect synchronous behaviour and even skip the calls to NTPClient::update()
164+
// completely relying only on this function for time updates.
165+
166+
// ensure we're initialized
167+
if (this->_state == State::uninitialized) {
168+
this->_udp->begin(this->_port);
169+
}
119170

120-
bool NTPClient::update() {
121-
if ((millis() - this->_lastUpdate >= this->_updateInterval) // Update after _updateInterval
122-
|| this->_lastUpdate == 0) { // Update if there was no update yet.
123-
if (!this->_udpSetup || this->_port != NTP_DEFAULT_LOCAL_PORT) this->begin(this->_port); // setup the UDP client if needed
124-
return this->forceUpdate();
171+
// At this point we can be in any state except for State::uninitialized.
172+
// Let's ignore that and switch right to State::send_request to send a
173+
// fresh NTP request.
174+
this->_state = State::send_request;
175+
176+
while (true) {
177+
if (this->update()) {
178+
// time updated
179+
return true;
180+
} else if (this->_state != State::idle) {
181+
// still waiting for response
182+
delay(10);
183+
} else {
184+
// failure
185+
return false;
186+
}
125187
}
126-
return false; // return false if update does not occur
127188
}
128189

129190
bool NTPClient::isTimeSet() const {
@@ -165,8 +226,7 @@ String NTPClient::getFormattedTime() const {
165226

166227
void NTPClient::end() {
167228
this->_udp->stop();
168-
169-
this->_udpSetup = false;
229+
this->_state = State::uninitialized;
170230
}
171231

172232
void NTPClient::setTimeOffset(int timeOffset) {
@@ -209,4 +269,7 @@ void NTPClient::sendNTPPacket() {
209269
void NTPClient::setRandomPort(unsigned int minValue, unsigned int maxValue) {
210270
randomSeed(analogRead(0));
211271
this->_port = random(minValue, maxValue);
272+
273+
// we've set a new port, remember to reinitialize UDP next time
274+
this->_state = State::uninitialized;
212275
}

Diff for: ‎NTPClient.h

+9-1
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@
1010

1111
class NTPClient {
1212
private:
13+
1314
UDP* _udp;
14-
bool _udpSetup = false;
1515

1616
const char* _poolServerName = "pool.ntp.org"; // Default time server
1717
IPAddress _poolServerIP;
@@ -22,6 +22,14 @@ class NTPClient {
2222

2323
unsigned long _currentEpoc = 0; // In s
2424
unsigned long _lastUpdate = 0; // In ms
25+
unsigned long _lastRequest = 0; // In ms
26+
27+
enum class State {
28+
uninitialized,
29+
idle,
30+
send_request,
31+
wait_response,
32+
} _state = State::uninitialized;
2533

2634
byte _packetBuffer[NTP_PACKET_SIZE];
2735

0 commit comments

Comments
 (0)