Fixes review comments.

maliput · Apr 23, 2021 · 602c897 · 602c897
1 parent f381eed
commit 602c897
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 55 deletions.
diff --git a/delphyne_gui/visualizer/layout2_with_teleop.config b/delphyne_gui/visualizer/layout2_with_teleop.config
@@ -225,7 +225,6 @@
 <plugin filename="TopicInterfacePlugin" read_only="true">
   <title>Scene tree</title>
   <topic>/scene</topic>
-  <message_type>ignition.msgs.Scene</message_type>
   <hide>ambient</hide>
   <hide>background</hide>
   <hide>fog</hide>

diff --git a/delphyne_gui/visualizer/topic_interface_plugin/message_widget.cc b/delphyne_gui/visualizer/topic_interface_plugin/message_widget.cc
@@ -4,10 +4,10 @@
 namespace delphyne {
 namespace gui {
 
-void MessageWidget::Parse(google::protobuf::Message* msg) {
-  typeName = msg->GetTypeName();
-  auto reflection = msg->GetReflection();
-  auto descriptor = msg->GetDescriptor();
+void MessageWidget::Parse(google::protobuf::Message* _msg) {
+  typeName = _msg->GetTypeName();
+  auto reflection = _msg->GetReflection();
+  auto descriptor = _msg->GetDescriptor();
 
   if (!descriptor) {
     // TODO Should throw!
@@ -21,45 +21,43 @@ void MessageWidget::Parse(google::protobuf::Message* msg) {
 
     const auto fieldType = fieldDescriptor->type();
 
-    const std::string fieldName = fieldDescriptor->name();
-
-    const std::string scopedName = fieldName;
+    const std::string scopedName = fieldDescriptor->name();
 
     if (fieldDescriptor->is_repeated()) {
       // Parse all fields of the repeated message.
-      for (int count = 0; count < reflection->FieldSize(*msg, fieldDescriptor); ++count) {
+      for (int count = 0; count < reflection->FieldSize(*_msg, fieldDescriptor); ++count) {
         // Append number to name to differentiate between repeated variables.
         const std::string name = scopedName + "::" + std::to_string(count);
         // Evaluate the type.
         if (fieldType == google::protobuf::FieldDescriptor::TYPE_MESSAGE) {
-          auto& value = reflection->GetRepeatedMessage(*msg, fieldDescriptor, count);
+          auto& value = reflection->GetRepeatedMessage(*_msg, fieldDescriptor, count);
           children[scopedName] = std::make_unique<MessageWidget>(&value);
         } else if (fieldType == google::protobuf::FieldDescriptor::TYPE_DOUBLE) {
-          const double value = reflection->GetRepeatedDouble(*msg, fieldDescriptor, count);
+          const double value = reflection->GetRepeatedDouble(*_msg, fieldDescriptor, count);
           children[name] = std::make_unique<MessageWidget>(value);
         } else if (fieldType == google::protobuf::FieldDescriptor::TYPE_FLOAT) {
-          const float value = reflection->GetRepeatedFloat(*msg, fieldDescriptor, count);
+          const float value = reflection->GetRepeatedFloat(*_msg, fieldDescriptor, count);
           children[name] = std::make_unique<MessageWidget>(value);
         } else if (fieldType == google::protobuf::FieldDescriptor::TYPE_INT64) {
-          const int64_t value = reflection->GetRepeatedInt64(*msg, fieldDescriptor, count);
+          const int64_t value = reflection->GetRepeatedInt64(*_msg, fieldDescriptor, count);
           children[name] = std::make_unique<MessageWidget>(value);
         } else if (fieldType == google::protobuf::FieldDescriptor::TYPE_INT32) {
-          const int32_t value = reflection->GetRepeatedInt32(*msg, fieldDescriptor, count);
+          const int32_t value = reflection->GetRepeatedInt32(*_msg, fieldDescriptor, count);
           children[name] = std::make_unique<MessageWidget>(value);
         } else if (fieldType == google::protobuf::FieldDescriptor::TYPE_UINT64) {
-          const uint64_t value = reflection->GetRepeatedUInt64(*msg, fieldDescriptor, count);
+          const uint64_t value = reflection->GetRepeatedUInt64(*_msg, fieldDescriptor, count);
           children[name] = std::make_unique<MessageWidget>(value);
         } else if (fieldType == google::protobuf::FieldDescriptor::TYPE_UINT32) {
-          const uint32_t value = reflection->GetRepeatedUInt32(*msg, fieldDescriptor, count);
+          const uint32_t value = reflection->GetRepeatedUInt32(*_msg, fieldDescriptor, count);
           children[name] = std::make_unique<MessageWidget>(value);
         } else if (fieldType == google::protobuf::FieldDescriptor::TYPE_BOOL) {
-          const bool value = reflection->GetRepeatedBool(*msg, fieldDescriptor, count);
+          const bool value = reflection->GetRepeatedBool(*_msg, fieldDescriptor, count);
           children[name] = std::make_unique<MessageWidget>(value);
         } else if (fieldType == google::protobuf::FieldDescriptor::TYPE_STRING) {
-          const std::string value = reflection->GetRepeatedString(*msg, fieldDescriptor, count);
+          const std::string value = reflection->GetRepeatedString(*_msg, fieldDescriptor, count);
           children[name] = std::make_unique<MessageWidget>(value);
         } else if (fieldType == google::protobuf::FieldDescriptor::TYPE_ENUM) {
-          auto enumValueDescriptor = reflection->GetRepeatedEnum(*msg, fieldDescriptor, count);
+          auto enumValueDescriptor = reflection->GetRepeatedEnum(*_msg, fieldDescriptor, count);
           const int enumValue = enumValueDescriptor->number();
           const std::string enumName = enumValueDescriptor->name();
           children[name] = std::make_unique<MessageWidget>(EnumValue{enumValue, enumName});
@@ -69,34 +67,34 @@ void MessageWidget::Parse(google::protobuf::Message* msg) {
       }
     } else {  // It's not a repeated message, then we just need to parse them.
       if (fieldType == google::protobuf::FieldDescriptor::TYPE_MESSAGE) {
-        auto& value = reflection->GetMessage(*msg, fieldDescriptor);
+        auto& value = reflection->GetMessage(*_msg, fieldDescriptor);
         children[scopedName] = std::make_unique<MessageWidget>(&value);
       } else if (fieldType == google::protobuf::FieldDescriptor::TYPE_DOUBLE) {
-        const double value = reflection->GetDouble(*msg, fieldDescriptor);
+        const double value = reflection->GetDouble(*_msg, fieldDescriptor);
         children[scopedName] = std::make_unique<MessageWidget>(value);
       } else if (fieldType == google::protobuf::FieldDescriptor::TYPE_FLOAT) {
-        const float value = reflection->GetFloat(*msg, fieldDescriptor);
+        const float value = reflection->GetFloat(*_msg, fieldDescriptor);
         children[scopedName] = std::make_unique<MessageWidget>(value);
       } else if (fieldType == google::protobuf::FieldDescriptor::TYPE_INT64) {
-        const int64_t value = reflection->GetInt64(*msg, fieldDescriptor);
+        const int64_t value = reflection->GetInt64(*_msg, fieldDescriptor);
         children[scopedName] = std::make_unique<MessageWidget>(value);
       } else if (fieldType == google::protobuf::FieldDescriptor::TYPE_INT32) {
-        const int32_t value = reflection->GetInt32(*msg, fieldDescriptor);
+        const int32_t value = reflection->GetInt32(*_msg, fieldDescriptor);
         children[scopedName] = std::make_unique<MessageWidget>(value);
       } else if (fieldType == google::protobuf::FieldDescriptor::TYPE_UINT64) {
-        const uint64_t value = reflection->GetUInt64(*msg, fieldDescriptor);
+        const uint64_t value = reflection->GetUInt64(*_msg, fieldDescriptor);
         children[scopedName] = std::make_unique<MessageWidget>(value);
       } else if (fieldType == google::protobuf::FieldDescriptor::TYPE_UINT32) {
-        const uint32_t value = reflection->GetUInt32(*msg, fieldDescriptor);
+        const uint32_t value = reflection->GetUInt32(*_msg, fieldDescriptor);
         children[scopedName] = std::make_unique<MessageWidget>(value);
       } else if (fieldType == google::protobuf::FieldDescriptor::TYPE_BOOL) {
-        const bool value = reflection->GetBool(*msg, fieldDescriptor);
+        const bool value = reflection->GetBool(*_msg, fieldDescriptor);
         children[scopedName] = std::make_unique<MessageWidget>(value);
       } else if (fieldType == google::protobuf::FieldDescriptor::TYPE_STRING) {
-        const std::string value = reflection->GetString(*msg, fieldDescriptor);
+        const std::string value = reflection->GetString(*_msg, fieldDescriptor);
         children[scopedName] = std::make_unique<MessageWidget>(value);
       } else if (fieldType == google::protobuf::FieldDescriptor::TYPE_ENUM) {
-        auto enumValueDescriptor = reflection->GetEnum(*msg, fieldDescriptor);
+        auto enumValueDescriptor = reflection->GetEnum(*_msg, fieldDescriptor);
         const int enumValue = enumValueDescriptor->number();
         const std::string enumName = enumValueDescriptor->name();
         children[scopedName] = std::make_unique<MessageWidget>(EnumValue{enumValue, enumName});

diff --git a/delphyne_gui/visualizer/topic_interface_plugin/message_widget.h b/delphyne_gui/visualizer/topic_interface_plugin/message_widget.h
@@ -106,9 +106,9 @@ class MessageWidget {
   const std::unordered_map<std::string, std::unique_ptr<MessageWidget>>& Children() const { return children; }
 
  private:
-  /// @brief Parses @p msg and stores the values into `variantValue` or
-  ///        `children`.
-  void Parse(google::protobuf::Message* msg);
+  /// @brief Parses @p _msg and stores children MessageWidget into
+  ////       `variantValue`.
+  void Parse(google::protobuf::Message* _msg);
 
   /// @brief Holds the type name of the data.
   std::string typeName{""};

diff --git a/delphyne_gui/visualizer/topic_interface_plugin/topic_interface_plugin.cc b/delphyne_gui/visualizer/topic_interface_plugin/topic_interface_plugin.cc
@@ -77,29 +77,13 @@ void TopicInterfacePlugin::LoadConfig(const tinyxml2::XMLElement* _pluginElem) {
     if (auto xmlTopicName = _pluginElem->FirstChildElement("topic")) {
       topicName = xmlTopicName->GetText();
     }
-    if (auto xmlMessageType = _pluginElem->FirstChildElement("message_type")) {
-      msgType = xmlMessageType->GetText();
-    }
-    if (msgType.empty()) {
-      ignwarn << "Message type not specified, widget will be constructed "
-              << "according to the first message received on topic [" << topicName << "]." << std::endl;
-    }
     // Visibility per widget.
     for (auto xmlHideWidgetElement = _pluginElem->FirstChildElement("hide"); xmlHideWidgetElement != nullptr;
          xmlHideWidgetElement = xmlHideWidgetElement->NextSiblingElement("hide")) {
       hideWidgets.push_back(xmlHideWidgetElement->GetText());
     }
   }
 
-  // Build the message widget.
-  if (!msgType.empty()) {
-    auto newMsg = ignition::msgs::Factory::New(msgType, "");
-    if (!newMsg) {
-      ignerr << "Unable to create message of type[" << msgType << "] "
-             << "widget will be initialized when a message is received." << std::endl;
-    }
-  }
-
   // Subscribe
   if (!node.Subscribe(topicName, &TopicInterfacePlugin::OnMessage, this)) {
     ignerr << "Failed to subscribe to topic [" << topicName << "]" << std::endl;

diff --git a/delphyne_gui/visualizer/topic_interface_plugin/topic_interface_plugin.h b/delphyne_gui/visualizer/topic_interface_plugin/topic_interface_plugin.h
@@ -23,8 +23,6 @@ namespace gui {
 ///          and get its fields.
 ///          - Use `<topic>ignition_topic_name</topic>` to configure the topic
 ///            name.
-///          - Use `<message_type>message::type::goes::here</message_type>` to
-///            indicate the topic to receive.
 ///          - Via the xml plugin configuration one can generate a blacklist of
 ///            types. Use multiple nodes like
 ///            `<hide>type::name::goes::here</hide>` to omit displaying that
@@ -49,11 +47,6 @@ class TopicInterfacePlugin : public ignition::gui::Plugin {
   /// @param _msg The received message.
   void OnMessage(const google::protobuf::Message& _msg);
 
- public slots:
-
- signals:
-
- protected:
  private:
   /// @brief The type of the message to receive.
   std::string msgType{};