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

Switching to examples_interfaces #377

Open
wants to merge 4 commits into
base: rolling
Choose a base branch
from

Conversation

CursedRock17
Copy link

Meant to resolve #356, this pull request switches all the deprecated instances of std_msgs to example_interfaces after deprecation of the many elements occured. Though, any msg with Header is still a std_msgs since they were not part of that deprecation.

Signed-off-by: CursedRock17 <[email protected]>
Signed-off-by: CursedRock17 <[email protected]>
Signed-off-by: CursedRock17 <[email protected]>
Signed-off-by: CursedRock17 <[email protected]>
@ahcorde
Copy link
Contributor

ahcorde commented Apr 30, 2024

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

@CursedRock17
Copy link
Author

Hmm looks like a regression with one test, do you think it needed to be patched in #374? I can't imagine changing to example_interfaces would alter the time it takes to deliver one specific message, but I could be wrong.

@fujitatomoya
Copy link
Collaborator

@CursedRock17 #374 is already included in your repo https://github.com/CursedRock17/examples/tree/deprecated_msgs? what do you mean by #377 (comment)?

@CursedRock17
Copy link
Author

Yeah, I just didn't know if that specific test had passed all the way through, I'm trying to figure out why that specific test continues to be a regression. I guess it did pass in #374's CI so my changes must have broke it somehow.

@@ -14,14 +14,14 @@

import unittest

from example_interfaces.msg import String
Copy link
Collaborator

Choose a reason for hiding this comment

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

i believe this changes led to https://ci.ros2.org/job/ci_linux/20965/testReport/junit/launch_testing_examples.launch_testing_examples/check_msgs_launch_test/launch_testing_examples_check_msgs_launch_test/.

this is because message type does not match anymore,

demo_nodes_cpp uses std_msgs/String, so i think this change requires the same change to https://github.com/ros2/demos

@ahcorde
Copy link
Contributor

ahcorde commented May 8, 2024

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

@CursedRock17
Copy link
Author

Ok I understand it's the changes I've made that cause downstream CI failures, but I just don't see the correlation between this repo and something like rmw_implementation which is currently failing. Since neither repo includes each other I assumed it could've been test_msgs but again there's no generation of examples. Could somebody point out what I'm missing when it comes to these failures? Thanks.

@clalancette
Copy link
Contributor

To be perfectly honest, I'm not entirely sure we should do this at all. I know we technically marked those messages as deprecated, but they are very heavily used; looking at the packages released into rolling, I see hundreds of uses of those messages. It just seems that they are too useful to be able to remove them.

Personally, I would actually be for reverting ros2/common_interfaces#116 , and closing this PR and the associated issue. But we'll have to discuss that with the rest of the team.

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.

Official ros2 examples still use deprecated msg?
4 participants