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

Consolidate web serving code (PLANE-294) #52

Merged
merged 5 commits into from
Jan 1, 2025

Conversation

cgpadwick
Copy link
Collaborator

Implements PLANE-294

As I was thinking through option 2 of ticket PLANE-120, I realized that it would be pretty clunky to start a ws_server process for each camera AND have to run the python code for each instance to get a UI. For example we'd need to do this:

Launch the ws_server processes:

ws_server -camera_idx 0 -cal_file file2.json -port 8080 &
ws_server -camera_idx 1 -cal_file file2.json -port 8090 &

And then launch a flask app for each instance from a laptop connected to the robot wifi:

cd app
source venv/bin/activate
python app.py --wsaddr <orin ip address>:8080 --port 5000 &
python app.py --wsaddr <orin ip address>:8090 --port 5001 &

After all that was done one could navigate to http://localhost:5000 to see camera 0 and http://localhost:5001 to see camera 1 in a different tab. Doable but quite clunky.

I realized that the websocket server Seasocks is actually capable of serving the webpage itself, so there is no need for the python flask app at all. This really simplifies things, as now with this change all we need to do is start the ws_server processes as above, but there's no need to start the python processes. All you would need to do is get a laptop connected to the robot wifi and simply navigate to http://<ip address of the orin>:8080 in one tab and http://<ip address of the orin>:8090 in another tab. And that's it!

It was quite easy to implement too - just a single line of code changed in ws_server.cu. I added a directory called public and added the index.html file from the app/template directory in there. I made a few small changes to implement the message parsing logic in javascript rather than python, and changed the websocket protocol to websockets instead of socket.io.

It was a very simple change and works really well in my testing.

@cgpadwick cgpadwick requested review from maxspier and cwosw December 29, 2024 03:21
Copy link
Member

@maxspier maxspier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks super legit! How should we go about combining this code with the code from PLANE-260 that I've been working on?

@cgpadwick
Copy link
Collaborator Author

Looks super legit! How should we go about combining this code with the code from PLANE-260 that I've been working on?

I just merged yours so I'll merge your changes in this PR

Copy link
Member

@maxspier maxspier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@maxspier maxspier merged commit 8bf8dd3 into main Jan 1, 2025
1 check passed
@cgpadwick cgpadwick deleted the chris.padwick/consolidatewebserver branch February 4, 2025 02:11
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