ros2 / rclcpp

rclcpp (ROS Client Library for C++)
Apache License 2.0
536 stars 417 forks source link

Fix TypeAdapted publishing with large messages. #2443

Closed clalancette closed 7 months ago

clalancette commented 7 months ago

Mostly by ensuring we aren't attempting to store large messages on the stack. Also add in tests. I verified that before these changes, the tests failed, while after them they succeed.

This should fix #2417

jmachowinski commented 7 months ago

lgtm

clalancette commented 7 months ago

CI:

clalancette commented 7 months ago

All right, all of the failing tests here are also failing on the nightlies. So I'm going to go ahead and merge this one in.

thomasmoore-torc commented 6 months ago

@clalancette - I noticed that the change associated with this PR uses std::make_unique directly and does not make use of the user-provided ros_message_type_allocator_ (e.g. create_ros_message_unique_ptr()). Was this intentional or an oversight?

clalancette commented 6 months ago

@clalancette - I noticed that the change associated with this PR uses std::make_unique directly and does not make use of the user-provided ros_message_type_allocator_ (e.g. create_ros_message_unique_ptr()). Was this intentional or an oversight?

I had no idea that was a thing.

thomasmoore-torc commented 6 months ago

I had no idea that was a thing.

That's fair. I've been digging into how the custom allocator functionality works, so I suppose it's fresh on my mind. I'm not in a position to be able to create a PR for this, so would it be best for me to create an issue?

clalancette commented 6 months ago

That's fair. I've been digging into how the custom allocator functionality works, so I suppose it's fresh on my mind. I'm not in a position to be able to create a PR for this, so would it be best for me to create an issue?

Yep, that would be great, thanks.