Skip to content

Commit

Permalink
Various improvements to NtpClient class (#1482)
Browse files Browse the repository at this point in the history 
Tested with SystemClock_NTP sample, but note that time will be wrong as we don't yet have any timezone management in place. The sample also sets local time, but NTP provides time in UTC. No point in fixing that yet as without timezone it just makes things worse.

Fix #include guard name
Tidy #includes
Put NTP_DEFAULT_SERVER into Flash
Use const String& for parameters
Use SimpleTimer instead of Timer as appropriate (efficiency, saves RAM)
timeoutTimer doesn't need to be initialised until a request has been queued, so moved out of constructor
Replace staticDnsResponse() with lambda
Tidy extraction of timestamp from received packet using LWIP macro
Rewrite using one timer
Set timer to retry on initial request, and only stop it on success or cancellation
Add retry if bad NTP packet received
  • Loading branch information
@mikee47 @slaff
mikee47 authored and slaff committed Oct 12, 2018
1 parent 6989cf4 commit 207b8c3
Show file tree
Hide file tree
Showing 2 changed files with 116 additions and 94 deletions.
128 changes: 63 additions & 65 deletions Sming/SmingCore/Network/NtpClient.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,50 +1,66 @@
/****
* Sming Framework Project - Open Source framework for high efficiency native ESP8266 development.
* Created 2015 by Skurydin Alexey
* http://github.com/anakod/Sming
* All files of the Sming Core are provided under the LGPL v3 license.
*
****/

#include "NtpClient.h"
#include "Platform/Station.h"
#include "SystemClock.h"

NtpClient::NtpClient() : NtpClient(NTP_DEFAULT_SERVER, NTP_DEFAULT_QUERY_INTERVAL_SECONDS, nullptr)
NtpClient::NtpClient() : NtpClient(NTP_DEFAULT_SERVER, NTP_DEFAULT_AUTOQUERY_SECONDS, nullptr)
{
}

NtpClient::NtpClient(NtpTimeResultDelegate onTimeReceivedCb)
: NtpClient(NTP_DEFAULT_SERVER, NTP_DEFAULT_QUERY_INTERVAL_SECONDS, onTimeReceivedCb)
: NtpClient(NTP_DEFAULT_SERVER, NTP_DEFAULT_AUTOQUERY_SECONDS, onTimeReceivedCb)
{
}

NtpClient::NtpClient(String reqServer, int reqIntervalSeconds, NtpTimeResultDelegate delegateFunction /* = NULL */)
NtpClient::NtpClient(const String& reqServer, unsigned reqIntervalSeconds, NtpTimeResultDelegate delegateFunction)
{
// init timer but do not start, correct interval set later below.
autoUpdateTimer.initializeMs(NTP_DEFAULT_QUERY_INTERVAL_SECONDS * 1000,
TimerDelegate(&NtpClient::requestTime, this));
debug_d("NtpClient(\"%s\", %u, 0x%08x", reqServer.c_str(), reqIntervalSeconds, delegateFunction);

timeoutTimer.initializeMs(NTP_RESPONSE_TIMEOUT_MS, TimerDelegate(&NtpClient::requestTime, this));
// Setup timer, but don't start it
timer.setCallback(TimerDelegate(&NtpClient::requestTime, this));

this->server = reqServer;
this->delegateCompleted = delegateFunction;
if(!delegateFunction) {
autoUpdateSystemClock = true;
}

if(reqIntervalSeconds == 0) {
setAutoQuery(false);
} else {
if(reqIntervalSeconds) {
setAutoQueryInterval(reqIntervalSeconds);
setAutoQuery(true);
requestTime();
}
}

NtpClient::~NtpClient()
{
}

void NtpClient::requestTime()
{
debug_d("NtpClient::requestTime()");

// Schedule a retry in anticipation of failure
startTimer(NTP_CONNECTION_TIMEOUT_MS);

if(!WifiStation.isConnected()) {
connectionTimer.initializeMs(1000, TimerDelegate(&NtpClient::requestTime, this)).startOnce();
debugf("NtpClient waiting for connection...");
return;
}

ip_addr_t resolvedIp;
int result = dns_gethostbyname(this->server.c_str(), &resolvedIp, staticDnsResponse, (void*)this);
int result = dns_gethostbyname(server.c_str(), &resolvedIp,
[](const char* name, LWIP_IP_ADDR_T* ip, void* arg) {
// We do a new request since the last one was never done.
if(ip)
reinterpret_cast<NtpClient*>(arg)->internalRequestTime(*ip);
},
this);

debug_d("dns_gethostbyname() returned %d", result);

switch(result) {
case ERR_OK:
Expand All @@ -58,7 +74,7 @@ void NtpClient::requestTime()
break;
case ERR_INPROGRESS:
// currently finding ip, internalRequestTime() will be called when its found.
//debug_d("DNS IP lookup in progress.");
debug_d("DNS IP lookup in progress...");
break;
default:
debug_d("DNS lookup error occurred.");
Expand All @@ -68,61 +84,50 @@ void NtpClient::requestTime()

void NtpClient::internalRequestTime(IPAddress serverIp)
{
// connect to current active serverIp, on NTP_PORT
this->connect(serverIp, NTP_PORT);
debug_d("NtpClient::internalRequestTime()");

uint8_t packet[NTP_PACKET_SIZE];
// connect to current active serverIp, on NTP_PORT
connect(serverIp, NTP_PORT);

// Setup the NTP request packet
memset(packet, 0, NTP_PACKET_SIZE);
char packet[NTP_PACKET_SIZE] = {0};

// These are the only required values for a SNTP request. See page 14:
// https://tools.ietf.org/html/rfc4330
// However size of packet should still be 48 bytes.
packet[0] = (NTP_VERSION << 3 | 0x03); // LI (0 = no warning), Protocol version (4), Client mode (3)
packet[1] = 0; // Stratum, or type of clock, unspecified.

// Start timeout timer, if no response is recieved within NTP_RESPONSE_TIMEOUT
// a new request will be sent.
timeoutTimer.startOnce();
// Start timer to retry if no response received
startTimer(NTP_RESPONSE_TIMEOUT_MS);

// Send to server, serverAddress & port is set in connect
NtpClient::send((char*)packet, NTP_PACKET_SIZE);
}

void NtpClient::setNtpServer(String server)
{
this->server = server;
NtpClient::send(packet, NTP_PACKET_SIZE);
}

void NtpClient::setAutoQuery(bool autoQuery)
{
if(autoQuery)
autoUpdateTimer.start();
else
autoUpdateTimer.stop();
}

void NtpClient::setAutoQueryInterval(int seconds)
{
// minimum 10 seconds interval.
if(seconds < 10)
autoUpdateTimer.setIntervalMs(10000);
else
autoUpdateTimer.setIntervalMs(seconds * 1000);
autoQueryEnabled = autoQuery;
if(autoQueryEnabled) {
startTimer(autoQuerySeconds * 1000U);
} else {
stopTimer();
}
}

void NtpClient::setAutoUpdateSystemClock(bool autoUpdateClock)
void NtpClient::setAutoQueryInterval(unsigned seconds)
{
autoUpdateSystemClock = autoUpdateClock;
autoQuerySeconds = max(seconds, NTP_MIN_AUTOQUERY_SECONDS);
if(autoQueryEnabled) {
setAutoQuery(true);
}
}

void NtpClient::onReceive(pbuf* buf, IPAddress remoteIP, uint16_t remotePort)
{
// stop timeout timer since we received a response.
if(timeoutTimer.isStarted()) {
timeoutTimer.stop();
}
debug_d("NtpClient::onReceive(%s:%u)", remoteIP.toString().c_str(), remotePort);

stopTimer();

// We do some basic check to see if it really is a ntp packet we receive.
// NTP version should be set to same as we used to send, NTP_VERSION
Expand All @@ -136,10 +141,9 @@ void NtpClient::onReceive(pbuf* buf, IPAddress remoteIP, uint16_t remotePort)
if(mode == NTP_MODE_SERVER && (ver == NTP_VERSION || ver == (NTP_VERSION - 1))) {
//Most likely a correct NTP packet received.

uint8_t data[4];
pbuf_copy_partial(buf, data, 4, 40); // Copy only timestamp.

uint32_t timestamp = (data[0] << 24 | data[1] << 16 | data[2] << 8 | data[3]);
uint32_t timestamp;
pbuf_copy_partial(buf, &timestamp, sizeof(timestamp), 40); // Copy only timestamp.
timestamp = ntohl(timestamp);

// Unix time starts on Jan 1 1970, subtract 70 years:
uint32_t epoch = timestamp - 0x83AA7E80;
Expand All @@ -149,19 +153,13 @@ void NtpClient::onReceive(pbuf* buf, IPAddress remoteIP, uint16_t remotePort)
}

if(delegateCompleted) {
this->delegateCompleted(*this, epoch);
delegateCompleted(*this, epoch);
}
}
}

void NtpClient::staticDnsResponse(const char* name, LWIP_IP_ADDR_T* ip, void* arg)
{
// DNS has been resolved

NtpClient* self = (NtpClient*)arg;

if(ip != NULL) {
// We do a new request since the last one was never done.
self->internalRequestTime(*ip);
// If auto query is enabled, schedule the next check
setAutoQuery(autoQueryEnabled);
} else {
// Got a dodgy packet so treat it as we would a response failure
startTimer(NTP_RESPONSE_TIMEOUT_MS);
}
}
82 changes: 53 additions & 29 deletions Sming/SmingCore/Network/NtpClient.h
View file
Original file line number Diff line number Diff line change
@@ -1,28 +1,37 @@
/****
* Sming Framework Project - Open Source framework for high efficiency native ESP8266 development.
* Created 2015 by Skurydin Alexey
* http://github.com/anakod/Sming
* All files of the Sming Core are provided under the LGPL v3 license.
*
****/

/** @defgroup ntp Network Time Protocol client
* @brief Provides NTP client
* @ingroup datetime
* @ingroup udp
* @{
*/
#ifndef APP_NTPCLIENT_H_
#define APP_NTPCLIENT_H_
#ifndef _SMING_CORE_NETWORK_NTPCLIENT_H_
#define _SMING_CORE_NETWORK_NTPCLIENT_H_

#include "UdpConnection.h"
#include "../Platform/System.h"
#include "../Timer.h"
#include "../SystemClock.h"
#include "../Platform/Station.h"
#include "../Delegate.h"
#include "Platform/System.h"
#include "Timer.h"
#include "../Services/DateTime/DateTime.h"
#include "Delegate.h"

#define NTP_PORT 123
#define NTP_PACKET_SIZE 48
#define NTP_VERSION 4
#define NTP_MODE_CLIENT 3
#define NTP_MODE_SERVER 4

#define NTP_DEFAULT_SERVER "pool.ntp.org"
#define NTP_DEFAULT_QUERY_INTERVAL_SECONDS 600 // 10 minutes
#define NTP_RESPONSE_TIMEOUT_MS 20000 // 20 seconds
#define NTP_DEFAULT_SERVER F("pool.ntp.org")
#define NTP_DEFAULT_AUTOQUERY_SECONDS 30U // (10U * SECS_PER_MIN) ///< Refresh time if autoupdate set
#define NTP_MIN_AUTOQUERY_SECONDS 10U ///< Minimum autoquery interval
#define NTP_CONNECTION_TIMEOUT_MS 1666U ///< Time to retry query when network connection unavailable
#define NTP_RESPONSE_TIMEOUT_MS 20000U ///< Time to wait before retrying NTP query

class NtpClient;

Expand All @@ -47,9 +56,11 @@ class NtpClient : protected UdpConnection
* @param reqIntervalSeconds Quantity of seconds between NTP requests
* @param onTimeReceivedCb Callback delegate to be called when NTP time result is received (Default: None)
*/
NtpClient(String reqServer, int reqIntervalSeconds, NtpTimeResultDelegate onTimeReceivedCb = nullptr);
NtpClient(const String& reqServer, unsigned reqIntervalSeconds, NtpTimeResultDelegate onTimeReceivedCb = nullptr);

virtual ~NtpClient();
virtual ~NtpClient()
{
}

/** @brief Request time from NTP server
* @note Instigates request. Result is handled by NTP result handler function if defined
Expand All @@ -59,7 +70,10 @@ class NtpClient : protected UdpConnection
/** @brief Set the NTP server
* @param server IP address or hostname of NTP server
*/
void setNtpServer(String server);
void setNtpServer(const String& server)
{
this->server = server;
}

/** @brief Enable / disable periodic query
* @param autoQuery True to enable periodic query of NTP server
Expand All @@ -69,12 +83,15 @@ class NtpClient : protected UdpConnection
/** @brief Set query period
* @param seconds Period in seconds between periodic queries
*/
void setAutoQueryInterval(int seconds);
void setAutoQueryInterval(unsigned seconds);

/** @brief Enable / disable update of system clock
* @param autoUpdateClock True to update system clock with NTP result.
*/
void setAutoUpdateSystemClock(bool autoUpdateClock);
void setAutoUpdateSystemClock(bool autoUpdateClock)
{
autoUpdateSystemClock = autoUpdateClock;
}

protected:
/** @brief Handle UDP message reception
Expand All @@ -89,24 +106,31 @@ class NtpClient : protected UdpConnection
*/
void internalRequestTime(IPAddress serverIp);

/** @brief Start the timer running
* @param time to run in milliseconds
*/
void startTimer(uint32_t milliseconds)
{
debug_d("NtpClient::startTimer(%u)", milliseconds);
timer.setIntervalMs(milliseconds);
timer.startOnce();
}

void stopTimer()
{
debug_d("NtpClient::stopTimer()");
timer.stop();
}

protected:
String server = NTP_DEFAULT_SERVER; ///< IP address or Hostname of NTP server
String server; ///< IP address or Hostname of NTP server

NtpTimeResultDelegate delegateCompleted = nullptr; ///< NTP result handler delegate
bool autoUpdateSystemClock = false; ///< True to update system clock with NTP time

Timer autoUpdateTimer; ///< Periodic query timer
Timer timeoutTimer; ///< NTP message timeout timer
Timer connectionTimer; ///< Wait for WiFi connection timer

/** @brief Handle DNS response
* @param name Pointer to c-string containing hostname
* @param ip Ponter to IP address
* @param arg Pointer to the NTP client object that made the DNS request
* @note This function is called when a DNS query is serviced
*/
static void staticDnsResponse(const char* name, LWIP_IP_ADDR_T* ip, void* arg);
bool autoQueryEnabled = false;
unsigned autoQuerySeconds = NTP_DEFAULT_AUTOQUERY_SECONDS;
Timer timer; ///< Deals with timeouts, retries and autoquery updates
};

/** @} */
#endif /* APP_NTPCLIENT_H_ */
#endif /* _SMING_CORE_NETWORK_NTPCLIENT_H_ */

0 comments on commit 207b8c3

Please sign in to comment.