Skip to content

Support contact sensor for ObjectSet-to-ObjectSet in PnP - #1233

Open
xyao-nv wants to merge 7 commits into
mainfrom
xyao/feature/multi-heterogeneous-objects
Open

Support contact sensor for ObjectSet-to-ObjectSet in PnP#1233
xyao-nv wants to merge 7 commits into
mainfrom
xyao/feature/multi-heterogeneous-objects

Conversation

@xyao-nv

@xyao-nv xyao-nv commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

Support contact sensor for ObjectSet-to-ObjectSet in PnP

Detailed description

  • Before: Cannot use 2 objectSet as the pickup object and place destination. Previously, a RigidObjectSet used an empty/synthetic usd_path, and the destination could point to /.../destination even when RigidBodyAPI was actually on /.../destination/rigid_body.

  • Changed: It resolves to the actual physics rigid-body prims, not merely the objects’ scene roots. Added the env spec example.

  • Why now? Visuals in release/0.3.0 landing page (https://isaac-sim.github.io/IsaacLab-Arena/release/0.3.0/_images/droid-heterogeneous-fruit-bottle-utensil-pnp.gif) asked by PM require this > <

@xyao-nv
xyao-nv marked this pull request as ready for review September 10, 2026 04:59
Comment thread isaaclab_arena/assets/object.py Outdated
@greptile-apps

greptile-apps Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

The PR is not yet safe to merge because valid referenced assets with auxiliary rigid bodies outside the default-prim subtree can fail during contact-sensor configuration.

Findings

  1. P1 Whole-stage search rejects assets

Summary

  • Resolves rigid-body suffixes relative to the complete default-prim path.
  • Reuses an object's contact-sensor prim when constructing contact filters.
  • Adds coverage for nested default prims and object-set contact targets.

Reviews (1) · Last reviewed commit: "wip"

Comment thread isaaclab_arena/utils/usd/rigid_bodies.py Outdated
Comment thread isaaclab_arena/utils/usd/rigid_bodies.py Outdated
Comment thread isaaclab_arena/utils/usd/rigid_bodies.py Outdated
Comment thread isaaclab_arena/assets/object.py Outdated
Comment thread isaaclab_arena/assets/object.py Outdated
@arena-review-bot

Copy link
Copy Markdown
Contributor

🤖 Isaac Lab-Arena Review Bot

Summary

This fixes two real bugs in contact-sensor wiring: _path_relative_to_usd_root now strips the actual default prim instead of just the first path component (which was wrong for the object-set cache USDs, where _set_default_prim_for_object_set_cache deliberately nests the default prim), and Object.get_contact_sensor_cfg now resolves the contact_against path polymorphically, so a RigidObjectSet on the other side uses its canonical member USD rather than its empty usd_path. Both are worth having, and the changes are small and well targeted.

Findings

🟡 isaaclab_arena/utils/usd/rigid_bodies.py:170 — the shallowest rigid body is still chosen by traversing the whole stage, so a body outside the default-prim subtree can win and then trip the new assert, even though it never composes when referenced. read_asset_rigid_body_paths in the same module already handles this by referencing into a throwaway stage. (Greptile flagged the same line.)

🔵 isaaclab_arena/utils/usd/rigid_bodies.py:110 — helper name and the public Returns: text still say "root of the USD file"; the anchor is now the default prim.

🔵 isaaclab_arena/assets/object.py:115 — a whole ContactSensorCfg is built for the other object just to read prim_path. A get_contact_sensor_prim_path() hook would be clearer and would let get_contact_sensor_cfg drop its usd_path parameter; it also makes the object_type == RIGID assert above redundant.

🔵 isaaclab_arena/assets/object.py:98 — the removed TODO also covered ObjectReference, whose behaviour is unchanged; the replacement comment reads as if that gap is closed.

Test Coverage

Good regression coverage for the main fix: test_find_shallowest_rigid_body_is_relative_to_nested_default_prim fails on the old prefix-stripping logic, and the object-set-against-object-set assertions in test_object_set.py pin the new polymorphic filter path. Both are plain Phase 1 unit tests, consistent with the surrounding files. The two new failure paths are untested — a USD with no default prim, and a rigid body outside the default-prim subtree — which is where the 🟡 above would bite. Also, _test_object_set_samples_and_stores_variant_indices is now covering three separate behaviours; the contact-sensor assertions might read better as their own test.

The PR description is still the unfilled template — worth a one-line summary and a couple of bullets per .github/pull_request_template.md.

Verdict

Minor fixes needed

Allow rigid object sets to resolve their canonical rigid-body path when used as either side of a pick-and-place contact sensor. Add a multi-set task regression and a representative DROID environment.

Signed-off-by: Xinjie Yao <xyao@nvidia.com>
@xyao-nv
xyao-nv force-pushed the xyao/feature/multi-heterogeneous-objects branch from 9dfa7e9 to a97e3e7 Compare September 11, 2026 05:06
@xyao-nv xyao-nv changed the title Xyao/feature/multi heterogeneous objects Support contact sensor for ObjectSet-to-ObjectSet in PnP Sep 11, 2026

@qianl-nv qianl-nv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks! Have one comment for clarification

Args:
stage: The stage to analyze
relative_to_root: Whether to return the path relative to the root of the USD file
relative_to_root: Whether to return the path relative to the USD default prim

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we rename the param to relative_to_default_prim then?

return prim_path[len(default_prim_path) :]


def find_shallowest_rigid_body_from_stage(stage: Usd.Stage, relative_to_root: bool = False) -> str | None:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

object_set_utils.py also call this function from a few places, and there it seems we expect to filter within the default prim but get the absolute paths to edit the stage? unlike calling from RigidObjectSet.get_contact_sensor_prim_path() wich expect it relative to default prim.

should we split the params into two

find_shallowest_rigid_body_from_stage(
    stage,
    within_default_prim=True,
    relative_to_default_prim=False,
)

@alexmillane alexmillane left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for closing that gap!

One small comment.

Comment on lines +91 to +96
def get_contact_sensor_prim_path(self) -> str:
"""Return the scene prim path where this object's contact sensor is attached."""
assert self.usd_path is not None, f"No USD path available for {self.name}. Can't add contact sensor."
return self._get_contact_sensor_prim_path_from_usd(self.usd_path)

def _get_contact_sensor_prim_path_from_usd(self, usd_path: str) -> str:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now the new function just does an assert and forwards to the next one.

We could just rename the existing function to get_contact_sensor_prim_path and do the assert in there.

Or maybe there's a reason for the indirection?

Comment on lines +115 to +117
# Handles Object and its subclasses, including RigidObjectSet.
# RigidObjectSet normalizes the USD paths for all members before spawning, so they have the same
# relative structure and rigid-body name. We add the contact sensor to the normalized rigid body beneath the its scene prim.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion to remove the comment about what is done outside the function. Potentially we could add a check that checks for the structure that we expect? If that's easy?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants