-
Notifications
You must be signed in to change notification settings - Fork 10
Feat/Quest VR teleoperation stack #1215
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
Conversation
Greptile OverviewGreptile SummaryThis PR adds a complete Meta Quest 3 teleoperation stack spanning:
The change fits into the codebase by registering new module entrypoints and blueprints ( Main issues to address before merge:
Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Quest as Quest Browser (WebXR)
participant Deno as Deno teleop_server.ts
participant LCM as LCM bus (UDP)
participant Py as QuestTeleopModule (Python)
Quest->>Deno: wss:// connect
loop ~80Hz
Quest->>Deno: WS binary (LCM packet)<br/>/vr_left_pose, /vr_right_pose
Quest->>Deno: WS binary (LCM packet)<br/>/vr_left_joy, /vr_right_joy
Deno->>LCM: publishPacket(packet)
LCM-->>Py: PoseStamped / Joy
Py->>Py: _on_pose/_on_joy
end
loop 50Hz control loop
Py->>Py: _handle_engage
Py->>Py: _get_output_pose (delta)
Py->>LCM: publish PoseStamped (/teleop/*)
Py->>LCM: publish QuestButtons (/teleop/buttons)
end
LCM-->>Deno: subscribePacket(raw)
Deno-->>Quest: WS binary (LCM packet)
|
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.
8 files reviewed, 5 comments
Additional Comments (5)
The
|
|
All Greptile feedback addressed in latest commits |
.gitignore
Outdated
| *mobileclip* | ||
| /results | ||
|
|
||
| dimos/assets/teleop_certs/ |
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.
dimos/ is only for code. Should this have been assets/teleop_certs/?
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.
Good catch, fixed this
server path + .gitignore are updated to /assets/teleop_certs/
dimos/teleop/quest/web/README.md
Outdated
| ## Running | ||
|
|
||
| ```bash | ||
| deno run --allow-net --allow-read --allow-run --allow-write --unstable-net dimos/teleop/quest/web/teleop_server.ts |
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.
Since you have the #!/usr/bin/env -S deno run --allow-net --allow-read --allow-run --allow-write --unstable-net she-bang there, couldn't this be just:
./dimos/teleop/quest/web/teleop_server.tsThere 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.
Yep, updated the README and added the execute bit.
Git tracks the file mode, so it'll be executable for all after this commit.
| console.log(`Server: https://localhost:${PORT}`); | ||
|
|
||
| // Forward all raw packets to browser (we are decoding LCM directly in the browser) | ||
| lcm.subscribePacket((packet) => { |
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.
oof you are sending ALL of the dimos pubsub packets into quest, this is A LOT of data that's not being processed at all, just loads the CPU and I assume will crash on applications that involve cameras or sensors
| ws.onclose = () => { | ||
| setStatus('WebSocket closed'); | ||
| }; | ||
| ws.onmessage = () => {}; |
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.
as you are forwarding every pubsub message, this function is actually called at hundreds or thousands of times a second
| buttons.push(gamepad.buttons[i]?.pressed ? 1 : 0); | ||
| } | ||
|
|
||
| const joyMsg = new sensor_msgs.Joy({ |
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.
nice, yes, that looks like a correct msg
leshy
left a comment
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.
there is this issue with forwarding all LCM packets to quest otherwise looks good.
Generally for PRs pls include an example how to run this so people can test easily, I'm missing for example an XAarm teleop blueprint or something to make it clear how this is to be used
dimos/teleop/quest/web/README.md
Outdated
| Deno server that bridges WebSocket and LCM: | ||
| - Serves WebXR client over HTTPS (required for Quest) | ||
| - Forwards controller data from browser to LCM | ||
| - Forwards LCM packets to browser |
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.
it shouldn't forward any LCM packets to browser right?
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.
Okay yes. Was trying out sending images to display them in the headset. This PR doesn't have it - not forwarding any messages to the browser. Let me remove the lines related to it. Thanks for pointing it out!
| document.getElementById('status').textContent = `Error: ${msg}`; | ||
| }; | ||
|
|
||
| import { encodePacket, geometry_msgs, std_msgs, sensor_msgs } from "https://esm.sh/jsr/@dimos/msgs@0.1.4"; |
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.
all good, just so we are aware, dependancy on the external package sstore makes the system not work without an internet connection on quest
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.
yeah, its loading the msgs package from an external CDN at runtime. can mention this somewhere in README for now.
| @@ -0,0 +1,95 @@ | |||
| #!/usr/bin/env -S deno run --allow-net --allow-read --allow-run --allow-write --unstable-net | |||
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.
for the next pass, generally I'd want to auto-start this somehow as a part of the blueprint, so I don't need an extra terminal when testing.. issue here is that this is TS so not a standard dimos module. we could add an empty hosting module that just runs the TS file
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.
yes, i was thinking about it. would be nice if we auto-start in the blueprint
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.
I think actual QuestTeleopModule should start this and stop it maybe? would this make it easily usable in blueprints? How come there is no example blueprint that works with xarm btw?
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.
That might be one way to do it. If QuestTeleopModule.start() autolaunches the server and if stop() kills it, there's no need for extra terminal, and nothing to mention in blueprint. This can be implemented using subprocess, and can be solved by making few additions in start() and stop().
But, this involves including server related lines in the Module, which doesn't feel very good. Receiving msgs from device via LCM gave us the benefit to keep the module independent of server/ws related code. So, not entirely sure on this.
Anonther pro - This webserver and deno bridge are explicitly for quest. so, just including its lifecycle with the quest module makes sense, rather than another hosting module to connect with quest module.
Its highly unlikely we would use that TS-server-module to connect with a non-quest module.
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.
Reg xarm+quest_teleop blueprint, as mentioned in the below comment - few changes and additions are required in dioms/control/tasks for the teleop to work with arms. So, preparing to make another short PR for this.
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.
In terms of pro/cons - you are designing an interface, think about what other developer "as a user" wants from you. They don't want to know or mange your lifecycle or implementation details, is it deno server? is it .apk using TCP? no one wants to know actually.
Ideally a dev can think only about stuff they care about, so topics you provide for teleop and start/stop, the rest is implementation details. so IMO starting a server (and whatever else you need to provide your interface) is ok but approving the PR for now and you can iterate
Modified - No forwarding to quest in this PR. Will be adding this feature later
yes, added test in PR description. We can run a blueprint and see poses and button presses through rerun viz. |
|
#1222 there is a way to do this without a separate TS server, others can prioritize when/if to do this |
Great. Will look into this, and any other alternatives too. I was trying a similar approach having a weserver on python side and receiving msgs in JSON. But, switched to have it in LCM to maintain DimOS style |
leshy
left a comment
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.
approving with proposed imho better approach #1222
Feature
Issue - Linear
closes DIM-420
closes DIM-394
Approach/Solution
Web stuff
QuestTeleopModulesubscribes to those LCM channels and decodesArchitecture
QuestTeleopModulereceives these Pose/Joy LCM streams. once engaged, computes deltas and publishes msg commands.QuestControllerState/'QuestButtons' wraps over raw Joy messages for clean controller data access.ArmTeleopModule(toggle-engage arm control),TwistTeleopModule(velocity commands),VisualizingTeleopModule(Rerun debug overlay) are included.Refer for Detailed Spec
Breaking Changes
None
How to Test
If you have Quest3
./dimos/teleop/quest/web/teleop_server.tsdimos run arm-teleop-visualizinghttps://<host-ip>:8443and click connect - opens passthroughFiles
assets/teleop_certs- for generated self-signed TLS certsdimos/teleop/- full subsystem (protocol, quest module, types, extensions, transforms, visualization, blueprints)dimos/msgs/std_msgs/UInt32.py- base type forQuestButtonsdimos/teleop/quest/web/- Deno bridge + WebXR client