Skip to content

Include type hash in topic endpoint info (rep2011)#1104

Merged
clalancette merged 1 commit intoros2:rollingfrom
achim-k:achim-k/rep2011_type_hash
Jun 13, 2023
Merged

Include type hash in topic endpoint info (rep2011)#1104
clalancette merged 1 commit intoros2:rollingfrom
achim-k:achim-k/rep2011_type_hash

Conversation

@achim-k
Copy link
Copy Markdown
Contributor

@achim-k achim-k commented Apr 3, 2023 •
edited
Loading

Part of ros2/ros2#1159

Features

  • Adds the TypeHash class
  • Adds the type hash to the TopicEndpointInfo class

This will print the type hash when printing out verbose topic information with rostopic info --verbose. Note that without the --no-daemon flag, ros2/ros2cli#816 is required which adds xmlrpc marshalling support for the new rclpy.type_hash.TypeHash class

@achim-k achim-k mentioned this pull request Apr 3, 2023
Merged
@achim-k achim-k changed the title Include type hash in topic endpoint info Include type hash in topic endpoint info (rep2011) Apr 3, 2023
achim-k
achim-k commented Apr 3, 2023
View reviewed changes
emersonknapp
emersonknapp approved these changes Apr 4, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@emersonknapp emersonknapp left a comment

Choose a reason for hiding this comment

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

LGTM - some small nitpicks you could address

@achim-k achim-k force-pushed the achim-k/rep2011_type_hash branch from 3b439dc to 6ba0ec1 Compare April 5, 2023 20:13
This was referenced Apr 11, 2023
Closed
Merged
@achim-k achim-k force-pushed the achim-k/rep2011_type_hash branch 2 times, most recently from 4200a55 to df8837a Compare April 13, 2023 20:02
clalancette
clalancette approved these changes Jun 8, 2023
View reviewed changes
Copy link
Copy Markdown
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.

This one looks good to me.

@achim-k Am I correct in thinking that this one has no other dependencies? If that's the case, then I'll go ahead and run CI on it.

@achim-k
Copy link
Copy Markdown
Contributor Author

achim-k commented Jun 8, 2023

@achim-k Am I correct in thinking that this one has no other dependencies?

Yes, that's correct.

@clalancette
Copy link
Copy Markdown
Contributor

clalancette commented Jun 8, 2023

CI:

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

@clalancette
Copy link
Copy Markdown
Contributor

clalancette commented Jun 8, 2023

@achim-k There were a bunch of test failures in here; can you take a look?

@achim-k
Copy link
Copy Markdown
Contributor Author

achim-k commented Jun 9, 2023

@achim-k Am I correct in thinking that this one has no other dependencies?

Yes, that's correct.

I was wrong. It depends on ros2/ros2cli#816 which adds marshalling support for the added TypeHash class. This is only required for runtime, but tests will fail without it.

@clalancette
Copy link
Copy Markdown
Contributor

clalancette commented Jun 9, 2023

I was wrong. It depends on ros2/ros2cli#816 which adds marshalling support for the added TypeHash class. This is only required for runtime, but tests will fail without it.

There also seems to be a flake8 issue in the tests, though it is in code that this doesn't touch. I'm going to rebase this to see if that improves the situation.

@achim-k @clalancette
Include type hash in topic endpoint info
0c94401 
Signed-off-by: Hans-Joachim Krauch <achim.krauch@gmail.com>
@clalancette clalancette force-pushed the achim-k/rep2011_type_hash branch from df8837a to 0c94401 Compare June 9, 2023 12:41
@clalancette
Copy link
Copy Markdown
Contributor

clalancette commented Jun 9, 2023

New CI:

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

@clalancette
Copy link
Copy Markdown
Contributor

clalancette commented Jun 9, 2023

It looks like there are still test failures.

@achim-k
Copy link
Copy Markdown
Contributor Author

achim-k commented Jun 9, 2023

It looks like there are still test failures.

Pushed a fix to ros2/ros2cli#816

@clalancette
Copy link
Copy Markdown
Contributor

clalancette commented Jun 12, 2023

All right, here is another CI run:

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

@clalancette clalancette merged commit ecf8958 into ros2:rolling Jun 13, 2023
@clalancette
Copy link
Copy Markdown
Contributor

clalancette commented Jun 13, 2023

@Mergifyio backport iron

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jun 13, 2023 •
edited
Loading

backport iron

✅ Backports have been created

Details

mergify bot pushed a commit that referenced this pull request Jun 13, 2023
@achim-k @mergify
Include type hash in topic endpoint info (#1104)
626a20f 
Signed-off-by: Hans-Joachim Krauch <achim.krauch@gmail.com>
(cherry picked from commit ecf8958)
@mergify mergify bot mentioned this pull request Jun 13, 2023
Merged
clalancette pushed a commit that referenced this pull request Jun 14, 2023
@mergify @achim-k
Include type hash in topic endpoint info (#1104) (#1134)
b53e413 
Signed-off-by: Hans-Joachim Krauch <achim.krauch@gmail.com>
(cherry picked from commit ecf8958)

Co-authored-by: Hans-Joachim Krauch <achim-k@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@clalancette clalancette clalancette approved these changes

@emersonknapp emersonknapp emersonknapp approved these changes

@sloretz sloretz Awaiting requested review from sloretz

@adityapande-1995 adityapande-1995 Awaiting requested review from adityapande-1995

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@achim-k @clalancette @emersonknapp