Skip to content

Migration to ROS 2 - #59

Closed
youtalk wants to merge 27 commits into
AutonomyLab:indigo-develfrom
youtalk:crystal-devel
Closed

youtalk wants to merge 27 commits into
AutonomyLab:indigo-develfrom
youtalk:crystal-devel

Conversation

@youtalk

@youtalk youtalk commented Mar 2, 2019 •

Copy link
Copy Markdown

This PR is my first trial to migrate the create_autonomy package to ROS 2 Crystal environment.

I created a new ca_driver2 package instead of the ca_driver package migration, because the difference is huge.
This branch can be built with AutonomyLab/libcreate#42 branch.
To build these packages, please read the updated README.md or follow the process:

$ mkdir -p ~/create_ws/src
$ cd ~/create_ws/src
$ git clone https://github.com/youtalk/create_autonomy.git -b crystal-devel
$ git clone https://github.com/AutonomyLab/libcreate.git -b package_format_3
$ cd ~/create_ws
$ rosdep update
$ rosdep install --from-paths src -i
$ colcon build

After merge this PR, I will continue to update:

  • reformat codes to pass ament_lint,
  • the launch system,
  • the lifecycle implementation,
  • and using dinagnostic_updater (there is no ROS 2 migration yet).

Thank you in advance. Your create_autonomy and libcreate packages are very helpful for my daily life with robot.

@jacobperron
jacobperron self-requested a review March 4, 2019 17:39
@OYTIS

OYTIS commented Mar 8, 2019 •

Copy link
Copy Markdown

Funnily enough I've just finished nearly the same work: https://github.com/OYTIS/create_autonomy
(should have subscribed to this repo before doing that of course). The differences I've noticed so far:

  • I didn't remove the dependency to diagnostic_updater relying on slightly modified this fork: https://github.com/vaibhavbhadade/diagnostics

  • I've added python launch files that are almost working (somehow I have a problem with leading / in my node name resulting in topics not being exposed - running the node manually doesn't show this problem). You might have done that as well, just forgotten to commit them - at least they are mentioned in your docs.

  • You've also done linting and test part - I didn't touch that.

Feel free to pull whatever you find useful in my implementation, and I'll try to test yours with my Roomba soon.

@jacobperron

jacobperron commented Mar 11, 2019 •

Copy link
Copy Markdown
Member

Since the change is significant, I think it would be better to replace the existing ca_driver package with the new code, rather than adding a new package alongside it.
Then we could merge it to a new ros2 branch.

@youtalk

youtalk commented Mar 12, 2019

Copy link
Copy Markdown
Author

@jacobperron Thank you for your advice. I've changed to replace ca_driver instead of add ca_driver2.

@OYTIS

OYTIS commented Aug 19, 2019

Copy link
Copy Markdown

@jacobperron Any update on this? Looks like the CI failure is not related to the PR.

@jacobperron

Copy link
Copy Markdown
Member

While looking into releasing for Melodic and ROS 2, I thought it would be better if the create_autonomy packages were refactored so that they follow ROS naming conventions and rules (REP 144). I'd like to rename the packages as such:

  • create_autonomy -> create_robot
  • ca_description -> create_description
  • ca_driver -> create_driver
  • ca_msgs -> create_msgs
  • ca_tools -> create_bringup

Therefore, I propose deprecating this repository for Melodic onward in favor of this fork: https://github.com/jacobperron/create_robot

If community members agree with my refactoring proposal, I would direct PRs to the new fork.

Apologies for the slow progress, but this is not my main focus atm. I'll try to get to it as soon as I can.

@youtalk

youtalk commented Aug 19, 2019

Copy link
Copy Markdown
Author

@jacobperron Thank you for your effort to follow the naming conventions.

If community members agree with my refactoring proposal, I would direct PRs to the new fork.
Apologies for the slow progress, but this is not my main focus atm. I'll try to get to it as soon as I can.

If you don't have enough time to maintain the repository, I can continue to maintain and release instead of you. I have already more progress. For example:

It's just my suggestion. I can also continue to contribute to your new repository.

@OYTIS

OYTIS commented Feb 18, 2020

Copy link
Copy Markdown

Hi @jacobperron ,

sorry for bothering you. Was there any progress with gathering the feedback from the community? create_robot looks great in my opinion, would be nice to have the ros2 port applied on top of it.

@jacobperron

Copy link
Copy Markdown
Member

@OYTIS Not a bother at all!

I did get some feedback, but I can't remember if there is a public thread I can point to. In any case, we should be able to move forward with the renaming of the create_autonomy packages as I proposed above. Unfortunately, I haven't had time to work towards a Melodic release or a ROS 2 port. Pull requests are welcome at https://github.com/jacobperron/create_robot.

Once things are settled on my fork, I'd like to merge everything back to this repository (and rename the repo on GitHub). This way any existing links to the repository should be properly redirected by GitHub.

@jacobperron

Copy link
Copy Markdown
Member

I've finally done the renaming I wanted and merged everything back upstream. In addition, a ROS 2 port was completed: https://github.com/AutonomyLab/create_robot/tree/foxy

Therefore, I'll close out this PR. Feel free to open up further PRs for any improvements to the ROS 2 implementation targeting the foxy branch.

@youtalk
youtalk deleted the crystal-devel branch October 14, 2020 04:57
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.

3 participants