Use updated model names for spawned models when generating SDFormat (#…

…166)

This fixes an issue with SDFormat generator where it was not using the updated model names of models spawned with a new name or models that get assigned a new name due to a name conflict. The issue only occurred if the generated SDFormat was expanded, i.e, not using <include>.

Changelog.md

@@ -2,6 +2,9 @@
 
 ### Ignition Gazebo 2.XX.XX (20XX-XX-XX)
 
+1. Use updated model names for spawned models when generating SDFormat
+    * [Pull Request 166](https://github.com/ignitionrobotics/ign-gazebo/pull/166)
+
 1. Allow renaming breadcrumb models if there is a name conflict
     * [Pull Request 155](https://github.com/ignitionrobotics/ign-gazebo/pull/155)
 

src/SdfGenerator.cc

@@ -376,9 +376,13 @@ namespace sdf_generator
 
     // Update sdf based current components. Here are the list of components to
     // be updated:
+    // - Name
     // - Pose
     // This list is to be updated as other components become updateable during
     // simulation
+    auto *nameComp = _ecm.Component<components::Name>(_entity);
+    _elem->GetAttribute("name")->Set(nameComp->Data());
+
     auto *poseComp = _ecm.Component<components::Pose>(_entity);
 
     auto poseElem = _elem->GetElement("pose");

test/integration/save_world.cc

@@ -51,6 +51,20 @@ class SdfGeneratorFixture : public ::testing::Test
     this->server = std::make_unique<Server>(serverConfig);
     EXPECT_FALSE(server->Running());
   }
+  public: std::string RequestGeneratedSdf()
+  {
+    transport::Node node;
+    msgs::SdfGeneratorConfig req;
+
+    msgs::StringMsg worldGenSdfRes;
+    bool result;
+    unsigned int timeout = 5000;
+    std::string service{"/world/save_world/generate_world_sdf"};
+    EXPECT_TRUE(node.Request(service, req, timeout, worldGenSdfRes, result));
+    EXPECT_TRUE(result);
+    return worldGenSdfRes.data();
+  }
+
   public: std::unique_ptr<Server> server;
 };
 
@@ -116,24 +130,23 @@ TEST_F(SdfGeneratorFixture, WorldWithModelsSpawnedAfterLoad)
   EXPECT_TRUE(this->server->EntityByName("spawned_model").has_value());
   EXPECT_NE(kNullEntity, this->server->EntityByName("spawned_model"));
 
-  msgs::SdfGeneratorConfig req;
-
-  msgs::StringMsg worldGenSdfRes;
-  bool result;
-  unsigned int timeout = 5000;
-  std::string service{"/world/save_world/generate_world_sdf"};
-
-  EXPECT_TRUE(node.Request(service, req, timeout, worldGenSdfRes, result));
-  EXPECT_TRUE(result);
+  const std::string worldGenSdfRes = this->RequestGeneratedSdf();
   sdf::Root root;
-  sdf::Errors err = root.LoadSdfString(worldGenSdfRes.data());
+  sdf::Errors err = root.LoadSdfString(worldGenSdfRes);
   EXPECT_TRUE(err.empty());
   auto *world = root.WorldByIndex(0);
   ASSERT_NE(nullptr, world);
   EXPECT_EQ(6u, world->ModelCount());
 
+  EXPECT_TRUE(world->ModelNameExists("inlineM1"));
+  EXPECT_TRUE(world->ModelNameExists("backpack1"));
+  EXPECT_TRUE(world->ModelNameExists("backpack2"));
+  EXPECT_TRUE(world->ModelNameExists("backpack3"));
+  EXPECT_TRUE(world->ModelNameExists("test_ground_plane"));
+  EXPECT_TRUE(world->ModelNameExists("spawned_model"));
+
   tinyxml2::XMLDocument genSdfDoc;
-  genSdfDoc.Parse(worldGenSdfRes.data().c_str());
+  genSdfDoc.Parse(worldGenSdfRes.c_str());
   ASSERT_NE(nullptr, genSdfDoc.RootElement());
   auto genWorld = genSdfDoc.RootElement()->FirstChildElement("world");
   ASSERT_NE(nullptr, genWorld);
@@ -180,3 +193,43 @@ TEST_F(SdfGeneratorFixture, WorldWithModelsSpawnedAfterLoad)
   EXPECT_EQ(2u, modelCount);
   EXPECT_EQ(1u, spawnedModelCount);
 }
+
+/////////////////////////////////////////////////
+TEST_F(SdfGeneratorFixture, ModelSpawnedWithNewName)
+{
+  this->LoadWorld("test/worlds/save_world.sdf");
+
+  auto modelStr = R"(
+<?xml version="1.0" ?>
+<sdf version='1.6'>
+  <model name='spawned_model'>
+    <link name='link'/>
+  </model>
+</sdf>)";
+
+  transport::Node node;
+  msgs::EntityFactory req;
+  msgs::Boolean res;
+  bool result;
+  unsigned int timeout = 5000;
+  std::string service{"/world/save_world/create"};
+
+  req.set_sdf(modelStr);
+  req.set_name("new_model_name");
+
+  EXPECT_TRUE(node.Request(service, req, timeout, res, result));
+  EXPECT_TRUE(result);
+  EXPECT_TRUE(res.data());
+  // Run an iteration and check it was created
+  server->Run(true, 1, false);
+  EXPECT_TRUE(this->server->EntityByName("new_model_name").has_value());
+  EXPECT_NE(kNullEntity, this->server->EntityByName("new_model_name"));
+
+  const std::string worldGenSdfRes = this->RequestGeneratedSdf();
+  sdf::Root root;
+  sdf::Errors err = root.LoadSdfString(worldGenSdfRes);
+  EXPECT_TRUE(err.empty());
+  auto *world = root.WorldByIndex(0);
+  ASSERT_NE(nullptr, world);
+  EXPECT_TRUE(world->ModelNameExists("new_model_name"));
+}