Adds fix for Cassie with ability to force newton to create joint drives

[AntoineRichard]

Description

Adds fix for Cassie with ability to force newton to create joint drives

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)

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

[greptile-apps]

Greptile Summary

This PR adds an ensure_drives_exist flag to JointDrivePropertiesCfg and wires it into modify_joint_drive_properties to work around a Newton physics backend issue where joints with stiffness=0 and damping=0 in the USD asset are treated as passive (no PD control). When enabled, a minimal stiffness of 1e-3 N-m/rad is injected so Newton recognises the drive as active; the actual actuator gains are applied afterward by the actuator model. The Cassie robot config is updated to opt into this fix.

• Missing changelog entries and version bumps: Both source/isaaclab/ and source/isaaclab_assets/ were modified but neither CHANGELOG.rst has a new version entry nor have the extension.toml version fields been incremented — required by project guidelines.

Confidence Score: 4/5

Safe to merge after adding the required changelog entries and version bumps for both packages.

The core logic is correct and the fix is well-scoped. The only blocking concern is the missing CHANGELOG and extension.toml version bumps for both isaaclab and isaaclab_assets, which are mandatory per project guidelines. All other findings are P2 documentation suggestions.

source/isaaclab/docs/CHANGELOG.rst, source/isaaclab/config/extension.toml, source/isaaclab_assets/docs/CHANGELOG.rst, source/isaaclab_assets/config/extension.toml

Important Files Changed

Filename	Overview
source/isaaclab/isaaclab/sim/schemas/schemas_cfg.py	Adds ensure_drives_exist: bool = False to JointDrivePropertiesCfg; field is well-placed, but the docstring omits the unit of the injected 1e-3 stiffness value.
source/isaaclab/isaaclab/sim/schemas/schemas.py	Implements the ensure_drives_exist logic: pops the flag, reads current USD stiffness/damping, and injects a minimal 1e-3 stiffness when both are zero/unset; logic is correct but the silent no-op when explicit stiffness=0.0 is passed is undocumented.
source/isaaclab_assets/isaaclab_assets/robots/cassie.py	Adds joint_drive_props=sim_utils.JointDrivePropertiesCfg(ensure_drives_exist=True) to the Cassie USD spawn config; correct usage of the new flag.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["modify_joint_drive_properties(prim_path, cfg)"] --> B{Prim is revolute\nor prismatic?}
    B -- No --> C[return False]
    B -- Yes --> D{Is tendon child\nwithout root?}
    D -- Yes --> C
    D -- No --> E[Get/Apply DriveAPI]
    E --> F["cfg = cfg.to_dict()\nensure_drives = cfg.pop('ensure_drives_exist')"]
    F --> G{ensure_drives AND\ncfg stiffness is None AND\ncfg damping is None?}
    G -- No --> I[Apply unit conversions\nrad to deg for angular]
    G -- Yes --> H["Read cur_stiffness,\ncur_damping from USD"]
    H --> H2{both zero\nor None?}
    H2 -- No --> I
    H2 -- Yes --> H3["cfg['stiffness'] = 1e-3\n(minimal placeholder)"]
    H3 --> I
    I --> J[Write attrs to USD\nvia DriveAPI]
    J --> K[return True]

Loading

Comments Outside Diff (1)

1. source/isaaclab/docs/CHANGELOG.rst, line 1-5 (link)

   Missing changelog entry and version bump

   Per project guidelines, every change touching source/<package>/ requires a new version heading in that package's CHANGELOG.rst and a matching bump in config/extension.toml. This PR modifies both source/isaaclab/ and source/isaaclab_assets/, but neither changelog has a new entry nor have the extension.toml version fields been incremented (both remain at 4.5.30 and 0.3.1 respectively).

   Required updates:

   • source/isaaclab/docs/CHANGELOG.rst — new 4.5.31 heading with an "Added" entry for ensure_drives_exist in JointDrivePropertiesCfg
   • source/isaaclab/config/extension.toml — bump version to 4.5.31
   • source/isaaclab_assets/docs/CHANGELOG.rst — new 0.3.2 heading with a "Changed" entry for the Cassie config
   • source/isaaclab_assets/config/extension.toml — bump version to 0.3.2

_{Reviews (1): Last reviewed commit: "Adds fix for Cassie with ability to forc..." | Re-trigger Greptile}

[greptile-apps]

Missing unit annotation for 1e-3 stiffness

The docstring states a minimal stiffness of 1e-3 is written but doesn't indicate the unit. Looking at the implementation in schemas.py (line 696), this value is placed in cfg["stiffness"] before the angular unit conversion (* math.pi / 180.0), so it is interpreted as N-m/rad (simulation units), which gets stored to USD as ~1.745e-5 N-m/deg. The docstring should clarify the unit so callers understand the scale of the placeholder value.

[greptile-apps]

ensure_drives_exist silently skips when explicit stiffness/damping are provided

The outer guard cfg["stiffness"] is None and cfg["damping"] is None means ensure_drives_exist=True has no effect if a caller also passes an explicit stiffness=0.0 or damping=0.0 in the same config. This edge case is consistent with the "don't override explicit user values" intent, but it is not documented anywhere — callers who pass stiffness=0.0 expecting the fix to still activate will be silently surprised. A short note in the docstring (or as an inline comment) would prevent confusion.

[isaaclab-review-bot]

🤖 Isaac Lab Review Bot

Summary

This PR fixes Cassie robot support on the Newton physics backend by adding an ensure_drives_exist flag to JointDrivePropertiesCfg. When enabled, it injects a minimal stiffness (1e-3) into USD joint drives that have both stiffness and damping at zero — preventing Newton from treating them as passive joints. The approach is sound: it solves a real backend compatibility issue with a targeted, backward-compatible config addition.

Design Assessment

Design is sound. The fix correctly targets the USD schema layer (where Newton reads drive configuration) rather than hacking around the problem downstream. Key design decisions are well-considered:

• Opt-in flag (ensure_drives_exist=False by default) — no impact on existing users or PhysX workflows
• Minimal stiffness injection — 1e-3 is small enough to not affect simulation behavior, while the real gains come from the actuator model override
• Guard conditions — only fires when config stiffness and damping are both None (user hasn't set them) and the USD-authored values are both zero
• Clean cfg.pop() — prevents the boolean from leaking into the USD attribute setter loop (which would raise TypeError)

The only alternative would be fixing this at the Newton backend level, but that's outside this project's control.

Findings

🟡 Warning: Missing test coverage for new ensure_drives_exist path — source/isaaclab/test/sim/test_schemas.py

The existing test_modify_properties_on_articulation_* tests use stiffness=10.0, damping=0.1, so they never exercise the new ensure_drives_exist code path. Since this is a bug fix PR, a regression test would strengthen confidence that the fix works and doesn't regress.

A useful test would:

1. Create a joint with stiffness=0, damping=0 in USD
2. Call modify_joint_drive_properties with JointDrivePropertiesCfg(ensure_drives_exist=True) (stiffness/damping left as None)
3. Verify that the resulting USD drive stiffness is non-zero (1e-3 for linear, 1e-3 * π/180 for angular)
4. Also verify the negative case: ensure_drives_exist=False leaves the zero drive untouched

🟡 Warning: Float equality check against USD-authored values — source/isaaclab/isaaclab/sim/schemas/schemas.py:695

if (cur_stiffness is None or cur_stiffness == 0.0) and (cur_damping is None or cur_damping == 0.0):

Direct == 0.0 comparison works for cleanly-authored USD attributes (which are typically set to exact 0.0), but could miss near-zero values written by other tools or roundtrip serialization. A tolerance-based check would be more robust:

        if (cur_stiffness is None or abs(cur_stiffness) < 1e-6) and (cur_damping is None or abs(cur_damping) < 1e-6):

This is a minor concern since USD attributes are typically authored as exact values, but worth considering for robustness.

🔵 Suggestion: Add CHANGELOG entry — CHANGELOG.rst

The PR checklist notes that changelog and version updates are expected for user-facing changes. Adding ensure_drives_exist to JointDrivePropertiesCfg is a public API addition that users may want to discover.

Test Coverage

• Bug fix PR: No regression test included
• New config field: Not tested with valid/invalid values
• Existing tests: Pass through this code path but don't exercise the new branch (stiffness/damping are always set to non-None values)
• Recommendation: Add at least one test for the ensure_drives_exist=True path with a zero-drive joint

CI Status

All checks pending (freshly pushed). Docker image builds passed. Pre-commit and test suites still running.

Verdict

COMMENT

The implementation is clean and the design is correct. The two warnings above are suggestions to improve robustness and test coverage — neither is a blocking issue. The fix addresses a real Newton backend limitation in a backward-compatible way, and the Cassie config change is the right consumer for it. Nice work.

[isaaclab-review-bot]

🟡 Warning: Direct == 0.0 comparison works for cleanly-authored USD values, but could miss near-zero values from roundtrip serialization or other tools. Consider a tolerance-based check:

Suggested change

	if (cur_stiffness is None or cur_stiffness == 0.0) and (cur_damping is None or cur_damping == 0.0):
	if (cur_stiffness is None or abs(cur_stiffness) < 1e-6) and (cur_damping is None or abs(cur_damping) < 1e-6):

Minor concern since USD attributes are typically exact values, but improves robustness.

[AntoineRichard]

This is not needed since we precisely want to avoid the case where the values would be exactly 0.0. This is because newton will not create a mujoco actuator if and only if the values are exactly 0.