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.

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

generate PINGREQ packets on idle input or output. #191

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
d06bb66
generate PINREQ packets on idle input or output.
adam-scislowicz May 25, 2022
b50758b
changes addressing Paul's feedback.
adam-scislowicz Jun 1, 2022
71233b0
changes to reflect feedback from Paul and Cobus.
adam-scislowicz Jun 1, 2022
b659a4c
further changes after discussions with Paul.
adam-scislowicz Jun 1, 2022
80b67c9
address issues raised by static analysis and formatting.
adam-scislowicz Jun 1, 2022
e28a87e
update documentation and unit tests.
adam-scislowicz Jun 1, 2022
d77cbae
use if else to clarify.
adam-scislowicz Jun 1, 2022
79b36f5
remove stale variable.
adam-scislowicz Jun 1, 2022
72acc76
fix logical error.
adam-scislowicz Jun 1, 2022
bed17ea
increment MQTT_TIMER_CALLS_PER_ITERATION by 1.
adam-scislowicz Jun 2, 2022
b02c62a
add lastpackettxtime to the lexicon.
adam-scislowicz Jun 2, 2022
b16906a
use a different uncrustify config and add rx to the lexicon.
adam-scislowicz Jun 2, 2022
38801dc
update unit test to acheive coverage and correct memory size expectat…
adam-scislowicz Jun 2, 2022
6a6865b
cover case where keep alive interval is greater than the tx timeout.
adam-scislowicz Jun 2, 2022
87523dc
use correct units.
adam-scislowicz Jun 2, 2022
6517e61
dont pre-set waitingForPingResp to true, we want handleKeepAlive to t…
adam-scislowicz Jun 2, 2022
28b06c0
expect success on the new subtest.
adam-scislowicz Jun 2, 2022
b248725
add additional test cases to cover two new branches.
adam-scislowicz Jun 7, 2022
70249a0
remove unused variable.
adam-scislowicz Jun 7, 2022
a295d19
MISRA compliance change.
adam-scislowicz Jun 7, 2022
c57d627
try to make both MISRA and uncrustify happy.
adam-scislowicz Jun 7, 2022
99ec326
try different version of uncrustify.
adam-scislowicz Jun 7, 2022
ff96ad4
Set PACKET_RX_TIMEOUT_MS to 30000U to match comment
paulbartell Jul 5, 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
8 changes: 4 additions & 4 deletions docs/doxygen/include/size_table.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
</tr>
<tr>
<td>core_mqtt.c</td>
<td><center>3.0K</center></td>
<td><center>2.6K</center></td>
<td><center>3.1K</center></td>
<td><center>2.7K</center></td>
</tr>
<tr>
<td>core_mqtt_state.c</td>
Expand All @@ -24,7 +24,7 @@
</tr>
<tr>
<td><b>Total estimates</b></td>
<td><b><center>6.9K</center></b></td>
<td><b><center>5.7K</center></b></td>
<td><b><center>7.0K</center></b></td>
<td><b><center>5.8K</center></b></td>
</tr>
</table>
2 changes: 1 addition & 1 deletion docs/doxygen/pages.dox
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ In this library, @ref mqtt_processloop_function handles sending of PINGREQs and
The standard does not specify the time duration within which the server has to respond to a ping request, noting only a "reasonable amount of time". If the response to a ping request is not received within @ref MQTT_PINGRESP_TIMEOUT_MS, this library assumes that the connection is dead.

If @ref mqtt_receiveloop_function is used instead of @ref mqtt_processloop_function, then no ping requests are sent. The application must ensure the connection does not remain idle for more than the keep-alive interval by calling @ref mqtt_ping_function to send ping requests.
The timestamp in @ref MQTTContext_t.lastPacketTime indicates when a packet was last sent by the library.
The timestamp in @ref MQTTContext_t.lastPacketTxTime indicates when a packet was last sent by the library.

Sending any ping request sets the @ref MQTTContext_t.waitingForPingResp flag. This flag is cleared by @ref mqtt_processloop_function when a ping response is received. If @ref mqtt_receiveloop_function is used instead, then this flag must be cleared manually by the application's callback.
*/
Expand Down
3 changes: 2 additions & 1 deletion lexicon.txt
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ iso
keepaliveintervalsec
keepalivems
keepaliveseconds
lastpackettime
lastpackettxtime
linux
logdebug
logerror
Expand Down Expand Up @@ -318,6 +318,7 @@ resending
reservestate
responsecode
rm
rx
sdk
searchstates
sendpacket
Expand Down
43 changes: 27 additions & 16 deletions source/core_mqtt.c
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ static int32_t sendPacket( MQTTContext_t * pContext,
/* Update time of last transmission if the entire packet is successfully sent. */
if( totalBytesSent > 0 )
{
pContext->lastPacketTime = lastSendTimeMs;
pContext->lastPacketTxTime = lastSendTimeMs;
LogDebug( ( "Successfully sent packet at time %lu.",
( unsigned long ) lastSendTimeMs ) );
}
Expand Down Expand Up @@ -1002,32 +1002,42 @@ static MQTTStatus_t sendPublishAcks( MQTTContext_t * pContext,
static MQTTStatus_t handleKeepAlive( MQTTContext_t * pContext )
{
MQTTStatus_t status = MQTTSuccess;
uint32_t now = 0U, keepAliveMs = 0U;
uint32_t now = 0U, packetTxTimeoutMs = 0U;

assert( pContext != NULL );
assert( pContext->getTime != NULL );

now = pContext->getTime();
keepAliveMs = 1000U * ( uint32_t ) pContext->keepAliveIntervalSec;

packetTxTimeoutMs = 1000U * ( uint32_t ) pContext->keepAliveIntervalSec;
paulbartell marked this conversation as resolved.
Show resolved Hide resolved

if( PACKET_TX_TIMEOUT_MS < packetTxTimeoutMs )
paulbartell marked this conversation as resolved.
Show resolved Hide resolved
{
packetTxTimeoutMs = PACKET_TX_TIMEOUT_MS;
}

paulbartell marked this conversation as resolved.
Show resolved Hide resolved
/* If keep alive interval is 0, it is disabled. */
if( keepAliveMs != 0U )
if( pContext->waitingForPingResp == true )
{
if( pContext->waitingForPingResp == true )
/* Has time expired? */
if( calculateElapsedTime( now, pContext->pingReqSendTimeMs ) >
MQTT_PINGRESP_TIMEOUT_MS )
{
/* Has time expired? */
if( calculateElapsedTime( now, pContext->pingReqSendTimeMs ) >
MQTT_PINGRESP_TIMEOUT_MS )
{
status = MQTTKeepAliveTimeout;
}
status = MQTTKeepAliveTimeout;
}
}
else
{
if( ( packetTxTimeoutMs != 0U ) && ( calculateElapsedTime( now, pContext->lastPacketTxTime ) >= packetTxTimeoutMs ) )
{
status = MQTT_Ping( pContext );
}
else if( ( PACKET_RX_TIMEOUT_MS != 0U ) && ( calculateElapsedTime( now, pContext->lastPacketRxTime ) >= PACKET_RX_TIMEOUT_MS ) )
{
status = MQTT_Ping( pContext );
}
adam-scislowicz marked this conversation as resolved.
Show resolved Hide resolved
else
{
if( calculateElapsedTime( now, pContext->lastPacketTime ) > keepAliveMs )
{
status = MQTT_Ping( pContext );
}
}
}

Expand Down Expand Up @@ -1340,6 +1350,7 @@ static MQTTStatus_t receiveSingleIteration( MQTTContext_t * pContext,
if( status == MQTTSuccess )
{
incomingPacket.pRemainingData = pContext->networkBuffer.pBuffer;
pContext->lastPacketRxTime = pContext->getTime();

/* PUBLISH packets allow flags in the lower four bits. For other
* packet types, they are reserved. */
Expand Down Expand Up @@ -2037,7 +2048,7 @@ MQTTStatus_t MQTT_Ping( MQTTContext_t * pContext )
}
else
{
pContext->pingReqSendTimeMs = pContext->lastPacketTime;
pContext->pingReqSendTimeMs = pContext->lastPacketTxTime;
pContext->waitingForPingResp = true;
LogDebug( ( "Sent %ld bytes of PINGREQ packet.",
( long int ) bytesSent ) );
Expand Down
11 changes: 8 additions & 3 deletions source/include/core_mqtt.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
* without a custom config. If a custom config is provided, the
* MQTT_DO_NOT_USE_CUSTOM_CONFIG macro should not be defined. */
#ifndef MQTT_DO_NOT_USE_CUSTOM_CONFIG
/* Include custom config file before other headers. */
/* Include custom config file before other headers. */
#include "core_mqtt_config.h"
#endif

Expand Down Expand Up @@ -210,7 +210,12 @@ typedef struct MQTTContext
/**
* @brief Timestamp of the last packet sent by the library.
*/
uint32_t lastPacketTime;
uint32_t lastPacketTxTime;

/**
* @brief Timestamp of the last packet received by the library.
*/
uint32_t lastPacketRxTime;

/**
* @brief Whether the library sent a packet during a call of #MQTT_ProcessLoop or
Expand Down Expand Up @@ -720,7 +725,7 @@ MQTTStatus_t MQTT_ProcessLoop( MQTTContext_t * pContext,
* {
* // Since this function does not send pings, the application may need
* // to in order to comply with keep alive.
* if( ( pContext->getTime() - pContext->lastPacketTime ) > keepAliveMs )
* if( ( pContext->getTime() - pContext->lastPacketTxTime ) > keepAliveMs )
* {
* status = MQTT_Ping( pContext );
* }
Expand Down
32 changes: 29 additions & 3 deletions source/include/core_mqtt_config_defaults.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
* <b>Default value:</b> `10`
*/
#ifndef MQTT_STATE_ARRAY_MAX_COUNT
/* Default value for the maximum acknowledgment pending PUBLISH messages. */
/* Default value for the maximum acknowledgment pending PUBLISH messages. */
#define MQTT_STATE_ARRAY_MAX_COUNT ( 10U )
#endif

Expand All @@ -95,7 +95,7 @@
* <b>Default value:</b> `5`
*/
#ifndef MQTT_MAX_CONNACK_RECEIVE_RETRY_COUNT
/* Default value for the CONNACK receive retries. */
/* Default value for the CONNACK receive retries. */
#define MQTT_MAX_CONNACK_RECEIVE_RETRY_COUNT ( 5U )
#endif

Expand All @@ -119,10 +119,36 @@
* <b>Default value:</b> `5000`
*/
#ifndef MQTT_PINGRESP_TIMEOUT_MS
/* Wait 5 seconds by default for a ping response. */
/* Wait 5 seconds by default for a ping response. */
#define MQTT_PINGRESP_TIMEOUT_MS ( 5000U )
#endif

/**
* @brief Maximum number of milliseconds of TX inactivity to wait
* before initiating a PINGREQ
*
* @note If this value is less than the keep alive interval than
* it will be used instead.
*
* <b>Possible values:</b> Any positive integer up to SIZE_MAX. <br>
* <b>Default value:</b> '30000'
*/
#ifndef PACKET_TX_TIMEOUT_MS
#define PACKET_TX_TIMEOUT_MS ( 30000U )
#endif

/**
* @brief Maximum number of milliseconds of RX inactivity to wait
* before initiating a PINGREQ
*
* <b>Possible values:</b> Any positive integer up to SIZE_MAX. <br>
* <b>Default value:</b> '30000'
*
*/
#ifndef PACKET_RX_TIMEOUT_MS
#define PACKET_RX_TIMEOUT_MS ( 30000U )
#endif

/**
* @brief The maximum duration between non-empty network reads while
* receiving an MQTT packet via the #MQTT_ProcessLoop or #MQTT_ReceiveLoop
Expand Down
60 changes: 46 additions & 14 deletions test/unit-test/core_mqtt_utest.c
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,11 @@
* that the API can support normal behavior even if the timer
* overflows.
*
* @note Currently, there are 5 calls within a single iteration.
* @note Currently, there are 6 calls within a single iteration.
* This can change when the implementation changes which would be
* caught through unit test failure.
*/
#define MQTT_TIMER_CALLS_PER_ITERATION ( 5 )
#define MQTT_TIMER_CALLS_PER_ITERATION ( 6 )

/**
* @brief Timeout for the timer overflow test.
Expand Down Expand Up @@ -465,6 +465,7 @@ static void expectProcessLoopCalls( MQTTContext_t * const pContext,
MQTTStatus_t processLoopStatus = pExpectParams->processLoopStatus;
bool incomingPublish = pExpectParams->incomingPublish;
MQTTPublishInfo_t * pPubInfo = pExpectParams->pPubInfo;
uint32_t packetTxTimeoutMs = 0U;

/* Modify incoming packet depending on type to be tested. */
incomingPacket.type = currentPacketType;
Expand All @@ -489,14 +490,25 @@ static void expectProcessLoopCalls( MQTTContext_t * const pContext,
/* When no data is available, the process loop tries to send a PINGREQ. */
if( modifyIncomingPacketStatus == MQTTNoDataAvailable )
{
if( ( pContext->waitingForPingResp == false ) &&
( pContext->keepAliveIntervalSec != 0U ) &&
( ( globalEntryTime - pContext->lastPacketTime ) > ( 1000U * pContext->keepAliveIntervalSec ) ) )
packetTxTimeoutMs = 1000U * ( uint32_t ) pContext->keepAliveIntervalSec;

if( PACKET_TX_TIMEOUT_MS < packetTxTimeoutMs )
{
MQTT_GetPingreqPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess );
/* Replace pointer parameter being passed to the method. */
MQTT_GetPingreqPacketSize_ReturnThruPtr_pPacketSize( &pingreqSize );
MQTT_SerializePingreq_ExpectAnyArgsAndReturn( serializeStatus );
packetTxTimeoutMs = PACKET_TX_TIMEOUT_MS;
}

if( pContext->waitingForPingResp == false )
{
if( ( ( packetTxTimeoutMs != 0U ) &&
( ( globalEntryTime - pContext->lastPacketTxTime ) >= packetTxTimeoutMs ) ) ||
( ( PACKET_RX_TIMEOUT_MS != 0U ) &&
( ( globalEntryTime - pContext->lastPacketRxTime ) >= PACKET_RX_TIMEOUT_MS ) ) )
{
MQTT_GetPingreqPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess );
/* Replace pointer parameter being passed to the method. */
MQTT_GetPingreqPacketSize_ReturnThruPtr_pPacketSize( &pingreqSize );
MQTT_SerializePingreq_ExpectAnyArgsAndReturn( serializeStatus );
}
}

expectMoreCalls = false;
Expand Down Expand Up @@ -1934,7 +1946,7 @@ void test_MQTT_ProcessLoop_handleKeepAlive_Happy_Paths( void )
mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer );
TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus );
context.keepAliveIntervalSec = MQTT_SAMPLE_KEEPALIVE_INTERVAL_S;
context.lastPacketTime = getTime();
context.lastPacketTxTime = getTime();
/* Set expected return values in the loop. All success. */
resetProcessLoopParams( &expectParams );
expectProcessLoopCalls( &context, &expectParams );
Expand All @@ -1944,7 +1956,7 @@ void test_MQTT_ProcessLoop_handleKeepAlive_Happy_Paths( void )
TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus );
context.waitingForPingResp = true;
context.keepAliveIntervalSec = MQTT_SAMPLE_KEEPALIVE_INTERVAL_S;
context.lastPacketTime = MQTT_ONE_SECOND_TO_MS;
context.lastPacketTxTime = MQTT_ONE_SECOND_TO_MS;
context.pingReqSendTimeMs = MQTT_ONE_SECOND_TO_MS;
/* Set expected return values in the loop. All success. */
resetProcessLoopParams( &expectParams );
Expand All @@ -1955,7 +1967,7 @@ void test_MQTT_ProcessLoop_handleKeepAlive_Happy_Paths( void )
TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus );
context.waitingForPingResp = false;
context.keepAliveIntervalSec = MQTT_SAMPLE_KEEPALIVE_INTERVAL_S;
context.lastPacketTime = 0;
context.lastPacketTxTime = 0;
/* Set expected return values in the loop. All success. */
resetProcessLoopParams( &expectParams );
expectProcessLoopCalls( &context, &expectParams );
Expand Down Expand Up @@ -1983,13 +1995,33 @@ void test_MQTT_ProcessLoop_handleKeepAlive_Error_Paths( void )
mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer );
TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus );
context.keepAliveIntervalSec = MQTT_SAMPLE_KEEPALIVE_INTERVAL_S;
context.lastPacketTime = 0;
context.lastPacketTxTime = 0;
context.lastPacketRxTime = 0;
context.pingReqSendTimeMs = 0;
context.waitingForPingResp = true;
/* Set expected return values in the loop. */
resetProcessLoopParams( &expectParams );
expectParams.processLoopStatus = MQTTKeepAliveTimeout;
expectProcessLoopCalls( &context, &expectParams );

context.keepAliveIntervalSec = PACKET_TX_TIMEOUT_MS + 1;
context.lastPacketTxTime = 0;
context.lastPacketRxTime = 0;
context.pingReqSendTimeMs = 0;
context.waitingForPingResp = false;
/* Set expected return values in the loop. */
resetProcessLoopParams( &expectParams );
expectProcessLoopCalls( &context, &expectParams );

globalEntryTime = PACKET_RX_TIMEOUT_MS + 1;
context.keepAliveIntervalSec = 0;
context.lastPacketTxTime = 0;
context.lastPacketRxTime = 0;
context.pingReqSendTimeMs = 0;
context.waitingForPingResp = false;
/* Set expected return values in the loop. */
resetProcessLoopParams( &expectParams );
expectProcessLoopCalls( &context, &expectParams );
}

/**
Expand Down Expand Up @@ -2391,7 +2423,7 @@ void test_MQTT_Ping_happy_path( void )
mqttStatus = MQTT_Ping( &context );
TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus );

TEST_ASSERT_EQUAL( context.lastPacketTime, context.pingReqSendTimeMs );
TEST_ASSERT_EQUAL( context.lastPacketTxTime, context.pingReqSendTimeMs );
TEST_ASSERT_TRUE( context.waitingForPingResp );
}

Expand Down