-
Notifications
You must be signed in to change notification settings - Fork 773
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
[gazebo_ros] Remove ROS remapping arguments from gazebo_ros launch scripts. #390
[gazebo_ros] Remove ROS remapping arguments from gazebo_ros launch scripts. #390
Conversation
Could you please merge the current jade-devel branch so we get some testing from the travis service? Thanks. |
Here we go... |
@@ -1,6 +1,10 @@ | |||
#!/bin/sh | |||
final="$@" | |||
|
|||
# remove ros remapping arguments like "__name:=node_name"; otherwise, they'd | |||
# be interpreted as a world name by gzserver, and it'd crash while loading it | |||
final=`echo "$final" | sed 's/[^ ]*:=[^ ]* \?//g'` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we probably don't need the echo call, does sed 's/[^ ]*:=[^ ]* \?//g' <<< $final
works for you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it works. But I haven't known something like <<<
exists. Isn't it more readable to do it with the echo
call?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it more readable to do it with the echo call?
I would say that is a subjective matter but we are saving one extra call and only run grep. minimal minimal peformance impact but better use of tools, IMHO. I leave it up to you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay. Then you know my preference :)
[gazebo_ros] Remove ROS remapping arguments from gazebo_ros launch scripts.
Thanks for the patch! |
Solves issue #387 .