Skip to content

Add a non-blocking SNTP request API to the SNTP client #33

Description

@butorinvs

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions