moveit_simple_controller_manager: name and namespace params mixed up?

[bit-pirate]

I'm using the moveit_simple_controller_manager to connect moveit with the controllers of my robot. However, I got confused by the parameters for the controller's name and namespace .

My controllers.yaml looks like this

controller_list:
    - name: /my_robot
      ns: arm_controller
      [...]

I expected to put the action server's name _armcontroller into name and its namespace _/myrobot into ns. However, I found out it is the opposite. So, to get it working, I had to modify the parameters like shown above. Unfortunately, this prevents me from adding more controllers, since they would all be named _/myrobot.

I don't know, if this behaviour is intended or has just been unnoticed. I would suggest to chance the order of namespace and name in action_based_controller_handle.h:

78    controller_action_client_.reset(new actionlib::SimpleActionClient<T>(name_ +"/" + namespace_, true));

@https://github.com/ros-planning/moveit_plugins/blob/groovy-devel/moveit_simple_controller_manager/include/moveit_simple_controller_manager/action_based_controller_handle.h

[mikeferguson]

Name is meant to be "arm_controller", ns is typically something like "follow_joint_trajectory", see the example setup on Maxwell: https://github.com/mikeferguson/maxwell/blob/sixdof/maxwell_moveit_config/config/controllers.yaml

I suppose here, what you are doing is actually pushing the entire plugin into a "my_robot" namespace, and that isn't what the namespace parameter is for. Does it work if you do "/my_robot/arm_controller" as the name?

[bit-pirate]

Hey fergs! I have started with your example and modified it until it worked for me.

Yes, it is true, my controller is living in the robot namespace (and with it all its topics). I suppose, name should be the action server name, which is in my case "arm_controller". I actually don't understand why one would want to add a namespace after the action server's name, since for connecting to an action server providing its name is enough.

I have tried your idea as well. Unfortunately, the simple_controller_manager adds _follow_jointtrajectory by default, if no name is supplied, i.e. "".

[isucan]

To try to clarify this a bit, at least in the moveit_pr2 plugin, we use action_ns instead of ns.

[mikeferguson]

@isucan Yep, see that now, that change looks like it probably happened after I forked off the original plugin for Maxwell.

@bit-pirate As for the namespace after the action server, this is a carry over from when the FollowJointTrajectory action was new, and also the PR2. For a while, I think the PR2 had (and might still have) /arm_controller/follow_joint_trajectory as well as an older arm_controller/joint_trajectory. I suppose at this point, we can stop using the ns afterwards, and fix up how the name is determined (there's no real downside to using 'arm_controller/follow_joint_trajectory' as the name of the controller I think).. I'll have to test that

[mikeferguson]

I've committed (7537025a2a7bf95559beece99fcd081a466c430a), a change that will:

• rename ns to action_ns, you can still use ns for now, but you will get a warning
• removes the defaults for ns, and avoids adding '/' if there no ns defined.
• warns if you have not defined any ns, action_ns, that there is no longer a default 'ns' value

I've tested on Maxwell, see updated config here: https://github.com/mikeferguson/maxwell/commit/2cb145059be586da85cd7654d9c04ec85675d293

@bit-pirate, could you try this out and make sure it meets your needs

@isucan I think you wanted to update baxter's config to use action_ns.

When I hear back that all looks good, I'll push a release out.

[bit-pirate]

@fergs this indeed meets my needs. Thanks a lot for the quick fix! Considering this done.