-
Notifications
You must be signed in to change notification settings - Fork 5
Update MIRA tutorial to match up with new assets #154
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
Signed-off-by: Yiheng Wang <[email protected]>
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.
Pull Request Overview
This PR updates the MIRA tutorial to align with the new assets and key mapping configurations. Key changes include reordering and updating imports and configurations, replacing hardcoded asset paths with values from the new configuration module, and adjusting the keyboard event handling logic to use the updated key maps.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tutorials/assets/bring_your_own_robot/Virtual_Incision_MIRA/teleoperate_virtual_incision_mira.py | Updated asset management, key mapping integration, and joint update logic to match new configurations. |
| tutorials/assets/bring_your_own_robot/Virtual_Incision_MIRA/mira_configs.py | Added new configuration module for asset paths and key mappings. |
Comments suppressed due to low confidence (1)
tutorials/assets/bring_your_own_robot/Virtual_Incision_MIRA/teleoperate_virtual_incision_mira.py:82
- [nitpick] The transition from uppercase constants (SWITCH_KEY) to lowercase (switch_key) in the updated code is acceptable given the new configurations, but ensure consistency across the code base and documentation.
if key == switch_key:
...rials/assets/bring_your_own_robot/Virtual_Incision_MIRA/teleoperate_virtual_incision_mira.py
Show resolved
Hide resolved
Signed-off-by: Yiheng Wang <[email protected]>
Signed-off-by: Yiheng Wang <[email protected]>
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.
Pull Request Overview
This PR updates the Virtual Incision MIRA tutorial to work with the new asset layout by externalizing asset paths and input mappings into a dedicated config module, and refactoring the main teleoperation script.
- Extracted USD paths, joint/camera mappings, and keys into
mira_configs.py. - Refactored
teleoperate_virtual_incision_mira.pyto import from the new config, reorganize theAssetsclass, and streamline initialization. - Adjusted keyboard handling to support rotation and grasp inputs and updated import ordering.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tutorials/assets/bring_your_own_robot/Virtual_Incision_MIRA/teleoperate_virtual_incision_mira.py | Refactored to use mira_configs, reorganized imports and asset initialization, and updated joint/camera update logic. |
| tutorials/assets/bring_your_own_robot/Virtual_Incision_MIRA/mira_configs.py | New configuration module defining asset paths, key maps, and related constants. |
Comments suppressed due to low confidence (1)
tutorials/assets/bring_your_own_robot/Virtual_Incision_MIRA/teleoperate_virtual_incision_mira.py:130
- Since this loop only handles a single index, consider operating directly on
camera_prims[0]instead of iterating over a single-element list to simplify the code.
for i in [0]:
...rials/assets/bring_your_own_robot/Virtual_Incision_MIRA/teleoperate_virtual_incision_mira.py
Show resolved
Hide resolved
...rials/assets/bring_your_own_robot/Virtual_Incision_MIRA/teleoperate_virtual_incision_mira.py
Show resolved
Hide resolved
tutorials/assets/bring_your_own_robot/Virtual_Incision_MIRA/mira_configs.py
Show resolved
Hide resolved
Signed-off-by: Yiheng Wang <[email protected]>
|
Hi @yiheng-wang-nv , if the new assets are working fine, can you update the hash in the i4h-asset-catalog repo? Thanks! |
Hi @mingxin-zheng , thanks, I tested and submitted a PR: isaac-for-healthcare/i4h-asset-catalog#24 |
Fixes #151
Description