search

ros2 / examples

Example packages for ROS 2
Apache License 2.0
681 stars 308 forks source link

Switching to examples_interfaces #377

Open CursedRock17 opened 2 months ago

CursedRock17 commented 2 months ago

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.

ahcorde commented 2 months ago
CursedRock17 commented 2 months ago

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 commented 2 months ago

@CursedRock17 https://github.com/ros2/examples/pull/374 is already included in your repo https://github.com/CursedRock17/examples/tree/deprecated_msgs? what do you mean by https://github.com/ros2/examples/pull/377#issuecomment-2086093785?

CursedRock17 commented 2 months ago

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.

ahcorde commented 2 months ago
CursedRock17 commented 2 months ago

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 commented 2 months ago

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 https://github.com/ros2/common_interfaces/pull/116 , and closing this PR and the associated issue. But we'll have to discuss that with the rest of the team.