Express the need of a non-blocking TransportRecv_t #317
Description
-----------
- changed documentation to express the need of a non-blocking
TransportRecv_t
- mentioned that a non-blocking implementation is highly recommended in
porting.dox and in the transportinterface
- did go more into detail in TransportRecv_t what the effects of a
blocking recv implementation are
Test Steps
-----------
only changed documentation
Checklist:
----------
<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
- [ ] I have tested my changes. No regression in existing tests.
- [ ] I have modified and/or added unit-tests to cover the code changes
in this Pull Request.
Related Issue
-----------
#317
By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
---------
Co-authored-by: Paul Höhn <paulh@MacBook-Air-von-Paul.local>
Co-authored-by: DakshitBabbar <100972343+DakshitBabbar@users.noreply.github.com>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: DakshitBabbar <dakshitbabbar.iitd@gmail.com>
Description
-----------
This PR:
Adds CBMC proofs for the new APIs added for publish retransmits in #308
Test Steps
-----------
Proofs run without any errors or warnings
Checklist:
----------
- [x] I have tested my changes. No regression in existing tests.
- [x] I have modified and/or added unit-tests to cover the code changes
in this Pull Request.
Related Issue
-----------
<!-- If any, please provide issue ID. -->
By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
---------
Co-authored-by: DakshitBabbar <dakshba@amazon.com>
Co-authored-by: Gaurav-Aggarwal-AWS <33462878+aggarg@users.noreply.github.com>
<!--- Title -->
Description
-----------
<!--- Describe your changes in detail. -->
Make required changes for passing the Coverity Static Analysis. Unit
tests are modified for the changes made.
Checklist:
----------
<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
- [x] I have tested my changes. No regression in existing tests.
- [x] I have modified and/or added unit-tests to cover the code changes
in this Pull Request.
Related Issue
-----------
<!-- If any, please provide issue ID. -->
By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
---------
Co-authored-by: DakshitBabbar <ubuntu@ip-172-31-24-168.ap-south-1.compute.internal>
Co-authored-by: Dakshit Babbar <dakshba@amazon.com>
<!--- Title -->
Description
-----------
<!--- Describe your changes in detail. -->
This PR enables the coreMQTT library to resend unacked publishes on an
unclean session connection.
Following is a brief summary of changes:
1. Add a new API `MQTT_InitRetransmits` that will initialise the context
to handle publish retransmits on an unclean session connection
2. Add signatures of callback function pointers that users will define
in order to:
a. copy and store outgoing publishes
b. retrieve copied publish on an unclean session connection to resend
c. clear a copied publish when a `PUBACK`/`PUBREC` is received
d. clear all copied publishes on a clean session connection
3. Update the API's to check if callback's are defined and implement
resend publishes as required.
Following are the specifics of the changes:
1. Add 3 new MQTTStatus_t values: MQTTPublishStoreFailed,
MQTTPublishRetrieveFailed and MQTTPublishClearAllFailed
2. Update `MQTTContext_t` to hold the callback function pointers
a. `MQTTRetransmitStorePacket storeFunction`
b. `MQTTRetransmitRetrievePacket retrieveFunction`
c. `MQTTRetransmitClearPacket clearFunction`
d. `MQTTRetransmitClearAllPackets clearAllFunction`
3. Update the `MQTT_Status_strerror` function to handle the new
`MQTTStatus_t` values
4. Add a new API function `MQTT_InitRetransmits` that will initialise
the new callback functions in the `MQTTContext_t`
5. Add this API to the core_mqtt.h file to make it available to users
6. Modify `MQTT_Publish`
a. copy the outgoing publish packet in form of an array of
`TransportOutVector_t` if the callback if defined
b. if copy fails then bubble up corresponding error status code
7. Modify `MQTT_ReceiveLoop`
a. on receiving a `PUBACK`/`PUBREC` clear the copy of that particular
publish after the state of the publish record has been successfully
updated, if the callback if defined
8. Modify `MQTT_Connect`
a. on a clean session clear all the copies of publishes stored if the
callback is defined
b. if clear all fails then bubble up corresponding error status code
c. on an unclean session get the packetID of the unacked publishes and
retrieve the copies of those if the callback is defined
d. if retrieve fails then bubble up corresponding error status code
Approaches Taken
---------------
- To let user know about the changes we have made we will add them to a
changelog and have a minor version bump
- To be in line with the zero copy principle in our library we chose to
provide and retrieve the publish packets for storing and resending in
form of an array of `TransportOutVector_t`
- Code is written in a way that on receiving a `PUBACK`/`PUBREC` the
copy will be cleared after the state of the publish record is changed so
that if state update fails the copy won't be cleared. Otherwise if the
state does not change and the copy is cleared then when a connection is
made with an unclean session there will be a retrieve fail as the system
is in an inconsistent state.
- We are storing the copies of the publishes with the Duplicate flag set
this is because on retrieving the packet we will get it in the form of a
`TransportOutVector_t` that holds the data in a `const` pointer which
cannot be changed after retrieving.
Pending Tasks
---------------
- [ ] Changelog
- [ ] Minor version bump
- [x] Doxygen example for the new API
- [x] Better API Names
- [x] Unit Test Updates
- [x] CBMC Proof
---------
Co-authored-by: Dakshit Babbar <dakshba@amazon.com>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: AniruddhaKanhere <60444055+AniruddhaKanhere@users.noreply.github.com>
<!--- Title -->
Description
-----------
<!--- Describe your changes in detail. -->
Following is a brief summary of changes:
1. Check the connected flag before doing any send operation on the
connection
2. Make all the APIs that do send operations, thread safe
3. Update the connected flag within MQTT_Disconnect regardless of the
return status of the send operation
Following are the specifics of the changes:
1. Add 3 new MQTTStatus_t values: MQTTStatusConnected,
MQTTStatusNotConnected and MQTTStatusDisconnectPending
2. Added 1 new MQTTConnectionStatus_t value: MQTTDisconnectPending
3. Update the MQTT_Status_strerror function to handle the new
MQTTStatus_t values
4. Add a new API function MQTT_CheckConnectStatus() that will check the
value of the context→connectStatus flag safely.
5. Add this API to the core_mqtt.h file to make it available to users
6. Check the connected flag before doing any Send operation (following
API's are updated)
a. sendPublishAcks
b. MQTT_Connect
c. MQTT_Subscribe
d. MQTT_Publish
e. MQTT_Ping
f. MQTT_Unsubscribe
g. MQTT_Disconnect
7. Use the MQTT_PRE_STATE_UPDATE_HOOK() and
MQTT_POST_STATE_UPDATE_HOOK() to make the send APIs thread safe
8. The connect status is set to MQTTDisconnectPending whenever a
transport send or receive function returns a negative error code
9. `const` keyword for the the MQTTStatus_t is removed in the input
parameters for the receive functions as we need to update the connection
status when the receive function returns a negative error code
Relevant Explanations
---------------
- MQTT_PRE_SEND_HOOK(): The Pre and Post Send hook Macros are not
required now, as the sending logic will be within the pre and post state
update hook itself. (because we cannot allow other threads to change the
connection state of the application until a send operation is complete).
- I have split the handleSessionResumption function. The part of that
function which was handling the clean session has been added within the
mutex calls in the [MQTT_Connect
API](https://github.com/FreeRTOS/coreMQTT/pull/305/files#diff-2534a3c0229ae9af3801f2a5c6a24eeef2cd0a686671f0371a11d2718ba4fdd6R2828)
and the unclean session part is handled by this new function that is
[called outside the mutex
calls](https://github.com/FreeRTOS/coreMQTT/pull/305/files#diff-2534a3c0229ae9af3801f2a5c6a24eeef2cd0a686671f0371a11d2718ba4fdd6R2866).
Pending Tasks
---------------
- [ ] Doxygen example for the new API
- [x] Unit Test Updates
- [x] CBMC Proof
---------
Co-authored-by: Dakshit Babbar <dakshba@amazon.com>
Co-authored-by: GitHub Action <action@github.com>
Description
-----------
With CBMC v6, unwinding assertions are enabled by default, and object
bits no longer need to be set at compile time. Update various build
rules to use the latest template as provided with CBMC starter kit.
Test Steps
-----------
Tested in CI.
Checklist:
----------
<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
- [ ] I have tested my changes. No regression in existing tests.
- n/a I have modified and/or added unit-tests to cover the code changes
in this Pull Request.
By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
<!--- Title -->
Description
-----------
<!--- Describe your changes in detail. -->
**Issue1:**
Mocked function calls jump to real implementation instead of mocks on
mac
**Solution:**
Convert the symbols of the mocked implementation from weak to strong
**Issue2:**
lcov generating wrong coverage reports on mac
**Solution:**
Rectifying network buffer size to be of the appropriate value otherwise
memset clears out more memory spaces than required which leads to
reseting the line coverage counters.
**Issue3:**
log statements being taken as branches
**Solution:**
Removing the ternary operation to figure out if there is a '/' in front
of the file name or not. This can be done as this feature is just for
debugging purposes.
**Issue4:**
Some tests failing randomly on mac
**Solution:**
Initialise variables at places where necessary
By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
---------
Co-authored-by: Dakshit Babbar <dakshba@amazon.com>
<!--- Title -->
Description
-----------
<!--- Describe your changes in detail. -->
Updated the coverage.cmake and create_test.cmake files to build the unit
tests on Mac
Updated the README file to include the correct Cmake command for the
users to use to build the unit tests
Checklist:
----------
<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
- [x] I have tested my changes. No regression in existing tests.
- [x] I have modified and/or added unit-tests to cover the code changes
in this Pull Request.
Related Issue
-----------
<!-- If any, please provide issue ID. -->
By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
---------
Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>
Co-authored-by: Dakshit Babbar <dakshba@amazon.com>
Co-authored-by: Gaurav Aggarwal <aggarg@amazon.com>
Description
-----------
The `mqtt.tag` was generated at `coreMQTT/docs/doxygen/output/mqtt.tag`.
The
[doxygen-generation](https://github.com/FreeRTOS/CI-CD-Github-Actions/tree/main/doxygen-generation)
GH action deploys the content of `coreMQTT/docs/doxygen/output/html` and
as a result, the `mqtt.tag` was not getting deployed.
This change updates the location of `mqtt.tag` to
`coreMQTT/docs/doxygen/output/html/mqtt.tag` to ensure that it gets
deployed by the doxygen-generation GH action.
Test Steps
-----------
Generated the doxygen documentation locally.
Checklist:
----------
<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
- [x] 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
-----------
NA.
By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
---------
Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>
Description
-----------
The upcoming CBMC version 6 release includes changes that may affect
existing proofs. This PR will make sure that coreMQTT PRs are not
negatively impacted by this release. After releasing CBMC version 6 we
will issue a follow-up PR that will return coreMQTT to using CBMC's
latest release, and will include any changes to proofs that may be
necessary to support the new version.
Test Steps
-----------
Tested in CI (no changes in behaviour at this point)
Checklist:
----------
<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
- [ ] I have tested my changes. No regression in existing tests.
- [ ] I have modified and/or added unit-tests to cover the code changes
in this Pull Request.
Related Issue
-----------
n/a
By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
Co-authored-by: Tony Josi <tonyjosi@amazon.com>
Co-authored-by: Aniruddha Kanhere <60444055+AniruddhaKanhere@users.noreply.github.com>
* Update the CHANGELOG.md to include v2.3.1 information
* Update version number to "v2.3.1+" in main branch in public header file macro, manifest.yml and config.doxyfile.
<!--- Title -->
Description
-----------
* Update the release action for version number include the following
files
- docs/doxygen/config.doxyfile - PROJECT_NUMBER
- manifest.yml file - version
- source file - version header
- core_mqtt.h - version number
* Add version number check in "Create ZIP and verify package for release
asset" steps. Including the following
- docs/doxygen/config.doxyfile - PROJECT_NUMBER
- manifest.yml file - version
- source file - version header
- core_mqtt.h - version number
* Update all the version number to "v2.3.0+" and "\<DEVELOPMENT
BRANCH\>"
Test Steps
-----------
Using release action to create release should update the following
* source/include/core_mqtt.h version number
* source files header version number
* doxygen version number
* manifest.yml number
* SBOM file
Tested in personal fork without problem :
https://github.com/FreshDevGo/coreMQTT/actions/runs/9885707328/job/27304218049
Test with wrong source file version number :
https://github.com/FreshDevGo/coreMQTT/actions/runs/9885727002/job/27304274003
Test with wrong manifest.yml version number :
https://github.com/FreshDevGo/coreMQTT/actions/runs/9885726029/job/27304270303
Test with wrong doxygen version number :
https://github.com/FreshDevGo/coreMQTT/actions/runs/9885723302/job/27304269170
Test with wrong version number macro in core_mqtt.h :
https://github.com/FreshDevGo/coreMQTT/actions/runs/9885724835/job/27304268841
Checklist:
----------
<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
- [x] I have tested my changes. No regression in existing tests.
- [ ] ~~I have modified and/or added unit-tests to cover the code
changes in this Pull Request.~~
Related Issue
-----------
<!-- If any, please provide issue ID. -->
By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
<!--- Title -->
Description
-----------
This PR update changelog, version numbers and .md (doxygen, size table)
files for release
Test Steps
-----------
<!-- Describe the steps to reproduce. -->
Checklist:
----------
<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
- [ ] I have tested my changes. No regression in existing tests.
- [ ] I have modified and/or added unit-tests to cover the code changes
in this Pull Request.
Related Issue
-----------
<!-- If any, please provide issue ID. -->
By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
<!--- Title -->
Description
-----------
This PR fixes a build issue when `Werror=sign-compare` is enabled while
building the library.
Test Steps
-----------
<!-- Describe the steps to reproduce. -->
Checklist:
----------
<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
- [x] I have tested my changes. No regression in existing tests.
- ~[ ] I have modified and/or added unit-tests to cover the code changes
in this Pull Request.~
Related Issue
-----------
#282
By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
Description
-----------
This PR adds a note about the user provided timer behavior when it
overflows.
Checklist:
----------
<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
- [NA] 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
-----------
https://github.com/FreeRTOS/coreMQTT/issues/277
By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
---------
Update Coverity configuration to meet the latest coverity standard.
Updated CMakelist to only build Coverity if required instead of building
the CMock based unit tests as well.
<!--- Title -->
Description
-----------
<!--- Describe your changes in detail. -->
Test Steps
-----------
<!-- Describe the steps to reproduce. -->
Checklist:
----------
<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
- [x] I have tested my changes. No regression in existing tests.
- [x] I have modified and/or added unit-tests to cover the code changes
in this Pull Request.
Related Issue
-----------
<!-- If any, please provide issue ID. -->
By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
---------
Co-authored-by: Soren Ptak <ptaksoren@gmail.com>
Description
-----------
Corrections highlight the non-blocking
expectations of the TransportRecv method.
Test Steps
-----------
No manual steps taken. I'm going to let the doxygen CI verify this.
Checklist:
----------
<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
- [ X ] I have tested my changes. No regression in existing tests.
- Testing with CI
- [ X ] I have modified and/or added unit-tests to cover the code
changes in this Pull Request.
- Not applicable
Related Issue
-----------
https://github.com/FreeRTOS/coreMQTT/issues/261
By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
The loggging defaults were split out since they are not namespaced to
coreMQTT, and they previously leaked to all files including a coreMQTT
header. Splitting them allowed the logging defaults to only be pulled
into coreMQTT source files. Now that no header files use the config
headers, and thus all coreMQTT config only affects coreMQTT source
files, the split is no longer needed.
core_mqtt_serializer.h included the user coreMQTT config, which spills
the config header's contents into all consumers of coreMQTT's headers.
Macros from the config are no longer used in the the API, so this is
also no longer used for anything, so can be removed.
The NetworkContext struct should be defined in each c file, not in the
config h file. The API uses it as an opaque type.
CMock needs a workaround for it's c files though, as it does not support
opaque types.
<!--- Title -->
Description
-----------
<!--- Describe your changes in detail. -->
Typecast outgoing and incomingpublish count to unsigned longs to match
format specifier.
Test Steps
-----------
<!-- Describe the steps to reproduce. -->
core OTA demo had a build error due to format specifier used when
logging with coreMQTT. Adding these typecasts fixes the build error.
Checklist:
----------
<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
- [X] I have tested my changes. No regression in existing tests.
- [ ] I have modified and/or added unit-tests to cover the code changes
in this Pull Request.
Related Issue
-----------
<!-- If any, please provide issue ID. -->
By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
<!--- Title -->
Description
-----------
<!--- Describe your changes in detail. -->
Test Steps
-----------
<!-- Describe the steps to reproduce. -->
Checklist:
----------
<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
- [ ] I have tested my changes. No regression in existing tests.
- [ ] I have modified and/or added unit-tests to cover the code changes
in this Pull Request.
Related Issue
-----------
<!-- If any, please provide issue ID. -->
By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
* Use new version of CI-CD Actions, checkout@v3 instead of checkout@v2 on all jobs
* Use cSpell spell check, and use ubuntu-20.04 for formatting check
* Add in bot formatting action
The code for handleKeepAlive() (which is invoked as part of MQTT_ProcessLoop()) does not invoke the STATE_UPDATE_HOOK macros, but still reads from pContext->lastPacketTxTime. Therefore, this causes a RW race condition which is picked up by ThreadAnalyzer. With a sufficiently meddlesome scheduler, the value of lastPacketTxTime could be different for each time it is checked inside of handleKeepAlive() , causing unreliable behavior in the transmission of KeepAlive packets.
* Fix timeout calculation to account for overflow
* Add unit tests to check for overflow
* Update timeout value in UT
* Fix formatting
* Update core_mqtt_utest.c
* Add one more unit test to check for one corner case
* Make unit-test more robust
* Fix MQTT_Status_strerror to return correct error on NeedMoreBytes error.
* Fix timeout calculation to account for overflow
* Add unit tests to check for overflow
* Update timeout value in UT
* Fix formatting
* Update core_mqtt_utest.c
* Add one more unit test to check for one corner case
* Make unit-test more robust
* Replaced magic numbers with macros and added comments
* Fix spell check and build check
* Fix formatting
* Remove macros which are only used once
* Fix formatting and build check
* Initialise variables in test before use
* Init variables in serializer tests
* Update uncrustify runner OS version
* Update doxygen config and doxygen used to latest
* Update brief and remove unused return description
* Update python version for latest ubuntu image
