Skip to content

Add tracepoint for publish/subscribe serialized message#485

Merged
mjcarroll merged 9 commits intoros2:rollingfrom
h-suzuki-isp:output_tracepoint_for_generic
Apr 5, 2024
Merged

Add tracepoint for publish/subscribe serialized message#485
mjcarroll merged 9 commits intoros2:rollingfrom
h-suzuki-isp:output_tracepoint_for_generic

Conversation

@h-suzuki-isp
Copy link
Copy Markdown
Contributor

Please, refer to ros2/rclcpp#2448 .
I have added the following trace points.

  • __rmw_publish_serialized_message()
    • tracepoint : rmw_publish
  • _take_serialized_message()
    • tracepoint : rmw_take

Signed-off-by: h-suzuki <h-suzuki@isp.co.jp>
Signed-off-by: h-suzuki <h-suzuki@isp.co.jp>
Signed-off-by: h-suzuki <h-suzuki@isp.co.jp>
Signed-off-by: h-suzuki <h-suzuki@isp.co.jp>
Signed-off-by: h-suzuki <h-suzuki@isp.co.jp>
Signed-off-by: h-suzuki <h-suzuki@isp.co.jp>
Signed-off-by: h-suzuki <h-suzuki@isp.co.jp>
Comment thread rmw_cyclonedds_cpp/src/rmw_node.cpp Outdated
Co-authored-by: eboasson <eb@ilities.com>
Signed-off-by: h-suzuki-isp <146712054+h-suzuki-isp@users.noreply.github.com>
@fujitatomoya
Copy link
Copy Markdown
Contributor

@mjcarroll /assign along with ros2/rmw_fastrtps#748

Copy link
Copy Markdown
Member

@christophebedard christophebedard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. @eboasson or @mjcarroll do you want to take a(nother) look?

@h-suzuki-isp
Copy link
Copy Markdown
Contributor Author

@eboasson @mjcarroll (Cc: @christophebedard )
I'd like to get this PR to Jazzy on time.
I would appreciate it if you could review it.

Copy link
Copy Markdown
Member

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly good, one nit and I would like clarification on the impact of the API change.

Comment thread rmw_cyclonedds_cpp/src/rmw_node.cpp
if (message_info) {
message_info_from_sample_info(info, message_info);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: unnecessary whitespace change

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 2b33c0b.

@mjcarroll
Copy link
Copy Markdown
Member

Note that CI was run here: ros2/rclcpp#2448 (comment)

Signed-off-by: h-suzuki-isp <h-suzuki@isp.co.jp>
@christophebedard
Copy link
Copy Markdown
Member

I think we can probably skip CI since the only new change is a new blank line.

@mjcarroll mjcarroll merged commit 2f0e1ef into ros2:rolling Apr 5, 2024
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.

5 participants