Add support for rosidl::Buffer type serialization#144
Conversation
Signed-off-by: CY Chen <cyc@nvidia.com>
hidmic
left a comment
There was a problem hiding this comment.
First pass. This code needs a lot of eyes. EmPy templates are always incredibly tough to read.
| auto * non_const_impl = const_cast<rosidl::BufferImplBase<T> *>(impl); | ||
| std::shared_ptr<void> impl_shared(static_cast<void *>(non_const_impl), [](void *){}); |
There was a problem hiding this comment.
@nvcyc hmm, this is.. unusual. Should we be passing the impl by reference to create_descriptor_with_endpoint()? Why is the buffer passed by const ref if we need to mutate it?
...dl_typesupport_fastrtps_cpp/include/rosidl_typesupport_fastrtps_cpp/buffer_serialization.hpp
Outdated
Show resolved
Hide resolved
| // Vendor backends: account for descriptor payload | ||
| // Conservative estimate: buffer data size + overhead for metadata fields | ||
| size_t buffer_data_size = buffer.size() * sizeof(T); | ||
| size_t metadata_overhead = 256; |
There was a problem hiding this comment.
@nvcyc meta: hmm, I'm confused. If the descriptor is serialized, won't it usually be much smaller than the buffer? Isn't the descriptor the metadata itself?
Either way, I'm fine with a metadata overhead bound but I do think it should be a public constant that backends should use to enforce the bound on descriptor serialization.
| // Conservative estimate: buffer data size + overhead for metadata fields | ||
| size_t buffer_data_size = buffer.size() * sizeof(T); | ||
| size_t metadata_overhead = 256; | ||
| current_alignment += buffer_data_size + metadata_overhead; |
There was a problem hiding this comment.
The last serialized element was a string. If T is larger than 1 byte, the first element in the buffer must be properly aligned.
Signed-off-by: CY Chen <cyc@nvidia.com>
38ecb2e to
9f6bd55
Compare
Signed-off-by: CY Chen <cyc@nvidia.com>
Description
This pull request adds support for rosidl::Buffer-aware serializaion in
rosidl_typesupport_fastrtps_cpp.operator<</operator>>forrosidl::Bufferare provided to offer backward-compatibility in the plain operator path which ensures non-CPU buffers are converted tostd::vectorbefore serialization.Backend-based endpoint-aware serialization/deserialization functions (
serialize_buffer_with_endpoint()) are added to support backend-backed buffer serialization/deserialization path. To serialize a non-CPU buffer, a backend callback (create_descriptor_with_endpoint()) is invoked to export the buffer as a backend-defined buffer descriptor message before serialization.A CPU-based buffer (
std::vector) is serialized exactly the same as the originalstd::vector<uint8_t>bytes (uint32 length + raw data) to preserve compatibility with existing traffic and rosbag2.For the serialized bytes of a non-CPU buffer descriptor, it starts with
kBufferDescriptorMarker = 0xFFFFFFFFfollowed by thebackend_typestring, then the serialized descriptor. Deserialization peeks the firstuint32to check ifkBufferDescriptorMarkeris present for the descriptor path; any other value is treated as a CPU-based buffer (std::vector) sequence length.This pull request consists of the following key changes:
buffer_serialization.hpp: New header with template helpers forrosidl::Bufferserialization and global registries for backend descriptor operations (BufferDescriptorOps) and FastCDR serializers (DescriptorSerializers).has_buffer_fieldsflag and endpoint-awarecdr_serialize_with_endpoint/cdr_deserialize_with_endpointfunction pointers, enabling per-endpoint serialization decisions.register_buffer_descriptor.hpp:register_buffer_descriptor<DescriptorMsgT>(backend_name)lets backends register FastCDR serializers using existing rosidl-generated type support. Backends are expected to call this in their constructor.Is this user-facing behavior change?
This pull request does not change existing behavior. The serialization of
uint8[]fields is wire-compatible with the existingstd::vector<uint8_t>format. The new descriptor path is only activated when users publish messages with non-CPU buffers.Did you use Generative AI?
Yes. Claude (claude-4.6-opus) via Cursor was used to assist with creating an initial prototype version of the changes contained in this PR.
Additional Information
This PR is part of the broader ROS 2 native buffer feature introduced in this post. It depends on PR #491 (
rosidl_buffer+rosidl_buffer_backend) and can be reviewed in parallel with PR #492 (C++ rosidl changes).