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

Add type hash support (pub/sub only) #349

Merged
merged 8 commits into from
Dec 3, 2024
Merged

Add type hash support (pub/sub only) #349

merged 8 commits into from
Dec 3, 2024

Conversation

JEnoch
Copy link
Member

@JEnoch JEnoch commented Dec 2, 2024

Add support of ROS 2 type hash set by rmw_cyclone_dds in USER_DATA QoS.

What this PR does if $ROS-DISTRO >= iron:

  • Declares the Reader/Writer on ros_discovevry_info topic with the expected type hash (hard coded) in USER_DATA QoS
  • For Topics Publishers/Subscribers: adds the discovered USER_DATA QoS to the encoded QoS already present in the Liveliness Tokens.
  • For Actions Services/Clients:
    • declare the Readers/Writers on status topic with the expected type hash (hard coded) in USER_DATA QoS
    • declare the Readers/Writers on feedback topic with an pseudo-type hash (hard coded) in USER_DATA QoS
      (currently rmw_cyclonedds_cpp only checks the presence of a type hash, not the compatibility...)

The propagation of type hashes for Services and Actions is not implemented in this PR.
A type hash encoded in USER_DATA QoS is 81 characters/bytes ("typehash=RIHS01_<64bytes>;"). For a Service on DDS, we need to propagate the type hashes for both the Request and Response topics (162 bytes). For an Action it would be 8*162=1296 bytes. Even if the Zenoh Liveliness Tokens have no size limit, this would add a burden on the discovery phase.

Moreover, at this stage rmw_cyclonedds_cpp set the type hash in USER_DATA for each DDS Reader/Writer, but on discovery it check if it's present only for Topics Pub/Sub, not for Services.

fix #290

@JEnoch JEnoch added the enhancement New feature or request label Dec 2, 2024
@JEnoch JEnoch self-assigned this Dec 2, 2024
@JEnoch JEnoch requested a review from gmartin82 December 2, 2024 16:12
Copy link
Contributor

@gmartin82 gmartin82 left a comment

Choose a reason for hiding this comment

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

Looks good apart from the compiler reporting unused code.

Tested with ROS2 Jazzy and warnings no longer show.

zenoh-plugin-ros2dds/src/ros2_utils.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@gmartin82 gmartin82 left a comment

Choose a reason for hiding this comment

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

Looks good.

@JEnoch JEnoch enabled auto-merge (squash) December 3, 2024 13:08
@JEnoch JEnoch merged commit ef5b953 into main Dec 3, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] [ROS2 Jazy] Failed to parse type hash for topic
2 participants