From 74070591fd56623cdae27df2e36a1acfe55c522c Mon Sep 17 00:00:00 2001 From: Louise Poubel Date: Thu, 3 Mar 2022 14:40:36 -0800 Subject: [PATCH 1/3] TransportSceneManager: Prevent freeze when inserted from menu Signed-off-by: Louise Poubel --- .../TransportSceneManager.cc | 60 +++++++++++++++---- .../TransportSceneManager.hh | 7 ++- .../TransportSceneManager.qml | 39 +++++++++--- 3 files changed, 84 insertions(+), 22 deletions(-) diff --git a/src/plugins/transport_scene_manager/TransportSceneManager.cc b/src/plugins/transport_scene_manager/TransportSceneManager.cc index 47c2d5103..b8499f24b 100644 --- a/src/plugins/transport_scene_manager/TransportSceneManager.cc +++ b/src/plugins/transport_scene_manager/TransportSceneManager.cc @@ -19,8 +19,11 @@ #include #include #include +#include #include +#include + #include #include #include @@ -44,6 +47,7 @@ #endif #include +#include #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; @@ -194,29 +201,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()->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 + << " * : " << this->dataPtr->service << std::endl + << " * : " << this->dataPtr->poseTopic << std::endl + << " * : " << this->dataPtr->deletionTopic << std::endl + << " * : " << this->dataPtr->sceneTopic << std::endl; + } + else + { + App()->findChild()->installEventFilter(this); + } } ///////////////////////////////////////////////// @@ -288,13 +322,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 +369,8 @@ void TransportSceneManagerPrivate::OnRender() if (nullptr == this->scene) return; - this->InitializeTransport(); + this->initializeTransport = std::thread( + &TransportSceneManagerPrivate::InitializeTransport, this); } std::lock_guard lock(this->msgMutex); diff --git a/src/plugins/transport_scene_manager/TransportSceneManager.hh b/src/plugins/transport_scene_manager/TransportSceneManager.hh index c9fd7198f..b13a9bfbd 100644 --- a/src/plugins/transport_scene_manager/TransportSceneManager.hh +++ b/src/plugins/transport_scene_manager/TransportSceneManager.hh @@ -36,10 +36,13 @@ namespace plugins /// ## Configuration /// /// * \ : Name of service where this system will request a scene - /// message. + /// message. Optional, defaults to "/scene". /// * \ : Name of topic to subscribe to receive pose updates. + /// Optional, defaults to "/pose". /// * \ : Name of topic to request entity deletions. - /// * \ : Name of topic to receive scene updates. + /// Optional, defaults to "/delete". + /// * \ : Name of topic to receive scene updates. Optional, + /// defaults to "/scene". class TransportSceneManager : public Plugin { Q_OBJECT diff --git a/src/plugins/transport_scene_manager/TransportSceneManager.qml b/src/plugins/transport_scene_manager/TransportSceneManager.qml index 873da3001..beff6a0f4 100644 --- a/src/plugins/transport_scene_manager/TransportSceneManager.qml +++ b/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://github.com/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: "Service: " + service + + "
Pose topic: " + poseTopic + + "
Deletion topic: " + deletionTopic + + "
Scene topic: " + sceneTopic + } + + + Item { + Layout.columnSpan: 1 + width: 10 + Layout.fillHeight: true + } } From c9c4c1dcbb6749f4db969596986983d258dece30 Mon Sep 17 00:00:00 2001 From: Louise Poubel Date: Thu, 3 Mar 2022 15:16:57 -0800 Subject: [PATCH 2/3] Prefix topics with / MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: ahcorde Signed-off-by: Louise Poubel Co-authored-by: Alejandro Hernández Cordero --- .../transport_scene_manager/TransportSceneManager.qml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/plugins/transport_scene_manager/TransportSceneManager.qml b/src/plugins/transport_scene_manager/TransportSceneManager.qml index beff6a0f4..adfbf7a85 100644 --- a/src/plugins/transport_scene_manager/TransportSceneManager.qml +++ b/src/plugins/transport_scene_manager/TransportSceneManager.qml @@ -36,10 +36,10 @@ GridLayout { Layout.columnSpan: 1 Layout.fillWidth: true wrapMode: Text.WordWrap - text: "Service: " + service + - "
Pose topic: " + poseTopic + - "
Deletion topic: " + deletionTopic + - "
Scene topic: " + sceneTopic + text: "Service: /" + service + + "
Pose topic: /" + poseTopic + + "
Deletion topic: /" + deletionTopic + + "
Scene topic: /" + sceneTopic } From 7cdf4672a7088d5d3ba440c0b9df0e8fe0abc33b Mon Sep 17 00:00:00 2001 From: Louise Poubel Date: Fri, 4 Mar 2022 09:25:43 -0800 Subject: [PATCH 3/3] fix tests Signed-off-by: Louise Poubel --- .../transport_scene_manager/TransportSceneManager.cc | 2 ++ test/integration/transport_scene_manager.cc | 11 ++++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/plugins/transport_scene_manager/TransportSceneManager.cc b/src/plugins/transport_scene_manager/TransportSceneManager.cc index b8499f24b..276ae26bf 100644 --- a/src/plugins/transport_scene_manager/TransportSceneManager.cc +++ b/src/plugins/transport_scene_manager/TransportSceneManager.cc @@ -187,6 +187,8 @@ TransportSceneManager::TransportSceneManager() ///////////////////////////////////////////////// TransportSceneManager::~TransportSceneManager() { + if (this->dataPtr->initializeTransport.joinable()) + this->dataPtr->initializeTransport.join(); } ///////////////////////////////////////////////// diff --git a/test/integration/transport_scene_manager.cc b/test/integration/transport_scene_manager.cc index aa52509cc..dec0e3efa 100644 --- a/test/integration/transport_scene_manager.cc +++ b/test/integration/transport_scene_manager.cc @@ -162,6 +162,8 @@ 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); @@ -169,8 +171,15 @@ TEST(MinimalSceneTest, IGN_UTILS_TEST_ENABLED_ONLY_ON_LINUX(Config)) 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(