FakeController: publish all via points of trajectory in real time

[rhaschke]

Current implementation of FakeController only published the last point of a trajectory, that is jumping to the end in one step. At least for debugging of #713 and #716, I found it useful to see the trajectory being traversed by the scene robot also in demo mode, which is what this PR implements.

If the old behavior has some specific value to anybody, we could also go for two fake controllers side by side, e.g. a new ViaPointController or implementing the old behavior as WarpController. By adapting controllers.yaml, one would need to switch between them.

What is your opinion?

[rhaschke]

Probably for benchmarking, we want to have the warping behavior.

[v4hn]

Could you have a look at https://github.com/ros-planning/moveit_ros/pull/708 ? These two requests are quite related as far as I can see.

[davetcoleman]

+1 to this version of a fake controller, see https://github.com/ros-planning/moveit_ros/pull/708#issuecomment-233097525

[rhaschke]

Don't yet merge this. I'm thinking of several alternative fake controllers to choose from. For benchmarking, the existing one (warping to the target) make sense. For debugging, the via-point controller makes sense. For nice visualization, the one from #708 makes sense.

I will re-open the PR if ready.

[v4hn]

Don't confuse me like that by closing a request I'm currently reading. XD If you want to support all three modes of operation, I would propose to have three different type entries aka FakeWarpController, FakeTrajectoryController, and FakeInterpolatedTrajectoryController.

One point I'm also very much interested in for the simulated controllers is a default configuration on startup. We have one setup where the simulated robotic arm always starts up colliding with a wall..

So what do we do with #706 ? Merge it there as an alternative? Close it and wait for your reimplementation here? Merge a version of it into moveit_plugins to work with the fake trajectory controller you plan to write?

[rhaschke]

Don't confuse me like that by closing a request I'm currently reading. XD If you want to support all three modes of operation, I would propose to have three different |type| entries aka |FakeWarpController|, |FakeTrajectoryController|, and |FakeInterpolatedTrajectoryController|.

This is what I planned.

One point I'm also very much interested in for the simulated controllers is a default configuration on startup. We have one setup where the simulated robotic arm always starts up colliding with a wall..

Good idea. But where do you want to specify this? I suggest to use an appropriately named default pose, whose name is a private rosparam of the controller node?

[davetcoleman]

I suggest to use an appropriately named default pose, whose name is a private rosparam of the controller node?

That is how my moveit_sim_controller currently works

I would propose to have three different |type| entries aka |FakeWarpController|, |FakeTrajectoryController|, and |FakeInterpolatedTrajectoryController|.

Is there a good use case for all three of these? Seems like FakeWarpController is simply a bad/quick implementation @isucan whipped up that could benefit from interpolation. Not sure the difference between the other two controllers you are listing.

Off topic side note: I hate that the ControllerManager is not named ControllerInterface, since that is actually what it is and things like ros_control are the ControllerManager

[rhaschke]

As I have written:

For benchmarking, the existing one (warping to the target) make sense - because its fast. For debugging, the via-point controller makes sense - you can see which via points were used. For nice visualization, the one from #708 makes sense - getting smooth interpolation.

Do you suggest to rename ControllerManager to ControllerInterface? I found the name clash with ros_control confusing too.

[v4hn]

+1 to ControllerManager being confusing. I do not believe this alone justifies a renaming though. I might be wrong here, but one could use the module as a "ControllerManager", right? It only happens to be the case that most implementations only interface between move_group and external action servers (or do not provide a proper controller at all). ros_control does its job quite well, so we could give up on that concept within MoveIt entirely and discourage people from trying to build controllers in MoveIt. This, together with the name conflict, might justify renaming the interface.

[davetcoleman]

I've never seriously put the suggestion forward because it seems like a big breaking change, but with recent momentum I guess we could

I might be wrong here, but one could use the module as a "ControllerManager", right?

Yes, there is a switchControllers(), getActiveControllers() etc. - good point.

ros_control does its job quite well, so we could give up on that concept within MoveIt entirely

I never thought of it that way, but I really like that idea. @isucan himself has said that MoveIt! tries to tackle too many things, I think it has suffered from some scope creep.

[v4hn]

On Tue, Jul 19, 2016 at 04:13:23AM -0700, Dave Coleman wrote:

I've never seriously put the suggestion forward because it seems like a big breaking change, but with recent momentum I guess we could

I do believe we could do this in kinetic

[davetcoleman]

I've turned this into an issue / proposal: https://github.com/ros-planning/moveit_ros/issues/725