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

ros2: fix size of publisher/subscription GIDs #94

Conversation

christophebedard
Copy link
Member

@christophebedard christophebedard commented Oct 8, 2024

This PR is related to (and is needed after) ros2/ros2_tracing#138.

ROS 2 publishers and subscriptions have unique IDs called GIDs (global IDs), which is part of the ROS 2 middleware interface (rmw). Their values are based on the corresponding objects in the underlying middleware, usually the GUID (globally unique ID) of the DDS data writer and data reader.

Historically, the ROS 2 (rmw) GID was a 24-byte array, while the DDS GUIDs are usually 16-byte arrays, at least for the "main" DDS implementations (Fast DDS, Cyclone DDS, Connext DDS). The rmw GID was then just the DDS GUID plus some 8-byte (zero) padding.

Since ros2/rmw#345, the rmw GID size has been reduced to 16 bytes, therefore matching the usual size of DDS GUIDs. However, ROS 2 traces were still incorrectly collecting 24-byte rmw GIDs: 16 valid bytes + 8 garbage bytes. In practice, these 8 extra bytes didn't change much, since nothing is currently relying on the GID values, at least not relying on getting the same GID for the same object from two different systems, where the first 16 bytes would be the same, but the last 8 bytes would most likely be different.

Since ros2/ros2_tracing#138 fixed the ROS 2 tracing instrumentation to collect 16-byte rmw GIDs, the trace processing code here needs to be adapted. Instead of removing the last 8 bytes to go from a 24-byte rmw GID to a 16-byte DDS GUIDs, just keep the first 16 bytes from the rmw GID. This will work for traces that still contain 24-byte rmw GIDs (since the last 8 bytes are garbage anyway) and will work for traces that contain 16-byte rmw GIDs.

Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
Copy link
Contributor

@MatthewKhouzam MatthewKhouzam left a comment

Choose a reason for hiding this comment

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

Yeah, looks great, I like the combined combineFastDdsGid usage.

@MatthewKhouzam MatthewKhouzam merged commit eb238a0 into eclipse-tracecompass-incubator:master Oct 9, 2024
2 checks passed
@christophebedard christophebedard deleted the christophebedard/ros-2-update-rmw-gid-storage-size branch October 9, 2024 22:24
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.

2 participants