-
Notifications
You must be signed in to change notification settings - Fork 96
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
specification: add ros2idl to MCAP spec appendix #553
Conversation
f3119e7
to
3058f6b
Compare
f8934cd
to
d81afbe
Compare
528ac42
to
48b4149
Compare
There was a problem hiding this 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.
48b4149
to
d8fca75
Compare
There was a problem hiding this 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.
8937efb
to
5c798e5
Compare
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