Library: CycloneTCP Open
Version: 2.1.6
Module: cyclone_tcp/sntp/sntp_client.{c,h}
Type: Feature request / enhancement (backward compatible)
Problem
sntpClientGetTimestamp() runs the whole NTP transaction (open socket -> send -> wait for response -> parse) inside a single while(!error) loop, so the call blocks the caller until a response arrives or the socket timeout expires.
In an RTOS application, a slow or unreachable NTP server keeps the calling thread blocked for the entire socket timeout (SNTP_CLIENT_DEFAULT_TIMEOUT, 30 s by default). This is a problem for any thread that must stay responsive and keep servicing its other periodic work while a synchronization is in progress: the thread cannot do anything else until the call returns. Lowering the socket timeout via sntpClientSetTimeout() is not an acceptable workaround, because it also shortens the legitimate wait for a reachable-but-slow server.
Request
A non-blocking way to issue an SNTP request, so the application can start a request and then poll for the result from its own periodic handler, never blocking the calling thread. The existing blocking sntpClientGetTimestamp() should stay unchanged for backward compatibility.
Proposed API (additive, no change to existing functions)
// Open the socket and send the request, then return immediately.
error_t sntpClientSendTimestampRequest(SntpClientContext *context);
// Non-blocking poll for the server's response.
// NO_ERROR - 'timestamp' is valid, request is finished
// ERROR_TIMEOUT - no response yet, call again later
// other - failure
error_t sntpClientPollTimestamp(SntpClientContext *context,
NtpTimestamp *timestamp);
// Release the socket and reset the state machine (cancel an in-flight request).
void sntpClientAbortRequest(SntpClientContext *context);
These reuse the existing internal helpers (sntpClientOpenConnection, sntpClientSendRequest, sntpClientReceiveResponse, sntpClientParseResponse, sntpClientCloseConnection) and the existing SntpClientState machine, so the diff is small and the blocking path is untouched.
Sketch of the implementation
error_t sntpClientSendTimestampRequest(SntpClientContext *context)
{
error_t error;
if(context == NULL)
return ERROR_INVALID_PARAMETER;
if(context->state == SNTP_CLIENT_STATE_INIT)
{
error = sntpClientOpenConnection(context);
if(!error)
{
context->startTime = osGetSystemTime();
context->retransmitTimeout = SNTP_CLIENT_INIT_RETRANSMIT_TIMEOUT;
context->state = SNTP_CLIENT_STATE_SENDING;
error = sntpClientSendRequest(context);
}
}
else
{
error = ERROR_WRONG_STATE;
}
return error;
}
error_t sntpClientPollTimestamp(SntpClientContext *context,
NtpTimestamp *timestamp)
{
error_t error;
if(context == NULL || timestamp == NULL)
return ERROR_INVALID_PARAMETER;
error = sntpClientReceiveResponse(context);
if(error == ERROR_TIMEOUT)
return error; // no datagram yet, poll again later
if(!error)
{
error = sntpClientParseResponse(context, timestamp);
if(error != ERROR_WOULD_BLOCK)
{
sntpClientCloseConnection(context);
context->state = SNTP_CLIENT_STATE_INIT;
}
}
return error;
}
void sntpClientAbortRequest(SntpClientContext *context)
{
if(context != NULL)
{
sntpClientCloseConnection(context);
context->state = SNTP_CLIENT_STATE_INIT;
}
}
Typical caller usage (from a periodic RTOS handler, e.g. every N ms), with a short socket timeout set via sntpClientSetTimeout() so each poll returns fast:
if(!sent) { sntpClientSendTimestampRequest(&ctx); sent = true; }
err = sntpClientPollTimestamp(&ctx, &ts);
if(err == NO_ERROR) { /* got time */ done = true; }
else if(err != ERROR_TIMEOUT) { sntpClientAbortRequest(&ctx); done = true; }
// after N polls without success -> sntpClientAbortRequest(&ctx)
Why upstream
We currently carry this as a local patch to the vendored library, which makes future CycloneTCP updates harder to merge. A supported non-blocking SNTP API in the library would let us drop the local patch entirely.
Environment
- MCU: Artery AT32F437 (Cortex-M4F)
- RTOS: FreeRTOS 10.4.6
- Toolchain: Keil MDK (ARM Compiler 6)
- CycloneTCP Open 2.1.6
Thanks for the library and for considering this. I'm happy to open a pull request with the implementation and a doc comment if the approach looks good.
Library: CycloneTCP Open
Version: 2.1.6
Module:
cyclone_tcp/sntp/sntp_client.{c,h}Type: Feature request / enhancement (backward compatible)
Problem
sntpClientGetTimestamp()runs the whole NTP transaction (open socket -> send -> wait for response -> parse) inside a singlewhile(!error)loop, so the call blocks the caller until a response arrives or the socket timeout expires.In an RTOS application, a slow or unreachable NTP server keeps the calling thread blocked for the entire socket timeout (
SNTP_CLIENT_DEFAULT_TIMEOUT, 30 s by default). This is a problem for any thread that must stay responsive and keep servicing its other periodic work while a synchronization is in progress: the thread cannot do anything else until the call returns. Lowering the socket timeout viasntpClientSetTimeout()is not an acceptable workaround, because it also shortens the legitimate wait for a reachable-but-slow server.Request
A non-blocking way to issue an SNTP request, so the application can start a request and then poll for the result from its own periodic handler, never blocking the calling thread. The existing blocking
sntpClientGetTimestamp()should stay unchanged for backward compatibility.Proposed API (additive, no change to existing functions)
These reuse the existing internal helpers (
sntpClientOpenConnection,sntpClientSendRequest,sntpClientReceiveResponse,sntpClientParseResponse,sntpClientCloseConnection) and the existingSntpClientStatemachine, so the diff is small and the blocking path is untouched.Sketch of the implementation
Typical caller usage (from a periodic RTOS handler, e.g. every N ms), with a short socket timeout set via
sntpClientSetTimeout()so each poll returns fast:Why upstream
We currently carry this as a local patch to the vendored library, which makes future CycloneTCP updates harder to merge. A supported non-blocking SNTP API in the library would let us drop the local patch entirely.
Environment
Thanks for the library and for considering this. I'm happy to open a pull request with the implementation and a doc comment if the approach looks good.