From 0789d9586da41299470db4036204a284d6c636f0 Mon Sep 17 00:00:00 2001 From: tempate Date: Mon, 15 Jul 2024 16:27:35 +0200 Subject: [PATCH] Apply suggestions Signed-off-by: tempate --- .../communication/dds/DdsBridge.hpp | 8 +++- .../include/ddspipe_core/core/DdsPipe.hpp | 7 +++- .../src/cpp/communication/dds/DdsBridge.cpp | 39 ++++++++++++------- ddspipe_core/src/cpp/core/DdsPipe.cpp | 12 +++--- 4 files changed, 41 insertions(+), 25 deletions(-) diff --git a/ddspipe_core/include/ddspipe_core/communication/dds/DdsBridge.hpp b/ddspipe_core/include/ddspipe_core/communication/dds/DdsBridge.hpp index 474d4ef8..1fadfb64 100644 --- a/ddspipe_core/include/ddspipe_core/communication/dds/DdsBridge.hpp +++ b/ddspipe_core/include/ddspipe_core/communication/dds/DdsBridge.hpp @@ -18,7 +18,6 @@ #include #include -#include #include #include #include @@ -52,7 +51,10 @@ class DdsBridge : public Bridge * @param participant_database: Collection of Participants to manage communication * @param payload_pool: Payload Pool that handles the reservation/release of payloads throughout the DDS Router * @param thread_pool: Shared pool of threads in charge of data transmission. - * @param enable: Whether the Bridge should be initialized as enabled + * @param routes_config: Configuration of the routes of the Bridge + * @param remove_unused_entities: Whether the Bridge should remove unused entities + * @param manual_topics: Topics that explicitally set a QoS attribute for this participant + * @param endpoint_kind: Kind of the endpoint that discovered the topic * * @throw InitializationException in case \c IWriters or \c IReaders creation fails. */ @@ -200,6 +202,8 @@ class DdsBridge : public Bridge * @brief Impose the Topic QoS that have been pre-configured for a participant. * * First, it imposes the Topic QoS configured at \c manual_topics and then the ones configured at \c participants. + * + * @param participant: The participant to impose the QoS on. */ utils::Heritable create_topic_for_participant_nts_( const std::shared_ptr& participant) noexcept; diff --git a/ddspipe_core/include/ddspipe_core/core/DdsPipe.hpp b/ddspipe_core/include/ddspipe_core/core/DdsPipe.hpp index d6471f82..36322db3 100644 --- a/ddspipe_core/include/ddspipe_core/core/DdsPipe.hpp +++ b/ddspipe_core/include/ddspipe_core/core/DdsPipe.hpp @@ -261,6 +261,7 @@ class DdsPipe * @note This is the only method that adds topics to \c current_topics_ * * @param [in] topic : topic discovered + * @param [in] endpoint_kind : kind of the endpoint */ void discovered_topic_nts_( const utils::Heritable& topic, @@ -303,11 +304,12 @@ class DdsPipe * It is created enabled if the DdsPipe is enabled. * * @param [in] topic : new topic + * @param [in] enabled : whether to enable the bridge on creation or not */ void create_new_bridge_nts_( const utils::Heritable& topic, - bool enabled = false, - const types::EndpointKind& endpoint_kind = types::EndpointKind::reader) noexcept; + const types::EndpointKind endpoint_kind = types::EndpointKind::reader, + bool enabled = false) noexcept; /** * @brief Create a new \c RpcBridge object @@ -325,6 +327,7 @@ class DdsPipe * If the topic did not exist before, the Bridge is created. * * @param [in] topic : Topic to be enabled + * @param [in] endpoint_kind : Kind of endpoint who discovered the topic */ void activate_topic_nts_( const utils::Heritable& topic, diff --git a/ddspipe_core/src/cpp/communication/dds/DdsBridge.cpp b/ddspipe_core/src/cpp/communication/dds/DdsBridge.cpp index c01d54bd..46b81cdc 100644 --- a/ddspipe_core/src/cpp/communication/dds/DdsBridge.cpp +++ b/ddspipe_core/src/cpp/communication/dds/DdsBridge.cpp @@ -110,13 +110,17 @@ void DdsBridge::create_endpoint( { std::lock_guard lock(mutex_); - if (discovered_endpoint_kind == EndpointKind::reader) + switch (discovered_endpoint_kind) { - create_writer_and_its_tracks_nts_(participant_id); - } - else - { - create_reader_and_its_track_nts_(participant_id); + case EndpointKind::reader: + create_writer_and_its_tracks_nts_(participant_id); + break; + case EndpointKind::writer: + create_reader_and_its_track_nts_(participant_id); + break; + default: + logError(DDSPIPE_DDSBRIDGE, "Invalid kind " << discovered_endpoint_kind << " to create an endpoint."); + break; } } @@ -126,13 +130,17 @@ void DdsBridge::remove_endpoint( { std::lock_guard lock(mutex_); - if (removed_endpoint_kind == EndpointKind::reader) - { - remove_writer_and_its_tracks_nts_(participant_id); - } - else + switch (removed_endpoint_kind) { - remove_reader_and_its_track_nts_(participant_id); + case EndpointKind::reader: + remove_writer_and_its_tracks_nts_(participant_id); + break; + case EndpointKind::writer: + remove_reader_and_its_track_nts_(participant_id); + break; + default: + logError(DDSPIPE_DDSBRIDGE, "Invalid kind " << removed_endpoint_kind << " to remove an endpoint."); + break; } } @@ -196,15 +204,16 @@ void DdsBridge::remove_writer_and_its_tracks_nts_( assert(participant_id != DEFAULT_PARTICIPANT_ID); // Remove the writer from the tracks and remove the tracks without writers - for (const auto& track_it : tracks_) + for (auto it = tracks_.cbegin(), next_it = it; it != tracks_.cend(); it = next_it) { - auto& track = track_it.second; + ++next_it; + const auto& track = it->second; track->remove_writer(participant_id); if (!track->has_writers()) { - tracks_.erase(track_it.first); + tracks_.erase(it); } } diff --git a/ddspipe_core/src/cpp/core/DdsPipe.cpp b/ddspipe_core/src/cpp/core/DdsPipe.cpp index 46d491a4..5eb292b9 100644 --- a/ddspipe_core/src/cpp/core/DdsPipe.cpp +++ b/ddspipe_core/src/cpp/core/DdsPipe.cpp @@ -391,13 +391,13 @@ bool DdsPipe::is_endpoint_relevant_( if (endpoint.active) { - // An active reader is relevant when it is the only active reader in a topic + // An active endpoint is relevant when it is the only active endpoint in a topic // with a discoverer participant id. return relevant_endpoints.size() == 1 && relevant_endpoints.count(endpoint.guid); } else { - // An inactive reader is relevant when there aren't any active readers in a topic + // An inactive endpoint is relevant when there aren't any active endpoints in a topic // with a discoverer participant id. return relevant_endpoints.size() == 0; } @@ -409,7 +409,7 @@ void DdsPipe::init_bridges_nts_( for (const auto& topic : builtin_topics) { discovered_topic_nts_(topic); - create_new_bridge_nts_(topic, false); + create_new_bridge_nts_(topic, EndpointKind::reader, false); } } @@ -487,8 +487,8 @@ void DdsPipe::removed_service_nts_( void DdsPipe::create_new_bridge_nts_( const utils::Heritable& topic, - bool enabled, /*= false*/ - const EndpointKind& endpoint_kind /*= EndpointKind::reader*/) noexcept + const EndpointKind endpoint_kind /*= EndpointKind::reader*/, + bool enabled /*= false*/) noexcept { EPROSIMA_LOG_INFO(DDSPIPE, "Creating Bridge for topic: " << topic << "."); @@ -546,7 +546,7 @@ void DdsPipe::activate_topic_nts_( if (it_bridge == bridges_.end()) { // The Bridge did not exist - create_new_bridge_nts_(topic, true, endpoint_kind); + create_new_bridge_nts_(topic, endpoint_kind, true); } else {