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

adding element default check #542

Merged
merged 16 commits into from
May 18, 2021
Merged
Show file tree
Hide file tree
Changes from 13 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
14 changes: 14 additions & 0 deletions include/sdf/Element.hh
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,17 @@ namespace sdf
/// \sa Element::SetRequired
public: const std::string &GetRequired() const;

/// \brief Set if the element and children where set or default
/// in the original file. This is meant to be used by the parser to
/// indicate whether the element was set by the user in the original
/// file or added by default during parsing.
/// \param[in] _value True if the element was set
public: void SetExplicitlySetInFile(const bool _value);
marcoag marked this conversation as resolved.
Show resolved Hide resolved

/// \brief Return if the element was been explicitly set in the file
/// \return True if the element was set in the file
public: bool GetExplicitlySetInFile() const;

/// \brief Set whether this element should copy its child elements
/// during parsing.
/// \param[in] _value True to copy Element's children.
Expand Down Expand Up @@ -450,6 +461,9 @@ namespace sdf
/// \brief True if element is required
public: std::string required;

/// \brief True if the element was set in the SDF file.
public: bool explicitlySetInFile;

/// \brief Element description
public: std::string description;

Expand Down
22 changes: 22 additions & 0 deletions src/Element.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ Element::Element()
{
this->dataPtr->copyChildren = false;
this->dataPtr->referenceSDF = "";
this->dataPtr->explicitlySetInFile = true;
}

/////////////////////////////////////////////////
Expand Down Expand Up @@ -93,6 +94,25 @@ void Element::SetCopyChildren(bool _value)
this->dataPtr->copyChildren = _value;
}

/////////////////////////////////////////////////
void Element::SetExplicitlySetInFile(const bool _value)
{
this->dataPtr->explicitlySetInFile = _value;

ElementPtr_V::const_iterator eiter;
for (eiter = this->dataPtr->elements.begin();
eiter != this->dataPtr->elements.end(); ++eiter)
{
(*eiter)->SetExplicitlySetInFile(_value);
}
}

/////////////////////////////////////////////////
bool Element::GetExplicitlySetInFile() const
{
return this->dataPtr->explicitlySetInFile;
}

/////////////////////////////////////////////////
bool Element::GetCopyChildren() const
{
Expand Down Expand Up @@ -155,6 +175,7 @@ ElementPtr Element::Clone() const
clone->dataPtr->referenceSDF = this->dataPtr->referenceSDF;
clone->dataPtr->path = this->dataPtr->path;
clone->dataPtr->originalVersion = this->dataPtr->originalVersion;
clone->dataPtr->explicitlySetInFile = this->dataPtr->explicitlySetInFile;

Param_V::const_iterator aiter;
for (aiter = this->dataPtr->attributes.begin();
Expand Down Expand Up @@ -196,6 +217,7 @@ void Element::Copy(const ElementPtr _elem)
this->dataPtr->referenceSDF = _elem->ReferenceSDF();
this->dataPtr->originalVersion = _elem->OriginalVersion();
this->dataPtr->path = _elem->FilePath();
this->dataPtr->explicitlySetInFile = _elem->GetExplicitlySetInFile();

for (Param_V::iterator iter = _elem->dataPtr->attributes.begin();
iter != _elem->dataPtr->attributes.end(); ++iter)
Expand Down
78 changes: 78 additions & 0 deletions src/Element_TEST.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,82 @@ TEST(Element, Required)
ASSERT_EQ(elem.GetRequired(), "1");
}

/////////////////////////////////////////////////
TEST(Element, SetExplicitlySetInFile)
aaronchongth marked this conversation as resolved.
Show resolved Hide resolved
{
// The heirarchy in xml:
// <parent>
// <elem>
// <child>
// <grandChild/>
// </child>
// <sibling/>
// <sibling2/>
// </elem>
// <elem2/>
// </parent>
sdf::ElementPtr parent = std::make_shared<sdf::Element>();
sdf::ElementPtr elem = std::make_shared<sdf::Element>();
elem->SetParent(parent);
parent->InsertElement(elem);
sdf::ElementPtr elem2 = std::make_shared<sdf::Element>();
elem2->SetParent(parent);
parent->InsertElement(elem2);

EXPECT_TRUE(elem->GetExplicitlySetInFile());

elem->SetExplicitlySetInFile(false);

EXPECT_FALSE(elem->GetExplicitlySetInFile());

elem->SetExplicitlySetInFile(true);

EXPECT_TRUE(elem->GetExplicitlySetInFile());

// the childs and siblings of the element should all be
// set to the same value when using this function
sdf::ElementPtr child = std::make_shared<sdf::Element>();
child->SetParent(elem);
elem->InsertElement(child);

sdf::ElementPtr sibling = std::make_shared<sdf::Element>();
sibling->SetParent(elem);
elem->InsertElement(sibling);

sdf::ElementPtr sibling2 = std::make_shared<sdf::Element>();
sibling2->SetParent(elem);
elem->InsertElement(sibling2);

sdf::ElementPtr child2 = std::make_shared<sdf::Element>();
child2->SetParent(child);
child->InsertElement(child2);
aaronchongth marked this conversation as resolved.
Show resolved Hide resolved

sdf::ElementPtr grandChild = std::make_shared<sdf::Element>();
grandChild->SetParent(child);
child->InsertElement(grandChild);

EXPECT_TRUE(elem->GetExplicitlySetInFile());
EXPECT_TRUE(child->GetExplicitlySetInFile());
EXPECT_TRUE(sibling->GetExplicitlySetInFile());
EXPECT_TRUE(sibling2->GetExplicitlySetInFile());
EXPECT_TRUE(child2->GetExplicitlySetInFile());
EXPECT_TRUE(grandChild->GetExplicitlySetInFile());
EXPECT_TRUE(elem2->GetExplicitlySetInFile());

elem->SetExplicitlySetInFile(false);
EXPECT_FALSE(elem->GetExplicitlySetInFile());
EXPECT_FALSE(child->GetExplicitlySetInFile());
EXPECT_FALSE(sibling->GetExplicitlySetInFile());
EXPECT_FALSE(sibling2->GetExplicitlySetInFile());
EXPECT_FALSE(child2->GetExplicitlySetInFile());
EXPECT_FALSE(grandChild->GetExplicitlySetInFile());

// SetExplicitlySetInFile(false) is be called only on `elem`. We expect
// GetExplicitlySetInFile() to be false for all children and grandchildren of
// `elem`, but true for `elem2`, which is a sibling of `elem`.
EXPECT_TRUE(elem2->GetExplicitlySetInFile());
}

/////////////////////////////////////////////////
TEST(Element, CopyChildren)
{
Expand Down Expand Up @@ -317,6 +393,7 @@ TEST(Element, Clone)
ASSERT_NE(newelem->GetFirstElement(), nullptr);
ASSERT_EQ(newelem->GetElementDescriptionCount(), 1UL);
ASSERT_EQ(newelem->GetAttributeCount(), 1UL);
ASSERT_TRUE(newelem->GetExplicitlySetInFile());
}

/////////////////////////////////////////////////
Expand Down Expand Up @@ -594,6 +671,7 @@ TEST(Element, Copy)
ASSERT_EQ(param->GetDescription(), "val description");

ASSERT_EQ(dest->GetAttributeCount(), 1UL);
ASSERT_TRUE(dest->GetExplicitlySetInFile());
param = dest->GetAttribute("test");
ASSERT_TRUE(param->IsType<std::string>());
ASSERT_EQ(param->GetKey(), "test");
Expand Down
3 changes: 2 additions & 1 deletion src/parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1131,7 +1131,8 @@ bool readXml(TiXmlElement *_xml, ElementPtr _sdf, Errors &_errors)
else
{
// Add default element
_sdf->AddElement(elemDesc->GetName());
ElementPtr defaultElement = _sdf->AddElement(elemDesc->GetName());
defaultElement->SetExplicitlySetInFile(false);
}
}
}
Expand Down
1 change: 1 addition & 0 deletions test/integration/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ set(tests
cfm_damping_implicit_spring_damper.cc
collision_dom.cc
converter.cc
default_elements.cc
deprecated_specs.cc
disable_fixed_joint_reduction.cc
fixed_joint_reduction.cc
Expand Down
155 changes: 155 additions & 0 deletions test/integration/default_elements.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
/*
* Copyright 2017 Open Source Robotics Foundation
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/

#include <iostream>
#include <string>
#include <gtest/gtest.h>

#include "sdf/SDFImpl.hh"
#include "sdf/parser.hh"
#include "sdf/Frame.hh"
#include "sdf/Model.hh"
#include "sdf/Root.hh"
#include "sdf/World.hh"
#include "sdf/Filesystem.hh"
#include "test_config.h"

//////////////////////////////////////////////////
TEST(ExplicitlySetInFile, EmptyRoadSphCoords)
{
const std::string test_file =
sdf::filesystem::append(PROJECT_SOURCE_PATH, "test", "sdf",
"empty_road_sph_coords.sdf");

sdf::Root root;
sdf::Errors errors = root.Load(test_file);
EXPECT_TRUE(errors.empty());

sdf::ElementPtr elementPtr = root.Element();
aaronchongth marked this conversation as resolved.
Show resolved Hide resolved
EXPECT_TRUE(root.Element()->GetExplicitlySetInFile());

elementPtr = elementPtr->GetFirstElement();
aaronchongth marked this conversation as resolved.
Show resolved Hide resolved
EXPECT_TRUE(elementPtr->GetExplicitlySetInFile());

elementPtr = elementPtr->GetFirstElement();
EXPECT_TRUE(elementPtr->GetExplicitlySetInFile());

sdf::ElementPtr road_ptr = elementPtr->GetFirstElement();
aaronchongth marked this conversation as resolved.
Show resolved Hide resolved
EXPECT_FALSE(road_ptr->GetExplicitlySetInFile());

road_ptr = road_ptr->GetNextElement();
EXPECT_FALSE(road_ptr->GetExplicitlySetInFile());

elementPtr = elementPtr->GetNextElement();
EXPECT_TRUE(elementPtr->GetExplicitlySetInFile());

sdf::ElementPtr spherical_coords_ptr = elementPtr->GetFirstElement();
EXPECT_FALSE(spherical_coords_ptr->GetExplicitlySetInFile());

spherical_coords_ptr = elementPtr->GetNextElement();
EXPECT_FALSE(spherical_coords_ptr->GetExplicitlySetInFile());

spherical_coords_ptr = elementPtr->GetNextElement();
EXPECT_FALSE(spherical_coords_ptr->GetExplicitlySetInFile());

spherical_coords_ptr = elementPtr->GetNextElement();
EXPECT_FALSE(spherical_coords_ptr->GetExplicitlySetInFile());

spherical_coords_ptr = elementPtr->GetNextElement();
EXPECT_FALSE(spherical_coords_ptr->GetExplicitlySetInFile());

elementPtr = elementPtr->GetNextElement();
EXPECT_FALSE(elementPtr->GetExplicitlySetInFile());

elementPtr = elementPtr->GetNextElement();
EXPECT_FALSE(elementPtr->GetExplicitlySetInFile());

elementPtr = elementPtr->GetNextElement();
EXPECT_FALSE(elementPtr->GetExplicitlySetInFile());

elementPtr = elementPtr->GetNextElement();
EXPECT_FALSE(elementPtr->GetExplicitlySetInFile());

sdf::ElementPtr physics_ptr = elementPtr->GetFirstElement();
EXPECT_FALSE(physics_ptr->GetExplicitlySetInFile());

physics_ptr = physics_ptr->GetNextElement();
EXPECT_FALSE(physics_ptr->GetExplicitlySetInFile());

physics_ptr = physics_ptr->GetNextElement();
EXPECT_FALSE(physics_ptr->GetExplicitlySetInFile());

elementPtr = elementPtr->GetNextElement();
EXPECT_FALSE(elementPtr->GetExplicitlySetInFile());

sdf::ElementPtr scene_ptr = elementPtr->GetFirstElement();
EXPECT_FALSE(scene_ptr->GetExplicitlySetInFile());

scene_ptr = scene_ptr->GetNextElement();
EXPECT_FALSE(scene_ptr->GetExplicitlySetInFile());

scene_ptr = scene_ptr->GetNextElement();
EXPECT_FALSE(scene_ptr->GetExplicitlySetInFile());
}

//////////////////////////////////////////////////
TEST(ExplicitlySetInFile, EmptyAxis)
{
const std::string test_file =
sdf::filesystem::append(PROJECT_SOURCE_PATH, "test", "sdf",
"empty_axis.sdf");

sdf::Root root;
sdf::Errors errors = root.Load(test_file);
EXPECT_TRUE(errors.empty());

sdf::ElementPtr elementPtr = root.Element();
EXPECT_TRUE(elementPtr->GetExplicitlySetInFile());

elementPtr = elementPtr->GetFirstElement();
EXPECT_TRUE(elementPtr->GetExplicitlySetInFile());

elementPtr = elementPtr->GetFirstElement();
EXPECT_TRUE(elementPtr->GetExplicitlySetInFile());

elementPtr = elementPtr->GetNextElement();
EXPECT_TRUE(elementPtr->GetExplicitlySetInFile());

elementPtr = elementPtr->GetNextElement();
EXPECT_TRUE(elementPtr->GetExplicitlySetInFile());

elementPtr = elementPtr->GetFirstElement();
EXPECT_TRUE(elementPtr->GetExplicitlySetInFile());

elementPtr = elementPtr->GetNextElement();
EXPECT_TRUE(elementPtr->GetExplicitlySetInFile());

elementPtr = elementPtr->GetNextElement();
EXPECT_TRUE(elementPtr->GetExplicitlySetInFile());

elementPtr = elementPtr->GetFirstElement();
EXPECT_FALSE(elementPtr->GetExplicitlySetInFile());

elementPtr = elementPtr->GetNextElement();
EXPECT_FALSE(elementPtr->GetExplicitlySetInFile());

elementPtr = elementPtr->GetFirstElement();
EXPECT_FALSE(elementPtr->GetExplicitlySetInFile());

elementPtr = elementPtr->GetNextElement();
EXPECT_FALSE(elementPtr->GetExplicitlySetInFile());
}
13 changes: 13 additions & 0 deletions test/sdf/empty_axis.sdf
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?xml version="1.0" ?>
<sdf version="1.6">
<model name="empty_axis">
<link name="link1" />
<link name="link2" />
<joint name="joint" type="fixed">
<parent>link1</parent>
<child>link2</child>
<axis>
</axis>
</joint>
</model>
</sdf>
7 changes: 7 additions & 0 deletions test/sdf/empty_road_sph_coords.sdf
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?xml version="1.0" ?>
<sdf version="1.7">
<world name="default">
<road name="empty_road"/>
<spherical_coordinates/>
aaronchongth marked this conversation as resolved.
Show resolved Hide resolved
</world>
</sdf>
2 changes: 1 addition & 1 deletion tools/code_check.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ CHECK_FILE_DIRS="./src ./include ./examples ./test/performance ./test/integratio
CPPCHECK_BASE="cppcheck -q --suppressions-list=$SUPPRESS --inline-suppr"
CPPCHECK_BASE2="cppcheck -q --suppressions-list=$SUPPRESS2 --inline-suppr"
CPPCHECK_FILES=`find $CHECK_FILE_DIRS -name "*.cc"`
CPPCHECK_INCLUDES="-I include -I . -I $builddir -I $builddir/include"
CPPCHECK_INCLUDES="-I include -I test -I . -I $builddir -I $builddir/include"
CPPCHECK_COMMAND1="-j 4 --enable=style,performance,portability,information $CPPCHECK_FILES"
# Unused function checking must happen in one job
CPPCHECK_COMMAND2="--enable=unusedFunction $CPPCHECK_FILES"
Expand Down