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

Ros2 port of map_msgs #4

Closed
wants to merge 3 commits into from
Closed

Ros2 port of map_msgs #4

wants to merge 3 commits into from

Conversation

wjwwood
Copy link
Contributor

@wjwwood wjwwood commented Jun 5, 2018

DO NOT MERGE, this is just for reference, the ros2 branch should remain separate for now.

Hi guys, I went ahead and ported map_msgs and put it on this ros2 branch because we need it in the map display for rviz in ROS 2. I hope you guys don't mind that I put it on a branch rather than a fork.

This does a few things:

  • map_msgs
    • adds myself as a maintainer, so that you guys don't have to support the ROS 2 branch alone
    • update the version to 2.0.0 (to avoid overlap with ROS 1 version for now)
    • update dependencies in package.xml to be ROS 2 style
    • update cmake code to use ament and do message generation in ROS 2
    • touchup the messages to remove trailing whitespace and use std_msgs/Header instead of Header (required in ROS 2)
  • move_base_msgs
    • disable with AMENT_IGNORE file (like CATKIN_IGNORE) for now since actions are still being ported

If you guys could close this pull request as an indication that this is all ok, that would be great. At that point I'd open a new issue to track that move_base_msgs needs porting when actions are done in ROS 2.

Also, this repository may be able to have one branch for both ROS 1 and ROS 2 in the future, but that depends on some on going work to smooth migration. So I'll have to come back and touch things again when that's possible.

I also built this on top of #3 in the hope that gets merged at some point.

Please let me know if you guys have any issues with this or questions for me.

Thanks!

One was three spaces, and the other was four spaces, so I switched both to fourspaces.
@mikeferguson
Copy link
Contributor

Looks good by me. I'm fine with with having ROS1/ROS2 in same repo, only comment would be that eventually this might want X-devel as branch name if we need different versions for different ROS releases (that is probably not likely for just messages).

Any timeline on actions being ported to ROS2?

@wjwwood
Copy link
Contributor Author

wjwwood commented Jun 6, 2018

only comment would be that eventually this might want X-devel as branch name if we need different versions for different ROS releases (that is probably not likely for just messages).

That might come up in the future, but for now, I think just ros2 is ok. We can change that at any point if necessary.

Any timeline on actions being ported to ROS2?

Doesn't look like it will get in this release, but some community people already started looking at it: ros2/design#143

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.

2 participants