From 2d480039d4393bd868f47fd5eacc85be82256093 Mon Sep 17 00:00:00 2001 From: aharshac Date: Sun, 10 May 2026 00:33:15 +0530 Subject: [PATCH 1/4] fix: zero-initialize NTP packet buffer - prevent garbage timestamps --- src/EasyNTPClient.cpp | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/src/EasyNTPClient.cpp b/src/EasyNTPClient.cpp index aa19643..0bc7a1a 100644 --- a/src/EasyNTPClient.cpp +++ b/src/EasyNTPClient.cpp @@ -41,9 +41,14 @@ void EasyNTPClient::setTimeOffset (int offset) { unsigned long EasyNTPClient::getServerTime () { static int udpInited = this->mUdp->begin(123); // open socket on arbitrary port - // Only the first four bytes of an outgoing NTP packet need to be set - // appropriately, the rest can be whatever. - const long ntpFirstFourBytes = 0xEC0600E3; // NTP request header + + // Only the first four bytes of an NTP request need to be set. The rest + // must be zero so the server does not interpret them as timestamp offsets. + byte packetBuffer[NTP_PACKET_SIZE] = {0}; + packetBuffer[0] = NTP_HEADER_LI | NTP_HEADER_VN | NTP_HEADER_MODE; + // byte 1 (stratum) stays 0 — server ignores client stratum + packetBuffer[2] = NTP_HEADER_POLL; + packetBuffer[3] = NTP_HEADER_PRECISION; // Fail if WiFiUdp.begin() could not init a socket if (! udpInited) @@ -54,7 +59,7 @@ unsigned long EasyNTPClient::getServerTime () { // Send an NTP request if (! (this->mUdp->beginPacket(this->mServerPool, 123) // 123 is the NTP port - && this->mUdp->write((byte *)&ntpFirstFourBytes, 48) == 48 + && this->mUdp->write(packetBuffer, NTP_PACKET_SIZE) == NTP_PACKET_SIZE && this->mUdp->endPacket())) return 0; // sending request failed @@ -62,24 +67,21 @@ unsigned long EasyNTPClient::getServerTime () { const int pollIntv = 150; // poll every this many ms const byte maxPoll = 15; // poll up to this many times int pktLen; // received packet length - for (byte i=0; imUdp->parsePacket()) == 48) - break; - delay(pollIntv); + for (byte i = 0; i < maxPoll; i++) { + if ((pktLen = this->mUdp->parsePacket()) == NTP_PACKET_SIZE) + break; + delay(pollIntv); } - if (pktLen != 48) + if (pktLen != NTP_PACKET_SIZE) return 0; // no correct packet received - // Read and discard the first useless bytes - // Set useless to 32 for speed; set to 40 for accuracy. - const byte useless = 40; - for (byte i = 0; i < useless; ++i) - this->mUdp->read(); + this->mUdp->read(packetBuffer, NTP_PACKET_SIZE); - // Read the integer part of sending time - unsigned long time = this->mUdp->read(); // NTP time - for (byte i = 1; i < 4; i++) - time = time << 8 | this->mUdp->read(); + // Read the integer part (32 bits) of the Transmit Timestamp at offset 40 + unsigned long time = packetBuffer[NTP_TX_TIMESTAMP_OFFSET]; + time = time << 8 | packetBuffer[NTP_TX_TIMESTAMP_OFFSET + 1]; + time = time << 8 | packetBuffer[NTP_TX_TIMESTAMP_OFFSET + 2]; + time = time << 8 | packetBuffer[NTP_TX_TIMESTAMP_OFFSET + 3]; // Round to the nearest second if we want accuracy // The fractionary part is the next byte divided by 256: if it is @@ -87,7 +89,7 @@ unsigned long EasyNTPClient::getServerTime () { // for an assumed network delay of 50ms, and (0.5-0.05)*256=115; // additionally, we account for how much we delayed reading the packet // since its arrival, which we assume on average to be pollIntv/2. - time += (this->mUdp->read() > 115 - pollIntv/8); + time += (packetBuffer[NTP_TX_TIMESTAMP_OFFSET + 4] > 115 - pollIntv / 8); // Discard the rest of the packet this->mUdp->flush(); From 6e789f44c303906b918e3d0fb09e89b75baa2ffb Mon Sep 17 00:00:00 2001 From: aharshac Date: Sun, 10 May 2026 00:45:24 +0530 Subject: [PATCH 2/4] fix: close UDP socket after each request --- src/EasyNTPClient.cpp | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/EasyNTPClient.cpp b/src/EasyNTPClient.cpp index 0bc7a1a..6adbdbd 100644 --- a/src/EasyNTPClient.cpp +++ b/src/EasyNTPClient.cpp @@ -46,22 +46,26 @@ unsigned long EasyNTPClient::getServerTime () { // must be zero so the server does not interpret them as timestamp offsets. byte packetBuffer[NTP_PACKET_SIZE] = {0}; packetBuffer[0] = NTP_HEADER_LI | NTP_HEADER_VN | NTP_HEADER_MODE; - // byte 1 (stratum) stays 0 — server ignores client stratum + // byte 1 (stratum) stays 0 - server ignores client stratum packetBuffer[2] = NTP_HEADER_POLL; packetBuffer[3] = NTP_HEADER_PRECISION; // Fail if WiFiUdp.begin() could not init a socket - if (! udpInited) - return 0; + if (!udpInited) { + this->mUdp->stop(); + return 0; + } // Clear received data from possible stray received packets this->mUdp->flush(); // Send an NTP request - if (! (this->mUdp->beginPacket(this->mServerPool, 123) // 123 is the NTP port + if (!(this->mUdp->beginPacket(this->mServerPool, NTP_SERVER_PORT) && this->mUdp->write(packetBuffer, NTP_PACKET_SIZE) == NTP_PACKET_SIZE - && this->mUdp->endPacket())) - return 0; // sending request failed + && this->mUdp->endPacket())) { + this->mUdp->stop(); + return 0; // sending request failed + } // Wait for response; check every pollIntv ms up to maxPoll times const int pollIntv = 150; // poll every this many ms @@ -72,8 +76,10 @@ unsigned long EasyNTPClient::getServerTime () { break; delay(pollIntv); } - if (pktLen != NTP_PACKET_SIZE) - return 0; // no correct packet received + if (pktLen != NTP_PACKET_SIZE) { + this->mUdp->stop(); + return 0; // no correct packet received + } this->mUdp->read(packetBuffer, NTP_PACKET_SIZE); @@ -91,8 +97,8 @@ unsigned long EasyNTPClient::getServerTime () { // since its arrival, which we assume on average to be pollIntv/2. time += (packetBuffer[NTP_TX_TIMESTAMP_OFFSET + 4] > 115 - pollIntv / 8); - // Discard the rest of the packet this->mUdp->flush(); + this->mUdp->stop(); return time + this->mOffset - 2208988800ul; // convert NTP time to Unix time } From 3b02c74f280ed9ef5d1f77a2bb53a4a25fddf423 Mon Sep 17 00:00:00 2001 From: aharshac Date: Sun, 10 May 2026 00:46:40 +0530 Subject: [PATCH 3/4] fix: remove static initializer for socket init status --- src/EasyNTPClient.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EasyNTPClient.cpp b/src/EasyNTPClient.cpp index 6adbdbd..5d6014c 100644 --- a/src/EasyNTPClient.cpp +++ b/src/EasyNTPClient.cpp @@ -40,7 +40,7 @@ void EasyNTPClient::setTimeOffset (int offset) { unsigned long EasyNTPClient::getServerTime () { - static int udpInited = this->mUdp->begin(123); // open socket on arbitrary port + int udpInited = this->mUdp->begin(NTP_REQUEST_PORT); // Only the first four bytes of an NTP request need to be set. The rest // must be zero so the server does not interpret them as timestamp offsets. From 2134568415c65c23af9df6d081e262377566da80 Mon Sep 17 00:00:00 2001 From: aharshac Date: Sun, 10 May 2026 00:54:00 +0530 Subject: [PATCH 4/4] test: add basic sync and socket reuse tests to NodeMCU suite --- examples/TestNodeMCU/TestNodeMCU.ino | 39 ++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/examples/TestNodeMCU/TestNodeMCU.ino b/examples/TestNodeMCU/TestNodeMCU.ino index 28ff884..d9342f4 100644 --- a/examples/TestNodeMCU/TestNodeMCU.ino +++ b/examples/TestNodeMCU/TestNodeMCU.ino @@ -52,6 +52,43 @@ void test_constants() { "header byte 0 matches original magic constant 0xE3"); } +// ── basic sync ─────────────────────────────────────────────────────────────── + +void test_basic_sync() { + Serial.println("\n-- basic sync --"); + WiFiUDP udp; + EasyNTPClient client(udp, "pool.ntp.org"); + + unsigned long t = client.getUnixTime(); + check(t > MIN_UNIX_2024, "time is after 2024-01-01"); + check(t < MAX_UNIX_2030, "time is before 2030-01-01"); +} + +// ── socket reuse ───────────────────────────────────────────────────────────── + +void test_client_reuse() { + Serial.println("\n-- socket reuse across client instances --"); + + // Both clients share the same WiFiUDP object. Without Fix B+C the second + // begin() is skipped (static flag) and the second sync fails. + WiFiUDP udp; + unsigned long t1 = 0, t2 = 0; + + { + EasyNTPClient c1(udp, "pool.ntp.org"); + t1 = c1.getUnixTime(); // opens socket, syncs, closes socket (Fix B) + } + delay(500); + { + EasyNTPClient c2(udp, "pool.ntp.org"); + t2 = c2.getUnixTime(); // Fix C re-runs begin(); Fix B already closed it cleanly + } + + check(t1 > MIN_UNIX_2024, "first client syncs successfully"); + check(t2 > MIN_UNIX_2024, "second client syncs on same UDP object"); + check(t2 >= t1 && (t2 - t1) < 5, "timestamps consistent between clients"); +} + // ── entry points ───────────────────────────────────────────────────────────── void setup() { @@ -62,6 +99,8 @@ void setup() { wifi_connect(); test_constants(); + test_basic_sync(); + test_client_reuse(); Serial.println("\n=== Results ==="); Serial.print(g_passed); Serial.println(" passed");