Skip to content

Commit

Permalink
Update sdfs and parser for warnings
Browse files Browse the repository at this point in the history
Signed-off-by: Stephen Brawner <[email protected]>
  • Loading branch information
brawner committed Dec 15, 2020
1 parent 5fcc295 commit 64f8ccc
Show file tree
Hide file tree
Showing 11 changed files with 166 additions and 86 deletions.
35 changes: 18 additions & 17 deletions src/parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1000,14 +1000,25 @@ bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf, Errors &_errors)
return false;
}

// For now there is only a warning if there is more than one model,
// actor or light element, or two different types of those elements. For
// compatibility with old behavior, this chooses the first element
// in the preference order: model->actor->light
sdf::ElementPtr topLevelElem;
int countTopLevelElements = 0;
for (const auto & elementType : {"model", "actor", "light"})
{
if (includeSDF->Root()->HasElement(elementType))
{
countTopLevelElements++;
topLevelElem = includeSDF->Root()->GetElement(elementType);
if (nullptr == topLevelElem)
{
topLevelElem = includeSDF->Root()->GetElement(elementType);
} else
{
sdfwarn << "Found other top level element <" << elementType
<< "> in addition to <" << topLevelElem->GetName()
<< "> in include file. This is unsupported and in future "
<< "versions of libsdformat will become an error";
}
}
}

Expand All @@ -1019,24 +1030,14 @@ bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf, Errors &_errors)
continue;
}

// Check if more than one type of top-level element
if (countTopLevelElements > 1)
{
_errors.push_back({ErrorCode::ELEMENT_INVALID,
"Found more than one of <model> / <actor> / <light> for "
"<include>\n"});
continue;
}

const auto topLevelElementType = topLevelElem->GetName();

printf("Top level element type: %s\n", topLevelElementType.c_str());
// Check for more than one of the discovered top-level element type
if (nullptr != topLevelElem->GetNextElement(topLevelElementType))
{
_errors.push_back({ErrorCode::ELEMENT_INVALID,
"Found more than one of " + topLevelElem->GetName() + " for "
"<include>\n"});
continue;
sdfwarn << "Found more than one of " << topLevelElem->GetName()
<< " for <include>. This is unsupported and in future "
<< "versions of libsdformat will become an error";
}

bool isModel = topLevelElementType == "model";
Expand Down
2 changes: 2 additions & 0 deletions test/integration/model/model_actor_light/model.sdf
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?xml version="1.0" ?>
<sdf version="1.8">
<!-- This is an example of an invalid file and should only contain one of
model, actor or light-->

<model name="model">
<pose>0 0 0 0 0 0</pose>
Expand Down
7 changes: 5 additions & 2 deletions test/integration/model/multiple_actors/model.sdf
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
<?xml version="1.0" ?>
<sdf version="1.8">
<!-- This file is not valid because it contains multiple actors which is not
allowed -->
<!-- This file is not valid because it contains multiple actor elements which
is not supported -->

<actor name="multiple_actors1">
<pose>0 0 0 0 0 0</pose>
<link name="link1"/>
</actor>

<actor name="multiple_actors2">
<pose>1 0 0 1.57 0 0</pose>
<link name="link2"/>
</actor>

<actor name="multiple_actors3">
<pose>2 0 0 3.14 0 0</pose>
<link name="link3"/>
</actor>
</sdf>
7 changes: 5 additions & 2 deletions test/integration/model/multiple_lights/model.sdf
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
<?xml version="1.0" ?>
<sdf version="1.8">
<!-- This file is not valid because it contains multiple lights which is not
allowed -->
<!-- This file is not valid because it contains multiple light elements which
is not supported -->

<light name="multiple_lights1" type="directional">
<pose>0 0 0 0 0 0</pose>
<direction>1 0 0</direction>
</light>

<light name="multiple_lights2" type="spot">
<pose>1 0 0 1.57 0 0</pose>
<direction>0 1 0</direction>
</light>

<light name="multiple_lights3" type="directional">
<pose>2 0 0 3.14 0 0</pose>
<direction>0 0 1</direction>
</light>

</sdf>
10 changes: 5 additions & 5 deletions test/integration/model/multiple_models/model.sdf
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
<?xml version="1.0" ?>
<sdf version="1.8">
<!-- This file is not valid because it contains multiple models which is not
allowed -->
<!-- This file is not valid because it contains multiple model elements which
is not supported -->

<model name="multiple_models1">
<pose>0 0 0 0 0 0</pose>
<link name="link"/>
<link name="link1"/>
</model>

<model name="multiple_models2">
<pose>1 0 0 1.570796326794895 0 0</pose>
<link name="link"/>
<link name="link2"/>
</model>


<model name="multiple_models3">
<pose>2 0 0 3.141592653589793 0 0</pose>
<link name="link"/>
<link name="link3"/>
</model>

</sdf>
18 changes: 9 additions & 9 deletions test/integration/model/nested_multiple_actors_error/model.sdf
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
<?xml version="1.0" ?>
<sdf version="1.8">
<!-- The included sdf model contains multiple actor elements, and is an
example of poorly formed file. Currently this is unsupported and will
eventually become a parsing error -->

<!-- The included sdf model contains multiple actor elements, and should
cause tests to fail -->
<model name="nested_multiple_actors_error">
<include>
<!-- This search path for this model must be added by the test code -->
<uri>multiple_actors</uri>
<pose>1 1 1 1.570796326794895 0 0</pose>
</include>
</model>
<include>
<!-- This search path for this model must be added by the test code -->
<uri>multiple_actors</uri>
<pose>1 1 1 1.570796326794895 0 0</pose>
<name>nested_actor</name>
</include>

</sdf>
19 changes: 9 additions & 10 deletions test/integration/model/nested_multiple_elements_error/model.sdf
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
<?xml version="1.0" ?>
<sdf version="1.8">
<!-- The included sdf model contain actor, light and actor elements, and is an
example of poorly formed file. Currently this is unsupported and will
eventually become a parsing error -->

<!-- The included sdf model contains model, actor and light elements, and
should cause tests to fail because the name resolution is ambiguous -->
<model name="nested_multiple_elements_error">
<include>
<!-- This search path for this model must be added by the test code -->
<uri>model_actor_light</uri>
<pose>1 1 1 1.570796326794895 0 0</pose>
</include>
</model>

<include>
<!-- This search path for this model must be added by the test code -->
<uri>model_actor_light</uri>
<pose>1 1 1 1.570796326794895 0 0</pose>
<name>nested_model</name>
</include>
</sdf>
19 changes: 9 additions & 10 deletions test/integration/model/nested_multiple_lights_error/model.sdf
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
<?xml version="1.0" ?>
<sdf version="1.8">
<!-- The included sdf model contains multiple light elements, and is an
example of poorly formed file. Currently this is unsupported and will
eventually become a parsing error -->

<!-- The included sdf model contains multiple light elements, and should
cause tests to fail -->
<model name="nested_multiple_lights_error">
<include>
<!-- This search path for this model must be added by the test code -->
<uri>multiple_lights</uri>
<pose>1 1 1 1.570796326794895 0 0</pose>
</include>
</model>

<include>
<!-- This search path for this model must be added by the test code -->
<uri>multiple_lights</uri>
<pose>1 1 1 1.570796326794895 0 0</pose>
<name>nested_light</name>
</include>
</sdf>
17 changes: 9 additions & 8 deletions test/integration/model/nested_multiple_models_error/model.sdf
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
<?xml version="1.0" ?>
<sdf version="1.8">
<!-- The included sdf model contains multiple model elements, and is an
example of poorly formed file. Currently this is unsupported and will
eventually become a parsing error -->

<model name="nested_multiple_models_error">
<include>
<!-- This search path for this model must be added by the test code -->
<uri>multiple_models</uri>
<pose>1 1 1 1.570796326794895 0 0</pose>
</include>
</model>

<include>
<!-- This search path for this model must be added by the test code -->
<uri>multiple_models</uri>
<pose>1 1 1 1.570796326794895 0 0</pose>
<name>nested_model</name>
</include>
</sdf>
Loading

0 comments on commit 64f8ccc

Please sign in to comment.