Modify SUBACK/UNSUBACK handling to not return ServerRefused on non-malformed packets#369
Open
AniruddhaKanhere wants to merge 4 commits intoFreeRTOS:mainfrom
Open
Modify SUBACK/UNSUBACK handling to not return ServerRefused on non-malformed packets#369AniruddhaKanhere wants to merge 4 commits intoFreeRTOS:mainfrom
AniruddhaKanhere wants to merge 4 commits intoFreeRTOS:mainfrom
Conversation
Server rejection of individual topic filters (reason codes >= 0x80) in SUBACK/UNSUBACK is not a library-level error. The application inspects per-topic reason codes via MQTTDeserializedInfo_t.pReasonCode in the event callback. Previously, readSubackStatus returned MQTTServerRefused when all topics were rejected, which propagated up through MQTT_ProcessLoop/ReceiveLoop as an undocumented error code and also prevented the receive buffer from being advanced. Changes: - readSubackStatus: return MQTTSuccess for all known reason codes (including rejections). Only unrecognized codes return MQTTBadResponse. - handleSubUnsubAck: remove dead MQTTServerRefused branch. - Update doxygen briefs for readSubackStatus, deserializeSubUnsubAck, handleSubUnsubAck, MQTT_DeserializeAck, MQTTEventCallback_t, and MQTTServerRefused enum.
- MQTT_ProcessLoop/MQTT_ReceiveLoop: Add MQTTNoMemory, MQTTPublishStoreFailed, MQTTEventCallbackFailed, MQTTStatusNotConnected, MQTTStatusDisconnectPending. Reorder to list MQTTSuccess and MQTTNeedMoreBytes first as non-error returns with explicit note that all other values indicate an error. - MQTT_Publish: Add MQTTNoMemory, MQTTStateCollision, MQTTIllegalState. - MQTT_Disconnect: Remove MQTTStatusDisconnectPending (never returned). - MQTT_GetSubAckStatusCodes: Add MQTTBadResponse. - MQTT_GetPacketId: Document zero return on NULL context.
The pOptionalMqttPacketType parameter of MQTTPropAdd_* functions
requires passing a pointer to a packet type byte for runtime
validation. The previous calling convention required compound
literals which are not C90 compatible and are cryptic:
MQTTPropAdd_SessionExpiry(&p, 3600, &(uint8_t){MQTT_PACKET_TYPE_CONNECT});
Add MQTT_PROP_VALIDATE_CONNECT/PUBLISH/SUBSCRIBE/UNSUBSCRIBE/DISCONNECT
macros backed by static const variables (C90 safe) so users can write:
MQTTPropAdd_SessionExpiry(&p, 3600, MQTT_PROP_VALIDATE_CONNECT);
MQTT_PROP_NO_VALIDATE (NULL) skips validation. Fully backward compatible.
- test_MQTTV5_suback: SUBACK with reason code 0x80 now yields MQTTSuccess instead of MQTTServerRefused. - test_MQTT_ProcessLoop_handleIncomingAck_Happy_Paths5: Remove the MQTTServerRefused mock setup; the deserializer no longer produces that status for SUBACK, so the callback is invoked with MQTTSuccess (the default). - size_table.md: Update core_mqtt_serializer.c and total sizes to match the new binary size after the code additions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
MQTTProcessLoop calls return ServerRefused when a SUBACK/UNSUBACK says that one of the subs was rejected by the server. This commit modifies that behavior so that processloop returns success since the packet was well formed and the application should decide what is to be done with the subscription.
This PR also fixes documentation of the functions to say what they actually return.
Test Steps
Checklist:
Related Issue
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.