-
Notifications
You must be signed in to change notification settings - Fork 602
Pim/feat/g1 groot wbc #2033
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
Open
Nabla7
wants to merge
33
commits into
main
Choose a base branch
from
pim/feat/g1-groot-wbc
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Pim/feat/g1 groot wbc #2033
Changes from all commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
fe0d810
feat(g1-wbc): groot-wbc real-hw + sim blueprints
Nabla7 35c5b28
feat(g1-wbc-sim): DIMOS_MUJOCO_VIEW=1 spawns native MuJoCo viewer in …
Nabla7 bf83913
fix(g1-wbc-sim): inject menagerie mesh assets in MuJoCo viewer subpro…
Nabla7 b7bb6c9
fix(g1-wbc-sim): tick_rate=50 + decimation=1 + ramp=0 to match GR00T …
Nabla7 c3c0f82
fix(g1-wbc-sim): strip lidar/scene-mesh/multi-camera from mujoco_sim_…
Nabla7 a3ccd2c
chore(g1-wbc): drop unused drive_trains/go2 + revert pyproject/uv.loc…
Nabla7 d039820
fix: restore dev's unitree_go2 drive_train (accidentally deleted in c…
Nabla7 0ed6efb
chore(g1-wbc): drop perception2/viser leaks + test file (defer to fol…
Nabla7 3c1aacc
chore(g1-wbc): drop __init__.py files, match repo namespace-package c…
Nabla7 8825549
chore(g1-wbc): drop simulation/mujoco/model.py changes (unrelated to PR)
Nabla7 62a1110
chore(g1-wbc): drop unused make_quadruped_joints + GRIPPER enum (carr…
Nabla7 2d2ba1d
chore(g1-wbc): strip unrelated leftovers (lidar scene_option, get_cam…
Nabla7 fef9460
fix(g1-wbc): passive lowstate sub + restore arm/dry_run wiring
Nabla7 a400d26
chore(g1-wbc): restore unrelated files accidentally swept into PR
Nabla7 b553645
Merge remote-tracking branch 'origin/dev' into pim/feat/g1-groot-wbc
Nabla7 a7aa3bf
fix(coordinator): merge duplicate _create_whole_body_adapter
Nabla7 74b6404
refactor(g1-wbc): drop monolith adapter, use Mustafa's transport bridge
Nabla7 7f91e52
fix(g1-wbc-sim): resolve MJCF path via get_data so CWD doesn't matter
Nabla7 117df92
refactor(g1-wbc): paul-review fixes + bridge composition smoke test
Nabla7 995c735
fix(g1-wbc): safe-stop lowcmd on disconnect + skip release when idle
Nabla7 224e7e9
chore(g1-wbc): strip section-marker comments
Nabla7 bbcc777
fix(g1-wbc): mypy errors uncovered by full CI-matching env
Nabla7 38d6c91
-
jeff-hykin 4aa9d08
-
jeff-hykin 76365d0
refactor(g1-groot-wbc): address PR #2033 review comments
Nabla7 14f1fbc
refactor(g1-groot-wbc): IMU on CoordinatorState, drop read_odom from …
Nabla7 b8db4ba
refactor(sim): MujocoEngine runs as main thread in a subprocess
Nabla7 863b2cc
fix(cli): apply CLI-config overrides to global_config before blueprin…
Nabla7 cc81d3b
fix(g1-wbc): harden sim subprocess review fixes
Nabla7 dda52aa
fix(cli): avoid resolving LFS defaults in help
Nabla7 98b427b
fix(g1-wbc): satisfy project policy checks
Nabla7 49eb7d4
refactor(g1-wbc): make task registry lazy
Nabla7 1960395
fix(g1-wbc): wire sim command controls
Nabla7 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Git LFS file not shown
Git LFS file not shown
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,29 +75,44 @@ class TaskConfig: | |
|
|
||
| Attributes: | ||
| name: Task name (e.g., "traj_arm") | ||
| type: Task type ("trajectory", "servo", "velocity", "cartesian_ik", "teleop_ik") | ||
| type: Registered task type name. See ``dimos.control.tasks.registry``. | ||
| joint_names: List of joint names this task controls | ||
| priority: Task priority (higher wins arbitration) | ||
| model_path: Path to URDF/MJCF for IK solver (cartesian_ik/teleop_ik only) | ||
| model_path: Optional model path used by task factories that need one. | ||
| ee_joint_id: End-effector joint ID in model (cartesian_ik/teleop_ik only) | ||
| hand: "left" or "right" controller hand (teleop_ik only) | ||
| gripper_joint: Joint name for gripper virtual joint | ||
| gripper_open_pos: Gripper position at trigger 0.0 | ||
| gripper_closed_pos: Gripper position at trigger 1.0 | ||
| hardware_id: Optional hardware dependency used by task factories. | ||
| """ | ||
|
|
||
| name: str | ||
| type: str = "trajectory" | ||
| joint_names: list[str] = field(default_factory=lambda: []) | ||
| priority: int = 10 | ||
| # Cartesian IK / Teleop IK specific | ||
| # Cartesian IK / Teleop IK / GR00T WBC specific | ||
| model_path: str | Path | None = None | ||
| ee_joint_id: int = 6 | ||
| hand: Literal["left", "right"] | None = None # teleop_ik only | ||
| # Teleop IK gripper specific | ||
| gripper_joint: str | None = None | ||
| gripper_open_pos: float = 0.0 | ||
| gripper_closed_pos: float = 0.0 | ||
| # For tasks that need access to a configured hardware adapter. | ||
| hardware_id: str | None = None | ||
| # Servo task: optional initial target held until/unless a new one arrives. | ||
| default_positions: list[float] | None = None | ||
| # Start the task immediately after registration. | ||
| auto_start: bool = False | ||
| # For tasks with arm()/disarm(): arm automatically on start. | ||
| auto_arm: bool = False | ||
| # For tasks with dry-run support: compute but suppress output on start. | ||
| auto_dry_run: bool = False | ||
| # Default arming ramp duration, for tasks that interpolate on arm(). | ||
| default_ramp_seconds: float = 10.0 | ||
| # Optional task-level compute decimation; ``None`` keeps the task default. | ||
| decimation: int | None = None | ||
|
|
||
|
|
||
| class ControlCoordinatorConfig(ModuleConfig): | ||
|
|
@@ -110,6 +125,11 @@ class ControlCoordinatorConfig(ModuleConfig): | |
| log_ticks: Whether to log tick information (verbose) | ||
| hardware: List of hardware configurations to create on start | ||
| tasks: List of task configurations to create on start | ||
|
|
||
| Odom is no longer published here — most ``WholeBodyAdapter`` impls | ||
| don't (and shouldn't) produce base pose. Sim engines publish odom | ||
| on their own Out port; real-hardware setups attach a dedicated | ||
| estimator Module that fuses IMU + leg kinematics. | ||
| """ | ||
|
|
||
| tick_rate: float = 100.0 | ||
|
|
@@ -166,11 +186,13 @@ class ControlCoordinator(Module): | |
| # Input: Teleop buttons for engage/disengage signaling | ||
| buttons: In[Buttons] | ||
|
|
||
| # Arming and dry-run are one-shot RPCs, not streams. | ||
|
|
||
| def __init__(self, *args: Any, **kwargs: Any) -> None: | ||
| super().__init__(*args, **kwargs) | ||
|
|
||
| # Connected hardware (keyed by hardware_id) | ||
| self._hardware: dict[HardwareId, ConnectedHardware] = {} | ||
| self._hardware: dict[HardwareId, ConnectedHardware | ConnectedWholeBody] = {} | ||
| self._hardware_lock = threading.Lock() | ||
|
|
||
| # Joint -> hardware mapping (built when hardware added) | ||
|
|
@@ -203,6 +225,10 @@ def _setup_from_config(self) -> None: | |
| for task_cfg in self.config.tasks: | ||
| task = self._create_task_from_config(task_cfg) | ||
| self.add_task(task) | ||
| if task_cfg.auto_start: | ||
| start = getattr(task, "start", None) | ||
| if callable(start): | ||
| start() | ||
|
|
||
| except Exception: | ||
| # Rollback: clean up all successfully added hardware | ||
|
|
@@ -262,105 +288,36 @@ def _create_twist_base_adapter(self, component: HardwareComponent) -> TwistBaseA | |
| def _create_whole_body_adapter(self, component: HardwareComponent) -> WholeBodyAdapter: | ||
| """Create a whole-body adapter from component config. | ||
|
|
||
| ``component.address`` carries the DDS network interface — int (CAN port) | ||
| or str ("enp60s0"); cyclonedds requires the right type, so try int() first | ||
| and fall back to keeping the original string. | ||
| ``component.address`` is the universal hardware-locator field. | ||
| It carries the DDS network interface for real-hw adapters (e.g. | ||
| ``"enp2s0"``) and the MJCF path for sim adapters — each adapter | ||
| knows how to interpret a string vs. an integer in its own | ||
| ``__init__``. Adapters that need extra arguments accept them via | ||
| ``**component.adapter_kwargs``. This unification (vs. the earlier | ||
| dual ``address`` + ``network_interface``) matches the convention | ||
| used by ``_create_manipulator_adapter`` and | ||
| ``_create_twist_base_adapter``. | ||
| """ | ||
| from dimos.hardware.whole_body.registry import whole_body_adapter_registry | ||
|
|
||
| addr: int | str | None = component.address | ||
| if addr is not None: | ||
| try: | ||
| addr = int(addr) | ||
| except ValueError: | ||
| pass # keep as string (e.g. "enp60s0") | ||
|
|
||
| return whole_body_adapter_registry.create( | ||
| component.adapter_type, | ||
| dof=len(component.joints), | ||
| hardware_id=component.hardware_id, | ||
| network_interface=addr if addr is not None else "", | ||
| address=component.address, | ||
| domain_id=component.domain_id, | ||
| **component.adapter_kwargs, | ||
| ) | ||
|
|
||
| def _create_task_from_config(self, cfg: TaskConfig) -> ControlTask: | ||
| """Create a control task from config.""" | ||
| task_type = cfg.type.lower() | ||
|
|
||
| if task_type == "trajectory": | ||
| from dimos.control.tasks.trajectory_task import ( | ||
| JointTrajectoryTask, | ||
| JointTrajectoryTaskConfig, | ||
| ) | ||
|
|
||
| return JointTrajectoryTask( | ||
| cfg.name, | ||
| JointTrajectoryTaskConfig( | ||
| joint_names=cfg.joint_names, | ||
| priority=cfg.priority, | ||
| ), | ||
| ) | ||
|
|
||
| elif task_type == "servo": | ||
| from dimos.control.tasks.servo_task import JointServoTask, JointServoTaskConfig | ||
|
|
||
| return JointServoTask( | ||
| cfg.name, | ||
| JointServoTaskConfig( | ||
| joint_names=cfg.joint_names, | ||
| priority=cfg.priority, | ||
| ), | ||
| ) | ||
|
|
||
| elif task_type == "velocity": | ||
| from dimos.control.tasks.velocity_task import JointVelocityTask, JointVelocityTaskConfig | ||
|
|
||
| return JointVelocityTask( | ||
| cfg.name, | ||
| JointVelocityTaskConfig( | ||
| joint_names=cfg.joint_names, | ||
| priority=cfg.priority, | ||
| ), | ||
| ) | ||
|
|
||
| elif task_type == "cartesian_ik": | ||
| from dimos.control.tasks.cartesian_ik_task import CartesianIKTask, CartesianIKTaskConfig | ||
|
|
||
| if cfg.model_path is None: | ||
| raise ValueError(f"CartesianIKTask '{cfg.name}' requires model_path in TaskConfig") | ||
|
|
||
| return CartesianIKTask( | ||
| cfg.name, | ||
| CartesianIKTaskConfig( | ||
| joint_names=cfg.joint_names, | ||
| model_path=cfg.model_path, | ||
| ee_joint_id=cfg.ee_joint_id, | ||
| priority=cfg.priority, | ||
| ), | ||
| ) | ||
| """Create a control task from config via the task registry. | ||
|
|
||
| elif task_type == "teleop_ik": | ||
| from dimos.control.tasks.teleop_task import TeleopIKTask, TeleopIKTaskConfig | ||
|
|
||
| if cfg.model_path is None: | ||
| raise ValueError(f"TeleopIKTask '{cfg.name}' requires model_path in TaskConfig") | ||
|
|
||
| return TeleopIKTask( | ||
| cfg.name, | ||
| TeleopIKTaskConfig( | ||
| joint_names=cfg.joint_names, | ||
| model_path=cfg.model_path, | ||
| ee_joint_id=cfg.ee_joint_id, | ||
| priority=cfg.priority, | ||
| hand=cfg.hand, | ||
| gripper_joint=cfg.gripper_joint, | ||
| gripper_open_pos=cfg.gripper_open_pos, | ||
| gripper_closed_pos=cfg.gripper_closed_pos, | ||
| ), | ||
| ) | ||
| Each task module self-registers a factory under its type name. | ||
| Factories can resolve hardware dependencies from the hardware map. | ||
| """ | ||
| from dimos.control.tasks.registry import control_task_registry | ||
|
|
||
| else: | ||
| raise ValueError(f"Unknown task type: {task_type}") | ||
| return control_task_registry.create(cfg.type, cfg, hardware=self._hardware) | ||
|
|
||
| @rpc | ||
| def add_hardware( | ||
|
|
@@ -594,12 +551,45 @@ def _on_twist_command(self, msg: Twist) -> None: | |
| joint_state = JointState(name=names, velocity=velocities) | ||
| self._on_joint_command(joint_state) | ||
|
|
||
| # Velocity-capable tasks opt in with set_velocity_command(). | ||
| t_now = time.perf_counter() | ||
| with self._task_lock: | ||
| for task in self._tasks.values(): | ||
| set_vel = getattr(task, "set_velocity_command", None) | ||
| if set_vel is not None: | ||
| set_vel(msg.linear.x, msg.linear.y, msg.angular.z, t_now) | ||
|
|
||
| def _on_buttons(self, msg: Buttons) -> None: | ||
| """Forward button state to all tasks.""" | ||
| with self._task_lock: | ||
| for task in self._tasks.values(): | ||
| task.on_buttons(msg) | ||
|
|
||
| @rpc | ||
| def set_activated(self, engaged: bool) -> None: | ||
| """Arm/disarm every task exposing ``arm()`` / ``disarm()``.""" | ||
| with self._task_lock: | ||
| for task in self._tasks.values(): | ||
| method_name = "arm" if engaged else "disarm" | ||
| handler = getattr(task, method_name, None) | ||
| if callable(handler): | ||
| try: | ||
| handler() | ||
| except Exception: | ||
| logger.exception(f"{method_name}() raised on task {task.name!r}") | ||
|
|
||
| @rpc | ||
| def set_dry_run(self, enabled: bool) -> None: | ||
| """Toggle dry-run on every task exposing ``set_dry_run``.""" | ||
| with self._task_lock: | ||
| for task in self._tasks.values(): | ||
| handler = getattr(task, "set_dry_run", None) | ||
| if callable(handler): | ||
| try: | ||
| handler(enabled) | ||
| except Exception: | ||
| logger.exception(f"set_dry_run() raised on task {task.name!r}") | ||
|
|
||
| @rpc | ||
| def task_invoke( | ||
| self, task_name: TaskName, method: str, kwargs: dict[str, Any] | None = None | ||
|
|
@@ -711,16 +701,21 @@ def start(self) -> None: | |
| "Use task_invoke RPC or set transport via blueprint." | ||
| ) | ||
|
|
||
| # Subscribe to twist commands if any twist base hardware configured | ||
| # Twist commands drive either base hardware or velocity-capable tasks. | ||
| has_twist_base = any(c.hardware_type == HardwareType.BASE for c in self.config.hardware) | ||
| if has_twist_base: | ||
| with self._task_lock: | ||
| has_velocity_task = any( | ||
| callable(getattr(task, "set_velocity_command", None)) | ||
| for task in self._tasks.values() | ||
| ) | ||
| if has_twist_base or has_velocity_task: | ||
| try: | ||
| self._twist_command_unsub = self.twist_command.subscribe(self._on_twist_command) | ||
| logger.info("Subscribed to twist_command for twist base control") | ||
| logger.info("Subscribed to twist_command for twist base / velocity-capable tasks") | ||
| except Exception: | ||
| logger.warning( | ||
| "Twist base configured but could not subscribe to twist_command. " | ||
| "Use task_invoke RPC or set transport via blueprint." | ||
| "Twist base or velocity-capable task configured but could not subscribe " | ||
| "to twist_command. Use task_invoke RPC or set transport via blueprint." | ||
| ) | ||
|
|
||
| # Subscribe to buttons if any teleop_ik tasks configured (engage/disengage) | ||
|
|
@@ -729,6 +724,8 @@ def start(self) -> None: | |
| self._buttons_unsub = self.buttons.subscribe(self._on_buttons) | ||
| logger.info("Subscribed to buttons for engage/disengage") | ||
|
|
||
| # Arming + dry-run are RPC-only; no stream subscription here. | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as mentioned above. These should be simple RPCs |
||
| logger.info(f"ControlCoordinator started at {self.config.tick_rate}Hz") | ||
|
|
||
| @rpc | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
We can skip so much verbose. one line comments are good enough in coordinator.