Make loaned messages const on the subscription side #1971
Conversation
Signed-off-by: Nikolai Morin <[email protected]>
…sage Signed-off-by: Nikolai Morin <[email protected]>
| typename std::enable_if<std::is_same<ROSMessageType, | ||
| typename std::remove_const<MaybeConstROSMessageType>::type | ||
| >::value | ||
| >::type | ||
| dispatch( |
There was a problem hiding this comment.
could you add tests making sure that loaned messages work well with all types of callbacks?
There're a lot of codepaths here, and it's pretty hard to make sure you didn't break anything else.
So it would be nice to make sure all cases are covered in tests (some of the cases might already be covered).
There was a problem hiding this comment.
lgtm with test cases mentioned https://github.com/ros2/rclcpp/pull/1971/files#r919331175
|
I've picked this up again – sorry for the delay. Are you fine if I make it an error to dispatch a loaned message to a callback that would require making a copy of the loaned message? Because I think that making a copy would in most cases not be intended. But type adaptation should probably be allowed. |
I'm not sure. @wjwwood what do you think? |
Related to ros2/rcl#992.
To make callback dispatching work, I generalized
dispatch()to work with both const and non-const messages. Maybe there is a better way.