Load nested models in Model::Load and update tests

Signed-off-by: Steve Peters <[email protected]>

src/Model.cc

@@ -200,14 +200,6 @@ Errors Model::Load(ElementPtr _sdf)
   // Load the pose. Ignore the return value since the model pose is optional.
   loadPose(_sdf, this->dataPtr->pose, this->dataPtr->poseRelativeTo);
 
-  // Nested models are not yet supported.
-  if (_sdf->HasElement("model"))
-  {
-    errors.push_back({ErrorCode::NESTED_MODELS_UNSUPPORTED,
-                     "Nested models are not yet supported by DOM objects, "
-                     "skipping model [" + this->dataPtr->name + "]."});
-  }
-
   if (!_sdf->HasUniqueChildNames())
   {
     sdfwarn << "Non-unique names detected in XML children of model with name["
@@ -218,17 +210,56 @@ Errors Model::Load(ElementPtr _sdf)
   // name collisions
   std::unordered_set<std::string> frameNames;
 
+  // Load nested models.
+  Errors nestedModelLoadErrors = loadUniqueRepeated<Model>(_sdf, "model",
+    this->dataPtr->models);
+  errors.insert(errors.end(),
+                nestedModelLoadErrors.begin(),
+                nestedModelLoadErrors.end());
+
+  // Nested models are loaded first, and loadUniqueRepeated ensures there are no
+  // duplicate names, so these names can be added to frameNames without
+  // checking uniqueness.
+  for (const auto &model : this->dataPtr->models)
+  {
+    frameNames.insert(model.Name());
+  }
+
   // Load all the links.
   Errors linkLoadErrors = loadUniqueRepeated<Link>(_sdf, "link",
     this->dataPtr->links);
   errors.insert(errors.end(), linkLoadErrors.begin(), linkLoadErrors.end());
 
-  // Links are loaded first, and loadUniqueRepeated ensures there are no
-  // duplicate names, so these names can be added to frameNames without
-  // checking uniqueness.
-  for (const auto &link : this->dataPtr->links)
+  // Check links for name collisions and modify and warn if so.
+  for (auto &link : this->dataPtr->links)
   {
-    frameNames.insert(link.Name());
+    std::string linkName = link.Name();
+    if (frameNames.count(linkName) > 0)
+    {
+      // This link has a name collision
+      if (sdfVersion < ignition::math::SemanticVersion(1, 7))
+      {
+        // This came from an old file, so try to workaround by renaming link
+        linkName += "_link";
+        int i = 0;
+        while (frameNames.count(linkName) > 0)
+        {
+          linkName = link.Name() + "_link" + std::to_string(i++);
+        }
+        sdfwarn << "Link with name [" << link.Name() << "] "
+                << "in model with name [" << this->Name() << "] "
+                << "has a name collision, changing link name to ["
+                << linkName << "].\n";
+        link.SetName(linkName);
+      }
+      else
+      {
+        sdferr << "Link with name [" << link.Name() << "] "
+               << "in model with name [" << this->Name() << "] "
+               << "has a name collision. Please rename this link.\n";
+      }
+    }
+    frameNames.insert(linkName);
   }
 
   // If the model is not static:

src/Model_TEST.cc

@@ -47,21 +47,35 @@ TEST(DOMModel, Construction)
   model.SetEnableWind(true);
   EXPECT_TRUE(model.EnableWind());
 
+  EXPECT_EQ(0u, model.ModelCount());
+  EXPECT_EQ(nullptr, model.ModelByIndex(0));
+  EXPECT_EQ(nullptr, model.ModelByIndex(1));
+  EXPECT_EQ(nullptr, model.ModelByName(""));
+  EXPECT_EQ(nullptr, model.ModelByName("default"));
+  EXPECT_FALSE(model.ModelNameExists(""));
+  EXPECT_FALSE(model.ModelNameExists("default"));
+
   EXPECT_EQ(0u, model.LinkCount());
   EXPECT_EQ(nullptr, model.LinkByIndex(0));
   EXPECT_EQ(nullptr, model.LinkByIndex(1));
+  EXPECT_EQ(nullptr, model.LinkByName(""));
+  EXPECT_EQ(nullptr, model.LinkByName("default"));
   EXPECT_FALSE(model.LinkNameExists(""));
   EXPECT_FALSE(model.LinkNameExists("default"));
 
   EXPECT_EQ(0u, model.JointCount());
   EXPECT_EQ(nullptr, model.JointByIndex(0));
   EXPECT_EQ(nullptr, model.JointByIndex(1));
+  EXPECT_EQ(nullptr, model.JointByName(""));
+  EXPECT_EQ(nullptr, model.JointByName("default"));
   EXPECT_FALSE(model.JointNameExists(""));
   EXPECT_FALSE(model.JointNameExists("default"));
 
   EXPECT_EQ(0u, model.FrameCount());
   EXPECT_EQ(nullptr, model.FrameByIndex(0));
   EXPECT_EQ(nullptr, model.FrameByIndex(1));
+  EXPECT_EQ(nullptr, model.FrameByName(""));
+  EXPECT_EQ(nullptr, model.FrameByName("default"));
   EXPECT_FALSE(model.FrameNameExists(""));
   EXPECT_FALSE(model.FrameNameExists("default"));
 

src/ign_TEST.cc

@@ -282,9 +282,7 @@ TEST(check, SDF)
     // Check nested_model.sdf
     std::string output =
       custom_exec_str(g_ignCommand + " sdf -k " + path + g_sdfVersion);
-    EXPECT_NE(output.find("Error: Nested models are not yet supported by DOM "
-              "objects, skipping model [top_level_model]."),
-              std::string::npos) << output;
+    EXPECT_EQ("Valid.\n", output) << output;
   }
 
   // Check an invalid SDF file that uses reserved names.

test/integration/model_dom.cc

@@ -155,10 +155,7 @@ TEST(DOMRoot, NestedModel)
   // Load the SDF file
   sdf::Root root;
   auto errors = root.Load(testFile);
-
-  // it should complain because nested models aren't yet supported
-  EXPECT_FALSE(errors.empty());
-  EXPECT_EQ(errors[0].Code(), sdf::ErrorCode::NESTED_MODELS_UNSUPPORTED);
+  EXPECT_TRUE(errors.empty());
 
   EXPECT_EQ(1u, root.ModelCount());
 
@@ -181,6 +178,24 @@ TEST(DOMRoot, NestedModel)
   EXPECT_EQ(nullptr, model->JointByIndex(1));
 
   EXPECT_TRUE(model->JointNameExists("top_level_joint"));
+
+  ASSERT_EQ(1u, model->ModelCount());
+  const sdf::Model *nestedModel = model->ModelByIndex(0);
+  ASSERT_NE(nullptr, nestedModel);
+  EXPECT_EQ(nullptr, model->ModelByIndex(1));
+
+  EXPECT_TRUE(model->ModelNameExists("nested_model"));
+  EXPECT_EQ(nestedModel, model->ModelByName("nested_model"));
+  EXPECT_EQ("nested_model", nestedModel->Name());
+
+  EXPECT_EQ(1u, nestedModel->LinkCount());
+  EXPECT_NE(nullptr, nestedModel->LinkByIndex(0));
+  EXPECT_EQ(nullptr, nestedModel->LinkByIndex(1));
+
+  EXPECT_TRUE(nestedModel->LinkNameExists("nested_link01"));
+
+  EXPECT_EQ(0u, nestedModel->JointCount());
+  EXPECT_EQ(0u, nestedModel->FrameCount());
 }
 
 /////////////////////////////////////////////////