Unify robot config protocol + add-robot skill#330
Open
yuecideng wants to merge 11 commits into
Open
Conversation
…s keys in merge_robot_cfg
…uple/validate/dead-solver-set
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors robot configuration to a unified RobotCfg protocol (via _build_defaults + merge_robot_cfg + inherited serialization), fixes several DexforceW1/CobotMagic issues, and adds documentation + an add-robot skill scaffold to standardize future robot additions.
Changes:
- Lifted common construction/serialization behaviors into
RobotCfgand updatedDexforceW1Cfg/CobotMagicCfgto follow the new_build_defaultshook +from_dicttemplate. - Hardened config merging/round-tripping (
configclassMRO guard,merge_robot_cfgkey filtering + URDF component normalization, URDFCfg sensors normalization). - Added the
add-robotskill (canonical + adapters), refreshed robot docs/tutorial, and introduced a focused robot-config test suite including PK DOF drift guards.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
tests/sim/objects/test_robot_cfg.py |
Adds round-trip + PK DOF drift-guard tests for the unified robot protocol. |
embodichain/utils/configclass.py |
Prevents @configclass from clobbering custom to_dict implementations in the MRO. |
embodichain/lab/sim/utility/cfg_utils.py |
Makes merge_robot_cfg safer for subclass variant keys and URDFCfg serialization shapes. |
embodichain/lab/sim/robots/dexforce_w1/utils.py |
Fixes exports and hand config fields; adjusts DexforceW1 config builder behavior. |
embodichain/lab/sim/robots/dexforce_w1/types.py |
Fixes __all__ and exports DexforceW1HandBrand. |
embodichain/lab/sim/robots/dexforce_w1/params.py |
Removes a nonexistent validate() call in from_dict. |
embodichain/lab/sim/robots/dexforce_w1/cfg.py |
Implements _build_defaults, unifies from_dict, and centralizes PK URDF selection. |
embodichain/lab/sim/robots/cobotmagic.py |
Switches to _build_defaults + template from_dict, adds __all__, and routes PK URDF via _pk_urdf_path. |
embodichain/lab/sim/robots/__init__.py |
Adds explicit __all__ exports for robot configs. |
embodichain/lab/sim/cfg.py |
Adds RobotCfg serialization helpers + _build_defaults hook; improves URDFCfg sensors round-trip handling. |
docs/source/tutorial/add_robot.rst |
Adds the missing step-by-step “add robot” tutorial. |
docs/source/guides/add_robot.rst |
Updates the quick reference to match the new RobotCfg protocol + pitfalls checklist. |
agent_context/topics/robot-system/robot-system.md |
Aligns agent-facing robot-system context with the unified protocol. |
.github/copilot/instructions.md |
Registers the Copilot add-robot adapter entry. |
.github/copilot/add-robot.md |
Adds the Copilot adapter that points to the canonical add-robot skill. |
.claude/skills/add-robot/SKILL.md |
Adds the Claude adapter for the canonical add-robot skill. |
.agents/skills/add-robot/SKILL.md |
Introduces the canonical add-robot skill definition and scaffold steps. |
.agents/skills/add-robot/agents/openai.yaml |
Registers the canonical add-robot skill for OpenAI agents. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| """ | ||
| init_dict = init_dict or {} | ||
| version = init_dict.get("version", DexforceW1Version.V021) | ||
| arm_kind = init_dict.get("arm_kind", DexforceW1ArmKind.INDUSTRIAL) |
Comment on lines
736
to
739
| if solver_cfg is not None: | ||
| cfg.solver_cfg = solver_cfg | ||
| else: | ||
| cfg.solver_cfg = build_dexforce_w1_solver_cfg( | ||
| arm_kind=arm_kind, | ||
| arm_sides=arm_sides, | ||
| component_versions=component_versions, | ||
| urdf_cfg=urdf_cfg, | ||
| ) | ||
|
|
||
| return cfg |
Comment on lines
+18
to
+32
| import enum | ||
|
|
||
| import numpy as np | ||
| import pytest | ||
|
|
||
| from embodichain.lab.sim.cfg import RobotCfg, JointDrivePropertiesCfg | ||
| from embodichain.lab.sim.robots.dexforce_w1 import DexforceW1Cfg | ||
| from embodichain.lab.sim.robots.dexforce_w1.types import ( | ||
| DexforceW1ArmKind, | ||
| DexforceW1Version, | ||
| ) | ||
| from embodichain.lab.sim.solvers import SRSSolverCfg | ||
| from embodichain.utils import configclass | ||
| from embodichain.lab.sim.utility.cfg_utils import merge_robot_cfg | ||
|
|
Comment on lines
+95
to
+96
| from embodichain.lab.sim.robots.cobotmagic import CobotMagicCfg | ||
| from embodichain.lab.sim.solvers import OPWSolverCfg |
Comment on lines
17
to
20
| from __future__ import annotations | ||
| import enum | ||
| import json | ||
| import os |
Comment on lines
62
to
66
| """Initialize DexforceW1Cfg from a dictionary. | ||
|
|
||
| Args: | ||
| init_dict (Dict[str, str | float | tuple | dict): Dictionary of configuration parameters. | ||
| init_dict: Dictionary of configuration parameters. | ||
|
|
9 tasks
matafela
approved these changes
Jun 26, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Implements the approved design (spec:
docs/superpowers/specs/2026-06-25-robot-refactor-design.md, plan:docs/superpowers/plans/2026-06-25-robot-refactor.md— on PR #328). UnifiesCobotMagicCfgandDexforceW1Cfgunder one sharedRobotCfgprotocol, fixes 10 confirmed bugs, refreshes theadd_robot.rstdocs (and creates the missing tutorial), and adds anadd-robotskill.The protocol (hybrid — lifted into base
RobotCfg)_build_defaults(self, init_dict=None)hook — subclasses override to read variant fields frominit_dictand populateurdf_cfg/control_parts/solver_cfg/drive_pros/attrs. (Base = no-op.)from_dict= 3-line template (cls()→_build_defaults→merge_robot_cfg) — no longer reimplemented per robot.to_dict/to_string/save_to_file) lifted into the base; round-trips (from_dict(cfg.to_dict())reproduces the cfg).build_pk_serial_chainroutes through a single_pk_urdf_pathsource per robot (property for constant paths, method for variant-dependent), so it can't silently drift from the sim URDF.Bug fixes (10)
solver_cfgset twice inDexforceW1Cfg.from_dict(deadPytorchSolveroverwritten bySRSSolverCfg) → set once in_build_defaults.build_pk_serial_chainused a different URDF than the sim in both robots → routed through_pk_urdf_path+ a DOF drift-guard test.allinstead of__all__indexforce_w1/types.py+utils.py(brokeimport *) →__all__.DexforceW1HandBrandexcluded fromtypes.__all__→ exported.HandManager.get_configreturned a 1-tuple(name,)forroot_link_name→str(both gripper branches).inst.validate()call inW1ArmKineParams.from_dict(no such method) → removed._build_default_cfgswas@staticmethodcalled via throwawaycls()→ instance_build_defaults.__all__inrobots/__init__.py→ added.docs/source/tutorial/add_robot.rst(broken:doc:link) → created.add-robotskill → added (canonical + Claude + Copilot adapters).Prerequisite fixes discovered during implementation (not in the original spec)
Two backward-compatible utility fixes were required for the protocol to work — both verified safe:
embodichain/utils/configclass.py:@configclassunconditionally clobbered any customto_dictwithclass_to_dict(which doesn't handle enums/numpy). Added an MRO guard so a class with its ownto_dictkeeps it; subclasses without one inherit it. Blast radius: onlyRobotCfg+DexforceW1Cfgdefine customto_dict; every other@configclass(incl.gpu_memory_config) is unchanged.embodichain/lab/sim/utility/cfg_utils.py:merge_robot_cfg(a) crashed onto_dict-serializedurdf_cfg.components(dict vs list) → normalized; (b) warned on subclass-only variant keys (arm_kind,version) passed through the clean template → filtered to baseRobotCfg.__dataclass_fields__. Both are no-ops for existing call patterns.Docs + skill
docs/source/guides/add_robot.rst(protocol, 10-item checklist, Key Parameters + Common Mistakes tables).docs/source/tutorial/add_robot.rst(Path A single-file + Path B package-with-variants).agent_context/topics/robot-system/robot-system.mdwith the unified protocol.add-robotskill:.agents/skills/add-robot/SKILL.md+agents/openai.yaml,.claude/skills/add-robot/SKILL.md,.github/copilot/add-robot.md+instructions.mdindex entry.Backward compatibility
from_dict(init_dict)signature unchanged externally — all ~10 call sites (examples,gym_utils.py, tests) unaffected._build_default_cfgs/_build_default_cfg/_build_default_physics_cfgs/_build_default_solver_cfgremoved (private, no external callers).DexforceW1Cfg.to_dict()now actually runs (was dead code) and correctly serializes enums as.value— a behavior improvement, but a change any consumer ofDexforceW1Cfg.to_dict()should note.Dependencies: none.
Type of change
Screenshots
N/A.
Checklist
black .command to format the code base.tests/sim/objects/test_robot_cfg.py: 7 tests — base round-trip, save_to_file, DexforceW1 round-trip + solver-set-once, CobotMagic round-trip, 2× DOF drift guard). FK golden regression (test_compute_fk) passes unchanged.