Add option to specify a message type#1153
Conversation
be85f63 to
adab4e6
Compare
Signed-off-by: Carlos <carlos.sanvicente@apex.ai> Signed-off-by: Carlos San Vicente <carlos.sanvicente@apex.ai>
Signed-off-by: Carlos San Vicente <carlos.sanvicente@apex.ai>
This reverts commit d07f4a8. Signed-off-by: Carlos San Vicente <carlos.sanvicente@apex.ai>
Signed-off-by: Carlos San Vicente <carlos.sanvicente@apex.ai>
Signed-off-by: Carlos San Vicente <carlos.sanvicente@apex.ai>
57fbe6f to
a806f63
Compare
|
@emersonknapp @adamdbrw Could you please review this PR? As explained in the description the motivation of this PR is to be able to run benchmarks using different message types instead of a generic type. The main reason would be to be able to reproduce as close as possible different scenarios. For this reason a new option, A logical question is what happens now when
Additional notes:
|
MichaelOrlov
left a comment
There was a problem hiding this comment.
@carlossvg Thanks for the PR.
In general it looks good to me among a few nitpicks.
I would really appreciate if you will address them.
..._performance/rosbag2_performance_benchmarking/include/msg_utils/message_producer_factory.hpp
Outdated
Show resolved
Hide resolved
rosbag2_performance/rosbag2_performance_benchmarking/src/benchmark_publishers.cpp
Outdated
Show resolved
Hide resolved
rosbag2_performance/rosbag2_performance_benchmarking/src/benchmark_publishers.cpp
Outdated
Show resolved
Hide resolved
rosbag2_performance/rosbag2_performance_benchmarking/src/benchmark_publishers.cpp
Outdated
Show resolved
Hide resolved
rosbag2_performance/rosbag2_performance_benchmarking/src/benchmark_publishers.cpp
Outdated
Show resolved
Hide resolved
|
|
||
| void generate_data(sensor_msgs::msg::Image & msg, size_t size) | ||
| { | ||
| // TODO(carlossvg): calculate message base size to set total size |
There was a problem hiding this comment.
@carlossvg Can you please clarify what this TODO about?
There was a problem hiding this comment.
@MichaelOrlov Currently the total message size will be all the message fixed size fields (https://github.com/ros2/common_interfaces/blob/rolling/sensor_msgs/msg/Image.msg#L4-L25) + the size of the data field (which is the size parameter passed as a function argument). Ideally the total message size would be equal to size. In order to do that we would have to calculate the size of all other fields except from data and then subtract this value to the size parameter to resize data the necessary amount.
There was a problem hiding this comment.
Could you please add meaningful comment to the todo with the same level of details?
rosbag2_performance/rosbag2_performance_benchmarking_msgs/CMakeLists.txt
Outdated
Show resolved
Hide resolved
rosbag2_performance/rosbag2_performance_benchmarking_msgs/msg/ByteArray.msg
Outdated
Show resolved
Hide resolved
rosbag2_performance/rosbag2_performance_benchmarking_msgs/package.xml
Outdated
Show resolved
Hide resolved
|
@carlossvg Kindly ping to reiterate on this PR. |
rosbag2_performance/rosbag2_performance_benchmarking_msgs/package.xml
Outdated
Show resolved
Hide resolved
Signed-off-by: Carlos San Vicente <carlos.sanvicente@apex.ai>
3fad493 to
78b2e8e
Compare
|
Gist: https://gist.githubusercontent.com/MichaelOrlov/7374b636991bac95242b44bc7924e462/raw/192fd2e9264b5bdc3ff901c799249cc8e5de6376/ros2.repos |
|
Failure in |
This PR adds an option to the Performance benchmarking package to use a predefined message type instead of generic type. The motivation for this option is to allow to replicate scenarios with exactly the same message types as the original case.