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

Use ament to build #214

Open
wants to merge 9 commits into
base: ros2
Choose a base branch
from
Open

Conversation

zmichaels11
Copy link

@zmichaels11 zmichaels11 commented Nov 14, 2019

Changes

  • Use ament to build for ROS2 instead of catkin
  • Define tests in root CMakeLists.txt

Testing

Actions Status

unused dependency

Signed-off-by: Zachary Michaels <[email protected]>
package.xml Outdated Show resolved Hide resolved
@thomas-moulard
Copy link

  • The C++ improvements should be merged into master if possible.
  • Is the ROS 2 branch up-to-date? I think we should start by merging master into this branch.
  • It would be preferable to bump the package.xml to version 3 and use the new syntax to support ROS1/ROS2 compatibility syntax: https://www.ros.org/reps/rep-0149.html#name <depend condition="$ROS_VERSION == 1">roscpp</depend> - this way we won't have two package.xml
  • it'd be great to minimize the diff, I'm seeing some changes in the CMakeLists.txt which seems to be more like cleaning-up changes, than required changes. It's ok to cleanup in master and then backport here, but doing it here is going to make merging from master challenging.

Signed-off-by: Zachary Michaels <[email protected]>
@zmichaels11 zmichaels11 force-pushed the ros2-ament branch 2 times, most recently from 2b45b88 to 22a104a Compare November 21, 2019 17:12
@zmichaels11 zmichaels11 changed the title [WIP] Use ament to build Use ament to build Nov 21, 2019
@zmichaels11
Copy link
Author

zmichaels11 commented Nov 21, 2019

Build and test running in GH Actions: Actions Status

Signed-off-by: Zachary Michaels <[email protected]>
@zmichaels11 zmichaels11 marked this pull request as ready for review November 21, 2019 17:45
@piraka9011
Copy link

@wjwwood could we get a review on this please?

@klintan
Copy link

klintan commented May 2, 2020

Any news on this one? Would be awesome to get a working ROS2 version on the main ros2 branch.

@zmichaels11
Copy link
Author

Hi @klintan ,

There has not been any recent work on this issue.
The tracking issue has been moved back into our backlog since its going to require additional refactoring.

@thomas-moulard
Copy link

@zmichaels11 do you mind adding that back for grooming this week, if this is close to completion, we should try to finish this work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants