Skip to content

Conversation

@hougantc-nvda
Copy link
Contributor

This PR adds XR motion controller support to the G1 robot locomanipulation environment (Isaac-PickPlace-Locomanipulation-G1-Abs-v0). Users can now teleoperate the G1 humanoid robot using XR motion controllers in addition to hand tracking, providing more robust locomotion and physical button controls for enhanced XR experiences. The implementation includes new retargeters for upper and lower body control and improved headset anchoring logic that anchors the XR headset to the robot. Users can now move through physical spaces while operating the robot and detach rotation for static locomanipulation tasks, providing flexible control modes for different operational scenarios.

Type of change

  • New feature (non-breaking change which adds functionality)

Screenshots

Please attach before and after screenshots of the change if applicable.

Checklist

  • I have read and understood the contribution guidelines
  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the changelog and the corresponding version in the extension's config/extension.toml file
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

…me. Update config values to compensate.

* Add support for decoupling anchor rotation as a toggle when pressing right controller A button.
* Fix reset not working
…oller. If we are doing only "handtracking" as teleop_device, then we don't want all the headset anchoring code.
…n instantiated so we know what device we dealing with and if we need to change the ground plane material
@github-actions github-actions bot added isaac-mimic Related to Isaac Mimic team isaac-lab Related to Isaac Lab team labels Oct 24, 2025
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Overview

Greptile Summary

This PR extends XR teleoperation capabilities for the G1 humanoid robot by adding motion controller support alongside existing hand tracking. The implementation introduces OpenXRDeviceMotionController class and corresponding retargeters (G1TriHandUpperBodyMotionControllerRetargeter, G1LowerBodyStandingMotionControllerRetargeter) that map thumbstick, trigger, and button inputs to robot locomotion and hand joint commands. A new callback system (add_pre_render_callback) allows XR devices to update anchor positions before rendering, enabling dynamic headset anchoring to the robot's pelvis. The environment class LocomanipulationG1ManagerBasedRLEnv extends base functionality with XR-specific USD stage modifications (hiding robot head mesh, changing ground materials) for improved user comfort. The anchor rotation system supports multiple modes (FIXED, FOLLOW_PRIM, FOLLOW_PRIM_SMOOTHED) allowing users to toggle between static and dynamic control scenarios via controller button press.

Important Files Changed

Filename Score Overview
source/isaaclab/isaaclab/devices/openxr/openxr_device_controller.py 3/5 Adds motion controller device class with button bindings, headset anchoring, and rotation mode management; subscription cleanup may leak resources and config override is silent
source/isaaclab/isaaclab/devices/openxr/retargeters/humanoid/unitree/g1_lower_body_standing.py 2/5 Implements lower body retargeter mapping thumbsticks to locomotion; uses wall clock time causing non-determinism and has no hip height bounds
source/isaaclab/isaaclab/devices/openxr/xr_cfg.py 3/5 Adds anchor configuration and rotation modes; critical bug in remove_camera_configs uses wrong variable name for deletion (line 138)
source/isaaclab_tasks/isaaclab_tasks/manager_based/locomanipulation/pick_place/locomanipulation_g1_env_cfg.py 3/5 Registers motion controller device and adjusts XR anchor; hardcodes env_0 path breaking multi-environment setups
source/isaaclab/isaaclab/devices/openxr/retargeters/humanoid/unitree/trihand/g1_upper_body_retargeter.py 3/5 Adds upper body retargeter with button-to-finger mappings; quaternion format docs conflict with implementation (line 280) and unused input validation
source/isaaclab_tasks/isaaclab_tasks/manager_based/locomanipulation/pick_place/locomanipulation_g1_env.py 3/5 Custom environment class with XR comfort modifications; hardcoded env_0 paths and private attribute access
source/isaaclab/isaaclab/envs/manager_based_env.py 5/5 Adds pre-render callback system for injecting custom logic before rendering
scripts/environments/teleoperation/teleop_se3_agent.py 4/5 Integrates motion controllers into teleop script with XR initialization and callback registration
source/isaaclab/isaaclab/devices/openxr/openxr_device.py 4/5 Adds dynamic anchor prim path support for following robot movements
source/isaaclab/isaaclab/devices/device_base.py 5/5 Adds abstract on_pre_render() hook for device subclasses
source/isaaclab/isaaclab/envs/manager_based_rl_env.py 3/5 Invokes pre-render hook in physics loop before rendering
source/isaaclab/isaaclab/devices/teleop_device_factory.py 4/5 Registers new motion controller device and retargeter types in factory maps
source/isaaclab_tasks/isaaclab_tasks/manager_based/locomanipulation/pick_place/init.py 4/5 Changes gym registration to use custom environment class enabling XR support
source/isaaclab/isaaclab/devices/openxr/init.py 4/5 Exports motion controller classes and enums; docstring still only mentions hand tracking
source/isaaclab/isaaclab/devices/init.py 4/5 Exports new motion controller classes to public API; module docstring outdated
source/isaaclab/isaaclab/devices/openxr/retargeters/init.py 5/5 Exports new motion controller retargeter classes
source/isaaclab_tasks/docs/CHANGELOG.rst 3.5/5 Documents feature addition; date shows 2025 instead of 2024 and missing ReStructuredText formatting
source/isaaclab_tasks/config/extension.toml 5/5 Version bump from 0.11.6 to 0.11.7
CONTRIBUTORS.md 5/5 Adds contributor name in alphabetical order

Confidence score: 3/5

  • This PR requires careful review due to several critical issues including hardcoded paths that break multi-environment setups, non-deterministic behavior from wall clock timing, and potential resource leaks in subscription management.
  • Score reflects critical bugs in remove_camera_configs (wrong variable name), hardcoded env_0 paths that prevent scaling to multiple environments, wall clock time usage causing non-deterministic hip height adjustments, subscription cleanup that may leak resources, and missing bounds validation on hip height that could accumulate to invalid values.
  • Pay close attention to source/isaaclab/isaaclab/devices/openxr/xr_cfg.py (line 138 variable name bug), source/isaaclab_tasks/isaaclab_tasks/manager_based/locomanipulation/pick_place/locomanipulation_g1_env_cfg.py (line 216 hardcoded path), source/isaaclab/isaaclab/devices/openxr/retargeters/humanoid/unitree/g1_lower_body_standing.py (wall clock timing and unbounded hip height), and source/isaaclab/isaaclab/devices/openxr/openxr_device_controller.py (subscription cleanup and silent config override).

Sequence Diagram

sequenceDiagram
    participant User
    participant teleop_se3_agent
    participant AppLauncher
    participant LocomanipulationG1ManagerBasedRLEnv
    participant OpenXRDeviceMotionController
    participant G1TriHandUpperBodyMotionControllerRetargeter
    participant G1LowerBodyStandingMotionControllerRetargeter
    participant SimulationContext
    participant InteractiveScene
    
    User->>teleop_se3_agent: Run with --xr and --teleop_device motion_controllers
    teleop_se3_agent->>teleop_se3_agent: Parse arguments (enable XR, set device)
    teleop_se3_agent->>AppLauncher: Create with xr=True
    AppLauncher->>SimulationContext: Initialize simulation
    
    teleop_se3_agent->>teleop_se3_agent: parse_env_cfg()
    teleop_se3_agent->>teleop_se3_agent: remove_camera_configs()
    teleop_se3_agent->>teleop_se3_agent: Set env_cfg.xr_enabled = True
    
    teleop_se3_agent->>LocomanipulationG1ManagerBasedRLEnv: gym.make(task, cfg=env_cfg)
    LocomanipulationG1ManagerBasedRLEnv->>InteractiveScene: Create scene with G1 robot
    InteractiveScene->>SimulationContext: Setup physics and render
    
    teleop_se3_agent->>teleop_se3_agent: create_teleop_device(motion_controllers)
    teleop_se3_agent->>OpenXRDeviceMotionController: Initialize with retargeters
    OpenXRDeviceMotionController->>G1TriHandUpperBodyMotionControllerRetargeter: Create retargeter
    OpenXRDeviceMotionController->>G1LowerBodyStandingMotionControllerRetargeter: Create retargeter
    OpenXRDeviceMotionController->>OpenXRDeviceMotionController: Setup XR anchor with rotation mode
    OpenXRDeviceMotionController->>OpenXRDeviceMotionController: Bind button callbacks (A button for anchor toggle)
    
    teleop_se3_agent->>teleop_se3_agent: Register callbacks (START, STOP, RESET)
    teleop_se3_agent->>LocomanipulationG1ManagerBasedRLEnv: setup_for_teleop_device()
    LocomanipulationG1ManagerBasedRLEnv->>LocomanipulationG1ManagerBasedRLEnv: Hide robot head mesh
    LocomanipulationG1ManagerBasedRLEnv->>LocomanipulationG1ManagerBasedRLEnv: Change ground material
    
    teleop_se3_agent->>LocomanipulationG1ManagerBasedRLEnv: reset()
    LocomanipulationG1ManagerBasedRLEnv->>InteractiveScene: Reset scene state
    teleop_se3_agent->>OpenXRDeviceMotionController: reset()
    
    teleop_se3_agent->>LocomanipulationG1ManagerBasedRLEnv: add_pre_render_callback()
    
    loop Simulation Loop
        User->>OpenXRDeviceMotionController: Move controllers / Press buttons
        OpenXRDeviceMotionController->>OpenXRDeviceMotionController: _query_controller() for left/right
        OpenXRDeviceMotionController->>OpenXRDeviceMotionController: Read thumbstick, trigger, squeeze, buttons
        
        alt User presses A button
            OpenXRDeviceMotionController->>OpenXRDeviceMotionController: _toggle_anchor_rotation()
        end
        
        teleop_se3_agent->>OpenXRDeviceMotionController: advance()
        OpenXRDeviceMotionController->>OpenXRDeviceMotionController: _get_raw_data()
        OpenXRDeviceMotionController->>G1TriHandUpperBodyMotionControllerRetargeter: retarget(controller_data)
        G1TriHandUpperBodyMotionControllerRetargeter->>G1TriHandUpperBodyMotionControllerRetargeter: Extract wrist poses
        G1TriHandUpperBodyMotionControllerRetargeter->>G1TriHandUpperBodyMotionControllerRetargeter: Map trigger/squeeze to hand joints
        G1TriHandUpperBodyMotionControllerRetargeter-->>OpenXRDeviceMotionController: Return [left_wrist(7), right_wrist(7), hand_joints(14)]
        
        OpenXRDeviceMotionController->>G1LowerBodyStandingMotionControllerRetargeter: retarget(controller_data)
        G1LowerBodyStandingMotionControllerRetargeter->>G1LowerBodyStandingMotionControllerRetargeter: Extract thumbstick inputs
        G1LowerBodyStandingMotionControllerRetargeter->>G1LowerBodyStandingMotionControllerRetargeter: Calculate movement and hip height
        G1LowerBodyStandingMotionControllerRetargeter-->>OpenXRDeviceMotionController: Return [x_vel, y_vel, yaw_vel, hip_height]
        
        OpenXRDeviceMotionController-->>teleop_se3_agent: Return concatenated actions
        
        alt Teleoperation Active
            teleop_se3_agent->>LocomanipulationG1ManagerBasedRLEnv: step(actions)
            LocomanipulationG1ManagerBasedRLEnv->>LocomanipulationG1ManagerBasedRLEnv: Process upper body IK actions
            LocomanipulationG1ManagerBasedRLEnv->>LocomanipulationG1ManagerBasedRLEnv: Process lower body joint actions
            
            loop Decimation steps
                LocomanipulationG1ManagerBasedRLEnv->>SimulationContext: step(render=False)
                SimulationContext->>InteractiveScene: Update physics
                
                alt Render interval reached
                    LocomanipulationG1ManagerBasedRLEnv->>OpenXRDeviceMotionController: on_pre_render()
                    OpenXRDeviceMotionController->>OpenXRDeviceMotionController: _sync_headset_to_anchor()
                    OpenXRDeviceMotionController->>OpenXRDeviceMotionController: Calculate anchor position and rotation
                    OpenXRDeviceMotionController->>OpenXRDeviceMotionController: Apply smoothing if FOLLOW_PRIM_SMOOTHED
                    SimulationContext->>SimulationContext: render()
                end
            end
            
            LocomanipulationG1ManagerBasedRLEnv->>LocomanipulationG1ManagerBasedRLEnv: Check terminations
            LocomanipulationG1ManagerBasedRLEnv->>LocomanipulationG1ManagerBasedRLEnv: Compute observations
            LocomanipulationG1ManagerBasedRLEnv-->>teleop_se3_agent: Return obs, extras
        else Teleoperation Inactive
            SimulationContext->>SimulationContext: render()
        end
        
        alt Reset triggered
            teleop_se3_agent->>LocomanipulationG1ManagerBasedRLEnv: reset()
            teleop_se3_agent->>OpenXRDeviceMotionController: reset()
        end
    end
    
    User->>teleop_se3_agent: Close application
    teleop_se3_agent->>LocomanipulationG1ManagerBasedRLEnv: close()
    teleop_se3_agent->>SimulationContext: close()
Loading

Additional Comments (2)

  1. source/isaaclab/isaaclab/devices/openxr/__init__.py, line 6 (link)

    syntax: docstring incorrectly refers to 'Keyboard device' but this is the OpenXR device module

  2. source/isaaclab/isaaclab/devices/__init__.py, line 13 (link)

    style: Module docstring describes OpenXR only for hand tracking ("index/thumb tip avg"), but this PR adds motion controller support. Update description to mention motion controllers.

19 files reviewed, 17 comments

Edit Code Review Agent Settings | Greptile

Comment on lines 110 to 117
if self._xr_cfg.anchor_prim_path is not None:
self._xr_anchor_headset_path = f"{self._xr_cfg.anchor_prim_path}/XRAnchor"
else:
self._xr_anchor_headset_path = "/World/XRAnchor"

_ = SingleXFormPrim(
self._xr_anchor_headset_path, position=self._xr_cfg.anchor_pos, orientation=self._xr_cfg.anchor_rot
)
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: the anchor prim path concatenation doesn't validate if anchor_prim_path ends with a slash, which could result in malformed paths like /path//XRAnchor. Should anchor_prim_path be normalized to ensure it doesn't end with a trailing slash?

elif "reset" in msg:
if "RESET" in self._additional_callbacks:
self._additional_callbacks["RESET"]()
self.reset()
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: calling self.reset() after the user's RESET callback could cause issues if the callback expects non-reset state. Should reset() be called before the user callback instead, or should this be left to the user's discretion?

Comment on lines 306 to 307
if len(inputs) <= MotionControllerInputIndex.BUTTON_0.value:
return hand_joints
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: check uses wrong index - BUTTON_0 is not used in the function, should check against max of TRIGGER and SQUEEZE indices

Comment on lines +88 to +89
headpose: Previous head pose as numpy array [x, y, z, w, x, y, z] (position + quaternion)
pose: Anchor prim pose as numpy array [x, y, z, w, x, y, z] (position + quaternion)
Copy link
Contributor

Choose a reason for hiding this comment

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

syntax: Docstring parameter names ('headpose', 'pose') don't match the lambda parameter names ('headpose', 'primpose'). Should be consistent.

@hougantc-nvda hougantc-nvda marked this pull request as draft October 24, 2025 21:52
@hougantc-nvda
Copy link
Contributor Author

For IsaacLab 2.4/3.0.

pass


class OpenXRDeviceMotionController(OpenXRDevice):
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm not sure if I like extending this base class or just adding the logic to the OpenXRDevice itself better. Why did you do it this way instead?

# GR1T2 retargeter
import pinocchio # noqa: F401
if "handtracking" in args_cli.teleop_device.lower():
if "handtracking" in args_cli.teleop_device.lower() or "motion_controllers" in args_cli.teleop_device.lower():
Copy link
Contributor

Choose a reason for hiding this comment

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

We should remove this and just make the user pass in "--xr" instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

isaac-lab Related to Isaac Lab team isaac-mimic Related to Isaac Mimic team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants