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

feat(autoware_internal_debug_msgs): add MultiArray msgs #33

Closed

Conversation

vish0012
Copy link

Description

Not applicable.

Effects on system behavior

Not applicable.

Interface changes

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

  • There are no open discussions or they are tracked via tickets.

After all checkboxes are checked, anyone who has write access can merge the PR.

Copy link
Member

@youtalk youtalk left a comment

Choose a reason for hiding this comment

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

@cyn-liu
Copy link
Contributor

cyn-liu commented Dec 23, 2024

I am working on task Autoware Universe port to Autoware Core, but this porting guideline requires that each package can only use messages defined under AutowareFoundation GitHub Org (e.g., autoware_msgs and autoware_internal_msgs), And I have noticed that someone has already moved tier4_xxx_msgs to autoware_internal_xxx_msgs, Will we replace the tier4_xxx_msgs in the universe with theautoware_internal_xxx_msgs next?

Why were two messages(ProcessingTimeNode.msg, ProcessingTimeTree.msg) missed when moving to autoware_internal_xxx_msgs, and these two messages are still being used in Autoware.Universe.

@youtalk
Copy link
Member

youtalk commented Jan 15, 2025

@cyn-liu If a part of autoware_universe_utils is to be ported to autoware.core, it will likely be necessary. Please create a migration PR.

Copy link

Thank you for contributing to the Autoware project!

🚧 If your pull request is in progress, switch it to draft mode.

Please ensure:

@youtalk
Copy link
Member

youtalk commented Jan 15, 2025

@cyn-liu If a part of autoware_universe_utils is to be ported to autoware.core, it will likely be necessary. Please create a migration PR.

Oops, they were already appended by yourself. Please ignore the comment.
#41

@mitsudome-r
Copy link
Member

mitsudome-r commented Jan 16, 2025

@youtalk

Why don't you use the original ones?

I think we added to tier4_autoware_msgs since the message seems to be deprecated as of Foxy, and we wanted to keep using it for Autoware: https://github.com/ros2/common_interfaces/blob/50cccb2dc164ae10a92c9360524eb169627bd36a/std_msgs/msg/MultiArrayLayout.msg#L2

@mitsudome-r
Copy link
Member

mitsudome-r commented Jan 16, 2025

If these messages are not necessary for Autoware Core, we might not need to port it to autoware_internal_msgs for now. I will check if we need this message.

@youtalk
Copy link
Member

youtalk commented Jan 16, 2025

It still seems possible to install via APT on both Humble and Jazzy. Wouldn’t it be fine to close it for now?

https://github.com/ros2/common_interfaces/blob/humble/std_msgs/CMakeLists.txt#L37-L38
https://github.com/ros2/common_interfaces/blob/jazzy/std_msgs/CMakeLists.txt#L38-L39

Comment on lines +1 to +7
# This is from ros2 common_interfaces.
# https://github.com/ros2/common_interfaces/blob/master/std_msgs/msg/MultiArrayDimension.msg

# This was originally provided as an example message.
# It is deprecated as of Foxy
# It is recommended to create your own semantically meaningful message.
# However if you would like to continue using this please use the equivalent in example_msgs.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe these comments can be updated for Autoware

Suggested change
# This is from ros2 common_interfaces.
# https://github.com/ros2/common_interfaces/blob/master/std_msgs/msg/MultiArrayDimension.msg
# This was originally provided as an example message.
# It is deprecated as of Foxy
# It is recommended to create your own semantically meaningful message.
# However if you would like to continue using this please use the equivalent in example_msgs.
# This is from ros2 common_interfaces.
# https://github.com/ros2/common_interfaces/blob/master/std_msgs/msg/MultiArrayDimension.msg
# The original message in ROS 2 was deprecated as of Foxy so we are defining it here for use in Autoware.

Comment on lines +1 to +7
# This is from ros2 common_interfaces.
# https://github.com/ros2/common_interfaces/blob/master/std_msgs/msg/MultiArrayLayout.msg

# This was originally provided as an example message.
# It is deprecated as of Foxy
# It is recommended to create your own semantically meaningful message.
# However if you would like to continue using this please use the equivalent in example_msgs.
Copy link
Member

Choose a reason for hiding this comment

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

Same here:

Suggested change
# This is from ros2 common_interfaces.
# https://github.com/ros2/common_interfaces/blob/master/std_msgs/msg/MultiArrayLayout.msg
# This was originally provided as an example message.
# It is deprecated as of Foxy
# It is recommended to create your own semantically meaningful message.
# However if you would like to continue using this please use the equivalent in example_msgs.
# This is from ros2 common_interfaces.
# https://github.com/ros2/common_interfaces/blob/master/std_msgs/msg/MultiArrayLayout.msg
# The original message in ROS 2 was deprecated as of Foxy so we are defining it here for use in Autoware.

@mitsudome-r
Copy link
Member

We can use the one from common_interfaces for Autoware Core until it is really removed so I think we can close this PR.

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.

4 participants