Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modify a check to make sure that keep alive is sent even when data is in the buffer #223

Merged
merged 5 commits into from
Sep 29, 2022
Merged

Modify a check to make sure that keep alive is sent even when data is in the buffer #223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
405a99e
Modify check to send keep alive when no data is received
AniruddhaKanhere Sep 29, 2022
dd0a95f
Modify control flow to process keepalive.
AniruddhaKanhere Sep 29, 2022
6391c34
Fix formatting and increase coverage
AniruddhaKanhere Sep 29, 2022
31e3632
Remove unused mocked functions
AniruddhaKanhere Sep 29, 2022
d709fc1
Update log messages
AniruddhaKanhere Sep 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 26 additions & 9 deletions source/core_mqtt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1678,31 +1678,48 @@ static MQTTStatus_t receiveSingleIteration( MQTTContext_t * pContext,
totalMQTTPacketLength = incomingPacket.remainingLength + incomingPacket.headerLength;
}

if( status == MQTTNoDataAvailable )
/* No data was received, check for keep alive timeout. */
if( recvBytes == 0 )
{
if( manageKeepAlive == true )
{
/* Keep the copy of the status to be reset later. */
MQTTStatus_t statusCopy = status;

/* Assign status so an error can be bubbled up to application,
* but reset it on success. */
status = handleKeepAlive( pContext );
}

if( status == MQTTSuccess )
{
/* Reset the status to indicate that nothing was read
* from the transport interface. */
status = MQTTNoDataAvailable;
if( status == MQTTSuccess )
{
/* Reset the status. */
status = statusCopy;
}
else
{
LogError( ( "Handling of keep alive failed. Status=%s",
MQTT_Status_strerror( status ) ) );
}
}
}

/* Check whether there is data available before processing the packet further. */
if( ( status == MQTTNeedMoreBytes ) || ( status == MQTTNoDataAvailable ) )
{
/* Do nothing as there is nothing to be processed right now. The proper
* error code will be bubbled up to the user. */
}
/* Any other error code. */
else if( status != MQTTSuccess )
{
LogError( ( "Receiving incoming packet length failed. Status=%s",
LogError( ( "Call to receiveSingleIteration failed. Status=%s",
MQTT_Status_strerror( status ) ) );
}
/* If the MQTT Packet size is bigger than the buffer itself. */
else if( totalMQTTPacketLength > pContext->networkBuffer.size )
{
/* Discard the packet from the buffer and from the socket buffer. */
/* Discard the packet from the receive buffer and drain the pending
* data from the socket buffer. */
status = discardStoredPacket( pContext,
&incomingPacket );
}
Expand Down
115 changes: 103 additions & 12 deletions test/unit-test/core_mqtt_utest.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3766,6 +3766,7 @@ void test_MQTT_ProcessLoop_handleKeepAlive_Error_Paths1( void )
setupNetworkBuffer( &networkBuffer );
setupTransportInterface( &transport );
transport.writev = transportWritevSuccess;
transport.recv = transportRecvNoData;

globalEntryTime = MQTT_PINGRESP_TIMEOUT_MS + 1;

Expand All @@ -3778,21 +3779,11 @@ void test_MQTT_ProcessLoop_handleKeepAlive_Error_Paths1( void )
context.lastPacketRxTime = 0;
context.pingReqSendTimeMs = 0;
context.waitingForPingResp = true;

/* Set expected return values in the loop. */
resetProcessLoopParams( &expectParams );
expectParams.processLoopStatus = MQTTKeepAliveTimeout;

MQTTPacketInfo_t incomingPacket = { 0 };
/* Modify incoming packet depending on type to be tested. */
currentPacketType = MQTT_PACKET_TYPE_PINGRESP;
incomingPacket.type = currentPacketType;
incomingPacket.remainingLength = MQTT_SAMPLE_REMAINING_LENGTH;
incomingPacket.headerLength = MQTT_SAMPLE_REMAINING_LENGTH;

MQTT_ProcessIncomingPacketTypeAndLength_ExpectAnyArgsAndReturn( MQTTNoDataAvailable );
MQTT_ProcessIncomingPacketTypeAndLength_ReturnThruPtr_pIncomingPacket( &incomingPacket );


mqttStatus = MQTT_ProcessLoop( &context );
TEST_ASSERT_EQUAL( MQTTKeepAliveTimeout, mqttStatus );
}
Expand Down Expand Up @@ -3859,6 +3850,7 @@ void test_MQTT_ProcessLoop_handleKeepAlive_Error_Paths3( void )

setupTransportInterface( &transport );
setupNetworkBuffer( &networkBuffer );
transport.recv = transportRecvNoData;

modifyIncomingPacketStatus = MQTTNoDataAvailable;
globalEntryTime = MQTT_PINGRESP_TIMEOUT_MS + 1;
Expand All @@ -3876,21 +3868,120 @@ void test_MQTT_ProcessLoop_handleKeepAlive_Error_Paths3( void )
/* Set expected return values in the loop. */
resetProcessLoopParams( &expectParams );

MQTT_GetPingreqPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess );
MQTT_SerializePingreq_ExpectAnyArgsAndReturn( MQTTSuccess );

mqttStatus = MQTT_ProcessLoop( &context );
TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus );
}

/**
* @brief This test case covers all calls to the private method,
* handleKeepAlive(...),
* that result in the process loop returning an error.
*/
void test_MQTT_ProcessLoop_handleKeepAlive_Error_Paths4( void )
{
MQTTStatus_t mqttStatus;
MQTTContext_t context = { 0 };
TransportInterface_t transport = { 0 };
MQTTFixedBuffer_t networkBuffer = { 0 };
ProcessLoopReturns_t expectParams = { 0 };

setupTransportInterface( &transport );
transport.recv = transportRecvNoData;

setupNetworkBuffer( &networkBuffer );

modifyIncomingPacketStatus = MQTTNoDataAvailable;
globalEntryTime = MQTT_PINGRESP_TIMEOUT_MS + 1;

/* Coverage for the branch path where PINGRESP timeout interval has expired. */
mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer );
TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus );

globalEntryTime = PACKET_RX_TIMEOUT_MS + 1;
context.keepAliveIntervalSec = ( PACKET_TX_TIMEOUT_MS / 1000 ) + 1U;
context.lastPacketTxTime = 0;
context.lastPacketRxTime = 0;
context.pingReqSendTimeMs = 0;
context.waitingForPingResp = false;

/* Set the index to non-zero value to show that there is some data in the buffer
* to be processed. */
context.index = 12;

/* Set expected return values in the loop. */
resetProcessLoopParams( &expectParams );

MQTTPacketInfo_t incomingPacket = { 0 };
/* Modify incoming packet depending on type to be tested. */
currentPacketType = MQTT_PACKET_TYPE_PINGRESP;
incomingPacket.type = currentPacketType;
incomingPacket.remainingLength = MQTT_SAMPLE_REMAINING_LENGTH;
incomingPacket.headerLength = MQTT_SAMPLE_REMAINING_LENGTH;

MQTT_ProcessIncomingPacketTypeAndLength_ExpectAnyArgsAndReturn( MQTTNoDataAvailable );
MQTT_ProcessIncomingPacketTypeAndLength_ExpectAnyArgsAndReturn( MQTTNeedMoreBytes );
MQTT_ProcessIncomingPacketTypeAndLength_ReturnThruPtr_pIncomingPacket( &incomingPacket );

MQTT_GetPingreqPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess );
MQTT_SerializePingreq_ExpectAnyArgsAndReturn( MQTTSuccess );

mqttStatus = MQTT_ProcessLoop( &context );
TEST_ASSERT_EQUAL( MQTTNeedMoreBytes, mqttStatus );
}

/**
* @brief This test case covers all calls to the private method,
* handleKeepAlive(...),
* that result in the process loop returning an error.
*/
void test_MQTT_ProcessLoop_handleKeepAlive_Error_Paths5( void )
{
MQTTStatus_t mqttStatus;
MQTTContext_t context = { 0 };
TransportInterface_t transport = { 0 };
MQTTFixedBuffer_t networkBuffer = { 0 };
ProcessLoopReturns_t expectParams = { 0 };

setupTransportInterface( &transport );
transport.recv = transportRecvNoData;

setupNetworkBuffer( &networkBuffer );

modifyIncomingPacketStatus = MQTTNoDataAvailable;
globalEntryTime = MQTT_PINGRESP_TIMEOUT_MS + 1;

/* Coverage for the branch path where PINGRESP timeout interval has expired. */
mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer );
TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus );

globalEntryTime = PACKET_RX_TIMEOUT_MS - 1U;
context.keepAliveIntervalSec = ( PACKET_TX_TIMEOUT_MS / 1000 ) + 1U;
context.lastPacketTxTime = 0;
context.lastPacketRxTime = 0;
context.pingReqSendTimeMs = 0;
context.waitingForPingResp = false;

/* Set the index to non-zero value to show that there is some data in the buffer
* to be processed. */
context.index = 12;

/* Set expected return values in the loop. */
resetProcessLoopParams( &expectParams );

MQTTPacketInfo_t incomingPacket = { 0 };
/* Modify incoming packet depending on type to be tested. */
currentPacketType = MQTT_PACKET_TYPE_PINGRESP;
incomingPacket.type = currentPacketType;
incomingPacket.remainingLength = MQTT_SAMPLE_REMAINING_LENGTH;
incomingPacket.headerLength = MQTT_SAMPLE_REMAINING_LENGTH;

MQTT_ProcessIncomingPacketTypeAndLength_ExpectAnyArgsAndReturn( MQTTNeedMoreBytes );
MQTT_ProcessIncomingPacketTypeAndLength_ReturnThruPtr_pIncomingPacket( &incomingPacket );

mqttStatus = MQTT_ProcessLoop( &context );
TEST_ASSERT_EQUAL( MQTTNeedMoreBytes, mqttStatus );
}

/**
Expand Down