Skip to content

specification: add ros2idl to MCAP spec appendix#553

Merged
james-rms merged 2 commits intomainfrom
jrms/rosidl-spec
Sep 8, 2022
Merged

specification: add ros2idl to MCAP spec appendix#553
james-rms merged 2 commits intomainfrom
jrms/rosidl-spec

Conversation

@james-rms
Copy link
Copy Markdown
Collaborator

@james-rms james-rms commented Aug 24, 2022

Public-Facing Changes
Adds ros2idl as a well-known schema encoding, as well as a more complete description of the concatenations scheme for .msg definitions.

Description

@james-rms james-rms force-pushed the jrms/rosidl-spec branch 2 times, most recently from f3119e7 to 3058f6b Compare August 24, 2022 05:43
@james-rms james-rms requested a review from emersonknapp August 24, 2022 05:50
@james-rms james-rms force-pushed the jrms/rosidl-spec branch 2 times, most recently from f8934cd to d81afbe Compare August 24, 2022 06:18
@james-rms james-rms force-pushed the jrms/rosidl-spec branch 4 times, most recently from 528ac42 to 48b4149 Compare September 6, 2022 01:09
Copy link
Copy Markdown
Contributor

@defunctzombie defunctzombie left a comment

Choose a reason for hiding this comment

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

👍 from me. I'd get some more thumbs since this is a schema change and will be with us forever.

Copy link
Copy Markdown
Contributor

@amacneil amacneil left a comment

Choose a reason for hiding this comment

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

I was going to write a comment that I thought ros2idl diverging from ros2msg and ros1msg (gendeps style) was a mistake.

However, I read the arguments in this PR and thought more about it, and it seems like a good choice that will simplify parsing. And, you can think of it as still being gendeps-compatible, only the first "anonymous message definition" is blank.

In fact, we could change the spec to support making the first anonymous message definition optional (and recommended to be blank) for ros2msg as well as ros2idl, and change the behavior in rosbag2_storage_mcap to explicitly specify the ===\nMSG: header for all schemas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants