Skip to content

fix: NTP packet correctness and UDP socket lifecycle#12

Merged
aharshac merged 4 commits into
masterfrom
fix/ntp-packet-and-socket-lifecycle
May 9, 2026
Merged

fix: NTP packet correctness and UDP socket lifecycle#12
aharshac merged 4 commits into
masterfrom
fix/ntp-packet-and-socket-lifecycle

Conversation

@aharshac
Copy link
Copy Markdown
Owner

@aharshac aharshac commented May 9, 2026

Summary

  • Zero-initialize NTP packet buffer:

    • The previous code cast a 4-byte magic constant to a 48-byte write, sending uninitialized stack memory as NTP timestamp fields.
    • Servers interpret those bytes as offsets and return wrong time.
    • Replaced with a byte packetBuffer[NTP_PACKET_SIZE] = {0} and explicit header-byte assignments.
    • Response is now read into the same buffer via indexed access.
    • Confirmed fix for ESP8266 hardware (reported in PR Make NTP request more robust. #7).
  • Close UDP socket on every exit path:

    • The socket opened by mUdp->begin() was never closed, causing begin() to fail when a second client was created on the same UDP object.
    • mUdp->stop() is now called before every return and after the successful response read.
  • Remove static from socket initializer:

    • static int udpInited was evaluated exactly once per program lifetime.
    • After the socket is closed by the previous fix, subsequent calls need to re-run begin().
    • Removing static makes getServerTime() fully self-contained: it opens a socket, uses it, and closes it on every call.
    • Local port changed from the hardcoded 123 to NTP_REQUEST_PORT (1123).

Adds test_basic_sync() and test_client_reuse() to examples/TestNodeMCU/TestNodeMCU.ino.

Closes PRs #6 & #7

@aharshac aharshac changed the title Fix/ntp packet and socket lifecycle fix: NTP packet correctness and UDP socket lifecycle May 9, 2026
@aharshac aharshac merged commit 34d8145 into master May 9, 2026
4 checks passed
@aharshac aharshac deleted the fix/ntp-packet-and-socket-lifecycle branch May 9, 2026 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant