Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix out of order PUBACK and PUBLISH handling #79

Merged
merged 2 commits into from May 7, 2024

Conversation

aggarg
Copy link
Member

@aggarg aggarg commented Apr 24, 2024

Description

The current implementation uses task notification to wait for acknowledgements and incoming publishes and therefore, cannot differentiate between acknowledgements and incoming publishes. This causes a crash when an incoming PUBLISH is received before PUBACK for the previous outgoing publish.

This PR uses an event group to wait for acknowledgements and incoming publishes. This allows us to differentiate between acknowledgements and incoming publishes by waiting for different bits.

Test Steps

Run pub-sub demo:

 (9822) sub_pub_unsub_demo: coreMQTT-Agent connected.
I (9832) core_mqtt_agent_manager: coreMQTT-Agent connected.
I (9842) sub_pub_unsub_demo: Task "SubPub0" sending subscribe request to coreMQTT-Agent for topic filter: /filter/SubPub0 with id 1
I (9852) sub_pub_unsub_demo: Task "SubPub1" sending subscribe request to coreMQTT-Agent for topic filter: /filter/SubPub1 with id 2
I (10542) sub_pub_unsub_demo: Subscribe 1 for topic filter /filter/SubPub0 succeeded for task "SubPub0".
I (10542) sub_pub_unsub_demo: Task "SubPub0" sending publish request to coreMQTT-Agent with message "SubPub0" on topic "/filter/SubPub0" with ID 3.
I (10552) sub_pub_unsub_demo: Task "SubPub0" waiting for publish 3 to complete.
I (12212) sub_pub_unsub_demo: Subscribe 2 for topic filter /filter/SubPub1 succeeded for task "SubPub1".
I (12222) sub_pub_unsub_demo: Task "SubPub1" sending publish request to coreMQTT-Agent with message "SubPub1" on topic "/filter/SubPub1" with ID 4.
I (12232) sub_pub_unsub_demo: Task "SubPub1" waiting for publish 4 to complete.
I (14212) coreMQTT: Publishing message to /filter/SubPub0.

I (15212) coreMQTT: Publishing message to /filter/SubPub1.

I (17002) coreMQTT: Ack packet deserialized with result: MQTTSuccess.
I (17002) coreMQTT: State record updated. New state=MQTTPublishDone.
I (17002) coreMQTT: Ack packet deserialized with result: MQTTSuccess.
I (17012) coreMQTT: State record updated. New state=MQTTPublishDone.
I (17022) coreMQTT: De-serialized incoming PUBLISH packet: DeserializerResult=MQTTSuccess.
I (17022) coreMQTT: State record updated. New state=MQTTPubAckSend.
I (17032) coreMQTT: De-serialized incoming PUBLISH packet: DeserializerResult=MQTTSuccess.
I (17042) coreMQTT: State record updated. New state=MQTTPubAckSend.
I (17052) sub_pub_unsub_demo: Publish 3 succeeded for task "SubPub0".
I (17052) sub_pub_unsub_demo: Task "SubPub0" received: SubPub0
I (17062) sub_pub_unsub_demo: Task "SubPub0" sending unsubscribe request to coreMQTT-Agent for topic filter: /filter/SubPub0 with id 5
I (17062) sub_pub_unsub_demo: Publish 4 succeeded for task "SubPub1".
I (17082) sub_pub_unsub_demo: Task "SubPub1" received: SubPub1
I (17092) sub_pub_unsub_demo: Task "SubPub1" sending unsubscribe request to coreMQTT-Agent for topic filter: /filter/SubPub1 with id 6
I (19652) sub_pub_unsub_demo: Unsubscribe 5 for topic filter /filter/SubPub0 succeeded for task "SubPub0".
I (19652) sub_pub_unsub_demo: Task "SubPub0" completed a loop. Delaying before next loop.
I (21292) sub_pub_unsub_demo: Unsubscribe 6 for topic filter /filter/SubPub1 succeeded for task "SubPub1".
I (21292) sub_pub_unsub_demo: Task "SubPub1" completed a loop. Delaying before next loop.

Checklist:

  • I have tested my changes. No regression in existing tests.
  • [NA] I have modified and/or added unit-tests to cover the code changes in this Pull Request.

Related Issue

#63

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

The current implementation uses task notification to wait for
acknowledgements and incoming publishes and therefore, cannot
differentiate between acknowledgements and incoming publishes. This
causes a crash when an incoming PUBLISH is received before PUBACK for
the previous outgoing publish.

This commit uses an event group to wait for acknowledgements and
incoming publishes. This allows us to differentiate between
acknowledgements and incoming publishes by waiting for different bits.

This was reported here - FreeRTOS#63.

Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>
Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>
@aggarg aggarg merged commit 06f5ce1 into FreeRTOS:main May 7, 2024
21 checks passed
@aggarg aggarg deleted the issue_63 branch May 7, 2024 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants