Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix loading nested include with custom attributes #789

Merged
merged 2 commits into from
Dec 17, 2021
Merged

Fix loading nested include with custom attributes #789

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1c6c0a5
nested include w/ custom elem bug fix
jennuine Dec 16, 2021
c54dccb
compare custom elems w/ string literals
jennuine Dec 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/parser.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1373,6 +1373,7 @@ void addNestedModel(ElementPtr _sdf, ElementPtr _includeSDF, Errors &_errors)
}

_includeSDF->ClearElements();
_includeSDF->RemoveAllAttributes();
readString(str, _includeSDF, _errors);

elem = _includeSDF->GetElement("model")->GetFirstElement();
Expand Down
5 changes: 5 additions & 0 deletions test/integration/model/model_with_custom_elements/model.config
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<?xml version="1.0"?>
<model>
<name>model_with_custom_elements</name>
<sdf version="1.7">model.sdf</sdf>
</model>
22 changes: 22 additions & 0 deletions test/integration/model/model_with_custom_elements/model.sdf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?xml version="1.0" ?>
<sdf version="1.7" xmlns:mysim="http://example.org/mysim/schema">
<model name="M1">
<link name="L1" mysim:custom_attr_str="A" mysim:custom_attr_int="5" />
<link name="L2" />
<joint name="J1" type="revolute">
<parent>L1</parent>
<child>L2</child>
</joint>

<model name="M2">
<link name="L1" mysim:custom_attr_str="B">
<mysim:transmission name="simple_trans">
<mysim:type>transmission_interface/SimpleTransmission</mysim:type>
<mysim:joint name="J1">
<mysim:hardwareInterface>EffortJointInterface</mysim:hardwareInterface>
</mysim:joint>
</mysim:transmission>
</link>
</model>
</model>
</sdf>
219 changes: 219 additions & 0 deletions test/integration/sdf_custom.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,222 @@ TEST(SDFParser, CustomElements)
tranJointElement->Get<std::string>("mysim:hardwareInterface");
EXPECT_EQ("EffortJointInterface", tranHwInterface);
}

/////////////////////////////////////////////////
TEST(SDFParser, ReloadCustomElements)
{
const std::string sdfTestFile =
sdf::testing::TestFile("integration", "custom_elems_attrs.sdf");

// load file with custom elements
sdf::Root root1;
sdf::Errors errors = root1.Load(sdfTestFile);
EXPECT_TRUE(errors.empty());

// reload string output of root1
sdf::Root root2;
errors = root2.LoadSdfString(root1.Element()->ToString(""));
EXPECT_TRUE(errors.empty());

// check that root1 and root2 equal
const sdf::World *world1 = root1.WorldByIndex(0);
const sdf::World *world2 = root2.WorldByIndex(0);
ASSERT_NE(nullptr, world1);
ASSERT_NE(nullptr, world2);

const sdf::Model *model1 = world1->ModelByIndex(0);
const sdf::Model *model2 = world2->ModelByIndex(0);
ASSERT_NE(nullptr, model1);
ASSERT_NE(nullptr, model2);
EXPECT_EQ(model1->Element()->ToString(""), model2->Element()->ToString(""));

const sdf::Link *link1 = model1->LinkByIndex(0);
const sdf::Link *link2 = model2->LinkByIndex(0);
ASSERT_NE(nullptr, link1);
ASSERT_NE(nullptr, link2);
EXPECT_EQ(link1->Element()->ToString(""), link2->Element()->ToString(""));

sdf::ElementPtr customElem1 =
model1->Element()->FindElement("mysim:transmission");
sdf::ElementPtr customElem2 =
model2->Element()->FindElement("mysim:transmission");
ASSERT_NE(nullptr, customElem1);
ASSERT_NE(nullptr, customElem2);
EXPECT_EQ(customElem1->ToString(""), customElem2->ToString(""));
sdf::ElementPtr customDesc1 =
world1->Element()->FindElement("mysim:description");
sdf::ElementPtr customDesc2 =
world2->Element()->FindElement("mysim:description");
ASSERT_NE(nullptr, customDesc1);
ASSERT_NE(nullptr, customDesc2);
EXPECT_EQ(customDesc1->ToString(""), customDesc2->ToString(""));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we compare the result of customDesc2->ToString with the expected string literal instead of trusting customDesc1->ToString is working properly? Here and below. Not to make to verbose, I would only do it on the comparisons that involve custom elements.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c54dccb

}

/////////////////////////////////////////////////
TEST(SDFParser, ReloadIncludedCustomElements)
{
const std::string modelPath = sdf::testing::TestFile("integration", "model");

sdf::setFindCallback(
[&](const std::string &_file)
{
return sdf::filesystem::append(modelPath, _file);
});

const std::string sdfStr =
R"(<sdf version='1.7'>
<world name='default'>
<include>
<uri>model_with_custom_elements</uri>
</include>
</world>
</sdf>
)";

// load included file with custom elements
sdf::Root root1;
sdf::Errors errors = root1.LoadSdfString(sdfStr);
EXPECT_TRUE(errors.empty());

// reload string output of root1
sdf::Root root2;
errors = root2.LoadSdfString(root1.Element()->ToString(""));
EXPECT_TRUE(errors.empty());

// check that root1 and root2 equal
EXPECT_EQ(root1.Element()->ToString(""), root2.Element()->ToString(""));

const sdf::World *world1 = root1.WorldByIndex(0);
const sdf::World *world2 = root2.WorldByIndex(0);
ASSERT_NE(nullptr, world1);
ASSERT_NE(nullptr, world2);

// //model[@name=M1]
const sdf::Model *model11 = world1->ModelByIndex(0);
const sdf::Model *model12 = world2->ModelByIndex(0);
ASSERT_NE(nullptr, model11);
ASSERT_NE(nullptr, model12);
EXPECT_EQ(model11->Element()->ToString(""), model12->Element()->ToString(""));

// //model[@name=M1]/link[@name=L1]
const sdf::Link *model11link1 = model11->LinkByIndex(0);
const sdf::Link *model12link2 = model12->LinkByIndex(0);
ASSERT_NE(nullptr, model11link1);
ASSERT_NE(nullptr, model12link2);
EXPECT_EQ(model11link1->Element()->ToString(""),
model12link2->Element()->ToString(""));

// //model[@name=M1]/model[@name=M2]
const sdf::Model *model21 = model11->ModelByIndex(0);
const sdf::Model *model22 = model12->ModelByIndex(0);
ASSERT_NE(nullptr, model21);
ASSERT_NE(nullptr, model22);
EXPECT_EQ(model21->Element()->ToString(""), model22->Element()->ToString(""));

// //model[@name=M1]/model[@name=M2]/link[@name=L1]
const sdf::Link *model21link1 = model21->LinkByIndex(0);
const sdf::Link *model22link2 = model22->LinkByIndex(0);
ASSERT_NE(nullptr, model21link1);
ASSERT_NE(nullptr, model22link2);
EXPECT_EQ(model21link1->Element()->ToString(""),
model22link2->Element()->ToString(""));

// //model[@name=M1]/model[@name=M2]/link[@name=L1]/mysim:transmission
sdf::ElementPtr customElem1 =
model21link1->Element()->FindElement("mysim:transmission");
sdf::ElementPtr customElem2 =
model22link2->Element()->FindElement("mysim:transmission");
ASSERT_NE(nullptr, customElem1);
ASSERT_NE(nullptr, customElem2);
EXPECT_EQ(customElem1->ToString(""), customElem2->ToString(""));
}

/////////////////////////////////////////////////
TEST(SDFParser, ReloadNestedIncludedCustomElements)
{
const std::string modelPath = sdf::testing::TestFile("integration", "model");

sdf::setFindCallback(
[&](const std::string &_file)
{
return sdf::filesystem::append(modelPath, _file);
});

const std::string sdfStr =
R"(<sdf version='1.7'>
<world name='default'>
<model name='test'>
<include>
<uri>model_with_custom_elements</uri>
</include>
</model>
</world>
</sdf>
)";

sdf::Root root1;
sdf::Errors errors = root1.LoadSdfString(sdfStr);
EXPECT_TRUE(errors.empty());

for (auto &e : errors)
std::cout << e.Message() << std::endl;

sdf::Root root2;
errors = root2.LoadSdfString(root1.Element()->ToString(""));
EXPECT_TRUE(errors.empty());

// check that root1 and root2 equal
EXPECT_EQ(root1.Element()->ToString(""), root2.Element()->ToString(""));

const sdf::World *world1 = root1.WorldByIndex(0);
const sdf::World *world2 = root2.WorldByIndex(0);
ASSERT_NE(nullptr, world1);
ASSERT_NE(nullptr, world2);

// //model[@name=test]
const sdf::Model *model11 = world1->ModelByIndex(0);
const sdf::Model *model12 = world2->ModelByIndex(0);
ASSERT_NE(nullptr, model11);
ASSERT_NE(nullptr, model12);
EXPECT_EQ(model11->Element()->ToString(""), model12->Element()->ToString(""));

// //model[@name=test]/frame[@name=M1::__model__]
const sdf::Frame *frame1 = model11->FrameByIndex(0);
const sdf::Frame *frame2 = model12->FrameByIndex(0);
ASSERT_NE(nullptr, frame1);
ASSERT_NE(nullptr, frame2);
EXPECT_EQ(frame1->Element()->ToString(""), frame2->Element()->ToString(""));

// //model[@name=test]/link[@name=M1::L1]
const sdf::Link *model11link1 = model11->LinkByIndex(0);
const sdf::Link *model12link2 = model12->LinkByIndex(0);
ASSERT_NE(nullptr, model11link1);
ASSERT_NE(nullptr, model12link2);
EXPECT_EQ(model11link1->Element()->ToString(""),
model12link2->Element()->ToString(""));

// //model[@name=test]/model[@name=M1::M2]
const sdf::Model *model21 = model11->ModelByIndex(0);
const sdf::Model *model22 = model12->ModelByIndex(0);
ASSERT_NE(nullptr, model21);
ASSERT_NE(nullptr, model22);
EXPECT_EQ(model21->Element()->ToString(""), model22->Element()->ToString(""));

// //model[@name=test]/model[@name=M1::M2]/link[@name=M1::L1]
const sdf::Link *model21link1 = model21->LinkByIndex(0);
const sdf::Link *model22link2 = model22->LinkByIndex(0);
ASSERT_NE(nullptr, model21link1);
ASSERT_NE(nullptr, model22link2);
EXPECT_EQ(model21link1->Element()->ToString(""),
model22link2->Element()->ToString(""));

// //model[@name=test]/model[@name=M1::M2]/link[@name=M1::L1]
// /mysim:transmission
sdf::ElementPtr customElem1 =
model21link1->Element()->FindElement("mysim:transmission");
sdf::ElementPtr customElem2 =
model22link2->Element()->FindElement("mysim:transmission");
ASSERT_NE(nullptr, customElem1);
ASSERT_NE(nullptr, customElem2);
EXPECT_EQ(customElem1->ToString(""), customElem2->ToString(""));
}