Fix contact sensor filter matching geometry children with same name#6556
Fix contact sensor filter matching geometry children with same name#6556ooctipus wants to merge 3 commits into
Conversation
github-actions
Bot
added
bug
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # same name as the rigid body (a common URDF-to-USD conversion artifact). | ||
| filter_prim_paths_glob = [] | ||
| for filter_expr in self.cfg.filter_prim_paths_expr: | ||
| filter_parent_expr, filter_leaf_str = filter_expr.rsplit("/", 1) |
There was a problem hiding this comment.
rsplit crash when filter expression contains no /
filter_expr.rsplit("/", 1) returns a one-element list when the expression has no /, so the two-variable unpack raises ValueError: not enough values to unpack. The original one-liner (expr.replace(".*", "*")) handled any string safely. While all valid USD prim-path expressions start with /, programmatically generated expressions that are bare patterns (e.g. a plain "Object") would crash the sensor at initialization instead of falling through to the safe raw-expression path.
| def _has_rigid_body(prim, _pattern=filter_leaf_re) -> bool: | ||
| return bool(_pattern.fullmatch(prim.GetName())) and prim.HasAPI(UsdPhysics.RigidBodyAPI) |
There was a problem hiding this comment.
_has_rigid_body requires UsdPhysics.RigidBodyAPI, so static (fixed) collision bodies used as filter targets — ground planes, walls, fixed URDF links — will never match and will fall through to the fallback at line 366. That fallback emits the bare-leaf expression, which is exactly the form that triggers PhysX prefix-matching and the count-consistency error the PR is fixing. If a user filters contacts against a static URDF asset whose collision geometry shares the same leaf name as its fixed link, they will still hit the original crash. Consider also accepting UsdPhysics.CollisionAPI (without RigidBodyAPI) as a second resolution pass, or filtering by CollisionAPI alone, to cover static bodies.
| filter_leaf_names = [prim.GetPath().pathString.rsplit("/", 1)[-1] for prim, _ in filter_matches] | ||
| _, filter_path_expr = filter_matches[0] | ||
| filter_parent_dest = filter_path_expr.rsplit("/", 1)[0] | ||
| filter_names_group = r"(" + "|".join(filter_leaf_names) + r")" | ||
| filter_prim_paths_glob.append(f"{filter_parent_dest}/{filter_names_group}".replace(".*", "*")) |
There was a problem hiding this comment.
filter_parent_dest anchored only to first match; deeper matches silently dropped
filter_parent_dest is derived exclusively from filter_matches[0]'s destination expression. If the filter expression resolves prims that live at different depths within the same env instance (e.g., env_.*/robot/Object and env_.*/Object both matching a broad pattern), only the first match's parent is used. The leaf names from all matches are joined into the alternation group, but the resulting glob path filter_parent_dest/(name1|name2) will not reach prims under a different parent. This is an edge case for narrow filter expressions, but worth a clarifying question: is the assumption that all filter expression matches always share the same parent guaranteed by the configuration API?
… name
Wrap the leaf segment of each filter_prim_paths_expr in PhysX alternation
syntax before the standard .* -> * conversion:
re.sub(r"([^/]+)$", r"(\1)", expr).replace(".*", "*")
PhysX's glob engine prefix-matches bare leaf patterns, so env_*/Object
also matches env_0/Object/Object (a geometry child sharing the same name
-- a common URDF-to-USD artifact). Wrapping in (leaf) anchors the match
to that exact path depth.
d724c5c to
af1e64f
Compare
| # anchors the match to that exact path depth. | ||
| filter_prim_paths_glob = [ | ||
| re.sub(r"([^/]+)$", r"(\1)", expr).replace(".*", "*") for expr in self.cfg.filter_prim_paths_expr | ||
| ] |
There was a problem hiding this comment.
Just need a positive and negative test for the change if it's failing before
- test_filter_prim_paths_glob_leaf_wrapping: parametrized unit test that directly exercises the re.sub + replace transformation. The expected_old column captures the pre-fix bare-leaf glob that prefix-matched geometry children; the expected_new column captures the anchored (leaf) form. - test_contact_sensor_filter_same_name_geometry_child: integration test that authors the URDF-to-USD nesting pattern (link prim /Table with geometry child /Table/Table sharing the same name) and verifies that ContactSensor initialization succeeds and reports exactly 1 filter shape per env. Before the fix this raised RuntimeError because PhysX matched 2 prims per env instead of 1.
…eometry PhysX contact view body resolution requires the RigidBodyAPI on an Xform parent prim; applying it directly to a UsdGeom.Cube shape left the body glob unresolvable, giving a null contact_view backend.
2 participants
Summary
filter_prim_paths_exprin PhysX alternation syntax(leaf)before the.*→*conversion.Root cause: URDF-to-USD conversion nests a rigid body and its geometry child under the same leaf name (e.g.
.../Objectbody,.../Object/Objectgeometry). PhysX's glob engine prefix-matches a bare leaf patternenv_*/Objectagainst both prims and raises a count-consistency error (expected 1, got 2). The same issue does not affect the sensor body resolution becausebody_names_globalready wraps leaf names in alternation(name1|name2)— anchoring the match to the correct path depth.Fix:
re.sub(r"([^/]+)$", r"(\1)", expr)captures the last path segment and wraps it in(). Example:env_.*/Objectenv_*/Objectenv_*/(Object)env_.*/Robot/.*_FOOTenv_*/Robot/*_FOOTenv_*/Robot/(*_FOOT)Not affected: Newton backend — it uses Python
fnmatchagainst physics-model body labels, which never crosses/.Test plan
ContactSensorCfgpointing at another such asset infilter_prim_paths_expr, verify initialization no longer raises a count error.🤖 Generated with Claude Code