Refine URDF assembly component prefixes and name casing policy

[chase6305]

Description

Summary

This PR refines the URDF assembly pipeline with configurable component name prefixes, a global name casing policy for links and joints, and extended signature semantics so that naming‑related configuration changes correctly invalidate cached assemblies.

---

Motivation

• Make component name prefixes configurable in a predictable, patch‑style way without exposing internal ordering details.
• Centralize and expose the link/joint lower()/upper() behavior via a single name_case policy instead of hard‑coded transformations scattered across managers.
• Ensure that changes to prefixes and naming policies are reflected in the assembly signature so that cached URDFs are rebuilt when naming semantics change.
• Align URDFCfg and documentation with the new configuration surface.

---

Changes

1. URDFAssemblyManager

   • Add support for per‑component name prefixes via the component_prefix property:
     • Internal storage remains _component_order_and_prefix: list[tuple[str, str | None]].
     • component_prefix acts as a patch‑style interface:
       • Accepts a list of (component_name, prefix) tuples.
       • Only existing component names (e.g. chassis, torso, left_arm, …) may be overridden.
       • Components not mentioned keep their original prefix.
       • Unknown component names raise a ValueError.
   • Introduce a global name casing policy via the optional name_case argument:
     • Signature: URDFAssemblyManager(..., name_case: dict[str, str] | None = None).
     • Valid keys: "joint", "link".
     • Valid values: "upper", "lower", "none".
     • Default behavior matches the legacy implementation:
       • joint names → upper case,
       • link names → lower case.
     • Provide a helper _apply_case(kind: str, name: str | None) -> str | None and remove scattered hard‑coded .lower() / .upper() calls in favor of this policy.
   • Propagate name_case to internal managers:
     • URDFComponentManager(mesh_manager, name_case=self._name_case)
     • URDFConnectionManager(self.base_link_name, name_case=self._name_case)
     • (Optional / if implemented) URDFSensorManager can also honor the same policy.
   • Extend assembly signature input:
     • When building component_info for URDFAssemblySignatureManager, inject:
       • __component_order_and_prefix__ = list(self.component_order_and_prefix)
       • __name_case__ = dict(self._name_case)
     • This ensures that changing prefixes or casing policy invalidates the cached assembly and forces a rebuild.

2. URDFAssemblySignatureManager

   • Extend signature_data to include:
     • component_order_and_prefix
     • name_case
   • Handle the special metadata keys when iterating components:
     • __component_order_and_prefix__ → stored in signature_data["component_order_and_prefix"]
     • __name_case__ → stored in signature_data["name_case"]
   • Keep existing component hashing logic unchanged, so previous behavior is preserved aside from the new fields.

3. URDFCfg integration

   • Add component_prefix support to URDFCfg:
     • Field type: list[tuple[str, str | None]] with a default matching URDFAssemblyManager’s internal order.
     • Initialization logic allows:
       • None → uses the default prefix list.
       • dict[str, str] → converted to list(component_prefix.items()) for convenience (e.g. {"left_hand": "l_", "right_hand": "r_"}).
       • list[tuple[str, str | None]] → used as is.
   • In assemble_urdf(), pass self.component_prefix directly to URDFAssemblyManager.component_prefix so that URDFCfg can drive per‑component prefixes from robot configs.
   • (If implemented) Add optional name_case to URDFCfg and forward it to URDFAssemblyManager to allow configuring naming policy from robot configuration.

4. Component and connection managers (if part of this PR)

   • URDFComponentManager
     • Accept name_case: dict[str, str] | None = None and store internally.
     • Add a local _apply_case(kind, name) helper mirroring the assembly manager.
     • Replace hard‑coded .lower() / .upper() on link and joint names with the configured casing policy.
   • URDFConnectionManager
     • Accept name_case: dict[str, str] | None = None.
     • Use _apply_case("joint", ...) for generated joint names.
     • Use _apply_case("link", ...) for parent/child link names in connection joints.
   • (Optional) ComponentRegistry
     • If included, normalize component keys via a key_case policy while keeping logical component names stable in the public API.

5. Documentation

   • Update urdf_assembly.md:
     • Add Component name prefixes (component_prefix) section describing:
       • The default prefix list.
       • Patch‑style semantics and error behavior on unknown components.
       • The fact that prefixes participate in the assembly signature, so changing them forces a rebuild.
     • Add Name casing policy (name_case) section describing:
       • Configuration via the URDFAssemblyManager constructor.
       • Valid keys/values and default behavior.
       • Propagation to internal managers and inclusion in the assembly signature.
       • The impact on cache invalidation.
     • Extend the Using with URDFCfg section to mention URDFCfg.component_prefix and its identical semantics to URDFAssemblyManager.component_prefix.

---

Backward compatibility

• Default behavior is preserved:
  • Joint names remain upper‑cased.
  • Link names remain lower‑cased.
  • Default component prefix list and processing order are unchanged.
• Existing code that does not set component_prefix or name_case should observe identical URDF outputs.
• New configuration is opt‑in and only affects behavior when explicitly set.

Checklist

• I have run the black . command to format the code base.
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• Dependencies have been updated, if applicable.

[Copilot]

Pull request overview

Refines the URDF assembly toolchain’s naming configuration by introducing per-component prefix overrides, a global link/joint casing policy, and signature inputs that invalidate cached assemblies when naming semantics change. Also wires these knobs into URDFCfg and user config merging, and documents the new configuration surface.

Changes:

• Add patch-style component_prefix and global name_case handling in URDF assembly, and include both in the assembly signature.
• Update component/connection naming normalization to use a centralized casing policy helper.
• Integrate new config fields through URDFCfg / config merging and extend documentation.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
examples/sim/robot/dexforce_w1.py	Formatting-only bracket/comma adjustment.
embodichain/toolkits/urdf_assembly/urdf_assembly_manager.py	Adds name_case + component_prefix logic and injects naming metadata into signature; updates manager construction to pass casing policy.
embodichain/toolkits/urdf_assembly/signature.py	Stores component_order_and_prefix + name_case metadata in signature JSON.
embodichain/toolkits/urdf_assembly/connection.py	Adds name casing policy support for generated connection names and sensor attachment rewriting.
embodichain/toolkits/urdf_assembly/component.py	Introduces casing policy and replaces hard-coded .lower()/.upper() with _apply_case.
embodichain/lab/sim/utility/cfg_utils.py	Extends robot cfg merge to include sensors and URDF naming configuration fields.
embodichain/lab/sim/cfg.py	Adds URDFCfg.component_prefix and forwards prefix/casing configuration into the assembly manager.
docs/source/features/toolkits/urdf_assembly.md	Documents component_prefix and name_case, plus URDFCfg integration.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The constructor hard-codes the default name casing policy but does not accept a name_case parameter. This conflicts with the documented usage (URDFAssemblyManager(name_case=...)) and will raise TypeError for users following the docs. Consider adding an optional name_case argument to __init__ (and validating/merging it) or updating the docs to reflect the property-based configuration.

[Copilot]

add_sensor_attachments appends <link> elements into the joints list (joints.append(link)), which will cause links to be emitted in the joint section if this method is used. Either accept a separate links list here (and append links to it) or rename/adjust the method so it cannot mix element types.

[chase6305]

@copilot open a new pull request to apply changes based on this feedback

[Copilot]

The documentation states that name_case is configured via a URDFAssemblyManager(..., name_case=...) constructor argument, but the current URDFAssemblyManager.__init__ signature does not accept this parameter. Update the docs to match the actual API, or implement the constructor argument as documented.

Suggested change

	##### Name casing policy (`name_case`)
	`URDFAssemblyManager` supports a global name casing policy that controls how
	link and joint names are normalized during assembly. This is configured via
	the optional `name_case` argument in the constructor:
	```python
	manager = URDFAssemblyManager(
	name_case={
	"joint": "upper", # or "lower" / "none"
	"link": "lower", # or "upper" / "none"
	}
	)
	```
	Semantics:
	- Valid keys: `"joint"`, `"link"`.
	- Valid values: `"upper"`, `"lower"`, `"none"`.
	- Default behavior matches the legacy implementation:
	##### Name casing policy
	`URDFAssemblyManager` applies a global name casing policy that controls how
	link and joint names are normalized during assembly.
	Semantics:
	- The policy applies separately to joint names and link names.
	- Current default behavior (matching the legacy implementation):

[Copilot]

New naming semantics (component_prefix patching + name_case) affect URDF outputs and caching behavior, but there are no tests asserting: (1) unknown component keys raise, (2) per-component prefix patches preserve default ordering, (3) name_case changes affect link/joint names, and (4) signature changes when either setting changes. Adding focused unit tests with minimal URDF XML fixtures would prevent regressions.

[chase6305]

@copilot open a new pull request to apply changes based on this feedback

[Copilot]

The component_prefix setter error message (and subsequent validation messages) refer to component_order_and_prefix, which is an internal name. Since this is the public configuration surface, the exception text should reference component_prefix (and ideally mention the patch-style semantics / allowed component keys) to avoid confusing users.

[Copilot]

When prefix is set, link names produced by _generate_unique_name(...) are no longer normalized via _apply_case("link", ...). This makes the emitted link names inconsistent with the configured name_case policy and can also break the collision checks that rely on global_link_names being normalized.

[Copilot]

_generate_unique_name claims it operates on a normalized form for uniqueness checks, but new_name is never normalized before comparing against existing_names. Since callers build existing_names using _apply_case(...), this can miss collisions (e.g., Left_Link vs left_link) and generate duplicate names. Normalize base_name/new_name according to the relevant case policy before the uniqueness loop.

[Copilot]

URDFCfg.add_sensor(...) requires a sensor_name positional argument, but this merge path calls add_sensor(sensor_type=..., urdf_path=..., ...) without providing it. This will raise a TypeError when override_cfg_dict['urdf_cfg']['sensors'] is present. Pass sensor_name=sensor.get('sensor_name') (or clearly define which field is the name) and forward the rest as kwargs.

Suggested change

	base_cfg.urdf_cfg.add_sensor(
	base_cfg.urdf_cfg.add_sensor(
	sensor_name=sensor.get("sensor_name"),

[Copilot]

name_case setter validates that the input is a dict and contains both keys, but it does not validate the values. If a caller passes an unsupported mode (e.g. "uppercase"), it will be accepted and _apply_case will silently fall back to returning the original name (since the mode won't match "upper"/"lower"). This contradicts the documented valid values; consider validating modes and raising a ValueError (or normalizing/merging with defaults) to fail fast.

Suggested change

	self._name_case = new_name_case
	self.component_manager.name_case = new_name_case
	self.sensor_manager.name_case = new_name_case
	# Validate and normalize the case modes to ensure they match the supported values.
	allowed_modes = {"upper", "lower", "none"}
	validated_name_case: dict[str, str] = {}
	for kind in ("joint", "link"):
	mode = new_name_case.get(kind)
	if not isinstance(mode, str):
	raise ValueError(
	f"name_case['{kind}'] must be a string; got {type(mode).__name__}."
	)
	normalized_mode = mode.lower()
	if normalized_mode not in allowed_modes:
	raise ValueError(
	f"Unsupported case mode {mode!r} for '{kind}'. "
	f"Expected one of: {', '.join(sorted(allowed_modes))}."
	)
	validated_name_case[kind] = normalized_mode
	self._name_case = validated_name_case
	self.component_manager.name_case = validated_name_case
	self.sensor_manager.name_case = validated_name_case

[Copilot]

@chase6305 I've opened a new pull request, #203, to work on those changes. Once the pull request is ready, I'll request review from you.

[Copilot]

@chase6305 I've opened a new pull request, #204, to work on those changes. Once the pull request is ready, I'll request review from you.

[yuecideng]

We should add some tests for URDF Assembly tool.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 10 comments.

Comments suppressed due to low confidence (1)

embodichain/toolkits/urdf_assembly/connection.py:124

• Even with a link casing policy configured, the base-link connection emits <parent link=self.base_link_name> and <child link=chassis_first_link> without applying _apply_case('link', ...). If name_case['link'] != 'none', this can produce link references that don't match the actual <link name=...> values emitted by the assembly. Normalize these link attributes consistently (including the base link name).

                joint = ET.Element("joint", name=joint_name, type="fixed")
                ET.SubElement(joint, "origin", xyz="0 0 0", rpy="0 0 0")
                ET.SubElement(joint, "parent", link=self.base_link_name)
                ET.SubElement(joint, "child", link=chassis_first_link)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

When prefix is provided, link renaming uses _generate_unique_name(...) but does not apply the configured link casing policy before writing link.set('name', new_name). Also, _generate_unique_name compares un-normalized names against existing_names (which are stored in normalized form), which can miss collisions. Apply _apply_case('link', ...) to generated link names and ensure uniqueness checks use the same normalized representation.

[Copilot]

Docstring markup has an unbalanced literal ("upper missing closing quote/backticks). This renders incorrectly in generated docs and can confuse readers about allowed name_case values.

Suggested change

	``"joint"`` and ``"link"`` with values ``"upper``,
	``"joint"`` and ``"link"`` with values ``"upper"``,

[Copilot]

These merges use truthiness checks (if component_prefix: / if name_case:), which prevents explicitly setting empty-but-intentional values (e.g., component_prefix=[] meaning “no overrides”). Prefer is not None checks so empty lists/dicts are applied.

Suggested change

	if component_prefix:
	base_cfg.urdf_cfg.component_prefix = component_prefix
	name_case = user_urdf_cfg.get("name_case", None)
	if name_case:
	if component_prefix is not None:
	base_cfg.urdf_cfg.component_prefix = component_prefix
	name_case = user_urdf_cfg.get("name_case", None)
	if name_case is not None:

[Copilot]

The orphan-component base connection still sets <parent link=self.base_link_name> without applying the configured link casing policy. If link casing is not 'none', this parent reference may not match the base link name used elsewhere. Consider normalizing base_link_name once and always emitting the normalized value.

[Copilot]

Docs show URDFAssemblyManager(name_case=...), but URDFAssemblyManager.__init__ in this PR does not accept a name_case argument. Either add the constructor parameter (as documented) or update the docs to the supported usage (manager.name_case = {...}) to avoid users hitting TypeError.

[Copilot]

URDFCfg introduces configurable name_case via __init__, but it’s not declared as a config field like component_prefix. If URDFCfg is expected to be serializable/introspectable (as a @configclass), consider adding name_case as a field with a default matching legacy behavior.

[Copilot]

With configurable name_case, existing_link_names is built using _apply_case, but the assembly’s base link is still created using raw self.base_link_name. If name_case['link'] changes (e.g., 'upper'), the normalized name in this set won’t match the actual <link name=...> emitted, and downstream collision/reference logic can break. Normalize the base link name consistently (or exclude it from normalization everywhere).

[Copilot]

to_serializable doesn’t handle tuples. Since __component_order_and_prefix__ is a list of (component, prefix) tuples, signature_data['component_order_and_prefix'] will contain tuples and json.dumps(...) will raise TypeError. Convert tuples to lists (or otherwise ensure this metadata is JSON-serializable) before hashing.

Suggested change

	signature_data["component_order_and_prefix"] = to_serializable(comp_obj)
	serializable_meta = to_serializable(comp_obj)
	# Ensure tuples are converted to lists so the structure is JSON-serializable
	if isinstance(serializable_meta, (list, tuple)):
	serializable_meta = [
	list(item) if isinstance(item, tuple) else item
	for item in serializable_meta
	]
	signature_data["component_order_and_prefix"] = serializable_meta

[Copilot]

component_order_and_prefix setter docstring says it may raise ValueError for invalid input, but the implementation just assigns self._component_order_and_prefix = new_order with no validation. Either implement the described validation or update the docstring to match the actual behavior.

Suggested change

	Raises:
	ValueError: If the new order is not a list of tuples or if it
	contains unknown component names.

[Copilot]

The comment says new prefixes are appended for “brand‑new components from new_order at the end”, but the code explicitly rejects unknown components (raises ValueError) and never appends. Update/remove this comment to reflect the actual (no-new-components) semantics.

Suggested change

	# - Append any brand‑new components from new_order at the end.
	# - Reject any components not present in the existing/default order (no new components allowed).