Correct breaks when plotting array data (#50)

* Removed utilization of DynamicData_ptr Signed-off-by: Douglas Bertol <dwbertol@gmail.com> * Using loaned values despite of coping memory. This prevents memory leaking and makes the code faster. Signed-off-by: Douglas Bertol <dwbertol@gmail.com> * Fixed some points related to code review from pull request #50 Signed-off-by: Douglas Bertol <dwbertol@gmail.com> Signed-off-by: Douglas Bertol <dwbertol@gmail.com>
eProsima · Jan 13, 2023 · 016db34 · 016db34
1 parent 3d968c8
commit 016db34
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 27 deletions.
diff --git a/plugins/datastreamer_plugin/fastdds/ReaderHandler.cpp b/plugins/datastreamer_plugin/fastdds/ReaderHandler.cpp
@@ -88,6 +88,9 @@ ReaderHandler::~ReaderHandler()
 {
     // Stop the reader
     stop();
+
+    // Delete created data 
+    eprosima::fastrtps::types::DynamicDataFactory::get_instance()->delete_data(data_);
 }
 
 ////////////////////////////////////////////////////
@@ -116,7 +119,7 @@ void ReaderHandler::on_data_available(
     while (!stop_ && read_ret == eprosima::fastrtps::types::ReturnCode_t::RETCODE_OK)
     {
         // Read next data
-        read_ret = reader->take_next_sample(data_.get(), &info);
+        read_ret = reader->take_next_sample(data_, &info);
 
         // If data has been read
         if (read_ret == eprosima::fastrtps::types::ReturnCode_t::RETCODE_OK &&
@@ -130,6 +133,7 @@ void ReaderHandler::on_data_available(
                 numeric_data_info_,
                 data_,
                 numeric_data_);
+
             utils::get_introspection_string_data(
                 string_data_info_,
                 data_,

diff --git a/plugins/datastreamer_plugin/fastdds/ReaderHandler.hpp b/plugins/datastreamer_plugin/fastdds/ReaderHandler.hpp
@@ -128,7 +128,7 @@ struct ReaderHandler : public eprosima::fastdds::dds::DataReaderListener
     eprosima::fastrtps::types::DynamicType_ptr type_;
 
     //! Data Type element
-    eprosima::fastrtps::types::DynamicData_ptr data_;
+    eprosima::fastrtps::types::DynamicData* data_;
 
     std::atomic<bool> stop_;
 
@@ -137,7 +137,6 @@ struct ReaderHandler : public eprosima::fastdds::dds::DataReaderListener
 
     utils::TypeIntrospectionNumericStruct numeric_data_;
     utils::TypeIntrospectionStringStruct string_data_;
-
 };
 
 } /* namespace fastdds */

diff --git a/plugins/datastreamer_plugin/utils/dynamic_types_utils.cpp b/plugins/datastreamer_plugin/utils/dynamic_types_utils.cpp
@@ -58,11 +58,11 @@ void get_introspection_type_names(
         const std::vector<TypeKind>& current_kinds_tree /* = {} */,
         const std::string& separator /* = "/" */)
 {
-    DEBUG("Getting types for type member " << base_type_name);
-
     // Get type kind and store it as kind tree
     TypeKind kind = type->get_kind();
 
+    DEBUG("Getting types for type member " << base_type_name << " of kind " + std::to_string(kind));
+
     switch (kind)
     {
         case fastrtps::types::TK_BOOLEAN:
@@ -92,8 +92,7 @@ void get_introspection_type_names(
 
         case fastrtps::types::TK_ARRAY:
         {
-            DynamicType_ptr internal_type =
-                    array_internal_kind(type);
+            DynamicType_ptr internal_type = array_internal_kind(type);
             unsigned int this_array_size = array_size(type);
 
             // Allow this array depending on data type configuration
@@ -180,7 +179,7 @@ void get_introspection_type_names(
 
 void get_introspection_numeric_data(
         const TypeIntrospectionCollection& numeric_type_names,
-        const DynamicData_ptr& data,
+        DynamicData* data,
         TypeIntrospectionNumericStruct& numeric_data_result)
 {
     DEBUG("Getting numeric data");
@@ -206,7 +205,7 @@ void get_introspection_numeric_data(
                 std::get<TypeKind>(member_type_info);
 
         // Get Data parent that has the member we are looking for
-        const auto& parent_data = get_parent_data_of_member(
+        auto parent_data = get_parent_data_of_member(
             data,
             members,
             kinds);
@@ -220,7 +219,7 @@ void get_introspection_numeric_data(
 
 void get_introspection_string_data(
         const TypeIntrospectionCollection& string_type_names,
-        const eprosima::fastrtps::types::DynamicData_ptr& data,
+        DynamicData* data,
         TypeIntrospectionStringStruct& string_data_result)
 {
     DEBUG("Getting string data");
@@ -246,7 +245,7 @@ void get_introspection_string_data(
                 std::get<TypeKind>(member_type_info);
 
         // Get Data parent that has the member we are looking for
-        const auto& parent_data = get_parent_data_of_member(
+        auto parent_data = get_parent_data_of_member(
             data,
             members,
             kinds);
@@ -258,13 +257,15 @@ void get_introspection_string_data(
     }
 }
 
-DynamicData_ptr get_parent_data_of_member(
-        const DynamicData_ptr& data,
+DynamicData* get_parent_data_of_member(
+        DynamicData* data,
         const std::vector<MemberId>& members_tree,
         const std::vector<TypeKind>& kind_tree,
         unsigned int array_indexes /* = 0 */)
 {
 
+    DEBUG("Getting parent data of type " << std::to_string(kind_tree[array_indexes]));
+
     if (array_indexes == members_tree.size() - 1)
     {
         // One-to-last value, so this one is the value to take, as it is the parent of the data
@@ -282,10 +283,13 @@ DynamicData_ptr get_parent_data_of_member(
             {
                 // Access to the data inside the structure
                 DynamicData* child_data;
-                data->get_complex_value(&child_data, member_id);
+                // Get data pointer to the child_data
+                // The loan and return is a workaround to avoid creating a unecessary copy of the data
+                child_data = data->loan_value(member_id);
+                data->return_loaned_value(child_data);
 
                 return get_parent_data_of_member(
-                    DynamicData_ptr(child_data),
+                    child_data,
                     members_tree,
                     kind_tree,
                     array_indexes + 1);
@@ -306,11 +310,11 @@ DynamicData_ptr get_parent_data_of_member(
 }
 
 double get_numeric_type_from_data(
-        const DynamicData_ptr& data,
+        DynamicData* data,
         const MemberId& member,
         const TypeKind& kind)
 {
-    DEBUG("Getting numeric data of kind " << kind << " in member " << member);
+    DEBUG("Getting numeric data of kind " << std::to_string(kind) << " in member " << member);
 
     switch (kind)
     {
@@ -354,11 +358,12 @@ double get_numeric_type_from_data(
 }
 
 std::string get_string_type_from_data(
-        const DynamicData_ptr& data,
+        DynamicData* data,
         const MemberId& member,
         const TypeKind& kind)
 {
-    DEBUG("Getting string data of kind " << kind << " in member " << member);
+    DEBUG("Getting string data of kind " << std::to_string(kind) << " in member " << member);
+
     switch (kind)
     {
         case fastrtps::types::TK_CHAR8:

diff --git a/plugins/datastreamer_plugin/utils/dynamic_types_utils.hpp b/plugins/datastreamer_plugin/utils/dynamic_types_utils.hpp
@@ -65,7 +65,7 @@ std::vector<std::string> get_introspection_type_names(
  * If \c type is a basic type, it adds its label, that is \c base_type_name to the
  * \c TypeIntrospectionCollection that it belongs, depending if it is numeric or text kind.
  * Along with the label, it is added the vector of parents as member Ids and kinds.
- * Those are useful to allow to acces this specific field from a \c DynamicData_ptr .
+ * Those are useful to allow to acces this specific field from a \c DynamicData* .
  *
  * If \c type is not a basic type (array or struct) this function is called for each value
  * updating \c base_type_name for each recursive call.
@@ -110,15 +110,15 @@ void get_introspection_type_names(
  */
 void get_introspection_numeric_data(
         const TypeIntrospectionCollection& numeric_type_names,
-        const DynamicData_ptr& data,
+        DynamicData* data,
         TypeIntrospectionNumericStruct& numeric_data_result);
 
 /**
  * @brief TODO
  */
 void get_introspection_string_data(
         const TypeIntrospectionCollection& string_type_names,
-        const DynamicData_ptr& data,
+        DynamicData* data,
         TypeIntrospectionStringStruct& string_data_result);
 
 /**
@@ -132,10 +132,10 @@ void get_introspection_string_data(
  * @param members_tree [in]
  * @param kind [in]
  * @param array_indexes [in]
- * @return DynamicData_ptr
+ * @return DynamicData*
  */
-DynamicData_ptr get_parent_data_of_member(
-        const DynamicData_ptr& data,
+DynamicData* get_parent_data_of_member(
+        DynamicData* data,
         const std::vector<MemberId>& members,
         const std::vector<TypeKind>& kinds,
         unsigned int array_index = 0);
@@ -149,7 +149,7 @@ DynamicData_ptr get_parent_data_of_member(
  * @return numeric data of the member casted to double
  */
 double get_numeric_type_from_data(
-        const DynamicData_ptr& data,
+        DynamicData* data,
         const MemberId& member,
         const TypeKind& kind);
 
@@ -162,7 +162,7 @@ double get_numeric_type_from_data(
  * @return string data of the member casted to string
  */
 std::string get_string_type_from_data(
-        const DynamicData_ptr& data,
+        DynamicData* data,
         const MemberId& member,
         const TypeKind& kind);