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

Add doc that keep-alive is unsupported with dummy timer #132

Conversation

aggarw13
Copy link
Contributor

@aggarw13 aggarw13 commented Dec 9, 2020

Update API doc to mention that the keep-alive mechanism is not supported by MQTT_ProcessLoop API when a dummy timer function that always returns zero is supplied to the library.

@@ -100,6 +100,12 @@
* If a ping response is not received before this timeout, then
* #MQTT_ProcessLoop will return #MQTTKeepAliveTimeout.
*
* @note If a dummy implementation of the #MQTTGetCurrentTimeFunc_t timer function,
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Let's just say that if a dummy implementation of the timer function is used. I don't we need to go into details of 'that always returns zero'. Let's keep it simple.
  2. processLoop is not supported without a 'real' time function. In that case receiveLoop should be used. ( receive loop is exactly processLopp mius keep alive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have incorporated point 1. The second point is not appropriate for this config that is only utilized by ProcessLoop.

@@ -598,21 +598,23 @@ MQTTStatus_t MQTT_Disconnect( MQTTContext_t * pContext );
* alive.
*
* @note Passing a timeout value of 0 will run the loop for a single iteration.
* If a dummy #MQTTGetCurrentTimeFunc_t was passed to #MQTT_Init, then the timeout
Copy link
Contributor

Choose a reason for hiding this comment

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

We should just say that it is not supported with dummy timer function and reference doxygen for timer function ( to explain what we mean by dummy)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The presence of #MQTTGetCurrentTimeFunc_t adds the reference to the documentation of the timer function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should just say that it is not supported with dummy timer function
Only the keep-alive part of the function is not supported with a dummy timer function. The rest of the functionality of receiving acks and incoming PUBLISH is supported

abhidixi11
abhidixi11 previously approved these changes Dec 9, 2020
@aggarw13 aggarw13 merged commit 9ef7fe9 into FreeRTOS:main Dec 9, 2020
@aggarw13 aggarw13 deleted the doc/mention-no-support-of-keep-alive-with-dummy-timer branch December 9, 2020 18:57
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.

3 participants