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 keepalive handling (various issues in original issue tracker) #14

Merged
merged 2 commits into from
Dec 9, 2024

Conversation

uschindler
Copy link

@uschindler uschindler commented Dec 7, 2024

Hi,
the original client has some problem with the state machine, which handles the keep alive packets. If the connection breaks and the client is waiting for a ping response, a reconnect with the same client will still wait for the pong response, as the state machine says pingOutstanding is true.

This code cleans it up a bit:

  • on connection it makes sure that pingOutstanding=false (that's the main issue)
  • it only sets it to true if it successfully sent the ping request
  • when the connection is closed, the pingOutstanding state variable is also reset to false for safety
  • it also makes sure that the timer is reset when publishing new topics and other places.

This fixes the issues mentioned in the original issue tracker:

This is mainly an improvement to the following pull request on original repo: knolleary#802

It would be nice to include this together with the urgent fix to unbreak passing a password on connect.

I am testing this at moment together with the PR #11 with the device BSB-LAN (for heaters): fredlcore/BSB-LAN#683

@uschindler uschindler changed the title Dev/fix keep alive Fix keepalive handling (various issues in original issue tracker) Dec 7, 2024
@uschindler
Copy link
Author

@imbeacon: it would be good to merge this and especially #11.
@MathewHDYT: maybe have a look, too

@MathewHDYT
Copy link

@imbeacon Looks good this should definetly be merged with #11 as soon as possible.

Thanks a lot for the improvements @uschindler.

@imbeacon imbeacon merged commit a1ef1c4 into thingsboard:master Dec 9, 2024
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