address feedback & added user_commands test

Signed-off-by: Jenn Nguyen <[email protected]>

src/EntityComponentManager.cc

@@ -499,9 +499,10 @@ Entity EntityComponentManager::CloneImpl(Entity _entity, Entity _parent,
  {
  // When a cloned entity is removed, it erases all components/data so a new
  // cloned entity should not have components to be updated
- ignerr << "The component's data needs to be updated but this is not "
- << "expected. Please submit an issue including your use case "
- << "so that this can be resolved." << std::endl;
+ // LCOV_EXCL_START
+ ignerr << "Internal error: The component's data needs to be updated but "
+ << "this should not happen." << std::endl;
+ // LCOV_EXCL_STOP
  }
  }
 

src/systems/user_commands/UserCommands.cc

@@ -1824,17 +1824,28 @@ bool VisualCommand::Execute()
  }
 
  Entity visualEntity = kNullEntity;
- std::string entityName = visualMsg->name();
- if (!entityName.empty())
+ if (visualMsg->id() != kNullEntity)
  {
- visualEntity =
- this->iface->ecm->EntityByComponents(components::Name(entityName));
+ visualEntity = visualMsg->id();
  }
- else if (visualMsg->id() != kNullEntity)
+ else if (!visualMsg->name().empty() && !visualMsg->parent_name().empty())
  {
- visualEntity = visualMsg->id();
+ Entity parentEntity =
+ this->iface->ecm->EntityByComponents(
+ components::Name(visualMsg->parent_name()));
+
+ auto entities =
+ this->iface->ecm->ChildrenByComponents(parentEntity,
+ components::Name(visualMsg->name()));
+
+ // When size > 1, we don't know which entity to modify
+ if (entities.size() == 1)
+ {
+ visualEntity = entities[0];
+ }
  }
- else
+
+ if (visualEntity == kNullEntity)
  {
  ignerr << "Failed to find visual entity" << std::endl;
  return false;

test/integration/user_commands.cc

@@ -20,6 +20,7 @@
 #include <ignition/msgs/entity_factory.pb.h>
 #include <ignition/msgs/light.pb.h>
 #include <ignition/msgs/physics.pb.h>
+#include <ignition/msgs/visual.pb.h>
 
 #include <ignition/common/Console.hh>
 #include <ignition/common/Util.hh>
@@ -29,10 +30,12 @@
 
 #include "ignition/gazebo/components/Light.hh"
 #include "ignition/gazebo/components/Link.hh"
+#include "ignition/gazebo/components/Material.hh"
 #include "ignition/gazebo/components/Model.hh"
 #include "ignition/gazebo/components/Name.hh"
 #include "ignition/gazebo/components/Physics.hh"
 #include "ignition/gazebo/components/Pose.hh"
+#include "ignition/gazebo/components/VisualCmd.hh"
 #include "ignition/gazebo/components/WheelSlipCmd.hh"
 #include "ignition/gazebo/components/World.hh"
 #include "ignition/gazebo/Model.hh"
@@ -1205,4 +1208,91 @@ TEST_F(UserCommandsTest, IGN_UTILS_TEST_DISABLED_ON_WIN32(WheelSlip))
  // Run two iterations, in the first one the WheelSlipCmd component is created
  // and processed.
  // The second one is just to check everything went fine.
- server.Run(true, 3, false);}
+ server.Run(true, 3, false);
+}
+
+/////////////////////////////////////////////////
+TEST_F(UserCommandsTest, IGN_UTILS_TEST_DISABLED_ON_WIN32(Visual))
+{
+ // Start server
+ ServerConfig serverConfig;
+ const auto sdfFile = common::joinPaths(
+ std::string(PROJECT_SOURCE_PATH), "test", "worlds", "shapes.sdf");
+ serverConfig.SetSdfFile(sdfFile);
+
+ Server server(serverConfig);
+ EXPECT_FALSE(server.Running());
+ EXPECT_FALSE(*server.Running(0));
+
+ // Create a system just to get the ECM
+ EntityComponentManager *ecm{nullptr};
+ test::Relay testSystem;
+ testSystem.OnPreUpdate([&](const gazebo::UpdateInfo &,
+ gazebo::EntityComponentManager &_ecm)
+ {
+ ecm = &_ecm;
+ });
+
+ server.AddSystem(testSystem.systemPtr);
+
+ // Run server and check we have the ECM
+ EXPECT_EQ(nullptr, ecm);
+ server.Run(true, 1, false);
+ ASSERT_NE(nullptr, ecm);
+
+ msgs::Visual req;
+ msgs::Boolean res;
+ transport::Node node;
+ bool result;
+ unsigned int timeout = 100;
+ std::string service{"/world/default/visual_config"};
+
+ auto boxVisualEntity =
+ ecm->EntityByComponents(components::Name("box_visual"));
+ ASSERT_NE(kNullEntity, boxVisualEntity);
+
+ // check box visual's initial values
+ auto boxVisualComp = ecm->Component<components::Material>(boxVisualEntity);
+ ASSERT_NE(nullptr, boxVisualComp);
+ EXPECT_EQ(math::Color(1.0f, 0.0f, 0.0f, 1.0f),
+ boxVisualComp->Data().Diffuse());
+
+ msgs::Set(req.mutable_material()->mutable_diffuse(),
+ math::Color(0.0f, 1.0f, 0.0f, 1.0f));
+
+ // This will fail to find the entity in VisualCommand::Execute()
+ // since no id was provided
+ EXPECT_TRUE(node.Request(service, req, timeout, res, result));
+ server.Run(true, 1, false);
+ // check that the VisualCmd component was not created
+ auto boxVisCmdComp = ecm->Component<components::VisualCmd>(boxVisualEntity);
+ EXPECT_EQ(nullptr, boxVisCmdComp);
+
+ // add id to msg and resend request
+ req.set_id(boxVisualEntity);
+ EXPECT_TRUE(node.Request(service, req, timeout, res, result));
+ server.Run(true, 1, false);
+ // check the VisualCmd was created and check the values
+ boxVisCmdComp = ecm->Component<components::VisualCmd>(boxVisualEntity);
+ ASSERT_NE(nullptr, boxVisualComp);
+ EXPECT_FLOAT_EQ(0.0f, boxVisCmdComp->Data().material().diffuse().r());
+ EXPECT_FLOAT_EQ(1.0f, boxVisCmdComp->Data().material().diffuse().g());
+ EXPECT_FLOAT_EQ(0.0f, boxVisCmdComp->Data().material().diffuse().b());
+ EXPECT_FLOAT_EQ(1.0f, boxVisCmdComp->Data().material().diffuse().a());
+
+ // update component using visual name and parent name
+ req.Clear();
+ req.set_name("box_visual");
+ req.set_parent_name("box_link");
+ msgs::Set(req.mutable_material()->mutable_diffuse(),
+ math::Color(0.0f, 0.0f, 1.0f, 1.0f));
+ EXPECT_TRUE(node.Request(service, req, timeout, res, result));
+ server.Run(true, 1, false);
+ // check the values
+ boxVisCmdComp = ecm->Component<components::VisualCmd>(boxVisualEntity);
+ ASSERT_NE(nullptr, boxVisualComp);
+ EXPECT_FLOAT_EQ(0.0f, boxVisCmdComp->Data().material().diffuse().r());
+ EXPECT_FLOAT_EQ(0.0f, boxVisCmdComp->Data().material().diffuse().g());
+ EXPECT_FLOAT_EQ(1.0f, boxVisCmdComp->Data().material().diffuse().b());
+ EXPECT_FLOAT_EQ(1.0f, boxVisCmdComp->Data().material().diffuse().a());
+}