-
Notifications
You must be signed in to change notification settings - Fork 774
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
Refactor gzserver launch args, add --initial_sim_time #1456
Conversation
Add a helper function to replace the functionality of _arg_command with the condition argument. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
gazebo_ros/launch/gzserver.launch.py
Outdated
def _arg_command(arg, join_with=" "): | ||
cmd = ['"--', arg, '" if "" != "', LaunchConfiguration(arg), '" else ""'] | ||
py_cmd = PythonExpression(cmd) | ||
return (py_cmd, join_with, LaunchConfiguration(arg)) |
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.
It might be harmless but in the case that the launch configuration is an empty string you'll still be adding the join with. I think anyway.
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.
And actually if the join with is = that would be a problem no?
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.
I knew I was forgetting something 🤦
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.
This refactors the _arg_command helper function to remove the condition parameter (which is now unused since the _conditional_command helper was added) and always passes the LaunchConfiguration along with the --command with a "join_with" separator. This simplifies the `cmd` array definition. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Expose gzserver CLI parameter added in gazebosim/gazebo-classic#3294. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
75df23b
to
c6f90bc
Compare
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.
Looks good to me!
This refactors some of the argument handling in
gzserver.launch.py
and adds support for the--initial_sim_time
parameter, which was recently added in gazebosim/gazebo-classic#3294. It is straightforward to review the changes by commit:_arg_command
helper with acondition
to a separate_conditional_command
helper function and update theparams_file
support to use_conditional_command
.condition
argument from_arg_command
and add ajoin_with
argument instead that defaults to a space' '
. Also, since all the remaining uses of_arg_command
also pass the correspondingLaunchConfiguration()
right afterward, change the return value of_arg_command
to a list joining the--command
string with theLaunchConfiguration()
value. This simplifies the specification of thecmd
variable in the launch description.--initial_sim_time
argument, usingjoin_with='='
so that it will be passed as--initial_sim_time=[time]
. This is a valid alternative to--initial_sim_time [time]
but allows old versions of gazebo to parse it all as a single argument and ignore it as unrecognized. Otherwise the--initial_sim_time
may be ignored, but the[time]
argument may linger and cause unintended parsing issues.