ros-controls / ign_ros_control

ros_control plugin for Ignition
Apache License 2.0
9 stars 4 forks source link

Use a more friendly way of ROS initialization #3

Open peci1 opened 1 year ago

peci1 commented 1 year ago

I've noticed this plugin assumes that it should be the one that initializes ROS. This doesn't seem to be very friendly to other plugins. Why doesn't it use a similar approach to gazebo_ros with a dedicated ROS API plugin that handles the CLI parameter loading, and all other gazebo-ros plugins just expect "somebody" has already done this for them? I see there isn't anything similar in ros_ign_gazebo, but I think that should be the way to go...

graiola commented 1 year ago

Hi @peci1, thanks for opening the issue. I am going to look into that in the next days.

graiola commented 1 year ago

Hi @peci1 , what do you mean by initialize ROS? ign_ros_control is supposed to work similarly to gazebo_ros_control plugin. Can you provide an example or some lines of code so that I can fully get the problem? Thank you :)

peci1 commented 1 year ago

I'm talking about these lines:

https://github.com/ros-controls/ign_ros_control/blob/master/ign_ros_control/src/ign_ros_control_plugin.cpp#L282-L287

With this implementation, no other plugin can initialize ros.

In gz classic, there is a specialized plugin (gazebo_ros_api_plugin) that is independent of all other plugins and is said to be the one that should call ros::init on behalf of all other plugins. All other gz-ros plugins just expect that somebody else has done the ros init call and do not try to make it themselves.

graiola commented 1 year ago

I see your point, but I don't know how it is handled with Ignition. I think the equivalent with ros2 is here:

https://github.com/ros-controls/gz_ros2_control/blob/master/ign_ros2_control/src/ign_ros2_control_plugin.cpp#L319-L326

I noticed that in the lines you pointed out there is no return, which means that it should work also if ros has been initialized from another plugin. Did you try that? In this case the ROS_ERROR should be converted into a ROS_WARN or ROS_INFO.

peci1 commented 1 year ago

You're right that technically, the program does not exit, so it runs in the end even if someone else started ROS. However, I still don't consider it correct as other systems might actually want to pass some non-trivial argc and argv to the init call, but if this system runs first, their arguments are never passed. See the gz-ros2 example you've mentioned - they allow to specify <remappings> tag in SDF whose contents will be added to the argv. I think this should be the task of a system that is independent from ros-control. Exactly the way it was done in gz_classic-ros1. The only difference here is that the true argc and argv read from the CLI command are not available in gz (or at least I don't see any easy way to read them). This means ROS remapping args and running this node in a namespace won't work.