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

Move gazebo_ros_control into separate git repository? #179

Closed
scpeters opened this issue Apr 8, 2014 · 34 comments
Closed

Move gazebo_ros_control into separate git repository? #179

scpeters opened this issue Apr 8, 2014 · 34 comments

Comments

@scpeters
Copy link
Member

scpeters commented Apr 8, 2014

The gazebo_ros_control package depends on several ros_controls packages. We could make it easier to build the rest of gazebo_ros_pkgs if we move gazebo_ros_control to its own repository.

Thoughts?

@scpeters
Copy link
Member Author

scpeters commented Apr 8, 2014

@davetcoleman Your thoughts?

@davetcoleman
Copy link
Collaborator

I understand what you mean about it being more difficult having those dependencies, but I'm not sure if that's such a good idea. Originally I had wanted gazebo_ros_control_plugin.cpp to be in gazebo_plugins package with everything else, but it was decided it would be in its own separate package until ros_control was stable. ROS Control is much more stable now and, while I'm not going to say we should merge the packages, I think removing gazebo_ros_control from the metapackage would allow it to be forgotten about and suffer from bit rot.

If no one is using this package then yea, its fine to remove it, but it seems to me having simulated controllers is an important component of simulating a robot.

Also, concerning the dependencies, we could make the same argument about the plugins in gazebo_plugins that require OpenCV and PCL components. Most of the dependencies in ros_control are from the PR2, anyway, so should be well supported.

Overall though, I'm not very motivated to work on those packages myself right now, I am focused mostly on MoveIt these days.

@scpeters
Copy link
Member Author

I think I was imprecise in my proposal. I would not propose changing the package.xml dependency of the meta-package, but I would propose moving the gazebo_ros_control package to a separate repository, so that the other gazebo packages can be released independently. Does that make sense?

@davetcoleman
Copy link
Collaborator

I understood what you are saying, I was just pointing out that gazebo_ros_control package is actually just another Gazebo plugin that belongs in gazebo_plugins, but I explained why it wasn't. So I don't think it should be removed since its part of the Gazebo interface to ROS... control is just as important as perception.

@scpeters
Copy link
Member Author

I was mistaken actually; gazebo_ros_control is not listed as a dependency of gazebo_ros_pkgs.

I agree that control is important, but it's a matter of separating dependencies. If we're concerned about bit-rot, we should set up continuous integration.

@adolfo-rt
Copy link

I was mistaken actually; gazebo_ros_control is not listed as a dependency of gazebo_ros_pkgs.

Is gazebo_ros_control not being released into Indigo yet causing issues then?.

The alternative of the current layout would be to have bindings between ros_control and 3rd party projects like Gazebo and MoveIt! live in independent repos of the ros-controls organization. I'd consider the rearrangement, but only if the dependencies and release timing are causing issues in those other projects.

@scpeters
Copy link
Member Author

gazebo_ros_pkgs is currently blocked by driver_base and ros_control. So ros_control is not the only bottleneck, but I expect driver_base to be released soon and am trying the plan for how to release the gazebo packages accordingly.

@scpeters scpeters changed the title Move gazebo_ros_control out of meta-package? Move gazebo_ros_control into separate git repository? Apr 22, 2015
@hsu
Copy link
Collaborator

hsu commented Apr 23, 2015

Looking at the history, @adolfo-rt and @davetcoleman did most of the recent developments, +1 for moving it into ros_control.

@wjwwood
Copy link
Member

wjwwood commented Apr 23, 2015

I'd also like to +1 moving it out of this repository. It's held up releasing Gazebo early in two ROS distro's so far, which is not ros_control's fault, but gazebo integration is part of desktop-full so I believe that minimizing its required dependencies is important.

If you guys decide which organization you'd like gazebo_ros_control to live in, then I'm willing to help with moving the gazebo_ros_control properly so no history is lost. I'm also willing to help with moving issues and pull requests on this repository which are related to gazebo_ros_control.

@davetcoleman
Copy link
Collaborator

If not inside ros-simulation it should go to https://github.com/ros-controls/

@wjwwood
Copy link
Member

wjwwood commented Apr 27, 2015

@adolfo-rt do you have any update on ros_control in Jade or any input on the final location of gazebo_ros_control?

@fmessmer
Copy link
Contributor

While not required immediately, it might be worth the consideration:
When moving gazebo_ros_control to a new repo, it might be a good thing to also bring cob_gazebo_ros_control (find it here) in there, as it only was out-sourced because it depended on features that were not merged at that point of time - but are now (see ros-controls/ros_control#200).

We'd be happy to share that plugin with the community as it offers the new HWI switching. Furthermore it would help to keep similar things sync'ed.

@adolfo-rt
Copy link

I'm fine with moving gazebo_ros_control to the ros-controls organization. I agree that gazebo_ros_control should not hold back desktop-full. @wjwwood, your help would be appreciated to speed up the migration.

On the other pending releases, I'll reply directly on ros-controls/control_toolbox#36 and ros-controls/ros_control#201

@adolfo-rt
Copy link

When moving gazebo_ros_control to a new repo, it might be a good thing to also bring cob_gazebo_ros_control (find it here) in there

My opinion is to keep robot-agnostic repos hosted in separate organizations from robot-specific ones. If the contents of cob_gazebo_ros_control can be presented (and named) in a robot-agnostic way, and there are non-cob users interested in it, we can surely find a good way to incorporate these contributions.

@fmessmer
Copy link
Contributor

cob_gazebo_ros_control is not cob-specific, except for the current package name.
I even had a predecessor of it targeted to gazebo_ros_control once (see here)
...but we were ahead of time 😉

@carlosjoserg
Copy link

Sorry to disturb here, I'd just like to give my opinion as an user.

I really agree with this comment above of gazebo_ros_control being within gazebo_plugins, as long as dependencies are not a trouble.

Besides, as there are plugins for different camera types, I think it is useful for the community if different robots start to appear in the plugin folder when they have differences w.r.t. to the default gazebo_ros_control plugin behavior (like the analogous case of the GazeboRosCamera and the GazeboRosProsilica, both inheriting from GazeboRosCameraUtils).

@wjwwood
Copy link
Member

wjwwood commented Apr 28, 2015

Sorry to disturb here, I'd just like to give my opinion as an user.

Thanks for chiming in, it's good to hear what people think about it.

I really agree with this comment above of gazebo_ros_control being within gazebo_plugins, as long as dependencies are not a trouble.

Well this is really coming up because the dependency is an issue. One of the reasons we work so hard to lower the barrier to making a package is that packages allow our code base to stay modular and decoupled. Currently releasing Gazebo-ROS stuff is coupled to ros_control stuff, which why it has been suggested.

Besides, as there are plugins for different camera types, I think it is useful for the community if different robots start to appear in the plugin folder when they have differences w.r.t. to the default gazebo_ros_control plugin behavior (like the analogous case of the GazeboRosCamera and the GazeboRosProsilica, both inheriting from GazeboRosCameraUtils).

I don't entire follow this, are you suggesting that by not moving ros_control into gazebo_plugins that people will not build off of it? That's a legitimate concern, but I don't see how that would be encouraged by having it in its own repository. Maybe this is an argument that the gazebo_plugins package should be more modular and less monolithic? (so that plugin layout is more consistent and doesn't accidentally appear to be promoting one set of plugins over another).

@carlosjoserg
Copy link

Well this is really coming up because the dependency is an issue...

Just recalling again the argument in the comment above. The gazebo-ros-pkgs is also coupled to PCL, which is external. The ros-controls is within ROS.org and provide catkin packages.

So, if gazebo integration is part of desktop-full, I wouldn't consider ros_control as a dependency to minimize, but to include due to its importance (don't know the implications of this, of course).

...are you suggesting that by not moving ros_control into gazebo_plugins that people will not build off of it?...

I think there is a typo, it is not ros_control, but the gazebo_ros_control plugin what's said. Well, I suppose if someone is interested will find the way to contribute no matter where the repo actually is. But at least for me, the first place I would look for the gazebo_ros_control plugin is within the gazebo_plugin folder.

Maybe this is an argument that the gazebo_plugins package should be more modular and less monolithic?...

This is actually another topic, I think.
Just mentioned the camera examples to give an analogy, I guess there are no 2 plugins providing the same functionality, so it would be a matter of choosing the right for the need instead of promoting.

I found the monolithic feature annoying when building all plugins from source, but that's how it is. When installing with apt-get, I would say it is rather appreciated to install only one package to have'em all.

@wjwwood
Copy link
Member

wjwwood commented Apr 29, 2015

Ok, well since there doesn't seem to be an agreement about what to do or where to put it, I am releasing the current version of the packages in this repository plus changes to the rosdep keys so that they use Gazebo5 (from the jade-devel branch) into jade with an ignore file for gazebo_ros_pkgs:

https://github.com/ros-gbp/gazebo_ros_pkgs-release/blob/master/jade.ignored

That file will cause bloom to ignore the gazebo_ros_control package. Later we can just remove that file and release again with bloom. It is important that we don't forget about that though.

I did create a split off repository for gazebo_ros_control in the ros-controls organization which preserves the history and tags, but only so that if we decide to migrate in the future we can do that:

https://github.com/ros-controls/gazebo_ros_control

Basically, I've deferred the issue until some consensus can be reached. But I don't want to wait until later to get the basic stuff for Jade.

@wjwwood wjwwood mentioned this issue Apr 29, 2015
@j-rivero
Copy link
Contributor

Gazebo users are starting to ask about the gazebo-ros-control package.

@adolfo-rt @davetcoleman: any news on getting a plan for migration, or fixing the current lack of the package? Thanks.

@scpeters
Copy link
Member Author

See adolfo's comment in the ros_control issue tracker.

@adolfo-rt
Copy link

I confirm that my comment on the ros_control issue stand. I'm running on prioritized best-effort mode, and the Jade release of ros_control (and gazebo_ros_control) has been preempted. It'll be a couple of weeks until the scheduler gives it some cycles again.

@j-rivero
Copy link
Contributor

No problem, thanks for the update.

@v4hn
Copy link
Contributor

v4hn commented Feb 23, 2016

It's been about a year and there's still no gazebo_ros_control in jade.
This is the last module missing for us to migrate to jade.
ros_control has been released in the meanwhile and as @wjwwood pointed out the possibility to ignore the folder with bloom, it looks like it does not matter at all whether or not the module is part of this repo or not.
Pretty Please release gazebo_ros_control into jade!

@scpeters
Copy link
Member Author

@j-rivero is planning to make a release of gazebo_ros_pkgs soon. Can you make sure to release gazebo_ros_control?

@j-rivero
Copy link
Contributor

@j-rivero is planning to make a release of gazebo_ros_pkgs soon. Can you make sure to release gazebo_ros_control?

Is the gazebo_ros_control code in a good shape to be released into jade? I can run the release process on top of it but I don't have any opinion or knowledge about it so I can not evaluate if it is correct, useful and valid to go into jade. @adolfo-rt any insight about the situation? What needs to be done or what should be done?

@Karsten1987
Copy link

Karsten1987 commented Apr 8, 2020

Happy 8 6th birthday, ticket :)

This question of where to host the ros control gazebo plugin just came up again today in the ros(2)-control working group. From what I understand, also by reading this ticket, is that ros-controls has mostly been the blocker for a release of gazebo_ros_pkgs in the past. Hence, I'd advocate to have the plugin live within the ros-controls GitHub org. @j-rivero would you still be interested in maintaining the plugin even outside this repo?

@j-rivero
Copy link
Contributor

j-rivero commented Apr 8, 2020

@j-rivero would you still be interested in maintaining the plugin even outside this repo?

eeeh not really, all yours. +1 from my side. Maybe @chapulina has other views on this?

@chapulina
Copy link
Contributor

Happy 8th birthday, ticket :)

I think you mean 6th? 🤔 😉

have the plugin live within the ros-controls GitHub org

No objections from my side. I'd just like to clarify that we're only talking about the ROS 2 version. I don't think we should touch the ROS 1 location at this point.

One question is whether it would continue being part of the gazebo_ros_pkgs metapackage? In the interest of reducing friction during release, I guess not, right?

would you still be interested in maintaining the plugin even outside this repo?

Open Robotics is definitely interested in helping with the initial push to implement it. Happy to give up maintainership after that.

@ddengster
Copy link

ddengster commented Apr 22, 2020

@chapulina
Sidetrack, I got a partially ported ROS2 gazebo_ros_controls, it might be of some help to you.
#1083

@chapulina
Copy link
Contributor

Hence, I'd advocate to have the plugin live within the ros-controls GitHub org.

As a first step towards that, I created a repository on this org so that we can target development there. We can move it to the ros-controls org once that's possible (I'm not sure who takes care of that org).

https://github.com/ros-simulation/gazebo_ros2_control

The idea of giving it its own repository is not to add Gazebo as a dependency to ros2_controls. Please speak up if you have other ideas.

@Karsten1987
Copy link

thanks @chapulina for pushing things forward.
/cc @bmagyar

@bmagyar
Copy link
Contributor

bmagyar commented May 24, 2020

@chapulina good call! Once you guys are happy with the repo we could take it over to ros-controls (I'm the owner of the org)

@chapulina
Copy link
Contributor

I'm going to close this issue, with the following resolution:

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