Skip to content

Unify robot config protocol + add-robot skill#330

Open
yuecideng wants to merge 11 commits into
mainfrom
feat/robot-unified-protocol
Open

Unify robot config protocol + add-robot skill#330
yuecideng wants to merge 11 commits into
mainfrom
feat/robot-unified-protocol

Conversation

@yuecideng

Copy link
Copy Markdown
Contributor

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). Unifies CobotMagicCfg and DexforceW1Cfg under one shared RobotCfg protocol, fixes 10 confirmed bugs, refreshes the add_robot.rst docs (and creates the missing tutorial), and adds an add-robot skill.

The protocol (hybrid — lifted into base RobotCfg)

  • _build_defaults(self, init_dict=None) hook — subclasses override to read variant fields from init_dict and populate urdf_cfg/control_parts/solver_cfg/drive_pros/attrs. (Base = no-op.)
  • from_dict = 3-line template (cls()_build_defaultsmerge_robot_cfg) — no longer reimplemented per robot.
  • Serialization (to_dict/to_string/save_to_file) lifted into the base; round-trips (from_dict(cfg.to_dict()) reproduces the cfg).
  • build_pk_serial_chain routes through a single _pk_urdf_path source per robot (property for constant paths, method for variant-dependent), so it can't silently drift from the sim URDF.

Bug fixes (10)

  • solver_cfg set twice in DexforceW1Cfg.from_dict (dead PytorchSolver overwritten by SRSSolverCfg) → set once in _build_defaults.
  • build_pk_serial_chain used a different URDF than the sim in both robots → routed through _pk_urdf_path + a DOF drift-guard test.
  • Lowercase all instead of __all__ in dexforce_w1/types.py + utils.py (broke import *) → __all__.
  • DexforceW1HandBrand excluded from types.__all__ → exported.
  • HandManager.get_config returned a 1-tuple (name,) for root_link_namestr (both gripper branches).
  • Phantom inst.validate() call in W1ArmKineParams.from_dict (no such method) → removed.
  • _build_default_cfgs was @staticmethod called via throwaway cls() → instance _build_defaults.
  • No __all__ in robots/__init__.py → added.
  • Missing full tutorial docs/source/tutorial/add_robot.rst (broken :doc: link) → created.
  • No add-robot skill → 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: @configclass unconditionally clobbered any custom to_dict with class_to_dict (which doesn't handle enums/numpy). Added an MRO guard so a class with its own to_dict keeps it; subclasses without one inherit it. Blast radius: only RobotCfg + DexforceW1Cfg define custom to_dict; every other @configclass (incl. gpu_memory_config) is unchanged.
  • embodichain/lab/sim/utility/cfg_utils.py: merge_robot_cfg (a) crashed on to_dict-serialized urdf_cfg.components (dict vs list) → normalized; (b) warned on subclass-only variant keys (arm_kind, version) passed through the clean template → filtered to base RobotCfg.__dataclass_fields__. Both are no-ops for existing call patterns.

Docs + skill

  • Rewrote docs/source/guides/add_robot.rst (protocol, 10-item checklist, Key Parameters + Common Mistakes tables).
  • Created docs/source/tutorial/add_robot.rst (Path A single-file + Path B package-with-variants).
  • Aligned agent_context/topics/robot-system/robot-system.md with the unified protocol.
  • Added add-robot skill: .agents/skills/add-robot/SKILL.md + agents/openai.yaml, .claude/skills/add-robot/SKILL.md, .github/copilot/add-robot.md + instructions.md index 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_cfg removed (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 of DexforceW1Cfg.to_dict() should note.

Dependencies: none.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (non-breaking change which improves an existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (existing functionality will not work without user modification)
  • Documentation update

Screenshots

N/A.

Checklist

  • I have run the black . command to format the code base.
  • I have made corresponding changes to the documentation (guide rewritten, tutorial created, robot-system.md aligned, skill added).
  • I have added tests that prove my fix is effective or that my feature works (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.
  • Dependencies have been updated, if applicable.

Copilot AI review requested due to automatic review settings June 25, 2026 18:30
@yuecideng yuecideng added enhancement New feature or request robot Module related to robot refactor docs Improvements or additions to documentation labels Jun 25, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 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 RobotCfg and updated DexforceW1Cfg / CobotMagicCfg to follow the new _build_defaults hook + from_dict template.
  • Hardened config merging/round-tripping (configclass MRO guard, merge_robot_cfg key filtering + URDF component normalization, URDFCfg sensors normalization).
  • Added the add-robot skill (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.

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

Labels

docs Improvements or additions to documentation enhancement New feature or request refactor robot Module related to robot

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants