From f79d06b175b2296b30f2e5994c138dd005b3ce22 Mon Sep 17 00:00:00 2001 From: Dakshit Babbar Date: Fri, 6 Sep 2024 10:53:54 +0530 Subject: [PATCH 01/17] Update the APIs to check the connected flag and be thread safe --- source/core_mqtt.c | 303 +++++++++++++++++++++++--- source/include/core_mqtt.h | 18 ++ source/include/core_mqtt_serializer.h | 4 +- 3 files changed, 292 insertions(+), 33 deletions(-) diff --git a/source/core_mqtt.c b/source/core_mqtt.c index deba63fda..096f29f9e 100644 --- a/source/core_mqtt.c +++ b/source/core_mqtt.c @@ -1272,6 +1272,7 @@ static MQTTStatus_t sendPublishAcks( MQTTContext_t * pContext, uint8_t packetTypeByte = 0U; MQTTPubAckType_t packetType; MQTTFixedBuffer_t localBuffer; + bool stateUpdateHookExecuted = false; uint8_t pubAckPacket[ MQTT_PUBLISH_ACK_PACKET_SIZE ]; localBuffer.pBuffer = pubAckPacket; @@ -1289,9 +1290,21 @@ static MQTTStatus_t sendPublishAcks( MQTTContext_t * pContext, packetTypeByte, packetId ); + if( status == MQTTSuccess ) + { + MQTT_PRE_STATE_UPDATE_HOOK( pContext ); + + stateUpdateHookExecuted = true; + + if( pContext->connectStatus != MQTTConnected) + { + status = MQTTDisconnected; + } + } + if( status == MQTTSuccess ) { - MQTT_PRE_SEND_HOOK( pContext ); + // MQTT_PRE_SEND_HOOK( pContext ); /* Here, we are not using the vector approach for efficiency. There is just one buffer * to be sent which can be achieved with a normal send call. */ @@ -1299,10 +1312,22 @@ static MQTTStatus_t sendPublishAcks( MQTTContext_t * pContext, localBuffer.pBuffer, MQTT_PUBLISH_ACK_PACKET_SIZE ); - MQTT_POST_SEND_HOOK( pContext ); + if( sendResult != ( int32_t ) MQTT_PUBLISH_ACK_PACKET_SIZE ) + { + status = MQTTSendFailed; + } + + // MQTT_POST_SEND_HOOK( pContext ); } - if( sendResult == ( int32_t ) MQTT_PUBLISH_ACK_PACKET_SIZE ) + if( stateUpdateHookExecuted == true ) + { + /* Regardless of the status, if the mutex was taken, then it should be relinquished. */ + MQTT_POST_STATE_UPDATE_HOOK( pContext ); + } + + // if( sendResult == ( int32_t ) MQTT_PUBLISH_ACK_PACKET_SIZE ) + if( status == MQTTSuccess ) { pContext->controlPacketSent = true; @@ -1328,7 +1353,7 @@ static MQTTStatus_t sendPublishAcks( MQTTContext_t * pContext, "PacketSize=%lu.", ( unsigned int ) packetTypeByte, ( long int ) sendResult, MQTT_PUBLISH_ACK_PACKET_SIZE ) ); - status = MQTTSendFailed; + // status = MQTTSendFailed; } } @@ -2482,6 +2507,30 @@ static MQTTStatus_t handleSessionResumption( MQTTContext_t * pContext, return status; } +static MQTTStatus_t handleUncleanSessionResumption( MQTTContext_t * pContext) +{ + MQTTStatus_t status = MQTTSuccess; + MQTTStateCursor_t cursor = MQTT_STATE_CURSOR_INITIALIZER; + uint16_t packetId = MQTT_PACKET_ID_INVALID; + MQTTPublishState_t state = MQTTStateNull; + + assert( pContext != NULL ); + + /* Get the next packet ID for which a PUBREL need to be resent. */ + packetId = MQTT_PubrelToResend( pContext, &cursor, &state ); + + /* Resend all the PUBREL acks after session is reestablished. */ + while( ( packetId != MQTT_PACKET_ID_INVALID ) && + ( status == MQTTSuccess ) ) + { + status = sendPublishAcks( pContext, packetId, state ); + + packetId = MQTT_PubrelToResend( pContext, &cursor, &state ); + } + + return status; +} + static MQTTStatus_t validatePublishParams( const MQTTContext_t * pContext, const MQTTPublishInfo_t * pPublishInfo, uint16_t packetId ) @@ -2676,6 +2725,34 @@ MQTTStatus_t MQTT_CancelCallback( const MQTTContext_t * pContext, /*-----------------------------------------------------------*/ +MQTTStatus_t MQTT_CheckConnectStatus(MQTTContext_t * pContext) +{ + bool isConnected; + MQTTStatus_t status = MQTTSuccess; + + if( pContext == NULL ) + { + LogError( ( "Argument cannot be NULL: pContext=%p", + ( void * ) pContext ) ); + status = MQTTBadParameter; + } + + if( status == MQTTSuccess ) + { + MQTT_PRE_STATE_UPDATE_HOOK( pContext ); + + isConnected = (pContext->connectStatus == MQTTConnected); + + MQTT_POST_STATE_UPDATE_HOOK( pContext ); + + status = isConnected? MQTTAlreadyConnected : MQTTDisconnected; + } + + return status; +} + +/*-----------------------------------------------------------*/ + MQTTStatus_t MQTT_Connect( MQTTContext_t * pContext, const MQTTConnectInfo_t * pConnectInfo, const MQTTPublishInfo_t * pWillInfo, @@ -2685,6 +2762,7 @@ MQTTStatus_t MQTT_Connect( MQTTContext_t * pContext, size_t remainingLength = 0UL, packetSize = 0UL; MQTTStatus_t status = MQTTSuccess; MQTTPacketInfo_t incomingPacket = { 0 }; + bool stateUpdateHookExecuted = false; incomingPacket.type = ( uint8_t ) 0; @@ -2712,14 +2790,26 @@ MQTTStatus_t MQTT_Connect( MQTTContext_t * pContext, if( status == MQTTSuccess ) { - MQTT_PRE_SEND_HOOK( pContext ); + MQTT_PRE_STATE_UPDATE_HOOK( pContext ); + + stateUpdateHookExecuted = true; + + if( pContext->connectStatus == MQTTConnected) + { + status = MQTTAlreadyConnected; + } + } + + if( status == MQTTSuccess ) + { + // MQTT_PRE_SEND_HOOK( pContext ); status = sendConnectWithoutCopy( pContext, pConnectInfo, pWillInfo, remainingLength ); - MQTT_POST_SEND_HOOK( pContext ); + // MQTT_POST_SEND_HOOK( pContext ); } /* Read CONNACK from transport layer. */ @@ -2734,23 +2824,70 @@ MQTTStatus_t MQTT_Connect( MQTTContext_t * pContext, if( status == MQTTSuccess ) { - /* Resend PUBRELs when reestablishing a session, or clear records for new sessions. */ - status = handleSessionResumption( pContext, *pSessionPresent ); - } + /* Reset the index and clear the buffer when a new session is established. */ + pContext->index = 0; + ( void ) memset( pContext->networkBuffer.pBuffer, 0, pContext->networkBuffer.size ); + + if( *pSessionPresent != true ) + { + /* Clear any existing records if a new session is established. */ + if( pContext->outgoingPublishRecordMaxCount > 0U ) + { + ( void ) memset( pContext->outgoingPublishRecords, + 0x00, + pContext->outgoingPublishRecordMaxCount * sizeof( *pContext->outgoingPublishRecords ) ); + } + + if( pContext->incomingPublishRecordMaxCount > 0U ) + { + ( void ) memset( pContext->incomingPublishRecords, + 0x00, + pContext->incomingPublishRecordMaxCount * sizeof( *pContext->incomingPublishRecords ) ); + } + } - if( status == MQTTSuccess ) - { - LogInfo( ( "MQTT connection established with the broker." ) ); pContext->connectStatus = MQTTConnected; /* Initialize keep-alive fields after a successful connection. */ pContext->keepAliveIntervalSec = pConnectInfo->keepAliveSeconds; pContext->waitingForPingResp = false; pContext->pingReqSendTimeMs = 0U; } + + if( stateUpdateHookExecuted == true ) + { + /* Regardless of the status, if the mutex was taken, then it should be relinquished. */ + MQTT_POST_STATE_UPDATE_HOOK( pContext ); + } + + if( status == MQTTSuccess && *pSessionPresent == true ) + { + /* Resend PUBRELs when reestablishing a session */ + // status = handleSessionResumption( pContext, *pSessionPresent ); + status = handleUncleanSessionResumption( pContext); + } + + if( status == MQTTSuccess ) + { + LogInfo( ( "MQTT connection established with the broker." ) ); + } + else if( status == MQTTAlreadyConnected ) + { + LogInfo( ( "MQTT connection already established, return status = %s.", + MQTT_Status_strerror( status ) ) ); + } else { LogError( ( "MQTT connection failed with status = %s.", MQTT_Status_strerror( status ) ) ); + + MQTT_PRE_STATE_UPDATE_HOOK( pContext ); + + if( pContext->connectStatus == MQTTConnected) + { + pContext->connectStatus = MQTTNotConnected; + } + + MQTT_POST_STATE_UPDATE_HOOK( pContext ); } return status; @@ -2763,6 +2900,7 @@ MQTTStatus_t MQTT_Subscribe( MQTTContext_t * pContext, size_t subscriptionCount, uint16_t packetId ) { + bool stateUpdateHookExecuted = false; size_t remainingLength = 0UL, packetSize = 0UL; /* Validate arguments. */ @@ -2785,7 +2923,19 @@ MQTTStatus_t MQTT_Subscribe( MQTTContext_t * pContext, if( status == MQTTSuccess ) { - MQTT_PRE_SEND_HOOK( pContext ); + MQTT_PRE_STATE_UPDATE_HOOK( pContext ); + + stateUpdateHookExecuted = true; + + if( pContext->connectStatus != MQTTConnected) + { + status = MQTTDisconnected; + } + } + + if( status == MQTTSuccess ) + { + // MQTT_PRE_SEND_HOOK( pContext ); /* Send MQTT SUBSCRIBE packet. */ status = sendSubscribeWithoutCopy( pContext, @@ -2794,7 +2944,13 @@ MQTTStatus_t MQTT_Subscribe( MQTTContext_t * pContext, packetId, remainingLength ); - MQTT_POST_SEND_HOOK( pContext ); + // MQTT_POST_SEND_HOOK( pContext ); + } + + if( stateUpdateHookExecuted == true ) + { + /* Regardless of the status, if the mutex was taken then it should be relinquished. */ + MQTT_POST_STATE_UPDATE_HOOK( pContext ); } return status; @@ -2844,13 +3000,25 @@ MQTTStatus_t MQTT_Publish( MQTTContext_t * pContext, &headerSize ); } - if( ( status == MQTTSuccess ) && ( pPublishInfo->qos > MQTTQoS0 ) ) + if(status == MQTTSuccess ) { MQTT_PRE_STATE_UPDATE_HOOK( pContext ); - /* Set the flag so that the corresponding hook can be called later. */ stateUpdateHookExecuted = true; + if( pContext->connectStatus != MQTTConnected) + { + status = MQTTDisconnected; + } + } + + if( ( status == MQTTSuccess ) && ( pPublishInfo->qos > MQTTQoS0 ) ) + { + // MQTT_PRE_STATE_UPDATE_HOOK( pContext ); + + /* Set the flag so that the corresponding hook can be called later. */ + // stateUpdateHookExecuted = true; + status = MQTT_ReserveState( pContext, packetId, pPublishInfo->qos ); @@ -2868,7 +3036,7 @@ MQTTStatus_t MQTT_Publish( MQTTContext_t * pContext, { /* Take the mutex as multiple send calls are required for sending this * packet. */ - MQTT_PRE_SEND_HOOK( pContext ); + // MQTT_PRE_SEND_HOOK( pContext ); status = sendPublishWithoutCopy( pContext, pPublishInfo, @@ -2877,7 +3045,7 @@ MQTTStatus_t MQTT_Publish( MQTTContext_t * pContext, packetId ); /* Give the mutex away for the next taker. */ - MQTT_POST_SEND_HOOK( pContext ); + // MQTT_POST_SEND_HOOK( pContext ); } if( ( status == MQTTSuccess ) && @@ -2901,7 +3069,12 @@ MQTTStatus_t MQTT_Publish( MQTTContext_t * pContext, } } - if( stateUpdateHookExecuted == true ) + /* mutex should be released and not before updating the state + * because we need to make sure that the state is updated + * after sending the publish packet, before the receive + * loop receives ack for this and would want to update its state + */ + if( stateUpdateHookExecuted == true ) { /* Regardless of the status, if the mutex was taken due to the * packet being of QoS > QoS0, then it should be relinquished. */ @@ -2927,6 +3100,7 @@ MQTTStatus_t MQTT_Ping( MQTTContext_t * pContext ) /* MQTT ping packets are of fixed length. */ uint8_t pingreqPacket[ 2U ]; MQTTFixedBuffer_t localBuffer; + bool stateUpdateHookExecuted = false; localBuffer.pBuffer = pingreqPacket; localBuffer.size = sizeof( pingreqPacket ); @@ -2964,7 +3138,20 @@ MQTTStatus_t MQTT_Ping( MQTTContext_t * pContext ) { /* Take the mutex as the send call should not be interrupted in * between. */ - MQTT_PRE_SEND_HOOK( pContext ); + + MQTT_PRE_STATE_UPDATE_HOOK( pContext ); + + stateUpdateHookExecuted = true; + + if( pContext->connectStatus != MQTTConnected) + { + status = MQTTDisconnected; + } + } + + if( status == MQTTSuccess ) + { + // MQTT_PRE_SEND_HOOK( pContext ); /* Send the serialized PINGREQ packet to transport layer. * Here, we do not use the vectored IO approach for efficiency as the @@ -2975,7 +3162,7 @@ MQTTStatus_t MQTT_Ping( MQTTContext_t * pContext ) packetSize ); /* Give the mutex away. */ - MQTT_POST_SEND_HOOK( pContext ); + // MQTT_POST_SEND_HOOK( pContext ); /* It is an error to not send the entire PINGREQ packet. */ if( sendResult < ( int32_t ) packetSize ) @@ -2992,6 +3179,12 @@ MQTTStatus_t MQTT_Ping( MQTTContext_t * pContext ) } } + if( stateUpdateHookExecuted == true ) + { + /* Regardless of the status, if the mutex was taken, then it should be relinquished. */ + MQTT_POST_STATE_UPDATE_HOOK( pContext ); + } + return status; } @@ -3002,6 +3195,7 @@ MQTTStatus_t MQTT_Unsubscribe( MQTTContext_t * pContext, size_t subscriptionCount, uint16_t packetId ) { + bool stateUpdateHookExecuted = false; size_t remainingLength = 0UL, packetSize = 0UL; /* Validate arguments. */ @@ -3025,7 +3219,20 @@ MQTTStatus_t MQTT_Unsubscribe( MQTTContext_t * pContext, if( status == MQTTSuccess ) { /* Take the mutex because the below call should not be interrupted. */ - MQTT_PRE_SEND_HOOK( pContext ); + MQTT_PRE_STATE_UPDATE_HOOK( pContext ); + + stateUpdateHookExecuted = true; + + if( pContext->connectStatus != MQTTConnected) + { + status = MQTTDisconnected; + } + } + + if( status == MQTTSuccess ) + { + + // MQTT_PRE_SEND_HOOK( pContext ); status = sendUnsubscribeWithoutCopy( pContext, pSubscriptionList, @@ -3034,7 +3241,13 @@ MQTTStatus_t MQTT_Unsubscribe( MQTTContext_t * pContext, remainingLength ); /* Give the mutex away. */ - MQTT_POST_SEND_HOOK( pContext ); + // MQTT_POST_SEND_HOOK( pContext ); + } + + if( stateUpdateHookExecuted == true ) + { + /* Regardless of the status, if the mutex was taken, then it should be relinquished. */ + MQTT_POST_STATE_UPDATE_HOOK( pContext ); } return status; @@ -3049,6 +3262,7 @@ MQTTStatus_t MQTT_Disconnect( MQTTContext_t * pContext ) MQTTStatus_t status = MQTTSuccess; MQTTFixedBuffer_t localBuffer; uint8_t disconnectPacket[ 2U ]; + bool stateUpdateHookExecuted = false; localBuffer.pBuffer = disconnectPacket; localBuffer.size = 2U; @@ -3077,7 +3291,28 @@ MQTTStatus_t MQTT_Disconnect( MQTTContext_t * pContext ) if( status == MQTTSuccess ) { /* Take the mutex because the below call should not be interrupted. */ - MQTT_PRE_SEND_HOOK( pContext ); + MQTT_PRE_STATE_UPDATE_HOOK( pContext ); + + stateUpdateHookExecuted = true; + + if( pContext->connectStatus != MQTTConnected) + { + status = MQTTDisconnected; + } + } + + if( status == MQTTSuccess ) + { + LogInfo( ( "Disconnected from the broker." ) ); + pContext->connectStatus = MQTTNotConnected; + + /* Reset the index and clean the buffer on a successful disconnect. */ + pContext->index = 0; + ( void ) memset( pContext->networkBuffer.pBuffer, 0, pContext->networkBuffer.size ); + + LogError( ( "MQTT Connection Disconnected Successfuly" ) ); + + // MQTT_PRE_SEND_HOOK( pContext ); /* Here we do not use vectors as the disconnect packet has fixed fields * which do not reside in user provided buffers. Thus, it can be sent @@ -3087,7 +3322,7 @@ MQTTStatus_t MQTT_Disconnect( MQTTContext_t * pContext ) packetSize ); /* Give the mutex away. */ - MQTT_POST_SEND_HOOK( pContext ); + // MQTT_POST_SEND_HOOK( pContext ); if( sendResult < ( int32_t ) packetSize ) { @@ -3101,14 +3336,10 @@ MQTTStatus_t MQTT_Disconnect( MQTTContext_t * pContext ) } } - if( status == MQTTSuccess ) + if( stateUpdateHookExecuted == true ) { - LogInfo( ( "Disconnected from the broker." ) ); - pContext->connectStatus = MQTTNotConnected; - - /* Reset the index and clean the buffer on a successful disconnect. */ - pContext->index = 0; - ( void ) memset( pContext->networkBuffer.pBuffer, 0, pContext->networkBuffer.size ); + /* Regardless of the status, if the mutex was taken, then it should be relinquished. */ + MQTT_POST_STATE_UPDATE_HOOK( pContext ); } return status; @@ -3379,6 +3610,14 @@ const char * MQTT_Status_strerror( MQTTStatus_t status ) case MQTTNeedMoreBytes: str = "MQTTNeedMoreBytes"; break; + + case MQTTAlreadyConnected: + str = "MQTTAlreadyConnected"; + break; + + case MQTTDisconnected: + str = "MQTTDisconnected"; + break; default: str = "Invalid MQTT Status code"; diff --git a/source/include/core_mqtt.h b/source/include/core_mqtt.h index 8e0bad49e..9e5821b6c 100644 --- a/source/include/core_mqtt.h +++ b/source/include/core_mqtt.h @@ -414,6 +414,24 @@ MQTTStatus_t MQTT_InitStatefulQoS( MQTTContext_t * pContext, size_t incomingPublishCount ); /* @[declare_mqtt_initstatefulqos] */ +/** + * @brief Checks the MQTT connection status with the broker. + * + * @param[in] pContext Initialized MQTT context. + * + * @return #MQTTBadParameter if invalid parameters are passed; + * #MQTTAlreadyConnected if the MQTT connection is established with the broker. + * #MQTTDisconnected otherwise + * + * Example + * @code{c} + * + * @endcode + */ +/* @[declare_mqtt_checkconnectstatus] */ +MQTTStatus_t MQTT_CheckConnectStatus(MQTTContext_t * pContext); +/* @[declare_mqtt_checkconnectstatus] */ + /** * @brief Establish an MQTT session. * diff --git a/source/include/core_mqtt_serializer.h b/source/include/core_mqtt_serializer.h index 4837ee654..a0ef10da0 100644 --- a/source/include/core_mqtt_serializer.h +++ b/source/include/core_mqtt_serializer.h @@ -96,9 +96,11 @@ typedef enum MQTTStatus MQTTIllegalState, /**< An illegal state in the state record. */ MQTTStateCollision, /**< A collision with an existing state record entry. */ MQTTKeepAliveTimeout, /**< Timeout while waiting for PINGRESP. */ - MQTTNeedMoreBytes /**< MQTT_ProcessLoop/MQTT_ReceiveLoop has received + MQTTNeedMoreBytes, /**< MQTT_ProcessLoop/MQTT_ReceiveLoop has received incomplete data; it should be called again (probably after a delay). */ + MQTTAlreadyConnected, /**< MQTT Connection is established with the broker */ + MQTTDisconnected /**< MQTT connection is not established with the broker */ } MQTTStatus_t; /** From 1e2cd7ab526b1e27dad8ec47009724a7b7dc82a4 Mon Sep 17 00:00:00 2001 From: Dakshit Babbar Date: Tue, 10 Sep 2024 17:04:59 +0530 Subject: [PATCH 02/17] Add an intermediate connection state to handle disconnects due to network failure --- source/core_mqtt.c | 253 +++++++++++++------------- source/core_mqtt_serializer.c | 24 ++- source/include/core_mqtt.h | 10 +- source/include/core_mqtt_serializer.h | 7 +- 4 files changed, 156 insertions(+), 138 deletions(-) diff --git a/source/core_mqtt.c b/source/core_mqtt.c index 096f29f9e..b3342d21e 100644 --- a/source/core_mqtt.c +++ b/source/core_mqtt.c @@ -266,7 +266,7 @@ static MQTTPubAckType_t getAckFromPacketType( uint8_t packetType ); * * @return Number of bytes received, or negative number on network error. */ -static int32_t recvExact( const MQTTContext_t * pContext, +static int32_t recvExact( MQTTContext_t * pContext, size_t bytesToRecv ); /** @@ -278,7 +278,7 @@ static int32_t recvExact( const MQTTContext_t * pContext, * * @return #MQTTRecvFailed or #MQTTNoDataAvailable. */ -static MQTTStatus_t discardPacket( const MQTTContext_t * pContext, +static MQTTStatus_t discardPacket( MQTTContext_t * pContext, size_t remainingLength, uint32_t timeoutMs ); @@ -302,7 +302,7 @@ static MQTTStatus_t discardStoredPacket( MQTTContext_t * pContext, * * @return #MQTTSuccess or #MQTTRecvFailed. */ -static MQTTStatus_t receivePacket( const MQTTContext_t * pContext, +static MQTTStatus_t receivePacket( MQTTContext_t * pContext, MQTTPacketInfo_t incomingPacket, uint32_t remainingTimeMs ); @@ -424,25 +424,21 @@ static MQTTStatus_t validateSubscribeUnsubscribeParams( const MQTTContext_t * pC * ##MQTTRecvFailed if transport recv failed; * #MQTTSuccess otherwise. */ -static MQTTStatus_t receiveConnack( const MQTTContext_t * pContext, +static MQTTStatus_t receiveConnack( MQTTContext_t * pContext, uint32_t timeoutMs, bool cleanSession, MQTTPacketInfo_t * pIncomingPacket, bool * pSessionPresent ); /** - * @brief Resends pending acks for a re-established MQTT session, or - * clears existing state records for a clean session. + * @brief Resends pending acks for a re-established MQTT session * * @param[in] pContext Initialized MQTT context. - * @param[in] sessionPresent Session present flag received from the MQTT broker. * * @return #MQTTSendFailed if transport send during resend failed; * #MQTTSuccess otherwise. */ -static MQTTStatus_t handleSessionResumption( MQTTContext_t * pContext, - bool sessionPresent ); - +static MQTTStatus_t handleUncleanSessionResumption( MQTTContext_t * pContext) /** * @brief Send the publish packet without copying the topic string and payload in @@ -823,6 +819,11 @@ static int32_t sendMessageVector( MQTTContext_t * pContext, { bytesSentOrError = sendResult; LogError( ( "sendMessageVector: Unable to send packet: Network Error." ) ); + + if(pContext->connectStatus == MQTTConnected) + { + pContext->connectStatus = MQTTDisconnectPending; + } } else { @@ -902,6 +903,11 @@ static int32_t sendBuffer( MQTTContext_t * pContext, { bytesSentOrError = sendResult; LogError( ( "sendBuffer: Unable to send packet: Network Error." ) ); + + if(pContext->connectStatus == MQTTConnected) + { + pContext->connectStatus = MQTTDisconnectPending; + } } else { @@ -962,7 +968,7 @@ static MQTTPubAckType_t getAckFromPacketType( uint8_t packetType ) /*-----------------------------------------------------------*/ -static int32_t recvExact( const MQTTContext_t * pContext, +static int32_t recvExact( MQTTContext_t * pContext, size_t bytesToRecv ) { uint8_t * pIndex = NULL; @@ -998,6 +1004,15 @@ static int32_t recvExact( const MQTTContext_t * pContext, ( long int ) bytesRecvd ) ); totalBytesRecvd = bytesRecvd; receiveError = true; + + MQTT_PRE_STATE_UPDATE_HOOK( pContext ); + + if(pContext->connectStatus == MQTTConnected) + { + pContext->connectStatus = MQTTDisconnectPending; + } + + MQTT_POST_STATE_UPDATE_HOOK( pContext ); } else if( bytesRecvd > 0 ) { @@ -1039,7 +1054,7 @@ static int32_t recvExact( const MQTTContext_t * pContext, /*-----------------------------------------------------------*/ -static MQTTStatus_t discardPacket( const MQTTContext_t * pContext, +static MQTTStatus_t discardPacket( MQTTContext_t * pContext, size_t remainingLength, uint32_t timeoutMs ) { @@ -1175,7 +1190,7 @@ static MQTTStatus_t discardStoredPacket( MQTTContext_t * pContext, /*-----------------------------------------------------------*/ -static MQTTStatus_t receivePacket( const MQTTContext_t * pContext, +static MQTTStatus_t receivePacket( MQTTContext_t * pContext, MQTTPacketInfo_t incomingPacket, uint32_t remainingTimeMs ) { @@ -1272,6 +1287,7 @@ static MQTTStatus_t sendPublishAcks( MQTTContext_t * pContext, uint8_t packetTypeByte = 0U; MQTTPubAckType_t packetType; MQTTFixedBuffer_t localBuffer; + MQTTConnectionStatus_t connectStatus; bool stateUpdateHookExecuted = false; uint8_t pubAckPacket[ MQTT_PUBLISH_ACK_PACKET_SIZE ]; @@ -1296,28 +1312,26 @@ static MQTTStatus_t sendPublishAcks( MQTTContext_t * pContext, stateUpdateHookExecuted = true; - if( pContext->connectStatus != MQTTConnected) + connectStatus=pContext->connectStatus; + + if( connectStatus != MQTTConnected) { - status = MQTTDisconnected; + status = (connectStatus==MQTTNotConnected) ? MQTTStatusNotConnected: MQTTStatusDisconnectPending; } } if( status == MQTTSuccess ) { - // MQTT_PRE_SEND_HOOK( pContext ); - /* Here, we are not using the vector approach for efficiency. There is just one buffer * to be sent which can be achieved with a normal send call. */ sendResult = sendBuffer( pContext, localBuffer.pBuffer, MQTT_PUBLISH_ACK_PACKET_SIZE ); - if( sendResult != ( int32_t ) MQTT_PUBLISH_ACK_PACKET_SIZE ) + if( sendResult < ( int32_t ) MQTT_PUBLISH_ACK_PACKET_SIZE ) { status = MQTTSendFailed; } - - // MQTT_POST_SEND_HOOK( pContext ); } if( stateUpdateHookExecuted == true ) @@ -1326,7 +1340,6 @@ static MQTTStatus_t sendPublishAcks( MQTTContext_t * pContext, MQTT_POST_STATE_UPDATE_HOOK( pContext ); } - // if( sendResult == ( int32_t ) MQTT_PUBLISH_ACK_PACKET_SIZE ) if( status == MQTTSuccess ) { pContext->controlPacketSent = true; @@ -1353,7 +1366,6 @@ static MQTTStatus_t sendPublishAcks( MQTTContext_t * pContext, "PacketSize=%lu.", ( unsigned int ) packetTypeByte, ( long int ) sendResult, MQTT_PUBLISH_ACK_PACKET_SIZE ) ); - // status = MQTTSendFailed; } } @@ -1713,6 +1725,15 @@ static MQTTStatus_t receiveSingleIteration( MQTTContext_t * pContext, { /* The receive function has failed. Bubble up the error up to the user. */ status = MQTTRecvFailed; + + MQTT_PRE_STATE_UPDATE_HOOK( pContext ); + + if(pContext->connectStatus == MQTTConnected) + { + pContext->connectStatus = MQTTDisconnectPending; + } + + MQTT_POST_STATE_UPDATE_HOOK( pContext ); } else if( ( recvBytes == 0 ) && ( pContext->index == 0U ) ) { @@ -2334,7 +2355,7 @@ static MQTTStatus_t sendConnectWithoutCopy( MQTTContext_t * pContext, /*-----------------------------------------------------------*/ -static MQTTStatus_t receiveConnack( const MQTTContext_t * pContext, +static MQTTStatus_t receiveConnack( MQTTContext_t * pContext, uint32_t timeoutMs, bool cleanSession, MQTTPacketInfo_t * pIncomingPacket, @@ -2365,6 +2386,24 @@ static MQTTStatus_t receiveConnack( const MQTTContext_t * pContext, pContext->transportInterface.pNetworkContext, pIncomingPacket ); + if( status == MQTTStatusDisconnectPending ) + { + /* Convert this status to MQTTRecvFailed as MQTTStatusDisconnectPending is + * reserved for cases where we need to let the user know about the MQTT + * connection status. + */ + status = MQTTRecvFailed; + + MQTT_PRE_STATE_UPDATE_HOOK( pContext ); + + if(pContext->connectStatus == MQTTConnected) + { + pContext->connectStatus = MQTTDisconnectPending; + } + + MQTT_POST_STATE_UPDATE_HOOK( pContext ); + } + /* The loop times out based on 2 conditions. * 1. If timeoutMs is greater than 0: * Loop times out based on the timeout calculated by getTime() @@ -2458,55 +2497,6 @@ static MQTTStatus_t receiveConnack( const MQTTContext_t * pContext, /*-----------------------------------------------------------*/ -static MQTTStatus_t handleSessionResumption( MQTTContext_t * pContext, - bool sessionPresent ) -{ - MQTTStatus_t status = MQTTSuccess; - MQTTStateCursor_t cursor = MQTT_STATE_CURSOR_INITIALIZER; - uint16_t packetId = MQTT_PACKET_ID_INVALID; - MQTTPublishState_t state = MQTTStateNull; - - assert( pContext != NULL ); - - /* Reset the index and clear the buffer when a new session is established. */ - pContext->index = 0; - ( void ) memset( pContext->networkBuffer.pBuffer, 0, pContext->networkBuffer.size ); - - if( sessionPresent == true ) - { - /* Get the next packet ID for which a PUBREL need to be resent. */ - packetId = MQTT_PubrelToResend( pContext, &cursor, &state ); - - /* Resend all the PUBREL acks after session is reestablished. */ - while( ( packetId != MQTT_PACKET_ID_INVALID ) && - ( status == MQTTSuccess ) ) - { - status = sendPublishAcks( pContext, packetId, state ); - - packetId = MQTT_PubrelToResend( pContext, &cursor, &state ); - } - } - else - { - /* Clear any existing records if a new session is established. */ - if( pContext->outgoingPublishRecordMaxCount > 0U ) - { - ( void ) memset( pContext->outgoingPublishRecords, - 0x00, - pContext->outgoingPublishRecordMaxCount * sizeof( *pContext->outgoingPublishRecords ) ); - } - - if( pContext->incomingPublishRecordMaxCount > 0U ) - { - ( void ) memset( pContext->incomingPublishRecords, - 0x00, - pContext->incomingPublishRecordMaxCount * sizeof( *pContext->incomingPublishRecords ) ); - } - } - - return status; -} - static MQTTStatus_t handleUncleanSessionResumption( MQTTContext_t * pContext) { MQTTStatus_t status = MQTTSuccess; @@ -2727,7 +2717,7 @@ MQTTStatus_t MQTT_CancelCallback( const MQTTContext_t * pContext, MQTTStatus_t MQTT_CheckConnectStatus(MQTTContext_t * pContext) { - bool isConnected; + MQTTConnectionStatus_t connectStatus; MQTTStatus_t status = MQTTSuccess; if( pContext == NULL ) @@ -2741,11 +2731,27 @@ MQTTStatus_t MQTT_CheckConnectStatus(MQTTContext_t * pContext) { MQTT_PRE_STATE_UPDATE_HOOK( pContext ); - isConnected = (pContext->connectStatus == MQTTConnected); + connectStatus = pContext->connectStatus == MQTTConnected; - MQTT_POST_STATE_UPDATE_HOOK( pContext ); + switch (connectStatus) + { + case MQTTConnected: + status = MQTTStatusConnected; + break; + + case MQTTNotConnected: + status = MQTTStatusNotConnected; + break; - status = isConnected? MQTTAlreadyConnected : MQTTDisconnected; + case MQTTDisconnectPending: + status = MQTTStatusDisconnectPending; + break; + + default: + break; + } + + MQTT_POST_STATE_UPDATE_HOOK( pContext ); } return status; @@ -2762,6 +2768,7 @@ MQTTStatus_t MQTT_Connect( MQTTContext_t * pContext, size_t remainingLength = 0UL, packetSize = 0UL; MQTTStatus_t status = MQTTSuccess; MQTTPacketInfo_t incomingPacket = { 0 }; + MQTTConnectionStatus_t connectStatus; bool stateUpdateHookExecuted = false; incomingPacket.type = ( uint8_t ) 0; @@ -2794,22 +2801,20 @@ MQTTStatus_t MQTT_Connect( MQTTContext_t * pContext, stateUpdateHookExecuted = true; - if( pContext->connectStatus == MQTTConnected) + connectStatus=pContext->connectStatus; + + if( connectStatus != MQTTNotConnected) { - status = MQTTAlreadyConnected; + status = (connectStatus==MQTTConnected) ? MQTTStatusConnected: MQTTStatusDisconnectPending; } } if( status == MQTTSuccess ) { - // MQTT_PRE_SEND_HOOK( pContext ); - status = sendConnectWithoutCopy( pContext, pConnectInfo, pWillInfo, remainingLength ); - - // MQTT_POST_SEND_HOOK( pContext ); } /* Read CONNACK from transport layer. */ @@ -2862,7 +2867,6 @@ MQTTStatus_t MQTT_Connect( MQTTContext_t * pContext, if( status == MQTTSuccess && *pSessionPresent == true ) { /* Resend PUBRELs when reestablishing a session */ - // status = handleSessionResumption( pContext, *pSessionPresent ); status = handleUncleanSessionResumption( pContext); } @@ -2870,9 +2874,9 @@ MQTTStatus_t MQTT_Connect( MQTTContext_t * pContext, { LogInfo( ( "MQTT connection established with the broker." ) ); } - else if( status == MQTTAlreadyConnected ) + else if( status == MQTTStatusConnected || status == MQTTStatusDisconnectPending) { - LogInfo( ( "MQTT connection already established, return status = %s.", + LogInfo( ( "MQTT Connection is either already established or a disconnect is pending, return status = %s.", MQTT_Status_strerror( status ) ) ); } else @@ -2901,6 +2905,7 @@ MQTTStatus_t MQTT_Subscribe( MQTTContext_t * pContext, uint16_t packetId ) { bool stateUpdateHookExecuted = false; + MQTTConnectionStatus_t connectStatus; size_t remainingLength = 0UL, packetSize = 0UL; /* Validate arguments. */ @@ -2927,24 +2932,22 @@ MQTTStatus_t MQTT_Subscribe( MQTTContext_t * pContext, stateUpdateHookExecuted = true; - if( pContext->connectStatus != MQTTConnected) + connectStatus=pContext->connectStatus; + + if( connectStatus != MQTTConnected) { - status = MQTTDisconnected; + status = (connectStatus==MQTTNotConnected) ? MQTTStatusNotConnected: MQTTStatusDisconnectPending; } } if( status == MQTTSuccess ) { - // MQTT_PRE_SEND_HOOK( pContext ); - /* Send MQTT SUBSCRIBE packet. */ status = sendSubscribeWithoutCopy( pContext, pSubscriptionList, subscriptionCount, packetId, remainingLength ); - - // MQTT_POST_SEND_HOOK( pContext ); } if( stateUpdateHookExecuted == true ) @@ -2966,6 +2969,7 @@ MQTTStatus_t MQTT_Publish( MQTTContext_t * pContext, size_t remainingLength = 0UL; size_t packetSize = 0UL; MQTTPublishState_t publishStatus = MQTTStateNull; + MQTTConnectionStatus_t connectStatus; bool stateUpdateHookExecuted = false; /* Maximum number of bytes required by the 'fixed' part of the PUBLISH @@ -3002,22 +3006,23 @@ MQTTStatus_t MQTT_Publish( MQTTContext_t * pContext, if(status == MQTTSuccess ) { + /* Take the mutex as multiple send calls are required for sending this + * packet. */ MQTT_PRE_STATE_UPDATE_HOOK( pContext ); stateUpdateHookExecuted = true; - if( pContext->connectStatus != MQTTConnected) + connectStatus=pContext->connectStatus; + + if( connectStatus != MQTTConnected) { - status = MQTTDisconnected; + status = (connectStatus==MQTTNotConnected) ? MQTTStatusNotConnected: MQTTStatusDisconnectPending; } } if( ( status == MQTTSuccess ) && ( pPublishInfo->qos > MQTTQoS0 ) ) { - // MQTT_PRE_STATE_UPDATE_HOOK( pContext ); - /* Set the flag so that the corresponding hook can be called later. */ - // stateUpdateHookExecuted = true; status = MQTT_ReserveState( pContext, packetId, @@ -3034,18 +3039,11 @@ MQTTStatus_t MQTT_Publish( MQTTContext_t * pContext, if( status == MQTTSuccess ) { - /* Take the mutex as multiple send calls are required for sending this - * packet. */ - // MQTT_PRE_SEND_HOOK( pContext ); - status = sendPublishWithoutCopy( pContext, pPublishInfo, mqttHeader, headerSize, packetId ); - - /* Give the mutex away for the next taker. */ - // MQTT_POST_SEND_HOOK( pContext ); } if( ( status == MQTTSuccess ) && @@ -3100,6 +3098,7 @@ MQTTStatus_t MQTT_Ping( MQTTContext_t * pContext ) /* MQTT ping packets are of fixed length. */ uint8_t pingreqPacket[ 2U ]; MQTTFixedBuffer_t localBuffer; + MQTTConnectionStatus_t connectStatus; bool stateUpdateHookExecuted = false; localBuffer.pBuffer = pingreqPacket; @@ -3143,16 +3142,16 @@ MQTTStatus_t MQTT_Ping( MQTTContext_t * pContext ) stateUpdateHookExecuted = true; - if( pContext->connectStatus != MQTTConnected) + connectStatus=pContext->connectStatus; + + if( connectStatus != MQTTConnected) { - status = MQTTDisconnected; + status = (connectStatus==MQTTNotConnected) ? MQTTStatusNotConnected: MQTTStatusDisconnectPending; } } if( status == MQTTSuccess ) { - // MQTT_PRE_SEND_HOOK( pContext ); - /* Send the serialized PINGREQ packet to transport layer. * Here, we do not use the vectored IO approach for efficiency as the * Ping packet does not have numerous fields which need to be copied @@ -3161,9 +3160,6 @@ MQTTStatus_t MQTT_Ping( MQTTContext_t * pContext ) localBuffer.pBuffer, packetSize ); - /* Give the mutex away. */ - // MQTT_POST_SEND_HOOK( pContext ); - /* It is an error to not send the entire PINGREQ packet. */ if( sendResult < ( int32_t ) packetSize ) { @@ -3196,6 +3192,7 @@ MQTTStatus_t MQTT_Unsubscribe( MQTTContext_t * pContext, uint16_t packetId ) { bool stateUpdateHookExecuted = false; + MQTTConnectionStatus_t connectStatus; size_t remainingLength = 0UL, packetSize = 0UL; /* Validate arguments. */ @@ -3223,25 +3220,21 @@ MQTTStatus_t MQTT_Unsubscribe( MQTTContext_t * pContext, stateUpdateHookExecuted = true; - if( pContext->connectStatus != MQTTConnected) + connectStatus=pContext->connectStatus; + + if( connectStatus != MQTTConnected) { - status = MQTTDisconnected; + status = (connectStatus==MQTTNotConnected) ? MQTTStatusNotConnected: MQTTStatusDisconnectPending; } } if( status == MQTTSuccess ) { - - // MQTT_PRE_SEND_HOOK( pContext ); - status = sendUnsubscribeWithoutCopy( pContext, pSubscriptionList, subscriptionCount, packetId, remainingLength ); - - /* Give the mutex away. */ - // MQTT_POST_SEND_HOOK( pContext ); } if( stateUpdateHookExecuted == true ) @@ -3262,6 +3255,7 @@ MQTTStatus_t MQTT_Disconnect( MQTTContext_t * pContext ) MQTTStatus_t status = MQTTSuccess; MQTTFixedBuffer_t localBuffer; uint8_t disconnectPacket[ 2U ]; + MQTTConnectionStatus_t connectStatus; bool stateUpdateHookExecuted = false; localBuffer.pBuffer = disconnectPacket; @@ -3295,9 +3289,11 @@ MQTTStatus_t MQTT_Disconnect( MQTTContext_t * pContext ) stateUpdateHookExecuted = true; - if( pContext->connectStatus != MQTTConnected) + connectStatus=pContext->connectStatus; + + if( connectStatus == MQTTNotConnected) { - status = MQTTDisconnected; + status = MQTTStatusNotConnected; } } @@ -3312,8 +3308,6 @@ MQTTStatus_t MQTT_Disconnect( MQTTContext_t * pContext ) LogError( ( "MQTT Connection Disconnected Successfuly" ) ); - // MQTT_PRE_SEND_HOOK( pContext ); - /* Here we do not use vectors as the disconnect packet has fixed fields * which do not reside in user provided buffers. Thus, it can be sent * using a simple send call. */ @@ -3321,9 +3315,6 @@ MQTTStatus_t MQTT_Disconnect( MQTTContext_t * pContext ) localBuffer.pBuffer, packetSize ); - /* Give the mutex away. */ - // MQTT_POST_SEND_HOOK( pContext ); - if( sendResult < ( int32_t ) packetSize ) { LogError( ( "Transport send failed for DISCONNECT packet." ) ); @@ -3611,12 +3602,16 @@ const char * MQTT_Status_strerror( MQTTStatus_t status ) str = "MQTTNeedMoreBytes"; break; - case MQTTAlreadyConnected: - str = "MQTTAlreadyConnected"; + case MQTTStatusConnected: + str = "MQTTStatusConnected"; + break; + + case MQTTStatusNotConnected: + str = "MQTTStatusNotConnected"; break; - case MQTTDisconnected: - str = "MQTTDisconnected"; + case MQTTStatusDisconnectPending: + str = "MQTTStatusDisconnectPending"; break; default: diff --git a/source/core_mqtt_serializer.c b/source/core_mqtt_serializer.c index 97022034c..ab6ac3d03 100644 --- a/source/core_mqtt_serializer.c +++ b/source/core_mqtt_serializer.c @@ -820,20 +820,24 @@ static size_t getRemainingLength( TransportRecv_t recvFunc, multiplier *= 128U; bytesDecoded++; } + else if( bytesReceived < 0) + { + remainingLength = -1; + } else { remainingLength = MQTT_REMAINING_LENGTH_INVALID; } } - if( remainingLength == MQTT_REMAINING_LENGTH_INVALID ) + if( remainingLength == MQTT_REMAINING_LENGTH_INVALID || remainingLength < 0) { break; } } while( ( encodedByte & 0x80U ) != 0U ); /* Check that the decoded remaining length conforms to the MQTT specification. */ - if( remainingLength != MQTT_REMAINING_LENGTH_INVALID ) + if( (remainingLength != MQTT_REMAINING_LENGTH_INVALID) && (remainingLength >=0) ) { expectedSize = remainingLengthEncodedSize( remainingLength ); @@ -2590,6 +2594,16 @@ MQTTStatus_t MQTT_GetIncomingPacketTypeAndLength( TransportRecv_t readFunc, { LogError( ( "Incoming packet remaining length invalid." ) ); status = MQTTBadResponse; + } + else if( pIncomingPacket->remainingLength < 0) + { + /* MQTT Connection status cannot be updated here hence bubble up + * MQTTStatusDisconnectPending status to the calling API that can update it. */ + status = MQTTStatusDisconnectPending; + } + else + { + /* Empty else MISRA 15.7 */ } } else @@ -2603,6 +2617,12 @@ MQTTStatus_t MQTT_GetIncomingPacketTypeAndLength( TransportRecv_t readFunc, { status = MQTTNoDataAvailable; } + else if( ( status != MQTTBadParameter ) && ( bytesReceived < 0 ) ) + { + /* MQTT Connection status cannot be updated here hence bubble up + * MQTTStatusDisconnectPending status to the calling API that can update it. */ + status = MQTTStatusDisconnectPending; + } /* If the input packet was valid, then any other number of bytes received is * a failure. */ diff --git a/source/include/core_mqtt.h b/source/include/core_mqtt.h index 9e5821b6c..4f8e72236 100644 --- a/source/include/core_mqtt.h +++ b/source/include/core_mqtt.h @@ -107,8 +107,9 @@ typedef void (* MQTTEventCallback_t )( struct MQTTContext * pContext, */ typedef enum MQTTConnectionStatus { - MQTTNotConnected, /**< @brief MQTT Connection is inactive. */ - MQTTConnected /**< @brief MQTT Connection is active. */ + MQTTNotConnected, /**< @brief MQTT Connection is inactive. */ + MQTTConnected, /**< @brief MQTT Connection is active. */ + MQTTDisconnectPending /**< @brief MQTT Connection needs to be disconnected as a transport error has occured. */ } MQTTConnectionStatus_t; /** @@ -420,8 +421,9 @@ MQTTStatus_t MQTT_InitStatefulQoS( MQTTContext_t * pContext, * @param[in] pContext Initialized MQTT context. * * @return #MQTTBadParameter if invalid parameters are passed; - * #MQTTAlreadyConnected if the MQTT connection is established with the broker. - * #MQTTDisconnected otherwise + * #MQTTStatusConnected if the MQTT connection is established with the broker. + * #MQTTSatusNotConnected if the MQTT connection is broker. + * #MQTTSatusDisconnectPending if Transport Interface has failed and MQTT connection needs to be closed. * * Example * @code{c} diff --git a/source/include/core_mqtt_serializer.h b/source/include/core_mqtt_serializer.h index a0ef10da0..86b3c6396 100644 --- a/source/include/core_mqtt_serializer.h +++ b/source/include/core_mqtt_serializer.h @@ -96,11 +96,12 @@ typedef enum MQTTStatus MQTTIllegalState, /**< An illegal state in the state record. */ MQTTStateCollision, /**< A collision with an existing state record entry. */ MQTTKeepAliveTimeout, /**< Timeout while waiting for PINGRESP. */ - MQTTNeedMoreBytes, /**< MQTT_ProcessLoop/MQTT_ReceiveLoop has received + MQTTNeedMoreBytes, /**< MQTT_ProcessLoop/MQTT_ReceiveLoop has received incomplete data; it should be called again (probably after a delay). */ - MQTTAlreadyConnected, /**< MQTT Connection is established with the broker */ - MQTTDisconnected /**< MQTT connection is not established with the broker */ + MQTTStatusConnected, /**< MQTT connection is established with the broker */ + MQTTStatusNotConnected, /**< MQTT connection is not established with the broker */ + MQTTStatusDisconnectPending /**< Transport Interface has failed and MQTT connection needs to be closed */ } MQTTStatus_t; /** From 19fd9673b46c8ae9973cb033c832316dfa24761c Mon Sep 17 00:00:00 2001 From: Dakshit Babbar Date: Wed, 11 Sep 2024 09:54:33 +0530 Subject: [PATCH 03/17] Rectify Unit Tests Errors --- source/core_mqtt.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/source/core_mqtt.c b/source/core_mqtt.c index b3342d21e..efada68dc 100644 --- a/source/core_mqtt.c +++ b/source/core_mqtt.c @@ -438,7 +438,7 @@ static MQTTStatus_t receiveConnack( MQTTContext_t * pContext, * @return #MQTTSendFailed if transport send during resend failed; * #MQTTSuccess otherwise. */ -static MQTTStatus_t handleUncleanSessionResumption( MQTTContext_t * pContext) +static MQTTStatus_t handleUncleanSessionResumption( MQTTContext_t * pContext); /** * @brief Send the publish packet without copying the topic string and payload in @@ -2879,6 +2879,11 @@ MQTTStatus_t MQTT_Connect( MQTTContext_t * pContext, LogInfo( ( "MQTT Connection is either already established or a disconnect is pending, return status = %s.", MQTT_Status_strerror( status ) ) ); } + else if ( status == MQTTBadParameter ) + { + LogError( ( "MQTT connection failed with status = %s.", + MQTT_Status_strerror( status ) ) ); + } else { LogError( ( "MQTT connection failed with status = %s.", From 5ead3f8d144be39231123c35af9e67f5bad4a947 Mon Sep 17 00:00:00 2001 From: Dakshit Babbar Date: Wed, 11 Sep 2024 10:21:25 +0530 Subject: [PATCH 04/17] Add a new macro to detect transport error in core_mqtt_serializer.h --- source/core_mqtt_serializer.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/source/core_mqtt_serializer.c b/source/core_mqtt_serializer.c index ab6ac3d03..1cf056574 100644 --- a/source/core_mqtt_serializer.c +++ b/source/core_mqtt_serializer.c @@ -135,6 +135,13 @@ */ #define MQTT_REMAINING_LENGTH_INVALID ( ( size_t ) 268435456 ) +/** + * @brief A value that represents transport layer error while retreiving the remaining length. + * + * This value is greater than what is allowed by the MQTT specification. + */ +#define MQTT_REMAINING_LENGTH_TRANSPORT_ERROR ( ( size_t ) 268435457 ) + /** * @brief The minimum remaining length for a QoS 0 PUBLISH. * @@ -822,7 +829,7 @@ static size_t getRemainingLength( TransportRecv_t recvFunc, } else if( bytesReceived < 0) { - remainingLength = -1; + remainingLength = MQTT_REMAINING_LENGTH_TRANSPORT_ERROR; } else { @@ -830,14 +837,14 @@ static size_t getRemainingLength( TransportRecv_t recvFunc, } } - if( remainingLength == MQTT_REMAINING_LENGTH_INVALID || remainingLength < 0) + if( remainingLength == MQTT_REMAINING_LENGTH_INVALID || remainingLength == MQTT_REMAINING_LENGTH_TRANSPORT_ERROR) { break; } } while( ( encodedByte & 0x80U ) != 0U ); /* Check that the decoded remaining length conforms to the MQTT specification. */ - if( (remainingLength != MQTT_REMAINING_LENGTH_INVALID) && (remainingLength >=0) ) + if( (remainingLength != MQTT_REMAINING_LENGTH_INVALID) && (remainingLength != MQTT_REMAINING_LENGTH_TRANSPORT_ERROR) ) { expectedSize = remainingLengthEncodedSize( remainingLength ); @@ -2595,7 +2602,7 @@ MQTTStatus_t MQTT_GetIncomingPacketTypeAndLength( TransportRecv_t readFunc, LogError( ( "Incoming packet remaining length invalid." ) ); status = MQTTBadResponse; } - else if( pIncomingPacket->remainingLength < 0) + else if( pIncomingPacket->remainingLength == MQTT_REMAINING_LENGTH_TRANSPORT_ERROR) { /* MQTT Connection status cannot be updated here hence bubble up * MQTTStatusDisconnectPending status to the calling API that can update it. */ From 431b34f57ae0f075adf687d089fc6f6ee022de20 Mon Sep 17 00:00:00 2001 From: Dakshit Babbar Date: Wed, 11 Sep 2024 10:56:17 +0530 Subject: [PATCH 05/17] Resolve Spellcheck and Doxygen Tests --- source/core_mqtt.c | 2 +- source/include/core_mqtt.h | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/source/core_mqtt.c b/source/core_mqtt.c index efada68dc..b4f09b40e 100644 --- a/source/core_mqtt.c +++ b/source/core_mqtt.c @@ -3311,7 +3311,7 @@ MQTTStatus_t MQTT_Disconnect( MQTTContext_t * pContext ) pContext->index = 0; ( void ) memset( pContext->networkBuffer.pBuffer, 0, pContext->networkBuffer.size ); - LogError( ( "MQTT Connection Disconnected Successfuly" ) ); + LogError( ( "MQTT Connection Disconnected Successfully" ) ); /* Here we do not use vectors as the disconnect packet has fixed fields * which do not reside in user provided buffers. Thus, it can be sent diff --git a/source/include/core_mqtt.h b/source/include/core_mqtt.h index 4f8e72236..e641df497 100644 --- a/source/include/core_mqtt.h +++ b/source/include/core_mqtt.h @@ -109,7 +109,7 @@ typedef enum MQTTConnectionStatus { MQTTNotConnected, /**< @brief MQTT Connection is inactive. */ MQTTConnected, /**< @brief MQTT Connection is active. */ - MQTTDisconnectPending /**< @brief MQTT Connection needs to be disconnected as a transport error has occured. */ + MQTTDisconnectPending /**< @brief MQTT Connection needs to be disconnected as a transport error has occurred. */ } MQTTConnectionStatus_t; /** @@ -422,8 +422,8 @@ MQTTStatus_t MQTT_InitStatefulQoS( MQTTContext_t * pContext, * * @return #MQTTBadParameter if invalid parameters are passed; * #MQTTStatusConnected if the MQTT connection is established with the broker. - * #MQTTSatusNotConnected if the MQTT connection is broker. - * #MQTTSatusDisconnectPending if Transport Interface has failed and MQTT connection needs to be closed. + * #MQTTStatusNotConnected if the MQTT connection is broker. + * #MQTTStatusDisconnectPending if Transport Interface has failed and MQTT connection needs to be closed. * * Example * @code{c} From f67688e41f35e2cf5411f9ba84e3dc65dae38b2f Mon Sep 17 00:00:00 2001 From: Dakshit Babbar Date: Wed, 11 Sep 2024 11:18:24 +0530 Subject: [PATCH 06/17] Resolve Formatting Test --- source/core_mqtt.c | 122 +++++++++++++++++----------------- source/core_mqtt_serializer.c | 16 ++--- source/include/core_mqtt.h | 8 +-- 3 files changed, 73 insertions(+), 73 deletions(-) diff --git a/source/core_mqtt.c b/source/core_mqtt.c index b4f09b40e..520dd4686 100644 --- a/source/core_mqtt.c +++ b/source/core_mqtt.c @@ -438,7 +438,7 @@ static MQTTStatus_t receiveConnack( MQTTContext_t * pContext, * @return #MQTTSendFailed if transport send during resend failed; * #MQTTSuccess otherwise. */ -static MQTTStatus_t handleUncleanSessionResumption( MQTTContext_t * pContext); +static MQTTStatus_t handleUncleanSessionResumption( MQTTContext_t * pContext ); /** * @brief Send the publish packet without copying the topic string and payload in @@ -820,7 +820,7 @@ static int32_t sendMessageVector( MQTTContext_t * pContext, bytesSentOrError = sendResult; LogError( ( "sendMessageVector: Unable to send packet: Network Error." ) ); - if(pContext->connectStatus == MQTTConnected) + if( pContext->connectStatus == MQTTConnected ) { pContext->connectStatus = MQTTDisconnectPending; } @@ -904,7 +904,7 @@ static int32_t sendBuffer( MQTTContext_t * pContext, bytesSentOrError = sendResult; LogError( ( "sendBuffer: Unable to send packet: Network Error." ) ); - if(pContext->connectStatus == MQTTConnected) + if( pContext->connectStatus == MQTTConnected ) { pContext->connectStatus = MQTTDisconnectPending; } @@ -1007,11 +1007,11 @@ static int32_t recvExact( MQTTContext_t * pContext, MQTT_PRE_STATE_UPDATE_HOOK( pContext ); - if(pContext->connectStatus == MQTTConnected) + if( pContext->connectStatus == MQTTConnected ) { pContext->connectStatus = MQTTDisconnectPending; } - + MQTT_POST_STATE_UPDATE_HOOK( pContext ); } else if( bytesRecvd > 0 ) @@ -1307,16 +1307,16 @@ static MQTTStatus_t sendPublishAcks( MQTTContext_t * pContext, packetId ); if( status == MQTTSuccess ) - { + { MQTT_PRE_STATE_UPDATE_HOOK( pContext ); stateUpdateHookExecuted = true; - connectStatus=pContext->connectStatus; + connectStatus = pContext->connectStatus; - if( connectStatus != MQTTConnected) + if( connectStatus != MQTTConnected ) { - status = (connectStatus==MQTTNotConnected) ? MQTTStatusNotConnected: MQTTStatusDisconnectPending; + status = ( connectStatus == MQTTNotConnected ) ? MQTTStatusNotConnected : MQTTStatusDisconnectPending; } } @@ -1728,7 +1728,7 @@ static MQTTStatus_t receiveSingleIteration( MQTTContext_t * pContext, MQTT_PRE_STATE_UPDATE_HOOK( pContext ); - if(pContext->connectStatus == MQTTConnected) + if( pContext->connectStatus == MQTTConnected ) { pContext->connectStatus = MQTTDisconnectPending; } @@ -2388,19 +2388,19 @@ static MQTTStatus_t receiveConnack( MQTTContext_t * pContext, if( status == MQTTStatusDisconnectPending ) { - /* Convert this status to MQTTRecvFailed as MQTTStatusDisconnectPending is - * reserved for cases where we need to let the user know about the MQTT + /* Convert this status to MQTTRecvFailed as MQTTStatusDisconnectPending is + * reserved for cases where we need to let the user know about the MQTT * connection status. */ status = MQTTRecvFailed; MQTT_PRE_STATE_UPDATE_HOOK( pContext ); - if(pContext->connectStatus == MQTTConnected) + if( pContext->connectStatus == MQTTConnected ) { pContext->connectStatus = MQTTDisconnectPending; } - + MQTT_POST_STATE_UPDATE_HOOK( pContext ); } @@ -2497,7 +2497,7 @@ static MQTTStatus_t receiveConnack( MQTTContext_t * pContext, /*-----------------------------------------------------------*/ -static MQTTStatus_t handleUncleanSessionResumption( MQTTContext_t * pContext) +static MQTTStatus_t handleUncleanSessionResumption( MQTTContext_t * pContext ) { MQTTStatus_t status = MQTTSuccess; MQTTStateCursor_t cursor = MQTT_STATE_CURSOR_INITIALIZER; @@ -2511,7 +2511,7 @@ static MQTTStatus_t handleUncleanSessionResumption( MQTTContext_t * pContext) /* Resend all the PUBREL acks after session is reestablished. */ while( ( packetId != MQTT_PACKET_ID_INVALID ) && - ( status == MQTTSuccess ) ) + ( status == MQTTSuccess ) ) { status = sendPublishAcks( pContext, packetId, state ); @@ -2715,7 +2715,7 @@ MQTTStatus_t MQTT_CancelCallback( const MQTTContext_t * pContext, /*-----------------------------------------------------------*/ -MQTTStatus_t MQTT_CheckConnectStatus(MQTTContext_t * pContext) +MQTTStatus_t MQTT_CheckConnectStatus( MQTTContext_t * pContext ) { MQTTConnectionStatus_t connectStatus; MQTTStatus_t status = MQTTSuccess; @@ -2733,22 +2733,22 @@ MQTTStatus_t MQTT_CheckConnectStatus(MQTTContext_t * pContext) connectStatus = pContext->connectStatus == MQTTConnected; - switch (connectStatus) + switch( connectStatus ) { - case MQTTConnected: - status = MQTTStatusConnected; - break; + case MQTTConnected: + status = MQTTStatusConnected; + break; - case MQTTNotConnected: - status = MQTTStatusNotConnected; - break; + case MQTTNotConnected: + status = MQTTStatusNotConnected; + break; - case MQTTDisconnectPending: - status = MQTTStatusDisconnectPending; - break; - - default: - break; + case MQTTDisconnectPending: + status = MQTTStatusDisconnectPending; + break; + + default: + break; } MQTT_POST_STATE_UPDATE_HOOK( pContext ); @@ -2798,14 +2798,14 @@ MQTTStatus_t MQTT_Connect( MQTTContext_t * pContext, if( status == MQTTSuccess ) { MQTT_PRE_STATE_UPDATE_HOOK( pContext ); - + stateUpdateHookExecuted = true; - connectStatus=pContext->connectStatus; + connectStatus = pContext->connectStatus; - if( connectStatus != MQTTNotConnected) + if( connectStatus != MQTTNotConnected ) { - status = (connectStatus==MQTTConnected) ? MQTTStatusConnected: MQTTStatusDisconnectPending; + status = ( connectStatus == MQTTConnected ) ? MQTTStatusConnected : MQTTStatusDisconnectPending; } } @@ -2839,15 +2839,15 @@ MQTTStatus_t MQTT_Connect( MQTTContext_t * pContext, if( pContext->outgoingPublishRecordMaxCount > 0U ) { ( void ) memset( pContext->outgoingPublishRecords, - 0x00, - pContext->outgoingPublishRecordMaxCount * sizeof( *pContext->outgoingPublishRecords ) ); + 0x00, + pContext->outgoingPublishRecordMaxCount * sizeof( *pContext->outgoingPublishRecords ) ); } if( pContext->incomingPublishRecordMaxCount > 0U ) { ( void ) memset( pContext->incomingPublishRecords, - 0x00, - pContext->incomingPublishRecordMaxCount * sizeof( *pContext->incomingPublishRecords ) ); + 0x00, + pContext->incomingPublishRecordMaxCount * sizeof( *pContext->incomingPublishRecords ) ); } } @@ -2864,22 +2864,22 @@ MQTTStatus_t MQTT_Connect( MQTTContext_t * pContext, MQTT_POST_STATE_UPDATE_HOOK( pContext ); } - if( status == MQTTSuccess && *pSessionPresent == true ) + if( ( status == MQTTSuccess ) && ( *pSessionPresent == true ) ) { /* Resend PUBRELs when reestablishing a session */ - status = handleUncleanSessionResumption( pContext); + status = handleUncleanSessionResumption( pContext ); } if( status == MQTTSuccess ) { LogInfo( ( "MQTT connection established with the broker." ) ); } - else if( status == MQTTStatusConnected || status == MQTTStatusDisconnectPending) + else if( ( status == MQTTStatusConnected ) || ( status == MQTTStatusDisconnectPending ) ) { LogInfo( ( "MQTT Connection is either already established or a disconnect is pending, return status = %s.", - MQTT_Status_strerror( status ) ) ); + MQTT_Status_strerror( status ) ) ); } - else if ( status == MQTTBadParameter ) + else if( status == MQTTBadParameter ) { LogError( ( "MQTT connection failed with status = %s.", MQTT_Status_strerror( status ) ) ); @@ -2891,7 +2891,7 @@ MQTTStatus_t MQTT_Connect( MQTTContext_t * pContext, MQTT_PRE_STATE_UPDATE_HOOK( pContext ); - if( pContext->connectStatus == MQTTConnected) + if( pContext->connectStatus == MQTTConnected ) { pContext->connectStatus = MQTTNotConnected; } @@ -2937,11 +2937,11 @@ MQTTStatus_t MQTT_Subscribe( MQTTContext_t * pContext, stateUpdateHookExecuted = true; - connectStatus=pContext->connectStatus; + connectStatus = pContext->connectStatus; - if( connectStatus != MQTTConnected) + if( connectStatus != MQTTConnected ) { - status = (connectStatus==MQTTNotConnected) ? MQTTStatusNotConnected: MQTTStatusDisconnectPending; + status = ( connectStatus == MQTTNotConnected ) ? MQTTStatusNotConnected : MQTTStatusDisconnectPending; } } @@ -3009,7 +3009,7 @@ MQTTStatus_t MQTT_Publish( MQTTContext_t * pContext, &headerSize ); } - if(status == MQTTSuccess ) + if( status == MQTTSuccess ) { /* Take the mutex as multiple send calls are required for sending this * packet. */ @@ -3017,11 +3017,11 @@ MQTTStatus_t MQTT_Publish( MQTTContext_t * pContext, stateUpdateHookExecuted = true; - connectStatus=pContext->connectStatus; + connectStatus = pContext->connectStatus; - if( connectStatus != MQTTConnected) + if( connectStatus != MQTTConnected ) { - status = (connectStatus==MQTTNotConnected) ? MQTTStatusNotConnected: MQTTStatusDisconnectPending; + status = ( connectStatus == MQTTNotConnected ) ? MQTTStatusNotConnected : MQTTStatusDisconnectPending; } } @@ -3077,7 +3077,7 @@ MQTTStatus_t MQTT_Publish( MQTTContext_t * pContext, * after sending the publish packet, before the receive * loop receives ack for this and would want to update its state */ - if( stateUpdateHookExecuted == true ) + if( stateUpdateHookExecuted == true ) { /* Regardless of the status, if the mutex was taken due to the * packet being of QoS > QoS0, then it should be relinquished. */ @@ -3147,11 +3147,11 @@ MQTTStatus_t MQTT_Ping( MQTTContext_t * pContext ) stateUpdateHookExecuted = true; - connectStatus=pContext->connectStatus; + connectStatus = pContext->connectStatus; - if( connectStatus != MQTTConnected) + if( connectStatus != MQTTConnected ) { - status = (connectStatus==MQTTNotConnected) ? MQTTStatusNotConnected: MQTTStatusDisconnectPending; + status = ( connectStatus == MQTTNotConnected ) ? MQTTStatusNotConnected : MQTTStatusDisconnectPending; } } @@ -3225,11 +3225,11 @@ MQTTStatus_t MQTT_Unsubscribe( MQTTContext_t * pContext, stateUpdateHookExecuted = true; - connectStatus=pContext->connectStatus; + connectStatus = pContext->connectStatus; - if( connectStatus != MQTTConnected) + if( connectStatus != MQTTConnected ) { - status = (connectStatus==MQTTNotConnected) ? MQTTStatusNotConnected: MQTTStatusDisconnectPending; + status = ( connectStatus == MQTTNotConnected ) ? MQTTStatusNotConnected : MQTTStatusDisconnectPending; } } @@ -3294,9 +3294,9 @@ MQTTStatus_t MQTT_Disconnect( MQTTContext_t * pContext ) stateUpdateHookExecuted = true; - connectStatus=pContext->connectStatus; + connectStatus = pContext->connectStatus; - if( connectStatus == MQTTNotConnected) + if( connectStatus == MQTTNotConnected ) { status = MQTTStatusNotConnected; } @@ -3606,7 +3606,7 @@ const char * MQTT_Status_strerror( MQTTStatus_t status ) case MQTTNeedMoreBytes: str = "MQTTNeedMoreBytes"; break; - + case MQTTStatusConnected: str = "MQTTStatusConnected"; break; diff --git a/source/core_mqtt_serializer.c b/source/core_mqtt_serializer.c index 1cf056574..2cc7623a6 100644 --- a/source/core_mqtt_serializer.c +++ b/source/core_mqtt_serializer.c @@ -140,7 +140,7 @@ * * This value is greater than what is allowed by the MQTT specification. */ -#define MQTT_REMAINING_LENGTH_TRANSPORT_ERROR ( ( size_t ) 268435457 ) +#define MQTT_REMAINING_LENGTH_TRANSPORT_ERROR ( ( size_t ) 268435457 ) /** * @brief The minimum remaining length for a QoS 0 PUBLISH. @@ -827,7 +827,7 @@ static size_t getRemainingLength( TransportRecv_t recvFunc, multiplier *= 128U; bytesDecoded++; } - else if( bytesReceived < 0) + else if( bytesReceived < 0 ) { remainingLength = MQTT_REMAINING_LENGTH_TRANSPORT_ERROR; } @@ -837,14 +837,14 @@ static size_t getRemainingLength( TransportRecv_t recvFunc, } } - if( remainingLength == MQTT_REMAINING_LENGTH_INVALID || remainingLength == MQTT_REMAINING_LENGTH_TRANSPORT_ERROR) + if( ( remainingLength == MQTT_REMAINING_LENGTH_INVALID ) || ( remainingLength == MQTT_REMAINING_LENGTH_TRANSPORT_ERROR ) ) { break; } } while( ( encodedByte & 0x80U ) != 0U ); /* Check that the decoded remaining length conforms to the MQTT specification. */ - if( (remainingLength != MQTT_REMAINING_LENGTH_INVALID) && (remainingLength != MQTT_REMAINING_LENGTH_TRANSPORT_ERROR) ) + if( ( remainingLength != MQTT_REMAINING_LENGTH_INVALID ) && ( remainingLength != MQTT_REMAINING_LENGTH_TRANSPORT_ERROR ) ) { expectedSize = remainingLengthEncodedSize( remainingLength ); @@ -2601,14 +2601,14 @@ MQTTStatus_t MQTT_GetIncomingPacketTypeAndLength( TransportRecv_t readFunc, { LogError( ( "Incoming packet remaining length invalid." ) ); status = MQTTBadResponse; - } - else if( pIncomingPacket->remainingLength == MQTT_REMAINING_LENGTH_TRANSPORT_ERROR) + } + else if( pIncomingPacket->remainingLength == MQTT_REMAINING_LENGTH_TRANSPORT_ERROR ) { /* MQTT Connection status cannot be updated here hence bubble up * MQTTStatusDisconnectPending status to the calling API that can update it. */ status = MQTTStatusDisconnectPending; } - else + else { /* Empty else MISRA 15.7 */ } @@ -2627,7 +2627,7 @@ MQTTStatus_t MQTT_GetIncomingPacketTypeAndLength( TransportRecv_t readFunc, else if( ( status != MQTTBadParameter ) && ( bytesReceived < 0 ) ) { /* MQTT Connection status cannot be updated here hence bubble up - * MQTTStatusDisconnectPending status to the calling API that can update it. */ + * MQTTStatusDisconnectPending status to the calling API that can update it. */ status = MQTTStatusDisconnectPending; } diff --git a/source/include/core_mqtt.h b/source/include/core_mqtt.h index e641df497..ba5219348 100644 --- a/source/include/core_mqtt.h +++ b/source/include/core_mqtt.h @@ -107,9 +107,9 @@ typedef void (* MQTTEventCallback_t )( struct MQTTContext * pContext, */ typedef enum MQTTConnectionStatus { - MQTTNotConnected, /**< @brief MQTT Connection is inactive. */ - MQTTConnected, /**< @brief MQTT Connection is active. */ - MQTTDisconnectPending /**< @brief MQTT Connection needs to be disconnected as a transport error has occurred. */ + MQTTNotConnected, /**< @brief MQTT Connection is inactive. */ + MQTTConnected, /**< @brief MQTT Connection is active. */ + MQTTDisconnectPending /**< @brief MQTT Connection needs to be disconnected as a transport error has occurred. */ } MQTTConnectionStatus_t; /** @@ -431,7 +431,7 @@ MQTTStatus_t MQTT_InitStatefulQoS( MQTTContext_t * pContext, * @endcode */ /* @[declare_mqtt_checkconnectstatus] */ -MQTTStatus_t MQTT_CheckConnectStatus(MQTTContext_t * pContext); +MQTTStatus_t MQTT_CheckConnectStatus( MQTTContext_t * pContext ); /* @[declare_mqtt_checkconnectstatus] */ /** From 15c6cbb36f21f828ca68ababce5ee180fb3b760c Mon Sep 17 00:00:00 2001 From: parallels Date: Wed, 11 Sep 2024 15:46:12 +0530 Subject: [PATCH 07/17] Fix Unit Tests --- source/core_mqtt.c | 30 +++--- test/unit-test/core_mqtt_serializer_utest.c | 4 +- test/unit-test/core_mqtt_utest.c | 111 +++++++++++++++++++- 3 files changed, 124 insertions(+), 21 deletions(-) diff --git a/source/core_mqtt.c b/source/core_mqtt.c index 520dd4686..6cae1380d 100644 --- a/source/core_mqtt.c +++ b/source/core_mqtt.c @@ -2833,22 +2833,22 @@ MQTTStatus_t MQTT_Connect( MQTTContext_t * pContext, pContext->index = 0; ( void ) memset( pContext->networkBuffer.pBuffer, 0, pContext->networkBuffer.size ); - if( *pSessionPresent != true ) + if( *pSessionPresent != true && pContext->outgoingPublishRecordMaxCount > 0U ) { /* Clear any existing records if a new session is established. */ - if( pContext->outgoingPublishRecordMaxCount > 0U ) - { - ( void ) memset( pContext->outgoingPublishRecords, - 0x00, - pContext->outgoingPublishRecordMaxCount * sizeof( *pContext->outgoingPublishRecords ) ); - } - - if( pContext->incomingPublishRecordMaxCount > 0U ) - { - ( void ) memset( pContext->incomingPublishRecords, - 0x00, - pContext->incomingPublishRecordMaxCount * sizeof( *pContext->incomingPublishRecords ) ); - } + ( void ) memset( pContext->outgoingPublishRecords, + 0x00, + pContext->outgoingPublishRecordMaxCount * sizeof( *pContext->outgoingPublishRecords ) ); + } + else if( *pSessionPresent != true && pContext->incomingPublishRecordMaxCount > 0U ) + { + ( void ) memset( pContext->incomingPublishRecords, + 0x00, + pContext->incomingPublishRecordMaxCount * sizeof( *pContext->incomingPublishRecords ) ); + } + else + { + /* MISRA Empty body */ } pContext->connectStatus = MQTTConnected; @@ -2879,7 +2879,7 @@ MQTTStatus_t MQTT_Connect( MQTTContext_t * pContext, LogInfo( ( "MQTT Connection is either already established or a disconnect is pending, return status = %s.", MQTT_Status_strerror( status ) ) ); } - else if( status == MQTTBadParameter ) + else if( pContext == NULL ) { LogError( ( "MQTT connection failed with status = %s.", MQTT_Status_strerror( status ) ) ); diff --git a/test/unit-test/core_mqtt_serializer_utest.c b/test/unit-test/core_mqtt_serializer_utest.c index e91aae72a..b521eafbc 100644 --- a/test/unit-test/core_mqtt_serializer_utest.c +++ b/test/unit-test/core_mqtt_serializer_utest.c @@ -1904,7 +1904,7 @@ void test_MQTT_GetIncomingPacketTypeAndLength( void ) memset( buffer, 0x00, 10 ); bufPtr = buffer; status = MQTT_GetIncomingPacketTypeAndLength( mockReceiveFailure, &networkContext, &mqttPacket ); - TEST_ASSERT_EQUAL( MQTTRecvFailed, status ); + TEST_ASSERT_EQUAL( MQTTStatusDisconnectPending, status ); /* Test if no data is available. */ bufPtr = buffer; @@ -1921,7 +1921,7 @@ void test_MQTT_GetIncomingPacketTypeAndLength( void ) bufPtr = buffer; buffer[ 0 ] = MQTT_PACKET_TYPE_PUBREL; status = MQTT_GetIncomingPacketTypeAndLength( mockReceiveSucceedThenFail, &networkContext, &mqttPacket ); - TEST_ASSERT_EQUAL( MQTTBadResponse, status ); + TEST_ASSERT_EQUAL( MQTTStatusDisconnectPending, status ); } /* ========================================================================== */ diff --git a/test/unit-test/core_mqtt_utest.c b/test/unit-test/core_mqtt_utest.c index df88964af..66db8aa92 100644 --- a/test/unit-test/core_mqtt_utest.c +++ b/test/unit-test/core_mqtt_utest.c @@ -1891,7 +1891,7 @@ void test_MQTT_Connect_resendPendingAcks( void ) MQTT_SerializeAck_ExpectAnyArgsAndReturn( MQTTBadParameter ); MQTT_PubrelToResend_ExpectAnyArgsAndReturn( MQTT_PACKET_TYPE_INVALID ); status = MQTT_Connect( &mqttContext, &connectInfo, NULL, timeout, &sessionPresentResult ); - TEST_ASSERT_EQUAL_INT( MQTTSendFailed, status ); + TEST_ASSERT_EQUAL_INT( MQTTBadParameter, status ); TEST_ASSERT_EQUAL_INT( MQTTNotConnected, mqttContext.connectStatus ); TEST_ASSERT_TRUE( sessionPresentResult ); @@ -1935,7 +1935,7 @@ void test_MQTT_Connect_resendPendingAcks( void ) /* Query for any remaining packets pending to ack. */ MQTT_PubrelToResend_ExpectAnyArgsAndReturn( packetIdentifier + 2 ); status = MQTT_Connect( &mqttContext, &connectInfo, NULL, timeout, &sessionPresent ); - TEST_ASSERT_EQUAL_INT( MQTTSendFailed, status ); + TEST_ASSERT_EQUAL_INT( MQTTBadParameter, status ); TEST_ASSERT_EQUAL_INT( MQTTNotConnected, mqttContext.connectStatus ); /* Test 5. Two packets found in ack pending state. Sent PUBREL successfully @@ -2430,6 +2430,7 @@ void test_MQTT_Publish_DuplicatePublish( void ) mqttContext.outgoingPublishRecordMaxCount = 10; mqttContext.outgoingPublishRecords = outgoingPublishRecord; + mqttContext.connectStatus=MQTTConnected; publishInfo.qos = MQTTQoS1; publishInfo.dup = true; @@ -2471,6 +2472,7 @@ void test_MQTT_Publish_DuplicatePublish_UpdateFailed( void ) mqttContext.outgoingPublishRecordMaxCount = 10; mqttContext.outgoingPublishRecords = outgoingPublishRecord; + mqttContext.connectStatus=MQTTConnected; publishInfo.qos = MQTTQoS1; publishInfo.dup = true; @@ -2514,6 +2516,7 @@ void test_MQTT_Publish_WriteVSendsPartialBytes( void ) mqttContext.outgoingPublishRecordMaxCount = 10; mqttContext.outgoingPublishRecords = outgoingPublishRecord; + mqttContext.connectStatus=MQTTConnected; publishInfo.qos = MQTTQoS1; publishInfo.dup = false; @@ -2548,6 +2551,7 @@ void test_MQTT_Publish7( void ) MQTT_GetPublishPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_SerializePublishHeaderWithoutTopic_ExpectAnyArgsAndReturn( MQTTSuccess ); + mqttContext.connectStatus=MQTTConnected; /* We need sendPacket to be called with at least 1 byte to send, so that * it can return failure. This argument is the output of serializing the @@ -2577,6 +2581,8 @@ void test_MQTT_Publish8( void ) memset( &publishInfo, 0x0, sizeof( publishInfo ) ); MQTT_Init( &mqttContext, &transport, getTime, eventCallback, &networkBuffer ); + mqttContext.connectStatus=MQTTConnected; + /* We want to test the first call to sendPacket within sendPublish succeeding, * and the second one failing. */ mqttContext.transportInterface.send = transportSendSucceedThenFail; @@ -2607,6 +2613,8 @@ void test_MQTT_Publish9( void ) memset( &publishInfo, 0x0, sizeof( publishInfo ) ); MQTT_Init( &mqttContext, &transport, getTime, eventCallback, &networkBuffer ); + mqttContext.connectStatus=MQTTConnected; + MQTT_GetPublishPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_SerializePublishHeaderWithoutTopic_ExpectAnyArgsAndReturn( MQTTSuccess ); mqttContext.transportInterface.send = transportSendSuccess; @@ -2633,6 +2641,8 @@ void test_MQTT_Publish10( void ) memset( &publishInfo, 0x0, sizeof( publishInfo ) ); MQTT_Init( &mqttContext, &transport, getTime, eventCallback, &networkBuffer ); + mqttContext.connectStatus=MQTTConnected; + /* Test that sending a publish without a payload succeeds. */ publishInfo.pPayload = NULL; publishInfo.payloadLength = 0; @@ -2662,6 +2672,8 @@ void test_MQTT_Publish11( void ) memset( &publishInfo, 0x0, sizeof( publishInfo ) ); MQTT_Init( &mqttContext, &transport, getTime, eventCallback, &networkBuffer ); + mqttContext.connectStatus=MQTTConnected; + /* Restore the test payload and length. */ publishInfo.pPayload = "Test"; publishInfo.payloadLength = 4; @@ -2711,6 +2723,8 @@ void test_MQTT_Publish12( void ) mqttContext.outgoingPublishRecords->qos = MQTTQoS2; + mqttContext.connectStatus=MQTTConnected; + expectedState = MQTTPublishSend; MQTT_GetPublishPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); @@ -2747,6 +2761,8 @@ void test_MQTT_Publish13( void ) memset( &publishInfo, 0x0, sizeof( publishInfo ) ); MQTT_Init( &mqttContext, &transport, getTime, eventCallback, &networkBuffer ); + mqttContext.connectStatus=MQTTConnected; + MQTT_InitStatefulQoS( &mqttContext, &outgoingRecords, 4, &incomingRecords, 4 ); @@ -2782,6 +2798,8 @@ void test_MQTT_Publish14( void ) memset( &publishInfo, 0x0, sizeof( publishInfo ) ); MQTT_Init( &mqttContext, &transport, getTime, eventCallback, &networkBuffer ); + mqttContext.connectStatus=MQTTConnected; + /* Duplicate publish. dup flag is marked by application. */ publishInfo.dup = true; @@ -2812,6 +2830,8 @@ void test_MQTT_Publish15( void ) memset( &publishInfo, 0x0, sizeof( publishInfo ) ); MQTT_Init( &mqttContext, &transport, getTime, eventCallback, &networkBuffer ); + mqttContext.connectStatus=MQTTConnected; + /* Duplicate publish. dup flag is marked by application. * State record is not present. */ publishInfo.dup = true; @@ -2847,6 +2867,8 @@ void test_MQTT_Publish_Send_Timeout( void ) /* Initialize the MQTT context. */ MQTT_Init( &mqttContext, &transport, getTime, eventCallback, &networkBuffer ); + mqttContext.connectStatus=MQTTConnected; + /* Setup for making sure that the test results in calling sendPacket function * where calls to transport send function are made (repeatedly to send packet * over the network).*/ @@ -3223,6 +3245,8 @@ void test_MQTT_ProcessLoop_HandleKeepAlive1( void ) mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); + context.connectStatus=MQTTConnected; + /* Verify MQTTSuccess is returned. */ MQTT_GetPingreqPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetPingreqPacketSize_ReturnThruPtr_pPacketSize( &pingreqSize ); @@ -3363,6 +3387,8 @@ void test_MQTT_ProcessLoop_handleIncomingPublish_Happy_Path1( void ) mqttStatus = MQTT_InitStatefulQoS( &context, NULL, 0, pIncomingCallback, 10 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + context.connectStatus=MQTTConnected; + modifyIncomingPacketStatus = MQTTSuccess; /* Assume QoS = 1 so that a PUBACK will be sent after receiving PUBLISH. @@ -3400,6 +3426,8 @@ void test_MQTT_ProcessLoop_handleIncomingPublish_Happy_Path2( void ) mqttStatus = MQTT_InitStatefulQoS( &context, NULL, 0, pIncomingPublish, 10 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + context.connectStatus=MQTTConnected; + modifyIncomingPacketStatus = MQTTSuccess; /* Assume QoS = 2 so that a PUBREC will be sent after receiving PUBLISH. @@ -3444,6 +3472,8 @@ void test_MQTT_ProcessLoop_handleIncomingPublish_Happy_Path3( void ) &outgoingRecords, 4, &incomingRecords, 4 ); + context.connectStatus=MQTTConnected; + modifyIncomingPacketStatus = MQTTSuccess; /* Duplicate QoS1 publish received. @@ -3490,6 +3520,8 @@ void test_MQTT_ProcessLoop_handleIncomingPublish_Happy_Path4( void ) mqttStatus = MQTT_InitStatefulQoS( &context, NULL, 0, pIncomingPublish, 10 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + context.connectStatus=MQTTConnected; + modifyIncomingPacketStatus = MQTTSuccess; /* Duplicate QoS2 publish received. @@ -3531,6 +3563,8 @@ void test_MQTT_ProcessLoop_handleIncomingPublish_Happy_Path5( void ) mqttStatus = MQTT_InitStatefulQoS( &context, NULL, 0, pIncomingPublish, 10 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + context.connectStatus=MQTTConnected; + modifyIncomingPacketStatus = MQTTSuccess; /* A publish is received when already a state record exists, but dup @@ -3569,6 +3603,8 @@ void test_MQTT_ProcessLoop_handleIncomingPublish_Happy_Path6( void ) mqttStatus = MQTT_InitStatefulQoS( &context, NULL, 0, pIncomingPublish, 10 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + context.connectStatus=MQTTConnected; + modifyIncomingPacketStatus = MQTTSuccess; /* Duplicate QoS2 publish received with no collision. @@ -3712,6 +3748,8 @@ void test_MQTT_ProcessLoop_handleIncomingAck_Happy_Paths( void ) mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + context.connectStatus=MQTTConnected; + modifyIncomingPacketStatus = MQTTSuccess; /* Mock the receiving of a PUBACK packet type and expect the appropriate @@ -3810,6 +3848,8 @@ void test_MQTT_ProcessLoop_handleIncomingAck_Error_Paths( void ) mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + context.connectStatus=MQTTConnected; + modifyIncomingPacketStatus = MQTTSuccess; /* Verify that MQTTBadResponse is propagated when deserialization fails upon @@ -3829,7 +3869,7 @@ void test_MQTT_ProcessLoop_handleIncomingAck_Error_Paths( void ) expectParams.stateAfterDeserialize = MQTTPubRelSend; expectParams.serializeStatus = MQTTNoMemory; expectParams.stateAfterSerialize = MQTTStateNull; - expectParams.processLoopStatus = MQTTSendFailed; + expectParams.processLoopStatus = MQTTNoMemory; expectProcessLoopCalls( &context, &expectParams ); /* Verify that MQTTBadResponse is propagated when deserialization fails upon @@ -3899,6 +3939,8 @@ void test_MQTT_ProcessLoop_handleKeepAlive_Happy_Paths1( void ) mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + context.connectStatus=MQTTConnected; + context.waitingForPingResp = false; context.keepAliveIntervalSec = 0; expectParams.incomingPublish = false; @@ -4184,6 +4226,8 @@ void test_MQTT_ProcessLoop_handleKeepAlive_Error_Paths3( void ) mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + context.connectStatus=MQTTConnected; + globalEntryTime = PACKET_RX_TIMEOUT_MS + 1; context.keepAliveIntervalSec = 0; context.lastPacketTxTime = 0; @@ -4225,6 +4269,8 @@ void test_MQTT_ProcessLoop_handleKeepAlive_Error_Paths4( void ) mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + context.connectStatus=MQTTConnected; + globalEntryTime = PACKET_RX_TIMEOUT_MS + 1; context.keepAliveIntervalSec = ( PACKET_TX_TIMEOUT_MS / 1000 ) + 1U; context.lastPacketTxTime = 0; @@ -4370,6 +4416,8 @@ void test_MQTT_ProcessLoop_Timer_Overflow( void ) mqttStatus = MQTT_InitStatefulQoS( &context, NULL, 0, incomingPublishRecords, 10 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + context.connectStatus=MQTTConnected; + MQTT_ProcessIncomingPacketTypeAndLength_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_ProcessIncomingPacketTypeAndLength_ReturnThruPtr_pIncomingPacket( &incomingPacket ); /* Assume QoS = 1 so that a PUBACK will be sent after receiving PUBLISH. */ @@ -4573,6 +4621,8 @@ void test_MQTT_Subscribe_happy_path( void ) &incomingRecords, 4 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + context.connectStatus=MQTTConnected; + /* Verify MQTTSuccess is returned with the following mocks. */ MQTT_GetSubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetSubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); @@ -4614,6 +4664,8 @@ void test_MQTT_Subscribe_happy_path1( void ) &incomingRecords, 4 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + context.connectStatus=MQTTConnected; + /* Verify MQTTSuccess is returned with the following mocks. */ MQTT_GetSubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetSubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); @@ -4656,6 +4708,8 @@ void test_MQTT_Subscribe_happy_path2( void ) &incomingRecords, 4 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + context.connectStatus=MQTTConnected; + /* Verify MQTTSuccess is returned with the following mocks. */ MQTT_GetSubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetSubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); @@ -4713,6 +4767,8 @@ void test_MQTT_Subscribe_MultipleSubscriptions( void ) &incomingRecords, 4 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + context.connectStatus=MQTTConnected; + /* Verify MQTTSuccess is returned with the following mocks. */ MQTT_GetSubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetSubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); @@ -4750,6 +4806,9 @@ void test_MQTT_Subscribe_error_paths1( void ) /* Initialize context. */ mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + + context.connectStatus=MQTTConnected; + /* Verify MQTTSendFailed is propagated when transport interface returns an error. */ MQTT_GetSubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetSubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); @@ -4787,6 +4846,8 @@ void test_MQTT_Subscribe_error_paths2( void ) mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + context.connectStatus=MQTTConnected; + MQTT_GetSubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetSubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); MQTT_GetSubscribePacketSize_ReturnThruPtr_pRemainingLength( &remainingLength ); @@ -4829,6 +4890,8 @@ void test_MQTT_Subscribe_error_paths_timerOverflowCheck( void ) mqttStatus = MQTT_Init( &context, &transport, getTimeMock, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + context.connectStatus=MQTTConnected; + MQTT_GetSubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetSubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); MQTT_GetSubscribePacketSize_ReturnThruPtr_pRemainingLength( &remainingLength ); @@ -4872,6 +4935,8 @@ void test_MQTT_Subscribe_error_paths_timerOverflowCheck1( void ) mqttStatus = MQTT_Init( &context, &transport, getTimeMockBigTimeStep, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + context.connectStatus=MQTTConnected; + MQTT_GetSubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetSubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); MQTT_GetSubscribePacketSize_ReturnThruPtr_pRemainingLength( &remainingLength ); @@ -4946,6 +5011,9 @@ void test_MQTT_Unsubscribe_happy_path( void ) /* Initialize context. */ mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + + context.connectStatus=MQTTConnected; + /* Verify MQTTSuccess is returned with the following mocks. */ MQTT_SerializeUnsubscribeHeader_Stub( MQTT_SerializeUnsubscribeHeader_cb ); MQTT_GetUnsubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); @@ -4986,6 +5054,8 @@ void test_MQTT_Unsubscribe_happy_path1( void ) &incomingRecords, 4 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + context.connectStatus=MQTTConnected; + /* Verify MQTTSuccess is returned with the following mocks. */ MQTT_GetUnsubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetUnsubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); @@ -5029,6 +5099,8 @@ void test_MQTT_unsubscribe_happy_path2( void ) &incomingRecords, 4 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + context.connectStatus=MQTTConnected; + /* Verify MQTTSuccess is returned with the following mocks. */ MQTT_GetUnsubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetUnsubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); @@ -5086,6 +5158,8 @@ void test_MQTT_Unsubscribe_MultipleSubscriptions( void ) &incomingRecords, 4 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + context.connectStatus=MQTTConnected; + /* Verify MQTTSuccess is returned with the following mocks. */ MQTT_GetUnsubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetUnsubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); @@ -5125,6 +5199,9 @@ void test_MQTT_Unsubscribe_error_path1( void ) /* Initialize context. */ mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + + context.connectStatus=MQTTConnected; + MQTT_SerializeUnsubscribeHeader_Stub( MQTT_SerializeUnsubscribeHeader_cb ); /* Verify MQTTSendFailed is propagated when transport interface returns an error. */ MQTT_GetUnsubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); @@ -5165,6 +5242,8 @@ void test_MQTT_Unsubscribe_error_path2( void ) mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + context.connectStatus=MQTTConnected; + MQTT_SerializeUnsubscribeHeader_Stub( MQTT_SerializeUnsubscribeHeader_cb ); transport.send = transportSendNoBytes; /* Use the mock function that returns zero bytes sent. */ MQTT_GetUnsubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); @@ -5208,6 +5287,9 @@ void test_MQTT_Ping_happy_path( void ) /* Initialize context. */ mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + + context.connectStatus=MQTTConnected; + /* Verify MQTTSuccess is returned. */ MQTT_GetPingreqPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetPingreqPacketSize_ReturnThruPtr_pPacketSize( &pingreqSize ); @@ -5243,6 +5325,9 @@ void test_MQTT_Ping_error_path( void ) /* Initialize context. */ mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + + context.connectStatus=MQTTConnected; + /* Verify MQTTSendFailed is propagated when transport interface returns an error. */ MQTT_GetPingreqPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetPingreqPacketSize_ReturnThruPtr_pPacketSize( &pingreqSize ); @@ -5257,6 +5342,9 @@ void test_MQTT_Ping_error_path( void ) /* Initialize context. */ mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + + context.connectStatus=MQTTConnected; + MQTT_GetPingreqPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetPingreqPacketSize_ReturnThruPtr_pPacketSize( &pingreqSize ); MQTT_SerializePingreq_ExpectAnyArgsAndReturn( MQTTSuccess ); @@ -5268,6 +5356,9 @@ void test_MQTT_Ping_error_path( void ) /* Initialize context. */ mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + + context.connectStatus=MQTTConnected; + /* Verify MQTTBadParameter is propagated when getting PINGREQ packet size fails. */ MQTT_GetPingreqPacketSize_ExpectAnyArgsAndReturn( MQTTBadParameter ); MQTT_GetPingreqPacketSize_ReturnThruPtr_pPacketSize( &pingreqSize ); @@ -5855,7 +5946,19 @@ void test_MQTT_Status_strerror( void ) str = MQTT_Status_strerror( status ); TEST_ASSERT_EQUAL_STRING( "MQTTNeedMoreBytes", str ); - status = MQTTNeedMoreBytes + 1; + status = MQTTStatusConnected; + str = MQTT_Status_strerror( status ); + TEST_ASSERT_EQUAL_STRING( "MQTTStatusConnected", str ); + + status = MQTTStatusNotConnected; + str = MQTT_Status_strerror( status ); + TEST_ASSERT_EQUAL_STRING( "MQTTStatusNotConnected", str ); + + status = MQTTStatusDisconnectPending; + str = MQTT_Status_strerror( status ); + TEST_ASSERT_EQUAL_STRING( "MQTTStatusDisconnectPending", str ); + + status = MQTTStatusDisconnectPending + 1; str = MQTT_Status_strerror( status ); TEST_ASSERT_EQUAL_STRING( "Invalid MQTT Status code", str ); } From 0eda2758821fd0c6a5f0eb47f022521c465163f4 Mon Sep 17 00:00:00 2001 From: GitHub Action Date: Wed, 11 Sep 2024 10:31:58 +0000 Subject: [PATCH 08/17] Uncrustify: triggered by comment. --- source/core_mqtt.c | 12 ++--- test/unit-test/core_mqtt_utest.c | 90 ++++++++++++++++---------------- 2 files changed, 51 insertions(+), 51 deletions(-) diff --git a/source/core_mqtt.c b/source/core_mqtt.c index 6cae1380d..5194c2ffc 100644 --- a/source/core_mqtt.c +++ b/source/core_mqtt.c @@ -2833,18 +2833,18 @@ MQTTStatus_t MQTT_Connect( MQTTContext_t * pContext, pContext->index = 0; ( void ) memset( pContext->networkBuffer.pBuffer, 0, pContext->networkBuffer.size ); - if( *pSessionPresent != true && pContext->outgoingPublishRecordMaxCount > 0U ) + if( ( *pSessionPresent != true ) && ( pContext->outgoingPublishRecordMaxCount > 0U ) ) { /* Clear any existing records if a new session is established. */ ( void ) memset( pContext->outgoingPublishRecords, - 0x00, - pContext->outgoingPublishRecordMaxCount * sizeof( *pContext->outgoingPublishRecords ) ); + 0x00, + pContext->outgoingPublishRecordMaxCount * sizeof( *pContext->outgoingPublishRecords ) ); } - else if( *pSessionPresent != true && pContext->incomingPublishRecordMaxCount > 0U ) + else if( ( *pSessionPresent != true ) && ( pContext->incomingPublishRecordMaxCount > 0U ) ) { ( void ) memset( pContext->incomingPublishRecords, - 0x00, - pContext->incomingPublishRecordMaxCount * sizeof( *pContext->incomingPublishRecords ) ); + 0x00, + pContext->incomingPublishRecordMaxCount * sizeof( *pContext->incomingPublishRecords ) ); } else { diff --git a/test/unit-test/core_mqtt_utest.c b/test/unit-test/core_mqtt_utest.c index 66db8aa92..1246d8a7d 100644 --- a/test/unit-test/core_mqtt_utest.c +++ b/test/unit-test/core_mqtt_utest.c @@ -2430,7 +2430,7 @@ void test_MQTT_Publish_DuplicatePublish( void ) mqttContext.outgoingPublishRecordMaxCount = 10; mqttContext.outgoingPublishRecords = outgoingPublishRecord; - mqttContext.connectStatus=MQTTConnected; + mqttContext.connectStatus = MQTTConnected; publishInfo.qos = MQTTQoS1; publishInfo.dup = true; @@ -2472,7 +2472,7 @@ void test_MQTT_Publish_DuplicatePublish_UpdateFailed( void ) mqttContext.outgoingPublishRecordMaxCount = 10; mqttContext.outgoingPublishRecords = outgoingPublishRecord; - mqttContext.connectStatus=MQTTConnected; + mqttContext.connectStatus = MQTTConnected; publishInfo.qos = MQTTQoS1; publishInfo.dup = true; @@ -2516,7 +2516,7 @@ void test_MQTT_Publish_WriteVSendsPartialBytes( void ) mqttContext.outgoingPublishRecordMaxCount = 10; mqttContext.outgoingPublishRecords = outgoingPublishRecord; - mqttContext.connectStatus=MQTTConnected; + mqttContext.connectStatus = MQTTConnected; publishInfo.qos = MQTTQoS1; publishInfo.dup = false; @@ -2551,7 +2551,7 @@ void test_MQTT_Publish7( void ) MQTT_GetPublishPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_SerializePublishHeaderWithoutTopic_ExpectAnyArgsAndReturn( MQTTSuccess ); - mqttContext.connectStatus=MQTTConnected; + mqttContext.connectStatus = MQTTConnected; /* We need sendPacket to be called with at least 1 byte to send, so that * it can return failure. This argument is the output of serializing the @@ -2581,7 +2581,7 @@ void test_MQTT_Publish8( void ) memset( &publishInfo, 0x0, sizeof( publishInfo ) ); MQTT_Init( &mqttContext, &transport, getTime, eventCallback, &networkBuffer ); - mqttContext.connectStatus=MQTTConnected; + mqttContext.connectStatus = MQTTConnected; /* We want to test the first call to sendPacket within sendPublish succeeding, * and the second one failing. */ @@ -2613,7 +2613,7 @@ void test_MQTT_Publish9( void ) memset( &publishInfo, 0x0, sizeof( publishInfo ) ); MQTT_Init( &mqttContext, &transport, getTime, eventCallback, &networkBuffer ); - mqttContext.connectStatus=MQTTConnected; + mqttContext.connectStatus = MQTTConnected; MQTT_GetPublishPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_SerializePublishHeaderWithoutTopic_ExpectAnyArgsAndReturn( MQTTSuccess ); @@ -2641,7 +2641,7 @@ void test_MQTT_Publish10( void ) memset( &publishInfo, 0x0, sizeof( publishInfo ) ); MQTT_Init( &mqttContext, &transport, getTime, eventCallback, &networkBuffer ); - mqttContext.connectStatus=MQTTConnected; + mqttContext.connectStatus = MQTTConnected; /* Test that sending a publish without a payload succeeds. */ publishInfo.pPayload = NULL; @@ -2672,7 +2672,7 @@ void test_MQTT_Publish11( void ) memset( &publishInfo, 0x0, sizeof( publishInfo ) ); MQTT_Init( &mqttContext, &transport, getTime, eventCallback, &networkBuffer ); - mqttContext.connectStatus=MQTTConnected; + mqttContext.connectStatus = MQTTConnected; /* Restore the test payload and length. */ publishInfo.pPayload = "Test"; @@ -2723,7 +2723,7 @@ void test_MQTT_Publish12( void ) mqttContext.outgoingPublishRecords->qos = MQTTQoS2; - mqttContext.connectStatus=MQTTConnected; + mqttContext.connectStatus = MQTTConnected; expectedState = MQTTPublishSend; @@ -2761,7 +2761,7 @@ void test_MQTT_Publish13( void ) memset( &publishInfo, 0x0, sizeof( publishInfo ) ); MQTT_Init( &mqttContext, &transport, getTime, eventCallback, &networkBuffer ); - mqttContext.connectStatus=MQTTConnected; + mqttContext.connectStatus = MQTTConnected; MQTT_InitStatefulQoS( &mqttContext, &outgoingRecords, 4, @@ -2798,7 +2798,7 @@ void test_MQTT_Publish14( void ) memset( &publishInfo, 0x0, sizeof( publishInfo ) ); MQTT_Init( &mqttContext, &transport, getTime, eventCallback, &networkBuffer ); - mqttContext.connectStatus=MQTTConnected; + mqttContext.connectStatus = MQTTConnected; /* Duplicate publish. dup flag is marked by application. */ publishInfo.dup = true; @@ -2830,7 +2830,7 @@ void test_MQTT_Publish15( void ) memset( &publishInfo, 0x0, sizeof( publishInfo ) ); MQTT_Init( &mqttContext, &transport, getTime, eventCallback, &networkBuffer ); - mqttContext.connectStatus=MQTTConnected; + mqttContext.connectStatus = MQTTConnected; /* Duplicate publish. dup flag is marked by application. * State record is not present. */ @@ -2867,7 +2867,7 @@ void test_MQTT_Publish_Send_Timeout( void ) /* Initialize the MQTT context. */ MQTT_Init( &mqttContext, &transport, getTime, eventCallback, &networkBuffer ); - mqttContext.connectStatus=MQTTConnected; + mqttContext.connectStatus = MQTTConnected; /* Setup for making sure that the test results in calling sendPacket function * where calls to transport send function are made (repeatedly to send packet @@ -3245,7 +3245,7 @@ void test_MQTT_ProcessLoop_HandleKeepAlive1( void ) mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; /* Verify MQTTSuccess is returned. */ MQTT_GetPingreqPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); @@ -3387,7 +3387,7 @@ void test_MQTT_ProcessLoop_handleIncomingPublish_Happy_Path1( void ) mqttStatus = MQTT_InitStatefulQoS( &context, NULL, 0, pIncomingCallback, 10 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; modifyIncomingPacketStatus = MQTTSuccess; @@ -3426,7 +3426,7 @@ void test_MQTT_ProcessLoop_handleIncomingPublish_Happy_Path2( void ) mqttStatus = MQTT_InitStatefulQoS( &context, NULL, 0, pIncomingPublish, 10 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; modifyIncomingPacketStatus = MQTTSuccess; @@ -3472,7 +3472,7 @@ void test_MQTT_ProcessLoop_handleIncomingPublish_Happy_Path3( void ) &outgoingRecords, 4, &incomingRecords, 4 ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; modifyIncomingPacketStatus = MQTTSuccess; @@ -3520,7 +3520,7 @@ void test_MQTT_ProcessLoop_handleIncomingPublish_Happy_Path4( void ) mqttStatus = MQTT_InitStatefulQoS( &context, NULL, 0, pIncomingPublish, 10 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; modifyIncomingPacketStatus = MQTTSuccess; @@ -3563,7 +3563,7 @@ void test_MQTT_ProcessLoop_handleIncomingPublish_Happy_Path5( void ) mqttStatus = MQTT_InitStatefulQoS( &context, NULL, 0, pIncomingPublish, 10 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; modifyIncomingPacketStatus = MQTTSuccess; @@ -3603,7 +3603,7 @@ void test_MQTT_ProcessLoop_handleIncomingPublish_Happy_Path6( void ) mqttStatus = MQTT_InitStatefulQoS( &context, NULL, 0, pIncomingPublish, 10 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; modifyIncomingPacketStatus = MQTTSuccess; @@ -3748,7 +3748,7 @@ void test_MQTT_ProcessLoop_handleIncomingAck_Happy_Paths( void ) mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; modifyIncomingPacketStatus = MQTTSuccess; @@ -3848,7 +3848,7 @@ void test_MQTT_ProcessLoop_handleIncomingAck_Error_Paths( void ) mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; modifyIncomingPacketStatus = MQTTSuccess; @@ -3939,7 +3939,7 @@ void test_MQTT_ProcessLoop_handleKeepAlive_Happy_Paths1( void ) mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; context.waitingForPingResp = false; context.keepAliveIntervalSec = 0; @@ -4226,7 +4226,7 @@ void test_MQTT_ProcessLoop_handleKeepAlive_Error_Paths3( void ) mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; globalEntryTime = PACKET_RX_TIMEOUT_MS + 1; context.keepAliveIntervalSec = 0; @@ -4269,7 +4269,7 @@ void test_MQTT_ProcessLoop_handleKeepAlive_Error_Paths4( void ) mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; globalEntryTime = PACKET_RX_TIMEOUT_MS + 1; context.keepAliveIntervalSec = ( PACKET_TX_TIMEOUT_MS / 1000 ) + 1U; @@ -4416,7 +4416,7 @@ void test_MQTT_ProcessLoop_Timer_Overflow( void ) mqttStatus = MQTT_InitStatefulQoS( &context, NULL, 0, incomingPublishRecords, 10 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; MQTT_ProcessIncomingPacketTypeAndLength_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_ProcessIncomingPacketTypeAndLength_ReturnThruPtr_pIncomingPacket( &incomingPacket ); @@ -4621,7 +4621,7 @@ void test_MQTT_Subscribe_happy_path( void ) &incomingRecords, 4 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; /* Verify MQTTSuccess is returned with the following mocks. */ MQTT_GetSubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); @@ -4664,7 +4664,7 @@ void test_MQTT_Subscribe_happy_path1( void ) &incomingRecords, 4 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; /* Verify MQTTSuccess is returned with the following mocks. */ MQTT_GetSubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); @@ -4708,7 +4708,7 @@ void test_MQTT_Subscribe_happy_path2( void ) &incomingRecords, 4 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; /* Verify MQTTSuccess is returned with the following mocks. */ MQTT_GetSubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); @@ -4767,7 +4767,7 @@ void test_MQTT_Subscribe_MultipleSubscriptions( void ) &incomingRecords, 4 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; /* Verify MQTTSuccess is returned with the following mocks. */ MQTT_GetSubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); @@ -4807,7 +4807,7 @@ void test_MQTT_Subscribe_error_paths1( void ) mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; /* Verify MQTTSendFailed is propagated when transport interface returns an error. */ MQTT_GetSubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); @@ -4846,7 +4846,7 @@ void test_MQTT_Subscribe_error_paths2( void ) mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; MQTT_GetSubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetSubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); @@ -4890,7 +4890,7 @@ void test_MQTT_Subscribe_error_paths_timerOverflowCheck( void ) mqttStatus = MQTT_Init( &context, &transport, getTimeMock, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; MQTT_GetSubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetSubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); @@ -4935,7 +4935,7 @@ void test_MQTT_Subscribe_error_paths_timerOverflowCheck1( void ) mqttStatus = MQTT_Init( &context, &transport, getTimeMockBigTimeStep, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; MQTT_GetSubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetSubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); @@ -5012,7 +5012,7 @@ void test_MQTT_Unsubscribe_happy_path( void ) mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; /* Verify MQTTSuccess is returned with the following mocks. */ MQTT_SerializeUnsubscribeHeader_Stub( MQTT_SerializeUnsubscribeHeader_cb ); @@ -5054,7 +5054,7 @@ void test_MQTT_Unsubscribe_happy_path1( void ) &incomingRecords, 4 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; /* Verify MQTTSuccess is returned with the following mocks. */ MQTT_GetUnsubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); @@ -5099,7 +5099,7 @@ void test_MQTT_unsubscribe_happy_path2( void ) &incomingRecords, 4 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; /* Verify MQTTSuccess is returned with the following mocks. */ MQTT_GetUnsubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); @@ -5158,7 +5158,7 @@ void test_MQTT_Unsubscribe_MultipleSubscriptions( void ) &incomingRecords, 4 ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; /* Verify MQTTSuccess is returned with the following mocks. */ MQTT_GetUnsubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); @@ -5200,7 +5200,7 @@ void test_MQTT_Unsubscribe_error_path1( void ) mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; MQTT_SerializeUnsubscribeHeader_Stub( MQTT_SerializeUnsubscribeHeader_cb ); /* Verify MQTTSendFailed is propagated when transport interface returns an error. */ @@ -5242,7 +5242,7 @@ void test_MQTT_Unsubscribe_error_path2( void ) mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; MQTT_SerializeUnsubscribeHeader_Stub( MQTT_SerializeUnsubscribeHeader_cb ); transport.send = transportSendNoBytes; /* Use the mock function that returns zero bytes sent. */ @@ -5288,7 +5288,7 @@ void test_MQTT_Ping_happy_path( void ) mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; /* Verify MQTTSuccess is returned. */ MQTT_GetPingreqPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); @@ -5326,7 +5326,7 @@ void test_MQTT_Ping_error_path( void ) mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; /* Verify MQTTSendFailed is propagated when transport interface returns an error. */ MQTT_GetPingreqPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); @@ -5343,7 +5343,7 @@ void test_MQTT_Ping_error_path( void ) mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; MQTT_GetPingreqPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetPingreqPacketSize_ReturnThruPtr_pPacketSize( &pingreqSize ); @@ -5357,8 +5357,8 @@ void test_MQTT_Ping_error_path( void ) mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; - + context.connectStatus = MQTTConnected; + /* Verify MQTTBadParameter is propagated when getting PINGREQ packet size fails. */ MQTT_GetPingreqPacketSize_ExpectAnyArgsAndReturn( MQTTBadParameter ); MQTT_GetPingreqPacketSize_ReturnThruPtr_pPacketSize( &pingreqSize ); From eb68f0cffb82289162de508ecba85977ef8a07a4 Mon Sep 17 00:00:00 2001 From: Dakshit Babbar Date: Thu, 12 Sep 2024 22:02:27 +0530 Subject: [PATCH 09/17] Add new unit tests for 100% code coverage --- source/core_mqtt.c | 32 +- source/core_mqtt_serializer.c | 31 +- test/unit-test/core_mqtt_serializer_utest.c | 4 +- test/unit-test/core_mqtt_utest.c | 467 +++++++++++++++++++- tools/cmock/coverage.cmake | 10 +- 5 files changed, 479 insertions(+), 65 deletions(-) diff --git a/source/core_mqtt.c b/source/core_mqtt.c index 5194c2ffc..9da735c7a 100644 --- a/source/core_mqtt.c +++ b/source/core_mqtt.c @@ -2386,24 +2386,6 @@ static MQTTStatus_t receiveConnack( MQTTContext_t * pContext, pContext->transportInterface.pNetworkContext, pIncomingPacket ); - if( status == MQTTStatusDisconnectPending ) - { - /* Convert this status to MQTTRecvFailed as MQTTStatusDisconnectPending is - * reserved for cases where we need to let the user know about the MQTT - * connection status. - */ - status = MQTTRecvFailed; - - MQTT_PRE_STATE_UPDATE_HOOK( pContext ); - - if( pContext->connectStatus == MQTTConnected ) - { - pContext->connectStatus = MQTTDisconnectPending; - } - - MQTT_POST_STATE_UPDATE_HOOK( pContext ); - } - /* The loop times out based on 2 conditions. * 1. If timeoutMs is greater than 0: * Loop times out based on the timeout calculated by getTime() @@ -2731,7 +2713,7 @@ MQTTStatus_t MQTT_CheckConnectStatus( MQTTContext_t * pContext ) { MQTT_PRE_STATE_UPDATE_HOOK( pContext ); - connectStatus = pContext->connectStatus == MQTTConnected; + connectStatus = pContext->connectStatus; switch( connectStatus ) { @@ -2739,15 +2721,12 @@ MQTTStatus_t MQTT_CheckConnectStatus( MQTTContext_t * pContext ) status = MQTTStatusConnected; break; - case MQTTNotConnected: - status = MQTTStatusNotConnected; - break; - case MQTTDisconnectPending: status = MQTTStatusDisconnectPending; break; default: + status = MQTTStatusNotConnected; break; } @@ -2840,16 +2819,13 @@ MQTTStatus_t MQTT_Connect( MQTTContext_t * pContext, 0x00, pContext->outgoingPublishRecordMaxCount * sizeof( *pContext->outgoingPublishRecords ) ); } - else if( ( *pSessionPresent != true ) && ( pContext->incomingPublishRecordMaxCount > 0U ) ) + + if( *pSessionPresent != true && pContext->incomingPublishRecordMaxCount > 0U ) { ( void ) memset( pContext->incomingPublishRecords, 0x00, pContext->incomingPublishRecordMaxCount * sizeof( *pContext->incomingPublishRecords ) ); } - else - { - /* MISRA Empty body */ - } pContext->connectStatus = MQTTConnected; /* Initialize keep-alive fields after a successful connection. */ diff --git a/source/core_mqtt_serializer.c b/source/core_mqtt_serializer.c index 2cc7623a6..97022034c 100644 --- a/source/core_mqtt_serializer.c +++ b/source/core_mqtt_serializer.c @@ -135,13 +135,6 @@ */ #define MQTT_REMAINING_LENGTH_INVALID ( ( size_t ) 268435456 ) -/** - * @brief A value that represents transport layer error while retreiving the remaining length. - * - * This value is greater than what is allowed by the MQTT specification. - */ -#define MQTT_REMAINING_LENGTH_TRANSPORT_ERROR ( ( size_t ) 268435457 ) - /** * @brief The minimum remaining length for a QoS 0 PUBLISH. * @@ -827,24 +820,20 @@ static size_t getRemainingLength( TransportRecv_t recvFunc, multiplier *= 128U; bytesDecoded++; } - else if( bytesReceived < 0 ) - { - remainingLength = MQTT_REMAINING_LENGTH_TRANSPORT_ERROR; - } else { remainingLength = MQTT_REMAINING_LENGTH_INVALID; } } - if( ( remainingLength == MQTT_REMAINING_LENGTH_INVALID ) || ( remainingLength == MQTT_REMAINING_LENGTH_TRANSPORT_ERROR ) ) + if( remainingLength == MQTT_REMAINING_LENGTH_INVALID ) { break; } } while( ( encodedByte & 0x80U ) != 0U ); /* Check that the decoded remaining length conforms to the MQTT specification. */ - if( ( remainingLength != MQTT_REMAINING_LENGTH_INVALID ) && ( remainingLength != MQTT_REMAINING_LENGTH_TRANSPORT_ERROR ) ) + if( remainingLength != MQTT_REMAINING_LENGTH_INVALID ) { expectedSize = remainingLengthEncodedSize( remainingLength ); @@ -2602,16 +2591,6 @@ MQTTStatus_t MQTT_GetIncomingPacketTypeAndLength( TransportRecv_t readFunc, LogError( ( "Incoming packet remaining length invalid." ) ); status = MQTTBadResponse; } - else if( pIncomingPacket->remainingLength == MQTT_REMAINING_LENGTH_TRANSPORT_ERROR ) - { - /* MQTT Connection status cannot be updated here hence bubble up - * MQTTStatusDisconnectPending status to the calling API that can update it. */ - status = MQTTStatusDisconnectPending; - } - else - { - /* Empty else MISRA 15.7 */ - } } else { @@ -2624,12 +2603,6 @@ MQTTStatus_t MQTT_GetIncomingPacketTypeAndLength( TransportRecv_t readFunc, { status = MQTTNoDataAvailable; } - else if( ( status != MQTTBadParameter ) && ( bytesReceived < 0 ) ) - { - /* MQTT Connection status cannot be updated here hence bubble up - * MQTTStatusDisconnectPending status to the calling API that can update it. */ - status = MQTTStatusDisconnectPending; - } /* If the input packet was valid, then any other number of bytes received is * a failure. */ diff --git a/test/unit-test/core_mqtt_serializer_utest.c b/test/unit-test/core_mqtt_serializer_utest.c index b521eafbc..e91aae72a 100644 --- a/test/unit-test/core_mqtt_serializer_utest.c +++ b/test/unit-test/core_mqtt_serializer_utest.c @@ -1904,7 +1904,7 @@ void test_MQTT_GetIncomingPacketTypeAndLength( void ) memset( buffer, 0x00, 10 ); bufPtr = buffer; status = MQTT_GetIncomingPacketTypeAndLength( mockReceiveFailure, &networkContext, &mqttPacket ); - TEST_ASSERT_EQUAL( MQTTStatusDisconnectPending, status ); + TEST_ASSERT_EQUAL( MQTTRecvFailed, status ); /* Test if no data is available. */ bufPtr = buffer; @@ -1921,7 +1921,7 @@ void test_MQTT_GetIncomingPacketTypeAndLength( void ) bufPtr = buffer; buffer[ 0 ] = MQTT_PACKET_TYPE_PUBREL; status = MQTT_GetIncomingPacketTypeAndLength( mockReceiveSucceedThenFail, &networkContext, &mqttPacket ); - TEST_ASSERT_EQUAL( MQTTStatusDisconnectPending, status ); + TEST_ASSERT_EQUAL( MQTTBadResponse, status ); } /* ========================================================================== */ diff --git a/test/unit-test/core_mqtt_utest.c b/test/unit-test/core_mqtt_utest.c index 1246d8a7d..234ee7777 100644 --- a/test/unit-test/core_mqtt_utest.c +++ b/test/unit-test/core_mqtt_utest.c @@ -1062,6 +1062,11 @@ static void expectProcessLoopCalls( MQTTContext_t * const pContext, } } + if( expectMoreCalls && (pContext->connectStatus != MQTTConnected) ) + { + expectMoreCalls = false; + } + /* Update the state based on the sent packet. */ if( expectMoreCalls ) { @@ -1156,6 +1161,46 @@ void test_MQTT_Init_Invalid_Params( void ) TEST_ASSERT_EQUAL( MQTTBadParameter, mqttStatus ); } +/* ========================================================================== */ + +/** + * @brief Test that NULL parameter causes MQTT_CheckConnectStatus to return MQTTBadParameter. + */ +void test_MQTT_CheckConnectStatus_invalid_params( void ) +{ + MQTTStatus_t mqttStatus = { 0 }; + + mqttStatus = MQTT_CheckConnectStatus(NULL); + TEST_ASSERT_EQUAL( MQTTBadParameter, mqttStatus ); +} + +/** + * @brief Test that MQTT_CheckConnectStatus returns correct status corresponding to the connection status. + */ +void test_MQTT_CheckConnectStatus_return_correct_status( void ) +{ + MQTTStatus_t mqttStatus = { 0 }; + MQTTContext_t context = { 0 }; + TransportInterface_t transport = { 0 }; + MQTTFixedBuffer_t networkBuffer = { 0 }; + + setupTransportInterface( &transport ); + setupNetworkBuffer( &networkBuffer ); + + context.connectStatus = MQTTConnected; + mqttStatus = MQTT_CheckConnectStatus(&context); + TEST_ASSERT_EQUAL( MQTTStatusConnected, mqttStatus ); + + context.connectStatus = MQTTNotConnected; + mqttStatus = MQTT_CheckConnectStatus(&context); + TEST_ASSERT_EQUAL( MQTTStatusNotConnected, mqttStatus ); + + context.connectStatus = MQTTDisconnectPending; + mqttStatus = MQTT_CheckConnectStatus(&context); + TEST_ASSERT_EQUAL( MQTTStatusDisconnectPending, mqttStatus ); +} + + /* ========================================================================== */ static uint8_t * MQTT_SerializeConnectFixedHeader_cb( uint8_t * pIndex, @@ -1172,6 +1217,72 @@ static uint8_t * MQTT_SerializeConnectFixedHeader_cb( uint8_t * pIndex, return pIndex; } +/** + * @brief Test MQTT_Connect, when the status is already MQTTConnected. + */ +void test_MQTT_Connect_sendConnect_already_connected( void ) +{ + MQTTContext_t mqttContext = { 0 }; + MQTTConnectInfo_t connectInfo = { 0 }; + uint32_t timeout = 2; + bool sessionPresent; + MQTTStatus_t status; + TransportInterface_t transport = { 0 }; + MQTTFixedBuffer_t networkBuffer = { 0 }; + size_t remainingLength; + size_t packetSize; + + memset( &mqttContext, 0x0, sizeof( mqttContext ) ); + MQTT_Init( &mqttContext, &transport, getTime, eventCallback, &networkBuffer ); + + /* set MQTT Connection status as connected*/ + mqttContext.connectStatus = MQTTConnected; + + MQTT_SerializeConnectFixedHeader_Stub( MQTT_SerializeConnectFixedHeader_cb ); + MQTT_GetConnectPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); + MQTT_GetConnectPacketSize_IgnoreArg_pPacketSize(); + MQTT_GetConnectPacketSize_IgnoreArg_pRemainingLength(); + MQTT_GetConnectPacketSize_ReturnThruPtr_pPacketSize( &packetSize ); + MQTT_GetConnectPacketSize_ReturnThruPtr_pRemainingLength( &remainingLength ); + + status = MQTT_Connect( &mqttContext, &connectInfo, NULL, timeout, &sessionPresent ); + + TEST_ASSERT_EQUAL_INT( MQTTStatusConnected, status ); +} + +/** + * @brief Test MQTT_Connect, when the status is MQTTDisconnectPending. + */ +void test_MQTT_Connect_sendConnect_disconnect_pending( void ) +{ + MQTTContext_t mqttContext = { 0 }; + MQTTConnectInfo_t connectInfo = { 0 }; + uint32_t timeout = 2; + bool sessionPresent; + MQTTStatus_t status; + TransportInterface_t transport = { 0 }; + MQTTFixedBuffer_t networkBuffer = { 0 }; + size_t remainingLength; + size_t packetSize; + + memset( &mqttContext, 0x0, sizeof( mqttContext ) ); + MQTT_Init( &mqttContext, &transport, getTime, eventCallback, &networkBuffer ); + + /* set MQTT Connection status as connected*/ + mqttContext.connectStatus = MQTTDisconnectPending; + + MQTT_SerializeConnectFixedHeader_Stub( MQTT_SerializeConnectFixedHeader_cb ); + MQTT_GetConnectPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); + MQTT_GetConnectPacketSize_IgnoreArg_pPacketSize(); + MQTT_GetConnectPacketSize_IgnoreArg_pRemainingLength(); + MQTT_GetConnectPacketSize_ReturnThruPtr_pPacketSize( &packetSize ); + MQTT_GetConnectPacketSize_ReturnThruPtr_pRemainingLength( &remainingLength ); + + status = MQTT_Connect( &mqttContext, &connectInfo, NULL, timeout, &sessionPresent ); + + TEST_ASSERT_EQUAL_INT( MQTTStatusDisconnectPending, status ); +} + /** * @brief Test MQTT_Connect, except for receiving the CONNACK. */ @@ -1312,6 +1423,7 @@ void test_MQTT_Connect_sendConnect2( void ) packetSize = 13; remainingLength = 11; mqttContext.transportInterface.send = transportSendFailure; + mqttContext.transportInterface.writev = NULL; MQTT_SerializeConnectFixedHeader_Stub( MQTT_SerializeConnectFixedHeader_cb ); MQTT_GetConnectPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetConnectPacketSize_IgnoreArg_pPacketSize(); @@ -1895,8 +2007,28 @@ void test_MQTT_Connect_resendPendingAcks( void ) TEST_ASSERT_EQUAL_INT( MQTTNotConnected, mqttContext.connectStatus ); TEST_ASSERT_TRUE( sessionPresentResult ); - /* Test 3. One packet found in ack pending state, Sent + /* Test 3. One packet found in ack pending state, but Transport Send failed. */ + sessionPresentResult = false; + mqttContext.connectStatus = MQTTNotConnected; + mqttContext.keepAliveIntervalSec = 0; + mqttContext.transportInterface.send = transportSendFailure; + MQTT_GetIncomingPacketTypeAndLength_ExpectAnyArgsAndReturn( MQTTSuccess ); + MQTT_GetIncomingPacketTypeAndLength_ReturnThruPtr_pIncomingPacket( &incomingPacket ); + MQTT_DeserializeAck_ExpectAnyArgsAndReturn( MQTTSuccess ); + MQTT_DeserializeAck_ReturnThruPtr_pSessionPresent( &sessionPresent ); + MQTT_PubrelToResend_ExpectAnyArgsAndReturn( packetIdentifier ); + MQTT_PubrelToResend_ReturnThruPtr_pState( &pubRelState ); + MQTT_SerializeAck_ExpectAnyArgsAndReturn( MQTTSuccess ); + MQTT_PubrelToResend_ExpectAnyArgsAndReturn( MQTT_PACKET_TYPE_INVALID ); + status = MQTT_Connect( &mqttContext, &connectInfo, NULL, timeout, &sessionPresentResult ); + TEST_ASSERT_EQUAL_INT( MQTTSendFailed, status ); + TEST_ASSERT_EQUAL_INT( MQTTDisconnectPending, mqttContext.connectStatus ); + TEST_ASSERT_TRUE( sessionPresentResult ); + mqttContext.transportInterface.send = transportSendSuccess; + + /* Test 4. One packet found in ack pending state, Sent * PUBREL successfully. */ + mqttContext.connectStatus = MQTTNotConnected; MQTT_GetIncomingPacketTypeAndLength_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetIncomingPacketTypeAndLength_ReturnThruPtr_pIncomingPacket( &incomingPacket ); MQTT_DeserializeAck_ExpectAnyArgsAndReturn( MQTTSuccess ); @@ -1913,7 +2045,7 @@ void test_MQTT_Connect_resendPendingAcks( void ) TEST_ASSERT_EQUAL_INT( MQTTConnected, mqttContext.connectStatus ); TEST_ASSERT_EQUAL_INT( connectInfo.keepAliveSeconds, mqttContext.keepAliveIntervalSec ); - /* Test 4. Three packets found in ack pending state. Sent PUBREL successfully + /* Test 5. Three packets found in ack pending state. Sent PUBREL successfully * for first and failed for second and no attempt for third. */ mqttContext.keepAliveIntervalSec = 0; mqttContext.connectStatus = MQTTNotConnected; @@ -1938,7 +2070,7 @@ void test_MQTT_Connect_resendPendingAcks( void ) TEST_ASSERT_EQUAL_INT( MQTTBadParameter, status ); TEST_ASSERT_EQUAL_INT( MQTTNotConnected, mqttContext.connectStatus ); - /* Test 5. Two packets found in ack pending state. Sent PUBREL successfully + /* Test 6. Two packets found in ack pending state. Sent PUBREL successfully * for first and failed for second. */ MQTT_GetIncomingPacketTypeAndLength_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetIncomingPacketTypeAndLength_ReturnThruPtr_pIncomingPacket( &incomingPacket ); @@ -2444,6 +2576,40 @@ void test_MQTT_Publish_DuplicatePublish( void ) TEST_ASSERT_EQUAL_INT( MQTTSuccess, status ); } +/** + * @brief Test that MQTT_Publish works as intended when the connection status is anything but MQTTConnected. + */ +void test_MQTT_Publish_not_connected( void ) +{ + MQTTContext_t mqttContext = { 0 }; + MQTTPublishInfo_t publishInfo = { 0 }; + TransportInterface_t transport = { 0 }; + MQTTFixedBuffer_t networkBuffer = { 0 }; + MQTTStatus_t status; + + setupTransportInterface( &transport ); + setupNetworkBuffer( &networkBuffer ); + transport.send = transportSendFailure; + + memset( &mqttContext, 0x0, sizeof( mqttContext ) ); + memset( &publishInfo, 0x0, sizeof( publishInfo ) ); + MQTT_Init( &mqttContext, &transport, getTime, eventCallback, &networkBuffer ); + + /* Test 1 connecttion status is MQTTNotConnected */ + mqttContext.connectStatus=MQTTNotConnected; + MQTT_GetPublishPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); + MQTT_SerializePublishHeaderWithoutTopic_ExpectAnyArgsAndReturn( MQTTSuccess ); + status = MQTT_Publish( &mqttContext, &publishInfo, 10 ); + TEST_ASSERT_EQUAL_INT( MQTTStatusNotConnected, status ); + + /* Test 2 connecttion status is MQTTDisconnectPending */ + mqttContext.connectStatus=MQTTDisconnectPending; + MQTT_GetPublishPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); + MQTT_SerializePublishHeaderWithoutTopic_ExpectAnyArgsAndReturn( MQTTSuccess ); + status = MQTT_Publish( &mqttContext, &publishInfo, 10 ); + TEST_ASSERT_EQUAL_INT( MQTTStatusDisconnectPending, status ); +} + /** * @brief Test that MQTT_Publish works as intended. */ @@ -2886,6 +3052,32 @@ void test_MQTT_Publish_Send_Timeout( void ) /* ========================================================================== */ +/** + * @brief Test that MQTT_Disconnect works as intended when the connection is already disconnected. + */ +void test_MQTT_Disconnect_already_disconnected( void ) +{ + MQTTContext_t mqttContext = { 0 }; + MQTTStatus_t status; + TransportInterface_t transport = { 0 }; + MQTTFixedBuffer_t networkBuffer = { 0 }; + size_t disconnectSize = 2; + + setupTransportInterface( &transport ); + setupNetworkBuffer( &networkBuffer ); + + memset( &mqttContext, 0x0, sizeof( mqttContext ) ); + MQTT_Init( &mqttContext, &transport, getTime, eventCallback, &networkBuffer ); + mqttContext.connectStatus = MQTTNotConnected; + + /* Send failure with network error. */ + MQTT_GetDisconnectPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); + MQTT_GetDisconnectPacketSize_ReturnThruPtr_pPacketSize( &disconnectSize ); + MQTT_SerializeDisconnect_ExpectAnyArgsAndReturn( MQTTSuccess ); + status = MQTT_Disconnect( &mqttContext ); + TEST_ASSERT_EQUAL( MQTTStatusNotConnected, status ); +} + /** * @brief Test that MQTT_Disconnect works as intended. */ @@ -2946,6 +3138,7 @@ void test_MQTT_Disconnect2( void ) MQTT_SerializeDisconnect_ExpectAnyArgsAndReturn( MQTTSuccess ); status = MQTT_Disconnect( &mqttContext ); TEST_ASSERT_EQUAL( MQTTSendFailed, status ); + TEST_ASSERT_EQUAL( MQTTNotConnected, mqttContext.connectStatus ); } /** @@ -3038,6 +3231,52 @@ void test_MQTT_Disconnect4( void ) /* At disconnect, the buffer is cleared of any pending packets. */ TEST_ASSERT_EACH_EQUAL_UINT8( 0, mqttBuffer, MQTT_TEST_BUFFER_LENGTH ); } + +/** + * @brief Test that MQTT_Disconnect works as intended when status is disconnect pending. + */ +void test_MQTT_Disconnect4_status_disconnect_pending( void ) +{ + MQTTContext_t mqttContext = { 0 }; + MQTTStatus_t status; + uint8_t buffer[ 10 ]; + uint8_t * bufPtr = buffer; + NetworkContext_t networkContext = { 0 }; + TransportInterface_t transport = { 0 }; + MQTTFixedBuffer_t networkBuffer = { 0 }; + size_t disconnectSize = 2; + + /* Fill the buffer with garbage data. */ + memset( mqttBuffer, 0xAB, MQTT_TEST_BUFFER_LENGTH ); + + setupTransportInterface( &transport ); + setupNetworkBuffer( &networkBuffer ); + networkContext.buffer = &bufPtr; + transport.pNetworkContext = &networkContext; + transport.recv = transportRecvSuccess; + transport.send = transportSendFailure; + transport.writev = NULL; + + memset( &mqttContext, 0x0, sizeof( mqttContext ) ); + MQTT_Init( &mqttContext, &transport, getTime, eventCallback, &networkBuffer ); + mqttContext.connectStatus = MQTTDisconnectPending; + + /* Successful send. */ + mqttContext.transportInterface.send = mockSend; + MQTT_GetDisconnectPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); + MQTT_GetDisconnectPacketSize_ReturnThruPtr_pPacketSize( &disconnectSize ); + MQTT_SerializeDisconnect_ExpectAnyArgsAndReturn( MQTTSuccess ); + MQTT_SerializeDisconnect_Stub( MQTT_SerializeDisconnect_stub ); + /* Write a disconnect packet into the buffer. */ + mqttBuffer[ 0 ] = MQTT_PACKET_TYPE_DISCONNECT; + + status = MQTT_Disconnect( &mqttContext ); + + TEST_ASSERT_EQUAL( MQTTSuccess, status ); + TEST_ASSERT_EQUAL( MQTTNotConnected, mqttContext.connectStatus ); + /* At disconnect, the buffer is cleared of any pending packets. */ + TEST_ASSERT_EACH_EQUAL_UINT8( 0, mqttBuffer, MQTT_TEST_BUFFER_LENGTH ); +} /* ========================================================================== */ /** @@ -3298,6 +3537,25 @@ void test_MQTT_ProcessLoop_RecvFailed( void ) transport.recv = transportRecvFailure; setupNetworkBuffer( &networkBuffer ); + mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); + context.connectStatus = MQTTConnected; + + mqttStatus = MQTT_ProcessLoop( &context ); + + TEST_ASSERT_EQUAL( MQTTRecvFailed, mqttStatus ); +} + +void test_MQTT_ProcessLoop_RecvFailed_disconnected( void ) +{ + MQTTContext_t context = { 0 }; + TransportInterface_t transport = { 0 }; + MQTTFixedBuffer_t networkBuffer = { 0 }; + MQTTStatus_t mqttStatus; + + setupTransportInterface( &transport ); + transport.recv = transportRecvFailure; + setupNetworkBuffer( &networkBuffer ); + mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); mqttStatus = MQTT_ProcessLoop( &context ); @@ -3350,6 +3608,8 @@ void test_MQTT_ProcessLoop_discardPacket_second_recv_fail( void ) mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + context.connectStatus = MQTTConnected; + context.networkBuffer.size = 20; incomingPacket.type = currentPacketType; @@ -3677,6 +3937,7 @@ void test_MQTT_ProcessLoop_handleIncomingPublish_Error_Paths( void ) expectParams.pPubInfo = &publishInfo; /* The other loop parameter fields are irrelevant. */ expectProcessLoopCalls( &context, &expectParams ); + TEST_ASSERT_FALSE( isEventCallbackInvoked ); } @@ -3872,6 +4133,32 @@ void test_MQTT_ProcessLoop_handleIncomingAck_Error_Paths( void ) expectParams.processLoopStatus = MQTTNoMemory; expectProcessLoopCalls( &context, &expectParams ); + /* Verify that MQTTStatusNotConnected propagated when receiving a any ACK, + * here PUBREC but thr connection status is MQTTNotConnected. */ + currentPacketType = MQTT_PACKET_TYPE_PUBREC; + /* Set expected return values in the loop. */ + resetProcessLoopParams( &expectParams ); + expectParams.stateAfterDeserialize = MQTTPubRelSend; + expectParams.stateAfterSerialize = MQTTPubCompPending; + expectParams.serializeStatus = MQTTSuccess; + expectParams.processLoopStatus = MQTTStatusNotConnected; + context.connectStatus = MQTTNotConnected; + expectProcessLoopCalls( &context, &expectParams ); + context.connectStatus = MQTTConnected; + + /* Verify that MQTTStatusNotConnected propagated when receiving a any ACK, + * here PUBREC but thr connection status is MQTTNotConnected. */ + currentPacketType = MQTT_PACKET_TYPE_PUBREC; + /* Set expected return values in the loop. */ + resetProcessLoopParams( &expectParams ); + expectParams.stateAfterDeserialize = MQTTPubRelSend; + expectParams.stateAfterSerialize = MQTTPubCompPending; + expectParams.serializeStatus = MQTTSuccess; + expectParams.processLoopStatus = MQTTStatusDisconnectPending; + context.connectStatus = MQTTDisconnectPending; + expectProcessLoopCalls( &context, &expectParams ); + context.connectStatus = MQTTConnected; + /* Verify that MQTTBadResponse is propagated when deserialization fails upon * receiving a PUBACK. */ currentPacketType = MQTT_PACKET_TYPE_PUBACK; @@ -4635,6 +4922,58 @@ void test_MQTT_Subscribe_happy_path( void ) TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); } +/** + * @brief This test case verifies that MQTT_Subscribe does not return success if the connect status + * is anythin but MQTTConnected. + */ +void test_MQTT_Subscribe_happy_path_not_connected( void ) +{ + MQTTStatus_t mqttStatus; + MQTTContext_t context = { 0 }; + TransportInterface_t transport = { 0 }; + MQTTFixedBuffer_t networkBuffer = { 0 }; + MQTTSubscribeInfo_t subscribeInfo = { 0 }; + size_t remainingLength = MQTT_SAMPLE_REMAINING_LENGTH; + size_t packetSize = MQTT_SAMPLE_REMAINING_LENGTH; + MQTTPubAckInfo_t incomingRecords = { 0 }; + MQTTPubAckInfo_t outgoingRecords = { 0 }; + + setupTransportInterface( &transport ); + setupNetworkBuffer( &networkBuffer ); + setupSubscriptionInfo( &subscribeInfo ); + + /* Initialize context. */ + mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); + TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + + mqttStatus = MQTT_InitStatefulQoS( &context, + &outgoingRecords, 4, + &incomingRecords, 4 ); + TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + + /* Test 1 connect status is MQTTNotConnected */ + context.connectStatus=MQTTNotConnected; + /* Verify MQTTSuccess is returned with the following mocks. */ + MQTT_GetSubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); + MQTT_GetSubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); + MQTT_GetSubscribePacketSize_ReturnThruPtr_pRemainingLength( &remainingLength ); + MQTT_SerializeSubscribeHeader_Stub( MQTT_SerializeSubscribedHeader_cb ); + /* Expect the above calls when running MQTT_Subscribe. */ + mqttStatus = MQTT_Subscribe( &context, &subscribeInfo, 1, MQTT_FIRST_VALID_PACKET_ID ); + TEST_ASSERT_EQUAL( MQTTStatusNotConnected, mqttStatus ); + + /* Test 2 connect status is MQTTDisconnectPending*/ + context.connectStatus=MQTTDisconnectPending; + /* Verify MQTTSuccess is returned with the following mocks. */ + MQTT_GetSubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); + MQTT_GetSubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); + MQTT_GetSubscribePacketSize_ReturnThruPtr_pRemainingLength( &remainingLength ); + MQTT_SerializeSubscribeHeader_Stub( MQTT_SerializeSubscribedHeader_cb ); + /* Expect the above calls when running MQTT_Subscribe. */ + mqttStatus = MQTT_Subscribe( &context, &subscribeInfo, 1, MQTT_FIRST_VALID_PACKET_ID ); + TEST_ASSERT_EQUAL( MQTTStatusDisconnectPending, mqttStatus ); +} + /** * @brief This test case verifies that MQTT_Subscribe returns successfully * when valid parameters are passed and all bytes are sent. @@ -4856,6 +5195,44 @@ void test_MQTT_Subscribe_error_paths2( void ) TEST_ASSERT_EQUAL( MQTTSendFailed, mqttStatus ); } +/** + * @brief This test case verifies that MQTT_Subscribe returns MQTTSendFailed + * if transport interface fails to send and the connection status is converted to + * MQTTDisconnectPending + */ +void test_MQTT_Subscribe_error_paths_with_transport_failure( void ) +{ + MQTTStatus_t mqttStatus; + MQTTContext_t context = { 0 }; + TransportInterface_t transport = { 0 }; + MQTTFixedBuffer_t networkBuffer = { 0 }; + MQTTSubscribeInfo_t subscribeInfo = { 0 }; + size_t remainingLength = MQTT_SAMPLE_REMAINING_LENGTH; + size_t packetSize = MQTT_SAMPLE_REMAINING_LENGTH; + + /* Verify that an error is propagated when transport interface returns an error. */ + setupNetworkBuffer( &networkBuffer ); + setupSubscriptionInfo( &subscribeInfo ); + subscribeInfo.qos = MQTTQoS0; + setupTransportInterface( &transport ); + transport.writev = NULL; + transport.send = transportSendFailure; + + /* Initialize context. */ + mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); + TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + + context.connectStatus=MQTTConnected; + + MQTT_GetSubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); + MQTT_GetSubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); + MQTT_GetSubscribePacketSize_ReturnThruPtr_pRemainingLength( &remainingLength ); + MQTT_SerializeSubscribeHeader_Stub( MQTT_SerializeSubscribedHeader_cb ); + mqttStatus = MQTT_Subscribe( &context, &subscribeInfo, 1, MQTT_FIRST_VALID_PACKET_ID ); + TEST_ASSERT_EQUAL( MQTTSendFailed, mqttStatus ); + TEST_ASSERT_EQUAL( MQTTDisconnectPending, context.connectStatus ); +} + /** * @brief This test case verifies that MQTT_Subscribe returns MQTTSendFailed * if transport interface send fails and timer overflows. @@ -5025,6 +5402,50 @@ void test_MQTT_Unsubscribe_happy_path( void ) TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); } +/** + * @brief This test case verifies that MQTT_Unsubscribe does not return success + * when the connection status is anything but MQTTConnected + */ +void test_MQTT_Unsubscribe_not_connected( void ) +{ + MQTTStatus_t mqttStatus; + MQTTContext_t context = { 0 }; + TransportInterface_t transport = { 0 }; + MQTTFixedBuffer_t networkBuffer = { 0 }; + MQTTSubscribeInfo_t subscribeInfo = { 0 }; + size_t remainingLength = MQTT_SAMPLE_REMAINING_LENGTH; + size_t packetSize = MQTT_SAMPLE_REMAINING_LENGTH; + + setupTransportInterface( &transport ); + setupNetworkBuffer( &networkBuffer ); + setupSubscriptionInfo( &subscribeInfo ); + subscribeInfo.qos = MQTTQoS0; + + /* Initialize context. */ + mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); + TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + + /* Test 1 Connection status is MQTTNotConnected*/ + context.connectStatus=MQTTNotConnected; + /* Verify MQTTSuccess is returned with the following mocks. */ + MQTT_GetUnsubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); + MQTT_GetUnsubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); + MQTT_GetUnsubscribePacketSize_ReturnThruPtr_pRemainingLength( &remainingLength ); + /* Expect the above calls when running MQTT_Unsubscribe. */ + mqttStatus = MQTT_Unsubscribe( &context, &subscribeInfo, 1, MQTT_FIRST_VALID_PACKET_ID ); + TEST_ASSERT_EQUAL( MQTTStatusNotConnected, mqttStatus ); + + /* Test 2 Connection status is MQTTDisconnectPending*/ + context.connectStatus=MQTTDisconnectPending; + /* Verify MQTTSuccess is returned with the following mocks. */ + MQTT_GetUnsubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); + MQTT_GetUnsubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); + MQTT_GetUnsubscribePacketSize_ReturnThruPtr_pRemainingLength( &remainingLength ); + /* Expect the above calls when running MQTT_Unsubscribe. */ + mqttStatus = MQTT_Unsubscribe( &context, &subscribeInfo, 1, MQTT_FIRST_VALID_PACKET_ID ); + TEST_ASSERT_EQUAL( MQTTStatusDisconnectPending, mqttStatus ); +} + /** * @brief This test case verifies that MQTT_Subscribe returns successfully * when valid parameters are passed and all bytes are sent. @@ -5302,6 +5723,46 @@ void test_MQTT_Ping_happy_path( void ) TEST_ASSERT_TRUE( context.waitingForPingResp ); } +/** + * @brief This test case verifies that MQTT_Ping does not returns success + * if the connection status is anything but MQTTConnect. + */ +void test_MQTT_Ping_not_connected( void ) +{ + MQTTStatus_t mqttStatus; + MQTTContext_t context = { 0 }; + TransportInterface_t transport = { 0 }; + MQTTFixedBuffer_t networkBuffer = { 0 }; + size_t pingreqSize = MQTT_PACKET_PINGREQ_SIZE; + + setupTransportInterface( &transport ); + setupNetworkBuffer( &networkBuffer ); + + /* Initialize context. */ + mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); + TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + + /* Test 1 when the connection status is MQTTNotConnected*/ + context.connectStatus=MQTTNotConnected; + /* Verify MQTTSuccess is returned. */ + MQTT_GetPingreqPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); + MQTT_GetPingreqPacketSize_ReturnThruPtr_pPacketSize( &pingreqSize ); + MQTT_SerializePingreq_ExpectAnyArgsAndReturn( MQTTSuccess ); + /* Expect the above calls when running MQTT_Ping. */ + mqttStatus = MQTT_Ping( &context ); + TEST_ASSERT_EQUAL( MQTTStatusNotConnected, mqttStatus ); + + /* Test 2 when the connection status is MQTTDisconnectPending*/ + context.connectStatus=MQTTDisconnectPending; + /* Verify MQTTSuccess is returned. */ + MQTT_GetPingreqPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); + MQTT_GetPingreqPacketSize_ReturnThruPtr_pPacketSize( &pingreqSize ); + MQTT_SerializePingreq_ExpectAnyArgsAndReturn( MQTTSuccess ); + /* Expect the above calls when running MQTT_Ping. */ + mqttStatus = MQTT_Ping( &context ); + TEST_ASSERT_EQUAL( MQTTStatusDisconnectPending, mqttStatus ); +} + /** * @brief This test case verifies that MQTT_Ping returns MQTTSendFailed * if transport interface send returns an error. diff --git a/tools/cmock/coverage.cmake b/tools/cmock/coverage.cmake index e035ea4c7..46d24c66f 100644 --- a/tools/cmock/coverage.cmake +++ b/tools/cmock/coverage.cmake @@ -15,8 +15,10 @@ execute_process( COMMAND lcov --directory ${CMAKE_BINARY_DIR} --initial --capture --rc lcov_branch_coverage=1 - --rc genhtml_branch_coverage=1 + --output-file=${CMAKE_BINARY_DIR}/base_coverage.info + --include "*source*" + ) file(GLOB files "${CMAKE_BINARY_DIR}/bin/tests/*") @@ -46,10 +48,11 @@ execute_process(COMMAND ruby execute_process( COMMAND lcov --capture --rc lcov_branch_coverage=1 - --rc genhtml_branch_coverage=1 + --base-directory ${CMAKE_BINARY_DIR} --directory ${CMAKE_BINARY_DIR} --output-file ${CMAKE_BINARY_DIR}/second_coverage.info + --include "*source*" ) # combile baseline results (zeros) with the one after running the tests @@ -59,8 +62,9 @@ execute_process( --add-tracefile ${CMAKE_BINARY_DIR}/base_coverage.info --add-tracefile ${CMAKE_BINARY_DIR}/second_coverage.info --output-file ${CMAKE_BINARY_DIR}/coverage.info - --no-external + --rc lcov_branch_coverage=1 + --include "*source*" ) execute_process( COMMAND genhtml --rc lcov_branch_coverage=1 From 0d7dade8bda7fa8f18947f0e0a349e993cef3475 Mon Sep 17 00:00:00 2001 From: Dakshit Babbar Date: Fri, 13 Sep 2024 00:31:29 +0530 Subject: [PATCH 10/17] Resolve Formatting and PR Comments --- source/core_mqtt.c | 8 ++++---- test/unit-test/core_mqtt_utest.c | 34 ++++++++++++++++---------------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/source/core_mqtt.c b/source/core_mqtt.c index 9da735c7a..3c2fe649d 100644 --- a/source/core_mqtt.c +++ b/source/core_mqtt.c @@ -2715,6 +2715,8 @@ MQTTStatus_t MQTT_CheckConnectStatus( MQTTContext_t * pContext ) connectStatus = pContext->connectStatus; + MQTT_POST_STATE_UPDATE_HOOK( pContext ); + switch( connectStatus ) { case MQTTConnected: @@ -2729,8 +2731,6 @@ MQTTStatus_t MQTT_CheckConnectStatus( MQTTContext_t * pContext ) status = MQTTStatusNotConnected; break; } - - MQTT_POST_STATE_UPDATE_HOOK( pContext ); } return status; @@ -2819,8 +2819,8 @@ MQTTStatus_t MQTT_Connect( MQTTContext_t * pContext, 0x00, pContext->outgoingPublishRecordMaxCount * sizeof( *pContext->outgoingPublishRecords ) ); } - - if( *pSessionPresent != true && pContext->incomingPublishRecordMaxCount > 0U ) + + if( ( *pSessionPresent != true ) && ( pContext->incomingPublishRecordMaxCount > 0U ) ) { ( void ) memset( pContext->incomingPublishRecords, 0x00, diff --git a/test/unit-test/core_mqtt_utest.c b/test/unit-test/core_mqtt_utest.c index 234ee7777..447d25c80 100644 --- a/test/unit-test/core_mqtt_utest.c +++ b/test/unit-test/core_mqtt_utest.c @@ -1062,8 +1062,8 @@ static void expectProcessLoopCalls( MQTTContext_t * const pContext, } } - if( expectMoreCalls && (pContext->connectStatus != MQTTConnected) ) - { + if( expectMoreCalls && ( pContext->connectStatus != MQTTConnected ) ) + { expectMoreCalls = false; } @@ -1170,7 +1170,7 @@ void test_MQTT_CheckConnectStatus_invalid_params( void ) { MQTTStatus_t mqttStatus = { 0 }; - mqttStatus = MQTT_CheckConnectStatus(NULL); + mqttStatus = MQTT_CheckConnectStatus( NULL ); TEST_ASSERT_EQUAL( MQTTBadParameter, mqttStatus ); } @@ -1188,15 +1188,15 @@ void test_MQTT_CheckConnectStatus_return_correct_status( void ) setupNetworkBuffer( &networkBuffer ); context.connectStatus = MQTTConnected; - mqttStatus = MQTT_CheckConnectStatus(&context); + mqttStatus = MQTT_CheckConnectStatus( &context ); TEST_ASSERT_EQUAL( MQTTStatusConnected, mqttStatus ); context.connectStatus = MQTTNotConnected; - mqttStatus = MQTT_CheckConnectStatus(&context); + mqttStatus = MQTT_CheckConnectStatus( &context ); TEST_ASSERT_EQUAL( MQTTStatusNotConnected, mqttStatus ); context.connectStatus = MQTTDisconnectPending; - mqttStatus = MQTT_CheckConnectStatus(&context); + mqttStatus = MQTT_CheckConnectStatus( &context ); TEST_ASSERT_EQUAL( MQTTStatusDisconnectPending, mqttStatus ); } @@ -2595,15 +2595,15 @@ void test_MQTT_Publish_not_connected( void ) memset( &publishInfo, 0x0, sizeof( publishInfo ) ); MQTT_Init( &mqttContext, &transport, getTime, eventCallback, &networkBuffer ); - /* Test 1 connecttion status is MQTTNotConnected */ - mqttContext.connectStatus=MQTTNotConnected; + /* Test 1 connection status is MQTTNotConnected */ + mqttContext.connectStatus = MQTTNotConnected; MQTT_GetPublishPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_SerializePublishHeaderWithoutTopic_ExpectAnyArgsAndReturn( MQTTSuccess ); status = MQTT_Publish( &mqttContext, &publishInfo, 10 ); TEST_ASSERT_EQUAL_INT( MQTTStatusNotConnected, status ); - /* Test 2 connecttion status is MQTTDisconnectPending */ - mqttContext.connectStatus=MQTTDisconnectPending; + /* Test 2 connection status is MQTTDisconnectPending */ + mqttContext.connectStatus = MQTTDisconnectPending; MQTT_GetPublishPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_SerializePublishHeaderWithoutTopic_ExpectAnyArgsAndReturn( MQTTSuccess ); status = MQTT_Publish( &mqttContext, &publishInfo, 10 ); @@ -4952,7 +4952,7 @@ void test_MQTT_Subscribe_happy_path_not_connected( void ) TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); /* Test 1 connect status is MQTTNotConnected */ - context.connectStatus=MQTTNotConnected; + context.connectStatus = MQTTNotConnected; /* Verify MQTTSuccess is returned with the following mocks. */ MQTT_GetSubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetSubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); @@ -4963,7 +4963,7 @@ void test_MQTT_Subscribe_happy_path_not_connected( void ) TEST_ASSERT_EQUAL( MQTTStatusNotConnected, mqttStatus ); /* Test 2 connect status is MQTTDisconnectPending*/ - context.connectStatus=MQTTDisconnectPending; + context.connectStatus = MQTTDisconnectPending; /* Verify MQTTSuccess is returned with the following mocks. */ MQTT_GetSubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetSubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); @@ -5222,7 +5222,7 @@ void test_MQTT_Subscribe_error_paths_with_transport_failure( void ) mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer ); TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); - context.connectStatus=MQTTConnected; + context.connectStatus = MQTTConnected; MQTT_GetSubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetSubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); @@ -5426,7 +5426,7 @@ void test_MQTT_Unsubscribe_not_connected( void ) TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); /* Test 1 Connection status is MQTTNotConnected*/ - context.connectStatus=MQTTNotConnected; + context.connectStatus = MQTTNotConnected; /* Verify MQTTSuccess is returned with the following mocks. */ MQTT_GetUnsubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetUnsubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); @@ -5436,7 +5436,7 @@ void test_MQTT_Unsubscribe_not_connected( void ) TEST_ASSERT_EQUAL( MQTTStatusNotConnected, mqttStatus ); /* Test 2 Connection status is MQTTDisconnectPending*/ - context.connectStatus=MQTTDisconnectPending; + context.connectStatus = MQTTDisconnectPending; /* Verify MQTTSuccess is returned with the following mocks. */ MQTT_GetUnsubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetUnsubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); @@ -5743,7 +5743,7 @@ void test_MQTT_Ping_not_connected( void ) TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); /* Test 1 when the connection status is MQTTNotConnected*/ - context.connectStatus=MQTTNotConnected; + context.connectStatus = MQTTNotConnected; /* Verify MQTTSuccess is returned. */ MQTT_GetPingreqPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetPingreqPacketSize_ReturnThruPtr_pPacketSize( &pingreqSize ); @@ -5753,7 +5753,7 @@ void test_MQTT_Ping_not_connected( void ) TEST_ASSERT_EQUAL( MQTTStatusNotConnected, mqttStatus ); /* Test 2 when the connection status is MQTTDisconnectPending*/ - context.connectStatus=MQTTDisconnectPending; + context.connectStatus = MQTTDisconnectPending; /* Verify MQTTSuccess is returned. */ MQTT_GetPingreqPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetPingreqPacketSize_ReturnThruPtr_pPacketSize( &pingreqSize ); From 5c3524e6166b545077a876b2b958ddf0ec982d60 Mon Sep 17 00:00:00 2001 From: Dakshit Babbar Date: Fri, 13 Sep 2024 10:53:31 +0530 Subject: [PATCH 11/17] Implement critical section without mutexTaken flag --- source/core_mqtt.c | 385 ++++++++++++++++++++++----------------------- 1 file changed, 186 insertions(+), 199 deletions(-) diff --git a/source/core_mqtt.c b/source/core_mqtt.c index 3c2fe649d..e8b62f71c 100644 --- a/source/core_mqtt.c +++ b/source/core_mqtt.c @@ -440,6 +440,15 @@ static MQTTStatus_t receiveConnack( MQTTContext_t * pContext, */ static MQTTStatus_t handleUncleanSessionResumption( MQTTContext_t * pContext ); +/** + * @brief Clears existing state records for a clean session. + * + * @param[in] pContext Initialized MQTT context. + * + * @return void. + */ +static void handleCleanSession( MQTTContext_t * pContext ); + /** * @brief Send the publish packet without copying the topic string and payload in * the buffer. @@ -1288,7 +1297,6 @@ static MQTTStatus_t sendPublishAcks( MQTTContext_t * pContext, MQTTPubAckType_t packetType; MQTTFixedBuffer_t localBuffer; MQTTConnectionStatus_t connectStatus; - bool stateUpdateHookExecuted = false; uint8_t pubAckPacket[ MQTT_PUBLISH_ACK_PACKET_SIZE ]; localBuffer.pBuffer = pubAckPacket; @@ -1310,33 +1318,28 @@ static MQTTStatus_t sendPublishAcks( MQTTContext_t * pContext, { MQTT_PRE_STATE_UPDATE_HOOK( pContext ); - stateUpdateHookExecuted = true; - connectStatus = pContext->connectStatus; if( connectStatus != MQTTConnected ) { status = ( connectStatus == MQTTNotConnected ) ? MQTTStatusNotConnected : MQTTStatusDisconnectPending; } - } - - if( status == MQTTSuccess ) - { - /* Here, we are not using the vector approach for efficiency. There is just one buffer - * to be sent which can be achieved with a normal send call. */ - sendResult = sendBuffer( pContext, - localBuffer.pBuffer, - MQTT_PUBLISH_ACK_PACKET_SIZE ); + - if( sendResult < ( int32_t ) MQTT_PUBLISH_ACK_PACKET_SIZE ) + if( status == MQTTSuccess ) { - status = MQTTSendFailed; + /* Here, we are not using the vector approach for efficiency. There is just one buffer + * to be sent which can be achieved with a normal send call. */ + sendResult = sendBuffer( pContext, + localBuffer.pBuffer, + MQTT_PUBLISH_ACK_PACKET_SIZE ); + + if( sendResult < ( int32_t ) MQTT_PUBLISH_ACK_PACKET_SIZE ) + { + status = MQTTSendFailed; + } } - } - if( stateUpdateHookExecuted == true ) - { - /* Regardless of the status, if the mutex was taken, then it should be relinquished. */ MQTT_POST_STATE_UPDATE_HOOK( pContext ); } @@ -2503,6 +2506,32 @@ static MQTTStatus_t handleUncleanSessionResumption( MQTTContext_t * pContext ) return status; } +static void handleCleanSession( MQTTContext_t * pContext ) +{ + assert( pContext != NULL ); + + /* Reset the index and clear the buffer when a new session is established. */ + pContext->index = 0; + ( void ) memset( pContext->networkBuffer.pBuffer, 0, pContext->networkBuffer.size ); + + if( pContext->outgoingPublishRecordMaxCount > 0U ) + { + /* Clear any existing records if a new session is established. */ + ( void ) memset( pContext->outgoingPublishRecords, + 0x00, + pContext->outgoingPublishRecordMaxCount * sizeof( *pContext->outgoingPublishRecords ) ); + } + + if( pContext->incomingPublishRecordMaxCount > 0U ) + { + ( void ) memset( pContext->incomingPublishRecords, + 0x00, + pContext->incomingPublishRecordMaxCount * sizeof( *pContext->incomingPublishRecords ) ); + } + + return; +} + static MQTTStatus_t validatePublishParams( const MQTTContext_t * pContext, const MQTTPublishInfo_t * pPublishInfo, uint16_t packetId ) @@ -2748,7 +2777,6 @@ MQTTStatus_t MQTT_Connect( MQTTContext_t * pContext, MQTTStatus_t status = MQTTSuccess; MQTTPacketInfo_t incomingPacket = { 0 }; MQTTConnectionStatus_t connectStatus; - bool stateUpdateHookExecuted = false; incomingPacket.type = ( uint8_t ) 0; @@ -2778,42 +2806,16 @@ MQTTStatus_t MQTT_Connect( MQTTContext_t * pContext, { MQTT_PRE_STATE_UPDATE_HOOK( pContext ); - stateUpdateHookExecuted = true; - connectStatus = pContext->connectStatus; if( connectStatus != MQTTNotConnected ) { status = ( connectStatus == MQTTConnected ) ? MQTTStatusConnected : MQTTStatusDisconnectPending; } - } - if( status == MQTTSuccess ) - { - status = sendConnectWithoutCopy( pContext, - pConnectInfo, - pWillInfo, - remainingLength ); - } - - /* Read CONNACK from transport layer. */ - if( status == MQTTSuccess ) - { - status = receiveConnack( pContext, - timeoutMs, - pConnectInfo->cleanSession, - &incomingPacket, - pSessionPresent ); - } - - if( status == MQTTSuccess ) - { - /* Reset the index and clear the buffer when a new session is established. */ - pContext->index = 0; - ( void ) memset( pContext->networkBuffer.pBuffer, 0, pContext->networkBuffer.size ); - - if( ( *pSessionPresent != true ) && ( pContext->outgoingPublishRecordMaxCount > 0U ) ) + if( status == MQTTSuccess ) { +<<<<<<< HEAD /* Clear any existing records if a new session is established. */ ( void ) memset( pContext->outgoingPublishRecords, 0x00, @@ -2825,18 +2827,38 @@ MQTTStatus_t MQTT_Connect( MQTTContext_t * pContext, ( void ) memset( pContext->incomingPublishRecords, 0x00, pContext->incomingPublishRecordMaxCount * sizeof( *pContext->incomingPublishRecords ) ); +======= + status = sendConnectWithoutCopy( pContext, + pConnectInfo, + pWillInfo, + remainingLength ); +>>>>>>> 62d8f5a (Implement critical section without mutexTaken flag) } - pContext->connectStatus = MQTTConnected; - /* Initialize keep-alive fields after a successful connection. */ - pContext->keepAliveIntervalSec = pConnectInfo->keepAliveSeconds; - pContext->waitingForPingResp = false; - pContext->pingReqSendTimeMs = 0U; - } + /* Read CONNACK from transport layer. */ + if( status == MQTTSuccess ) + { + status = receiveConnack( pContext, + timeoutMs, + pConnectInfo->cleanSession, + &incomingPacket, + pSessionPresent ); + } + + if( (status == MQTTSuccess) && ( *pSessionPresent != true ) ) + { + handleCleanSession( pContext ); + } + + if( status == MQTTSuccess ) + { + pContext->connectStatus = MQTTConnected; + /* Initialize keep-alive fields after a successful connection. */ + pContext->keepAliveIntervalSec = pConnectInfo->keepAliveSeconds; + pContext->waitingForPingResp = false; + pContext->pingReqSendTimeMs = 0U; + } - if( stateUpdateHookExecuted == true ) - { - /* Regardless of the status, if the mutex was taken, then it should be relinquished. */ MQTT_POST_STATE_UPDATE_HOOK( pContext ); } @@ -2885,7 +2907,6 @@ MQTTStatus_t MQTT_Subscribe( MQTTContext_t * pContext, size_t subscriptionCount, uint16_t packetId ) { - bool stateUpdateHookExecuted = false; MQTTConnectionStatus_t connectStatus; size_t remainingLength = 0UL, packetSize = 0UL; @@ -2911,29 +2932,24 @@ MQTTStatus_t MQTT_Subscribe( MQTTContext_t * pContext, { MQTT_PRE_STATE_UPDATE_HOOK( pContext ); - stateUpdateHookExecuted = true; - connectStatus = pContext->connectStatus; if( connectStatus != MQTTConnected ) { status = ( connectStatus == MQTTNotConnected ) ? MQTTStatusNotConnected : MQTTStatusDisconnectPending; } - } + - if( status == MQTTSuccess ) - { - /* Send MQTT SUBSCRIBE packet. */ - status = sendSubscribeWithoutCopy( pContext, - pSubscriptionList, - subscriptionCount, - packetId, - remainingLength ); - } + if( status == MQTTSuccess ) + { + /* Send MQTT SUBSCRIBE packet. */ + status = sendSubscribeWithoutCopy( pContext, + pSubscriptionList, + subscriptionCount, + packetId, + remainingLength ); + } - if( stateUpdateHookExecuted == true ) - { - /* Regardless of the status, if the mutex was taken then it should be relinquished. */ MQTT_POST_STATE_UPDATE_HOOK( pContext ); } @@ -2951,7 +2967,6 @@ MQTTStatus_t MQTT_Publish( MQTTContext_t * pContext, size_t packetSize = 0UL; MQTTPublishState_t publishStatus = MQTTStateNull; MQTTConnectionStatus_t connectStatus; - bool stateUpdateHookExecuted = false; /* Maximum number of bytes required by the 'fixed' part of the PUBLISH * packet header according to the MQTT specifications. @@ -2991,72 +3006,65 @@ MQTTStatus_t MQTT_Publish( MQTTContext_t * pContext, * packet. */ MQTT_PRE_STATE_UPDATE_HOOK( pContext ); - stateUpdateHookExecuted = true; - connectStatus = pContext->connectStatus; if( connectStatus != MQTTConnected ) { status = ( connectStatus == MQTTNotConnected ) ? MQTTStatusNotConnected : MQTTStatusDisconnectPending; } - } - if( ( status == MQTTSuccess ) && ( pPublishInfo->qos > MQTTQoS0 ) ) - { - /* Set the flag so that the corresponding hook can be called later. */ + if( ( status == MQTTSuccess ) && ( pPublishInfo->qos > MQTTQoS0 ) ) + { + /* Set the flag so that the corresponding hook can be called later. */ - status = MQTT_ReserveState( pContext, - packetId, - pPublishInfo->qos ); + status = MQTT_ReserveState( pContext, + packetId, + pPublishInfo->qos ); - /* State already exists for a duplicate packet. - * If a state doesn't exist, it will be handled as a new publish in - * state engine. */ - if( ( status == MQTTStateCollision ) && ( pPublishInfo->dup == true ) ) - { - status = MQTTSuccess; + /* State already exists for a duplicate packet. + * If a state doesn't exist, it will be handled as a new publish in + * state engine. */ + if( ( status == MQTTStateCollision ) && ( pPublishInfo->dup == true ) ) + { + status = MQTTSuccess; + } } - } - if( status == MQTTSuccess ) - { - status = sendPublishWithoutCopy( pContext, - pPublishInfo, - mqttHeader, - headerSize, - packetId ); - } - - if( ( status == MQTTSuccess ) && - ( pPublishInfo->qos > MQTTQoS0 ) ) - { - /* Update state machine after PUBLISH is sent. - * Only to be done for QoS1 or QoS2. */ - status = MQTT_UpdateStatePublish( pContext, - packetId, - MQTT_SEND, - pPublishInfo->qos, - &publishStatus ); + if( status == MQTTSuccess ) + { + status = sendPublishWithoutCopy( pContext, + pPublishInfo, + mqttHeader, + headerSize, + packetId ); + } - if( status != MQTTSuccess ) + if( ( status == MQTTSuccess ) && + ( pPublishInfo->qos > MQTTQoS0 ) ) { - LogError( ( "Update state for publish failed with status %s." - " However PUBLISH packet was sent to the broker." - " Any further handling of ACKs for the packet Id" - " will fail.", - MQTT_Status_strerror( status ) ) ); + /* Update state machine after PUBLISH is sent. + * Only to be done for QoS1 or QoS2. */ + status = MQTT_UpdateStatePublish( pContext, + packetId, + MQTT_SEND, + pPublishInfo->qos, + &publishStatus ); + + if( status != MQTTSuccess ) + { + LogError( ( "Update state for publish failed with status %s." + " However PUBLISH packet was sent to the broker." + " Any further handling of ACKs for the packet Id" + " will fail.", + MQTT_Status_strerror( status ) ) ); + } } - } - /* mutex should be released and not before updating the state - * because we need to make sure that the state is updated - * after sending the publish packet, before the receive - * loop receives ack for this and would want to update its state - */ - if( stateUpdateHookExecuted == true ) - { - /* Regardless of the status, if the mutex was taken due to the - * packet being of QoS > QoS0, then it should be relinquished. */ + /* mutex should be released and not before updating the state + * because we need to make sure that the state is updated + * after sending the publish packet, before the receive + * loop receives ack for this and would want to update its state + */ MQTT_POST_STATE_UPDATE_HOOK( pContext ); } @@ -3080,7 +3088,6 @@ MQTTStatus_t MQTT_Ping( MQTTContext_t * pContext ) uint8_t pingreqPacket[ 2U ]; MQTTFixedBuffer_t localBuffer; MQTTConnectionStatus_t connectStatus; - bool stateUpdateHookExecuted = false; localBuffer.pBuffer = pingreqPacket; localBuffer.size = sizeof( pingreqPacket ); @@ -3121,44 +3128,38 @@ MQTTStatus_t MQTT_Ping( MQTTContext_t * pContext ) MQTT_PRE_STATE_UPDATE_HOOK( pContext ); - stateUpdateHookExecuted = true; - connectStatus = pContext->connectStatus; if( connectStatus != MQTTConnected ) { status = ( connectStatus == MQTTNotConnected ) ? MQTTStatusNotConnected : MQTTStatusDisconnectPending; } - } - if( status == MQTTSuccess ) - { - /* Send the serialized PINGREQ packet to transport layer. - * Here, we do not use the vectored IO approach for efficiency as the - * Ping packet does not have numerous fields which need to be copied - * from the user provided buffers. Thus it can be sent directly. */ - sendResult = sendBuffer( pContext, - localBuffer.pBuffer, - packetSize ); - - /* It is an error to not send the entire PINGREQ packet. */ - if( sendResult < ( int32_t ) packetSize ) - { - LogError( ( "Transport send failed for PINGREQ packet." ) ); - status = MQTTSendFailed; - } - else + if( status == MQTTSuccess ) { - pContext->pingReqSendTimeMs = pContext->lastPacketTxTime; - pContext->waitingForPingResp = true; - LogDebug( ( "Sent %ld bytes of PINGREQ packet.", - ( long int ) sendResult ) ); + /* Send the serialized PINGREQ packet to transport layer. + * Here, we do not use the vectored IO approach for efficiency as the + * Ping packet does not have numerous fields which need to be copied + * from the user provided buffers. Thus it can be sent directly. */ + sendResult = sendBuffer( pContext, + localBuffer.pBuffer, + packetSize ); + + /* It is an error to not send the entire PINGREQ packet. */ + if( sendResult < ( int32_t ) packetSize ) + { + LogError( ( "Transport send failed for PINGREQ packet." ) ); + status = MQTTSendFailed; + } + else + { + pContext->pingReqSendTimeMs = pContext->lastPacketTxTime; + pContext->waitingForPingResp = true; + LogDebug( ( "Sent %ld bytes of PINGREQ packet.", + ( long int ) sendResult ) ); + } } - } - if( stateUpdateHookExecuted == true ) - { - /* Regardless of the status, if the mutex was taken, then it should be relinquished. */ MQTT_POST_STATE_UPDATE_HOOK( pContext ); } @@ -3172,7 +3173,6 @@ MQTTStatus_t MQTT_Unsubscribe( MQTTContext_t * pContext, size_t subscriptionCount, uint16_t packetId ) { - bool stateUpdateHookExecuted = false; MQTTConnectionStatus_t connectStatus; size_t remainingLength = 0UL, packetSize = 0UL; @@ -3199,28 +3199,22 @@ MQTTStatus_t MQTT_Unsubscribe( MQTTContext_t * pContext, /* Take the mutex because the below call should not be interrupted. */ MQTT_PRE_STATE_UPDATE_HOOK( pContext ); - stateUpdateHookExecuted = true; - connectStatus = pContext->connectStatus; if( connectStatus != MQTTConnected ) { status = ( connectStatus == MQTTNotConnected ) ? MQTTStatusNotConnected : MQTTStatusDisconnectPending; } - } - if( status == MQTTSuccess ) - { - status = sendUnsubscribeWithoutCopy( pContext, - pSubscriptionList, - subscriptionCount, - packetId, - remainingLength ); - } + if( status == MQTTSuccess ) + { + status = sendUnsubscribeWithoutCopy( pContext, + pSubscriptionList, + subscriptionCount, + packetId, + remainingLength ); + } - if( stateUpdateHookExecuted == true ) - { - /* Regardless of the status, if the mutex was taken, then it should be relinquished. */ MQTT_POST_STATE_UPDATE_HOOK( pContext ); } @@ -3237,7 +3231,6 @@ MQTTStatus_t MQTT_Disconnect( MQTTContext_t * pContext ) MQTTFixedBuffer_t localBuffer; uint8_t disconnectPacket[ 2U ]; MQTTConnectionStatus_t connectStatus; - bool stateUpdateHookExecuted = false; localBuffer.pBuffer = disconnectPacket; localBuffer.size = 2U; @@ -3268,49 +3261,43 @@ MQTTStatus_t MQTT_Disconnect( MQTTContext_t * pContext ) /* Take the mutex because the below call should not be interrupted. */ MQTT_PRE_STATE_UPDATE_HOOK( pContext ); - stateUpdateHookExecuted = true; - connectStatus = pContext->connectStatus; if( connectStatus == MQTTNotConnected ) { status = MQTTStatusNotConnected; } - } - if( status == MQTTSuccess ) - { - LogInfo( ( "Disconnected from the broker." ) ); - pContext->connectStatus = MQTTNotConnected; + if( status == MQTTSuccess ) + { + LogInfo( ( "Disconnected from the broker." ) ); + pContext->connectStatus = MQTTNotConnected; - /* Reset the index and clean the buffer on a successful disconnect. */ - pContext->index = 0; - ( void ) memset( pContext->networkBuffer.pBuffer, 0, pContext->networkBuffer.size ); + /* Reset the index and clean the buffer on a successful disconnect. */ + pContext->index = 0; + ( void ) memset( pContext->networkBuffer.pBuffer, 0, pContext->networkBuffer.size ); - LogError( ( "MQTT Connection Disconnected Successfully" ) ); + LogError( ( "MQTT Connection Disconnected Successfully" ) ); - /* Here we do not use vectors as the disconnect packet has fixed fields - * which do not reside in user provided buffers. Thus, it can be sent - * using a simple send call. */ - sendResult = sendBuffer( pContext, - localBuffer.pBuffer, - packetSize ); + /* Here we do not use vectors as the disconnect packet has fixed fields + * which do not reside in user provided buffers. Thus, it can be sent + * using a simple send call. */ + sendResult = sendBuffer( pContext, + localBuffer.pBuffer, + packetSize ); - if( sendResult < ( int32_t ) packetSize ) - { - LogError( ( "Transport send failed for DISCONNECT packet." ) ); - status = MQTTSendFailed; - } - else - { - LogDebug( ( "Sent %ld bytes of DISCONNECT packet.", - ( long int ) sendResult ) ); + if( sendResult < ( int32_t ) packetSize ) + { + LogError( ( "Transport send failed for DISCONNECT packet." ) ); + status = MQTTSendFailed; + } + else + { + LogDebug( ( "Sent %ld bytes of DISCONNECT packet.", + ( long int ) sendResult ) ); + } } - } - if( stateUpdateHookExecuted == true ) - { - /* Regardless of the status, if the mutex was taken, then it should be relinquished. */ MQTT_POST_STATE_UPDATE_HOOK( pContext ); } From 2382195ba8a57f099bc9cea4659b44fabe138923 Mon Sep 17 00:00:00 2001 From: Dakshit Babbar Date: Fri, 13 Sep 2024 11:39:42 +0530 Subject: [PATCH 12/17] Rectify conflicts --- source/core_mqtt.c | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/source/core_mqtt.c b/source/core_mqtt.c index e8b62f71c..0f449af88 100644 --- a/source/core_mqtt.c +++ b/source/core_mqtt.c @@ -2815,24 +2815,10 @@ MQTTStatus_t MQTT_Connect( MQTTContext_t * pContext, if( status == MQTTSuccess ) { -<<<<<<< HEAD - /* Clear any existing records if a new session is established. */ - ( void ) memset( pContext->outgoingPublishRecords, - 0x00, - pContext->outgoingPublishRecordMaxCount * sizeof( *pContext->outgoingPublishRecords ) ); - } - - if( ( *pSessionPresent != true ) && ( pContext->incomingPublishRecordMaxCount > 0U ) ) - { - ( void ) memset( pContext->incomingPublishRecords, - 0x00, - pContext->incomingPublishRecordMaxCount * sizeof( *pContext->incomingPublishRecords ) ); -======= status = sendConnectWithoutCopy( pContext, pConnectInfo, pWillInfo, remainingLength ); ->>>>>>> 62d8f5a (Implement critical section without mutexTaken flag) } /* Read CONNACK from transport layer. */ From b48537549f42821f1d365a2a9d83ba19ee2ad9e5 Mon Sep 17 00:00:00 2001 From: Dakshit Babbar Date: Fri, 13 Sep 2024 21:57:22 +0530 Subject: [PATCH 13/17] Resolve issues with failing unit tests on Mac and Memory Statistics --- docs/doxygen/include/size_table.md | 8 ++++---- source/core_mqtt.c | 2 -- tools/cmock/project.yml | 1 - 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/docs/doxygen/include/size_table.md b/docs/doxygen/include/size_table.md index 10de57852..1fa92ff64 100644 --- a/docs/doxygen/include/size_table.md +++ b/docs/doxygen/include/size_table.md @@ -9,8 +9,8 @@ core_mqtt.c -
4.1K
-
3.5K
+
4.4K
+
3.8K
core_mqtt_state.c @@ -24,7 +24,7 @@ Total estimates -
8.6K
 -
7.0K
 +
8.9K
 +
7.3K
 diff --git a/source/core_mqtt.c b/source/core_mqtt.c index 0f449af88..82c479df9 100644 --- a/source/core_mqtt.c +++ b/source/core_mqtt.c @@ -444,8 +444,6 @@ static MQTTStatus_t handleUncleanSessionResumption( MQTTContext_t * pContext ); * @brief Clears existing state records for a clean session. * * @param[in] pContext Initialized MQTT context. - * - * @return void. */ static void handleCleanSession( MQTTContext_t * pContext ); diff --git a/tools/cmock/project.yml b/tools/cmock/project.yml index 560136ebf..b243af6e0 100644 --- a/tools/cmock/project.yml +++ b/tools/cmock/project.yml @@ -24,5 +24,4 @@ :includes_c_post_header: - :treat_externs: :exclude # Now the extern-ed functions will be mocked. - :weak: __attribute__((weak)) :treat_externs: :include From 8ca97abda186da79cfde231377c1683f4596e04a Mon Sep 17 00:00:00 2001 From: Dakshit Babbar Date: Thu, 19 Sep 2024 13:58:55 +0530 Subject: [PATCH 14/17] Resolve Formatting --- source/core_mqtt.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/source/core_mqtt.c b/source/core_mqtt.c index 9177b2b89..cbad607a8 100644 --- a/source/core_mqtt.c +++ b/source/core_mqtt.c @@ -1314,7 +1314,6 @@ static MQTTStatus_t sendPublishAcks( MQTTContext_t * pContext, { status = ( connectStatus == MQTTNotConnected ) ? MQTTStatusNotConnected : MQTTStatusDisconnectPending; } - if( status == MQTTSuccess ) { @@ -2511,15 +2510,13 @@ static void handleCleanSession( MQTTContext_t * pContext ) 0x00, pContext->outgoingPublishRecordMaxCount * sizeof( *pContext->outgoingPublishRecords ) ); } - + if( pContext->incomingPublishRecordMaxCount > 0U ) { ( void ) memset( pContext->incomingPublishRecords, 0x00, pContext->incomingPublishRecordMaxCount * sizeof( *pContext->incomingPublishRecords ) ); } - - return; } static MQTTStatus_t validatePublishParams( const MQTTContext_t * pContext, @@ -2821,7 +2818,7 @@ MQTTStatus_t MQTT_Connect( MQTTContext_t * pContext, pSessionPresent ); } - if( (status == MQTTSuccess) && ( *pSessionPresent != true ) ) + if( ( status == MQTTSuccess ) && ( *pSessionPresent != true ) ) { handleCleanSession( pContext ); } @@ -2914,7 +2911,6 @@ MQTTStatus_t MQTT_Subscribe( MQTTContext_t * pContext, { status = ( connectStatus == MQTTNotConnected ) ? MQTTStatusNotConnected : MQTTStatusDisconnectPending; } - if( status == MQTTSuccess ) { From 819808f629b3e12a835846115f31bb7cabca4479 Mon Sep 17 00:00:00 2001 From: Dakshit Babbar Date: Tue, 24 Sep 2024 15:44:03 +0530 Subject: [PATCH 15/17] Update the case when re-transmits fail after connack is received --- source/core_mqtt.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/source/core_mqtt.c b/source/core_mqtt.c index cbad607a8..7d88ce61a 100644 --- a/source/core_mqtt.c +++ b/source/core_mqtt.c @@ -2864,7 +2864,13 @@ MQTTStatus_t MQTT_Connect( MQTTContext_t * pContext, if( pContext->connectStatus == MQTTConnected ) { - pContext->connectStatus = MQTTNotConnected; + /* This will only be executed if after the connack is received + * the retransmits fail for some reason on an unclean session + * connection. In this case we need to retry the re-transmits + * which can only be done using the connect API and that can only + * be done once we are disconnected, hence we ask the user to + * call disconnect here */ + pContext->connectStatus = MQTTDisconnectPending; } MQTT_POST_STATE_UPDATE_HOOK( pContext ); From cca86bef9fedd23faf9e8e3b2a9c6b249649a504 Mon Sep 17 00:00:00 2001 From: GitHub Action Date: Tue, 24 Sep 2024 10:32:40 +0000 Subject: [PATCH 16/17] Uncrustify: triggered by comment. --- source/core_mqtt.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/source/core_mqtt.c b/source/core_mqtt.c index 7d88ce61a..23324974d 100644 --- a/source/core_mqtt.c +++ b/source/core_mqtt.c @@ -2865,10 +2865,10 @@ MQTTStatus_t MQTT_Connect( MQTTContext_t * pContext, if( pContext->connectStatus == MQTTConnected ) { /* This will only be executed if after the connack is received - * the retransmits fail for some reason on an unclean session + * the retransmits fail for some reason on an unclean session * connection. In this case we need to retry the re-transmits - * which can only be done using the connect API and that can only - * be done once we are disconnected, hence we ask the user to + * which can only be done using the connect API and that can only + * be done once we are disconnected, hence we ask the user to * call disconnect here */ pContext->connectStatus = MQTTDisconnectPending; } From 33340d2ab9f16d2e62982ce735297a31ac039555 Mon Sep 17 00:00:00 2001 From: Dakshit Babbar Date: Tue, 24 Sep 2024 16:04:34 +0530 Subject: [PATCH 17/17] Update unit tests --- test/unit-test/core_mqtt_utest.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/unit-test/core_mqtt_utest.c b/test/unit-test/core_mqtt_utest.c index 374f1654c..131f255df 100644 --- a/test/unit-test/core_mqtt_utest.c +++ b/test/unit-test/core_mqtt_utest.c @@ -2004,7 +2004,7 @@ void test_MQTT_Connect_resendPendingAcks( void ) MQTT_PubrelToResend_ExpectAnyArgsAndReturn( MQTT_PACKET_TYPE_INVALID ); status = MQTT_Connect( &mqttContext, &connectInfo, NULL, timeout, &sessionPresentResult ); TEST_ASSERT_EQUAL_INT( MQTTBadParameter, status ); - TEST_ASSERT_EQUAL_INT( MQTTNotConnected, mqttContext.connectStatus ); + TEST_ASSERT_EQUAL_INT( MQTTDisconnectPending, mqttContext.connectStatus ); TEST_ASSERT_TRUE( sessionPresentResult ); /* Test 3. One packet found in ack pending state, but Transport Send failed. */ @@ -2068,10 +2068,11 @@ void test_MQTT_Connect_resendPendingAcks( void ) MQTT_PubrelToResend_ExpectAnyArgsAndReturn( packetIdentifier + 2 ); status = MQTT_Connect( &mqttContext, &connectInfo, NULL, timeout, &sessionPresent ); TEST_ASSERT_EQUAL_INT( MQTTBadParameter, status ); - TEST_ASSERT_EQUAL_INT( MQTTNotConnected, mqttContext.connectStatus ); + TEST_ASSERT_EQUAL_INT( MQTTDisconnectPending, mqttContext.connectStatus ); /* Test 6. Two packets found in ack pending state. Sent PUBREL successfully * for first and failed for second. */ + mqttContext.connectStatus = MQTTNotConnected; MQTT_GetIncomingPacketTypeAndLength_ExpectAnyArgsAndReturn( MQTTSuccess ); MQTT_GetIncomingPacketTypeAndLength_ReturnThruPtr_pIncomingPacket( &incomingPacket ); MQTT_DeserializeAck_ExpectAnyArgsAndReturn( MQTTSuccess );