Sync Keep Alive fix from C-SDK (#100)

2 files changed, 67 insertions, 0 deletions

diff --git a/FreeRTOS-Plus/Source/FreeRTOS-IoT-Libraries/c_sdk/standard/mqtt/src/iot_mqtt_operation.c b/FreeRTOS-Plus/Source/FreeRTOS-IoT-Libraries/c_sdk/standard/mqtt/src/iot_mqtt_operation.c
index fe2edd002..986441d43 100755
--- a/FreeRTOS-Plus/Source/FreeRTOS-IoT-Libraries/c_sdk/standard/mqtt/src/iot_mqtt_operation.c
+++ b/FreeRTOS-Plus/Source/FreeRTOS-IoT-Libraries/c_sdk/standard/mqtt/src/iot_mqtt_operation.c
@@ -886,8 +886,54 @@ void _IotMqtt_ProcessKeepAlive( taskPoolJob_t * pKeepAliveJob,
             IotLogDebug( "(MQTT connection %p) PINGRESP was received.", pMqttConnection );
 
             /* PINGRESP was received. Schedule the next PINGREQ transmission. */
+
+            /* This function is called for two purposes:
+             *
+             * 1. To send a PINGREQ.
+             * 2. To check that the corresponding PINGRESP is received within
+             * IOT_MQTT_RESPONSE_WAIT_MS.
+             *
+             * The way it differentiates between the two is by checking
+             * ping.nextPeriodMs in pPingreqOperation->u.operation.periodic:
+             *
+             * If ping.nextPeriodMs is set to ping.keepAliveMs,
+             * the invocation is for sending PINGREQ.
+             * Otherwise, the invocation is for checking that PINGRESP is received
+             * within IOT_MQTT_RESPONSE_WAIT_MS.
+             *
+             * Therefore, it is necessary to set ping.nextPeriodMs
+             * to ping.keepAliveMs to ensure that PINGREQ is sent in
+             * the next invocation. But we must ensure that the next time to send
+             * PINGREQ is calculated from the moment last PINGREQ was sent and NOT
+             * when we checked for PINGRESP. As a result we need to schedule the next
+             * invocation at ping.keepAliveMs - IOT_MQTT_RESPONSE_WAIT_MS.
+             * The following diagram also explains it:
+             *
+             *      WaitMS    KeepAliveMS - WaitMS
+             *    <-------->|<-------------------->
+             *    ---------------------------------
+             *    ^         ^                     ^
+             *    |         |                     |
+             * PINGREQ   PINGRESP              PINGREQ
+             * (Call 1)  (Call 2)             (Call 3)
+             *    <------------------------------->
+             *                 KeepAliveMS
+             * WaitMS = IOT_MQTT_RESPONSE_WAIT_MS.
+             * KeepAliveMS = pPingreqOperation->u.operation.periodic.ping.keepAliveMs.
+             * Call 1 - First PINGREQ is sent.
+             * Call 2 - PINGRESP is checked after IOT_MQTT_RESPONSE_WAIT_MS.
+             * Call 3 - Next PINGREQ is sent. Time difference between Call 2 and
+             * Call 3 is KeepAliveMS - WaitMS, while time difference between Call 1
+             * and Call 3 is KeepAliveMS. */
             pPingreqOperation->u.operation.periodic.ping.nextPeriodMs =
                 pPingreqOperation->u.operation.periodic.ping.keepAliveMs;
+
+            IotMqtt_Assert( pPingreqOperation->u.operation.periodic.ping.keepAliveMs
+                            > IOT_MQTT_RESPONSE_WAIT_MS );
+
+            /* Subtract time taken for PINGRESP check. */
+            scheduleDelay = pPingreqOperation->u.operation.periodic.ping.keepAliveMs
+                            - IOT_MQTT_RESPONSE_WAIT_MS;
         }
         else
         {
diff --git a/FreeRTOS-Plus/Source/FreeRTOS-IoT-Libraries/c_sdk/standard/mqtt/src/iot_mqtt_validate.c b/FreeRTOS-Plus/Source/FreeRTOS-IoT-Libraries/c_sdk/standard/mqtt/src/iot_mqtt_validate.c
index 26381594f..cdcca62f0 100755
--- a/FreeRTOS-Plus/Source/FreeRTOS-IoT-Libraries/c_sdk/standard/mqtt/src/iot_mqtt_validate.c
+++ b/FreeRTOS-Plus/Source/FreeRTOS-IoT-Libraries/c_sdk/standard/mqtt/src/iot_mqtt_validate.c
@@ -595,6 +595,27 @@ bool _IotMqtt_ValidateConnect( const IotMqttConnectInfo_t * pConnectInfo )
 
     if( status == true )
     {
+        /* Check that keep alive is not too short. */
+        if( pConnectInfo->keepAliveSeconds != 0U )
+        {
+            /* After sending a PINGREQ, we wait for IOT_MQTT_RESPONSE_WAIT_MS to
+             * receive the corresponding PINGRESP. If the PINGRESP is received within
+             * IOT_MQTT_RESPONSE_WAIT_MS, we schedule another job to send PINGREQ.
+             * If the IOT_MQTT_RESPONSE_WAIT_MS is longer than keep alive interval,
+             * we will fail to send PINGRESP on time. */
+            if( ( pConnectInfo->keepAliveSeconds * 1000U ) <= IOT_MQTT_RESPONSE_WAIT_MS )
+            {
+                IotLogError( "Keep alive interval %u ms must be longer than response wait time %u ms.",
+                             pConnectInfo->keepAliveSeconds * 1000U,
+                             IOT_MQTT_RESPONSE_WAIT_MS );
+
+                status = false;
+            }
+        }
+    }
+
+    if( status == true )
+    {
         /* If will info is provided, check that it is valid. */
         if( pConnectInfo->pWillInfo != NULL )
         {