Add rosidl_buffer for native Buffer type support by nvcyc · Pull Request #938 · ros2/rosidl

nvcyc · 2026-03-10T04:24:17Z

Description

This pull request adds the rosidl_buffer package - core C++ buffer types for the ROS 2 native buffer feature. This package introduces rosidl::Buffer<T>, a polymorphic container that's designed to replace std::vector<T> for uint8[] message fields (to be adopted in later pull requests; we focus on only the core buffer types in this pull request.) The native buffer type enables vendor-specific memory backends (CUDA, ROCm, etc.) while maintaining backward compatibility for existing CPU-based std::vector<T> user code.

This pull request consists of the following key components:

Buffer<T>: PIMPL-based container providing std::vector<T>-compatible API. All vector-compatible operations (element access, iterators, modifiers) are CPU-only and throw std::runtime_error for non-CPU backends. Backend management APIs (get_backend_type(), set_impl(), get_impl(), to_vector()) and copy/move semantics work for all backends.
BufferImplBase<T>: Minimal abstract base class of buffer implementation. All backend-specific APIs are to be provided by the vendor-specific backend implementations.
CpuBufferImpl<T>: CPU-based buffer implementation wrapping std::vector<T>.

Is this user-facing behavior change?

This pull request does not change existing behavior.
When adopted in later pull requests, message types with uint8[] fields (e.g., sensor_msgs/msg/Image.data) will use rosidl::Buffer<uint8_t> instead of std::vector<uint8_t>. For CPU backends (the default), Buffer<T> is a transparent drop-in replacement.

Did you use Generative AI?

Yes. Claude (claude-4.6-opus) via Cursor was used to assist with the std::vector<T> compatibility feature in the rosidl::Buffer<T> class and generate portions of the unit tests.

Additional Information

This package/PL is part of the broader ROS2 native buffer feature introduced in this post.

Signed-off-by: CY Chen <cyc@nvidia.com>

MiguelCompany · 2026-03-10T11:14:18Z

+
+// Test default construction
+TEST(TestBuffer, default_construction) {
+  Buffer<int> buffer;


Since the goal of the feature is to handle buffers of uint8_t, I think it makes sense to test with that type instead of int.

Thanks for the suggestions. Tests are updated to test uint8_t.

MiguelCompany · 2026-03-10T11:16:53Z

+  EXPECT_EQ(buffer1.size(), buffer2.size());
+  EXPECT_EQ(buffer1.get_backend_type(), buffer2.get_backend_type());
+  for (size_t i = 0; i < buffer1.size(); ++i) {
+    EXPECT_EQ(buffer1[i], buffer2[i]);


Shall we check that they are in different addresses?

Suggested change

EXPECT_EQ(buffer1[i], buffer2[i]);

EXPECT_EQ(buffer1[i], buffer2[i]);

EXPECT_NE(&buffer1[i], &buffer2[i]);

Added in new update.

MiguelCompany · 2026-03-10T11:17:36Z

+
+  EXPECT_EQ(buffer1.size(), buffer2.size());
+  for (size_t i = 0; i < buffer1.size(); ++i) {
+    EXPECT_EQ(buffer1[i], buffer2[i]);


Added in new update.

MiguelCompany · 2026-03-10T11:24:14Z

+  std::vector<Point> copied = buffer.to_vector();
+  EXPECT_DOUBLE_EQ(3.0, copied[0].z);
+}
+


Add some tests that check std::runtime_exception is thrown for certain operations when the implementation is not cpu

Added a test implementation class NonCpuBufferImpl and tests to check the throwing exception cases.

MiguelCompany · 2026-03-10T11:25:18Z

The functions in this file shall be checked in tests.

Added test_c_helpers.cpp for testing functions in c_helpers.cpp.

MiguelCompany · 2026-03-11T07:17:51Z

+  {
+    impl_ = std::move(impl);
+    backend_type_ = backend_type;
+  }


If this is called with "cpu" as the backend type, we should check that the implementation can be cast to CpuBufferImpl

Suggested change

}

void set_impl(

std::unique_ptr<BufferImplBase<T>> impl,

const std::string & backend_type)

{

if ((backend_type == "cpu") && (nullptr == dynamic_cast<CpuBufferImpl<T>*>(impl.get()))) {

throw std::runtime_error("CPU backend shall inherit from CpuBufferImpl.");

}

impl_ = std::move(impl);

backend_type_ = backend_type;

}

A follow up question would be, why store the backend identifier here instead of querying the underlying implementation for it? Why not let the underlying implementation decide what it does and doesn't support? You can follow that line of thought and say that rosidl::Buffer could simply forward calls to the implementation +/- CPU backend checks.

That is a valid suggestion. I'm updating the code so that the buffer implementation is the source of truth for the buffer backend type.

ahcorde · 2026-03-11T15:48:34Z

Pulls: #938
Gist: https://gist.githubusercontent.com/ahcorde/189205809e28fadd8f0c809e165b7f09/raw/f345f50215a60fba5be8164ecd41c2d1ff9df91b/ros2.repos
BUILD args: --packages-above-and-dependencies rosidl_buffer
TEST args: --packages-above rosidl_buffer
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/18438

Linux
Linux-aarch64
Linux-rhel
Windows

ahcorde · 2026-03-11T15:33:20Z

+  // Type aliases for std::vector compatibility
+  using value_type = T;
+  using allocator_type = Allocator;
+  using size_type = size_t;


include <cstddef>

Added in new update.

ahcorde · 2026-03-11T16:43:52Z

+<?xml-model href="http://download.ros.org/schema/package_format3.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
+<package format="3">
+  <name>rosidl_buffer</name>
+  <version>1.0.0</version>


version should match with the current packages 5.1.2

Fixed in new update.

ahcorde · 2026-03-11T16:45:50Z

+    Provides Buffer container type with support for multiple memory backends (CPU, GPU, custom).
+  </description>
+
+  <maintainer email="cyc@nvidia.com">CY Chen</maintainer>


missing <author> tag

Added in new update.

hidmic

First pass, meta comments.

hidmic · 2026-03-11T15:30:05Z

+  {
+    impl_ = std::move(impl);
+    backend_type_ = backend_type;
+  }


A follow up question would be, why store the backend identifier here instead of querying the underlying implementation for it? Why not let the underlying implementation decide what it does and doesn't support? You can follow that line of thought and say that rosidl::Buffer could simply forward calls to the implementation +/- CPU backend checks.

hidmic · 2026-03-11T16:46:38Z

+  /// implementations.
+  /// @param impl Backend implementation instance.
+  /// @param backend_type Backend identifier string.
+  void set_impl(


@nvcyc meta: hmm, have we considered passing implementation on buffer construction? Allowing swapping implementations on the fly creates race conditions in the other methods.

You are right and this is also not allowed by my initial design. Removing this function to keep it clean and aligned.

hidmic · 2026-03-11T16:48:05Z

+/// Resize a Buffer<uint8_t>. Throws for non-CPU backends.
+/// @param buffer_ptr Opaque pointer to an rosidl::Buffer<uint8_t>
+/// @param size New size
+void rosidl_buffer_uint8_resize(void * buffer_ptr, size_t size);


@nvcyc hmm, I would've expected a rosidl_uint8_buffer_t, even if it's just a type erased wrapper for rosidl::Buffer<uint8_t>. Mind to explain the use case?

Thanks for the comment.
These C helpers are mainly for the C introspection typesupport path (in later PLs) but also they are not intended to be used against non-CPU-based buffers (and hence we throw exception if accessed with these APIs). In C path, the current design is that CPU-based uint8[] will always be the data field in rosidl_runtime_c__uint8__Sequence as it originally does, so we won't have the case where CPU-based utin8[] is stored in rosidl::Buffer in the C path.

For this reason, I think we actually don't need these introspection C helper functions here as they are not supported by backend buffers. I'm optimizing from the C introspection typesupport side to simply call rosidl_buffer_uint8_throw_if_not_cpu() as guards there.

I'll make the changes to keep them simple in my new update.

nvcyc · 2026-03-14T20:24:39Z

Thanks for your review.

To facilitate the review process with cleaner future PR dependencies, I'm creating a new PR (#941) that's based on a branch in this repo (in contrast to a fork) to replace this one. All the changes for addressing the comments in this PR have been included in the new PR.

Please continue our review in #941 and I'll close this PR subsequently.

Add rosidl_buffer for Buffer type support

b23b82b

Signed-off-by: CY Chen <cyc@nvidia.com>

nvcyc requested review from MiguelCompany, ahcorde, hidmic, karsten-nvidia, mjcarroll, skyegalaxy and tfoote March 10, 2026 04:26

MiguelCompany suggested changes Mar 10, 2026

View reviewed changes

MiguelCompany suggested changes Mar 11, 2026

View reviewed changes

ahcorde requested changes Mar 11, 2026

View reviewed changes

hidmic reviewed Mar 11, 2026

View reviewed changes

nvcyc closed this Mar 14, 2026

	EXPECT_EQ(buffer1[i], buffer2[i]);
	EXPECT_EQ(buffer1[i], buffer2[i]);
	EXPECT_NE(&buffer1[i], &buffer2[i]);

-  }
+  void set_impl(
+    std::unique_ptr<BufferImplBase<T>> impl,
+    const std::string & backend_type)
+  {
+    if ((backend_type == "cpu") && (nullptr == dynamic_cast<CpuBufferImpl<T>*>(impl.get()))) {
+      throw std::runtime_error("CPU backend shall inherit from CpuBufferImpl.");
+    }
+    impl_ = std::move(impl);
+    backend_type_ = backend_type;
+  }

Conversation

nvcyc commented Mar 10, 2026

Description

Is this user-facing behavior change?

Did you use Generative AI?

Additional Information

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hidmic Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ahcorde commented Mar 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hidmic left a comment

Choose a reason for hiding this comment

Uh oh!

hidmic Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nvcyc commented Mar 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hidmic Mar 11, 2026 •

edited

Loading

hidmic Mar 11, 2026 •

edited

Loading