Merge branch 'sdf12' into ahcorde/usd_to_sdf/fix_world_errors

Author: ahcorde

include/sdf/Model.hh

@@ -24,6 +24,7 @@
 #include <ignition/math/Pose3.hh>
 #include <ignition/utils/ImplPtr.hh>
 #include "sdf/Element.hh"
+#include "sdf/ParserConfig.hh"
 #include "sdf/Plugin.hh"
 #include "sdf/SemanticPose.hh"
 #include "sdf/Types.hh"
@@ -420,7 +421,8 @@ namespace sdf
     /// model.
     /// Note that parameter passing functionality is not captured with this
     /// function.
-    /// \param[in] _useIncludeTag When true, the model's URI is used to create
+    /// \param[in] _config Parser configuration. When the ToElementUseIncludeTag
+    /// policy is true, the model's URI is used to create
     /// an SDF `<include>` rather than a `<model>`. The model's URI must be
     /// first set using the `Model::SetUri` function. If the model's URI is
     /// empty, then a `<model>` element will be generated. The default is true
@@ -429,7 +431,8 @@ namespace sdf
     /// is loaded from an `<include>` tag since the parser will
     /// automatically expand an `<include>` element to a `<model>` element.
     /// \return SDF element pointer with updated model values.
-    public: sdf::ElementPtr ToElement(bool _useIncludeTag = true) const;
+    public: sdf::ElementPtr ToElement(
+        const ParserConfig &_config = ParserConfig::GlobalConfig()) const;
 
     /// \brief Check if a given name exists in the FrameAttachedTo graph at the
     /// scope of the model.

include/sdf/ParserConfig.hh

@@ -174,6 +174,28 @@ class SDFORMAT_VISIBLE ParserConfig
   /// \brief Get the preserveFixedJoint flag value.
   public: bool URDFPreserveFixedJoint() const;
 
+  /// \brief Several DOM classes have ToElement() methods that return an
+  /// XML Element populated from the contents of the DOM object. When
+  /// populating the details of a model that was included using the
+  /// <include> tag, one may wish to retain the exact include tag and
+  /// URI instead of copying the full details of the included model.
+  /// This method lets you set this behavior.
+  /// \param[in] _useIncludeTag When true, the model's URI is used to create
+  /// an SDF `<include>` rather than a `<model>`. The model's URI must be
+  /// first set using the `Model::SetUri` function. If the model's URI is
+  /// empty, then a `<model>` element will be generated. The default is true
+  /// so that URI values are used when ToElement is called from a
+  /// World object. Make sure to use `Model::SetUri` even when the model
+  /// is loaded from an `<include>` tag since the parser will
+  /// automatically expand an `<include>` element to a `<model>` element.
+  public: void SetToElementUseIncludeTag(bool _useIncludeTag);
+
+  /// \brief Get the policy value about whether <include> tags are
+  /// reconstituted in ToElement() invocations.
+  /// \return True if include tags are reconstituted, or false
+  /// if the fully populated model is returned instead.
+  public: bool ToElementUseIncludeTag() const;
+
   /// \brief Private data pointer.
   IGN_UTILS_IMPL_PTR(dataPtr)
 };

include/sdf/Root.hh

@@ -20,6 +20,7 @@
 #include <string>
 #include <ignition/utils/ImplPtr.hh>
 
+#include "sdf/ParserConfig.hh"
 #include "sdf/SDFImpl.hh"
 #include "sdf/Types.hh"
 #include "sdf/sdf_config.h"
@@ -202,10 +203,10 @@ namespace sdf
     /// root.
     /// Note that parameter passing functionality is not captured with this
     /// function.
-    /// \param[in] _useIncludeTag This will pass the _useIncludeTag to
-    /// sdf::Model::ToElement.
+    /// \param[in] _config Custom parser configuration
     /// \return SDF element pointer with updated root values.
-    public: sdf::ElementPtr ToElement(bool _useIncludeTag = true) const;
+    public: sdf::ElementPtr ToElement(
+        const ParserConfig &_config = ParserConfig::GlobalConfig()) const;
 
     /// \brief Private data pointer
     IGN_UTILS_UNIQUE_IMPL_PTR(dataPtr)

include/sdf/World.hh

@@ -27,6 +27,7 @@
 #include "sdf/Atmosphere.hh"
 #include "sdf/Element.hh"
 #include "sdf/Gui.hh"
+#include "sdf/ParserConfig.hh"
 #include "sdf/Plugin.hh"
 #include "sdf/Scene.hh"
 #include "sdf/Types.hh"
@@ -431,10 +432,10 @@ namespace sdf
     /// world.
     /// Note that parameter passing functionality is not captured with this
     /// function.
-    /// \param[in] _useIncludeTag This parameter is passed through to the
-    /// Model::ToElement function.
+    /// \param[in] _config Custom parser configuration
     /// \return SDF element pointer with updated world values.
-    public: sdf::ElementPtr ToElement(bool _useIncludeTag = true) const;
+    public: sdf::ElementPtr ToElement(
+        const ParserConfig &_config = ParserConfig::GlobalConfig()) const;
 
     /// \brief Get the plugins attached to this object.
     /// \return A vector of Plugin, which will be empty if there are no

include/sdf/parser.hh

@@ -46,6 +46,13 @@ namespace sdf
   SDFORMAT_VISIBLE
   bool init(SDFPtr _sdf);
 
+  /// \brief Initialize the SDF interface from the embedded root spec file
+  /// \param[out] _sdf Pointer to an SDF object.
+  /// \param[in] _config Custom parser configuration
+  /// \return True if successful.
+  SDFORMAT_VISIBLE
+  bool init(SDFPtr _sdf, const ParserConfig &_config);
+
   /// \brief Initialize the SDF interface using a file
   /// \param[in] _filename Name of the SDF file
   /// \param[out] _sdf Pointer to an SDF object.

src/Model.cc

@@ -123,8 +123,8 @@ class sdf::Model::Implementation
   /// </world>
   ///
   /// and the included model could also have a plugin. We want the
-  /// ToElement(true) function to output only the plugin specified in the
-  /// <include> tag.
+  /// ToElement() function to output only the plugin specified in the
+  /// <include> tag when the ToElementUseIncludeTag policy is true..
   public: std::vector<Plugin> includePlugins;
 };
 
@@ -974,12 +974,12 @@ void Model::SetUri(const std::string &_uri)
 }
 
 /////////////////////////////////////////////////
-sdf::ElementPtr Model::ToElement(bool _useIncludeTag) const
+sdf::ElementPtr Model::ToElement(const ParserConfig &_config) const
 {
-  if (_useIncludeTag && !this->dataPtr->uri.empty())
+  if (_config.ToElementUseIncludeTag() && !this->dataPtr->uri.empty())
   {
     sdf::ElementPtr worldElem(new sdf::Element);
-    sdf::initFile("world.sdf", worldElem);
+    sdf::initFile("world.sdf", _config, worldElem);
 
     sdf::ElementPtr includeElem = worldElem->AddElement("include");
     includeElem->GetElement("uri")->Set(this->Uri());
@@ -1005,7 +1005,7 @@ sdf::ElementPtr Model::ToElement(bool _useIncludeTag) const
   }
 
   sdf::ElementPtr elem(new sdf::Element);
-  sdf::initFile("model.sdf", elem);
+  sdf::initFile("model.sdf", _config, elem);
   elem->GetAttribute("name")->Set(this->Name());
 
   if (!this->dataPtr->canonicalLink.empty())
@@ -1043,7 +1043,7 @@ sdf::ElementPtr Model::ToElement(bool _useIncludeTag) const
 
   // Model
   for (const sdf::Model &model : this->dataPtr->models)
-    elem->InsertElement(model.ToElement(_useIncludeTag), true);
+    elem->InsertElement(model.ToElement(_config), true);
 
   // Add in the plugins
   for (const Plugin &plugin : this->dataPtr->plugins)

src/Model_TEST.cc

@@ -455,7 +455,9 @@ TEST(DOMModel, Uri)
 
   // ToElement NOT using the URI, which should result in a <model>
   {
-    sdf::ElementPtr elem = model.ToElement(false);
+    sdf::ParserConfig config = sdf::ParserConfig::GlobalConfig();
+    config.SetToElementUseIncludeTag(false);
+    sdf::ElementPtr elem = model.ToElement(config);
     elem->PrintValues("  ");
 
     // Should be a <model>

src/ParamPassing.cc

@@ -184,7 +184,8 @@ void updateParams(const ParserConfig &_config,
     }
     else if (actionStr == "replace")
     {
-      ElementPtr newElem = initElementDescription(childElemXml, _errors);
+      ElementPtr newElem =
+        initElementDescription(childElemXml, _config, _errors);
       if (!newElem)
         continue;
 
@@ -334,12 +335,13 @@ ElementPtr getElementByName(const ElementPtr _elem,
 
 //////////////////////////////////////////////////
 ElementPtr initElementDescription(const tinyxml2::XMLElement *_xml,
+                                  const ParserConfig &_config,
                                   Errors &_errors)
 {
   ElementPtr elemDesc = std::make_shared<Element>();
   std::string filename = std::string(_xml->Name()) + ".sdf";
 
-  if (!initFile(filename, elemDesc))
+  if (!initFile(filename, _config, elemDesc))
   {
     // TODO(jenn) not sure if we should load the element anyway
     // (e.g., user created their own element), maybe future implementation
@@ -360,7 +362,7 @@ void handleIndividualChildActions(const ParserConfig &_config,
                                   ElementPtr _elem,
                                   Errors &_errors)
 {
-  ElementPtr elemDesc = initElementDescription(_childrenXml, _errors);
+  ElementPtr elemDesc = initElementDescription(_childrenXml, _config, _errors);
   if (!elemDesc)
     return;
 
@@ -504,7 +506,7 @@ void handleIndividualChildActions(const ParserConfig &_config,
 void add(const ParserConfig &_config, const std::string &_source,
          tinyxml2::XMLElement *_childXml, ElementPtr _elem, Errors &_errors)
 {
-  ElementPtr newElem = initElementDescription(_childXml, _errors);
+  ElementPtr newElem = initElementDescription(_childXml, _config, _errors);
   if (!newElem)
     return;
 

src/ParamPassing.hh

@@ -96,10 +96,12 @@ namespace sdf
 
     /// \brief Initialize an sdf element description from the xml element
     /// \param[in] _xml Pointer to xml element
+    /// \param[in] _config Custom parser configuration
     /// \param[out] _errors Captures errors found during parsing
     /// \return ElementPtr to the initialized element description,
     /// nullptr if undefined/unknown sdf element
     ElementPtr initElementDescription(const tinyxml2::XMLElement *_xml,
+                                      const ParserConfig &_config,
                                       Errors &_errors);
 
     /// \brief Handles individual actions of children in _childrenXml

src/ParserConfig.cc

@@ -49,6 +49,10 @@ class sdf::ParserConfig::Implementation
   /// \brief Flag to explicitly preserve fixed joints when
   /// reading the SDF/URDF file.
   public: bool preserveFixedJoint = false;
+
+  /// \brief Flag to use <include> tags within ToElement methods instead of
+  /// the fully included model.
+  public: bool toElementUseIncludeTag = true;
 };
 
 
@@ -169,3 +173,15 @@ bool ParserConfig::URDFPreserveFixedJoint() const
 {
   return this->dataPtr->preserveFixedJoint;
 }
+
+/////////////////////////////////////////////////
+void ParserConfig::SetToElementUseIncludeTag(bool _useIncludeTag)
+{
+  this->dataPtr->toElementUseIncludeTag = _useIncludeTag;
+}
+
+/////////////////////////////////////////////////
+bool ParserConfig::ToElementUseIncludeTag() const
+{
+  return this->dataPtr->toElementUseIncludeTag;
+}

src/Root.cc

@@ -179,7 +179,7 @@ Errors Root::LoadSdfString(const std::string &_sdf, const ParserConfig &_config)
 {
   Errors errors;
   SDFPtr sdfParsed(new SDF());
-  init(sdfParsed);
+  init(sdfParsed, _config);
 
   // Read an SDF string, and store the result in sdfParsed.
   if (!readString(_sdf, _config, sdfParsed, errors))
@@ -525,16 +525,16 @@ void Root::Implementation::UpdateGraphs(sdf::Model &_model,
 }
 
 /////////////////////////////////////////////////
-sdf::ElementPtr Root::ToElement(bool _useIncludeTag) const
+sdf::ElementPtr Root::ToElement(const ParserConfig &_config) const
 {
   sdf::ElementPtr elem(new sdf::Element);
-  sdf::initFile("root.sdf", elem);
+  sdf::initFile("root.sdf", _config, elem);
 
   elem->GetAttribute("version")->Set(this->Version());
 
   if (this->Model() != nullptr)
   {
-    elem->InsertElement(this->Model()->ToElement(_useIncludeTag), true);
+    elem->InsertElement(this->Model()->ToElement(_config), true);
   }
   else if (this->Light() != nullptr)
   {
@@ -548,7 +548,7 @@ sdf::ElementPtr Root::ToElement(bool _useIncludeTag) const
   {
     // Worlds
     for (const sdf::World &world : this->dataPtr->worlds)
-      elem->InsertElement(world.ToElement(_useIncludeTag), true);
+      elem->InsertElement(world.ToElement(_config), true);
   }
 
   return elem;

src/World.cc

@@ -856,10 +856,10 @@ Errors World::Implementation::LoadSphericalCoordinates(
 }
 
 /////////////////////////////////////////////////
-sdf::ElementPtr World::ToElement(bool _useIncludeTag) const
+sdf::ElementPtr World::ToElement(const ParserConfig &_config) const
 {
   sdf::ElementPtr elem(new sdf::Element);
-  sdf::initFile("world.sdf", elem);
+  sdf::initFile("world.sdf", _config, elem);
 
   elem->GetAttribute("name")->Set(this->Name());
   elem->GetElement("gravity")->Set(this->Gravity());
@@ -874,7 +874,7 @@ sdf::ElementPtr World::ToElement(bool _useIncludeTag) const
 
   // Models
   for (const sdf::Model &model : this->dataPtr->models)
-    elem->InsertElement(model.ToElement(_useIncludeTag), true);
+    elem->InsertElement(model.ToElement(_config), true);
 
   // Actors
   for (const sdf::Actor &actor : this->dataPtr->actors)

src/parser.cc

@@ -369,11 +369,17 @@ static void insertIncludedElement(sdf::SDFPtr _includeSDF,
 
 //////////////////////////////////////////////////
 bool init(SDFPtr _sdf)
+{
+  return init(_sdf, ParserConfig::GlobalConfig());
+}
+
+//////////////////////////////////////////////////
+bool init(SDFPtr _sdf, const ParserConfig &_config)
 {
   std::string xmldata = SDF::EmbeddedSpec("root.sdf", false);
   auto xmlDoc = makeSdfDoc();
   xmlDoc.Parse(xmldata.c_str());
-  return initDoc(&xmlDoc, ParserConfig::GlobalConfig(), _sdf);
+  return initDoc(&xmlDoc, _config, _sdf);
 }
 
 //////////////////////////////////////////////////
@@ -646,7 +652,7 @@ SDFPtr readFile(
 {
   // Create and initialize the data structure that will hold the parsed SDF data
   sdf::SDFPtr sdfParsed(new sdf::SDF());
-  sdf::init(sdfParsed);
+  sdf::init(sdfParsed, _config);
 
   // Read an SDF file, and store the result in sdfParsed.
   if (!sdf::readFile(_filename, _config, sdfParsed, _errors))
@@ -1587,12 +1593,12 @@ bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf,
           // a new sdf pointer is created here by cloning a fresh sdf template
           // pointer instead of calling init every iteration.
           // SDFPtr includeSDF(new SDF);
-          // init(includeSDF);
+          // init(includeSDF, _config);
           static SDFPtr includeSDFTemplate;
           if (!includeSDFTemplate)
           {
             includeSDFTemplate.reset(new SDF);
-            init(includeSDFTemplate);
+            init(includeSDFTemplate, _config);
           }
           SDFPtr includeSDF(new SDF);
           includeSDF->Root(includeSDFTemplate->Root()->Clone());

test/integration/model_dom.cc

@@ -700,7 +700,8 @@ TEST(DOMModel, IncludeModelWithPlugin)
 
   // Test ToElement with useInclude == true
   {
-    sdf::ElementPtr elem = model->ToElement(true);
+    config.SetToElementUseIncludeTag(true);
+    sdf::ElementPtr elem = model->ToElement(config);
 
     // There should be a uri
     ASSERT_TRUE(elem->HasElement("uri"));
@@ -723,7 +724,8 @@ TEST(DOMModel, IncludeModelWithPlugin)
   // model SDF which would have two plugins, one from the <include> tag and
   // one from the included model
   {
-    sdf::ElementPtr elem = model->ToElement(false);
+    config.SetToElementUseIncludeTag(false);
+    sdf::ElementPtr elem = model->ToElement(config);
 
     // There should NOT be a uri
     ASSERT_FALSE(elem->HasElement("uri"));