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

How can I pull request ROS 2 packages #79

Open
hijimasa opened this issue May 4, 2024 · 10 comments
Open

How can I pull request ROS 2 packages #79

hijimasa opened this issue May 4, 2024 · 10 comments

Comments

@hijimasa
Copy link

hijimasa commented May 4, 2024

Thank you for creating this very useful repository.
I have already moved to ROS 2, but I really wanted to use this repository, so I created a ROS 2 package.
https://github.com/hijimasa/open3d_slam/tree/ros2

If you don't mind, I would like to make a pull request for the package I created, how do I send the request?
I would like to keep the master, but I cannot create new merge destination.

@nubertj
Copy link
Member

nubertj commented May 6, 2024

Dear @hijimasa,
Thanks a lot, that's great!
So, just to be sure, you can create a PR, but only against the master branch? I can check on that or create a new branch for you, but maybe having a PR against master is not a bad idea. What do you think? Instead of only having open3d_slam_ros, we could add another package, open3d_slam_ros2?

@hijimasa
Copy link
Author

hijimasa commented May 6, 2024

Dear @nubertj ,
Thanks to reply.
CMakeLists.txt for ROS 2 is different to ROS 1. And ROS 1 users still exist.
So I think the best way is to create new branch.
If you create new branch for ROS 2, I can create PR for the branch.

@nubertj
Copy link
Member

nubertj commented May 6, 2024

yes but we should be able to get all the packages independent of catkin except open3d_slam_ros, i.e. to allow the repository to be used in catkin through open3d_slam_ros, and in ament through open3d_slam_ros2?

I understand that this will require some additional refactoring, but only that way it will be truly useful to others without getting out of sync.

@hijimasa
Copy link
Author

hijimasa commented May 6, 2024

So you want to be able to build on ROS 1 and ROS 2 without splitting branches.

@nubertj
Copy link
Member

nubertj commented May 6, 2024

That would be great, and we will do this anyway in the coming months as we are soon also moving to ROS2 internally.
But if you don't want to be part of this, it is totally fine. Then I can prepare a branch for you, and we will check internally how we can do the further migration after that.

@hijimasa
Copy link
Author

hijimasa commented May 6, 2024

I would like to participate, but I don't know if we can make a common branch between ROS 1 and ROS 2. I have an idea for CMakeLists.txt, but I don't know what to do about package.xml. Do you have any ideas over there?

@nubertj
Copy link
Member

nubertj commented May 6, 2024

In the end everything builds with cmake, so the catkin packages do not care about any ament packages, and vice versa (as long as you do not try to build them).

Maybe let's try to create a separate branch for now, and I'll try to bring it all together in a couple of weeks.

@hijimasa
Copy link
Author

hijimasa commented May 6, 2024

I agree with creating a separated branch first.

@nubertj
Copy link
Member

nubertj commented May 6, 2024

can you try opening a PR against that branch: https://github.com/leggedrobotics/open3d_slam/tree/feature/ros2
?

@hijimasa
Copy link
Author

hijimasa commented May 6, 2024

Thank you to create the branch. I created a PR.

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

No branches or pull requests

2 participants