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

Include topic type and hash in key expression #171

Merged
merged 6 commits into from
Jun 24, 2024

Conversation

Yadunund
Copy link
Member

@Yadunund Yadunund commented May 10, 2024

Fix #117 while factoring in #201 (comment).

The keyexprs now adopt the format

<ros_domain_id>/<topic_name>/<topic_type>/<topic_hash>

Topic names are no longer mangled.

I will follow up with a separate PR to also add the topic_hash to the liveliness tokens so that we can include the hash in the endpoint info when querying topic info.

@Yadunund Yadunund force-pushed the yadu/fix_type_mismatch branch 2 times, most recently from ffedca3 to fdde316 Compare June 20, 2024 02:16
@Yadunund Yadunund marked this pull request as ready for review June 20, 2024 02:16
@Yadunund Yadunund force-pushed the yadu/fix_type_mismatch branch from fdde316 to 839b425 Compare June 20, 2024 02:21
@Yadunund Yadunund changed the title Include topic type in key expression Include topic type and hash in key expression Jun 20, 2024
@Yadunund Yadunund requested a review from clalancette June 20, 2024 02:21
rmw_zenoh_cpp/src/rmw_zenoh.cpp Outdated Show resolved Hide resolved
rmw_zenoh_cpp/src/rmw_zenoh.cpp Show resolved Hide resolved
rmw_zenoh_cpp/src/rmw_zenoh.cpp Outdated Show resolved Hide resolved
rmw_zenoh_cpp/src/rmw_zenoh.cpp Show resolved Hide resolved
rmw_zenoh_cpp/src/rmw_zenoh.cpp Show resolved Hide resolved
rmw_zenoh_cpp/src/rmw_zenoh.cpp Show resolved Hide resolved
Yadunund and others added 2 commits June 21, 2024 11:50
Co-authored-by: Chris Lalancette <[email protected]>
Signed-off-by: Yadu <[email protected]>
Signed-off-by: Yadunund <[email protected]>
Copy link
Collaborator

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

I've left two things inline to fix; once those are done, I think we are good to go here.

rmw_zenoh_cpp/src/rmw_zenoh.cpp Outdated Show resolved Hide resolved
rmw_zenoh_cpp/src/rmw_zenoh.cpp Outdated Show resolved Hide resolved
Yadunund and others added 2 commits June 24, 2024 09:27
Co-authored-by: Chris Lalancette <[email protected]>
Signed-off-by: Yadu <[email protected]>
Signed-off-by: Yadunund <[email protected]>
Copy link
Collaborator

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

This looks great to me, thanks for the fixups.

@Yadunund Yadunund merged commit 08704af into rolling Jun 24, 2024
7 checks passed
@Yadunund Yadunund deleted the yadu/fix_type_mismatch branch June 24, 2024 18:39
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.

Crash when deserializing message of matching topic but different type.
2 participants