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 to rmw_topic_endpoint_info_t (rep2011) #348

Merged
merged 6 commits into from
Mar 15, 2023

Conversation

emersonknapp
Copy link
Contributor

@emersonknapp emersonknapp commented Feb 14, 2023

Part of ros2/ros2#1159
Depends on ros2/rosidl#722

Adds topic_type_hash field to rmw_topic_endpoint_info_t so that it can be filled by RMW implementations.

@emersonknapp emersonknapp changed the base branch from master to rolling February 14, 2023 23:01
@emersonknapp emersonknapp changed the title Add field topic_type_hash to rmw_topic_endpoint_info_t [WIP] Add field topic_type_hash to rmw_topic_endpoint_info_t Feb 14, 2023
@emersonknapp emersonknapp changed the title [WIP] Add field topic_type_hash to rmw_topic_endpoint_info_t [WIP] Add topic_type_hash to rmw_topic_endpoint_info_t Feb 15, 2023
@emersonknapp emersonknapp changed the title [WIP] Add topic_type_hash to rmw_topic_endpoint_info_t [WIP] Add field topic_type_hash to rmw_topic_endpoint_info_t Feb 16, 2023
@emersonknapp emersonknapp changed the title [WIP] Add field topic_type_hash to rmw_topic_endpoint_info_t [WIP] Add type hash to rmw_topic_endpoint_info_t (rep2011) Feb 21, 2023
@emersonknapp emersonknapp marked this pull request as ready for review February 23, 2023 21:50
@emersonknapp emersonknapp changed the title [WIP] Add type hash to rmw_topic_endpoint_info_t (rep2011) Add type hash to rmw_topic_endpoint_info_t (rep2011) Mar 2, 2023
@emersonknapp emersonknapp force-pushed the emersonknapp/type-version-hash branch from 5e82fe0 to 4795cca Compare March 12, 2023 02:31
@emersonknapp
Copy link
Contributor Author

@wjwwood @methylDragon @ivanpauno this is ready for review, the types are finalized in the dependency PR, we're only hashing out final details

Copy link
Contributor

@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.

One minor thing to fix here, then I think this is good to go.

rmw/include/rmw/topic_endpoint_info.h Outdated Show resolved Hide resolved
Copy link
Contributor

@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.

Looks good to me with green CI.

@clalancette
Copy link
Contributor

Now that ros2/rosidl#722 is merged, we can run CI on this one alone and merge it in:

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
@clalancette clalancette force-pushed the emersonknapp/type-version-hash branch from 049a83b to 04f9e43 Compare March 15, 2023 14:28
@clalancette
Copy link
Contributor

I had to rebase this to get the incompatible type stuff in. I've done that now, here is another CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@clalancette
Copy link
Contributor

The test failures on Windows are known flakes. So going ahead and merging this one in.

@clalancette clalancette merged commit 114e1e7 into ros2:rolling Mar 15, 2023
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.

None yet

2 participants