Address reviewer feedback.

Signed-off-by: Michael Carroll <[email protected]>

rcl/include/rcl/subscription.h

@@ -26,6 +26,8 @@ extern "C"
 #include "rcl/node.h"
 #include "rcl/visibility_control.h"
 
+#include "rmw/message_sequence.h"
+
 /// Internal rcl implementation struct.
 struct rcl_subscription_impl_t;
 
@@ -203,7 +205,7 @@ rcl_subscription_get_default_options(void);
 /// Take a ROS message from a topic using a rcl subscription.
 /**
  * It is the job of the caller to ensure that the type of the ros_message
- * argument and the type associate with the subscription, via the type
+ * argument and the type associated with the subscription, via the type
  * support, match.
  * Passing a different type to rcl_take produces undefined behavior and cannot
  * be checked by this function and therefore no deliberate error will occur.
@@ -227,7 +229,7 @@ rcl_subscription_get_default_options(void);
  * be allocated for a dynamically sized array in the target message, then the
  * allocator given in the subscription options is used.
  *
- * The rmw message_info struct contains meta information about this particular
+ * The rmw_message_info struct contains meta information about this particular
  * message instance, like what the GUID of the publisher which published it
  * originally or whether or not the message received from within the same
  * process.
@@ -266,6 +268,48 @@ rcl_take(
   rmw_subscription_allocation_t * allocation
 );
 
+/// Take a sequence of messages from a topic using a rcl subscription.
+/**
+ * In contrast to `rcl_take`, this function can take multiple messages at
+ * the same time.
+ * It is the job of the caller to enusre that the type of the message_sequence
+ * argument and the type associated with the subscription, via the type
+ * support, match.
+ *
+ * The message_sequence pointer should point to an already allocated sequence
+ * of ROS messages of the correct type, into which the taken ROS messages will
+ * be copied if messages are available.
+ * The message_sequence `size` member will be set to the number of messages
+ * correctly taken.
+ *
+ * The rmw_message_info_sequence struct contains meta information about the
+ * corresponding message instance index.
+ * The message_info_sequence argument should be an already allocated
+ * rmw_message_info_sequence_t structure.
+ *
+ * <hr>
+ * Attribute          | Adherence
+ * ------------------ | -------------
+ * Allocates Memory   | Maybe [1]
+ * Thread-Safe        | No
+ * Uses Atomics       | No
+ * Lock-Free          | Yes
+ * <i>[1] only if storage in the serialized_message is insufficient</i>
+ *
+ * \param[in] subscription the handle to the subscription from which to take.
+ * \param[in] count number of messages to attempt to take.
+ * \param[inout] message_sequence pointer to a (pre-allocated) message sequence.
+ * \param[inout] message_info_sequence pointer to a (pre-allocated) message info sequence .
+ * \param[in] allocation structure pointer used for memory preallocation (may be NULL)
+ * \return `RCL_RET_OK` if the message was published, or
+ * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or
+ * \return `RCL_RET_SUBSCRIPTION_INVALID` if the subscription is invalid, or
+ * \return `RCL_RET_BAD_ALLOC` if allocating memory failed, or
+ * \return `RCL_RET_SUBSCRIPTION_TAKE_FAILED` if take failed but no error
+ *         occurred in the middleware, or
+ * \return `RCL_RET_ERROR` if an unspecified error occurs.
+ */
+
 RCL_PUBLIC
 RCL_WARN_UNUSED
 rcl_ret_t

rcl/src/rcl/subscription.c

@@ -318,11 +318,7 @@ rcl_take_sequence(
     allocation);
   if (ret != RMW_RET_OK) {
     RCL_SET_ERROR_MSG(rmw_get_error_string().str);
-
-    if (RMW_RET_BAD_ALLOC == ret) {
-      return RCL_RET_BAD_ALLOC;
-    }
-    return RCL_RET_ERROR;
+    return rcl_convert_rmw_ret_to_rcl_ret(ret);
   }
   RCUTILS_LOG_DEBUG_NAMED(
     ROS_PACKAGE_NAME, "Subscription took %zu messages", taken);

rcl/test/rcl/test_subscription.cpp

@@ -293,7 +293,7 @@ TEST_F(
   {
     test_msgs__msg__Strings msg;
     test_msgs__msg__Strings__init(&msg);
-    ASSERT_TRUE(rosidl_generator_c__String__assign(&msg.string_value, test_string));
+    ASSERT_TRUE(rosidl_runtime_c__String__assign(&msg.string_value, test_string));
     ret = rcl_publish(&publisher, &msg, nullptr);
     ret = rcl_publish(&publisher, &msg, nullptr);
     ret = rcl_publish(&publisher, &msg, nullptr);
@@ -303,13 +303,14 @@ TEST_F(
   bool success;
   wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100, success);
   ASSERT_TRUE(success);
+  auto allocator = rcutils_get_default_allocator();
   {
     size_t size = 1;
     rmw_message_info_sequence_t message_infos;
-    rmw_message_info_sequence_init(&message_infos, size);
+    rmw_message_info_sequence_init(&message_infos, size, &allocator);
 
     rmw_message_sequence_t messages;
-    rmw_message_sequence_init(&messages, size);
+    rmw_message_sequence_init(&messages, size, &allocator);
 
     auto seq = test_msgs__msg__Strings__Sequence__create(size);
 
@@ -335,10 +336,10 @@ TEST_F(
   {
     size_t size = 5;
     rmw_message_info_sequence_t message_infos;
-    rmw_message_info_sequence_init(&message_infos, size);
+    rmw_message_info_sequence_init(&message_infos, size, &allocator);
 
     rmw_message_sequence_t messages;
-    rmw_message_sequence_init(&messages, size);
+    rmw_message_sequence_init(&messages, size, &allocator);
 
     auto seq = test_msgs__msg__Strings__Sequence__create(size);
 
@@ -362,7 +363,7 @@ TEST_F(
   {
     test_msgs__msg__Strings msg;
     test_msgs__msg__Strings__init(&msg);
-    ASSERT_TRUE(rosidl_generator_c__String__assign(&msg.string_value, test_string));
+    ASSERT_TRUE(rosidl_runtime_c__String__assign(&msg.string_value, test_string));
     ret = rcl_publish(&publisher, &msg, nullptr);
     ret = rcl_publish(&publisher, &msg, nullptr);
     ret = rcl_publish(&publisher, &msg, nullptr);
@@ -378,10 +379,10 @@ TEST_F(
   {
     size_t size = 3;
     rmw_message_info_sequence_t message_infos;
-    rmw_message_info_sequence_init(&message_infos, size);
+    rmw_message_info_sequence_init(&message_infos, size, &allocator);
 
     rmw_message_sequence_t messages;
-    rmw_message_sequence_init(&messages, size);
+    rmw_message_sequence_init(&messages, size, &allocator);
 
     auto seq = test_msgs__msg__Strings__Sequence__create(size);