Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make NTP request more robust. #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

electricant
Copy link

@electricant electricant commented Apr 7, 2021

Previously, only the memory for the NTP header was declared and set.
Actually, apart from the headers, the rest of the NTP packet (48 bytes)
contains timestamps.
Those timestamps are interpreted by the NTP server as offsets from their time
and are used to calculate the new time. Therefore, if the rest of the
packet is not zero, a wrong time is received.

By reserving memory only of the header and sending 48 bytes nonetheless
we send as timestamps whatever values the microcontroller has on the stack. If
the stack content is not zero, we receive back a garbage timestamp.

This commit fixes this issue and also make the code a bit more readable
by moving some magic constants in the header file.

This code has been tested on an ESP8266 and works correctly.

Previously, only the memory for the NTP header was declared and set.
Actually, apart from the headers, the rest of the NTP packet (48 bytes)
contains timestamps.
Those timestamps are interpreted by the NTP server as offsets from their time
and are used to calculate the new time. Therefore, if the rest of the
packet is not zero, a wrong time is received.

By reserving memory only of the header and sending 48 bytes nonetheless
we send as timestams whatever values the microcontroller has on the stack. If
the stack content is not zero, we receive back a garbage timestamp.

This commit fixes this issue and also make the code a bit more readable
by moving some magic constants in the header file.
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