Apply reviewer suggestion

Signed-off-by: Ivan Santiago Paunovic <[email protected]>

rmw_connextdds_common/include/rmw_connextdds/context.hpp

@@ -85,7 +85,21 @@ struct rmw_context_impl_t
   std::string qos_ctx_name;
   std::string qos_ctx_namespace;
   bool override_publish_mode;
-  bool use_topic_profiles;
+
+  enum class qos_profile_loading_policy_t
+  {
+    // ROS QoS are used, but if a profile matching the topic name is found that one will be used
+    // and the ROS QoS settings will be ignored.
+    ProfileByTopic,
+    // Ignore ROS qos settings, load qos from an external qos profile file using topic filters.
+    DdsDefault,
+    // Use ROS qos settings.
+    // Non ROS qos settings and ROS settings set to "SYSTEM DEFAULT" can be loaded from an external
+    // qos profile file which can use topic filters.
+    None
+  };
+
+  qos_profile_loading_policy_t qos_profile_loading_policy;
 
   /* Participant reference count*/
   size_t node_count{0};

rmw_connextdds_common/include/rmw_connextdds/static_config.hpp

@@ -49,8 +49,8 @@
 #define RMW_CONNEXT_ENV_DO_NOT_OVERRIDE_PUBLISH_MODE     "RMW_CONNEXT_DO_NOT_OVERRIDE_PUBLISH_MODE"
 #endif /* RMW_CONNEXT_ENV_DO_NOT_OVERRIDE_PUBLISH_MODE */
 
-#ifndef RMW_CONNEXT_ENV_ALLOW_TOPIC_QOS_PROFILES
-#define RMW_CONNEXT_ENV_ALLOW_TOPIC_QOS_PROFILES     "RMW_CONNEXT_ALLOW_TOPIC_QOS_PROFILES"
+#ifndef RMW_CONNEXT_ENV_OVERRIDE_ENDPOINT_QOS_POLICY
+#define RMW_CONNEXT_ENV_OVERRIDE_ENDPOINT_QOS_POLICY     "RMW_CONNEXT_OVERRIDE_ENDPOINT_QOS_POLICY"
 #endif /* RMW_CONNEXT_ENV_ALLOW_TOPIC_QOS_PROFILES */
 
 

rmw_connextdds_common/src/common/rmw_context.cpp

@@ -204,24 +204,33 @@ rmw_context_impl_t::initialize_node(
   this->override_publish_mode = '\0' == do_not_override_publish_mode_env[0];
 
   /* Lookup RMW_CONNEXT_ENV_ALLOW_TOPIC_QOS_PROFILES env variable.*/
-  const char * use_topic_profiles = nullptr;
+  const char * endpoint_qos_policy = nullptr;
   lookup_rc = rcutils_get_env(
-    RMW_CONNEXT_ENV_ALLOW_TOPIC_QOS_PROFILES, &use_topic_profiles);
+    RMW_CONNEXT_ENV_OVERRIDE_ENDPOINT_QOS_POLICY, &endpoint_qos_policy);
 
-  if (nullptr != lookup_rc || nullptr == use_topic_profiles) {
+  if (nullptr != lookup_rc || nullptr == endpoint_qos_policy) {
     RMW_CONNEXT_LOG_ERROR_A_SET(
       "failed to lookup from environment: "
       "var=%s, "
       "rc=%s ",
-      RMW_CONNEXT_ENV_ALLOW_TOPIC_QOS_PROFILES,
+      RMW_CONNEXT_ENV_OVERRIDE_ENDPOINT_QOS_POLICY,
       lookup_rc)
     return RMW_RET_ERROR;
   }
 
-  // When RMW_CONNEXT_ENV_ALLOW_TOPIC_QOS_PROFILES is set, if a profile matching the topic name is
-  // found that will be used and the ros profile will be ignored.
-  // If not profile the topic name is found, the ros profile will be used.
-  this->use_topic_profiles = '\0' == use_topic_profiles[0];
+  this->qos_profile_loading_policy = rmw_context_impl_t::qos_profile_loading_policy_t::None;
+  if (0 == strcmp(endpoint_qos_policy, "profile_by_topic_name")) {
+    this->qos_profile_loading_policy =
+      rmw_context_impl_t::qos_profile_loading_policy_t::ProfileByTopic;
+  } else if (0 == strcmp(endpoint_qos_policy, "dds_default")) {
+    this->qos_profile_loading_policy = rmw_context_impl_t::qos_profile_loading_policy_t::DdsDefault;
+  } else if (endpoint_qos_policy[0] != '\0' && strcmp(endpoint_qos_policy, "none") != 0) {
+    RMW_CONNEXT_LOG_INFO_A(
+      "Environment variable {%s} has an unexpected value {%s}."
+      "Allowed values are {profile_by_topic_name}, {dds_default} or {none}.",
+      RMW_CONNEXT_ENV_OVERRIDE_ENDPOINT_QOS_POLICY,
+      endpoint_qos_policy);
+  }
 
   if (RMW_RET_OK != rmw_connextdds_initialize_participant_factory(this)) {
     RMW_CONNEXT_LOG_ERROR(

rmw_connextdds_common/src/common/rmw_impl.cpp

@@ -1260,11 +1260,22 @@ RMW_Connext_Publisher::create(
       }
     });
 
-  if (DDS_RETCODE_OK !=
-    DDS_Publisher_get_default_datawriter_qos(pub, &dw_qos))
+  if (ctx->qos_profile_loading_policy !=
+    rmw_context_impl_t::qos_profile_loading_policy_t::ProfileByTopic)
   {
-    RMW_CONNEXT_LOG_ERROR_SET("failed to get default writer QoS")
-    return nullptr;
+    if (DDS_RETCODE_OK !=
+      DDS_Publisher_get_default_datawriter_qos_w_topic_name(pub, &dw_qos, fqtopic_name.c_str()))
+    {
+      RMW_CONNEXT_LOG_ERROR_SET("failed to get default writer QoS")
+      return nullptr;
+    }
+  } else {
+    if (DDS_RETCODE_OK !=
+      DDS_Publisher_get_default_datawriter_qos(pub, &dw_qos))
+    {
+      RMW_CONNEXT_LOG_ERROR_SET("failed to get default writer QoS")
+      return nullptr;
+    }
   }
 
   DDS_DataWriter * const dds_writer =
@@ -1784,11 +1795,22 @@ RMW_Connext_Subscriber::create(
       DDS_DataReaderQos_finalize(dr_qos_ptr);
     });
 
-  if (DDS_RETCODE_OK !=
-    DDS_Subscriber_get_default_datareader_qos(sub, &dr_qos))
+  if (ctx->qos_profile_loading_policy !=
+    rmw_context_impl_t::qos_profile_loading_policy_t::ProfileByTopic)
   {
-    RMW_CONNEXT_LOG_ERROR_SET("failed to get default reader QoS")
-    return nullptr;
+    if (DDS_RETCODE_OK !=
+      DDS_Subscriber_get_default_datareader_qos_w_topic_name(sub, &dr_qos, fqtopic_name.c_str()))
+    {
+      RMW_CONNEXT_LOG_ERROR_SET("failed to get default reader QoS")
+      return nullptr;
+    }
+  } else {
+    if (DDS_RETCODE_OK !=
+      DDS_Subscriber_get_default_datareader_qos(sub, &dr_qos))
+    {
+      RMW_CONNEXT_LOG_ERROR_SET("failed to get default reader QoS")
+      return nullptr;
+    }
   }
 
   DDS_DataReader * dds_reader =

rmw_connextdds_common/src/ndds/dds_api_ndds.cpp

@@ -351,8 +351,15 @@ rmw_connextdds_get_datawriter_qos(
 {
   UNUSED_ARG(topic);
 
-  bool ignore_ros_profile = false;
-  if (ctx->use_topic_profiles && ctx->qos_library != "") {
+  bool ignore_ros_profile = (
+    ctx->qos_profile_loading_policy ==
+    rmw_context_impl_t::qos_profile_loading_policy_t::DdsDefault);
+
+  if (
+    ctx->qos_profile_loading_policy ==
+    rmw_context_impl_t::qos_profile_loading_policy_t::ProfileByTopic &&
+    ctx->qos_library != "")
+  {
     DDS_DomainParticipantFactory * dpf = DDS_DomainParticipantFactory_get_instance();
     struct DDS_StringSeq profile_names = DDS_SEQUENCE_INITIALIZER;
     DDS_ReturnCode_t ret_code = DDS_DomainParticipantFactory_get_qos_profiles(
@@ -432,8 +439,15 @@ rmw_connextdds_get_datareader_qos(
   UNUSED_ARG(ctx);
   UNUSED_ARG(topic_desc);
 
-  bool ignore_ros_profile = false;
-  if (ctx->use_topic_profiles && ctx->qos_library != "") {
+  bool ignore_ros_profile = (
+    ctx->qos_profile_loading_policy ==
+    rmw_context_impl_t::qos_profile_loading_policy_t::DdsDefault);
+
+  if (
+    ctx->qos_profile_loading_policy ==
+    rmw_context_impl_t::qos_profile_loading_policy_t::ProfileByTopic &&
+    ctx->qos_library != "")
+  {
     DDS_DomainParticipantFactory * dpf = DDS_DomainParticipantFactory_get_instance();
     struct DDS_StringSeq profile_names = DDS_SEQUENCE_INITIALIZER;
     if (
@@ -454,8 +468,6 @@ rmw_connextdds_get_datareader_qos(
     }
   }
 
-// DDS_DomainParticipantFactory_get_qos_profiles
-
   if (!ignore_ros_profile) {
     if (RMW_RET_OK !=
       rmw_connextdds_get_readerwriter_qos(