From a1d7a1fef1841ca423d46a149624c4205267c901 Mon Sep 17 00:00:00 2001 From: Miaofei Date: Tue, 11 Dec 2018 14:24:47 -0800 Subject: [PATCH 1/5] fix node graph unit tests --- rmw_opensplice_cpp/src/guid.hpp | 123 ++---------------- .../src/rmw_node_info_and_types.cpp | 11 +- rmw_opensplice_cpp/src/types.cpp | 20 +-- rmw_opensplice_cpp/src/types.hpp | 12 +- 4 files changed, 29 insertions(+), 137 deletions(-) diff --git a/rmw_opensplice_cpp/src/guid.hpp b/rmw_opensplice_cpp/src/guid.hpp index 888631af..1c346562 100644 --- a/rmw_opensplice_cpp/src/guid.hpp +++ b/rmw_opensplice_cpp/src/guid.hpp @@ -24,126 +24,19 @@ #include #include -// TODO(ross-desmond): This should all be in opensplice -typedef char octet; - -/** - * Structure to hold GUID information for DDS instances. - * http://www.eprosima.com/docs/fast-rtps/1.6.0/html/_guid_8h_source.html - * - */ -struct GuidPrefix_t -{ - static constexpr unsigned int kSize = 12; - octet value[kSize]; - - GuidPrefix_t() - { - memset(value, 0, kSize); - } - - explicit GuidPrefix_t(octet guid[kSize]) - { - memcpy(value, guid, kSize); - } - - GuidPrefix_t(const GuidPrefix_t & g) - { - memcpy(value, g.value, kSize); - } - - GuidPrefix_t(GuidPrefix_t && g) - { - memmove(value, g.value, kSize); - } - - GuidPrefix_t & operator=(const GuidPrefix_t & guidpre) - { - memcpy(value, guidpre.value, kSize); - return *this; - } - - GuidPrefix_t & operator=(GuidPrefix_t && guidpre) - { - memmove(value, guidpre.value, kSize); - return *this; - } - -#ifndef DOXYGEN_SHOULD_SKIP_THIS_PUBLIC - - bool operator==(const GuidPrefix_t & prefix) const - { - return memcmp(value, prefix.value, kSize) == 0; - } - - bool operator!=(const GuidPrefix_t & prefix) const - { - return memcmp(value, prefix.value, kSize) != 0; - } - -#endif -}; - -inline bool operator<(const GuidPrefix_t & g1, const GuidPrefix_t & g2) -{ - for (uint8_t i = 0; i < GuidPrefix_t::kSize; ++i) { - if (g1.value[i] < g2.value[i]) { - return true; - } else if (g1.value[i] > g2.value[i]) { - return false; - } - } - return false; -} - -inline std::ostream & operator<<(std::ostream & output, const GuidPrefix_t & guiP) -{ - output << std::hex; - for (uint8_t i = 0; i < GuidPrefix_t::kSize - 1; ++i) { - output << static_cast(guiP.value[i]) << "."; - } - output << static_cast(guiP.value[GuidPrefix_t::kSize - 1]); - return output << std::dec; -} - -// TODO(ross-desmond): check this with opensplice source code to ensure compatibility -/** - * Convert an instance handle to a guid. - * - * @param guid [out] the resulting guid - * @param domain_id to prepend to the guid - * @param instance_handle to append to the guid - */ -inline void DDS_InstanceHandle_to_GUID( - struct GuidPrefix_t * guid, - DDS::DomainId_t domain_id, - DDS::InstanceHandle_t instance_handle) -{ - memcpy(guid->value, reinterpret_cast(&domain_id), sizeof(DDS::DomainId_t)); - memcpy(guid->value + sizeof(DDS::DomainId_t), - reinterpret_cast(&instance_handle), sizeof(DDS::InstanceHandle_t)); -} inline void DDS_BuiltinTopicKey_to_GUID( - struct GuidPrefix_t * guid, + DDS::InstanceHandle_t * guid, DDS::BuiltinTopicKey_t buitinTopicKey) { -#if BIG_ENDIAN - /* Big Endian */ - memcpy(guid->value, reinterpret_cast(buitinTopicKey), GuidPrefix_t::kSize); -#else - /* Little Endian */ - int i; - octet * topicKeyBuffer = reinterpret_cast(buitinTopicKey); - for (i = 0; i < 3; ++i) { - octet * guidElement = &guid->value[i * 3]; - octet * keyBufferElement = reinterpret_cast(&buitinTopicKey[i * 3]); - guidElement[0] = keyBufferElement[2]; - guidElement[1] = keyBufferElement[1]; - guidElement[2] = keyBufferElement[0]; - } + v_builtinTopicKey gid; + + // copyInTopicKey(builtinTopicKey, &gid); + gid.systemId = buitinTopicKey[0]; + gid.localId = buitinTopicKey[1]; + gid.serial = buitinTopicKey[2]; -#endif + *guid = u_instanceHandleFromGID(gid); } #endif // GUID_HPP_ diff --git a/rmw_opensplice_cpp/src/rmw_node_info_and_types.cpp b/rmw_opensplice_cpp/src/rmw_node_info_and_types.cpp index ba4218b4..18b9e613 100644 --- a/rmw_opensplice_cpp/src/rmw_node_info_and_types.cpp +++ b/rmw_opensplice_cpp/src/rmw_node_info_and_types.cpp @@ -81,7 +81,7 @@ __get_key( OpenSpliceStaticNodeInfo * node_info, const char * node_name, const char * node_namespace, - GuidPrefix_t & key) + DDS::InstanceHandle_t & key) { auto participant = node_info->participant; if (!participant) { @@ -93,8 +93,7 @@ __get_key( auto dds_ret = participant->get_qos(dpqos); // @todo: ross-desmond implement self discovery if (dds_ret == DDS::RETCODE_OK && __is_node_match(dpqos.user_data, node_name, node_namespace)) { - DDS_InstanceHandle_to_GUID(&key, - node_info->participant->get_domain_id(), node_info->participant->get_instance_handle()); + key = node_info->participant->get_instance_handle(); return RMW_RET_OK; } @@ -177,7 +176,7 @@ rmw_get_subscriber_names_and_types_by_node( } auto node_info = static_cast(node->data); - GuidPrefix_t key; + DDS::InstanceHandle_t key; auto get_guid_err = __get_key(node_info, node_name, node_namespace, key); if (get_guid_err != RMW_RET_OK) { return get_guid_err; @@ -217,7 +216,7 @@ rmw_get_publisher_names_and_types_by_node( } auto node_info = static_cast(node->data); - GuidPrefix_t key; + DDS::InstanceHandle_t key; auto get_guid_err = __get_key(node_info, node_name, node_namespace, key); if (get_guid_err != RMW_RET_OK) { return get_guid_err; @@ -257,7 +256,7 @@ rmw_get_service_names_and_types_by_node( } auto node_info = static_cast(node->data); - GuidPrefix_t key; + DDS::InstanceHandle_t key; auto get_guid_err = __get_key(node_info, node_name, node_namespace, key); if (get_guid_err != RMW_RET_OK) { return get_guid_err; diff --git a/rmw_opensplice_cpp/src/types.cpp b/rmw_opensplice_cpp/src/types.cpp index d0b14102..9e304d6c 100644 --- a/rmw_opensplice_cpp/src/types.cpp +++ b/rmw_opensplice_cpp/src/types.cpp @@ -117,7 +117,7 @@ CustomDataReaderListener::fill_service_names_and_types( void CustomDataReaderListener::fill_topic_names_and_types_by_guid( bool no_demangle, std::map> & tnat, - GuidPrefix_t & participant_guid) + DDS::InstanceHandle_t & participant_guid) { std::lock_guard lock(mutex_); const auto & map = topic_cache.getTopicTypesByGuid(participant_guid); @@ -139,7 +139,7 @@ void CustomDataReaderListener::fill_topic_names_and_types_by_guid( void CustomDataReaderListener::fill_service_names_and_types_by_guid( std::map> & services, - GuidPrefix_t & participant_guid) + DDS::InstanceHandle_t & participant_guid) { std::lock_guard lock(mutex_); const auto & map = topic_cache.getTopicTypesByGuid(participant_guid); @@ -186,8 +186,8 @@ print_discovery_logging( } void CustomDataReaderListener::add_information( - const GuidPrefix_t & participant_guid, - const GuidPrefix_t & topic_guid, + const DDS::InstanceHandle_t & participant_guid, + const DDS::InstanceHandle_t & topic_guid, const std::string & topic_name, const std::string & topic_type, const EndPointType endpoint_type) @@ -199,11 +199,11 @@ void CustomDataReaderListener::add_information( } void CustomDataReaderListener::remove_information( - const GuidPrefix_t & topic_guid, + const DDS::InstanceHandle_t & topic_guid, const EndPointType endpoint_type) { if (print_discovery_logging_) { - TopicCache::TopicInfo topic_info; + TopicCache::TopicInfo topic_info; if (topic_cache.getTopic(topic_guid, topic_info)) { print_discovery_logging("-", topic_info.name, topic_info.type, endpoint_type); } @@ -240,11 +240,11 @@ CustomPublisherListener::on_data_available(DDS::DataReader * reader) for (DDS::ULong i = 0; i < data_seq.length(); ++i) { std::string topic_name = ""; - GuidPrefix_t topic_guid; + DDS::InstanceHandle_t topic_guid; DDS_BuiltinTopicKey_to_GUID(&topic_guid, data_seq[i].key); if (info_seq[i].valid_data && info_seq[i].instance_state == DDS::ALIVE_INSTANCE_STATE) { topic_name = data_seq[i].topic_name.in(); - GuidPrefix_t participant_guid; + DDS::InstanceHandle_t participant_guid; DDS_BuiltinTopicKey_to_GUID(&participant_guid, data_seq[i].participant_key); add_information(participant_guid, topic_guid, topic_name, data_seq[i].type_name.in(), PublisherEP); @@ -292,12 +292,12 @@ CustomSubscriberListener::on_data_available(DDS::DataReader * reader) for (DDS::ULong i = 0; i < data_seq.length(); ++i) { std::string topic_name = ""; - GuidPrefix_t topic_guid; + DDS::InstanceHandle_t topic_guid; DDS_BuiltinTopicKey_to_GUID(&topic_guid, data_seq[i].key); if (info_seq[i].valid_data) { std::string topic_name = ""; - GuidPrefix_t participant_guid; + DDS::InstanceHandle_t participant_guid; DDS_BuiltinTopicKey_to_GUID(&participant_guid, data_seq[i].participant_key); if (info_seq[i].instance_state == DDS::ALIVE_INSTANCE_STATE) { topic_name = data_seq[i].topic_name.in(); diff --git a/rmw_opensplice_cpp/src/types.hpp b/rmw_opensplice_cpp/src/types.hpp index 79219061..468c2c22 100644 --- a/rmw_opensplice_cpp/src/types.hpp +++ b/rmw_opensplice_cpp/src/types.hpp @@ -94,11 +94,11 @@ class CustomDataReaderListener void fill_topic_names_and_types_by_guid( bool no_demangle, std::map> & tnat, - GuidPrefix_t & guid); + DDS::InstanceHandle_t & guid); void fill_service_names_and_types_by_guid( std::map> & services, - GuidPrefix_t & guid); + DDS::InstanceHandle_t & guid); size_t count_topic(const char * topic_name); @@ -118,8 +118,8 @@ class CustomDataReaderListener * @param endpoint_type the endpoint type of this topic instance */ void add_information( - const GuidPrefix_t & participant_guid, - const GuidPrefix_t & topic_guid, + const DDS::InstanceHandle_t & participant_guid, + const DDS::InstanceHandle_t & topic_guid, const std::string & topic_name, const std::string & topic_type, EndPointType endpoint_type); @@ -130,14 +130,14 @@ class CustomDataReaderListener * @param endpoint_type the endpoint type of this topic instance */ void remove_information( - const GuidPrefix_t & topic_guid, + const DDS::InstanceHandle_t & topic_guid, const EndPointType endpoint_type); protected: std::mutex mutex_; // The topic cache to handle relationship of readers, writers, and participants through discovery. - TopicCache topic_cache; + TopicCache topic_cache; private: bool print_discovery_logging_; From ca30481193ca99b4d7cf5faa497edafc42b39a5a Mon Sep 17 00:00:00 2001 From: Miaofei Date: Tue, 11 Dec 2018 14:35:45 -0800 Subject: [PATCH 2/5] cleanup header #includes --- rmw_opensplice_cpp/src/guid.hpp | 7 ------- 1 file changed, 7 deletions(-) diff --git a/rmw_opensplice_cpp/src/guid.hpp b/rmw_opensplice_cpp/src/guid.hpp index 1c346562..be76ee09 100644 --- a/rmw_opensplice_cpp/src/guid.hpp +++ b/rmw_opensplice_cpp/src/guid.hpp @@ -17,13 +17,6 @@ #include -#include -#include -#include -#include -#include -#include - inline void DDS_BuiltinTopicKey_to_GUID( DDS::InstanceHandle_t * guid, From 5b8efb4d5d085339c0d9aace1a3f8d509d529434 Mon Sep 17 00:00:00 2001 From: Miaofei Date: Mon, 17 Dec 2018 13:18:08 -0800 Subject: [PATCH 3/5] addressing comments in code review --- rmw_opensplice_cpp/src/guid.hpp | 8 +-- .../src/rmw_node_info_and_types.cpp | 10 ++-- rmw_opensplice_cpp/src/types.cpp | 54 +++++++++---------- rmw_opensplice_cpp/src/types.hpp | 20 +++---- 4 files changed, 46 insertions(+), 46 deletions(-) diff --git a/rmw_opensplice_cpp/src/guid.hpp b/rmw_opensplice_cpp/src/guid.hpp index be76ee09..864973d5 100644 --- a/rmw_opensplice_cpp/src/guid.hpp +++ b/rmw_opensplice_cpp/src/guid.hpp @@ -18,18 +18,18 @@ #include -inline void DDS_BuiltinTopicKey_to_GUID( - DDS::InstanceHandle_t * guid, +inline void DDS_BuiltinTopicKey_to_InstanceHandle( + DDS::InstanceHandle_t * instance, DDS::BuiltinTopicKey_t buitinTopicKey) { v_builtinTopicKey gid; - // copyInTopicKey(builtinTopicKey, &gid); + // the following logic came from copyInTopicKey() in opensplice source code gid.systemId = buitinTopicKey[0]; gid.localId = buitinTopicKey[1]; gid.serial = buitinTopicKey[2]; - *guid = u_instanceHandleFromGID(gid); + *instance = u_instanceHandleFromGID(gid); } #endif // GUID_HPP_ diff --git a/rmw_opensplice_cpp/src/rmw_node_info_and_types.cpp b/rmw_opensplice_cpp/src/rmw_node_info_and_types.cpp index 18b9e613..c41e4432 100644 --- a/rmw_opensplice_cpp/src/rmw_node_info_and_types.cpp +++ b/rmw_opensplice_cpp/src/rmw_node_info_and_types.cpp @@ -72,7 +72,7 @@ __is_node_match( * @param node_info to discover nodes * @param node_name to match * @param node_namespace to match - * @param key [out] guid key that matches the node name and namespace + * @param key [out] key (an InstanceHandle) that matches the node name and namespace * * @return RMW_RET_OK if success, ERROR otherwise */ @@ -123,7 +123,7 @@ __get_key( if (strcmp(node_name, name.c_str()) == 0 && strcmp(node_namespace, ns.c_str()) == 0) { - DDS_BuiltinTopicKey_to_GUID(&key, pbtd.key); + DDS_BuiltinTopicKey_to_InstanceHandle(&key, pbtd.key); return RMW_RET_OK; } } @@ -183,7 +183,7 @@ rmw_get_subscriber_names_and_types_by_node( } // combine publisher and subscriber information std::map> topics; - node_info->subscriber_listener->fill_topic_names_and_types_by_guid(no_demangle, topics, key); + node_info->subscriber_listener->fill_topic_names_and_types_by_participant(no_demangle, topics, key); rmw_ret_t rmw_ret; rmw_ret = copy_topics_names_and_types(topics, allocator, no_demangle, topic_names_and_types); @@ -224,7 +224,7 @@ rmw_get_publisher_names_and_types_by_node( // combine publisher and subscriber information std::map> topics; - node_info->publisher_listener->fill_topic_names_and_types_by_guid(no_demangle, topics, key); + node_info->publisher_listener->fill_topic_names_and_types_by_participant(no_demangle, topics, key); rmw_ret_t rmw_ret; rmw_ret = copy_topics_names_and_types(topics, allocator, no_demangle, topic_names_and_types); @@ -264,7 +264,7 @@ rmw_get_service_names_and_types_by_node( // combine publisher and subscriber information std::map> services; - node_info->subscriber_listener->fill_service_names_and_types_by_guid(services, key); + node_info->subscriber_listener->fill_service_names_and_types_by_participant(services, key); rmw_ret_t rmw_ret; rmw_ret = copy_services_to_names_and_types(services, allocator, service_names_and_types); diff --git a/rmw_opensplice_cpp/src/types.cpp b/rmw_opensplice_cpp/src/types.cpp index 9e304d6c..70eb5eff 100644 --- a/rmw_opensplice_cpp/src/types.cpp +++ b/rmw_opensplice_cpp/src/types.cpp @@ -114,17 +114,17 @@ CustomDataReaderListener::fill_service_names_and_types( } } -void CustomDataReaderListener::fill_topic_names_and_types_by_guid( +void CustomDataReaderListener::fill_topic_names_and_types_by_participant( bool no_demangle, std::map> & tnat, - DDS::InstanceHandle_t & participant_guid) + DDS::InstanceHandle_t & participant) { std::lock_guard lock(mutex_); - const auto & map = topic_cache.getTopicTypesByGuid(participant_guid); + const auto & map = topic_cache.getTopicTypesByGuid(participant); if (map.size() == 0) { RCUTILS_LOG_DEBUG_NAMED( "rmw_opensplice_cpp", - "No topics for participant_guid"); + "No topics for participant"); return; } for (auto & it : map) { @@ -137,16 +137,16 @@ void CustomDataReaderListener::fill_topic_names_and_types_by_guid( } } -void CustomDataReaderListener::fill_service_names_and_types_by_guid( +void CustomDataReaderListener::fill_service_names_and_types_by_participant( std::map> & services, - DDS::InstanceHandle_t & participant_guid) + DDS::InstanceHandle_t & participant) { std::lock_guard lock(mutex_); - const auto & map = topic_cache.getTopicTypesByGuid(participant_guid); + const auto & map = topic_cache.getTopicTypesByGuid(participant); if (map.size() == 0) { RCUTILS_LOG_DEBUG_NAMED( "rmw_opensplice_cpp", - "No services for participant_guid"); + "No services for participant"); return; } for (auto & it : map) { @@ -186,29 +186,29 @@ print_discovery_logging( } void CustomDataReaderListener::add_information( - const DDS::InstanceHandle_t & participant_guid, - const DDS::InstanceHandle_t & topic_guid, + const DDS::InstanceHandle_t & participant, + const DDS::InstanceHandle_t & topic, const std::string & topic_name, const std::string & topic_type, const EndPointType endpoint_type) { - topic_cache.addTopic(participant_guid, topic_guid, topic_name, topic_type); + topic_cache.addTopic(participant, topic, topic_name, topic_type); if (print_discovery_logging_) { print_discovery_logging("+", topic_name, topic_type, endpoint_type); } } void CustomDataReaderListener::remove_information( - const DDS::InstanceHandle_t & topic_guid, + const DDS::InstanceHandle_t & topic, const EndPointType endpoint_type) { if (print_discovery_logging_) { TopicCache::TopicInfo topic_info; - if (topic_cache.getTopic(topic_guid, topic_info)) { + if (topic_cache.getTopic(topic, topic_info)) { print_discovery_logging("-", topic_info.name, topic_info.type, endpoint_type); } } - topic_cache.removeTopic(topic_guid); + topic_cache.removeTopic(topic); } CustomPublisherListener::CustomPublisherListener(rmw_guard_condition_t * graph_guard_condition) @@ -240,16 +240,16 @@ CustomPublisherListener::on_data_available(DDS::DataReader * reader) for (DDS::ULong i = 0; i < data_seq.length(); ++i) { std::string topic_name = ""; - DDS::InstanceHandle_t topic_guid; - DDS_BuiltinTopicKey_to_GUID(&topic_guid, data_seq[i].key); + DDS::InstanceHandle_t topic; + DDS_BuiltinTopicKey_to_InstanceHandle(&topic, data_seq[i].key); if (info_seq[i].valid_data && info_seq[i].instance_state == DDS::ALIVE_INSTANCE_STATE) { topic_name = data_seq[i].topic_name.in(); - DDS::InstanceHandle_t participant_guid; - DDS_BuiltinTopicKey_to_GUID(&participant_guid, data_seq[i].participant_key); - add_information(participant_guid, topic_guid, topic_name, + DDS::InstanceHandle_t participant; + DDS_BuiltinTopicKey_to_InstanceHandle(&participant, data_seq[i].participant_key); + add_information(participant, topic, topic_name, data_seq[i].type_name.in(), PublisherEP); } else { - remove_information(topic_guid, PublisherEP); + remove_information(topic, PublisherEP); } } @@ -292,22 +292,22 @@ CustomSubscriberListener::on_data_available(DDS::DataReader * reader) for (DDS::ULong i = 0; i < data_seq.length(); ++i) { std::string topic_name = ""; - DDS::InstanceHandle_t topic_guid; + DDS::InstanceHandle_t topic; - DDS_BuiltinTopicKey_to_GUID(&topic_guid, data_seq[i].key); + DDS_BuiltinTopicKey_to_InstanceHandle(&topic, data_seq[i].key); if (info_seq[i].valid_data) { std::string topic_name = ""; - DDS::InstanceHandle_t participant_guid; - DDS_BuiltinTopicKey_to_GUID(&participant_guid, data_seq[i].participant_key); + DDS::InstanceHandle_t participant; + DDS_BuiltinTopicKey_to_InstanceHandle(&participant, data_seq[i].participant_key); if (info_seq[i].instance_state == DDS::ALIVE_INSTANCE_STATE) { topic_name = data_seq[i].topic_name.in(); - add_information(participant_guid, topic_guid, topic_name, + add_information(participant, topic, topic_name, data_seq[i].type_name.in(), SubscriberEP); } else { - remove_information(topic_guid, SubscriberEP); + remove_information(topic, SubscriberEP); } } else { - remove_information(topic_guid, SubscriberEP); + remove_information(topic, SubscriberEP); } } diff --git a/rmw_opensplice_cpp/src/types.hpp b/rmw_opensplice_cpp/src/types.hpp index 468c2c22..6002fc34 100644 --- a/rmw_opensplice_cpp/src/types.hpp +++ b/rmw_opensplice_cpp/src/types.hpp @@ -91,14 +91,14 @@ class CustomDataReaderListener void fill_service_names_and_types( std::map> & services); - void fill_topic_names_and_types_by_guid( + void fill_topic_names_and_types_by_participant( bool no_demangle, std::map> & tnat, - DDS::InstanceHandle_t & guid); + DDS::InstanceHandle_t & participant); - void fill_service_names_and_types_by_guid( + void fill_service_names_and_types_by_participant( std::map> & services, - DDS::InstanceHandle_t & guid); + DDS::InstanceHandle_t & participant); size_t count_topic(const char * topic_name); @@ -111,26 +111,26 @@ class CustomDataReaderListener /** * Add topic pub/sub information to discovery cache. * - * @param participant_guid the topic is related to - * @param topic_guid the topic reader/writer unique id + * @param participant the topic is related to + * @param topic the topic reader/writer unique id * @param topic_name the topic name * @param topic_type the topic type * @param endpoint_type the endpoint type of this topic instance */ void add_information( - const DDS::InstanceHandle_t & participant_guid, - const DDS::InstanceHandle_t & topic_guid, + const DDS::InstanceHandle_t & participant, + const DDS::InstanceHandle_t & topic, const std::string & topic_name, const std::string & topic_type, EndPointType endpoint_type); /** * Remove topic pub/sub information from the discovery cache. - * @param topic_guid the topic reader/writer unique id + * @param topic the topic reader/writer unique id * @param endpoint_type the endpoint type of this topic instance */ void remove_information( - const DDS::InstanceHandle_t & topic_guid, + const DDS::InstanceHandle_t & topic, const EndPointType endpoint_type); protected: From f1f7c0a371972f8fe367c8536f1e85760581a9f0 Mon Sep 17 00:00:00 2001 From: Miaofei Date: Fri, 21 Dec 2018 18:32:26 -0800 Subject: [PATCH 4/5] fix formatting issue caught by linter --- rmw_opensplice_cpp/src/rmw_node_info_and_types.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rmw_opensplice_cpp/src/rmw_node_info_and_types.cpp b/rmw_opensplice_cpp/src/rmw_node_info_and_types.cpp index c41e4432..50229dce 100644 --- a/rmw_opensplice_cpp/src/rmw_node_info_and_types.cpp +++ b/rmw_opensplice_cpp/src/rmw_node_info_and_types.cpp @@ -183,7 +183,8 @@ rmw_get_subscriber_names_and_types_by_node( } // combine publisher and subscriber information std::map> topics; - node_info->subscriber_listener->fill_topic_names_and_types_by_participant(no_demangle, topics, key); + node_info->subscriber_listener->fill_topic_names_and_types_by_participant(no_demangle, topics, + key); rmw_ret_t rmw_ret; rmw_ret = copy_topics_names_and_types(topics, allocator, no_demangle, topic_names_and_types); @@ -224,7 +225,8 @@ rmw_get_publisher_names_and_types_by_node( // combine publisher and subscriber information std::map> topics; - node_info->publisher_listener->fill_topic_names_and_types_by_participant(no_demangle, topics, key); + node_info->publisher_listener->fill_topic_names_and_types_by_participant(no_demangle, topics, + key); rmw_ret_t rmw_ret; rmw_ret = copy_topics_names_and_types(topics, allocator, no_demangle, topic_names_and_types); From cce3be598f4f099559873f23455660d7cbaa04bd Mon Sep 17 00:00:00 2001 From: Miaofei Date: Thu, 24 Jan 2019 13:00:38 -0800 Subject: [PATCH 5/5] address comments from OSRF --- rmw_opensplice_cpp/src/guid.hpp | 13 ++++++------ .../src/rmw_node_info_and_types.cpp | 2 +- rmw_opensplice_cpp/src/types.cpp | 20 ++++++++----------- 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/rmw_opensplice_cpp/src/guid.hpp b/rmw_opensplice_cpp/src/guid.hpp index 864973d5..c81a819c 100644 --- a/rmw_opensplice_cpp/src/guid.hpp +++ b/rmw_opensplice_cpp/src/guid.hpp @@ -18,18 +18,17 @@ #include -inline void DDS_BuiltinTopicKey_to_InstanceHandle( - DDS::InstanceHandle_t * instance, - DDS::BuiltinTopicKey_t buitinTopicKey) +inline DDS::InstanceHandle_t DDS_BuiltinTopicKey_to_InstanceHandle( + DDS::BuiltinTopicKey_t builtinTopicKey) { v_builtinTopicKey gid; // the following logic came from copyInTopicKey() in opensplice source code - gid.systemId = buitinTopicKey[0]; - gid.localId = buitinTopicKey[1]; - gid.serial = buitinTopicKey[2]; + gid.systemId = builtinTopicKey[0]; + gid.localId = builtinTopicKey[1]; + gid.serial = builtinTopicKey[2]; - *instance = u_instanceHandleFromGID(gid); + return u_instanceHandleFromGID(gid); } #endif // GUID_HPP_ diff --git a/rmw_opensplice_cpp/src/rmw_node_info_and_types.cpp b/rmw_opensplice_cpp/src/rmw_node_info_and_types.cpp index 50229dce..ac5c208d 100644 --- a/rmw_opensplice_cpp/src/rmw_node_info_and_types.cpp +++ b/rmw_opensplice_cpp/src/rmw_node_info_and_types.cpp @@ -123,7 +123,7 @@ __get_key( if (strcmp(node_name, name.c_str()) == 0 && strcmp(node_namespace, ns.c_str()) == 0) { - DDS_BuiltinTopicKey_to_InstanceHandle(&key, pbtd.key); + key = DDS_BuiltinTopicKey_to_InstanceHandle(pbtd.key); return RMW_RET_OK; } } diff --git a/rmw_opensplice_cpp/src/types.cpp b/rmw_opensplice_cpp/src/types.cpp index 70eb5eff..11bfc305 100644 --- a/rmw_opensplice_cpp/src/types.cpp +++ b/rmw_opensplice_cpp/src/types.cpp @@ -121,7 +121,7 @@ void CustomDataReaderListener::fill_topic_names_and_types_by_participant( { std::lock_guard lock(mutex_); const auto & map = topic_cache.getTopicTypesByGuid(participant); - if (map.size() == 0) { + if (map.empty()) { RCUTILS_LOG_DEBUG_NAMED( "rmw_opensplice_cpp", "No topics for participant"); @@ -143,7 +143,7 @@ void CustomDataReaderListener::fill_service_names_and_types_by_participant( { std::lock_guard lock(mutex_); const auto & map = topic_cache.getTopicTypesByGuid(participant); - if (map.size() == 0) { + if (map.empty()) { RCUTILS_LOG_DEBUG_NAMED( "rmw_opensplice_cpp", "No services for participant"); @@ -240,12 +240,11 @@ CustomPublisherListener::on_data_available(DDS::DataReader * reader) for (DDS::ULong i = 0; i < data_seq.length(); ++i) { std::string topic_name = ""; - DDS::InstanceHandle_t topic; - DDS_BuiltinTopicKey_to_InstanceHandle(&topic, data_seq[i].key); + DDS::InstanceHandle_t topic = DDS_BuiltinTopicKey_to_InstanceHandle(data_seq[i].key); if (info_seq[i].valid_data && info_seq[i].instance_state == DDS::ALIVE_INSTANCE_STATE) { topic_name = data_seq[i].topic_name.in(); - DDS::InstanceHandle_t participant; - DDS_BuiltinTopicKey_to_InstanceHandle(&participant, data_seq[i].participant_key); + DDS::InstanceHandle_t participant = DDS_BuiltinTopicKey_to_InstanceHandle( + data_seq[i].participant_key); add_information(participant, topic, topic_name, data_seq[i].type_name.in(), PublisherEP); } else { @@ -291,14 +290,11 @@ CustomSubscriberListener::on_data_available(DDS::DataReader * reader) } for (DDS::ULong i = 0; i < data_seq.length(); ++i) { - std::string topic_name = ""; - DDS::InstanceHandle_t topic; - - DDS_BuiltinTopicKey_to_InstanceHandle(&topic, data_seq[i].key); + DDS::InstanceHandle_t topic = DDS_BuiltinTopicKey_to_InstanceHandle(data_seq[i].key); if (info_seq[i].valid_data) { std::string topic_name = ""; - DDS::InstanceHandle_t participant; - DDS_BuiltinTopicKey_to_InstanceHandle(&participant, data_seq[i].participant_key); + DDS::InstanceHandle_t participant = DDS_BuiltinTopicKey_to_InstanceHandle( + data_seq[i].participant_key); if (info_seq[i].instance_state == DDS::ALIVE_INSTANCE_STATE) { topic_name = data_seq[i].topic_name.in(); add_information(participant, topic, topic_name,