From 8025c83f5640c235d323a221bb792b940a46c282 Mon Sep 17 00:00:00 2001 From: Muneeb Ahmed <54290492+muneebahmed10@users.noreply.github.com> Date: Wed, 8 Jan 2020 18:42:47 -0700 Subject: [PATCH] Fix: GGD Test failures (#1616) * Fix: GGD Test failures on ST and other boards GGD_JSONRequestGetFile() would not receive the entire file in one call on some boards. This adds a loop to receive the file in chunks if the request was not completed. * Add logging statement to GGD File Retrieval --- .../include/aws_greengrass_discovery.h | 6 +- .../greengrass/src/aws_greengrass_discovery.c | 27 ++++--- .../test/aws_test_greengrass_discovery.c | 71 ++++++++++++------- 3 files changed, 62 insertions(+), 42 deletions(-) diff --git a/libraries/freertos_plus/aws/greengrass/include/aws_greengrass_discovery.h b/libraries/freertos_plus/aws/greengrass/include/aws_greengrass_discovery.h index 712201f02c3..a4a2e41ad4c 100644 --- a/libraries/freertos_plus/aws/greengrass/include/aws_greengrass_discovery.h +++ b/libraries/freertos_plus/aws/greengrass/include/aws_greengrass_discovery.h @@ -145,7 +145,7 @@ BaseType_t GGD_JSONRequestGetSize( Socket_t * pxSocket, * GGD_JSONRequestStart. * * @note 1. Parse the HTML response to extract the JSON file. - * This function allows the JSON file retrieval in chunck. + * This function allows the JSON file retrieval in chunks. * The user can call the function several time with small buffer size. * The JSON file will be retrieved bit by bit until * xJSONFileRetrieveCompleted is set to true. @@ -161,7 +161,7 @@ BaseType_t GGD_JSONRequestGetSize( Socket_t * pxSocket, * * @param [in] ulBufferSize: Size of the memory buffer provided. * - * @param [in] pulJSONFileSize: Size of JSON file to be retrieved. + * @param [in] ulJSONFileSize: Size of JSON file to be retrieved. * * @param [out] pulByteRead: Must be set to zero by the user * prior first calling of GGD_GetJSONFile. Then the number of @@ -182,7 +182,7 @@ BaseType_t GGD_JSONRequestGetFile( Socket_t * pxSocket, const uint32_t ulBufferSize, uint32_t * pulByteRead, BaseType_t * pxJSONFileRetrieveCompleted, - const uint32_t pulJSONFileSize ); + const uint32_t ulJSONFileSize ); /* * @brief Need to be called if GGD_JSONRequestGetFile cannot be called. diff --git a/libraries/freertos_plus/aws/greengrass/src/aws_greengrass_discovery.c b/libraries/freertos_plus/aws/greengrass/src/aws_greengrass_discovery.c index e824ea57f27..4769e30d105 100644 --- a/libraries/freertos_plus/aws/greengrass/src/aws_greengrass_discovery.c +++ b/libraries/freertos_plus/aws/greengrass/src/aws_greengrass_discovery.c @@ -411,7 +411,7 @@ BaseType_t GGD_JSONRequestGetFile( Socket_t * pxSocket, const uint32_t ulBufferSize, uint32_t * pulByteRead, BaseType_t * pxJSONFileRetrieveCompleted, - const uint32_t pulJSONFileSize ) + const uint32_t ulJSONFileSize ) { BaseType_t xStatus; uint32_t ulDataSizeRead; @@ -433,11 +433,11 @@ BaseType_t GGD_JSONRequestGetFile( Socket_t * pxSocket, *pulByteRead += ulDataSizeRead; /* We retrieved more than expected, this is failed. */ - if( *pulByteRead > ( pulJSONFileSize - ( uint32_t ) 1 ) ) + if( *pulByteRead > ( ulJSONFileSize - ( uint32_t ) 1 ) ) { ggdconfigPRINT( "JSON parsing - Received %ld, expected at most %ld \r\n", *pulByteRead, - pulJSONFileSize - ( uint32_t ) 1 ); + ulJSONFileSize - ( uint32_t ) 1 ); xStatus = pdFAIL; } } @@ -445,31 +445,30 @@ BaseType_t GGD_JSONRequestGetFile( Socket_t * pxSocket, if( xStatus == pdPASS ) { /* We still have more to retrieve. */ - if( *pulByteRead < ( pulJSONFileSize - ( uint32_t ) 1 ) ) + if( *pulByteRead < ( ulJSONFileSize - ( uint32_t ) 1 ) ) { *pxJSONFileRetrieveCompleted = pdFALSE; + ggdconfigPRINT( "JSON file retrieval incomplete, received %ld out of %ld bytes\r\n", + *pulByteRead, + ulJSONFileSize - ( uint32_t ) 1 ); } else { /* Add the escape character. */ - pcBuffer[ pulJSONFileSize - ( uint32_t ) 1 ] = '\0'; + pcBuffer[ ulJSONFileSize - ( uint32_t ) 1 ] = '\0'; *pxJSONFileRetrieveCompleted = pdTRUE; + ggdconfigPRINT( "JSON file retrieval completed\r\n" ); + + /* Close the connection. */ + GGD_SecureConnect_Disconnect( pxSocket ); } } - - if( xStatus == pdFAIL ) + else { ggdconfigPRINT( "JSON parsing - JSON file retrieval failed\r\n" ); /* Don't forget to close the connection. */ GGD_SecureConnect_Disconnect( pxSocket ); } - else - { - if( *pxJSONFileRetrieveCompleted == pdTRUE ) - { - GGD_SecureConnect_Disconnect( pxSocket ); - } - } return xStatus; } diff --git a/libraries/freertos_plus/aws/greengrass/test/aws_test_greengrass_discovery.c b/libraries/freertos_plus/aws/greengrass/test/aws_test_greengrass_discovery.c index 29111fbce9e..277da60d047 100644 --- a/libraries/freertos_plus/aws/greengrass/test/aws_test_greengrass_discovery.c +++ b/libraries/freertos_plus/aws/greengrass/test/aws_test_greengrass_discovery.c @@ -47,6 +47,7 @@ #define ggdTestJSON_PORT_ADDRESS_1 1234 #define ggdTestJSON_PORT_ADDRESS_3 4321 #define ggdTestLOOP_NUMBER 10 +#define ggdTestMAX_REQUEST_LOOP_COUNT 3 #define ggdJSON_FILE_GROUPID "GGGroupId" #define ggdJSON_FILE_THING_ARN "thingArn" @@ -64,6 +65,12 @@ static const char cMY_CORE_ARN[] = "myGreenGrassCoreArn"; static jsmntok_t pxTok[ ggdTestJSON_MAX_TOKENS ]; static Socket_t xSocket; +/* Helper function to call GGD_JSONRequestGetFile() in a loop. */ +static BaseType_t prvGGD_JSONRequestGetFileLoop( uint32_t ulBufferSize, + uint32_t * pulByteRead, + BaseType_t * pxJSONFileRetrieveCompleted, + uint32_t ulJSONFileSize ); + TEST_GROUP( Full_GGD ); TEST_SETUP( Full_GGD ) @@ -102,6 +109,28 @@ TEST_GROUP_RUNNER( Full_GGD ) RUN_TEST_CASE( Full_GGD, GetGGCIPandCertificate ); } +static BaseType_t prvGGD_JSONRequestGetFileLoop( uint32_t ulBufferSize, + uint32_t * pulByteRead, + BaseType_t * pxJSONFileRetrieveCompleted, + uint32_t ulJSONFileSize ) +{ + BaseType_t xStatus = pdPASS; + /* Limit loop count to avoid any chance of infinite loops. */ + uint32_t ulRequestLoopCounter = 0; + + do + { + xStatus = GGD_JSONRequestGetFile( &xSocket, + &cBuffer[ *pulByteRead ], + ulBufferSize - *pulByteRead, + pulByteRead, + pxJSONFileRetrieveCompleted, + ulJSONFileSize ); + } while( ( xStatus == pdPASS ) && ( *pxJSONFileRetrieveCompleted != pdTRUE ) && ( ulBufferSize > *pulByteRead ) && ( ++ulRequestLoopCounter < ggdTestMAX_REQUEST_LOOP_COUNT ) ); + + return xStatus; +} + TEST( Full_GGD, JSONRequestAbort ) { /** @brief check for stability for all meaningfull values of socket. @@ -318,12 +347,10 @@ TEST( Full_GGD, GetIPandCertificateFromJSON ) if( xStatus == pdPASS ) { ulByteRead = 0; - xStatus = GGD_JSONRequestGetFile( &xSocket, - cBuffer, - ulBufferSize, - &ulByteRead, - &xJSONFileRetrieveCompleted, - ulJSONFileSize ); + xStatus = prvGGD_JSONRequestGetFileLoop( ulBufferSize, + &ulByteRead, + &xJSONFileRetrieveCompleted, + ulJSONFileSize ); } TEST_ASSERT_EQUAL_INT32( pdPASS, xStatus ); @@ -933,12 +960,10 @@ TEST( Full_GGD, JSONRequestGetFile ) if( xStatus == pdPASS ) { ulByteRead = 0; - xStatus = GGD_JSONRequestGetFile( &xSocket, - cBuffer, - ulBufferSize, - &ulByteRead, - &xJSONFileRetrieveCompleted, - ulJSONFileSize ); + xStatus = prvGGD_JSONRequestGetFileLoop( ulBufferSize, + &ulByteRead, + &xJSONFileRetrieveCompleted, + ulJSONFileSize ); } TEST_ASSERT_EQUAL_INT32( SOCKETS_INVALID_SOCKET, xSocket ); @@ -948,7 +973,7 @@ TEST( Full_GGD, JSONRequestGetFile ) /** @}*/ - /** @brief Retrieve the JSON file in two separate chunks. + /** @brief Retrieve the JSON file in separate chunks. * @{ */ xStatus = GGD_JSONRequestStart( clientcredentialMQTT_BROKER_ENDPOINT, @@ -975,12 +1000,10 @@ TEST( Full_GGD, JSONRequestGetFile ) TEST_ASSERT_EQUAL_INT32( pdPASS, xStatus ); TEST_ASSERT_EQUAL_INT32( pdFALSE, xJSONFileRetrieveCompleted ); /* Not yet retrieved - only half of it. */ - xStatus = GGD_JSONRequestGetFile( &xSocket, - &cBuffer[ ulByteRead ], - ulBufferSize - ( ulJSONFileSize / 2 ), - &ulByteRead, - &xJSONFileRetrieveCompleted, - ulJSONFileSize ); + xStatus = prvGGD_JSONRequestGetFileLoop( ulBufferSize, + &ulByteRead, + &xJSONFileRetrieveCompleted, + ulJSONFileSize ); } TEST_ASSERT_EQUAL_INT32_MESSAGE( pdPASS, xStatus, "GGD_JSONRequestGetFile() failed to return pdPASS." ); @@ -1004,12 +1027,10 @@ TEST( Full_GGD, JSONRequestGetFile ) if( xStatus == pdPASS ) { ulByteRead = 0; - xStatus = GGD_JSONRequestGetFile( &xSocket, - cBuffer, - ulBufferSize, - &ulByteRead, - &xJSONFileRetrieveCompleted, - ulJSONFileSize - 1 ); /* Remove one byte to the expected JSON file size. */ + xStatus = prvGGD_JSONRequestGetFileLoop( ulBufferSize, + &ulByteRead, + &xJSONFileRetrieveCompleted, + ulJSONFileSize - 1 ); /* Remove one byte from the expected JSON file size. */ } TEST_ASSERT_EQUAL_INT32( pdFAIL, xStatus );