Skip to content

Commit

Permalink
Fix: GGD Test failures (aws#1616)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@muneebahmed10 @yuhui-zheng
muneebahmed10 authored and yuhui-zheng committed Feb 13, 2020
1 parent 4c066b1 commit 8025c83
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 42 deletions.
6 changes: 3 additions & 3 deletions libraries/freertos_plus/aws/greengrass/include/aws_greengrass_discovery.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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.
Expand Down
27 changes: 13 additions & 14 deletions libraries/freertos_plus/aws/greengrass/src/aws_greengrass_discovery.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -433,43 +433,42 @@ 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;
}
}

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;
}
Expand Down
71 changes: 46 additions & 25 deletions libraries/freertos_plus/aws/greengrass/test/aws_test_greengrass_discovery.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 )
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 );
Expand Down Expand Up @@ -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 );
Expand All @@ -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,
Expand All @@ -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." );
Expand All @@ -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 );
Expand Down

0 comments on commit 8025c83

Please sign in to comment.