Attempt to fix the goal time violated issue

[firesurfer]

This is the same as #880 but is based on main and does not include #871.
Tries to fix the issue described in #871 @fmauch

[firesurfer]

@RobertWilbrandt might want to review this patch ?

[firesurfer]

@fmauch and @RobertWilbrandt ?

[firesurfer]

And again @fmauch and @RobertWilbrandt

[fmauch]

While I do see the issue you've pointed out (thank you for that by the way), I'm afraid I don't think this solution satisfies the controller's intentioned behavior regarding timing failures.

I'm not sure whether we ever stated that clearly, it's definitively not inside the documentation, but

• The controller should not throw a PATH_TOLERANCE_VIOLATED due to being scaled down. This is definitively covered by your approach, since the time difference only takes the "virtual" trajectory time into account. (While writing this I think this isn't actually fulfilled with the current main branch, either)
• The controller should throw a GOAL_TOLERANCE_VIOLATED if it doesn't reach its goal in the specified time (and goal time tolerance is actually set). This is not fulfilled by your approach since that uses the same time difference for calculating the time difference at the trajectory end.

In fact, I wasn't able to generate any timing-related violation with the code from this PR.

So, if we would use the "real" time passed when the trajectory is done, we should be at the desired goal. @firesurfer do you agree?

Sidenote: The upstream code regarding tolerances will change soon so we can actually set the tolerances in the trajectory goal which will enable us to provide proper integration tests for this feature :-)

[firesurfer]

Hi @fmauch

I think the only thing we have to agree on is: What does a violated goal time mean if we scale down the trajectory execution.

From my point of view the only reasonable answer is: The goal time has to be extended in the same way the trajectory has been scaled down. I think this patch implements this behavior. (See explanation below)
In order to create a goal time violated error I think you will need to: 1. Disable check if path tolerances. 2. Block the simulated arm in some way. The JTC will then check for the configured amount of time if the goal has been reached and it not return a goal time violated error

The reason I created this patch was that the Scaled JTC on the main branch is currently not usable with a configured goal time - which is on the other hand needed if you want the JTC to actually reach its target within the specified goal tolerance. (This change was introduced a while ago in the upstream JTC - see: ros-controls/ros2_controllers#759)

[fmauch]

Yes, I am aware of the problems on the main branch and I agree we need to do something about it.

As written above my understanding of goal time constraints when under scaling is different, but I do also understand that handling this differently to the checks during execution is somehow confusing.

Since the current state isn't really functional I would be fine with going the approached way at least temporarily and reevaluate once we adapt to the mentioned upstream changes that will make this question much more relevant.

[firesurfer]

Given that we use this patch in combination with #883 for multiple months now in our setup I think it is safe to merge :)

[firesurfer]

@fmauch So whats the state with this PR? I saw in the ros2_control wg meeting document that you plan on upstreaming this controller rather sooner than later. From my point of view it would be desirable to have this issue fixed before any code is upstreamed.

[firesurfer]

A friendly ping @fmauch :)

[fmauch]

As written above, let's go with that change for now until we figured out the final version inside ros2_controllers.

[fmauch]

Thanks again @firesurfer for fixing this and for your persistence!