Fix clock drift issue in SNTP demo (#655)

Issue
The existing clock discipline algorithm in the coreSNTP demo yields an inaccurate wall-clock time (in UTC) that is always behind the astronomical UTC time by the chosen polling interval period of the demo. The issue with the clock discipline algorithm was that it was calculating the slew rate to make the system clock be gradually compensated for the previous time synchronization clock drift over the period of the subsequent polling time interval. Thereby, the clock slew rate correction calculated is always lower than it should be (because it does not incorporate possible clock drift that would occur over another period of time polling interval till the next time synchronization attempt).

Fix
This PR fixes the clock discipline algorithm to make the demo calculate its system time that matches the astronomical UTC time.
The fix involves simplifying the clock discipline algorithm to use a combination of step and slew correction approaches where:

1. Step correction is applied on EVERY time successful synchronization with a time server. This immediately corrects the system time to match server time.
2. Slew rate is calculated ONCE (on the second successful time synchronization with server) and applied henceforth, on every time polling interval period to compensate gradually for the system clock drift during the period.

3 files changed, 66 insertions, 92 deletions

diff --git a/FreeRTOS-Plus/Demo/coreSNTP_Windows_Simulator/DemoTasks/SNTPClientTask.c b/FreeRTOS-Plus/Demo/coreSNTP_Windows_Simulator/DemoTasks/SNTPClientTask.c
index a3497d463..423f07630 100755
--- a/FreeRTOS-Plus/Demo/coreSNTP_Windows_Simulator/DemoTasks/SNTPClientTask.c
+++ b/FreeRTOS-Plus/Demo/coreSNTP_Windows_Simulator/DemoTasks/SNTPClientTask.c
@@ -98,12 +98,8 @@
     #error "Define the democonfigLIST_OF_AUTHENTICATION_KEY_IDS config by following the instructions in demo_config.h file."
 #endif
 
-#ifndef democonfigDESIRED_CLOCK_ACCURACY_MS
-    #error "Define the democonfigDESIRED_CLOCK_ACCURACY_MS config by following instructions in demo_config.h file."
-#endif
-
-#ifndef democonfigSYSTEM_CLOCK_TOLERANCE_PPM
-    #error "Define the democonfigSYSTEM_CLOCK_TOLERANCE_PPM config by following instructions in demo_config.h file."
+#ifndef democonfigSNTP_CLIENT_POLLING_INTERVAL_SECONDS
+    #error "Define the democonfigSNTP_CLIENT_POLLING_INTERVAL_SECONDS config by following instructions in demo_config.h file."
 #endif
 
 #ifndef democonfigSYSTEM_START_YEAR
@@ -434,30 +430,33 @@ static void sntpClient_GetTime( SntpTimestamp_t * pCurrentTime );
  * from the server response and the clock-offset value calculated by
  * the coreSNTP library.
  *
- * @note This demo uses either the "slew" OR "step" methodology of system
- * clock correction based on the use-case:
- * 1. "Step" correction is used if:
- *   - System time is ahead of server time so that system time is immediately
- *     corrected instead of potentially receding back in time with a "slew"
- *     correction approach.
- *                                      OR
- *   - It is the first time synchronization for the system since boot-up. Using
- *     "step" approach immediately corrects the system if it is far away from the
- *     server time on device startup instead of slowly correcting over time with
- *     the "slew" approach.
+ * @note This demo uses a combination of "step" AND "slew" methodology
+ * for system clock correction.
+ * 1. "Step" correction is ALWAYS used to immediately correct the system clock
+ *    to match server time on every successful time synchronization with a
+ *    time server (that occurs periodically on the poll interval gaps).
  *
- * 2. The "slew" correction approach is used for all cases other than the above
- *    as they represent regular time synchronization during device runtime where
- *    the system time may have drifted behind the server time, and can be corrected
- *    gradually over the SNTP client's polling interval period.
+ * 2. "Slew" correction approach is used for compensating system clock drift
+ *    during the poll interval period between time synchronization attempts with
+ *    time server(s) when latest time server is not known. The "slew rate" is
+ *    calculated ONLY once on the occasion of the second successful time
+ *    synchronization with a time server. This is because the demo initializes
+ *    system time with (the first second of) the democonfigSYSTEM_START_YEAR
+ *    configuration, and thus, the the actual system clock drift over a period
+ *    of time can be calculated only AFTER the demo system time has been synchronized
+ *    with server time once. Thus, after the first time period of poll interval has
+ *    transpired, the system clock drift is calculated correctly on the subsequent
+ *    successful time synchronization with a time server.
  *
  * @note The above system clock correction algorithm is just one example of a correction
- * approach. It can be modified to suit your application needs. Examples include:
- * - Always using "slew" correction if the device is always within a small time offset from
- *   server and your application is sensitive to non-abrupt changes in time (that could occur
- *   with "step" approach) for use-cases like logging events in correct order
- * - Always using a "step" approach for a simplicity if your application is not sensitive to
- *   abrupt changes/progress in time.
+ * approach. It can be modified to suit your application needs. For example, your
+ * application can use ONLY the "step" correction methodology for simplicity of system clock
+ * time calculation logic if the application is not sensitive to abrupt time changes
+ * (that occur at the instances of periodic time synchronization attempts). In such a case,
+ * the Sntp_CalculatePollInterval() API of coreSNTP library can be used to calculate
+ * the optimum time polling period for your application based on the factors of your
+ * system's clock drift rate and the maximum clock drift tolerable by your application.
+ *
  *
  * @param[in] pTimeServer The time server from whom the time has been received.
  * @param[in] pServerTime The most recent time of the server, @p pTimeServer, sent in its
@@ -874,58 +873,42 @@ static void sntpClient_SetTime( const SntpServerInfo_t * pTimeServer,
     /* Obtain the mutext for accessing system clock variables. */
     xSemaphoreTake( xMutex, portMAX_DELAY );
 
-    /* Use "step" approach if:
-     * The system clock has drifted ahead of server time.
-     *                         OR
-     * This is the first time synchronization with NTP server since device boot-up.
-     */
-    if( ( clockOffsetMs < 0 ) || ( systemClock.firstTimeSyncDone == false ) )
-    {
-        SntpStatus_t status;
-        uint32_t unixSecs;
-        uint32_t unixMicroSecs;
-
-        /* Convert server time from NTP timestamp to UNIX format. */
-        status = Sntp_ConvertToUnixTime( pServerTime,
-                                         &unixSecs,
-                                         &unixMicroSecs );
-        configASSERT( status == SntpSuccess );
+    /* Always correct the system base time on receiving time from server.*/
+    SntpStatus_t status;
+    uint32_t unixSecs;
+    uint32_t unixMicroSecs;
 
-        /* Immediately correct the base time of the system clock as server time. */
-        systemClock.baseTime.secs = unixSecs;
-        systemClock.baseTime.msecs = unixMicroSecs / 1000;
+    /* Convert server time from NTP timestamp to UNIX format. */
+    status = Sntp_ConvertToUnixTime( pServerTime,
+                                     &unixSecs,
+                                     &unixMicroSecs );
+    configASSERT( status == SntpSuccess );
 
-        /* Reset slew rate to zero as the time has been immediately corrected to server time. */
-        systemClock.slewRate = 0;
+    /* Always correct the base time of the system clock as the time received from the server. */
+    systemClock.baseTime.secs = unixSecs;
+    systemClock.baseTime.msecs = unixMicroSecs / 1000;
 
-        /* Store the tick count of the current time synchronization in the system clock. */
-        systemClock.lastSyncTickCount = xTaskGetTickCount();
+    /* Set the clock adjustment "slew" rate of system clock if it wasn't set already and this is NOT
+     * the first clock synchronization since device boot-up. */
+    if( ( systemClock.firstTimeSyncDone == true ) && ( systemClock.slewRate == 0 ) )
+    {
+        /* We will use a "slew" correction approach to compensate for system clock
+         * drift over poll interval period that exists between consecutive time synchronizations
+         * with time server. */
 
-        /* Set the system clock flag that indicates completion of the first time synchronization since device boot-up. */
-        if( systemClock.firstTimeSyncDone == false )
-        {
-            systemClock.firstTimeSyncDone = true;
-        }
+        /* Calculate the "slew" rate for system clock as milliseconds of adjustment needed per second. */
+        systemClock.slewRate = clockOffsetMs / systemClock.pollPeriod;
     }
 
-    /* As the system clock is behind server time, we will use a "slew" approach to gradually
-     * correct system time over the poll interval period. */
-    else
+    /* Set the system clock flag that indicates completion of the first time synchronization since device boot-up. */
+    if( systemClock.firstTimeSyncDone == false )
     {
-        /* Update the base time based on the previous slew rate and the time period transpired
-         * since last time synchronization. */
-        calculateCurrentTime( &systemClock.baseTime,
-                              systemClock.lastSyncTickCount,
-                              systemClock.slewRate,
-                              &systemClock.baseTime );
-
-        /* Calculate the new slew rate as offset in milliseconds of adjustment per second. */
-        systemClock.slewRate = clockOffsetMs / systemClock.pollPeriod;
-
-        /* Store the tick count of the current time synchronization in the system clock. */
-        systemClock.lastSyncTickCount = xTaskGetTickCount();
+        systemClock.firstTimeSyncDone = true;
     }
 
+    /* Store the tick count of the current time synchronization in the system clock. */
+    systemClock.lastSyncTickCount = xTaskGetTickCount();
+
     xSemaphoreGive( xMutex );
 }
 
@@ -1531,11 +1514,8 @@ void sntpTask( void * pParameters )
         SntpStatus_t status;
         bool backoffModeFlag = false;
 
-        /* Calculate Poll interval of SNTP client based on desired accuracy and clock tolerance of the system. */
-        status = Sntp_CalculatePollInterval( democonfigSYSTEM_CLOCK_TOLERANCE_PPM,
-                                             democonfigDESIRED_CLOCK_ACCURACY_MS,
-                                             &systemClock.pollPeriod );
-        configASSERT( status == SntpSuccess );
+        /* Set the polling interval for periodic time synchronization attempts by the SNTP client. */
+        systemClock.pollPeriod = democonfigSNTP_CLIENT_POLLING_INTERVAL_SECONDS;
 
         LogDebug( ( "Minimum SNTP client polling interval calculated as %lus", systemClock.pollPeriod ) );
 
diff --git a/FreeRTOS-Plus/Demo/coreSNTP_Windows_Simulator/WIN32.vcxproj b/FreeRTOS-Plus/Demo/coreSNTP_Windows_Simulator/WIN32.vcxproj
index 17cdea642..de738b93c 100755
--- a/FreeRTOS-Plus/Demo/coreSNTP_Windows_Simulator/WIN32.vcxproj
+++ b/FreeRTOS-Plus/Demo/coreSNTP_Windows_Simulator/WIN32.vcxproj
@@ -58,7 +58,7 @@
     </Midl>
     <ClCompile>
       <Optimization>Disabled</Optimization>
-      <AdditionalIncludeDirectories>..\..\Source\Utilities\mbedtls_freertos;..\..\..\Source\FreeRTOS-Plus-Trace\Include;..\..\..\FreeRTOS-Plus\Source\FreeRTOS-Plus-TCP\include;..\..\..\FreeRTOS-Plus\Source\FreeRTOS-Plus-TCP\portable\BufferManagement;..\..\..\FreeRTOS-Plus\Source\FreeRTOS-Plus-TCP\portable\Compiler\MSVC;..\..\..\FreeRTOS-Plus\Source\Utilities\logging;..\Common\WinPCap;..\..\..\FreeRTOS\Source\include;..\..\..\FreeRTOS-Plus\Source\corePKCS11\source\include;..\..\..\FreeRTOS-Plus\Source\corePKCS11\source\dependency\3rdparty\pkcs11;..\..\ThirdParty\mbedtls\include;..\..\..\FreeRTOS-Plus\Source\corePKCS11\source\dependency\3rdparty\mbedtls_utils;..\..\..\FreeRTOS\Source\portable\MSVC-MingW;..\..\Source\Application-Protocols\coreSNTP\source\include;.;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
+      <AdditionalIncludeDirectories>..\..\Source\Utilities\mbedtls_freertos;..\..\..\Source\FreeRTOS-Plus-Trace\Include;..\..\..\FreeRTOS-Plus\Source\FreeRTOS-Plus-TCP\include;..\..\..\FreeRTOS-Plus\Source\FreeRTOS-Plus-TCP\portable\BufferManagement;..\..\..\FreeRTOS-Plus\Source\FreeRTOS-Plus-TCP\portable\Compiler\MSVC;..\..\..\FreeRTOS-Plus\Source\Utilities\logging;..\Common\WinPCap;..\..\..\FreeRTOS\Source\include;..\..\..\FreeRTOS-Plus\Source\corePKCS11\source\include;..\..\..\FreeRTOS-Plus\Source\corePKCS11\source\dependency\3rdparty\pkcs11;..\..\ThirdParty\mbedtls\include;..\..\..\FreeRTOS-Plus\Source\corePKCS11\source\dependency\3rdparty\mbedtls_utils;..\..\..\FreeRTOS\Source\portable\MSVC-MingW;..\..\Source\Application-Protocols\coreSNTP\source\include;..\..\Source\Utilities\backoff_algorithm\source\include;.;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
       <PreprocessorDefinitions>WIN32;_DEBUG;_CONSOLE;_WIN32_WINNT=0x0500;WINVER=0x400;_CRT_SECURE_NO_WARNINGS;MBEDTLS_CONFIG_FILE="mbedtls_config.h";CONFIG_MEDTLS_USE_AFR_MEMORY;%(PreprocessorDefinitions)</PreprocessorDefinitions>
       <MinimalRebuild>false</MinimalRebuild>
       <BasicRuntimeChecks>EnableFastChecks</BasicRuntimeChecks>
@@ -248,6 +248,7 @@
     <ClCompile Include="..\..\..\FreeRTOS-Plus\Source\FreeRTOS-Plus-TCP\FreeRTOS_UDP_IP.c" />
     <ClCompile Include="..\..\..\FreeRTOS-Plus\Source\FreeRTOS-Plus-TCP\portable\BufferManagement\BufferAllocation_2.c" />
     <ClCompile Include="..\..\..\FreeRTOS-Plus\Source\FreeRTOS-Plus-TCP\portable\NetworkInterface\WinPCap\NetworkInterface.c" />
+    <ClCompile Include="..\..\Source\Utilities\backoff_algorithm\source\backoff_algorithm.c" />
     <ClCompile Include="..\..\Source\Application-Protocols\coreSNTP\source\core_sntp_serializer.c" />
     <ClCompile Include="..\..\Source\Application-Protocols\coreSNTP\source\core_sntp_client.c" />
     <ClCompile Include="..\..\..\FreeRTOS-Plus\Demo\Common\Logging\windows\Logging_WinSim.c" />
@@ -378,6 +379,7 @@
     <ClInclude Include="..\..\Source\corePKCS11\source\include\core_pkcs11.h" />
     <ClInclude Include="..\..\Source\corePKCS11\source\include\core_pkcs11_pal.h" />
     <ClInclude Include="..\..\Source\corePKCS11\source\include\core_pki_utils.h" />
+    <ClInclude Include="..\..\Source\Utilities\backoff_algorithm\source\include\backoff_algorithm.h" />
     <ClInclude Include="..\..\Source\Application-Protocols\coreSNTP\source\include\core_sntp_config_defaults.h" />
     <ClInclude Include="..\..\Source\Application-Protocols\coreSNTP\source\include\core_sntp_serializer.h" />
     <ClInclude Include="..\..\Source\Application-Protocols\coreSNTP\source\include\core_sntp_client.h" />
diff --git a/FreeRTOS-Plus/Demo/coreSNTP_Windows_Simulator/demo_config.h b/FreeRTOS-Plus/Demo/coreSNTP_Windows_Simulator/demo_config.h
index 3f62b4dc9..a73395231 100755
--- a/FreeRTOS-Plus/Demo/coreSNTP_Windows_Simulator/demo_config.h
+++ b/FreeRTOS-Plus/Demo/coreSNTP_Windows_Simulator/demo_config.h
@@ -68,23 +68,15 @@ extern void vLoggingPrintf( const char * pcFormatString,
 /************ End of logging configuration ****************/
 
 /**
- * @brief The desired accuracy (in milliseconds) of system clock in relation with internet time.
- * In other words, this is the maximum tolerance desired for clock drift in the system.
+ * @brief The time period between consecutive time polling requests that are sent by the
+ * SNTP client in the demo application.
+ *
+ * @note According to the SNTPv4 specification, the polling interval MUST NOT be less
+ * than 15 seconds for responsible use of time servers by SNTP clients.
+ * 
+ * 
+ * #define democonfigSNTP_CLIENT_POLLING_INTERVAL_SECONDS                  ( 16 )
  */
-#define democonfigDESIRED_CLOCK_ACCURACY_MS     ( 1000 )
-
-/**
- * @brief The system clock tolerance (in parts per million) that represents the rate of clock
- * drift of the system. The rate can be viewed as the system clock drift that occurs as milliseconds
- * per second.
- *
- * @note In systems with a HW oscillator based clock, the frequency tolerance can be obtained from the
- * hardware specification of the clock oscillator.
- * @note This demo DOES NOT use a hardware clock oscillator as it runs on Windows Simulator. The configuration
- * ONLY provides the user to view the impact of the settings for "Clock Tolerance" and "Desired
- * Clock Accuracy" configurations on the calculated "SNTP polling period".
- */
-#define democonfigSYSTEM_CLOCK_TOLERANCE_PPM    ( 32000 )
 
 /**
  * @brief The set of time servers, in decreasing order of priority, for configuring the SNTP client.