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

Reply to WebSocket ping frames #14

Merged
merged 1 commit into from
Nov 22, 2022
Merged

Conversation

jasoncodes
Copy link
Contributor

This fixes WebSocket connections to Mosquitto brokers disconnecting every 5 minutes due to the server not receiving any response to WebSocket ping frames. Mosquitto uses libwebsockets which defaults to sending a ping frame every 5 minutes with a 10 second timeout.

@sroebert
Copy link
Owner

Ah, I figured this was not needed as MQTT has its own ping mechanism to keep the connection alive. Thanks for the contribution.

@jasoncodes
Copy link
Contributor Author

Yeah, it surprised me a little too. I expected WebSocket pings to not be invoked on a connection which is not idling (due to the MQTT pings) but libwebsockets seems to always ping regardless, right on 5 minutes.

I debugged this issue and tested the fix using websocat in verbose mode as a proxy as that made it easy to enable and configure pings (over recompiling Mosquitto with a custom config).

I looked around for where to add test coverage but didn’t find anywhere suitable. Happy to add some coverage if you see the need and you can point me in the right direction.

@sroebert
Copy link
Owner

I think for now that it is fine, I still have to find some time to add tests for MQTT pings as well, will try to make this part of that as well at that point.

@sroebert sroebert merged commit ad1f0bc into sroebert:main Nov 22, 2022
jasoncodes added a commit to jasoncodes/mqtt_display that referenced this pull request Nov 22, 2022
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.

2 participants