Merge pull request #1463 from gazebosim/scpeters/merge_13_14

Merge sdf13 ➡️ sdf14
gazebosim · Jul 29, 2024 · 17cabcb · 17cabcb
2 parents d80af03 + 608ca14
commit 17cabcb
Show file tree

Hide file tree

Showing 8 changed files with 143 additions and 44 deletions.
diff --git a/include/sdf/Error.hh b/include/sdf/Error.hh
@@ -183,6 +183,9 @@ namespace sdf
     /// \brief The joint axis mimic does not refer to a valid joint in the
     /// current scope.
     JOINT_AXIS_MIMIC_INVALID,
+
+    /// \brief Error at the XML level.
+    XML_ERROR,
   };
 
   class SDFORMAT_VISIBLE Error

diff --git a/src/Converter.cc b/src/Converter.cc
@@ -264,15 +264,15 @@ void Converter::ConvertImpl(tinyxml2::XMLElement *_elem,
     }
     else if (name == "copy")
     {
-      Move(_elem, childElem, true);
+      Move(_elem, childElem, true, _errors);
     }
     else if (name == "map")
     {
       Map(_elem, childElem, _errors);
     }
     else if (name == "move")
     {
-      Move(_elem, childElem, false);
+      Move(_elem, childElem, false, _errors);
     }
     else if (name == "add")
     {
@@ -923,7 +923,8 @@ void Converter::Map(tinyxml2::XMLElement *_elem,
 /////////////////////////////////////////////////
 void Converter::Move(tinyxml2::XMLElement *_elem,
                      tinyxml2::XMLElement *_moveElem,
-                     const bool _copy)
+                     const bool _copy,
+                     sdf::Errors &_errors)
 {
   SDF_ASSERT(_elem != NULL, "SDF element is NULL");
   SDF_ASSERT(_moveElem != NULL, "Move element is NULL");
@@ -1024,7 +1025,8 @@ void Converter::Move(tinyxml2::XMLElement *_elem,
 
     if (toElemStr && !toAttrStr)
     {
-      tinyxml2::XMLNode *cloned = DeepClone(moveFrom->GetDocument(), moveFrom);
+      tinyxml2::XMLNode *cloned = DeepClone(_errors, moveFrom->GetDocument(),
+                                            moveFrom);
       tinyxml2::XMLElement *moveTo = static_cast<tinyxml2::XMLElement*>(cloned);
 
       moveTo->SetValue(toName);

diff --git a/src/Converter.hh b/src/Converter.hh
@@ -108,9 +108,11 @@ namespace sdf
     /// \param[in] _moveElem A 'convert' element that describes the move
     /// operation.
     /// \param[in] _copy True to copy the element
+    /// \param[out] _errors Vector of errors.
     private: static void Move(tinyxml2::XMLElement *_elem,
                               tinyxml2::XMLElement *_moveElem,
-                              const bool _copy);
+                              const bool _copy,
+                              sdf::Errors &_errors);
 
     /// \brief Add an element or attribute to an element.
     /// \param[in] _elem The element to receive the value.

diff --git a/src/Converter_TEST.cc b/src/Converter_TEST.cc
@@ -3370,7 +3370,8 @@ TEST(Converter, World_17_to_18)
   ASSERT_TRUE(errors.empty());
 
   // Compare converted xml with expected
-  std::string convertedXmlStr = ElementToString(xmlDoc.RootElement());
+  std::string convertedXmlStr = ElementToString(errors, xmlDoc.RootElement());
+  ASSERT_TRUE(errors.empty());
   ASSERT_FALSE(convertedXmlStr.empty());
 
   std::string expectedXmlStr = R"(
@@ -3393,7 +3394,9 @@ TEST(Converter, World_17_to_18)
   tinyxml2::XMLDocument expectedXmlDoc;
   expectedXmlDoc.Parse(expectedXmlStr.c_str());
 
-  EXPECT_EQ(ElementToString(expectedXmlDoc.RootElement()), convertedXmlStr);
+  EXPECT_EQ(ElementToString(errors, expectedXmlDoc.RootElement()),
+            convertedXmlStr);
+  ASSERT_TRUE(errors.empty());
 
     // Check some basic elements
   tinyxml2::XMLElement *convertedElem =  xmlDoc.FirstChildElement();
@@ -3509,7 +3512,8 @@ TEST(Converter, World_17_to_18)
   ASSERT_TRUE(errors.empty());
 
   // Compare converted xml with expected
-  convertedXmlStr = ElementToString(xmlDoc.RootElement());
+  convertedXmlStr = ElementToString(errors, xmlDoc.RootElement());
+  ASSERT_TRUE(errors.empty());
   ASSERT_FALSE(convertedXmlStr.empty());
 
   expectedXmlStr = R"(
@@ -3577,7 +3581,9 @@ TEST(Converter, World_17_to_18)
   expectedXmlDoc.Clear();
   expectedXmlDoc.Parse(expectedXmlStr.c_str());
 
-  EXPECT_EQ(ElementToString(expectedXmlDoc.RootElement()), convertedXmlStr);
+  EXPECT_EQ(ElementToString(errors, expectedXmlDoc.RootElement()),
+            convertedXmlStr);
+  ASSERT_TRUE(errors.empty());
 
 
   // ------- Another flattened world in 1.7 format
@@ -3616,7 +3622,8 @@ TEST(Converter, World_17_to_18)
   EXPECT_TRUE(buffer.str().empty()) << buffer.str();
 
   // Compare converted xml with expected
-  convertedXmlStr = ElementToString(xmlDoc.RootElement());
+  convertedXmlStr = ElementToString(errors, xmlDoc.RootElement());
+  ASSERT_TRUE(errors.empty());
   ASSERT_FALSE(convertedXmlStr.empty());
 
   expectedXmlStr = R"(
@@ -3655,7 +3662,9 @@ TEST(Converter, World_17_to_18)
   expectedXmlDoc.Clear();
   expectedXmlDoc.Parse(expectedXmlStr.c_str());
 
-  EXPECT_EQ(ElementToString(expectedXmlDoc.RootElement()), convertedXmlStr);
+  EXPECT_EQ(ElementToString(errors, expectedXmlDoc.RootElement()),
+            convertedXmlStr);
+  ASSERT_TRUE(errors.empty());
 
   // Check some basic elements
   convertedElem =  xmlDoc.FirstChildElement();

diff --git a/src/ParamPassing.cc b/src/ParamPassing.cc
@@ -52,7 +52,7 @@ void updateParams(const ParserConfig &_config,
       _errors.push_back({ErrorCode::ATTRIBUTE_MISSING,
         "Element identifier requires an element_id attribute, but the "
         "element_id is not set. Skipping element alteration:\n"
-        + ElementToString(childElemXml)
+        + ElementToString(_errors, childElemXml)
       });
       continue;
     }
@@ -67,7 +67,7 @@ void updateParams(const ParserConfig &_config,
       _errors.push_back({ErrorCode::ATTRIBUTE_INVALID,
         "Missing name after double colons in element identifier. "
         "Skipping element alteration:\n"
-        + ElementToString(childElemXml)
+        + ElementToString(_errors, childElemXml)
       });
       continue;
     }
@@ -83,7 +83,7 @@ void updateParams(const ParserConfig &_config,
       {
         _errors.push_back({ErrorCode::ATTRIBUTE_INVALID,
           "Action [" + actionStr + "] is not a valid action. Skipping "
-          "element alteration:\n" + ElementToString(childElemXml)
+          "element alteration:\n" + ElementToString(_errors, childElemXml)
         });
         continue;
       }
@@ -102,7 +102,7 @@ void updateParams(const ParserConfig &_config,
         _errors.push_back({ErrorCode::ATTRIBUTE_MISSING,
           "Element to be added is missing a 'name' attribute. "
           "Skipping element addition:\n"
-          + ElementToString(childElemXml)
+          + ElementToString(_errors, childElemXml)
         });
         continue;
       }
@@ -112,7 +112,7 @@ void updateParams(const ParserConfig &_config,
       {
         _errors.push_back({ErrorCode::ATTRIBUTE_INVALID,
           "The 'name' attribute can not be empty. Skipping element addition:\n"
-          + ElementToString(childElemXml)
+          + ElementToString(_errors, childElemXml)
         });
         continue;
       }
@@ -129,7 +129,7 @@ void updateParams(const ParserConfig &_config,
           + " element_id='" + childElemXml->Attribute("element_id")
           + "'> because element already exists in included model. "
           + "Skipping element addition:\n"
-          + ElementToString(childElemXml)
+          + ElementToString(_errors, childElemXml)
         });
         continue;
       }
@@ -157,7 +157,8 @@ void updateParams(const ParserConfig &_config,
       _errors.push_back({ErrorCode::ELEMENT_MISSING,
         "Could not find element <" + std::string(childElemXml->Name())
         + " element_id='" + childElemXml->Attribute("element_id") + "'>. " +
-        "Skipping element modification:\n" + ElementToString(childElemXml)
+        "Skipping element modification:\n" +
+        ElementToString(_errors, childElemXml)
       });
       continue;
     }
@@ -194,7 +195,7 @@ void updateParams(const ParserConfig &_config,
       {
         _errors.push_back({ErrorCode::ELEMENT_INVALID,
           "Unable to convert XML to SDF. Skipping element replacement:\n"
-          + ElementToString(childElemXml)
+          + ElementToString(_errors, childElemXml)
         });
         continue;
       }
@@ -355,7 +356,7 @@ ElementPtr initElementDescription(const tinyxml2::XMLElement *_xml,
     _errors.push_back({ErrorCode::ELEMENT_INVALID,
       "Element [" + std::string(_xml->Name()) + "] is not a defined "
       "SDF element. Skipping element alteration\n: "
-      + ElementToString(_xml)
+      + ElementToString(_errors, _xml)
     });
     return nullptr;
   }
@@ -384,7 +385,7 @@ void handleIndividualChildActions(const ParserConfig &_config,
         "Missing an action attribute. Skipping child element modification "
         "with parent <" + std::string(_childrenXml->Name()) + " element_id='"
         + std::string(_childrenXml->Attribute("element_id")) + "'>:\n"
-        + ElementToString(xmlChild)
+        + ElementToString(_errors, xmlChild)
       });
       continue;
     }
@@ -397,7 +398,7 @@ void handleIndividualChildActions(const ParserConfig &_config,
         "child element modification with parent <"
         + std::string(_childrenXml->Name()) + " element_id='"
         + std::string(_childrenXml->Attribute("element_id")) + "'>:\n"
-        + ElementToString(xmlChild)
+        + ElementToString(_errors, xmlChild)
       });
       continue;
     }
@@ -411,7 +412,7 @@ void handleIndividualChildActions(const ParserConfig &_config,
           "Could not find element. Skipping child element removal "
           "with parent <" + std::string(_childrenXml->Name()) + " element_id='"
           + std::string(_childrenXml->Attribute("element_id")) + "'>:\n"
-          + ElementToString(xmlChild)
+          + ElementToString(_errors, xmlChild)
         });
       }
       else
@@ -430,7 +431,7 @@ void handleIndividualChildActions(const ParserConfig &_config,
           "Could not find element. Skipping child element modification "
           "with parent <" + std::string(_childrenXml->Name()) + " element_id='"
           + std::string(_childrenXml->Attribute("element_id")) + "'>:\n"
-          + ElementToString(xmlChild)
+          + ElementToString(_errors, xmlChild)
         });
       }
       else
@@ -455,7 +456,7 @@ void handleIndividualChildActions(const ParserConfig &_config,
         "child element modification with parent <"
         + std::string(_childrenXml->Name()) + " element_id='"
         + std::string(_childrenXml->Attribute("element_id")) + "'>:\n"
-        + ElementToString(xmlChild)
+        + ElementToString(_errors, xmlChild)
       });
       continue;
     }
@@ -468,7 +469,7 @@ void handleIndividualChildActions(const ParserConfig &_config,
         "Unable to convert XML to SDF. Skipping child element alteration "
         "with parent <" + std::string(_childrenXml->Name()) + " element_id='"
         + std::string(_childrenXml->Attribute("element_id")) + "'>:\n"
-        + ElementToString(xmlChild)
+        + ElementToString(_errors, xmlChild)
       });
       continue;
     }
@@ -486,7 +487,7 @@ void handleIndividualChildActions(const ParserConfig &_config,
           "Could not find element. Skipping child element replacement "
           "with parent <" + std::string(_childrenXml->Name()) + " element_id='"
           + std::string(_childrenXml->Attribute("element_id")) + "'>:\n"
-          + ElementToString(xmlChild)
+          + ElementToString(_errors, xmlChild)
         });
         continue;
       }
@@ -498,7 +499,7 @@ void handleIndividualChildActions(const ParserConfig &_config,
           "Replacement element is missing a 'name' attribute. "
           "Skipping element replacement <" + std::string(_childrenXml->Name())
           + " element_id='" + std::string(_childrenXml->Attribute("element_id"))
-          + "'>:\n" + ElementToString(xmlChild)
+          + "'>:\n" + ElementToString(_errors, xmlChild)
         });
         continue;
       }
@@ -525,7 +526,7 @@ void add(const ParserConfig &_config, const std::string &_source,
   {
     _errors.push_back({ErrorCode::ELEMENT_INVALID,
       "Unable to convert XML to SDF. Skipping element addition:\n"
-      + ElementToString(_childXml)
+      + ElementToString(_errors, _childXml)
     });
   }
 }
@@ -557,7 +558,8 @@ void modifyAttributes(tinyxml2::XMLElement *_xml,
       {
         _errors.push_back({ErrorCode::ATTRIBUTE_INVALID,
           "Attribute [" + attrName + "] is invalid. "
-          "Skipping attribute modification in:\n" + ElementToString(_xml)
+          "Skipping attribute modification in:\n" +
+          ElementToString(_errors, _xml)
         });
         continue;
       }
@@ -582,7 +584,7 @@ void modifyChildren(tinyxml2::XMLElement *_xml,
     {
       _errors.push_back({ErrorCode::ELEMENT_MISSING,
         "Could not find element [" + elemName + "]. "
-        "Skipping modification for:\n" + ElementToString(_xml)
+        "Skipping modification for:\n" + ElementToString(_errors, _xml)
       });
       continue;
     }
@@ -599,7 +601,7 @@ void modifyChildren(tinyxml2::XMLElement *_xml,
         _errors.push_back({ErrorCode::ELEMENT_INVALID,
           "Value [" + std::string(xmlChild->GetText()) + "] for element ["
           + elemName + "] is invalid. Skipping modification for:\n"
-          + ElementToString(_xml)
+          + ElementToString(_errors, _xml)
         });
         continue;
       }
@@ -620,9 +622,9 @@ void modifyChildren(tinyxml2::XMLElement *_xml,
         // sdf has child elements but no children were specified in xml
         std::stringstream ss;
         ss << "No modifications for element "
-           << ElementToString(xmlChild)
+           << ElementToString(_errors, xmlChild)
            << " provided, skipping modification for:\n"
-           << ElementToString(_xml);
+           << ElementToString(_errors, _xml);
         Error err(ErrorCode::WARNING, ss.str());
         enforceConfigurablePolicyCondition(
             _config.WarningsPolicy(), err, _errors);
@@ -650,7 +652,7 @@ void modify(tinyxml2::XMLElement *_xml,  const sdf::ParserConfig &_config,
       _errors.push_back({ErrorCode::ELEMENT_INVALID,
         "Value [" + std::string(_xml->GetText()) + "] for element [" +
         std::string(_xml->Name()) + "] is invalid. Skipping modification for:\n"
-        + ElementToString(_xml)
+        + ElementToString(_errors, _xml)
       });
     }
   }
@@ -688,7 +690,7 @@ void remove(const tinyxml2::XMLElement *_xml, const sdf::ParserConfig &_config,
           + std::string(xmlParent->Name()) + " element_id='"
           + std::string(xmlParent->Attribute("element_id")) + "'> with parent <"
           + std::string(_xml->Name()) + ">:\n"
-          + ElementToString(xmlChild)
+          + ElementToString(_errors, xmlChild)
         });
         continue;
       }