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.

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

Allow files paths for include URIs #448

Merged
merged 4 commits into from
Jan 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion sdf/1.8/model.sdf
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@
<include filename="gripper.sdf" required="*"/>

<element name="include" required="*">
<description>Include resources from a URI. This can be used to nest models. Included resources can only contain one 'model', 'light' or 'actor' element.</description>
<description>
Include resources from a URI. This can be used to nest models. Included resources can only contain one 'model', 'light' or 'actor' element. The URI can point to a directory or a file. If the URI is a directory, it must conform to the model database structure (see /tutorials?tut=composition&amp;cat=specification&amp;#defining-models-in-separate-files).
</description>
<element name="uri" type="string" default="__default__" required="1">
<description>URI to a resource, such as a model</description>
</element>
Expand Down
4 changes: 3 additions & 1 deletion sdf/1.8/world.sdf
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
</element>

<element name="include" required="*">
<description>Include resources from a URI. Included resources can only contain one 'model', 'light' or 'actor' element.</description>
<description>
Include resources from a URI. Included resources can only contain one 'model', 'light' or 'actor' element. The URI can point to a directory or a file. If the URI is a directory, it must conform to the model database structure (see /tutorials?tut=composition&amp;cat=specification&amp;#defining-models-in-separate-files).
</description>
<element name="uri" type="string" default="__default__" required="1">
<description>URI to a resource, such as a model</description>
</element>
Expand Down
35 changes: 19 additions & 16 deletions src/parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -978,24 +978,27 @@ bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf, Errors &_errors)
}
else
{
if (!sdf::filesystem::is_directory(modelPath))
if (sdf::filesystem::is_directory(modelPath))
{
_errors.push_back({ErrorCode::DIRECTORY_NONEXISTANT,
"Directory doesn't exist[" + modelPath + "]"});
continue;
}
}

// Get the config.xml filename
filename = getModelFilePath(modelPath);
// Get the model.config filename
filename = getModelFilePath(modelPath);

if (filename.empty())
{
_errors.push_back({ErrorCode::URI_LOOKUP,
"Unable to resolve uri[" + uri + "] to model path [" +
modelPath + "] since it does not contain a model.config " +
"file."});
continue;
if (filename.empty())
{
_errors.push_back({ErrorCode::URI_LOOKUP,
"Unable to resolve uri[" + uri + "] to model path [" +
modelPath + "] since it does not contain a model.config " +
"file."});
continue;
}
}
else
{
// This is a file path and since sdf::findFile returns an empty
// string if the file doesn't exist, we don't have to check for
// existence again here.
filename = modelPath;
}
}
}
else
Expand Down
19 changes: 16 additions & 3 deletions test/integration/includes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,11 @@ TEST(IncludesTest, Includes)

ASSERT_NE(nullptr, root.Element());
EXPECT_EQ(worldFile, root.Element()->FilePath());
EXPECT_EQ("1.6", root.Element()->OriginalVersion());
EXPECT_EQ("1.8", root.Element()->OriginalVersion());

const sdf::World *world = root.WorldByIndex(0);
ASSERT_NE(nullptr, world);
EXPECT_EQ("1.6", world->Element()->OriginalVersion());
EXPECT_EQ("1.8", world->Element()->OriginalVersion());

// Actors
EXPECT_EQ(2u, world->ActorCount());
Expand Down Expand Up @@ -149,7 +149,7 @@ TEST(IncludesTest, Includes)
EXPECT_EQ("", pointLight1->PoseRelativeTo());

// Models
EXPECT_EQ(2u, world->ModelCount());
EXPECT_EQ(3u, world->ModelCount());
EXPECT_FALSE(world->ModelNameExists(""));

// Model without overrides
Expand Down Expand Up @@ -209,6 +209,19 @@ TEST(IncludesTest, Includes)
EXPECT_EQ("", model1->PoseRelativeTo());
ASSERT_NE(nullptr, model1->Element());
EXPECT_TRUE(model1->Element()->HasElement("plugin"));

const sdf::Model *model2 = world->ModelByIndex(2);
ASSERT_NE(nullptr, model2);
EXPECT_EQ("test_model_with_file", model2->Name());
EXPECT_FALSE(model2->Static());
EXPECT_EQ(1u, model2->LinkCount());
ASSERT_NE(nullptr, model2->LinkByIndex(0));
ASSERT_NE(nullptr, model2->LinkByName("link"));
EXPECT_EQ(model2->LinkByName("link")->Name(), model2->LinkByIndex(0)->Name());
EXPECT_EQ(nullptr, model2->LinkByIndex(1));
EXPECT_TRUE(model2->LinkNameExists("link"));
EXPECT_FALSE(model2->LinkNameExists("coconut"));
EXPECT_EQ("1.6", model2->Element()->OriginalVersion());
}

//////////////////////////////////////////////////
Expand Down
7 changes: 6 additions & 1 deletion test/sdf/includes.sdf
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<?xml version="1.0" ?>
<sdf version="1.6">
<sdf version="1.8">
<world name="default">

<include>
Expand All @@ -14,6 +14,11 @@
<plugin name="plugin_name" filename="file.so"/>
</include>

<include>
<uri>test_model/model.sdf</uri>
<name>test_model_with_file</name>
</include>

<include>
<uri>test_light</uri>
</include>
Expand Down