From fe65126709c7d72d82143265b36bb60d9dc4e4fa Mon Sep 17 00:00:00 2001 From: Louise Poubel Date: Fri, 15 Jan 2021 14:26:02 -0800 Subject: [PATCH 1/6] Prepare GuiRunner to be made private Signed-off-by: Louise Poubel --- include/ignition/gazebo/gui/GuiRunner.hh | 18 ++++---- src/cmd/ign.cc | 27 ++++++++++++ src/gui/Gui.cc | 26 ++++++++++++ src/gui/GuiRunner.cc | 52 ++++++++++++++++-------- 4 files changed, 96 insertions(+), 27 deletions(-) diff --git a/include/ignition/gazebo/gui/GuiRunner.hh b/include/ignition/gazebo/gui/GuiRunner.hh index a60f77274c..8bf5b19576 100644 --- a/include/ignition/gazebo/gui/GuiRunner.hh +++ b/include/ignition/gazebo/gui/GuiRunner.hh @@ -20,6 +20,7 @@ #include #include +#include #include #include @@ -33,6 +34,7 @@ namespace gazebo { // Inline bracket to help doxygen filtering. inline namespace IGNITION_GAZEBO_VERSION_NAMESPACE { +class GuiRunnerPrivate; /// \brief Responsible for running GUI systems as new states are received from /// the backend. class IGNITION_GAZEBO_VISIBLE GuiRunner : public QObject @@ -41,7 +43,9 @@ class IGNITION_GAZEBO_VISIBLE GuiRunner : public QObject /// \brief Constructor /// \param[in] _worldName World name. - public: explicit GuiRunner(const std::string &_worldName); + /// \todo Move to src/gui on v6. + public: explicit IGN_DEPRECATED(5.0) GuiRunner( + const std::string &_worldName); /// \brief Destructor public: ~GuiRunner() override; @@ -61,17 +65,9 @@ class IGNITION_GAZEBO_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. + private: std::unique_ptr dataPtr; }; } } diff --git a/src/cmd/ign.cc b/src/cmd/ign.cc index a4a58c129b..145ccec843 100644 --- a/src/cmd/ign.cc +++ b/src/cmd/ign.cc @@ -298,7 +298,20 @@ extern "C" IGNITION_GAZEBO_VISIBLE 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 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" IGNITION_GAZEBO_VISIBLE 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/Gui.cc b/src/gui/Gui.cc index b375e912fe..c9c3380311 100644 --- a/src/gui/Gui.cc +++ b/src/gui/Gui.cc @@ -170,7 +170,20 @@ std::unique_ptr 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; @@ -208,7 +221,20 @@ std::unique_ptr createGui( ignerr << "Service call failed for [" << service << "]" << std::endl; // 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 index 565838a06c..ad6c35619b 100644 --- a/src/gui/GuiRunner.cc +++ b/src/gui/GuiRunner.cc @@ -30,8 +30,25 @@ using namespace ignition; using namespace gazebo; +///////////////////////////////////////////////// +class ignition::gazebo::GuiRunnerPrivate +{ + /// \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(std::make_unique()) { this->setProperty("worldName", QString::fromStdString(_worldName)); @@ -40,14 +57,14 @@ GuiRunner::GuiRunner(const std::string &_worldName) winWorldNames.append(QString::fromStdString(_worldName)); win->setProperty("worldNames", winWorldNames); - this->stateTopic = "/world/" + _worldName + "/state"; + this->dataPtr->stateTopic = "/world/" + _worldName + "/state"; common::addFindFileURICallback([] (common::URI _uri) { return fuel_tools::fetchResource(_uri.Str()); }); - igndbg << "Requesting initial state from [" << this->stateTopic << "]..." + igndbg << "Requesting initial state from [" << this->dataPtr->stateTopic << "]..." << std::endl; this->RequestState(); @@ -62,13 +79,13 @@ 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->node.Advertise(reqSrv, &GuiRunner::OnStateAsyncService, this); + this->dataPtr->node.Options().NameSpace() + "/" + id + "/state_async"; + 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); } ///////////////////////////////////////////////// @@ -82,7 +99,7 @@ void GuiRunner::OnPluginAdded(const QString &_objectName) return; } - plugin->Update(this->updateInfo, this->ecm); + plugin->Update(this->dataPtr->updateInfo, this->dataPtr->ecm); } ///////////////////////////////////////////////// @@ -94,12 +111,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); + } } ///////////////////////////////////////////////// @@ -108,17 +128,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(_msg.stats()); + this->dataPtr->updateInfo = convert(_msg.stats()); auto plugins = gui::App()->findChildren(); 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(); } From 66a219238d16cfb2b7612ae564efec18422722d0 Mon Sep 17 00:00:00 2001 From: Louise Poubel Date: Tue, 19 Jan 2021 17:55:10 -0800 Subject: [PATCH 2/6] move includes Signed-off-by: Louise Poubel --- include/ignition/gazebo/gui/GuiRunner.hh | 3 --- src/gui/GuiRunner.cc | 2 ++ 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/include/ignition/gazebo/gui/GuiRunner.hh b/include/ignition/gazebo/gui/GuiRunner.hh index 8bf5b19576..c4440a5db6 100644 --- a/include/ignition/gazebo/gui/GuiRunner.hh +++ b/include/ignition/gazebo/gui/GuiRunner.hh @@ -23,9 +23,6 @@ #include #include -#include - -#include "ignition/gazebo/EntityComponentManager.hh" #include "ignition/gazebo/Export.hh" namespace ignition diff --git a/src/gui/GuiRunner.cc b/src/gui/GuiRunner.cc index ad6c35619b..b8984decc3 100644 --- a/src/gui/GuiRunner.cc +++ b/src/gui/GuiRunner.cc @@ -20,10 +20,12 @@ #include #include #include +#include // 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" From df637e612fd653091792c3ddedee34eee1f3e836 Mon Sep 17 00:00:00 2001 From: Louise Poubel Date: Wed, 3 Feb 2021 13:47:26 -0800 Subject: [PATCH 3/6] codecheck Signed-off-by: Louise Poubel --- src/gui/GuiRunner.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gui/GuiRunner.cc b/src/gui/GuiRunner.cc index b01296a344..0757526baf 100644 --- a/src/gui/GuiRunner.cc +++ b/src/gui/GuiRunner.cc @@ -73,8 +73,8 @@ GuiRunner::GuiRunner(const std::string &_worldName) return fuel_tools::fetchResource(_uri.Str()); }); - igndbg << "Requesting initial state from [" << this->dataPtr->stateTopic << "]..." - << std::endl; + igndbg << "Requesting initial state from [" << this->dataPtr->stateTopic + << "]..." << std::endl; this->RequestState(); } From b34117fe60e8c6d159225fd65137c7ec7c96811e Mon Sep 17 00:00:00 2001 From: Louise Poubel Date: Mon, 8 Feb 2021 20:58:30 -0800 Subject: [PATCH 4/6] fix include Signed-off-by: Louise Poubel --- include/ignition/gazebo/gui/GuiRunner.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/ignition/gazebo/gui/GuiRunner.hh b/include/ignition/gazebo/gui/GuiRunner.hh index 02b4fecfc9..3bb5ad8449 100644 --- a/include/ignition/gazebo/gui/GuiRunner.hh +++ b/include/ignition/gazebo/gui/GuiRunner.hh @@ -17,7 +17,7 @@ #ifndef IGNITION_GAZEBO_GUI_GUIRUNNER_HH_ #define IGNITION_GAZEBO_GUI_GUIRUNNER_HH_ -#include +#include #include #include From ae049d031469b1c0ecfd9193052d492314e78d95 Mon Sep 17 00:00:00 2001 From: Louise Poubel Date: Tue, 9 Feb 2021 10:38:13 -0800 Subject: [PATCH 5/6] Use ign-utils implptr Signed-off-by: Louise Poubel --- include/ignition/gazebo/gui/GuiRunner.hh | 10 ++++------ src/gui/CMakeLists.txt | 1 + src/gui/GuiRunner.cc | 7 ++----- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/include/ignition/gazebo/gui/GuiRunner.hh b/include/ignition/gazebo/gui/GuiRunner.hh index 3bb5ad8449..f808f5674b 100644 --- a/include/ignition/gazebo/gui/GuiRunner.hh +++ b/include/ignition/gazebo/gui/GuiRunner.hh @@ -23,6 +23,9 @@ #include #include +#include + +#include "ignition/gazebo/config.hh" #include "ignition/gazebo/gui/Export.hh" namespace ignition @@ -31,7 +34,6 @@ namespace gazebo { // Inline bracket to help doxygen filtering. inline namespace IGNITION_GAZEBO_VERSION_NAMESPACE { -class GuiRunnerPrivate; /// \brief Responsible for running GUI systems as new states are received from /// the backend. @@ -45,9 +47,6 @@ class IGNITION_GAZEBO_GUI_VISIBLE GuiRunner : public QObject public: explicit IGN_DEPRECATED(5.0) GuiRunner( const std::string &_worldName); - /// \brief Destructor - public: ~GuiRunner() override; - /// \brief Callback when a plugin has been added. /// \param[in] _objectName Plugin's object name. public slots: void OnPluginAdded(const QString &_objectName); @@ -63,9 +62,8 @@ class IGNITION_GAZEBO_GUI_VISIBLE GuiRunner : public QObject /// \param[in] _msg New state message. private: void OnState(const msgs::SerializedStepMap &_msg); - /// \brief Pointer to private data. - private: std::unique_ptr dataPtr; + IGN_UTILS_UNIQUE_IMPL_PTR(dataPtr) }; } } diff --git a/src/gui/CMakeLists.txt b/src/gui/CMakeLists.txt index 2cec87fd48..190e1444cd 100644 --- 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/GuiRunner.cc b/src/gui/GuiRunner.cc index 0757526baf..1654c4c5d6 100644 --- a/src/gui/GuiRunner.cc +++ b/src/gui/GuiRunner.cc @@ -33,7 +33,7 @@ using namespace ignition; using namespace gazebo; ///////////////////////////////////////////////// -class ignition::gazebo::GuiRunnerPrivate +class ignition::gazebo::GuiRunner::Implementation { /// \brief Entity-component manager. public: gazebo::EntityComponentManager ecm; @@ -50,7 +50,7 @@ class ignition::gazebo::GuiRunnerPrivate ///////////////////////////////////////////////// GuiRunner::GuiRunner(const std::string &_worldName) - : dataPtr(std::make_unique()) + : dataPtr(utils::MakeUniqueImpl()) { this->setProperty("worldName", QString::fromStdString(_worldName)); @@ -79,9 +79,6 @@ GuiRunner::GuiRunner(const std::string &_worldName) this->RequestState(); } -///////////////////////////////////////////////// -GuiRunner::~GuiRunner() = default; - ///////////////////////////////////////////////// void GuiRunner::RequestState() { From 7d190e2cf1fe193d6dde35aff8d4fdb56962f14c Mon Sep 17 00:00:00 2001 From: Louise Poubel Date: Tue, 9 Feb 2021 16:08:21 -0800 Subject: [PATCH 6/6] call constructor? Signed-off-by: Louise Poubel --- src/gui/GuiRunner.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gui/GuiRunner.cc b/src/gui/GuiRunner.cc index 1654c4c5d6..96d8aa35a2 100644 --- a/src/gui/GuiRunner.cc +++ b/src/gui/GuiRunner.cc @@ -39,7 +39,7 @@ class ignition::gazebo::GuiRunner::Implementation public: gazebo::EntityComponentManager ecm; /// \brief Transport node. - public: transport::Node node; + public: transport::Node node{}; /// \brief Topic to request state public: std::string stateTopic;