Support to use gazebo and ROS parameters when using rosrun

[peci1]

If I try to run gzserver from a roslaunch file, e.g. like this:

<node name="gzserver" pkg="gazebo_ros" type="gzserver" />

gzserver crashes with

[DEBUG]: remap: __name => gzserver
[DEBUG]: Adding tcp socket [11] to pollset
[DEBUG]: UDPROS server listening on port [58980]
[DEBUG]: Publisher update for [/clock]:  already have these connections: 
[DEBUG]: Started node [/gzserver], pid [30771], bound on [peci1-Latitude-E6540], xmlrpc port [40902], tcpros port [60227], using [sim] time
[ INFO]: Finished loading Gazebo ROS API Plugin.
[DEBUG]: XML-RPC call [lookupService] returned an error (-1): [no provider]
[ INFO]: waitForService: Service [/gazebo/set_physics_properties] has not been advertised, waiting...
[DEBUG]: Accepted connection on socket [11], new socket [14]
[DEBUG]: Adding tcp socket [14] to pollset
[DEBUG]: TCPROS received a connection from [127.0.0.1:45466]
[DEBUG]: Connection: Creating TransportSubscriberLink for topic [/rosout] connected to [callerid=[/rosout] address=[TCPROS connection on port 60227 to [127.0.0.1:45466 on socket 14]]]
[DEBUG]: shutting down due to exit() or end of main() without cleanup of all NodeHandles
gzserver: /usr/include/boost/thread/pthread/recursive_mutex.hpp:101: boost::recursive_mutex::~recursive_mutex(): Assertion `!pthread_mutex_destroy(&m)' failed.
Aborted (core dumped)

I found out it is because of the __name and __log ROS parameters passed to rosrun. The commandline parameters actually are:

__name:=gzserver __log:=notimportant -s path/devel/lib/libgazebo_ros_paths_plugin.so -s path/devel/lib/libgazebo_ros_api_plugin.so

And I've found out that gzserver doesn't like the ROS arguments on its commandline (nor does rosrun remove them, which I'd expect).

I propose to patch the gzserver, gzclient and gazebo scripts by adding a line like:

final=`echo "$final" | sed 's/__[^ ]*:=[^ ]* \?//g'`

which would remove all ROS remapping arguments from the commandline. Or would it be better to remove all ROS argument mappings, just for case someone passes some params?

Or is this whole case just a misconfiguration of my computer? Could you please test with e.g.

rosrun gazebo_ros gzserver __name:=gzserver

[scpeters]

Which version of gazebo are you using? I was just able to rosrun gazebo_ros gzserver without any problems.

[peci1]

I run a source build of Gazebo 7.0.0 . I can also test with 2.2.6 tomorrow.

[scpeters]

I tested with gazebo7 built from source

[scpeters]

what do you see if you add --verbose?

[peci1]

...together with ROS Indigo on Trusty, if that matters.

[peci1]

This is the error when running rosrun gazebo_ros gazebo --verbose __name:=asd :

[Err] [Server.cc:365] Could not open file[__name:=asd]

This issue is present in gazebo 2.2.3 installed via apt, in 2.2.6 compiled from source, and in 7.0.0 compiled from source (with appropriate gazebo_ros_pkgs).

I actually think it makes sense rosrun doesn't strip the remapping arguments, because normally they are processed by the ROS node itself in a call to ros::init(argv, argc, defaultName).

[scpeters]

Hm...I might not understand what the problem is. I wouldn't expect the following to work because it gives invalid arguments to gazebo:

rosrun gazebo_ros gazebo --verbose __name:=asd

But gazebo can be launched from a launch file (empty_world.launch). Is there something broken about empty_world.launch (as an example)?

[peci1]

Aha, the behavior is kind of weird.

 rosrun gazebo_ros gazebo worlds/joints.world --verbose __name:=asd  __log:=asdsa

runs correctly.

 rosrun gazebo_ros gazebo __name:=asd  __log:=asdsa worlds/joints.world --verbose 

fails with

 [Server.cc:365] Could not open file[__name:=asd]

So it seems Gazebo requires the first "non-option" (without dash)
argument to name a world, and the other are ignored. Therefore, the
first call succeeds (and the __name and __log args are just ignored),
whereas the second one misinterprets __name as a world path.

This fortunately means that the current way of running gazebo via
roslaunch works.

However, it may stop working at any time when Gazebo stops silently
ignoring the additional arguments. Therefore, I still propose to remove
the ROS remappings from gzserver command-line in scripts/gzserver.

Martin Pecka

[scpeters]

removing the ROS remappings from the scripts sounds good to me

[j-rivero]

thanks for the contribution. closing.

[jonbinney]

Removing the ROS remappings here breaks things (like remappings!)

[peci1]

@jonbinney Hmm, seems you're right. We fixed a possible segfault, but obviously have broken ROS remapping support.

@j-rivero @scpeters Should we rather fix this in Server::ParseArgs()? I can imagine we could strip all [^ ]*:=[^ ] \?* arguments from the argv before passing it to the command line parser. That way, the ROS remappings could be located anywhere in the commandline.

[j-rivero]

Removing the ROS remappings here breaks things (like remappings!)

eeeh, ooh, ... I failed reading the propose of the script and though that it only took care of the gazebo call. Sorry for that.

@j-rivero @scpeters Should we rather fix this in Server::ParseArgs()? I can imagine we could strip all [^ ]:=[^ ] ? arguments from the argv before passing it to the command line parser. That way, the ROS remappings could be located anywhere in the commandline.

Upstream gazebo code should know nothing about ROS so we need to implement the patch in gazebo_ros_pkgs code.

I'm going to revert the change and release tomorrow while we find a different solution.

[peci1]

Ok, so we need a "bash magic" that'd go through all arguments, split them into ROS- and non-ROS arguments, put the non-ROS ones first, and the ROS arguments last? Would that work well?

At least for (my) original scenario, that should fix the problem.

[jonbinney]

In ROS nodes, the ros client code magically removes the ROS specific arguments, so the program's normal command line parsing only sees what is left: http://wiki.ros.org/roscpp/Overview/Initialization%20and%20Shutdown#Accessing_Your_Command_Line_Arguments

One extreme option would be to write a thin C++ wrapper to gazebo that just initializes ROS and then runs gazebo as normal. I'm not very familiar with the gazebo codebase though, so there are probably reasons that would be bad.

[j-rivero]

After reviewing the whole issue I have some clear ideas now:

• The issue originally was about the use of ROS arguments and Gazebo arguments in the same rosrun command, if I understand it correctly now. Note that while this is possible, rosrun should not handle directly any upstream application argument passed in the command line since it expects ROS arguments. upstream application arguments should be wrapped inside ROS and managed using ROS mechanism.

• What happened after we removed the remappings from the wrapper scripts in gazebo_ros_pkgs was that we killed them for ROS since they need to arrive to the ros::init function. They arrive to the ros::init function since the wrapper script calls gazebo using the gazebo_ros_api_plugin and that plugin passes the whole command executed in the terminal to ros::init. If you look into the ros::init implementation, it is handling the remappings at the beggining.

• Although I highly discourage from mixing gazebo and ROS arguments, I think that we can easily workaround on the problem and make Martin examples' works. If we parse the command line arguments in the wrapper scripts and move the remappings to the end, that should be enough. Let me send the PR.

Anything more I could be missing?

[jonbinney]

Re-ordering the arguments sounds like a reasonable approach to me. In that case the only argument that gazebo will use is the first one, which will be the world name, right? Does gazebo ignore all arguments after the first one?

[j-rivero]

Does gazebo ignore all arguments after the first one?

My knowledge of boost program_options is limited but I would say that it allows unregistered arguments after it found the world file. That makes sense if we want to allow the plugins to use command line arguments.

[j-rivero]

Martin, John if you could give a quick try to the PR to make sure that I'm not breaking anything serious, that would be awesome. Thanks for the comments.

[peci1]

@j-rivero Sounds reasonable to me.

Maybe the wrapper should issue some kind of warning about what it did with the arguments. It's just not clear to me what kind of warning it should be - neither ROS nor Gazebo are initialized in the wrapper script to provide the logging capabilities. Maybe a simple stderr printout? (An obscure way to get some "standardized" log record would be publishing to /rosout from the wrapper script, but it sounds too obscure even to me.)

I got a little bit confused when I looked at rosrun docs and source. There it is stated that even rosrun itself can take arguments other than package, program and the remappings (like --prefix gdb). Fortunately, according to the source code, these arguments should come before the package name and get "eaten" by the rosrun script before the argument list is passed to the program. So now I'm practically convinced that moving the remappings, and only the remappings is the right thing to be done.

Last, we could try to somehow formalize all the steps that take part in this argument reordering magic. I think it would be nice to say it explicitly in the docs (of upstream Gazebo) that all arguments following the world name are ignored by gazebo server itself and are dedicated for processing by system plugins. And add a test for this case (if it's not yet there).