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

ping handling deserves improvement #198

Closed
vladak opened this issue Jan 21, 2024 · 0 comments
Closed

ping handling deserves improvement #198

vladak opened this issue Jan 21, 2024 · 0 comments

Comments

@vladak
Copy link
Contributor

vladak commented Jan 21, 2024

While working on #190, I noticed several things:

  • the loop() will return once it gets a PINGRESP if it sent PINGREQ. This is against the promise in the docstring w.r.t. the timeout argument (timeout: return after this timeout, in seconds.). The loop() code should handle the ping and keep receiving messages until it hits the timeout.
  • the trigger to send PINGREQ should be based on the traffic/messages sent rather than the last time loop() was called. It would save unnecessary PINGREQs. Also, the initial current_time setting assumes that first call to loop() will happen shortly after the last message sent (whatever it is). Also, if loop() is called with timeout bigger then the keep alive timeout, the PINGREQ will not be sent if the preceding call was done before keep alive timeout ago.

For the latter, the MQTT 3.1.1 spec says in section 2.2:

The client has a responsibility to send a message within each Keep Alive time period. In the absence of a data-related message during the time period, the client sends a PINGREQ message, which the server acknowledges with a PINGRESP message.

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

No branches or pull requests

1 participant