Address peer review comments

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

rcl/include/rcl/expand_and_remap.h

@@ -0,0 +1,91 @@
+// Copyright 2020 Open Source Robotics Foundation, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef RCL__EXPAND_AND_REMAP_TOPIC_NAME_H_
+#define RCL__EXPAND_AND_REMAP_TOPIC_NAME_H_
+
+#ifdef __cplusplus
+extern "C"
+{
+#endif
+
+#include "rcl/node.h"
+#include "rcl/visibility_control.h"
+
+/// Expand and apply remapping rules in a given name.
+/**
+ * The input_topic_name, node_name, and node_namespace arguments must all be
+ * vaid, null terminated c strings.
+ * The output_topic_name will not be assigned a value in the event of an error.
+ *
+ * The output_topic_name will be null terminated.
+ * It is also allocated, so it needs to be deallocated, when it is no longer
+ * needed, with the same allocator given to this function.
+ * Make sure the `char *` which is passed for the output_topic_name does not
+ * point to allocated memory before calling this function, because it will be
+ * overwritten and therefore leaked if this function is successful.
+ *
+ * The input topic name is validated using rcl_validate_topic_name(),
+ * but if it fails validation RCL_RET_TOPIC_NAME_INVALID is returned.
+ *
+ * The input node name is validated using rmw_validate_node_name(),
+ * but if it fails validation RCL_RET_NODE_INVALID_NAME is returned.
+ *
+ * The input node namespace is validated using rmw_validate_namespace(),
+ * but if it fails validation RCL_RET_NODE_INVALID_NAMESPACE is returned.
+ *
+ * /sa rcl_expand_topic_name
+ * /sa rcl_remap_topic_name
+ * /sa rmw_remap_service_name
+ *
+ * <hr>
+ * Attribute          | Adherence
+ * ------------------ | -------------
+ * Allocates Memory   | Yes
+ * Thread-Safe        | No
+ * Uses Atomics       | No
+ * Lock-Free          | Yes
+ *
+ * \param[in] input_topic_name topic name to be expanded
+ * \param[in] node_name name of the node associated with the topic
+ * \param[in] node_namespace namespace of the node associated with the topic
+ * \param[in] allocator the allocator to be used when creating the output topic
+ * \param[in] is_service indicates that a service name should be expanded when `true`.
+ *  If not, a topic name is expanded.
+ * \param[out] output_topic_name output char * pointer
+ * \return `RCL_RET_OK` if the topic name was expanded successfully, or
+ * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or
+ * \return `RCL_RET_BAD_ALLOC` if allocating memory failed, or
+ * \return `RCL_RET_TOPIC_NAME_INVALID` if the given topic name is invalid, or
+ * \return `RCL_RET_NODE_INVALID_NAME` if the name is invalid, or
+ * \return `RCL_RET_NODE_INVALID_NAMESPACE` if the namespace_ is invalid, or
+ * \return `RCL_RET_UNKNOWN_SUBSTITUTION` for unknown substitutions in name, or
+ * \return `RCL_RET_ERROR` if an unspecified error occurs.
+ */
+RCL_PUBLIC
+RCL_WARN_UNUSED
+rcl_ret_t
+rcl_expand_and_remap_name(
+  const char * input_topic_name,
+  const char * node_name,
+  const char * node_namespace,
+  rcl_allocator_t allocator,
+  bool is_service,
+  char ** output_topic_name);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif  // RCL__EXPAND_AND_REMAP_TOPIC_NAME_H_

rcl/src/expand_and_remap.c

@@ -0,0 +1,99 @@
+// Copyright 2020 Open Source Robotics Foundation, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "rcl/expand_and_remap.h"
+
+#include "rcutils/logging_macros.h"
+
+#include "rcl/expand_topic_name.h"
+#include "rcl/remap.h"
+
+rcl_ret_t
+rcl_expand_and_remap_name(
+  const char * input_topic_name,
+  const char * node_name,
+  const char * node_namespace,
+  rcl_allocator_t * allocator,
+  bool is_service,
+  char ** output_topic_name)
+{
+  // Expand the given topic name.
+  rcutils_allocator_t rcutils_allocator = *allocator;  // implicit conversion to rcutils version
+  rcutils_string_map_t substitutions_map = rcutils_get_zero_initialized_string_map();
+  rcutils_ret_t rcutils_ret = rcutils_string_map_init(&substitutions_map, 0, rcutils_allocator);
+  if (rcutils_ret != RCUTILS_RET_OK) {
+    RCL_SET_ERROR_MSG(rcutils_get_error_string().str);
+    if (rcutils_ret == RCUTILS_RET_BAD_ALLOC) {
+      return RCL_RET_BAD_ALLOC;
+    }
+    return RCL_RET_ERROR;
+  }
+  rcl_ret_t ret = rcl_get_default_topic_name_substitutions(&substitutions_map);
+  if (ret != RCL_RET_OK) {
+    rcutils_ret = rcutils_string_map_fini(&substitutions_map);
+    if (rcutils_ret != RCUTILS_RET_OK) {
+      RCUTILS_LOG_ERROR_NAMED(
+        ROS_PACKAGE_NAME,
+        "failed to fini string_map (%d) during error handling: %s",
+        rcutils_ret,
+        rcutils_get_error_string().str);
+    }
+    if (ret == RCL_RET_BAD_ALLOC) {
+      return ret;
+    }
+    return RCL_RET_ERROR;
+  }
+  char * expanded_topic_name = NULL;
+  ret = rcl_expand_topic_name(
+    input_topic_name,
+    node_name,
+    node_namespace,
+    &substitutions_map,
+    *allocator,
+    &expanded_topic_name);
+  rcutils_ret = rcutils_string_map_fini(&substitutions_map);
+  if (rcutils_ret != RCUTILS_RET_OK) {
+    RCL_SET_ERROR_MSG(rcutils_get_error_string().str);
+    ret = RCL_RET_ERROR;
+    goto cleanup;
+  }
+  if (ret != RCL_RET_OK) {
+    if (ret == RCL_RET_TOPIC_NAME_INVALID || ret == RCL_RET_UNKNOWN_SUBSTITUTION) {
+      ret = RCL_RET_TOPIC_NAME_INVALID;
+    } else {
+      ret = RCL_RET_ERROR;
+    }
+    goto cleanup;
+  }
+  RCUTILS_LOG_DEBUG_NAMED(ROS_PACKAGE_NAME, "Expanded topic name '%s'", expanded_topic_name);
+
+  const rcl_node_options_t * node_options = rcl_node_get_options(node);
+  if (NULL == node_options) {
+    ret = RCL_RET_ERROR;
+    goto cleanup;
+  }
+  rcl_arguments_t * global_args = NULL;
+  if (node_options->use_global_arguments) {
+    global_args = &(node->context->global_arguments);
+  }
+  ret = rcl_remap_topic_name(
+    &(node_options->arguments), global_args, expanded_topic_name,
+    rcl_node_get_name(node), rcl_node_get_namespace(node), *allocator, &remapped_topic_name);
+  if (RCL_RET_OK != ret) {
+    goto fail;
+  } else if (NULL == remapped_topic_name) {
+    remapped_topic_name = expanded_topic_name;
+    expanded_topic_name = NULL;
+  }
+}

rcl/src/rcl/logging_rosout.c

@@ -176,8 +176,8 @@ rcl_ret_t rcl_logging_rosout_init_publisher_for_node(
   // Late joining subscriptions get the last 10 seconds of logs, up to 1000 logs.
   options.qos.depth = 1000;
   options.qos.durability = RMW_QOS_POLICY_DURABILITY_TRANSIENT_LOCAL;
-  options.qos.lifespan.nsec = 0;
   options.qos.lifespan.sec = 10;
+  options.qos.lifespan.nsec = 0;
   new_entry.publisher = rcl_get_zero_initialized_publisher();
   status =
     rcl_publisher_init(&new_entry.publisher, node, type_support, ROSOUT_TOPIC_NAME, &options);

rcl/test/rcl/test_logging.cpp

@@ -0,0 +1,47 @@
+// Copyright 2020 Open Source Robotics Foundation, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <gtest/gtest.h>
+
+#include "rcl/logging.h"
+
+#ifdef RMW_IMPLEMENTATION
+# define CLASSNAME_(NAME, SUFFIX) NAME ## __ ## SUFFIX
+# define CLASSNAME(NAME, SUFFIX) CLASSNAME_(NAME, SUFFIX)
+#else
+# define CLASSNAME(NAME, SUFFIX) NAME
+#endif
+
+class CLASSNAME (TestLoggingFixture, RMW_IMPLEMENTATION) : public ::testing::Test {};
+
+#define CONTAINS_LOGGING_ARGUMENTS(argv) \
+  rcl_contains_logging_arguments(sizeof(argv) / sizeof(const char *), argv)
+
+TEST_F(CLASSNAME(TestLoggingFixture, RMW_IMPLEMENTATION), contains_logging_arguments) {
+  const char * with_logging_arguments[] = {
+    "--log-level", "debug", "--log-config-file", "asd.config"
+  };
+  const char * with_one_logging_argument[] = {"--enable-stdout-logs", "bsd"};
+  const char * with_mixed_arguments[] = {
+    "asd", "--log-level", "debug", "bsd", "--log-config-file", "asd.config"
+  };
+  const char * with_other_argument[] = {"--asd"};
+  const char * with_other_arguments[] = {"--asd", "bsd"};
+
+  EXPECT_TRUE(CONTAINS_LOGGING_ARGUMENTS(with_logging_arguments));
+  EXPECT_TRUE(CONTAINS_LOGGING_ARGUMENTS(with_one_logging_argument));
+  EXPECT_TRUE(CONTAINS_LOGGING_ARGUMENTS(with_mixed_arguments));
+  EXPECT_FALSE(CONTAINS_LOGGING_ARGUMENTS(with_other_argument));
+  EXPECT_FALSE(CONTAINS_LOGGING_ARGUMENTS(with_other_arguments));
+}