Skip to content

Fix rmw_take_serialized. (backport #881)#885

Merged
clalancette merged 1 commit into
mergify/bp/lyrical/pr-880from
mergify/bp/lyrical/pr-881
May 8, 2026
Merged

Fix rmw_take_serialized. (backport #881)#885
clalancette merged 1 commit into
mergify/bp/lyrical/pr-880from
mergify/bp/lyrical/pr-881

Conversation

@mergify
Copy link
Copy Markdown

@mergify mergify Bot commented May 5, 2026

Description

rmw_take and rmw_take_with_info already check info->is_buffer_aware_ to do special handling of buffer aware data. However, the serialized versions (rmw_take_serialized_message and
rmw_take_serialized_message_with_info) don't do that check. That means that there is no data delivered in the serialized path.

The fix is to add in a new helper for both functions which looks for is_buffer_aware_ first. If it is buffer aware, it copies the data out, and if not, it continues on to the normal __rmw_take_serialized_message path.

This should be merged at approximately the same time as ros2/system_tests#592 , #880 , and #879

Is this user-facing behavior change?

No.

Did you use Generative AI?

Yes, Claude Opus 4.7

Additional Information

This should be backported to Lyrical.


This is an automatic backport of pull request #881 done by Mergify.

* Fix rmw_take_serialized.

rmw_take and rmw_take_with_info already check info->is_buffer_aware_
to do special handling of buffer aware data.  However, the serialized
versions (rmw_take_serialized_message and
rmw_take_serialized_message_with_info) don't do that check.  That means
that there is no data delivered in the serialized path.

The fix is to add in a new helper for both functions which
looks for is_buffer_aware_ first.  If it is buffer aware, it
copies the data out, and if not, it continues on to the normal
__rmw_take_serialized_message path.

Signed-off-by: Chris Lalancette <clalancette@gmail.com>

* Feedback from review.

Signed-off-by: Chris Lalancette <clalancette@gmail.com>

---------

Signed-off-by: Chris Lalancette <clalancette@gmail.com>
(cherry picked from commit fa15754)
@mergify mergify Bot mentioned this pull request May 5, 2026
@clalancette clalancette changed the base branch from lyrical to mergify/bp/lyrical/pr-880 May 8, 2026 00:33
@clalancette
Copy link
Copy Markdown
Contributor

I changed the base and I'm going to merge this into #884 so I can test the lot together.

@clalancette clalancette merged commit d6dbb1f into mergify/bp/lyrical/pr-880 May 8, 2026
3 checks passed
@clalancette clalancette deleted the mergify/bp/lyrical/pr-881 branch May 8, 2026 00:34
clalancette added a commit that referenced this pull request May 9, 2026
* Change the buffer-aware BUFBE: -> bufbe. (#880)

The buffer-aware work appends a BUFBE: sentinel to the
publisher's user_data QoS field.  But parse_key_value
does not support colons in keys, so all type hashes
for buffers using this would be INVALID.

Fix this by dropping the colon from the sentinel, which
makes it pass parse_key_value and keep type hashes working.

Also add in a regression test here.

Signed-off-by: Chris Lalancette <clalancette@gmail.com>
(cherry picked from commit 35a62b8)

* Fix rmw_take_serialized. (#881) (#885)

* Fix rmw_take_serialized.

rmw_take and rmw_take_with_info already check info->is_buffer_aware_
to do special handling of buffer aware data.  However, the serialized
versions (rmw_take_serialized_message and
rmw_take_serialized_message_with_info) don't do that check.  That means
that there is no data delivered in the serialized path.

The fix is to add in a new helper for both functions which
looks for is_buffer_aware_ first.  If it is buffer aware, it
copies the data out, and if not, it continues on to the normal
__rmw_take_serialized_message path.



* Feedback from review.



---------


(cherry picked from commit fa15754)

Signed-off-by: Chris Lalancette <clalancette@gmail.com>
Co-authored-by: Chris Lalancette <clalancette@gmail.com>

---------

Signed-off-by: Chris Lalancette <clalancette@gmail.com>
Co-authored-by: Chris Lalancette <clalancette@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants