SPAWNER: Wait for the clock to be available before proceeding

[guillaumeautran]

This patch is to resolve an issue when the simulated clock is not published right away (coming from a remote gazebo instance for example). If the clock is not published right away, the rosservice wait_for_service times out prematurely and the controllers fail to be loaded since the system is not yet ready (no Gazebo clock).

With no, gazebo system, the clock will be ready immediately and the spawner will proceed.

Issue: #431

[matthew-reynolds]

It looks like the same issue was fixed upstream for C++ in roscpp here, would this be better suited as an upstream fix for Python in rospy?

[guillaumeautran]

I can make a similar fix in rospy if you think it is "safe" and has no side-effects.

[guillaumeautran]

Actually, @matthew-reynolds , the change that you pointed is actually the opposite to what this change is suggesting. In the referenced change, the service wait fails because it uses the ros::Duration as opposed to the ros::WallDuration. The former takes in consideration the /clock topic while the later does not. rospy wait_for_service does not currently rely on the simulated clock, which is consistent with the roscpp version. So, no, modifying ros_comm is not the right approach.

[matthew-reynolds]

@guillaumeautran @mikepurvis Bump. Is this approach being used at Clearpath? Assuming it's seen some use and has had a positive impact, we should try to get it tidied up and merged.

Edit: And re-targetted to Noetic

[bmagyar]

I think this is a useful feature we could incorporate, happy to cherry-pick to melodic too but indeed let's aim the PR to noetic first.

If @matthew-reynolds is happy I'm happy to merge & release to Noetic

[guillaumeautran]

Very sorry for dropping the ball on this one. I know its been a while..... The patch is now rebased to target Noetic (instead of Melodic).

@matthew-reynolds WRT wrapping the check in the use_sim_time param, any clean suggestions on implementation? The sim time check is not used anywhere in ros-control (from what I can tell). Does not mean it should not be done but just making sure that's what we want before introducing a new dependency here.

Here is what I am thinking:

    # Wait for the clock to be published
    if rospy.get_param('/use_sim_time', False):
        while rospy.get_rostime() == rospy.Time(0):
            rospy.loginfo_throttle(30, "Waiting for /clock to be available...")
            rospy.sleep(0.2)
        rospy.loginfo("/clock is published. Proceeding to load the controller(s).")

[matthew-reynolds]

The sim time check is not used anywhere in ros-control (from what I can tell). Does not mean it should not be done but just making sure that's what we want before introducing a new dependency here.

I think that's fine. In general I agree, ros-control should be agnostic to time base, but I think it's appropriate here in a spawner script.

if rospy.get_param('/use_sim_time', False):

Shouldn't this be the other way (True)? We only want to do this check if using sim time (e.g. from Gazebo), right?

[guillaumeautran]

The statement: if rospy.get_param('/use_sim_time', False): Means that if the parameter is not defined, the returned value (default) should be False hence bypassing the check altogether. I am assuming that the rosparam will only be defined if in simulation...

[guillaumeautran]

Perfect. Thanks much!

[guillaumeautran]

ping @bmagyar for review.