Informative error if mis-naming the top-level default class.

Also:
- Move the the top-level default class name into the mjCModel constructor.
- Fix bug in default class propagation in frame elements.
PiperOrigin-RevId: 646384669
Change-Id: Icc6154124361a234b84a07e2b5ecaa7c8b18ebf7
This commit is contained in:
Yuval Tassa
2024-06-25 01:45:10 -07:00
committed by Copybara-Service
parent 9e9a0c637e
commit d8b85245a7
4 changed files with 86 additions and 8 deletions
+1
View File
@@ -97,6 +97,7 @@ mjCModel::mjCModel() {
//------------------------ master default set
defaults_.push_back(new mjCDef);
defaults_.back()->name = "main";
// world body
mjCBody* world = new mjCBody(this);
+9 -6
View File
@@ -2673,17 +2673,15 @@ void mjXReader::Default(XMLElement* section, int parentid) {
mjsDefault* def;
int thisid;
// create new default, except at top level (already added in mjCModel ctor)
// create new default, except at top level (already added in mjCModel constructor)
text.clear();
ReadAttrTxt(section, "class", text);
if (text.empty()) {
if (parentid>=0) {
if (parentid >= 0) {
throw mjXError(section, "empty class name");
} else {
text = "main";
}
}
if (parentid>=0) {
if (parentid >= 0) {
def = mjs_addDefault(model, text.c_str(), parentid, &thisid);
if (!def) {
throw mjXError(section, "repeated default class name");
@@ -2691,7 +2689,9 @@ void mjXReader::Default(XMLElement* section, int parentid) {
} else {
thisid = 0;
def = mjs_getSpecDefault(model);
mjs_setString(def->name, text.c_str());
if (!text.empty() && text != "main") {
throw mjXError(section, "top-level default class 'main' cannot be renamed");
}
}
// iterate over elements other than nested defaults
@@ -3497,6 +3497,9 @@ void mjXReader::Body(XMLElement* section, mjsBody* pbody, mjsFrame* frame) {
mjs_setString(pchild->info,
std::string("line " + std::to_string(elem->GetLineNum())).c_str());
// set default from class or childclass
mjs_setDefault(pchild->element, childdef ? childdef : def);
// read attributes
std::string name, childclass;
if (ReadAttrTxt(elem, "name", name)) {
+72
View File
@@ -464,6 +464,39 @@ TEST_F(XMLReaderTest, InvalidDoubleOrientation) {
}
}
TEST_F(XMLReaderTest, ClassOverridesChildclass) {
static constexpr char xml[] = R"(
<mujoco>
<default>
<default class="size2">
<geom size="2"/>
</default>
<default class="size3">
<geom size="3"/>
</default>
</default>
<worldbody>
<frame childclass="size2">
<geom/>
<geom class="size3"/>
<body childclass="size2">
<geom/>
<geom class="size3"/>
</body>
</frame>
</worldbody>
</mujoco>
)";
std::array<char, 1024> error;
mjModel* model = LoadModelFromString(xml, error.data(), error.size());
ASSERT_THAT(model, NotNull()) << error.data();
EXPECT_EQ(model->geom_size[3*0], 2);
EXPECT_EQ(model->geom_size[3*1], 3);
EXPECT_EQ(model->geom_size[3*2], 2);
EXPECT_EQ(model->geom_size[3*3], 3);
mj_deleteModel(model);
}
TEST_F(XMLReaderTest, RepeatedDefaultName) {
static constexpr char xml[] = R"(
<mujoco>
@@ -511,6 +544,45 @@ TEST_F(XMLReaderTest, InvalidDefaultClassName) {
HasSubstr("Element 'geom'"), HasSubstr("line 10")));
}
TEST_F(XMLReaderTest, InvalidTopDefaultClassName) {
static constexpr char xml[] = R"(
<mujoco>
<default class="sphere">
<geom type="sphere" size="1"/>
</default>
<worldbody>
<body>
<geom class="sphere"/>
</body>
</worldbody>
</mujoco>
)";
std::array<char, 1024> error;
mjModel* model = LoadModelFromString(xml, error.data(), error.size());
ASSERT_THAT(model, IsNull()) << error.data();
EXPECT_THAT(error.data(),
HasSubstr("top-level default class 'main' cannot be renamed"));
}
TEST_F(XMLReaderTest, ValidTopDefaultClassName) {
static constexpr char xml[] = R"(
<mujoco>
<default class="main">
<geom type="sphere" size="1"/>
</default>
<worldbody>
<body>
<geom class="main"/>
</body>
</worldbody>
</mujoco>
)";
std::array<char, 1024> error;
mjModel* model = LoadModelFromString(xml, error.data(), error.size());
ASSERT_THAT(model, NotNull()) << error.data();
mj_deleteModel(model);
}
// ------------------------ test including -------------------------------------
// tiny RGB 2 x 3 PNG file
+4 -2
View File
@@ -727,8 +727,10 @@ TEST_F(XMLWriterTest, WritesActuatorDefaults) {
TEST_F(XMLWriterTest, WritesFrameDefaults) {
static constexpr char xml[] = R"(
<mujoco>
<default class="dframe">
<geom size=".1"/>
<default>
<default class="dframe">
<geom size=".1"/>
</default>
</default>
<worldbody>