Unverified Commit 8fbd397d authored by Mayank Mittal's avatar Mayank Mittal Committed by GitHub

Adds safe checks for input prim path to sim.utils functions (#415)

# Description

A user had a typo in their input prim path to the asset class. They did
"World/Robot" instead of "/World/Robot". The error message they got
because of this was out-of-index which is confusing.

This MR adds a check for global paths and gives a cleaner error message.

## Type of change

- Bug fix (non-breaking change which fixes an issue)

## Checklist

- [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with
`./orbit.sh --format`
- [x] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] I have run all the tests with `./orbit.sh --test` and they pass
- [ ] I have updated the changelog and the corresponding version in the
extension's `config/extension.toml` file
- [x] I have added my name to the `CONTRIBUTORS.md` or my name already
exists there
parent 05cab760
...@@ -217,6 +217,9 @@ def clone(func: Callable) -> Callable: ...@@ -217,6 +217,9 @@ def clone(func: Callable) -> Callable:
@functools.wraps(func) @functools.wraps(func)
def wrapper(prim_path: str, cfg: SpawnerCfg, *args, **kwargs): def wrapper(prim_path: str, cfg: SpawnerCfg, *args, **kwargs):
# check prim path is global
if not prim_path.startswith("/"):
raise ValueError(f"Prim path '{prim_path}' is not global. It must start with '/'.")
# resolve: {SPAWN_NS}/AssetName # resolve: {SPAWN_NS}/AssetName
# note: this assumes that the spawn namespace already exists in the stage # note: this assumes that the spawn namespace already exists in the stage
root_path, asset_path = prim_path.rsplit("/", 1) root_path, asset_path = prim_path.rsplit("/", 1)
...@@ -424,7 +427,15 @@ def export_prim_to_file(path: str, source_prim_path: str, target_prim_path: str ...@@ -424,7 +427,15 @@ def export_prim_to_file(path: str, source_prim_path: str, target_prim_path: str
Defaults to None, in which case the source prim path is used. Defaults to None, in which case the source prim path is used.
stage: The stage where the prim exists. Defaults to None, in which case the stage: The stage where the prim exists. Defaults to None, in which case the
current stage is used. current stage is used.
Raises:
ValueError: If the prim paths are not global (i.e: do not start with '/').
""" """
# check prim path is global
if not source_prim_path.startswith("/"):
raise ValueError(f"Source prim path '{source_prim_path}' is not global. It must start with '/'.")
if target_prim_path is not None and not target_prim_path.startswith("/"):
raise ValueError(f"Target prim path '{target_prim_path}' is not global. It must start with '/'.")
# get current stage # get current stage
if stage is None: if stage is None:
stage: Usd.Stage = omni.usd.get_context().get_stage() stage: Usd.Stage = omni.usd.get_context().get_stage()
...@@ -474,9 +485,14 @@ def make_uninstanceable(prim_path: str, stage: Usd.Stage | None = None): ...@@ -474,9 +485,14 @@ def make_uninstanceable(prim_path: str, stage: Usd.Stage | None = None):
Args: Args:
prim_path: The prim path to check. prim_path: The prim path to check.
stage: The stage where the prim exists. stage: The stage where the prim exists. Defaults to None, in which case the current stage is used.
Defaults to None, in which case the current stage is used.
Raises:
ValueError: If the prim path is not global (i.e: does not start with '/').
""" """
# check if prim path is global
if not prim_path.startswith("/"):
raise ValueError(f"Prim path '{prim_path}' is not global. It must start with '/'.")
# get current stage # get current stage
if stage is None: if stage is None:
stage = stage_utils.get_current_stage() stage = stage_utils.get_current_stage()
...@@ -515,7 +531,13 @@ def get_first_matching_child_prim( ...@@ -515,7 +531,13 @@ def get_first_matching_child_prim(
Returns: Returns:
The first prim on the path that passes the predicate. If no prim passes the predicate, it returns None. The first prim on the path that passes the predicate. If no prim passes the predicate, it returns None.
Raises:
ValueError: If the prim path is not global (i.e: does not start with '/').
""" """
# check if prim path is global
if not prim_path.startswith("/"):
raise ValueError(f"Prim path '{prim_path}' is not global. It must start with '/'.")
# get current stage # get current stage
if stage is None: if stage is None:
stage = stage_utils.get_current_stage() stage = stage_utils.get_current_stage()
...@@ -555,7 +577,13 @@ def get_all_matching_child_prims( ...@@ -555,7 +577,13 @@ def get_all_matching_child_prims(
Returns: Returns:
A list containing all the prims matching the predicate. A list containing all the prims matching the predicate.
Raises:
ValueError: If the prim path is not global (i.e: does not start with '/').
""" """
# check if prim path is global
if not prim_path.startswith("/"):
raise ValueError(f"Prim path '{prim_path}' is not global. It must start with '/'.")
# get current stage # get current stage
if stage is None: if stage is None:
stage = stage_utils.get_current_stage() stage = stage_utils.get_current_stage()
...@@ -594,7 +622,13 @@ def find_first_matching_prim(prim_path_regex: str, stage: Usd.Stage | None = Non ...@@ -594,7 +622,13 @@ def find_first_matching_prim(prim_path_regex: str, stage: Usd.Stage | None = Non
Returns: Returns:
The first prim that matches input expression. If no prim matches, returns None. The first prim that matches input expression. If no prim matches, returns None.
Raises:
ValueError: If the prim path is not global (i.e: does not start with '/').
""" """
# check prim path is global
if not prim_path_regex.startswith("/"):
raise ValueError(f"Prim path '{prim_path_regex}' is not global. It must start with '/'.")
# get current stage # get current stage
if stage is None: if stage is None:
stage = stage_utils.get_current_stage() stage = stage_utils.get_current_stage()
...@@ -618,7 +652,13 @@ def find_matching_prims(prim_path_regex: str, stage: Usd.Stage | None = None) -> ...@@ -618,7 +652,13 @@ def find_matching_prims(prim_path_regex: str, stage: Usd.Stage | None = None) ->
Returns: Returns:
A list of prims that match input expression. A list of prims that match input expression.
Raises:
ValueError: If the prim path is not global (i.e: does not start with '/').
""" """
# check prim path is global
if not prim_path_regex.startswith("/"):
raise ValueError(f"Prim path '{prim_path_regex}' is not global. It must start with '/'.")
# get current stage # get current stage
if stage is None: if stage is None:
stage = stage_utils.get_current_stage() stage = stage_utils.get_current_stage()
...@@ -649,6 +689,9 @@ def find_matching_prim_paths(prim_path_regex: str, stage: Usd.Stage | None = Non ...@@ -649,6 +689,9 @@ def find_matching_prim_paths(prim_path_regex: str, stage: Usd.Stage | None = Non
Returns: Returns:
A list of prim paths that match input expression. A list of prim paths that match input expression.
Raises:
ValueError: If the prim path is not global (i.e: does not start with '/').
""" """
# obtain matching prims # obtain matching prims
output_prims = find_matching_prims(prim_path_regex, stage) output_prims = find_matching_prims(prim_path_regex, stage)
......
...@@ -53,6 +53,10 @@ class TestUtilities(unittest.TestCase): ...@@ -53,6 +53,10 @@ class TestUtilities(unittest.TestCase):
orbit_result = sim_utils.get_all_matching_child_prims("/World") orbit_result = sim_utils.get_all_matching_child_prims("/World")
self.assertListEqual(isaac_sim_result, orbit_result) self.assertListEqual(isaac_sim_result, orbit_result)
# test valid path
with self.assertRaises(ValueError):
sim_utils.get_all_matching_child_prims("World/Room")
def test_find_matching_prim_paths(self): def test_find_matching_prim_paths(self):
"""Test find_matching_prim_paths() function.""" """Test find_matching_prim_paths() function."""
# create scene # create scene
...@@ -78,6 +82,10 @@ class TestUtilities(unittest.TestCase): ...@@ -78,6 +82,10 @@ class TestUtilities(unittest.TestCase):
orbit_result = sim_utils.find_matching_prim_paths("/World/Floor_.*/Sphere/childSphere.*") orbit_result = sim_utils.find_matching_prim_paths("/World/Floor_.*/Sphere/childSphere.*")
self.assertListEqual(isaac_sim_result, orbit_result) self.assertListEqual(isaac_sim_result, orbit_result)
# test valid path
with self.assertRaises(ValueError):
sim_utils.get_all_matching_child_prims("World/Floor_.*")
if __name__ == "__main__": if __name__ == "__main__":
try: try:
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment