auto sim_time parameter

[Timple]

I've been wanting to implement something that prevens us from forwarding the sim_time parameter for all nodes.

Now I noticed that such an implementation is already there in the form of sl.auto_sim_time() :rocket:.

It does depend however on the /clock topic. But when launching a lot of nodes and the simulator in a single launchfile there is a race-condition correct? The /clock topic is not present until the simulator is up and running.

[oKermorgant]

Hi,

Indeed auto_sim_time relies on /clock, that makes it unsuitable to launch both the simulation and other things in a single launchfile. It is the same with the new IgnitionBridge things that need to communicate with Ignition to detect models.

As usual when I do a release I forget things, it should be easy to pass a Boolean to auto_sim_time that would forward it to all nodes without even checking /clock. The only drawback that I see is that this Boolean has to be written explicitly in the launch file and potentially other, included, launch files. This can be managed through a launch argument but in this case I guess we cannot fallback on checking /clock as it is difficult to do a raw Python condition from a launch argument.

Having an optional parameter does not change the current auto_sim_time API, so I'll add this in the source repo for now and wait for a few more ideas before releasing.

[oKermorgant]

Corresponding fix in https://github.com/oKermorgant/simple_launch/commit/c2da759a18507cb2754658499611f8940b6ceecf I'll try to find a way to fallback to checking /clock to make it possible to write such a file:

sl.declare_arg('use_sim_time', None)
# fallback to check /clock is argument is not set to True or False
sl.auto_sim_time(sl.arg('use_sim_time'))

For now it works if the launch argument is either True or False, but not None.

[Timple]

The only drawback that I see is that this Boolean has to be written explicitly in the launch file and potentially other, included, launch files. This can be managed through a launch argument

Yes, this is still an inconvenience, but is very much in-line with the complete launch infrastructure. So one can choose to use simple_launch for some files, but others can include these without knowing (or caring about) the internals.

Perhaps the argument can automatically be added and set?

sl = SimpleLauncher(auto_sim_time=true)

[oKermorgant]

The proposed syntax, through the constructor, is interesting and especially it prevents from adding nodes before setting the sim_time.

On the opposite, using a separate function after the construction allows not taking care of this aspect (by not calling sl.auto_sim_time). Also, I am not sure can can use a launch argument in the constructor as the SimpleLauncher instance needs to be created in order to declare and retrieve such launch arguments. So it would be fine for a raw Boolean.

I think I'll stick to the auto_sim_time function, and try to allow:

• setting to True/False (possible for now, raw Boolean or through launch argument)
• detecting /clock (possible for now by calling sl.auto_sim_time() without argument)
• not caring (possible for now by not calling the function)

Also to my point of view, seeing this function call makes it more explicit for the reader that we are tampering with the use_sim_time parameters.

[oKermorgant]

Ok I think we can set it in the constructor as the instance can then automagically create a launch argument. Also it is slightly less auto now so probably just using use_sim_time is more clear.

SimpleLauncher(namespace = '', use_sim_time = None)
'''
  Initializes entities in the given workspace
  If use_sim_time is True or False, creates a `use_sim_time` launch argument with such default value and forwards it to all nodes
  If use_sim_time is 'auto', then SimpleLauncher will set it to True if the /clock topic is advertized (case of an already running simulation), and to False otherwise
'''

[Timple]

That might be unexpected. People might have a /clock topic (unused). But suddenly their nodes behave different.

In that case the explicit option is better. Or use_sim_time="auto". But it might be a bit weird that you can pass a boolean and a string.

[oKermorgant]

All right a constructor option (thanks for the good idea) was pushed in https://github.com/oKermorgant/simple_launch/commit/1971ed6484175998e6bea82c2ea4e3ab24401a67.

I have no particular feeling on variable types for arguments, to my point of view it is a classical Python workaround for overloading. I have put a small warning in the Readme.

[oKermorgant]

Part of the last foxy / galactic sync http://repo.ros2.org/status_page/ros_foxy_default.html?q=simple_launch http://repo.ros2.org/status_page/ros_galactic_default.html?q=simple_launch

The last commit (use_sim_time in constructor) did not make it to Rolling which is now stabilized for 22.04. I'll forward it once things get rolling again.