From 9876a6089c88ac39408e658b6b4b2928abab8377 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20A=2E=20Guti=C3=A9rrez?= Date: Wed, 19 May 2021 04:45:21 +0800 Subject: [PATCH] Add API for determining if an element was set by the user (#542) There's already a way to do this for the Attribute class so adding a similar function for the Element class. To keep consistency with the code from in the Attribute class, we set it to true if the element was set, and false if it was created by default. Signed-off-by: Marco A. Gutierrez Co-authored-by: Addisu Z. Taddese --- include/sdf/Element.hh | 14 +++ src/Element.cc | 22 ++++ src/Element_TEST.cc | 94 ++++++++++++++++ src/parser.cc | 3 +- test/integration/CMakeLists.txt | 1 + test/integration/default_elements.cc | 155 +++++++++++++++++++++++++++ test/sdf/empty_axis.sdf | 19 ++++ test/sdf/empty_road_sph_coords.sdf | 30 ++++++ tools/code_check.sh | 2 +- 9 files changed, 338 insertions(+), 2 deletions(-) create mode 100644 test/integration/default_elements.cc create mode 100644 test/sdf/empty_axis.sdf create mode 100644 test/sdf/empty_road_sph_coords.sdf diff --git a/include/sdf/Element.hh b/include/sdf/Element.hh index 92ef562ed..6064abf3a 100644 --- a/include/sdf/Element.hh +++ b/include/sdf/Element.hh @@ -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); + + /// \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. @@ -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; diff --git a/src/Element.cc b/src/Element.cc index fe181a960..7df8928df 100644 --- a/src/Element.cc +++ b/src/Element.cc @@ -31,6 +31,7 @@ Element::Element() { this->dataPtr->copyChildren = false; this->dataPtr->referenceSDF = ""; + this->dataPtr->explicitlySetInFile = true; } ///////////////////////////////////////////////// @@ -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 { @@ -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(); @@ -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) diff --git a/src/Element_TEST.cc b/src/Element_TEST.cc index f5504f87d..ec7897a11 100644 --- a/src/Element_TEST.cc +++ b/src/Element_TEST.cc @@ -72,6 +72,98 @@ TEST(Element, Required) ASSERT_EQ(elem.GetRequired(), "1"); } +///////////////////////////////////////////////// +TEST(Element, SetExplicitlySetInFile) +{ + // The heirarchy in xml: + // + // + // + // + // + // + // + // + // + // + // + // + + sdf::ElementPtr parent = std::make_shared(); + sdf::ElementPtr elem = std::make_shared(); + elem->SetParent(parent); + parent->InsertElement(elem); + sdf::ElementPtr elem2 = std::make_shared(); + 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(); + child->SetParent(elem); + elem->InsertElement(child); + + sdf::ElementPtr sibling = std::make_shared(); + sibling->SetParent(elem); + elem->InsertElement(sibling); + + sdf::ElementPtr sibling2 = std::make_shared(); + sibling2->SetParent(elem); + elem->InsertElement(sibling2); + + sdf::ElementPtr child2 = std::make_shared(); + child2->SetParent(child); + child->InsertElement(child2); + + sdf::ElementPtr grandChild = std::make_shared(); + 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, SetExplicitlySetInFileWithInsert) +{ + sdf::ElementPtr parent = std::make_shared(); + parent->SetExplicitlySetInFile(false); + sdf::ElementPtr child = std::make_shared(); + child->SetParent(parent); + parent->InsertElement(child); + + EXPECT_FALSE(parent->GetExplicitlySetInFile()); + EXPECT_TRUE(child->GetExplicitlySetInFile()); +} + ///////////////////////////////////////////////// TEST(Element, CopyChildren) { @@ -317,6 +409,7 @@ TEST(Element, Clone) ASSERT_NE(newelem->GetFirstElement(), nullptr); ASSERT_EQ(newelem->GetElementDescriptionCount(), 1UL); ASSERT_EQ(newelem->GetAttributeCount(), 1UL); + ASSERT_TRUE(newelem->GetExplicitlySetInFile()); } ///////////////////////////////////////////////// @@ -594,6 +687,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()); ASSERT_EQ(param->GetKey(), "test"); diff --git a/src/parser.cc b/src/parser.cc index b94d08222..06a3b89f5 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -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); } } } diff --git a/test/integration/CMakeLists.txt b/test/integration/CMakeLists.txt index 1bb4d3719..9dcd39b55 100644 --- a/test/integration/CMakeLists.txt +++ b/test/integration/CMakeLists.txt @@ -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 diff --git a/test/integration/default_elements.cc b/test/integration/default_elements.cc new file mode 100644 index 000000000..e8dcfbac1 --- /dev/null +++ b/test/integration/default_elements.cc @@ -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 +#include +#include + +#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 rootPtr = root.Element(); + EXPECT_TRUE(rootPtr->GetExplicitlySetInFile()); + + sdf::ElementPtr worldPtr = rootPtr->GetFirstElement(); + EXPECT_TRUE(worldPtr->GetExplicitlySetInFile()); + + sdf::ElementPtr roadPtr = worldPtr->GetFirstElement(); + EXPECT_TRUE(roadPtr->GetExplicitlySetInFile()); + + sdf::ElementPtr roadWidthPtr = roadPtr->GetFirstElement(); + EXPECT_FALSE(roadWidthPtr->GetExplicitlySetInFile()); + + sdf::ElementPtr roadPointPtr = roadWidthPtr->GetNextElement(); + EXPECT_FALSE(roadPointPtr->GetExplicitlySetInFile()); + + sdf::ElementPtr sphericalCoordsPtr = roadPtr->GetNextElement(); + EXPECT_TRUE(sphericalCoordsPtr->GetExplicitlySetInFile()); + + sdf::ElementPtr surfaceModel = sphericalCoordsPtr->GetFirstElement(); + EXPECT_FALSE(surfaceModel->GetExplicitlySetInFile()); + + sdf::ElementPtr latitudeDegPtr = surfaceModel->GetNextElement(); + EXPECT_FALSE(latitudeDegPtr->GetExplicitlySetInFile()); + + sdf::ElementPtr longitudeDegPtr = latitudeDegPtr->GetNextElement(); + EXPECT_FALSE(longitudeDegPtr->GetExplicitlySetInFile()); + + sdf::ElementPtr elevationPtr = longitudeDegPtr->GetNextElement(); + EXPECT_FALSE(elevationPtr->GetExplicitlySetInFile()); + + sdf::ElementPtr headingDegPtr = elevationPtr->GetNextElement(); + EXPECT_FALSE(headingDegPtr->GetExplicitlySetInFile()); + + sdf::ElementPtr gravityPtr = sphericalCoordsPtr->GetNextElement(); + EXPECT_FALSE(gravityPtr->GetExplicitlySetInFile()); + + sdf::ElementPtr magneticFieldPtr = gravityPtr->GetNextElement(); + EXPECT_FALSE(magneticFieldPtr->GetExplicitlySetInFile()); + + sdf::ElementPtr atmosphereTypePtr = magneticFieldPtr->GetNextElement(); + EXPECT_FALSE(atmosphereTypePtr->GetExplicitlySetInFile()); + + sdf::ElementPtr physicsPtr = atmosphereTypePtr->GetNextElement(); + EXPECT_FALSE(physicsPtr->GetExplicitlySetInFile()); + + sdf::ElementPtr maxStepSizePtr = physicsPtr->GetFirstElement(); + EXPECT_FALSE(maxStepSizePtr->GetExplicitlySetInFile()); + + sdf::ElementPtr realTimeFactorPtr = maxStepSizePtr->GetNextElement(); + EXPECT_FALSE(realTimeFactorPtr->GetExplicitlySetInFile()); + + sdf::ElementPtr realTimeUpdate = realTimeFactorPtr->GetNextElement(); + EXPECT_FALSE(realTimeUpdate->GetExplicitlySetInFile()); + + sdf::ElementPtr scenePtr = physicsPtr->GetNextElement(); + EXPECT_FALSE(scenePtr->GetExplicitlySetInFile()); + + sdf::ElementPtr ambientPtr = scenePtr->GetFirstElement(); + EXPECT_FALSE(ambientPtr->GetExplicitlySetInFile()); + + sdf::ElementPtr backgroundPtr = ambientPtr->GetNextElement(); + EXPECT_FALSE(backgroundPtr->GetExplicitlySetInFile()); + + sdf::ElementPtr shadowsPtr = backgroundPtr->GetNextElement(); + EXPECT_FALSE(shadowsPtr->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 rootPtr = root.Element(); + EXPECT_TRUE(rootPtr->GetExplicitlySetInFile()); + + sdf::ElementPtr modelPtr = rootPtr->GetFirstElement(); + EXPECT_TRUE(modelPtr->GetExplicitlySetInFile()); + + sdf::ElementPtr link1Ptr = modelPtr->GetFirstElement(); + EXPECT_TRUE(link1Ptr->GetExplicitlySetInFile()); + + sdf::ElementPtr link2Ptr = link1Ptr->GetNextElement(); + EXPECT_TRUE(link2Ptr->GetExplicitlySetInFile()); + + sdf::ElementPtr jointPtr = link2Ptr->GetNextElement(); + EXPECT_TRUE(jointPtr->GetExplicitlySetInFile()); + + sdf::ElementPtr parentPtr = jointPtr->GetFirstElement(); + EXPECT_TRUE(parentPtr->GetExplicitlySetInFile()); + + sdf::ElementPtr childPtr = parentPtr->GetNextElement(); + EXPECT_TRUE(childPtr->GetExplicitlySetInFile()); + + sdf::ElementPtr axisPtr = childPtr->GetNextElement(); + EXPECT_TRUE(axisPtr->GetExplicitlySetInFile()); + + sdf::ElementPtr xyzPtr = axisPtr->GetFirstElement(); + EXPECT_FALSE(xyzPtr->GetExplicitlySetInFile()); + + sdf::ElementPtr limitPtr = xyzPtr->GetNextElement(); + EXPECT_FALSE(limitPtr->GetExplicitlySetInFile()); + + sdf::ElementPtr lowerLimitPtr = limitPtr->GetFirstElement(); + EXPECT_FALSE(lowerLimitPtr->GetExplicitlySetInFile()); + + sdf::ElementPtr upperLimitPtr = lowerLimitPtr->GetNextElement(); + EXPECT_FALSE(upperLimitPtr->GetExplicitlySetInFile()); +} diff --git a/test/sdf/empty_axis.sdf b/test/sdf/empty_axis.sdf new file mode 100644 index 000000000..6e6f42e3d --- /dev/null +++ b/test/sdf/empty_axis.sdf @@ -0,0 +1,19 @@ + + + + + + + + link1 + link2 + + + + + + diff --git a/test/sdf/empty_road_sph_coords.sdf b/test/sdf/empty_road_sph_coords.sdf new file mode 100644 index 000000000..f9d720d1c --- /dev/null +++ b/test/sdf/empty_road_sph_coords.sdf @@ -0,0 +1,30 @@ + + + + + + + + + + + + + diff --git a/tools/code_check.sh b/tools/code_check.sh index 9754e4b81..bd24da512 100755 --- a/tools/code_check.sh +++ b/tools/code_check.sh @@ -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"