From 9a27fc14c25f1101cbb8779f6c7d4aac1942d2e0 Mon Sep 17 00:00:00 2001 From: Nimrod Gileadi Date: Wed, 4 Sep 2024 07:19:27 -0700 Subject: [PATCH] Raise appropriate errors for mj_addBufferVFS in Python bindings. When loading models in the XML bindings, different issues with the asset dictionary were all reported as "assets dict is too big". PiperOrigin-RevId: 670960339 Change-Id: I2e47d91a6b433fd90ebdf9960a9b5b47413af410 --- doc/changelog.rst | 1 + python/mujoco/bindings_test.py | 9 ++++++++- python/mujoco/structs.cc | 8 +++++++- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/doc/changelog.rst b/doc/changelog.rst index 338ed5fd..3ca2baab 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -59,6 +59,7 @@ Bug fixes Python bindings ^^^^^^^^^^^^^^^ - Added support for engine plugins in :ref:`mjSpec` (:github:issue:`1903`). +- Better error reporting for issues with the assets dictionary, when loading models. Version 3.2.2 (Aug 8, 2024) diff --git a/python/mujoco/bindings_test.py b/python/mujoco/bindings_test.py index 52b3efd1..32f5c0e6 100644 --- a/python/mujoco/bindings_test.py +++ b/python/mujoco/bindings_test.py @@ -128,6 +128,14 @@ class MuJoCoBindingsTest(parameterized.TestCase): self.assertEqual( mujoco.mj_name2id(model, mujoco.mjtObj.mjOBJ_GEOM, 'ball'), 2) + def test_load_xml_repeated_asset_name(self): + # Assets aren't allowed to have the same filename (even if they have + # different paths). + with self.assertRaisesRegex(ValueError, r'Repeated.*'): + mujoco.MjModel.from_xml_string( + '', {'asset.xml': b'asset1', 'path/asset.xml': b'asset2'} + ) + def test_can_read_array(self): np.testing.assert_array_equal( self.model.body_pos, @@ -766,7 +774,6 @@ class MuJoCoBindingsTest(parameterized.TestCase): np.testing.assert_array_equal(qvel, self.data.qvel) np.testing.assert_array_equal(act, self.data.act) - def test_mj_angmomMat(self): # pylint: disable=invalid-name self.data.qvel = np.ones(self.model.nv, np.float64) mujoco.mj_forward(self.model, self.data) diff --git a/python/mujoco/structs.cc b/python/mujoco/structs.cc index 20b6678a..fd060fb1 100644 --- a/python/mujoco/structs.cc +++ b/python/mujoco/structs.cc @@ -345,7 +345,13 @@ static raw::MjModel* LoadModelFileImpl( const int vfs_error = InterceptMjErrors(mj_addBufferVFS)( vfs_ptr, buffer_name.c_str(), asset.content, asset.content_size); if (vfs_error) { - throw py::value_error("assets dict is too big"); + mj_deleteVFS(vfs_ptr); + if (vfs_error == 2) { + throw py::value_error("Repeated file name in assets dict: " + + buffer_name); + } else { + throw py::value_error("Asset failed to load: " + buffer_name); + } } } }