From 016db34bf04ee0807a8d394b69340e60e672a769 Mon Sep 17 00:00:00 2001 From: Douglas Wildgrube Bertol Date: Fri, 13 Jan 2023 09:50:23 +0100 Subject: [PATCH] Correct breaks when plotting array data (#50) * Removed utilization of DynamicData_ptr Signed-off-by: Douglas Bertol * Using loaned values despite of coping memory. This prevents memory leaking and makes the code faster. Signed-off-by: Douglas Bertol * Fixed some points related to code review from pull request #50 Signed-off-by: Douglas Bertol Signed-off-by: Douglas Bertol --- .../fastdds/ReaderHandler.cpp | 6 ++- .../fastdds/ReaderHandler.hpp | 3 +- .../utils/dynamic_types_utils.cpp | 37 +++++++++++-------- .../utils/dynamic_types_utils.hpp | 16 ++++---- 4 files changed, 35 insertions(+), 27 deletions(-) diff --git a/plugins/datastreamer_plugin/fastdds/ReaderHandler.cpp b/plugins/datastreamer_plugin/fastdds/ReaderHandler.cpp index e80e3af..66915c9 100644 --- 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 index 90aaec5..fd38421 100644 --- 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 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 index 1f12d0a..42c9897 100644 --- 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& 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(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(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& members_tree, const std::vector& 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 index 88974cd..3bd7298 100644 --- a/plugins/datastreamer_plugin/utils/dynamic_types_utils.hpp +++ b/plugins/datastreamer_plugin/utils/dynamic_types_utils.hpp @@ -65,7 +65,7 @@ std::vector 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,7 +110,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); /** @@ -118,7 +118,7 @@ void get_introspection_numeric_data( */ 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& members, const std::vector& 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);