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

Minor websocket client issues #2827

Merged
merged 3 commits into from
Jun 24, 2024

Conversation

mikee47
Copy link
Contributor

@mikee47 mikee47 commented Jun 22, 2024

This PR adds a few fixes for websockets I seem to have kicking about.

  • Tidy, remove unused websocket strings
  • wsserver tool missing port argument handling, default is 8000 not 9999

Check return value from http connect()

I can force this to fail by calling 'connect' twice in succession, though I do notice a disparity here. If the http connection is 'processing' then WebsocketClient::connect returns false; however, HttpClientConnection::connect returns true in this situation.

TODO:

@mikee47 mikee47 changed the title [WIP] Fix websocket client errors [WIP] Improve websocket client error handling Jun 22, 2024
@mikee47 mikee47 force-pushed the fix/websocket-client-errors branch from 960b450 to d0af2b6 Compare June 22, 2024 20:10
@mikee47 mikee47 changed the title [WIP] Improve websocket client error handling [WIP] Minor websocket client issues Jun 22, 2024
@mikee47 mikee47 changed the title [WIP] Minor websocket client issues Minor websocket client issues Jun 23, 2024
@slaff slaff added this to the 5.2.0 milestone Jun 24, 2024
@slaff
Copy link
Contributor

slaff commented Jun 24, 2024

@mikee47 can you rebase this PR?

@mikee47 mikee47 force-pushed the fix/websocket-client-errors branch from d0af2b6 to dd317eb Compare June 24, 2024 07:46
@slaff slaff merged commit 244105b into SmingHub:develop Jun 24, 2024
48 checks passed
@mikee47 mikee47 deleted the fix/websocket-client-errors branch June 24, 2024 11:20
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