From 181a0d5980a56ccc711a7071afee2951e2bfa631 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Mon, 28 Aug 2023 13:44:41 -0500 Subject: [PATCH] Suppress new Jammy warnings (#404) * Use explicit functions for Connections Cleans up corresponding deprecation warnings Signed-off-by: Michael Carroll * Clean up log spew related to "Cannot read property" Signed-off-by: Michael Carroll * More log spew cleanup This narrows the warnings to LoadBadInheritancePlugin. Signed-off-by: Michael Carroll * Lint Signed-off-by: Michael Carroll * Add backward to debug Signed-off-by: Michael Carroll * Fix race in camera_tracking Signed-off-by: Michael Carroll * Remove unloadEngine call Signed-off-by: Michael Carroll * Examples quality of life improvements Signed-off-by: Michael Carroll * Lint Signed-off-by: Michael Carroll * Iteration Signed-off-by: Michael Carroll * Don't unload anywhere Signed-off-by: Michael Carroll * Add a sleep for testing Signed-off-by: Michael Carroll * Force processing events Signed-off-by: Michael Carroll * Remove backward Signed-off-by: Michael Carroll * Undo examples changes Signed-off-by: Michael Carroll * Change inheritance in test Signed-off-by: Michael Carroll * Fix Windows warning Signed-off-by: Michael Carroll --------- Signed-off-by: Michael Carroll Signed-off-by: Michael Carroll --- include/gz/gui/qml/Main.qml | 4 +- include/gz/gui/qml/PluginMenu.qml | 2 +- include/gz/gui/qml/StyleDialog.qml | 6 +- src/Application.cc | 19 ++- src/Application_TEST.cc | 155 ++++++++++-------- src/MainWindow_TEST.cc | 15 +- src/plugins/grid_config/GridConfig.qml | 2 +- src/plugins/image_display/ImageDisplay.qml | 2 +- .../minimal_scene/MinimalSceneRhiOpenGL.cc | 3 +- src/plugins/world_control/WorldControl.qml | 4 +- test/integration/camera_tracking.cc | 15 +- test/integration/minimal_scene.cc | 1 - test/integration/transport_scene_manager.cc | 1 - test/plugins/TestBadInheritancePlugin.cc | 4 +- test/plugins/TestBadInheritancePlugin.hh | 4 +- 15 files changed, 140 insertions(+), 97 deletions(-) diff --git a/include/gz/gui/qml/Main.qml b/include/gz/gui/qml/Main.qml index 132bac2bf..c18154f29 100644 --- a/include/gz/gui/qml/Main.qml +++ b/include/gz/gui/qml/Main.qml @@ -101,10 +101,10 @@ ApplicationWindow // C++ signals to QML slots Connections { target: MainWindow - onNotify: { + function onNotify(_message) { notificationDialog.setTextDuration(_message, 0) } - onNotifyWithDuration: { + function onNotifyWithDuration(_message, _duration) { notificationDialog.setTextDuration(_message, _duration) } } diff --git a/include/gz/gui/qml/PluginMenu.qml b/include/gz/gui/qml/PluginMenu.qml index 53d1f0d37..890a7e7d4 100644 --- a/include/gz/gui/qml/PluginMenu.qml +++ b/include/gz/gui/qml/PluginMenu.qml @@ -26,7 +26,7 @@ Popup { Connections { target: MainWindow - onConfigChanged: { + function onConfigChanged() { filteredModel.model = MainWindow.PluginListModel() } } diff --git a/include/gz/gui/qml/StyleDialog.qml b/include/gz/gui/qml/StyleDialog.qml index 7845f2ed2..f8566b0ca 100644 --- a/include/gz/gui/qml/StyleDialog.qml +++ b/include/gz/gui/qml/StyleDialog.qml @@ -97,21 +97,21 @@ Dialog { // Connections (C++ signal to QML slot) Connections { target: MainWindow - onMaterialThemeChanged: { + function onMaterialThemeChanged() { updateTheme(MainWindow.materialTheme); } } Connections { target: MainWindow - onMaterialPrimaryChanged: { + function onMaterialPrimaryChanged() { updatePrimary(MainWindow.materialPrimary); } } Connections { target: MainWindow - onMaterialAccentChanged: { + function onMaterialAccentChanged() { updateAccent(MainWindow.materialAccent); } } diff --git a/src/Application.cc b/src/Application.cc index 25fe4672a..062ce0f2e 100644 --- a/src/Application.cc +++ b/src/Application.cc @@ -252,8 +252,7 @@ Application::~Application() } if (this->dataPtr->mainWin->QuickWindow()->isVisible()) this->dataPtr->mainWin->QuickWindow()->close(); - delete this->dataPtr->mainWin; - this->dataPtr->mainWin = nullptr; + this->dataPtr->mainWin->deleteLater(); } for (auto dialog : this->dataPtr->dialogs) @@ -264,10 +263,7 @@ Application::~Application() } this->dataPtr->dialogs.clear(); - if (this->dataPtr->engine) - { - this->dataPtr->engine->deleteLater(); - } + delete this->dataPtr->engine; std::queue> empty; std::swap(this->dataPtr->pluginsToAdd, empty); @@ -385,11 +381,20 @@ bool Application::LoadConfig(const std::string &_config) this->dataPtr->pluginsAdded.clear(); // Process each plugin + bool successful = true; for (auto pluginElem = doc.FirstChildElement("plugin"); pluginElem != nullptr; pluginElem = pluginElem->NextSiblingElement("plugin")) { auto filename = pluginElem->Attribute("filename"); - this->LoadPlugin(filename, pluginElem); + if (!this->LoadPlugin(filename, pluginElem)) + { + successful = false; + } + } + + if (!successful) + { + return false; } // Process window properties diff --git a/src/Application_TEST.cc b/src/Application_TEST.cc index 703cdb243..c3f3faefb 100644 --- a/src/Application_TEST.cc +++ b/src/Application_TEST.cc @@ -42,8 +42,8 @@ TEST(ApplicationTest, GZ_UTILS_TEST_ENABLED_ONLY_ON_LINUX(Constructor)) common::Console::SetVerbosity(4); // No Qt app - EXPECT_EQ(nullptr, qGuiApp); - EXPECT_EQ(nullptr, App()); + ASSERT_EQ(nullptr, qGuiApp); + ASSERT_EQ(nullptr, App()); // One app construct - destruct { @@ -65,85 +65,108 @@ TEST(ApplicationTest, GZ_UTILS_TEST_ENABLED_ONLY_ON_LINUX(Constructor)) ////////////////////////////////////////////////// TEST(ApplicationTest, GZ_UTILS_TEST_ENABLED_ONLY_ON_LINUX(LoadPlugin)) { - common::Console::SetVerbosity(4); + gz::common::Console::SetVerbosity(4); // No Qt app - EXPECT_EQ(nullptr, qGuiApp); - - // Official plugin - { - Application app(g_argc, g_argv); + ASSERT_EQ(nullptr, qGuiApp); + Application app(g_argc, g_argv); - EXPECT_TRUE(app.LoadPlugin("Publisher")); - } + EXPECT_TRUE(app.LoadPlugin("Publisher")); +} +////////////////////////////////////////////////// +TEST(ApplicationTest, + GZ_UTILS_TEST_ENABLED_ONLY_ON_LINUX(LoadNonexistantPlugin)) +{ + gz::common::Console::SetVerbosity(4); + // No Qt app + ASSERT_EQ(nullptr, qGuiApp); + Application app(g_argc, g_argv); - // Inexistent plugin - { - Application app(g_argc, g_argv); + EXPECT_FALSE(app.LoadPlugin("_doesnt_exist")); + EXPECT_FALSE(app.RemovePlugin("_doesnt_exist")); +} - EXPECT_FALSE(app.LoadPlugin("_doesnt_exist")); - EXPECT_FALSE(app.RemovePlugin("_doesnt_exist")); - } +////////////////////////////////////////////////// +TEST(ApplicationTest, + GZ_UTILS_TEST_ENABLED_ONLY_ON_LINUX(LoadProgrammaticPlugin)) +{ + gz::common::Console::SetVerbosity(4); + // No Qt app + ASSERT_EQ(nullptr, qGuiApp); + Application app(g_argc, g_argv); - // Plugin path added programmatically + std::string pluginName; + app.connect(&app, &Application::PluginAdded, [&pluginName]( + const QString &_pluginName) { - Application app(g_argc, g_argv); - - std::string pluginName; - app.connect(&app, &Application::PluginAdded, [&pluginName]( - const QString &_pluginName) - { - pluginName = _pluginName.toStdString(); - }); + pluginName = _pluginName.toStdString(); + }); - app.AddPluginPath(std::string(PROJECT_BINARY_PATH) + "/lib"); + app.AddPluginPath(std::string(PROJECT_BINARY_PATH) + "/lib"); - EXPECT_TRUE(app.LoadPlugin("TestPlugin")); - EXPECT_EQ(0u, pluginName.find("plugin")); + 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()); + auto plugin = app.PluginByName(pluginName); + ASSERT_NE(nullptr, plugin); + ASSERT_NE(nullptr, plugin->CardItem()); - EXPECT_EQ(pluginName, plugin->CardItem()->objectName().toStdString()); + EXPECT_EQ(pluginName, plugin->CardItem()->objectName().toStdString()); - EXPECT_TRUE(app.RemovePlugin(pluginName)); - } - - // Plugin path added by env var - { - setenv("TEST_ENV_VAR", - (std::string(PROJECT_BINARY_PATH) + "/lib").c_str(), 1); + EXPECT_TRUE(app.RemovePlugin(pluginName)); +} - Application app(g_argc, g_argv); - app.SetPluginPathEnv("TEST_ENV_VAR"); +////////////////////////////////////////////////// +TEST(ApplicationTest, GZ_UTILS_TEST_ENABLED_ONLY_ON_LINUX(LoadEnvPlugin)) +{ + gz::common::Console::SetVerbosity(4); + // No Qt app + ASSERT_EQ(nullptr, qGuiApp); + Application app(g_argc, g_argv); - EXPECT_TRUE(app.LoadPlugin("TestPlugin")); - } + setenv("TEST_ENV_VAR", + (std::string(PROJECT_BINARY_PATH) + "/lib").c_str(), 1); + app.SetPluginPathEnv("TEST_ENV_VAR"); + EXPECT_TRUE(app.LoadPlugin("TestPlugin")); +} - // Plugin which doesn't inherit from gui::Plugin - { - Application app(g_argc, g_argv); - app.AddPluginPath(std::string(PROJECT_BINARY_PATH) + "/lib"); +////////////////////////////////////////////////// +TEST(ApplicationTest, + GZ_UTILS_TEST_ENABLED_ONLY_ON_LINUX(LoadBadInheritancePlugin)) +{ + gz::common::Console::SetVerbosity(4); + // No Qt app + ASSERT_EQ(nullptr, qGuiApp); + Application app(g_argc, g_argv); - EXPECT_FALSE(app.LoadPlugin("TestBadInheritancePlugin")); - } + app.AddPluginPath(std::string(PROJECT_BINARY_PATH) + "/lib"); + EXPECT_FALSE(app.LoadPlugin("TestBadInheritancePlugin")); +} - // Plugin which is not registered - { - Application app(g_argc, g_argv); - app.AddPluginPath(std::string(PROJECT_BINARY_PATH) + "/lib"); +////////////////////////////////////////////////// +TEST(ApplicationTest, + GZ_UTILS_TEST_ENABLED_ONLY_ON_LINUX(LoadNotRegisteredPlugin)) +{ + gz::common::Console::SetVerbosity(4); + // No Qt app + ASSERT_EQ(nullptr, qGuiApp); + Application app(g_argc, g_argv); - EXPECT_FALSE(app.LoadPlugin("TestNotRegisteredPlugin")); - } + app.AddPluginPath(std::string(PROJECT_BINARY_PATH) + "/lib"); + EXPECT_FALSE(app.LoadPlugin("TestNotRegisteredPlugin")); +} - // Plugin with invalid QML - { - Application app(g_argc, g_argv); - app.AddPluginPath(std::string(PROJECT_BINARY_PATH) + "/lib"); +////////////////////////////////////////////////// +TEST(ApplicationTest, + GZ_UTILS_TEST_ENABLED_ONLY_ON_LINUX(LoadInvalidQmlPlugin)) +{ + gz::common::Console::SetVerbosity(4); + // No Qt app + ASSERT_EQ(nullptr, qGuiApp); + Application app(g_argc, g_argv); - EXPECT_FALSE(app.LoadPlugin("TestInvalidQmlPlugin")); - } + app.AddPluginPath(std::string(PROJECT_BINARY_PATH) + "/lib"); + EXPECT_FALSE(app.LoadPlugin("TestInvalidQmlPlugin")); } ////////////////////////////////////////////////// @@ -151,7 +174,7 @@ TEST(ApplicationTest, GZ_UTILS_TEST_ENABLED_ONLY_ON_LINUX(LoadConfig)) { common::Console::SetVerbosity(4); - EXPECT_EQ(nullptr, qGuiApp); + ASSERT_EQ(nullptr, qGuiApp); // Empty string { @@ -201,7 +224,7 @@ TEST(ApplicationTest, GZ_UTILS_TEST_ENABLED_ONLY_ON_LINUX(LoadDefaultConfig)) { common::Console::SetVerbosity(4); - EXPECT_EQ(nullptr, qGuiApp); + ASSERT_EQ(nullptr, qGuiApp); // Test config file { @@ -227,7 +250,7 @@ TEST(ApplicationTest, { common::Console::SetVerbosity(4); - EXPECT_EQ(nullptr, qGuiApp); + ASSERT_EQ(nullptr, qGuiApp); // No plugins { @@ -297,7 +320,7 @@ TEST(ApplicationTest, GZ_UTILS_TEST_ENABLED_ONLY_ON_LINUX(Dialog)) { common::Console::SetVerbosity(4); - EXPECT_EQ(nullptr, qGuiApp); + ASSERT_EQ(nullptr, qGuiApp); // Single dialog { @@ -372,7 +395,7 @@ TEST(ApplicationTest, GZ_UTILS_TEST_ENABLED_ONLY_ON_LINUX(messageHandler)) { common::Console::SetVerbosity(4); - EXPECT_EQ(nullptr, qGuiApp); + ASSERT_EQ(nullptr, qGuiApp); Application app(g_argc, g_argv); diff --git a/src/MainWindow_TEST.cc b/src/MainWindow_TEST.cc index 58d25478b..c5c12e21d 100644 --- a/src/MainWindow_TEST.cc +++ b/src/MainWindow_TEST.cc @@ -54,7 +54,7 @@ TEST(MainWindowTest, GZ_UTILS_TEST_ENABLED_ONLY_ON_LINUX(Constructor)) auto mainWindow = new MainWindow; ASSERT_NE(nullptr, mainWindow); - delete mainWindow; + mainWindow->deleteLater(); } ///////////////////////////////////////////////// @@ -91,7 +91,7 @@ TEST(MainWindowTest, GZ_UTILS_TEST_ENABLED_ONLY_ON_LINUX(OnSaveConfig)) std::remove(kTestConfigFile.c_str()); } - delete mainWindow; + mainWindow->deleteLater(); } ///////////////////////////////////////////////// @@ -127,7 +127,7 @@ TEST(MainWindowTest, GZ_UTILS_TEST_ENABLED_ONLY_ON_LINUX(SaveConfigAs)) std::remove(kTestConfigFile.c_str()); } - delete mainWindow; + mainWindow->deleteLater(); } ///////////////////////////////////////////////// @@ -720,6 +720,8 @@ TEST(MainWindowTest, GZ_UTILS_TEST_ENABLED_ONLY_ON_LINUX(ApplyConfig)) auto mainWindow = new MainWindow; ASSERT_NE(nullptr, mainWindow); + app.processEvents(QEventLoop::ExcludeUserInputEvents); + // Default config { auto c = mainWindow->CurrentWindowConfig(); @@ -729,6 +731,7 @@ TEST(MainWindowTest, GZ_UTILS_TEST_ENABLED_ONLY_ON_LINUX(ApplyConfig)) EXPECT_TRUE(c.pluginsFromPaths); EXPECT_TRUE(c.showPlugins.empty()); EXPECT_TRUE(c.ignoredProps.empty()); + } // Apply a config @@ -746,9 +749,11 @@ TEST(MainWindowTest, GZ_UTILS_TEST_ENABLED_ONLY_ON_LINUX(ApplyConfig)) // c.showPlugins.push_back("watermelon"); // c.ignoredProps.insert("position"); - mainWindow->ApplyConfig(c); + EXPECT_TRUE(mainWindow->ApplyConfig(c)); } + app.processEvents(QEventLoop::ExcludeUserInputEvents); + // Check applied config { auto c = mainWindow->CurrentWindowConfig(); @@ -769,5 +774,5 @@ TEST(MainWindowTest, GZ_UTILS_TEST_ENABLED_ONLY_ON_LINUX(ApplyConfig)) // EXPECT_EQ(c.ignoredProps.size(), 1u); } - delete mainWindow; + mainWindow->deleteLater(); } diff --git a/src/plugins/grid_config/GridConfig.qml b/src/plugins/grid_config/GridConfig.qml index 70262006b..36e46b7d0 100644 --- a/src/plugins/grid_config/GridConfig.qml +++ b/src/plugins/grid_config/GridConfig.qml @@ -37,7 +37,7 @@ GridLayout { Connections { target: GridConfig - onNewParams: { + function onNewParams(_hCellCount, _vCellCount, _cellLength, _pos, _rot, _color) { horizontalCellCount.value = _hCellCount; verticalCellCount.value = _vCellCount; cellLength.value = _cellLength; diff --git a/src/plugins/image_display/ImageDisplay.qml b/src/plugins/image_display/ImageDisplay.qml index 41e3841de..48d1191a6 100644 --- a/src/plugins/image_display/ImageDisplay.qml +++ b/src/plugins/image_display/ImageDisplay.qml @@ -49,7 +49,7 @@ Rectangle { Connections { target: ImageDisplay - onNewImage: image.reload(); + function onNewImage(){ image.reload(); } } ColumnLayout { diff --git a/src/plugins/minimal_scene/MinimalSceneRhiOpenGL.cc b/src/plugins/minimal_scene/MinimalSceneRhiOpenGL.cc index 67abc2f98..84896e291 100644 --- a/src/plugins/minimal_scene/MinimalSceneRhiOpenGL.cc +++ b/src/plugins/minimal_scene/MinimalSceneRhiOpenGL.cc @@ -144,7 +144,8 @@ std::string RenderThreadRhiOpenGL::Initialize() void RenderThreadRhiOpenGL::Update(rendering::CameraPtr _camera) { const GLuint glId = this->dataPtr->engineToQtInterface->TextureId(_camera); - this->dataPtr->texturePtr = reinterpret_cast(glId); + this->dataPtr->texturePtr = reinterpret_cast( + static_cast(glId)); } ///////////////////////////////////////////////// diff --git a/src/plugins/world_control/WorldControl.qml b/src/plugins/world_control/WorldControl.qml index 723fd6b52..9cf54c98e 100644 --- a/src/plugins/world_control/WorldControl.qml +++ b/src/plugins/world_control/WorldControl.qml @@ -29,10 +29,10 @@ RowLayout { Connections { target: WorldControl - onPlaying: { + function onPlaying() { paused = false; } - onPaused: { + function onPaused() { paused = true; } } diff --git a/test/integration/camera_tracking.cc b/test/integration/camera_tracking.cc index 8152437e0..010e12c62 100644 --- a/test/integration/camera_tracking.cc +++ b/test/integration/camera_tracking.cc @@ -98,23 +98,34 @@ TEST(MinimalSceneTest, GZ_UTILS_TEST_ENABLED_ONLY_ON_LINUX(Config)) win->QuickWindow()->show(); // Get camera pose + std::mutex poseMutex; msgs::Pose poseMsg; auto poseCb = std::function( [&](const auto &_msg) { + std::unique_lock lk(poseMutex); poseMsg = _msg; }); transport::Node node; - node.Subscribe("/gui/camera/pose", poseCb); + ASSERT_TRUE(node.Subscribe("/gui/camera/pose", poseCb)); int sleep = 0; int maxSleep = 30; - while (!poseMsg.has_position() && sleep++ < maxSleep) + while (sleep++ < maxSleep) { std::this_thread::sleep_for(100ms); QCoreApplication::processEvents(); + + std::unique_lock lk(poseMutex); + if (poseMsg.has_position()) + { + break; + } } + + // Unsubscribe from updates + node.Unsubscribe("/gui/camera/pose"); EXPECT_LT(sleep, maxSleep); EXPECT_TRUE(poseMsg.has_position()); EXPECT_TRUE(poseMsg.has_orientation()); diff --git a/test/integration/minimal_scene.cc b/test/integration/minimal_scene.cc index a789cc396..0f215c650 100644 --- a/test/integration/minimal_scene.cc +++ b/test/integration/minimal_scene.cc @@ -144,7 +144,6 @@ TEST(MinimalSceneTest, GZ_UTILS_TEST_ENABLED_ONLY_ON_LINUX(Config)) EXPECT_TRUE(app.RemovePlugin(pluginName)); plugins.clear(); - scene.reset(); win->QuickWindow()->close(); } diff --git a/test/integration/transport_scene_manager.cc b/test/integration/transport_scene_manager.cc index 6cfd41ada..9c71e08e2 100644 --- a/test/integration/transport_scene_manager.cc +++ b/test/integration/transport_scene_manager.cc @@ -263,4 +263,3 @@ TEST(TransportSceneManagerTest, GZ_UTILS_TEST_ENABLED_ONLY_ON_LINUX(Config)) scene.reset(); win->QuickWindow()->close(); } - diff --git a/test/plugins/TestBadInheritancePlugin.cc b/test/plugins/TestBadInheritancePlugin.cc index db3601258..04bfdeae1 100644 --- a/test/plugins/TestBadInheritancePlugin.cc +++ b/test/plugins/TestBadInheritancePlugin.cc @@ -25,7 +25,7 @@ using namespace gui; ///////////////////////////////////////////////// TestBadInheritancePlugin::TestBadInheritancePlugin() - : MainWindow() + : Dialog () { } @@ -36,4 +36,4 @@ TestBadInheritancePlugin::~TestBadInheritancePlugin() // Register this plugin GZ_ADD_PLUGIN(TestBadInheritancePlugin, - gui::MainWindow) + gui::Dialog) diff --git a/test/plugins/TestBadInheritancePlugin.hh b/test/plugins/TestBadInheritancePlugin.hh index bd301cf36..59550f1be 100644 --- a/test/plugins/TestBadInheritancePlugin.hh +++ b/test/plugins/TestBadInheritancePlugin.hh @@ -20,14 +20,14 @@ #ifndef Q_MOC_RUN #include - #include + #include #endif namespace gz { namespace gui { - class TestBadInheritancePlugin : public MainWindow + class TestBadInheritancePlugin : public Dialog { Q_OBJECT