Changes default value in USD attribute setters and fixes tests (#219)

# Description

This MR does the following:
* Removes the default value for argument `camel_case` in the USD setter
functions. This was causing confusion when reading code.
* Fixed the UI behavior that material was getting selected on spawning
* Fixed the unit tests under `sim`

## 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`
- [ ] 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 updated the changelog and the corresponding version in the
extension's `config/extension.toml` file

source/extensions/omni.isaac.orbit/config/extension.toml

 [package]
 
 # Note: Semantic Versioning is used: https://semver.org/
-version = "0.9.26"
+version = "0.9.27"
 
 # Description
 title = "ORBIT framework for Robot Learning"

source/extensions/omni.isaac.orbit/docs/CHANGELOG.rst

 Changelog
 ---------
 
+0.9.27 (2023-10-31)
+~~~~~~~~~~~~~~~~~~~
+
+Changed
+^^^^^^^
+
+* Removed the default value of the argument ``camel_case`` in setters of USD attributes. This is to avoid
+  confusion with the naming of the attributes in the USD file.
+
+Fixed
+^^^^^
+
+* Fixed the selection of material prim in the :class:`omni.isaac.orbit.sim.spawners.materials.spawn_preview_surface`
+  method. Earlier, the created prim was being selected in the viewport which interfered with the selection of
+  prims by the user.
+* Updated :class:`omni.isaac.orbit.sim.converters.MeshConverter` to use a different stage than the default stage
+  for the conversion. This is to avoid the issue of the stage being closed when the conversion is done.
+
+
 0.9.26 (2023-10-31)
 ~~~~~~~~~~~~~~~~~~~
 

source/extensions/omni.isaac.orbit/omni/isaac/orbit/sim/converters/mesh_converter.py

@@ -106,12 +106,16 @@ class MeshConverter(AssetConverterBase):
             stage_or_context_kwargs = {}
         else:
             # create a new stage
-            stage = Usd.Stage.CreateNew(os.path.join(self.usd_dir, self.usd_instanceable_meshes_path))
+            stage = Usd.Stage.Open(self.usd_path)
             # add USD to stage cache
             stage_id = UsdUtils.StageCache.Get().Insert(stage)
             # need to make kwargs for compatibility with 2022
             stage_kwargs = {"stage": stage}
             stage_or_context_kwargs = {"stage_or_context": stage}
+            # FIXME: we need to hack this into command because Kit 105 has a bug.
+            from omni.usd.commands import MovePrimCommand
+
+            MovePrimCommand._selection = None  # type: ignore
         # Set stage up-axis to Z
         # note: later we need to rotate the mesh so that it is Z-up in the world
         UsdGeom.SetStageUpAxis(stage, UsdGeom.Tokens.z)
@@ -162,7 +166,7 @@ class MeshConverter(AssetConverterBase):
                 mesh_collision_api.GetApproximationAttr().Set(cfg.collision_approximation)
                 # -- Collider properties such as offset, scale, etc.
                 schemas.define_collision_properties(
-                    prim_path=new_child_mesh_prim_path, cfg=cfg.collision_props, stage=stage
+                    prim_path=child_mesh_prim.GetPath(), cfg=cfg.collision_props, stage=stage
                 )
         # Delete the old Xform and make the new Xform the default prim
         stage.SetDefaultPrim(new_xform_prim)

source/extensions/omni.isaac.orbit/omni/isaac/orbit/sim/schemas/schemas.py

@@ -97,7 +97,7 @@ def modify_articulation_root_properties(
     cfg = cfg.to_dict()
     # set into physx api
     for attr_name, value in cfg.items():
-        safe_set_attribute_on_usd_schema(physx_articulation_api, attr_name, value)
+        safe_set_attribute_on_usd_schema(physx_articulation_api, attr_name, value, camel_case=True)
     # success
     return True
 
@@ -187,10 +187,10 @@ def modify_rigid_body_properties(
     # set into USD API
     for attr_name in ["rigid_body_enabled", "kinematic_enabled"]:
         value = cfg.pop(attr_name, None)
-        safe_set_attribute_on_usd_schema(usd_rigid_body_api, attr_name, value)
+        safe_set_attribute_on_usd_schema(usd_rigid_body_api, attr_name, value, camel_case=True)
     # set into PhysX API
     for attr_name, value in cfg.items():
-        safe_set_attribute_on_usd_schema(physx_rigid_body_api, attr_name, value)
+        safe_set_attribute_on_usd_schema(physx_rigid_body_api, attr_name, value, camel_case=True)
     # success
     return True
 
@@ -277,10 +277,10 @@ def modify_collision_properties(
     # set into USD API
     for attr_name in ["collision_enabled"]:
         value = cfg.pop(attr_name, None)
-        safe_set_attribute_on_usd_schema(usd_collision_api, attr_name, value)
+        safe_set_attribute_on_usd_schema(usd_collision_api, attr_name, value, camel_case=True)
     # set into PhysX API
     for attr_name, value in cfg.items():
-        safe_set_attribute_on_usd_schema(physx_collision_api, attr_name, value)
+        safe_set_attribute_on_usd_schema(physx_collision_api, attr_name, value, camel_case=True)
     # success
     return True
 
@@ -362,7 +362,7 @@ def modify_mass_properties(prim_path: str, cfg: schemas_cfg.MassPropertiesCfg, s
     # set into USD API
     for attr_name in ["mass", "density"]:
         value = cfg.pop(attr_name, None)
-        safe_set_attribute_on_usd_schema(usd_physics_mass_api, attr_name, value)
+        safe_set_attribute_on_usd_schema(usd_physics_mass_api, attr_name, value, camel_case=True)
     # success
     return True
 

source/extensions/omni.isaac.orbit/omni/isaac/orbit/sim/spawners/lights/lights.py

@@ -79,6 +79,6 @@ def spawn_light(
             else:
                 prim_prop_name = f"inputs:{attr_name}"
             # set the attribute
-            safe_set_attribute_on_usd_prim(prim, prim_prop_name, value)
+            safe_set_attribute_on_usd_prim(prim, prim_prop_name, value, camel_case=True)
     # return the prim
     return prim

source/extensions/omni.isaac.orbit/omni/isaac/orbit/sim/spawners/materials/physics_materials.py

@@ -65,9 +65,9 @@ def spawn_rigid_body_material(prim_path: str, cfg: physics_materials_cfg.RigidBo
     # set into USD API
     for attr_name in ["static_friction", "dynamic_friction", "restitution"]:
         value = cfg.pop(attr_name, None)
-        safe_set_attribute_on_usd_schema(usd_physics_material_api, attr_name, value)
+        safe_set_attribute_on_usd_schema(usd_physics_material_api, attr_name, value, camel_case=True)
     # set into PhysX API
     for attr_name, value in cfg.items():
-        safe_set_attribute_on_usd_schema(physx_material_api, attr_name, value)
+        safe_set_attribute_on_usd_schema(physx_material_api, attr_name, value, camel_case=True)
     # return the prim
     return prim

source/extensions/omni.isaac.orbit/omni/isaac/orbit/sim/spawners/materials/visual_materials.py

@@ -48,7 +48,7 @@ def spawn_preview_surface(prim_path: str, cfg: visual_materials_cfg.PreviewSurfa
     """
     # spawn material if it doesn't exist.
     if not prim_utils.is_prim_path_valid(prim_path):
-        omni.kit.commands.execute("CreatePreviewSurfaceMaterialPrim", mtl_path=prim_path, select_new_prim=True)
+        omni.kit.commands.execute("CreatePreviewSurfaceMaterialPrim", mtl_path=prim_path, select_new_prim=False)
     else:
         raise ValueError(f"A prim already exists at path: '{prim_path}'.")
     # obtain prim
@@ -57,7 +57,7 @@ def spawn_preview_surface(prim_path: str, cfg: visual_materials_cfg.PreviewSurfa
     cfg = cfg.to_dict()
     del cfg["func"]
     for attr_name, attr_value in cfg.items():
-        safe_set_attribute_on_usd_prim(prim, f"inputs:{attr_name}", attr_value)
+        safe_set_attribute_on_usd_prim(prim, f"inputs:{attr_name}", attr_value, camel_case=True)
     # return prim
     return prim
 

source/extensions/omni.isaac.orbit/omni/isaac/orbit/sim/utils.py

@@ -6,6 +6,7 @@
 from __future__ import annotations
 
 import functools
+import inspect
 import re
 from typing import TYPE_CHECKING, Any, Callable
 
@@ -29,7 +30,7 @@ Attribute - Setters.
 """
 
 
-def safe_set_attribute_on_usd_schema(schema_api: Usd.APISchemaBase, name: str, value: Any, camel_case: bool = True):
+def safe_set_attribute_on_usd_schema(schema_api: Usd.APISchemaBase, name: str, value: Any, camel_case: bool):
     """Set the value of an attribute on its USD schema if it exists.
 
     A USD API schema serves as an interface or API for authoring and extracting a set of attributes.
@@ -40,7 +41,7 @@ def safe_set_attribute_on_usd_schema(schema_api: Usd.APISchemaBase, name: str, v
         schema_api: The USD schema to set the attribute on.
         name: The name of the attribute.
         value: The value to set the attribute to.
-        camel_case: Whether to convert the attribute name to camel case. Defaults to True.
+        camel_case: Whether to convert the attribute name to camel case.
 
     Raises:
         TypeError: When the input attribute name does not exist on the provided schema API.
@@ -66,7 +67,7 @@ def safe_set_attribute_on_usd_schema(schema_api: Usd.APISchemaBase, name: str, v
         raise TypeError(f"Attribute '{attr_name}' does not exist on prim '{schema_api.GetPath()}'.")
 
 
-def safe_set_attribute_on_usd_prim(prim: Usd.Prim, attr_name: str, value: Any, camel_case: bool = True):
+def safe_set_attribute_on_usd_prim(prim: Usd.Prim, attr_name: str, value: Any, camel_case: bool):
     """Set the value of a attribute on its USD prim.
 
     The function creates a new attribute if it does not exist on the prim. This is because in some cases (such
@@ -77,7 +78,7 @@ def safe_set_attribute_on_usd_prim(prim: Usd.Prim, attr_name: str, value: Any, c
         prim: The USD prim to set the attribute on.
         attr_name: The name of the attribute.
         value: The value to set the attribute to.
-        camel_case: Whether to convert the attribute name to camel case. Defaults to True.
+        camel_case: Whether to convert the attribute name to camel case.
     """
     # if value is None, do nothing
     if value is None:
@@ -142,8 +143,12 @@ def apply_nested(func: Callable) -> Callable:
 
     @functools.wraps(func)
     def wrapper(prim_path: str, *args, **kwargs):
+        # map args and kwargs to function signature so we can get the stage
+        # note: we do this to check if stage is given in arg or kwarg
+        sig = inspect.signature(func)
+        bound_args = sig.bind(prim_path, *args, **kwargs)
         # get current stage
-        stage = kwargs.get("stage")
+        stage = bound_args.arguments.get("stage")
         if stage is None:
             stage = stage_utils.get_current_stage()
         # get USD prim

source/extensions/omni.isaac.orbit/test/sensors/test_camera.py

@@ -83,6 +83,7 @@ class TestCamera(unittest.TestCase):
         self.sim._timeline.stop()
         # clear the stage
         self.sim.clear()
+        self.sim.clear_all_callbacks()
         self.sim.clear_instance()
 
     """

source/extensions/omni.isaac.orbit/test/sim/test_mesh_converter.py

@@ -87,8 +87,6 @@ class TestMeshConverter(unittest.TestCase):
         mesh_converter = MeshConverter(mesh_config)
         time_usd_file_created = os.stat(mesh_converter.usd_path).st_mtime_ns
 
-        omni.usd.get_context().close_stage()
-
         # change the config
         new_config = mesh_config
         new_config.make_instanceable = not mesh_config.make_instanceable

source/extensions/omni.isaac.orbit/test/sim/test_schemas.py

@@ -19,6 +19,7 @@ import unittest
 
 import carb
 import omni.isaac.core.utils.prims as prim_utils
+import omni.isaac.core.utils.stage as stage_utils
 from omni.isaac.core.simulation_context import SimulationContext
 from pxr import UsdPhysics
 
@@ -32,6 +33,8 @@ class TestPhysicsSchema(unittest.TestCase):
 
     def setUp(self) -> None:
         """Create a blank new stage for each test."""
+        # Create a new stage
+        stage_utils.create_new_stage()
         # Simulation time-step
         self.dt = 0.1
         # Load kit helper
@@ -76,6 +79,8 @@ class TestPhysicsSchema(unittest.TestCase):
         # stop simulation
         self.sim.stop()
         self.sim.clear()
+        self.sim.clear_all_callbacks()
+        self.sim.clear_instance()
 
     def test_valid_properties_cfg(self):
         """Test that all the config instances have non-None values.

source/extensions/omni.isaac.orbit/test/sim/test_spawn_from_files.py

@@ -31,6 +31,8 @@ class TestSpawningFromFiles(unittest.TestCase):
 
     def setUp(self) -> None:
         """Create a blank new stage for each test."""
+        # Create a new stage
+        stage_utils.create_new_stage()
         # Simulation time-step
         self.dt = 0.1
         # Load kit helper
@@ -45,6 +47,8 @@ class TestSpawningFromFiles(unittest.TestCase):
         # stop simulation
         self.sim.stop()
         self.sim.clear()
+        self.sim.clear_all_callbacks()
+        self.sim.clear_instance()
 
     """
     Basic spawning.

source/extensions/omni.isaac.orbit/test/sim/test_spawn_lights.py

@@ -33,6 +33,8 @@ class TestSpawningLights(unittest.TestCase):
 
     def setUp(self) -> None:
         """Create a blank new stage for each test."""
+        # Create a new stage
+        stage_utils.create_new_stage()
         # Simulation time-step
         self.dt = 0.1
         # Load kit helper
@@ -47,6 +49,8 @@ class TestSpawningLights(unittest.TestCase):
         # stop simulation
         self.sim.stop()
         self.sim.clear()
+        self.sim.clear_all_callbacks()
+        self.sim.clear_instance()
 
     """
     Basic spawning.
@@ -133,10 +137,13 @@ class TestSpawningLights(unittest.TestCase):
             prim_path: The prim name.
             cfg: The configuration for the light source.
         """
+        # default list of params to skip
+        non_usd_params = ["func", "prim_type", "visible", "semantic_tags", "copy_from_source"]
+        # obtain prim
         prim = prim_utils.get_prim_at_path(prim_path)
         for attr_name, attr_value in cfg.__dict__.items():
             # skip names we know are not present
-            if attr_name in ["func", "prim_type"] or attr_value is None:
+            if attr_name in non_usd_params or attr_value is None:
                 continue
             # deal with texture input names
             if "texture" in attr_name:
@@ -155,7 +162,7 @@ class TestSpawningLights(unittest.TestCase):
                 # configured value
                 configured_value = prim.GetAttribute(prim_prop_name).Get()
             # validate the values
-            self.assertEqual(configured_value, attr_value)
+            self.assertEqual(configured_value, attr_value, msg=f"Failed for attribute: '{attr_name}'")
 
 
 if __name__ == "__main__":

source/extensions/omni.isaac.orbit/test/sim/test_spawn_materials.py

@@ -32,6 +32,8 @@ class TestSpawningMaterials(unittest.TestCase):
 
     def setUp(self) -> None:
         """Create a blank new stage for each test."""
+        # Create a new stage
+        stage_utils.create_new_stage()
         # Simulation time-step
         self.dt = 0.1
         # Load kit helper
@@ -44,6 +46,8 @@ class TestSpawningMaterials(unittest.TestCase):
         # stop simulation
         self.sim.stop()
         self.sim.clear()
+        self.sim.clear_all_callbacks()
+        self.sim.clear_instance()
 
     def test_spawn_preview_surface(self):
         """Test spawning preview surface."""

source/extensions/omni.isaac.orbit/test/sim/test_spawn_sensors.py

@@ -36,6 +36,8 @@ class TestSpawningSensors(unittest.TestCase):
 
     def setUp(self) -> None:
         """Create a blank new stage for each test."""
+        # Create a new stage
+        stage_utils.create_new_stage()
         # Simulation time-step
         self.dt = 0.1
         # Load kit helper
@@ -50,6 +52,8 @@ class TestSpawningSensors(unittest.TestCase):
         # stop simulation
         self.sim.stop()
         self.sim.clear()
+        self.sim.clear_all_callbacks()
+        self.sim.clear_instance()
 
     """
     Basic spawning.

source/extensions/omni.isaac.orbit/test/sim/test_spawn_shapes.py

@@ -30,6 +30,8 @@ class TestSpawningUsdGeometries(unittest.TestCase):
 
     def setUp(self) -> None:
         """Create a blank new stage for each test."""
+        # Create a new stage
+        stage_utils.create_new_stage()
         # Simulation time-step
         self.dt = 0.1
         # Load kit helper
@@ -42,6 +44,8 @@ class TestSpawningUsdGeometries(unittest.TestCase):
         # stop simulation
         self.sim.stop()
         self.sim.clear()
+        self.sim.clear_all_callbacks()
+        self.sim.clear_instance()
 
     """
     Basic spawning.

source/extensions/omni.isaac.orbit/test/sim/test_urdf_converter.py

@@ -24,6 +24,7 @@ import unittest
 
 import carb
 import omni.isaac.core.utils.prims as prim_utils
+import omni.isaac.core.utils.stage as stage_utils
 from omni.isaac.core.articulations import ArticulationView
 from omni.isaac.core.simulation_context import SimulationContext
 from omni.isaac.core.utils.extensions import get_extension_path_from_name
@@ -37,6 +38,8 @@ class TestUrdfConverter(unittest.TestCase):
 
     def setUp(self):
         """Create a blank new stage for each test."""
+        # Create a new stage
+        stage_utils.create_new_stage()
         # Isaac Sim version
         self.isaacsim_version_year = int(get_version()[2])
         # retrieve path to urdf importer extension
@@ -138,10 +141,12 @@ class TestUrdfConverter(unittest.TestCase):
         np.testing.assert_array_equal(drive_stiffness[:, :7], expected_drive_stiffness)
         np.testing.assert_array_equal(drive_damping[:, :7], expected_drive_damping)
         # -- for the hand (prismatic joints)
+        # note: from isaac sim 2023.1, the test asset has mimic joints for the hand
+        #  so the mimic joint doesn't have drive values
         expected_drive_stiffness = self.config.default_drive_stiffness
         expected_drive_damping = self.config.default_drive_damping
-        np.testing.assert_array_equal(drive_stiffness[:, 7:], expected_drive_stiffness)
-        np.testing.assert_array_equal(drive_damping[:, 7:], expected_drive_damping)
+        np.testing.assert_array_equal(drive_stiffness[:, 7], expected_drive_stiffness)
+        np.testing.assert_array_equal(drive_damping[:, 7], expected_drive_damping)
 
         # check drive values for the robot (read from usd)
         self.sim.stop()
@@ -153,10 +158,12 @@ class TestUrdfConverter(unittest.TestCase):
         np.testing.assert_array_equal(drive_stiffness[:, :7], expected_drive_stiffness)
         np.testing.assert_array_equal(drive_damping[:, :7], expected_drive_damping)
         # -- for the hand (prismatic joints)
+        # note: from isaac sim 2023.1, the test asset has mimic joints for the hand
+        #  so the mimic joint doesn't have drive values
         expected_drive_stiffness = self.config.default_drive_stiffness
         expected_drive_damping = self.config.default_drive_damping
-        np.testing.assert_array_equal(drive_stiffness[:, 7:], expected_drive_stiffness)
-        np.testing.assert_array_equal(drive_damping[:, 7:], expected_drive_damping)
+        np.testing.assert_array_equal(drive_stiffness[:, 7], expected_drive_stiffness)
+        np.testing.assert_array_equal(drive_damping[:, 7], expected_drive_damping)
 
 
 if __name__ == "__main__":