From 34d95879c2e620ed10df69515e4ad211746f6031 Mon Sep 17 00:00:00 2001 From: Steve Peters Date: Wed, 29 Sep 2021 03:43:46 -0700 Subject: [PATCH 1/7] Add empty //inertial/pose/@relative_to in test sdf Signed-off-by: Steve Peters --- test/sdf/model_frame_relative_to_joint.sdf | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/sdf/model_frame_relative_to_joint.sdf b/test/sdf/model_frame_relative_to_joint.sdf index 8c52e382f..a88a05abc 100644 --- a/test/sdf/model_frame_relative_to_joint.sdf +++ b/test/sdf/model_frame_relative_to_joint.sdf @@ -3,6 +3,9 @@ 1 0 0 0 0 0 + + 0 0 0 0 0 0 + 0 0 10 0 0 0 From ca77b90e0ee78003a8d9640b3ec05da54c2cdf90 Mon Sep 17 00:00:00 2001 From: Steve Peters Date: Wed, 29 Sep 2021 04:14:19 -0700 Subject: [PATCH 2/7] Add remove_empty functionality to Converter The tag in a .convert file will remove any empty attributes or elemements with no child elements or text values that match the specified name. Signed-off-by: Steve Peters --- src/Converter.cc | 23 ++++-- src/Converter.hh | 6 +- src/Converter_TEST.cc | 171 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 191 insertions(+), 9 deletions(-) diff --git a/src/Converter.cc b/src/Converter.cc index 818501a50..8b6e74aed 100644 --- a/src/Converter.cc +++ b/src/Converter.cc @@ -259,6 +259,10 @@ void Converter::ConvertImpl(tinyxml2::XMLElement *_elem, { Remove(_elem, childElem); } + else if (name == "remove_empty") + { + Remove(_elem, childElem, true); + } else if (name == "unflatten") { Unflatten(_elem); @@ -640,10 +644,11 @@ void Converter::Add(tinyxml2::XMLElement *_elem, tinyxml2::XMLElement *_addElem) ///////////////////////////////////////////////// void Converter::Remove(tinyxml2::XMLElement *_elem, - tinyxml2::XMLElement *_removeElem) + tinyxml2::XMLElement *_removeElem, + bool _removeEmpty) { SDF_ASSERT(_elem != NULL, "SDF element is NULL"); - SDF_ASSERT(_removeElem != NULL, "Move element is NULL"); + SDF_ASSERT(_removeElem != NULL, "remove element is NULL"); const char *attributeName = _removeElem->Attribute("attribute"); const char *elementName = _removeElem->Attribute("element"); @@ -657,7 +662,11 @@ void Converter::Remove(tinyxml2::XMLElement *_elem, if (attributeName) { - _elem->DeleteAttribute(attributeName); + const char * attributeValue = _elem->Attribute(attributeName); + if (!_removeEmpty || (attributeValue && attributeValue[0] == '\0')) + { + _elem->DeleteAttribute(attributeName); + } } else { @@ -665,8 +674,12 @@ void Converter::Remove(tinyxml2::XMLElement *_elem, while (childElem) { - _elem->DeleteChild(childElem); - childElem = _elem->FirstChildElement(elementName); + auto nextSibling = childElem->NextSiblingElement(elementName); + if (!_removeEmpty || (childElem->NoChildren() && !childElem->GetText())) + { + _elem->DeleteChild(childElem); + } + childElem = nextSibling; } } } diff --git a/src/Converter.hh b/src/Converter.hh index ce24c65dd..a31ae6c26 100644 --- a/src/Converter.hh +++ b/src/Converter.hh @@ -100,9 +100,11 @@ namespace sdf /// \brief Remove an element. /// \param[in] _elem The element that has the _removeElem child. - /// \param[in] _removeElem The element to remove. + /// \param[in] _removeElem The metadata about what to remove. + /// \param[in] _removeEmpty If true, only remove empty nodes. private: static void Remove(tinyxml2::XMLElement *_elem, - tinyxml2::XMLElement *_removeElem); + tinyxml2::XMLElement *_removeElem, + bool _removeEmpty = false); /// \brief Unflatten an element (conversion from SDFormat <= 1.7 to 1.8) /// \param[in] _elem The element to unflatten diff --git a/src/Converter_TEST.cc b/src/Converter_TEST.cc index 96b32b2d2..e8eb65635 100644 --- a/src/Converter_TEST.cc +++ b/src/Converter_TEST.cc @@ -48,7 +48,8 @@ std::string getRepeatedXmlString() std::stringstream stream; stream << "" << " " - << " " + << " " + << " " << " D" << " D" << " D" @@ -618,6 +619,118 @@ TEST(Converter, RemoveDescendantElement) ASSERT_TRUE(convertedElem == nullptr); } +//////////////////////////////////////////////////// +/// Ensure that Converter::Remove function is working +/// Test removing empty elements only +TEST(Converter, RemoveEmptyElement) +{ + // Set up an xml string for testing + std::string xmlString = getRepeatedXmlString(); + + // Verify the xml + tinyxml2::XMLDocument xmlDoc; + xmlDoc.Parse(xmlString.c_str()); + tinyxml2::XMLElement *childElem = xmlDoc.FirstChildElement(); + ASSERT_NE(nullptr, childElem); + EXPECT_STREQ(childElem->Name(), "elemA"); + childElem = childElem->FirstChildElement(); + ASSERT_NE(nullptr, childElem); + EXPECT_STREQ(childElem->Name(), "elemB"); + childElem = childElem->FirstChildElement(); + ASSERT_NE(nullptr, childElem); + EXPECT_STREQ(childElem->Name(), "elemC"); + childElem = childElem->FirstChildElement(); + ASSERT_NE(nullptr, childElem); + EXPECT_STREQ(childElem->Name(), "elemD"); + + // Test removing empty elements + // Set up a convert file + std::stringstream convertStream; + convertStream << "" + << " " + << " " + << " " + << " " + << " " + << ""; + tinyxml2::XMLDocument convertXmlDoc; + convertXmlDoc.Parse(convertStream.str().c_str()); + sdf::Converter::Convert(&xmlDoc, &convertXmlDoc); + + tinyxml2::XMLElement *convertedElem = xmlDoc.FirstChildElement(); + EXPECT_STREQ(convertedElem->Name(), "elemA"); + convertedElem = convertedElem->FirstChildElement(); + ASSERT_NE(nullptr, convertedElem); + EXPECT_STREQ(convertedElem->Name(), "elemB"); + EXPECT_NE(nullptr, convertedElem->FirstChildElement("elemC")); + convertedElem = convertedElem->FirstChildElement("elemC"); + ASSERT_NE(nullptr, convertedElem); + auto elemD = convertedElem->FirstChildElement("elemD"); + ASSERT_NE(elemD, nullptr); + while (elemD) + { + std::string elemValue = elemD->GetText(); + EXPECT_EQ(elemValue, "D"); + elemD = elemD->NextSiblingElement("elemD"); + } +} + +//////////////////////////////////////////////////// +/// Ensure that Converter::Remove function is working with descendant_name +/// Test removing empty elements only +TEST(Converter, RemoveEmptyDescendantElement) +{ + // Set up an xml string for testing + std::string xmlString = getRepeatedXmlString(); + + // Verify the xml + tinyxml2::XMLDocument xmlDoc; + xmlDoc.Parse(xmlString.c_str()); + tinyxml2::XMLElement *childElem = xmlDoc.FirstChildElement(); + ASSERT_NE(nullptr, childElem); + EXPECT_STREQ(childElem->Name(), "elemA"); + childElem = childElem->FirstChildElement(); + ASSERT_NE(nullptr, childElem); + EXPECT_STREQ(childElem->Name(), "elemB"); + childElem = childElem->FirstChildElement(); + ASSERT_NE(nullptr, childElem); + EXPECT_STREQ(childElem->Name(), "elemC"); + childElem = childElem->FirstChildElement(); + ASSERT_NE(nullptr, childElem); + EXPECT_STREQ(childElem->Name(), "elemD"); + + // Test removing empty elements + // Set up a convert file + std::stringstream convertStream; + convertStream << "" + << " " + << " " + << " " + << ""; + tinyxml2::XMLDocument convertXmlDoc; + convertXmlDoc.Parse(convertStream.str().c_str()); + sdf::Converter::Convert(&xmlDoc, &convertXmlDoc); + + tinyxml2::XMLElement *convertedElem = xmlDoc.FirstChildElement(); + + EXPECT_STREQ(convertedElem->Name(), "elemA"); + convertedElem = convertedElem->FirstChildElement(); + ASSERT_NE(nullptr, convertedElem); + EXPECT_STREQ(convertedElem->Name(), "elemB"); + EXPECT_NE(nullptr, convertedElem->FirstChildElement("elemC")); + convertedElem = convertedElem->FirstChildElement("elemC"); + ASSERT_NE(nullptr, convertedElem); + auto elemD = convertedElem->FirstChildElement("elemD"); + ASSERT_NE(elemD, nullptr); + while (elemD) + { + std::string elemValue = elemD->GetText(); + EXPECT_EQ(elemValue, "D"); + elemD = elemD->NextSiblingElement("elemD"); + } +} + +//////////////////////////////////////////////////// TEST(Converter, RemoveDescendantNestedElement) { // Set up an xml string for testing @@ -673,6 +786,7 @@ TEST(Converter, RemoveDescendantNestedElement) EXPECT_STREQ(xmlDocOut.CStr(), expectedXmlDocOut.CStr()); } + //////////////////////////////////////////////////// /// Ensure that Converter ignores descendants of or namespaced elements TEST(Converter, DescendantIgnorePluginOrNamespacedElements) @@ -805,7 +919,7 @@ TEST(Converter, RemoveAttr) ASSERT_NE(nullptr, childElem); EXPECT_STREQ(childElem->Name(), "elemD"); - // Test adding element + // Test removing attrC attributes // Set up a convert file std::stringstream convertStream; convertStream << "" @@ -832,6 +946,59 @@ TEST(Converter, RemoveAttr) ASSERT_NE(nullptr, convertedElem); } +//////////////////////////////////////////////////// +/// Ensure that Converter::Remove function is working +/// Test removing empty attributes only +TEST(Converter, RemoveEmptyAttr) +{ + // Set up an xml string for testing + std::string xmlString = getRepeatedXmlString(); + + // Verify the xml + tinyxml2::XMLDocument xmlDoc; + xmlDoc.Parse(xmlString.c_str()); + tinyxml2::XMLElement *childElem = xmlDoc.FirstChildElement(); + ASSERT_NE(nullptr, childElem); + EXPECT_STREQ(childElem->Name(), "elemA"); + childElem = childElem->FirstChildElement(); + ASSERT_NE(nullptr, childElem); + EXPECT_STREQ(childElem->Name(), "elemB"); + childElem = childElem->FirstChildElement(); + ASSERT_NE(nullptr, childElem); + EXPECT_STREQ(childElem->Name(), "elemC"); + childElem = childElem->FirstChildElement(); + ASSERT_NE(nullptr, childElem); + EXPECT_STREQ(childElem->Name(), "elemD"); + + // Test adding element + // Set up a convert file + std::stringstream convertStream; + convertStream << "" + << " " + << " " + << " " + << " " + << " " + << " " + << ""; + tinyxml2::XMLDocument convertXmlDoc; + convertXmlDoc.Parse(convertStream.str().c_str()); + sdf::Converter::Convert(&xmlDoc, &convertXmlDoc); + + tinyxml2::XMLElement *convertedElem = xmlDoc.FirstChildElement(); + EXPECT_STREQ(convertedElem->Name(), "elemA"); + convertedElem = convertedElem->FirstChildElement(); + ASSERT_NE(nullptr, convertedElem); + EXPECT_STREQ(convertedElem->Name(), "elemB"); + EXPECT_NE(nullptr, convertedElem->FirstChildElement("elemC")); + convertedElem = convertedElem->FirstChildElement("elemC"); + ASSERT_NE(nullptr, convertedElem); + EXPECT_NE(nullptr, convertedElem->Attribute("attrC")); + EXPECT_EQ(nullptr, convertedElem->Attribute("attrEmpty")); + convertedElem = convertedElem->FirstChildElement("elemD"); + ASSERT_NE(nullptr, convertedElem); +} + //////////////////////////////////////////////////// TEST(Converter, RemoveNoElement) { From 4dd9a258e12ed82ec38df4dd61f04a984d769665 Mon Sep 17 00:00:00 2001 From: Steve Peters Date: Wed, 29 Sep 2021 04:15:49 -0700 Subject: [PATCH 3/7] 1_7->1.8: remove empty inertial pose relative_to This attribute was removed from the 1.8 spec, but causes warnings during forward conversion. This removes empty attributes to silence trivial warnings. Signed-off-by: Steve Peters --- sdf/1.8/1_7.convert | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sdf/1.8/1_7.convert b/sdf/1.8/1_7.convert index 86b85c3dd..c69de4fca 100644 --- a/sdf/1.8/1_7.convert +++ b/sdf/1.8/1_7.convert @@ -6,4 +6,10 @@ + + + + + + From 4df59689dd05d4e8ff20029879777cfc143efacf Mon Sep 17 00:00:00 2001 From: Steve Peters Date: Wed, 29 Sep 2021 11:38:06 -0700 Subject: [PATCH 4/7] Converter_TEST: add assertions Signed-off-by: Steve Peters --- src/Converter_TEST.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Converter_TEST.cc b/src/Converter_TEST.cc index e8eb65635..c855123a7 100644 --- a/src/Converter_TEST.cc +++ b/src/Converter_TEST.cc @@ -559,6 +559,7 @@ TEST(Converter, RemoveElement) sdf::Converter::Convert(&xmlDoc, &convertXmlDoc); tinyxml2::XMLElement *convertedElem = xmlDoc.FirstChildElement(); + ASSERT_NE(nullptr, convertedElem); EXPECT_STREQ(convertedElem->Name(), "elemA"); convertedElem = convertedElem->FirstChildElement(); ASSERT_NE(nullptr, convertedElem); @@ -607,6 +608,7 @@ TEST(Converter, RemoveDescendantElement) sdf::Converter::Convert(&xmlDoc, &convertXmlDoc); tinyxml2::XMLElement *convertedElem = xmlDoc.FirstChildElement(); + ASSERT_NE(nullptr, convertedElem); EXPECT_STREQ(convertedElem->Name(), "elemA"); convertedElem = convertedElem->FirstChildElement(); @@ -658,6 +660,7 @@ TEST(Converter, RemoveEmptyElement) sdf::Converter::Convert(&xmlDoc, &convertXmlDoc); tinyxml2::XMLElement *convertedElem = xmlDoc.FirstChildElement(); + ASSERT_NE(nullptr, convertedElem); EXPECT_STREQ(convertedElem->Name(), "elemA"); convertedElem = convertedElem->FirstChildElement(); ASSERT_NE(nullptr, convertedElem); @@ -712,6 +715,7 @@ TEST(Converter, RemoveEmptyDescendantElement) sdf::Converter::Convert(&xmlDoc, &convertXmlDoc); tinyxml2::XMLElement *convertedElem = xmlDoc.FirstChildElement(); + ASSERT_NE(nullptr, convertedElem); EXPECT_STREQ(convertedElem->Name(), "elemA"); convertedElem = convertedElem->FirstChildElement(); From be159db6bfc592edecc05cfd64bcc16701716eb2 Mon Sep 17 00:00:00 2001 From: Steve Peters Date: Wed, 29 Sep 2021 11:47:54 -0700 Subject: [PATCH 5/7] Converter: use strlen Signed-off-by: Steve Peters --- src/Converter.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Converter.cc b/src/Converter.cc index 8b6e74aed..07635bdd2 100644 --- a/src/Converter.cc +++ b/src/Converter.cc @@ -663,7 +663,7 @@ void Converter::Remove(tinyxml2::XMLElement *_elem, if (attributeName) { const char * attributeValue = _elem->Attribute(attributeName); - if (!_removeEmpty || (attributeValue && attributeValue[0] == '\0')) + if (!_removeEmpty || (attributeValue && strlen(attributeValue) == 0)) { _elem->DeleteAttribute(attributeName); } @@ -707,12 +707,12 @@ void Converter::Map(tinyxml2::XMLElement *_elem, tinyxml2::XMLElement *_mapElem) const char *fromNameStr = fromConvertElem->Attribute("name"); const char *toNameStr = toConvertElem->Attribute("name"); - if (!fromNameStr || fromNameStr[0] == '\0') + if (!fromNameStr || strlen(fromNameStr) == 0) { sdferr << "Map: element requires a non-empty name attribute.\n"; return; } - if (!toNameStr || toNameStr[0] == '\0') + if (!toNameStr || strlen(toNameStr) == 0) { sdferr << "Map: element requires a non-empty name attribute.\n"; return; From e6eb29391e12f320619d2ef9d408b847603b2c47 Mon Sep 17 00:00:00 2001 From: Steve Peters Date: Wed, 29 Sep 2021 11:48:13 -0700 Subject: [PATCH 6/7] Improve docs, rename argument to _removeOnlyEmpty Signed-off-by: Steve Peters --- src/Converter.cc | 7 ++++--- src/Converter.hh | 10 ++++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/Converter.cc b/src/Converter.cc index 07635bdd2..1438e25c5 100644 --- a/src/Converter.cc +++ b/src/Converter.cc @@ -645,7 +645,7 @@ void Converter::Add(tinyxml2::XMLElement *_elem, tinyxml2::XMLElement *_addElem) ///////////////////////////////////////////////// void Converter::Remove(tinyxml2::XMLElement *_elem, tinyxml2::XMLElement *_removeElem, - bool _removeEmpty) + bool _removeOnlyEmpty) { SDF_ASSERT(_elem != NULL, "SDF element is NULL"); SDF_ASSERT(_removeElem != NULL, "remove element is NULL"); @@ -663,7 +663,7 @@ void Converter::Remove(tinyxml2::XMLElement *_elem, if (attributeName) { const char * attributeValue = _elem->Attribute(attributeName); - if (!_removeEmpty || (attributeValue && strlen(attributeValue) == 0)) + if (!_removeOnlyEmpty || (attributeValue && strlen(attributeValue) == 0)) { _elem->DeleteAttribute(attributeName); } @@ -675,7 +675,8 @@ void Converter::Remove(tinyxml2::XMLElement *_elem, while (childElem) { auto nextSibling = childElem->NextSiblingElement(elementName); - if (!_removeEmpty || (childElem->NoChildren() && !childElem->GetText())) + if (!_removeOnlyEmpty || + (childElem->NoChildren() && !childElem->GetText())) { _elem->DeleteChild(childElem); } diff --git a/src/Converter.hh b/src/Converter.hh index a31ae6c26..c8bc75e6a 100644 --- a/src/Converter.hh +++ b/src/Converter.hh @@ -98,13 +98,15 @@ namespace sdf private: static void Add(tinyxml2::XMLElement *_elem, tinyxml2::XMLElement *_addElem); - /// \brief Remove an element. - /// \param[in] _elem The element that has the _removeElem child. + /// \brief Remove an attribute or elements. + /// \param[in] _elem The element from which data may be removed. /// \param[in] _removeElem The metadata about what to remove. - /// \param[in] _removeEmpty If true, only remove empty nodes. + /// \param[in] _removeOnlyEmpty If true, only remove an attribute + /// containing an empty string or elements that contain no value nor + /// child elements (though possibly attributes). private: static void Remove(tinyxml2::XMLElement *_elem, tinyxml2::XMLElement *_removeElem, - bool _removeEmpty = false); + bool _removeOnlyEmpty = false); /// \brief Unflatten an element (conversion from SDFormat <= 1.7 to 1.8) /// \param[in] _elem The element to unflatten From 9588e0f82f78fa2ddb57e2c8a54339ca1625b55c Mon Sep 17 00:00:00 2001 From: Steve Peters Date: Wed, 29 Sep 2021 14:20:09 -0700 Subject: [PATCH 7/7] Remove empty elements if they have no attributes Signed-off-by: Steve Peters --- src/Converter.cc | 4 ++-- src/Converter.hh | 4 ++-- src/Converter_TEST.cc | 24 ++++++++++++++++++++++-- 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/Converter.cc b/src/Converter.cc index 1438e25c5..542c14813 100644 --- a/src/Converter.cc +++ b/src/Converter.cc @@ -675,8 +675,8 @@ void Converter::Remove(tinyxml2::XMLElement *_elem, while (childElem) { auto nextSibling = childElem->NextSiblingElement(elementName); - if (!_removeOnlyEmpty || - (childElem->NoChildren() && !childElem->GetText())) + if (!_removeOnlyEmpty || (!childElem->FirstAttribute() && + childElem->NoChildren() && !childElem->GetText())) { _elem->DeleteChild(childElem); } diff --git a/src/Converter.hh b/src/Converter.hh index c8bc75e6a..5c3badb17 100644 --- a/src/Converter.hh +++ b/src/Converter.hh @@ -102,8 +102,8 @@ namespace sdf /// \param[in] _elem The element from which data may be removed. /// \param[in] _removeElem The metadata about what to remove. /// \param[in] _removeOnlyEmpty If true, only remove an attribute - /// containing an empty string or elements that contain no value nor - /// child elements (though possibly attributes). + /// containing an empty string or elements that contain neither value nor + /// child elements nor attributes. private: static void Remove(tinyxml2::XMLElement *_elem, tinyxml2::XMLElement *_removeElem, bool _removeOnlyEmpty = false); diff --git a/src/Converter_TEST.cc b/src/Converter_TEST.cc index c855123a7..7f34f86d6 100644 --- a/src/Converter_TEST.cc +++ b/src/Converter_TEST.cc @@ -50,6 +50,7 @@ std::string getRepeatedXmlString() << " " << " " << " " + << " " << " D" << " D" << " D" @@ -668,8 +669,17 @@ TEST(Converter, RemoveEmptyElement) EXPECT_NE(nullptr, convertedElem->FirstChildElement("elemC")); convertedElem = convertedElem->FirstChildElement("elemC"); ASSERT_NE(nullptr, convertedElem); + auto elemD = convertedElem->FirstChildElement("elemD"); - ASSERT_NE(elemD, nullptr); + ASSERT_NE(nullptr, elemD); + + // first should have an attribute but no value + EXPECT_EQ(nullptr, elemD->GetText()); + ASSERT_NE(nullptr, elemD->Attribute("attrD")); + EXPECT_EQ("D", std::string(elemD->Attribute("attrD"))); + + // subsequent should have value "D" + elemD = elemD->NextSiblingElement("elemD"); while (elemD) { std::string elemValue = elemD->GetText(); @@ -724,8 +734,18 @@ TEST(Converter, RemoveEmptyDescendantElement) EXPECT_NE(nullptr, convertedElem->FirstChildElement("elemC")); convertedElem = convertedElem->FirstChildElement("elemC"); ASSERT_NE(nullptr, convertedElem); + + auto elemD = convertedElem->FirstChildElement("elemD"); - ASSERT_NE(elemD, nullptr); + ASSERT_NE(nullptr, elemD); + + // first should have an attribute but no value + EXPECT_EQ(nullptr, elemD->GetText()); + ASSERT_NE(nullptr, elemD->Attribute("attrD")); + EXPECT_EQ("D", std::string(elemD->Attribute("attrD"))); + + // subsequent should have value "D" + elemD = elemD->NextSiblingElement("elemD"); while (elemD) { std::string elemValue = elemD->GetText();