Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Need to pass introspection TS to converter plugin for it to be useful #896

Merged
merged 1 commit into from
Oct 28, 2021

Conversation

emersonknapp
Copy link
Collaborator

@emersonknapp emersonknapp commented Oct 27, 2021

Nobody (that I know of) actually uses this function signature argument

  • default conversion is through RMW implementation, which does just fine when passed the introspection version
  • rosbag2_bag_v2 implements a deserializer, which does not use the passed in typesupport object

rosbag2_converter_yaml revealed that a meaningful implementation of a Converter would need introspection information, so we might as well just start passing this in. It doesn't change the API/ABI, only the underlying data within the passed argument.

Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
@emersonknapp emersonknapp requested a review from a team as a code owner October 27, 2021 22:14
@emersonknapp emersonknapp requested review from mjeronimo and lihui815 and removed request for a team October 27, 2021 22:14
@emersonknapp
Copy link
Collaborator Author

Gist: https://gist.githubusercontent.com/emersonknapp/61fa119284ac020e3cc59b5cfc34893d/raw/1600028b1de6835d4cc4d589abcca6c1a3e42718/ros2.repos
BUILD args: --packages-up-to rosbag2_cpp rosbag2_tests rosbag2
TEST args: --packages-select rosbag2_cpp rosbag2_tests rosbag2
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/9253

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@emersonknapp emersonknapp merged commit 2e6ec0d into master Oct 28, 2021
@delete-merged-branch delete-merged-branch bot deleted the emersonknapp/converter-needs-introspection-ts branch October 28, 2021 16:10
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.

None yet

2 participants