Refs #22682: Add suggested changes

Signed-off-by: Carlosespicur <[email protected]>
eProsima · Feb 28, 2025 · 5dc3dd5 · 5dc3dd5
1 parent 95d0990
commit 5dc3dd5
Show file tree

Hide file tree

Showing 6 changed files with 49 additions and 24 deletions.
diff --git a/src/cpp/fastdds/domain/DomainParticipantImpl.cpp b/src/cpp/fastdds/domain/DomainParticipantImpl.cpp
@@ -2024,12 +2024,13 @@ rpc::Service* DomainParticipantImpl::create_service(
     }
 
     // Check if the request/reply content filter factory is registered. If not, register it.
-    IContentFilterFactory* factory = find_content_filter_factory(rpc::__BUILTIN_REQUEST_REPLY_CONTENT_FILTER__);
+    IContentFilterFactory* factory = find_content_filter_factory(rpc::RequestReplyContentFilterFactory::FILTER_NAME);
 
     if (!factory)
     {
         factory = new rpc::RequestReplyContentFilterFactory();
-        ReturnCode_t ret_code = register_content_filter_factory(rpc::__BUILTIN_REQUEST_REPLY_CONTENT_FILTER__, factory);
+        ReturnCode_t ret_code =
+                register_content_filter_factory(rpc::RequestReplyContentFilterFactory::FILTER_NAME, factory);
 
         if (RETCODE_OK != ret_code)
         {
@@ -2129,11 +2130,11 @@ ReturnCode_t DomainParticipantImpl::delete_service(
         }
 
         // Check that the service is disabled
-        if (!it->second->is_enabled())
+        if (it->second->is_enabled())
         {
             EPROSIMA_LOG_INFO(PARTICIPANT, "Trying to delete an enabled service.");
-            ReturnCode_t retcode;
-            if (RETCODE_OK != (retcode = it->second->close()))
+            ReturnCode_t retcode = it->second->close();
+            if (RETCODE_OK != retcode)
             {
                 EPROSIMA_LOG_ERROR(PARTICIPANT, "Error closing service: " << retcode);
                 return retcode;

diff --git a/src/cpp/fastdds/rpc/ReplierImpl.cpp b/src/cpp/fastdds/rpc/ReplierImpl.cpp
@@ -128,7 +128,9 @@ ReturnCode_t ReplierImpl::enable()
             return RETCODE_PRECONDITION_NOT_MET;
         }
 
-        if (RETCODE_OK != (retcode = create_dds_entities(qos_)))
+        retcode = create_dds_entities(qos_);
+
+        if (RETCODE_OK != retcode)
         {
             EPROSIMA_LOG_ERROR(REPLIER, "Unable to enable replier");
             // If any error occurs, delete the created entities
@@ -149,7 +151,9 @@ ReturnCode_t ReplierImpl::close()
 
     if (enabled_)
     {
-        if (RETCODE_OK != (retcode = delete_contained_entities()))
+        retcode = delete_contained_entities();
+
+        if (RETCODE_OK != retcode)
         {
             EPROSIMA_LOG_ERROR(REPLIER, "Error deleting DDS entities");
             return retcode;

diff --git a/src/cpp/fastdds/rpc/RequestReplyContentFilter.hpp b/src/cpp/fastdds/rpc/RequestReplyContentFilter.hpp
@@ -24,7 +24,8 @@ namespace rpc {
 
 /**
  * @brief Content filter that allows to filter samples based on the GUID of the reader that received the sample
- * @note This filter is used to filter samples in a multiple requester - single replier service scenario. Each requester will receive only the replies that match some of its requests
+ * @note This filter is used to filter samples in a multiple requester - single replier service scenario.
+ * Each requester will receive only the replies that match their requests
  */
 class RequestReplyContentFilter : public IContentFilter
 {

diff --git a/src/cpp/fastdds/rpc/RequestReplyContentFilterFactory.hpp b/src/cpp/fastdds/rpc/RequestReplyContentFilterFactory.hpp
@@ -24,15 +24,15 @@ namespace fastdds {
 namespace dds {
 namespace rpc {
 
-static const char* __BUILTIN_REQUEST_REPLY_CONTENT_FILTER__ = "REQUEST_REPLY_CONTENT_FILTER";
-
 /**
  * @brief This class represents the factory used to create the filter for Request/Reply topics
  */
 class RequestReplyContentFilterFactory : public IContentFilterFactory
 {
 public:
 
+    constexpr static const char* FILTER_NAME = "__BUILTIN_REQUEST_REPLY_CONTENT_FILTER__";
+
     ReturnCode_t create_content_filter(
             const char* filter_class_name,
             const char* type_name,
@@ -46,12 +46,11 @@ class RequestReplyContentFilterFactory : public IContentFilterFactory
         static_cast<void>(filter_expression);
         static_cast<void>(filter_parameters);
 
-        if (0 != strcmp(filter_class_name, __BUILTIN_REQUEST_REPLY_CONTENT_FILTER__))
+        if (0 != strcmp(filter_class_name, FILTER_NAME))
         {
             return RETCODE_BAD_PARAMETER;
         }
 
-        // TODO: return RETCODE_BAD_PARAMETER if type_name is not a request/reply type?
         filter_instance = &filter_instance_;
 
         return RETCODE_OK;
@@ -63,7 +62,7 @@ class RequestReplyContentFilterFactory : public IContentFilterFactory
     {
         static_cast<void>(filter_instance);
 
-        if (0 != strcmp(filter_class_name, __BUILTIN_REQUEST_REPLY_CONTENT_FILTER__))
+        if (0 != strcmp(filter_class_name, FILTER_NAME))
         {
             return RETCODE_BAD_PARAMETER;
         }

diff --git a/src/cpp/fastdds/rpc/RequesterImpl.cpp b/src/cpp/fastdds/rpc/RequesterImpl.cpp
@@ -115,7 +115,9 @@ ReturnCode_t RequesterImpl::enable()
             return RETCODE_PRECONDITION_NOT_MET;
         }
 
-        if (RETCODE_OK != (retcode = create_dds_entities(qos_)))
+        retcode = create_dds_entities(qos_);
+
+        if (RETCODE_OK != retcode)
         {
             EPROSIMA_LOG_ERROR(REQUESTER, "Error creating DDS entities");
             // If any error occurs, delete the created entities
@@ -136,7 +138,9 @@ ReturnCode_t RequesterImpl::close()
 
     if (enabled_)
     {
-        if (RETCODE_OK != (retcode = delete_contained_entities()))
+        retcode = delete_contained_entities();
+
+        if (RETCODE_OK != retcode)
         {
             EPROSIMA_LOG_ERROR(REQUESTER, "Error deleting DDS entities");
             return retcode;

diff --git a/src/cpp/fastdds/rpc/ServiceImpl.cpp b/src/cpp/fastdds/rpc/ServiceImpl.cpp
@@ -69,7 +69,9 @@ ReturnCode_t ServiceImpl::remove_requester(
     }
 
     // In case the requester is enabled, disable it first
-    if (RETCODE_OK != (ret = (*it)->close()))
+    ret = (*it)->close();
+
+    if (RETCODE_OK != ret)
     {
         EPROSIMA_LOG_ERROR(SERVICE, "Error closing Requester for Service '" << service_name_ << "'");
         return ret;
@@ -94,7 +96,9 @@ ReturnCode_t ServiceImpl::remove_replier(
     }
 
     // In case the replier is enabled, disable it first
-    if (RETCODE_OK != (ret = (*it)->close()))
+    ret = (*it)->close();
+
+    if (RETCODE_OK != ret)
     {
         EPROSIMA_LOG_ERROR(SERVICE, "Error closing Replier for Service '" << service_name_ << "'");
         return ret;
@@ -190,7 +194,9 @@ ReturnCode_t ServiceImpl::enable()
     if (!enabled_)
     {
         // Create request/reply topics
-        if (RETCODE_OK != (ret = create_request_reply_topics()))
+        ret = create_request_reply_topics();
+
+        if (RETCODE_OK != ret)
         {
             EPROSIMA_LOG_ERROR(SERVICE, "Error creating request/reply topics for Service '" << service_name_ << "'");
             return ret;
@@ -205,7 +211,9 @@ ReturnCode_t ServiceImpl::enable()
             std::lock_guard<std::mutex> lock(mtx_requesters_);
             for (auto requester : requesters_)
             {
-                if (RETCODE_OK != (ret = requester->enable()))
+                ret = requester->enable();
+
+                if (RETCODE_OK != ret)
                 {
                     EPROSIMA_LOG_ERROR(SERVICE, "Error enabling Requester for Service '" << service_name_ << "'");
                 }
@@ -222,7 +230,9 @@ ReturnCode_t ServiceImpl::enable()
             std::lock_guard<std::mutex> lock(mtx_repliers_);
             for (auto replier : repliers_)
             {
-                if (RETCODE_OK != (ret = replier->enable()))
+                ret = replier->enable();
+
+                if (RETCODE_OK != ret)
                 {
                     EPROSIMA_LOG_ERROR(SERVICE, "Error enabling Replier for Service '" << service_name_ << "'");
                 }
@@ -248,7 +258,9 @@ ReturnCode_t ServiceImpl::close()
             std::lock_guard<std::mutex> lock(mtx_requesters_);
             for (auto requester : requesters_)
             {
-                if (RETCODE_OK != (ret = requester->close()))
+                ret = requester->close();
+
+                if (RETCODE_OK != ret)
                 {
                     EPROSIMA_LOG_ERROR(SERVICE, "Error closing Requester for Service '" << service_name_ << "'");
                     return ret;
@@ -260,7 +272,9 @@ ReturnCode_t ServiceImpl::close()
             std::lock_guard<std::mutex> lock(mtx_repliers_);
             for (auto replier : repliers_)
             {
-                if (RETCODE_OK != (ret = replier->close()))
+                ret = replier->close();
+
+                if (RETCODE_OK != ret)
                 {
                     EPROSIMA_LOG_ERROR(SERVICE, "Error closing Replier for Service '" << service_name_ << "'");
                     return ret;
@@ -321,7 +335,7 @@ ReturnCode_t ServiceImpl::create_request_reply_topics()
         reply_topic_,
         " ",
         std::vector<std::string>(),
-        __BUILTIN_REQUEST_REPLY_CONTENT_FILTER__);
+        rpc::RequestReplyContentFilterFactory::FILTER_NAME);
 
     if (!reply_filtered_topic_)
     {
@@ -338,7 +352,9 @@ ReturnCode_t ServiceImpl::delete_contained_entities()
     ReturnCode_t ret = RETCODE_OK;
 
     // Close the Service
-    if (RETCODE_OK != (ret = close()))
+    ret = close();
+
+    if (RETCODE_OK != ret)
     {
         EPROSIMA_LOG_ERROR(SERVICE, "Error closing Service '" << service_name_ << "'");
         return ret;