TransportSceneManager: Prevent freeze when inserted from menu (#365)

Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Alejandro Hernández Cordero <[email protected]>

Co-authored-by: Alejandro Hernández Cordero <[email protected]>

src/plugins/transport_scene_manager/TransportSceneManager.cc

@@ -19,8 +19,11 @@
 #include <map>
 #include <sstream>
 #include <string>
+#include <thread>
 #include <vector>
 
+#include <QQmlProperty>
+
 #include <ignition/common/Console.hh>
 #include <ignition/common/MeshManager.hh>
 #include <ignition/math/Pose3.hh>
@@ -44,6 +47,7 @@
 #endif
 
 #include <ignition/transport/Node.hh>
+#include <ignition/transport/TopicUtils.hh>
 
 #include "ignition/gui/Application.hh"
 #include "ignition/gui/Conversions.hh"
@@ -124,16 +128,16 @@ class ignition::gui::plugins::TransportSceneManagerPrivate
  public: void DeleteEntity(const unsigned int _entity);
 
  //// \brief Ign-transport scene service name
- public: std::string service;
+ public: std::string service{"scene"};
 
  //// \brief Ign-transport pose topic name
- public: std::string poseTopic;
+ public: std::string poseTopic{"pose"};
 
  //// \brief Ign-transport deletion topic name
- public: std::string deletionTopic;
+ public: std::string deletionTopic{"delete"};
 
  //// \brief Ign-transport scene topic name
- public: std::string sceneTopic;
+ public: std::string sceneTopic{"scene"};
 
  //// \brief Pointer to the rendering scene
  public: rendering::ScenePtr scene{nullptr};
@@ -165,6 +169,9 @@ class ignition::gui::plugins::TransportSceneManagerPrivate
  /// \brief Transport node for making service request and subscribing to
  /// pose topic
  public: ignition::transport::Node node;
+
+ /// \brief Thread to wait for transport initialization
+ public: std::thread initializeTransport;
 };
 
 using namespace ignition;
@@ -180,6 +187,8 @@ TransportSceneManager::TransportSceneManager()
 /////////////////////////////////////////////////
 TransportSceneManager::~TransportSceneManager()
 {
+ if (this->dataPtr->initializeTransport.joinable())
+ this->dataPtr->initializeTransport.join();
 }
 
 /////////////////////////////////////////////////
@@ -194,29 +203,56 @@ void TransportSceneManager::LoadConfig(const tinyxml2::XMLElement *_pluginElem)
  auto elem = _pluginElem->FirstChildElement("service");
  if (nullptr != elem && nullptr != elem->GetText())
  {
- this->dataPtr->service = elem->GetText();
+ this->dataPtr->service =
+ transport::TopicUtils::AsValidTopic(elem->GetText());
  }
 
  elem = _pluginElem->FirstChildElement("pose_topic");
  if (nullptr != elem && nullptr != elem->GetText())
  {
- this->dataPtr->poseTopic = elem->GetText();
+ this->dataPtr->poseTopic =
+ transport::TopicUtils::AsValidTopic(elem->GetText());
  }
 
  elem = _pluginElem->FirstChildElement("deletion_topic");
  if (nullptr != elem && nullptr != elem->GetText())
  {
- this->dataPtr->deletionTopic = elem->GetText();
+ this->dataPtr->deletionTopic =
+ transport::TopicUtils::AsValidTopic(elem->GetText());
  }
 
  elem = _pluginElem->FirstChildElement("scene_topic");
  if (nullptr != elem && nullptr != elem->GetText())
  {
- this->dataPtr->sceneTopic = elem->GetText();
+ this->dataPtr->sceneTopic =
+ transport::TopicUtils::AsValidTopic(elem->GetText());
  }
  }
 
- App()->findChild<MainWindow *>()->installEventFilter(this);
+ QQmlProperty::write(this->PluginItem(), "service",
+ QString::fromStdString(this->dataPtr->service));
+ QQmlProperty::write(this->PluginItem(), "poseTopic",
+ QString::fromStdString(this->dataPtr->poseTopic));
+ QQmlProperty::write(this->PluginItem(), "deletionTopic",
+ QString::fromStdString(this->dataPtr->deletionTopic));
+ QQmlProperty::write(this->PluginItem(), "sceneTopic",
+ QString::fromStdString(this->dataPtr->sceneTopic));
+
+ if (this->dataPtr->service.empty() ||
+ this->dataPtr->poseTopic.empty() ||
+ this->dataPtr->deletionTopic.empty() ||
+ this->dataPtr->sceneTopic.empty())
+ {
+ ignerr << "One or more transport parameters invalid:" << std::endl
+ << " * <service>: " << this->dataPtr->service << std::endl
+ << " * <pose_topic>: " << this->dataPtr->poseTopic << std::endl
+ << " * <deletion_topic>: " << this->dataPtr->deletionTopic << std::endl
+ << " * <scene_topic>: " << this->dataPtr->sceneTopic << std::endl;
+ }
+ else
+ {
+ App()->findChild<MainWindow *>()->installEventFilter(this);
+ }
 }
 
 /////////////////////////////////////////////////
@@ -288,13 +324,14 @@ void TransportSceneManagerPrivate::Request()
  if (publishers.size() > 0)
  break;
  std::this_thread::sleep_for(sleepDuration);
- igndbg << "Waiting for service " << this->service << "\n";
+ igndbg << "Waiting for service [" << this->service << "]\n";
  }
 
  if (publishers.empty() || !this->node.Request(this->service,
  &TransportSceneManagerPrivate::OnSceneSrvMsg, this))
  {
- ignerr << "Error making service request to " << this->service << std::endl;
+ ignerr << "Error making service request to [" << this->service << "]"
+ << std::endl;
  }
 }
 
@@ -334,7 +371,8 @@ void TransportSceneManagerPrivate::OnRender()
  if (nullptr == this->scene)
  return;
 
- this->InitializeTransport();
+ this->initializeTransport = std::thread(
+ &TransportSceneManagerPrivate::InitializeTransport, this);
  }
 
  std::lock_guard<std::mutex> lock(this->msgMutex);

src/plugins/transport_scene_manager/TransportSceneManager.hh

@@ -36,10 +36,13 @@ namespace plugins
  /// ## Configuration
  ///
  /// * \<service\> : Name of service where this system will request a scene
- /// message.
+ /// message. Optional, defaults to "/scene".
  /// * \<pose_topic\> : Name of topic to subscribe to receive pose updates.
+ /// Optional, defaults to "/pose".
  /// * \<deletion_topic\> : Name of topic to request entity deletions.
- /// * \<scene_topic\> : Name of topic to receive scene updates.
+ /// Optional, defaults to "/delete".
+ /// * \<scene_topic\> : Name of topic to receive scene updates. Optional,
+ /// defaults to "/scene".
  class TransportSceneManager : public Plugin
  {
  Q_OBJECT

src/plugins/transport_scene_manager/TransportSceneManager.qml

@@ -15,14 +15,37 @@
  *
 */
 
-import QtQuick 2.0
-import QtQuick.Controls 2.0
+import QtQuick 2.9
+import QtQuick.Controls 2.1
 import QtQuick.Layouts 1.3
 
-// TODO: remove invisible rectangle, see
-// https:/ignitionrobotics/ign-gui/issues/220
-Rectangle {
- visible: false
- Layout.minimumWidth: 100
- Layout.minimumHeight: 100
+GridLayout {
+ columns: 1
+ columnSpacing: 10
+ Layout.minimumWidth: 350
+ Layout.minimumHeight: 200
+ anchors.fill: parent
+ anchors.margins: 10
+
+ property string service: 'N/A'
+ property string poseTopic: 'N/A'
+ property string deletionTopic: 'N/A'
+ property string sceneTopic: 'N/A'
+
+ Label {
+ Layout.columnSpan: 1
+ Layout.fillWidth: true
+ wrapMode: Text.WordWrap
+ text: "<b>Service</b>: /" + service +
+ "<br><b>Pose topic</b>: /" + poseTopic +
+ "<br><b>Deletion topic</b>: /" + deletionTopic +
+ "<br><b>Scene topic</b>: /" + sceneTopic
+ }
+
+
+ Item {
+ Layout.columnSpan: 1
+ width: 10
+ Layout.fillHeight: true
+ }
 }

test/integration/transport_scene_manager.cc

@@ -162,15 +162,24 @@ TEST(MinimalSceneTest, IGN_UTILS_TEST_ENABLED_ONLY_ON_LINUX(Config))
  QCoreApplication::processEvents();
  sleep++;
  }
+ EXPECT_TRUE(sceneRequested);
+ EXPECT_LT(sleep, maxSleep);
 
  auto scene = engine->SceneByName("banana");
  ASSERT_NE(nullptr, scene);
 
  auto root = scene->RootVisual();
  ASSERT_NE(nullptr, root);
 
+ for (sleep = 0; root->ChildCount() < 2 && sleep < maxSleep; ++sleep)
+ {
+ std::this_thread::sleep_for(std::chrono::milliseconds(100));
+ QCoreApplication::processEvents();
+ }
+ EXPECT_LT(sleep, maxSleep);
+
  // Check scene is populated
- EXPECT_EQ(2u, root->ChildCount());
+ ASSERT_EQ(2u, root->ChildCount());
 
  // First child is user camera
  auto camera = std::dynamic_pointer_cast<rendering::Camera>(