Merge ign-gui3 ➡️ ign-gui6 (#452)

Signed-off-by: Louise Poubel <louise@openrobotics.org>

Changelog.md

@@ -458,7 +458,25 @@
 1. Depend on ign-rendering4
     * [BitBucket pull request 243](https://osrf-migration.github.io/ignition-gh-pages/#!/ignitionrobotics/ign-gui/pull-requests/243)
 
-## Ignition Gui 3
+## Gazebo GUI 3
+
+### Gazebo GUI 3.11.0 (2022-08-02)
+
+1. Dialog read attribute fixes
+    * [Pull request #450](https://github.com/gazebosim/gz-gui/pull/450)
+    * [Pull request #442](https://github.com/gazebosim/gz-gui/pull/442)
+
+1. Fixed topic echo test
+    * [Pull request #448](https://github.com/gazebosim/gz-gui/pull/448)
+
+1. Teleop: Refactor and support vertical
+    * [Pull request #440](https://github.com/gazebosim/gz-gui/pull/440)
+
+1. Change `IGN_DESIGNATION` to `GZ_DESIGNATION`
+    * [Pull request #437](https://github.com/gazebosim/gz-gui/pull/437)
+
+1. Ignition -> Gazebo
+    * [Pull request #435](https://github.com/gazebosim/gz-gui/pull/435)
 
 ### Ignition Gui 3.10.0 (2022-07-13)
 

Migration.md

@@ -38,6 +38,10 @@ release will remove the deprecated code.
   Use `ignition::msgs::Convert` to `std::chrono::steady_clock::time_point`
   instead.
 
+## Ignition GUI 3.10 to 3.11
+
+* `Dialog::ReadConfigAttribute` doesn't create a missing file anymore.
+
 ## Ignition GUI 3.6 to 3.7
 
 * The `Application::PluginAdded` signal used to send empty strings. Now it

include/ignition/gui/Dialog.hh

@@ -69,9 +69,9 @@ namespace ignition
         const std::string &_path, const std::string &_attribute,
         const bool _value) const;
 
-      /// \brief Gets a config attribute value, if not found in config
-      /// write the default in the config and get it.
-      /// creates config file if it doesn't exist.
+      /// \brief Gets a config attribute value.
+      /// It will return an empty string if the config file or the attribute
+      /// don't exist.
       /// \param[in] _path config path
       /// \param[in] _attribute attribute name
       /// \return attribute value as string

src/Dialog.cc

@@ -138,83 +138,33 @@ void Dialog::SetDefaultConfig(const std::string &_config)
 std::string Dialog::ReadConfigAttribute(const std::string &_path,
   const std::string &_attribute) const
 {
-  tinyxml2::XMLDocument doc;
-  std::string value {""};
-  std::string config = "<?xml version=\"1.0\"?>\n\n";
-  tinyxml2::XMLPrinter defaultPrinter;
-  bool configExists{true};
-  std::string dialogName = this->objectName().toStdString();
-
-  auto Value = [&_attribute, &dialogName](const tinyxml2::XMLDocument &_doc)
-  {
-    // Process each dialog
-    // If multiple attributes share the same name, return the first one
-    for (auto dialogElem = _doc.FirstChildElement("dialog");
-      dialogElem != nullptr;
-      dialogElem = dialogElem->NextSiblingElement("dialog"))
-    {
-      if (dialogElem->Attribute("name") == dialogName)
-      {
-        if (dialogElem->Attribute(_attribute.c_str()))
-          return dialogElem->Attribute(_attribute.c_str());
-      }
-    }
-    return "";
-  };
-
-  // Check if the passed in config file exists.
-  // (If the default config path doesn't exist yet, it's expected behavior.
-  // It will be created the first time now.)
   if (!common::exists(_path))
   {
-    configExists = false;
-    doc.Parse(this->dataPtr->config.c_str());
-    value = Value(doc);
+    return std::string();
   }
-  else
-  {
-    auto success = !doc.LoadFile(_path.c_str());
-    if (!success)
-    {
-      ignerr << "Failed to load file [" << _path << "]: XMLError"
-             << std::endl;
-      return "";
-    }
-    value = Value(doc);
 
-    // config exists but attribute not there read from default config
-    if (value.empty())
-    {
-      tinyxml2::XMLDocument missingDoc;
-      missingDoc.Parse(this->dataPtr->config.c_str());
-      value = Value(missingDoc);
-      missingDoc.Print(&defaultPrinter);
-    }
-  }
-
-  // Write config file
-  tinyxml2::XMLPrinter printer;
-  doc.Print(&printer);
-
-  // Don't write the xml version decleration if file exists
-  if (configExists)
+  tinyxml2::XMLDocument doc;
+  auto success = !doc.LoadFile(_path.c_str());
+  if (!success)
   {
-    config = "";
+    ignerr << "Failed to load file [" << _path << "]: XMLError"
+           << std::endl;
+    return std::string();
   }
 
-  igndbg << "Setting dialog " << this->objectName().toStdString()
-    << " default config." << std::endl;
-  config += printer.CStr();
-  config += defaultPrinter.CStr();
-  std::ofstream out(_path.c_str(), std::ios::out);
-  if (!out)
+  // Process each dialog
+  // If multiple attributes share the same name, return the first one
+  std::string dialogName = this->objectName().toStdString();
+  for (auto dialogElem = doc.FirstChildElement("dialog");
+      dialogElem != nullptr;
+      dialogElem = dialogElem->NextSiblingElement("dialog"))
   {
-    ignerr << "Unable to open file: " << _path
-           << ".\nCheck file permissions.\n";
-    return "";
+    if (dialogElem->Attribute("name") == dialogName &&
+        dialogElem->Attribute(_attribute.c_str()))
+    {
+      return dialogElem->Attribute(_attribute.c_str());
+    }
   }
-  else
-    out << config;
 
-  return value;
+  return std::string();
 }

src/Dialog_TEST.cc

@@ -33,61 +33,63 @@ char* g_argv[] =
 
 using namespace ignition;
 using namespace gui;
-using namespace std::chrono_literals;
 
 /////////////////////////////////////////////////
 TEST(DialogTest, IGN_UTILS_TEST_DISABLED_ON_WIN32(UpdateDialogConfig))
 {
-  ignition::common::Console::SetVerbosity(4);
-  Application app(g_argc, g_argv, ignition::gui::WindowType::kDialog);
-
-  // Change default config path
-  App()->SetDefaultConfigPath(kTestConfigFile);
+  common::Console::SetVerbosity(4);
+  Application app(g_argc, g_argv, WindowType::kDialog);
 
   auto dialog = new Dialog;
   ASSERT_NE(nullptr, dialog);
+  dialog->setObjectName("quick_menu");
+
+  // Start without a file
+  std::remove(kTestConfigFile.c_str());
 
-  // Read attribute value when the default the config is not set
+  // Read attribute value when the config doesn't exist
   {
     EXPECT_FALSE(common::exists(kTestConfigFile));
-    std::string allow = dialog->ReadConfigAttribute(app.DefaultConfigPath(),
+    std::string allow = dialog->ReadConfigAttribute(kTestConfigFile,
       "allow");
-    EXPECT_EQ(allow, "");
-
-    // Config file is created when a read is attempted
-    EXPECT_TRUE(common::exists(kTestConfigFile));
+    EXPECT_TRUE(allow.empty());
 
-    // Delete file
-    std::remove(kTestConfigFile.c_str());
+    // Config file still doesn't exist
+    EXPECT_FALSE(common::exists(kTestConfigFile));
   }
 
   // Read a non existing attribute
   {
     EXPECT_FALSE(common::exists(kTestConfigFile));
-    dialog->setObjectName("quick_menu");
-    dialog->SetDefaultConfig(std::string(
-      "<dialog name=\"quick_menu\" show=\"true\"/>"));
-    std::string allow = dialog->ReadConfigAttribute(app.DefaultConfigPath(),
-      "allow");
-    EXPECT_EQ(allow, "");
 
-    // Config file is created when a read is attempted
+    // Create file
+    std::ofstream configFile(kTestConfigFile);
+    configFile << "<dialog name='quick_menu'/>";
+    configFile.close();
     EXPECT_TRUE(common::exists(kTestConfigFile));
 
+    std::string allow = dialog->ReadConfigAttribute(kTestConfigFile,
+      "allow");
+    EXPECT_TRUE(allow.empty());
+
     // Delete file
     std::remove(kTestConfigFile.c_str());
   }
 
   // Read an existing attribute
   {
     EXPECT_FALSE(common::exists(kTestConfigFile));
-    std::string show = dialog->ReadConfigAttribute(app.DefaultConfigPath(),
-      "show");
-    EXPECT_EQ(show, "true");
 
-    // Config file is created when a read is attempted
+    // Create file
+    std::ofstream configFile(kTestConfigFile);
+    configFile << "<dialog name='quick_menu' show='true'/>";
+    configFile.close();
     EXPECT_TRUE(common::exists(kTestConfigFile));
 
+    std::string show = dialog->ReadConfigAttribute(kTestConfigFile,
+      "show");
+    EXPECT_EQ(show, "true");
+
     // Delete file
     std::remove(kTestConfigFile.c_str());
   }
@@ -96,19 +98,18 @@ TEST(DialogTest, IGN_UTILS_TEST_DISABLED_ON_WIN32(UpdateDialogConfig))
   {
     EXPECT_FALSE(common::exists(kTestConfigFile));
 
-    // Call a read to create config file
-    std::string allow = dialog->ReadConfigAttribute(app.DefaultConfigPath(),
-      "allow");
+    // Create file
+    std::ofstream configFile(kTestConfigFile);
+    configFile << "<dialog name='quick_menu'/>";
+    configFile.close();
+    EXPECT_TRUE(common::exists(kTestConfigFile));
 
-    // Empty string for a non existing attribute
-    EXPECT_EQ(allow, "");
-    dialog->UpdateConfigAttribute(app.DefaultConfigPath(), "allow", true);
-    allow = dialog->ReadConfigAttribute(app.DefaultConfigPath(),
-      "allow");
-    EXPECT_EQ(allow, "true");
+    // Update value
+    dialog->UpdateConfigAttribute(kTestConfigFile, "allow", true);
 
-    // Config file is created when a read is attempted
-    EXPECT_TRUE(common::exists(kTestConfigFile));
+    // Read value
+    auto allow = dialog->ReadConfigAttribute(kTestConfigFile, "allow");
+    EXPECT_EQ(allow, "true");
 
     // Delete file
     std::remove(kTestConfigFile.c_str());
@@ -118,17 +119,19 @@ TEST(DialogTest, IGN_UTILS_TEST_DISABLED_ON_WIN32(UpdateDialogConfig))
   {
     EXPECT_FALSE(common::exists(kTestConfigFile));
 
-    // Call a read to create config file
-    std::string allow = dialog->ReadConfigAttribute(app.DefaultConfigPath(),
-      "allow");
-    dialog->UpdateConfigAttribute(app.DefaultConfigPath(), "allow", false);
-    allow = dialog->ReadConfigAttribute(app.DefaultConfigPath(),
-      "allow");
-    EXPECT_EQ(allow, "false");
-
-    // Config file is created when a read is attempted
+    // Create file
+    std::ofstream configFile(kTestConfigFile);
+    configFile << "<dialog name='quick_menu' show='true'/>";
+    configFile.close();
     EXPECT_TRUE(common::exists(kTestConfigFile));
 
+    // Update value
+    dialog->UpdateConfigAttribute(kTestConfigFile, "allow", false);
+
+    // Read value
+    auto allow = dialog->ReadConfigAttribute(kTestConfigFile, "allow");
+    EXPECT_EQ(allow, "false");
+
     // Delete file
     std::remove(kTestConfigFile.c_str());
   }

src/plugins/topic_echo/CMakeLists.txt

@@ -4,6 +4,5 @@ ign_gui_add_plugin(TopicEcho
   QT_HEADERS
     TopicEcho.hh
   TEST_SOURCES
-    # TopicEcho_TEST.cc
+    TopicEcho_TEST.cc
 )
-

src/plugins/topic_echo/TopicEcho.cc

@@ -180,4 +180,3 @@ void TopicEcho::SetPaused(const bool &_paused)
 // Register this plugin
 IGNITION_ADD_PLUGIN(ignition::gui::plugins::TopicEcho,
                     ignition::gui::Plugin)
-

src/plugins/topic_echo/TopicEcho.hh

@@ -26,6 +26,16 @@
 #pragma warning(pop)
 #endif
 
+#ifndef _WIN32
+#  define TopicEcho_EXPORTS_API
+#else
+#  if (defined(TopicEcho_EXPORTS))
+#    define TopicEcho_EXPORTS_API __declspec(dllexport)
+#  else
+#    define TopicEcho_EXPORTS_API __declspec(dllimport)
+#  endif
+#endif
+
 #include <memory>
 
 #include "ignition/gui/Plugin.hh"
@@ -42,7 +52,7 @@ namespace plugins
   ///
   /// ## Configuration
   /// This plugin doesn't accept any custom configuration.
-  class TopicEcho : public Plugin
+  class TopicEcho_EXPORTS_API TopicEcho : public Plugin
   {
     Q_OBJECT
 

src/plugins/topic_echo/TopicEcho.qml

@@ -40,12 +40,14 @@ Rectangle {
 
         TextField {
           id: topicField
+          objectName: "topicField"
           text: TopicEcho.topic
           selectByMouse: true
         }
       }
 
       Switch {
+        objectName: "echoSwitch"
         text: qsTr("Echo")
         onToggled: {
           TopicEcho.topic = topicField.text
@@ -64,13 +66,15 @@ Rectangle {
 
     SpinBox {
       id: bufferField
+      objectName: "bufferField"
       value: 10
       onValueChanged: {
         TopicEcho.OnBuffer(value)
       }
     }
 
     CheckBox {
+      objectName: "pauseCheck"
       text: qsTr("Pause")
       checked: TopicEcho.paused
       onClicked: {
@@ -90,6 +94,7 @@ Rectangle {
 
       ListView {
         id: listView
+        objectName: "listView"
         clip: true
         anchors.fill: parent