-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[openhabcloud] Handle WebSocket connections to server. #16403
base: main
Are you sure you want to change the base?
Conversation
Dores it depend on openhab-cloud openhab/openhab-cloud#447 ? |
@digitaldan or @kaikreuzer : as openHAB cloud maintainer or binding maintainer, you would like to review this PR? |
Yes, i will review. I have been procrastinating getting the cloud side of a related PR merged in and deployed to a dev instance so i can test these changes end to end. I'm out of town until the end of next week, so it may be a bit longer for me to review. |
Hi @GiviMAD I finally found some time to review this as well as the cloud PR . Took me a bit to get everything up for testing. Just FYI after my first test i'm running into an issue where a simple web socket connection seems to be failing in the binding. I'm sure it's something small and easily fixable, but it does make me curious how you are testing this? I don't really use the websocket API in OH, which is a shame, as it would be nice to get our Main UI on it. Right now i have postman creating a websocket connection and sending heartbeat messages. This works against the local OH instance, but when i do it through OH cloud, the cloud binding does receive the request, but with this error: 09:29:26.791 [DEBUG] [.io.openhabcloud.internal.CloudClient] - on(): request
09:29:26.791 [DEBUG] [.io.openhabcloud.internal.CloudClient] - Got request 669
09:29:26.791 [DEBUG] [.io.openhabcloud.internal.CloudClient] - Path /ws/
09:29:26.791 [DEBUG] [.io.openhabcloud.internal.CloudClient] - Method GET
09:29:26.791 [DEBUG] [.io.openhabcloud.internal.CloudClient] - Headers: {"upgrade":"websocket","sec-websocket-extensions":"permessage-deflate; client_max_window_bits","host":"*******","connection":"upgrade","sec-websocket-key":"d3tiKK5c3jE8stTc2bWaaQ==","sec-websocket-version":"13","user-agent":"openhab-cloud/0.0.1"}
09:29:26.791 [DEBUG] [.io.openhabcloud.internal.CloudClient] - JSONObject["body"] not found. I'll keep looking at it, but curious what your setup is and how you are testing. Thanks!!! |
Hello @digitaldan, thank you for looking at it. For my initial testing I just created a websocket connection to the server using the browser console
I send an empty json each 5 seconds to about connection to be closed in case of no logs, because the browser websocket client doesn't allow to send a ping to the server. I also tested it with the audio websocket I have WIP in different PRs. I just tested it on my laptop but it worked fine for me. I'm going to setup a custom cloud instance for my normal oh installation and see if it works fine and share here the steps I have followed. Best regards. |
@GiviMAD did you have any success with the custom cloud instance? Only a few weeks left for next stable. |
Signed-off-by: Miguel Álvarez <[email protected]>
bf47c03
to
4ff8cd5
Compare
@GiviMAD let me know when you are ready for testing and i will help out. |
Hey @digitaldan , @lsiepel. I've been looking at this today and I was able to make it work with the included docker-compose setup, and for me it seems to be working correctly. I followed these steps:
To verify the websocket, after loading my openHAB dashboard through the cloud, I pasted these javascript code in the console, it connects to the event websocket and ping it each 6 seconds to keep the connection open.
I can see the openHAB events and the responses to the PING messages in the log. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, LGTM. @GiviMAD had a positive test result, after @digitaldan is also fine with this I’ll merge this.
@GiviMAD can you solve the conflicts? Hopefully @digitaldan can approve this pr |
These changes are related to PR at openhab-cloud openhab/openhab-cloud#447.
The PR implements and registers a custom ProtocolHandler in the Jelly Http client to catch upgraded websocket responses, access its underling EndPoint and upgrade its Connection to a custom implementation that will proxy the plain socket data between the connector and the OpenHAB server allowing the websocket protocol to work.
These PR needs from the core PR openhab/openhab-core#4092 which allows to access the Jetty EndPoint used to read/write from the request socket.