Option to disable publishZeroVelocity in the controller server

[reinzor]

Feature request

Feature description

Option to disable publishZeroVelocity in the controller server.

Implementation considerations

For our use case, we would like to start a new path when driving so our end state is not necessarily standing still. We would like to have full control of the command velocity references within our controller. Related issues in move base flex (ros1 implementation that we were using):

• mbf publishes zero cmd_vel if computeVelocityCmd returns False naturerobots/move_base_flex#195
• Publish zero velocity when reached to goal.  naturerobots/move_base_flex#109

Would it be possible to register a new parameter for the controller_server, e.g. publish_zero_velocity that defaults to true and use this parameter in an if statement for this line?

Thanks!

[SteveMacenski]

Do you also want to reset the controllers? If not, then I think that if statement should be inline every time publishZeroVelocity is called (or change to publishZeroVelocityIfNecessary and have the if be around the entire block).

This seems reasonable. Can you motivate an example of what you're doing to help me model this in my brain for future additions?

[reinzor]

I think we only do not want to publish the zero velocity. If we name the parameter publish_zero_velocity and this would change the reset behavior, that is not really expected as a user. So I would suggest only skipping this actual zero velocity publishing.

Maybe we should refactor it as follows by creating a new function onGoalExit that is called when a goal exits, so the current calls of publishZeroVelocity can be replaced by calling this new function. In this function we call optionally publishZeroVelocity and we move the reset controller logic to this function as well. Would that be an idea?

In our use case we are sending path segments to the controller where every path segment is a new goal for the controller. The last point of our path segment is not necessarily a zero velocity path point, this is currently assumed by the controller server.

[SteveMacenski]

a new function onGoalExit

Who would register this - the controller plugins? In that case, we're just kicking the can down the the pipeline and each controller needs to implement that same parameterization for what is a 'system' behavior. I don't dislike the idea of making it user set, but I don't see the path to do so that doesn't involve a new plugin class (which may be overkill for a bool param unless you think we may need to do additional things in the future).

In our use case we are sending path segments to the controller where every path segment is a new goal for the controller. The last point of our path segment is not necessarily a zero velocity path point, this is currently assumed by the controller server.

All good and fine, but why not preempt the controller with a new path segment before it reaches the end (I would think you'd want to do that any way such that it doesn't hit the end and undefined behavior for a 10-100ms before a new segment comes in). We can have this new param merged, I'm not arguing with you, but curious. Maybe I think overly cautiously.

[reinzor]

Who would register this - the controller plugins?

We are just renaming the publishZeroVelocity method in this approach. So it will still be functionality from the controller server. We could also keep the existing naming in tact but I think it is better to rename since the function is currently doing more than just publishing the velocity: it is also resetting the controllers.

why not preempt the controller with a new path segment before it reaches the end

This would mean external tracking if a goal is (almost) finished. It's easier to let the controller determine that

undefined behavior for a 10-100ms before a new segment comes in

Currently this is acceptable for us, something to keep in mind for our roadmap.