Fix plugin added signal, add PluginByName (#249)

Signed-off-by: Louise Poubel <louise@openrobotics.org>

Migration.md

@@ -5,6 +5,11 @@ Deprecated code produces compile-time warnings. These warning serve as
 notification to users that their code should be upgraded. The next major
 release will remove the deprecated code.
 
+## Ignition GUI 3.6 to 3.7
+
+* The `Application::PluginAdded` signal used to send empty strings. Now it
+  sends the plugin's unique name.
+
 ## Ignition GUI 2.x to 3.x
 
 * Use rendering3, transport8 and msgs5.

include/ignition/gui/Application.hh

@@ -150,10 +150,18 @@ namespace ignition
       /// \brief Remove plugin by name. The plugin is removed from the
       /// application and its shared library unloaded if this was its last
       /// instance.
-      /// \param[in] _pluginName Plugn instance's unique name
+      /// \param[in] _pluginName Plugn instance's unique name. This is the
+      /// plugin card's object name.
       /// \return True if successful
       public: bool RemovePlugin(const std::string &_pluginName);
 
+      /// \brief Get a plugin by its unique name.
+      /// \param[in] _pluginName Plugn instance's unique name. This is the
+      /// plugin card's object name.
+      /// \return Pointer to plugin object, null if not found.
+      public: std::shared_ptr<Plugin> PluginByName(
+          const std::string &_pluginName) const;
+
       /// \brief Notify that a plugin has been added.
       /// \param[in] _objectName Plugin's object name.
       signals: void PluginAdded(const QString &_objectName);

src/Application.cc

@@ -179,36 +179,30 @@ Application *ignition::gui::App()
 /////////////////////////////////////////////////
 bool Application::RemovePlugin(const std::string &_pluginName)
 {
-  bool found{false};
-  for (auto plugin : this->dataPtr->pluginsAdded)
-  {
-    auto cardItem = plugin->CardItem();
-    if (!cardItem)
-      continue;
-
-    if (cardItem->objectName().toStdString() == _pluginName)
-    {
-      // Remove on QML
-      cardItem->deleteLater();
+  auto plugin = this->PluginByName(_pluginName);
+  if (nullptr == plugin)
+    return false;
 
-      // Remove split on QML
-      auto bgItem = this->dataPtr->mainWin->QuickWindow()
-          ->findChild<QQuickItem *>("background");
-      if (bgItem)
-      {
-        QMetaObject::invokeMethod(bgItem, "removeSplitItem",
-            Q_ARG(QVariant, cardItem->parentItem()->objectName()));
-      }
+  auto cardItem = plugin->CardItem();
+  if (nullptr == cardItem)
+    return false;
 
-      // Unload shared library
-      this->RemovePlugin(plugin);
+  // Remove on QML
+  cardItem->deleteLater();
 
-      found = true;
-      break;
-    }
+  // Remove split on QML
+  auto bgItem = this->dataPtr->mainWin->QuickWindow()
+      ->findChild<QQuickItem *>("background");
+  if (bgItem)
+  {
+    QMetaObject::invokeMethod(bgItem, "removeSplitItem",
+        Q_ARG(QVariant, cardItem->parentItem()->objectName()));
   }
 
-  return found;
+  // Unload shared library
+  this->RemovePlugin(plugin);
+
+  return true;
 }
 
 /////////////////////////////////////////////////
@@ -402,13 +396,31 @@ bool Application::LoadPlugin(const std::string &_filename,
   else
     this->InitializeDialogs();
 
-  this->PluginAdded(plugin->objectName());
+  this->PluginAdded(plugin->CardItem()->objectName());
   ignmsg << "Loaded plugin [" << _filename << "] from path [" << pathToLib
          << "]" << std::endl;
 
   return true;
 }
 
+/////////////////////////////////////////////////
+std::shared_ptr<Plugin> Application::PluginByName(
+    const std::string &_pluginName) const
+{
+  for (auto &plugin : this->dataPtr->pluginsAdded)
+  {
+    auto cardItem = plugin->CardItem();
+    if (!cardItem)
+      continue;
+
+    if (cardItem->objectName().toStdString() != _pluginName)
+      continue;
+
+    return plugin;
+  }
+  return nullptr;
+}
+
 /////////////////////////////////////////////////
 bool Application::InitializeMainWindow()
 {

src/Application_TEST.cc

@@ -82,14 +82,32 @@ TEST(ApplicationTest, IGN_UTILS_TEST_DISABLED_ON_WIN32(LoadPlugin))
     Application app(g_argc, g_argv);
 
     EXPECT_FALSE(app.LoadPlugin("_doesnt_exist"));
+    EXPECT_FALSE(app.RemovePlugin("_doesnt_exist"));
   }
 
   // Plugin path added programmatically
   {
     Application app(g_argc, g_argv);
+
+    std::string pluginName;
+    app.connect(&app, &Application::PluginAdded, [&pluginName](
+        const QString &_pluginName)
+    {
+      pluginName = _pluginName.toStdString();
+    });
+
     app.AddPluginPath(std::string(PROJECT_BINARY_PATH) + "/lib");
 
     EXPECT_TRUE(app.LoadPlugin("TestPlugin"));
+    EXPECT_EQ(0u, pluginName.find("plugin"));
+
+    auto plugin = app.PluginByName(pluginName);
+    ASSERT_NE(nullptr, plugin);
+    ASSERT_NE(nullptr, plugin->CardItem());
+
+    EXPECT_EQ(pluginName, plugin->CardItem()->objectName().toStdString());
+
+    EXPECT_TRUE(app.RemovePlugin(pluginName));
   }
 
   // Plugin path added by env var