Support contact sensor for ObjectSet-to-ObjectSet in PnP - #1233
Conversation
|
🤖 Isaac Lab-Arena Review BotSummaryThis fixes two real bugs in contact-sensor wiring: Findings🟡 🔵 🔵 🔵 Test CoverageGood regression coverage for the main fix: The PR description is still the unfilled template — worth a one-line summary and a couple of bullets per VerdictMinor 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>
9dfa7e9 to
a97e3e7
Compare
Signed-off-by: Xinjie Yao <xyao@nvidia.com>
qianl-nv
left a comment
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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: |
There was a problem hiding this comment.
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
left a comment
There was a problem hiding this comment.
Thanks for closing that gap!
One small comment.
| 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: |
There was a problem hiding this comment.
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?
| # 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. |
There was a problem hiding this comment.
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?
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 > <