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.

Sign up for GitHub

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

Jump to bottom

Add hooks to the source code #200

Merged
merged 16 commits into from
Sep 14, 2022
Merged

Add hooks to the source code #200

merged 16 commits into from
Sep 14, 2022

Conversation

AniruddhaKanhere
Copy link
Member

No description provided.

@AniruddhaKanhere AniruddhaKanhere changed the title Add mutex hooks Add hooks to the source code Sep 6, 2022
@archigup
Copy link
Member

MQTT_GetPacketId also may need a mutex

archigup
archigup previously approved these changes Sep 13, 2022
View reviewed changes
archigup
archigup previously approved these changes Sep 13, 2022
View reviewed changes
paulbartell
paulbartell reviewed Sep 13, 2022
View reviewed changes
source/core_mqtt.c
}

if( bytesSent == ( int32_t ) MQTT_PUBLISH_ACK_PACKET_SIZE )
{
pContext->controlPacketSent = true;

Copy link
Contributor

@paulbartell paulbartell Sep 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it be necessary to keep hold of the mutex between the sendPacket and MQTT_UpdateStateAck calls?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think not.
They are decoupled altogether. Also, the state will be protected by its mutex.

@paulbartell
Copy link
Contributor

Is a lock needed during calls to recv?

@paulbartell
Copy link
Contributor

Where is the user meant to store their mutex / lock context(s)?

Is it necessary to lock socket operations separately from state update operations?

@AniruddhaKanhere
Copy link
Member Author

Is a lock needed during calls to recv?

I think that receive should be done through one thread only. So, it doesn't seem like it would need to be locked.
There is one exception -> the call to MQTT_Connect will be receiving data by itself and not use processloop. But again, it doesn't make sense to do recv before connecting to the broker.

@AniruddhaKanhere
Copy link
Member Author

Where is the user meant to store their mutex / lock context(s)?

Is it necessary to lock socket operations separately from state update operations?

The users are free to store the context wherever they like. To aid them to link the mutex with the context, the MQTT context is provided to the user through the macro's parameter.

I think since the send and state update ops are different and distinct, it is imperative that we separate them.

@AniruddhaKanhere AniruddhaKanhere merged commit fed0ad9 into FreeRTOS:dev Sep 14, 2022
@AniruddhaKanhere AniruddhaKanhere mentioned this pull request Sep 19, 2022
Merged
AniruddhaKanhere added a commit that referenced this pull request Sep 19, 2022
@AniruddhaKanhere @jasonpcarroll
@alfred2g @archigup
Bring in changes from dev branch to main (#214)
ee13cbd 
* Update ci.yml (#197)

* Update ci.yml

* Add main branch in the CI YAML

* Add cancel callback API (#196)

* Add cancel callback command

* Update the function name

* Fix formatting

* Update memory estimates

* Add State to MQTT_ProcessLoop so that it can be called in a non-blocking manner (#198)

* Add stateful process-loop function

* Add extra checks; fix bugs and add description of functions

* Add index based stateful processloop

* Clean up

* Renamed functions to make them more coherent with their function

* Remove unused function declarations

* Fixed failing CI checks from previous commits except unit-test

* Fixed spell check and updated size-table

* Fix CBMC proofs

* Empty-Commit to trigger CBMC proofs

* Fix loop unwinding values in the Makefile

* Add upper bound on the buffer size of MQTT

* Increase minimum limit on buffer size to >0

* Add upper bound on the size of the buffer as well

* CBMC: Add memmove stub to accelerate coverage

The commit adds a stub for memmove accelerate CBMC coverage
calculation.  Without this stub, coverage for `MQTT_ProcessLoop` and
`MQTT_ReceiveLoop` fails to converge (gets stuck generating the SAT
formula for the memmove in `receiveSingleIteration`).  This stub
checks that src and dst are nonnull pointers and havocs dst.

* Fix formatting

Co-authored-by: Aniruddha Kanhere <[email protected]>
Co-authored-by: Mark R. Tuttle <[email protected]>

* Remove the use of common buffer (#199)

* Make publish use internal buffers

* Fix comment about packet ID

* Try a different approach for ping and disconnect

* Use writev and flush in conjuction with send

* Update the publish method to use vectors

* Add vectored IO to all functions

* Fix formatting

* Reduce complexity score

* Fix spell check and complexity score

* Fix breaking build

* Add doxygen comments

* Fix doxygen part 2

* Doxygen fix part 3

* Fix doxygen part 4

* Fix some checks

* Fix memory tables

* Fix some small errors

* Fix compiler warnings and breaking CI checks from previous commit

* Fix spell check and doxygen

* Fix a couple of CBMC proofs

* Fix ping and publish proofs

* Update the function name

* Fix more CBMC proofs

* Fix MQTT Connect proof

* Add unwinding loops

* Fix last CBMC proof

* Fix formatting

* Update the Subscribe and Unsubscribe functions

* Fix formatting and doxygen checks

* Fix broken CBMC proofs

* Fix memory statistic table

* Revert changes from serializer source

* update comments to clarify write requirements

* Add a note for write function pointer

* Fix spell check

* Update changelog (#202)

* Update MQTT logging so that log levels of the library do not leak (#205)

* Replace publish state arrays with pointers
Added an MQTT_InIt function for QoS > 0 publishes
Fixed functions which were dealing with state arrays

* Fix CI checks and clean up

* Fix CBMC proofs

* Fix sub and unsub CBMC proofs

* Fix remaining proofs

* Fix remaining CI checks

* Fix spell check

* Minor typo fix (#209)

* Update core_mqtt.h (#208)

* Update core_mqtt.h

* Update core_mqtt.h

Co-authored-by: Aniruddha Kanhere <[email protected]>

* Update documentation and Doxygen comments in the source (#206)

* Update comments of the MQTT_InIt function

* Updating documentation of more of functions

* Fix formatting and spell check

* Update core_mqtt.h

* Add hooks to the source code (#200)

* Add mutex hooks

* Clean up of code

* Add doxygen comments and fix spell check

* Fix LogError call

* Fix formatting and memory table

* Fix dereference failure

* Update the hook names

* Fix broken builds

* Update the macros and variables

* Reword the briefs of hooks and uncrustify

* Fir formatting

* Protect get packet ID

* Fix formatting

* Fix Unit tests (#212)

* Fix Unit tests

* Update unit tests after new changes

* Fix more UT

* add dummy calls to the transport

* fix build error

* Remove usused variables

* Remove unsused variables

* Remove usused variables

* Unsued parameter

* Fix ut failure

* Fix uninitialized unit test variables

* Fix ut expectation

* Fix unit-tests

* Fix unit test uninitialized variable

* increase unit test coverage

* increase unit test coverage

* Fix unit test build

* State coverage 100%

* Serializer 100% coverage

* 100% UT coverage

* Fix formatting

* Fix size table

* Address PR comments

Co-authored-by: Aniruddha Kanhere <[email protected]>

* Remove unnecessary ternaries (#211)

Co-authored-by: Aniruddha Kanhere <[email protected]>

* Fix misra deviations  (#213)

* MISRA compliance update 1

* Fix MISRA errors

* Zero MISRA violations; 13 suppressed

* Fix CI checks

* Update MISRA.md

* Remove deviations

* Fix MISRA.md file

* Fix bug - wasn't incrementing vector length properly.

* Remove unwanted files

* Update comment to clarify the control flow of UT

* Remove 'dev' branch from the CI checks

* Update horrid threshold = 10

Co-authored-by: Aniruddha Kanhere <[email protected]>
Co-authored-by: Mark R. Tuttle <[email protected]>
Co-authored-by: jasonpcarroll <[email protected]>
Co-authored-by: alfred gedeon <[email protected]>
Co-authored-by: Archit Gupta <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@archigup archigup archigup approved these changes

@paulbartell paulbartell paulbartell approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AniruddhaKanhere @archigup @paulbartell