Prepare GuiRunner to be made private (#567)

Signed-off-by: Louise Poubel <[email protected]> Co-authored-by: Carlos Agüero <[email protected]> Co-authored-by: Alejandro Hernández Cordero <[email protected]>
gazebosim · Feb 10, 2021 · 8f1f44f · 8f1f44f
1 parent 2bc062d
commit 8f1f44f
Show file tree

Hide file tree

Showing 5 changed files with 103 additions and 39 deletions.
diff --git a/include/ignition/gazebo/gui/GuiRunner.hh b/include/ignition/gazebo/gui/GuiRunner.hh
@@ -17,14 +17,15 @@
 #ifndef IGNITION_GAZEBO_GUI_GUIRUNNER_HH_
 #define IGNITION_GAZEBO_GUI_GUIRUNNER_HH_
 
-#include <ignition/msgs/serialized.pb.h>
+#include <ignition/msgs/serialized_map.pb.h>
 
 #include <QtCore>
+#include <memory>
 #include <string>
 
-#include <ignition/transport/Node.hh>
+#include <ignition/utils/ImplPtr.hh>
 
-#include "ignition/gazebo/EntityComponentManager.hh"
+#include "ignition/gazebo/config.hh"
 #include "ignition/gazebo/gui/Export.hh"
 
 namespace ignition
@@ -42,10 +43,9 @@ class IGNITION_GAZEBO_GUI_VISIBLE GuiRunner : public QObject
 
   /// \brief Constructor
   /// \param[in] _worldName World name.
-  public: explicit GuiRunner(const std::string &_worldName);
-
-  /// \brief Destructor
-  public: ~GuiRunner() override;
+  /// \todo Move to src/gui on v6.
+  public: explicit IGN_DEPRECATED(5.0) GuiRunner(
+      const std::string &_worldName);
 
   /// \brief Callback when a plugin has been added.
   /// \param[in] _objectName Plugin's object name.
@@ -62,17 +62,8 @@ class IGNITION_GAZEBO_GUI_VISIBLE GuiRunner : public QObject
   /// \param[in] _msg New state message.
   private: void OnState(const msgs::SerializedStepMap &_msg);
 
-  /// \brief Entity-component manager.
-  private: gazebo::EntityComponentManager ecm;
-
-  /// \brief Transport node.
-  private: transport::Node node;
-
-  /// \brief Topic to request state
-  private: std::string stateTopic;
-
-  /// \brief Latest update info
-  private: UpdateInfo updateInfo;
+  /// \brief Pointer to private data.
+  IGN_UTILS_UNIQUE_IMPL_PTR(dataPtr)
 };
 }
 }

diff --git a/src/cmd/ign.cc b/src/cmd/ign.cc
@@ -298,7 +298,20 @@ extern "C" int runGui(const char *_guiConfig)
   if (!executed || !result || worldsMsg.data().empty())
     return false;
 
+  // Remove warning suppression in v6
+#ifndef _WIN32
+# pragma GCC diagnostic push
+# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+#else
+# pragma warning(push)
+# pragma warning(disable: 4996)
+#endif
   std::vector<ignition::gazebo::GuiRunner *> runners;
+#ifndef _WIN32
+# pragma GCC diagnostic pop
+#else
+# pragma warning(pop)
+#endif
 
   // Configuration file from command line
   if (_guiConfig != nullptr && std::strlen(_guiConfig) > 0)
@@ -313,7 +326,21 @@ extern "C" int runGui(const char *_guiConfig)
     // TODO(anyone) Most of ign-gazebo's transport API includes the world name,
     // which makes it complicated to mix configurations across worlds.
     // We could have a way to use world-agnostic topics like Gazebo-classic's ~
+
+    // Remove warning suppression in v6
+#ifndef _WIN32
+# pragma GCC diagnostic push
+# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+#else
+# pragma warning(push)
+# pragma warning(disable: 4996)
+#endif
     auto runner = new ignition::gazebo::GuiRunner(worldsMsg.data(0));
+#ifndef _WIN32
+# pragma GCC diagnostic pop
+#else
+# pragma warning(pop)
+#endif
     runner->connect(&app, &ignition::gui::Application::PluginAdded, runner,
         &ignition::gazebo::GuiRunner::OnPluginAdded);
     runners.push_back(runner);

diff --git a/src/gui/CMakeLists.txt b/src/gui/CMakeLists.txt
@@ -37,6 +37,7 @@ target_link_libraries(${gui_target}
     ignition-common${IGN_COMMON_VER}::ignition-common${IGN_COMMON_VER}
     ignition-gui${IGN_GUI_VER}::ignition-gui${IGN_GUI_VER}
     ignition-transport${IGN_TRANSPORT_VER}::ignition-transport${IGN_TRANSPORT_VER}
+    ignition-utils${IGN_UTILS_VER}::ignition-utils${IGN_UTILS_VER}
     ${Qt5Core_LIBRARIES}
     ${Qt5Widgets_LIBRARIES}
 )

diff --git a/src/gui/Gui.cc b/src/gui/Gui.cc
@@ -170,7 +170,20 @@ std::unique_ptr<ignition::gui::Application> createGui(
     // TODO(anyone) Most of ign-gazebo's transport API includes the world name,
     // which makes it complicated to mix configurations across worlds.
     // We could have a way to use world-agnostic topics like Gazebo-classic's ~
+    // Remove warning suppression in v6
+#ifndef _WIN32
+# pragma GCC diagnostic push
+# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+#else
+# pragma warning(push)
+# pragma warning(disable: 4996)
+#endif
     auto runner = new ignition::gazebo::GuiRunner(worldsMsg.data(0));
+#ifndef _WIN32
+# pragma GCC diagnostic pop
+#else
+# pragma warning(pop)
+#endif
     runner->connect(app.get(), &ignition::gui::Application::PluginAdded, runner,
         &ignition::gazebo::GuiRunner::OnPluginAdded);
     ++runnerCount;
@@ -221,7 +234,20 @@ std::unique_ptr<ignition::gui::Application> createGui(
       }
 
       // GUI runner
+      // Remove warning suppression in v6
+#ifndef _WIN32
+# pragma GCC diagnostic push
+# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+#else
+# pragma warning(push)
+# pragma warning(disable: 4996)
+#endif
       auto runner = new ignition::gazebo::GuiRunner(worldName);
+#ifndef _WIN32
+# pragma GCC diagnostic pop
+#else
+# pragma warning(pop)
+#endif
       runner->connect(app.get(), &ignition::gui::Application::PluginAdded,
                       runner, &ignition::gazebo::GuiRunner::OnPluginAdded);
       runner->setParent(ignition::gui::App());

diff --git a/src/gui/GuiRunner.cc b/src/gui/GuiRunner.cc
@@ -20,18 +20,37 @@
 #include <ignition/fuel_tools/Interface.hh>
 #include <ignition/gui/Application.hh>
 #include <ignition/gui/MainWindow.hh>
+#include <ignition/transport/Node.hh>
 
 // Include all components so they have first-class support
 #include "ignition/gazebo/components/components.hh"
 #include "ignition/gazebo/Conversions.hh"
+#include "ignition/gazebo/EntityComponentManager.hh"
 #include "ignition/gazebo/gui/GuiRunner.hh"
 #include "ignition/gazebo/gui/GuiSystem.hh"
 
 using namespace ignition;
 using namespace gazebo;
 
+/////////////////////////////////////////////////
+class ignition::gazebo::GuiRunner::Implementation
+{
+  /// \brief Entity-component manager.
+  public: gazebo::EntityComponentManager ecm;
+
+  /// \brief Transport node.
+  public: transport::Node node{};
+
+  /// \brief Topic to request state
+  public: std::string stateTopic;
+
+  /// \brief Latest update info
+  public: UpdateInfo updateInfo;
+};
+
 /////////////////////////////////////////////////
 GuiRunner::GuiRunner(const std::string &_worldName)
+  : dataPtr(utils::MakeUniqueImpl<Implementation>())
 {
   this->setProperty("worldName", QString::fromStdString(_worldName));
 
@@ -40,9 +59,9 @@ GuiRunner::GuiRunner(const std::string &_worldName)
   winWorldNames.append(QString::fromStdString(_worldName));
   win->setProperty("worldNames", winWorldNames);
 
-  this->stateTopic = transport::TopicUtils::AsValidTopic("/world/" +
+  this->dataPtr->stateTopic = transport::TopicUtils::AsValidTopic("/world/" +
       _worldName + "/state");
-  if (this->stateTopic.empty())
+  if (this->dataPtr->stateTopic.empty())
   {
     ignerr << "Failed to generate valid topic for world [" << _worldName << "]"
            << std::endl;
@@ -54,22 +73,19 @@ GuiRunner::GuiRunner(const std::string &_worldName)
     return fuel_tools::fetchResource(_uri.Str());
   });
 
-  igndbg << "Requesting initial state from [" << this->stateTopic << "]..."
-         << std::endl;
+  igndbg << "Requesting initial state from [" << this->dataPtr->stateTopic
+         << "]..." << std::endl;
 
   this->RequestState();
 }
 
-/////////////////////////////////////////////////
-GuiRunner::~GuiRunner() = default;
-
 /////////////////////////////////////////////////
 void GuiRunner::RequestState()
 {
   // set up service for async state response callback
   std::string id = std::to_string(gui::App()->applicationPid());
   std::string reqSrv =
-      this->node.Options().NameSpace() + "/" + id + "/state_async";
+      this->dataPtr->node.Options().NameSpace() + "/" + id + "/state_async";
   auto reqSrvValid = transport::TopicUtils::AsValidTopic(reqSrv);
   if (reqSrvValid.empty())
   {
@@ -79,12 +95,12 @@ void GuiRunner::RequestState()
   }
   reqSrv = reqSrvValid;
 
-  this->node.Advertise(reqSrv, &GuiRunner::OnStateAsyncService, this);
+  this->dataPtr->node.Advertise(reqSrv, &GuiRunner::OnStateAsyncService, this);
   ignition::msgs::StringMsg req;
   req.set_data(reqSrv);
 
   // send async state request
-  this->node.Request(this->stateTopic + "_async", req);
+  this->dataPtr->node.Request(this->dataPtr->stateTopic + "_async", req);
 }
 
 /////////////////////////////////////////////////
@@ -98,7 +114,7 @@ void GuiRunner::OnPluginAdded(const QString &_objectName)
     return;
   }
 
-  plugin->Update(this->updateInfo, this->ecm);
+  plugin->Update(this->dataPtr->updateInfo, this->dataPtr->ecm);
 }
 
 /////////////////////////////////////////////////
@@ -110,12 +126,15 @@ void GuiRunner::OnStateAsyncService(const msgs::SerializedStepMap &_res)
   // and in RequestState()
   std::string id = std::to_string(gui::App()->applicationPid());
   std::string reqSrv =
-      this->node.Options().NameSpace() + "/" + id + "/state_async";
-  this->node.UnadvertiseSrv(reqSrv);
+      this->dataPtr->node.Options().NameSpace() + "/" + id + "/state_async";
+  this->dataPtr->node.UnadvertiseSrv(reqSrv);
 
   // Only subscribe to periodic updates after receiving initial state
-  if (this->node.SubscribedTopics().empty())
-    this->node.Subscribe(this->stateTopic, &GuiRunner::OnState, this);
+  if (this->dataPtr->node.SubscribedTopics().empty())
+  {
+    this->dataPtr->node.Subscribe(this->dataPtr->stateTopic,
+        &GuiRunner::OnState, this);
+  }
 }
 
 /////////////////////////////////////////////////
@@ -124,17 +143,17 @@ void GuiRunner::OnState(const msgs::SerializedStepMap &_msg)
   IGN_PROFILE_THREAD_NAME("GuiRunner::OnState");
   IGN_PROFILE("GuiRunner::Update");
 
-  this->ecm.SetState(_msg.state());
+  this->dataPtr->ecm.SetState(_msg.state());
 
   // Update all plugins
-  this->updateInfo = convert<UpdateInfo>(_msg.stats());
+  this->dataPtr->updateInfo = convert<UpdateInfo>(_msg.stats());
   auto plugins = gui::App()->findChildren<GuiSystem *>();
   for (auto plugin : plugins)
   {
-    plugin->Update(this->updateInfo, this->ecm);
+    plugin->Update(this->dataPtr->updateInfo, this->dataPtr->ecm);
   }
-  this->ecm.ClearNewlyCreatedEntities();
-  this->ecm.ProcessRemoveEntityRequests();
-  this->ecm.ClearRemovedComponents();
+  this->dataPtr->ecm.ClearNewlyCreatedEntities();
+  this->dataPtr->ecm.ProcessRemoveEntityRequests();
+  this->dataPtr->ecm.ClearRemovedComponents();
 }