Can't initialize effort_controllers/JointPositionController with non-global robot_description parameter

[piyushk]

Ref: http://answers.ros.org/question/249138/cant-initialize-effort_controllersjointpositioncontroller-with-non-global-robot_description-parameter

Here's an outline of my problem:

1. I have multiple robots in Gazebo with different URDF descriptions. Each robot is pushed into a namespace (/robot1/...), and each frame id is pre-pended with a prefix (robot1_....). Consequently, I have no robot_description param in the global namespace.
2. libgazebo_ros_control (running inside Gazebo) initializes ControllerManager. Gazebo runs in the global namespace.
3. effort_controllers/JointPositionController (running inside Gazebo) calls urdf::Model::initParam which does not respect the controller's constructed namespace, and executes a searchParam in Gazebo's global namespace, which fails.

I can't figure out how to tell the controller to read the robot_description from robot1/robot_description.

Furthermore, It's unclear to me why each controller is trying to read the URDF independently in the first place. Doesn't it make more sense to use the controller manager.

If this issues needs a patch, and I can provide one. Would an acceptable solution be that I duplicate Model's search inside a controller, since the controller has the correct NodeHandle?

[bmagyar]

Hiya,
I am one of those guys who never really ran multi-robot simulations so forgive my ignorance.
You have separate controller managers per robot, right?
The controller here should use the appropriate namespacing for the robot_description parameter, possibly taken from the nodehandle instance that is passed to the init function.

Which ROS distribution are you running?

[piyushk]

@bmagyar. I'm on Kinetic. Multi-robot simulations have always been a mess :)

I have separate controller managers per robot, each launched via a separate gazebo_ros_control plugin (here).

Now gazebo_ros_control creates a correctly namespaced NodeHandle, and passes it to the init function as you mentioned (even though the node is running in the global namespace). However, Model::initParam creates its own nodehandle here, and does not use the NodeHandle passed in the init function of the controller. I believe they are expecting initParam to run as part of a node in the correct namespace (/robot1), but instead here it runs inside the gazebo node, which is in the global namespace.

I propose to replicate Model::initParam's search functionality in each controller, using the NodeHandle provided in the init function.

[bmagyar]

How about adding an extra optional nodehandle argument to initParam? That would probably be a more consistent change and many others could benefit from it directly. Then the nodehandle in controllers could be passed.

I'm trying to think about the cleanest solution here.

[piyushk]

@bmagyar: That sounds fine to me, and I think I can introduce the change in backwards compatible way. I'm fine implementing whatever change you suggest, since my experience with ros_control is fairly limited.

[bmagyar]

@wjwwood, what is your opinion on adding this to the ROS urdf API?

[progtologist]

Wouldn't it be simpler and less intrusive if we just added a parameter in the namespace of the controller just as the other parameters that specifies (if set) the path to the URDF file in the server? That would even allow different names (not only robot_description) and would cause no changes to the API of the urdf::Model class.

[bmagyar]

@piyushk Could you please provide the matching PR here as well so that guys over at robot_model can assess the need for it?

@progtologist ros_control is not a minor project in ROS and it's ok to extend APIs if need be, besides I've seen this issue come up before, I think many could benefit from an update to the urdf API. I would stick to the name robot_description, it is one of our standard names that is consistent through different robots.

[progtologist]

@bmagyar I am proposing something as simple as this:

std::string urdf_path;
  if (!n.getParam("urdf_path", urdf_path))
  {
    ROS_WARN("No urdf_path given (namespace: %s), using global /robot_description", n.getNamespace().c_str());
    urdf_path = "robot_description";
  }
if (!urdf.initParam(urdf_path)) {
  ...
}

[bmagyar]

I understand the workaround but I believe we should not do it.

I prefer to only add new parameters relevant to the controllers, plus the point about not allowing names other than NAMESPACE/robot_description.

There's always a way to add more configuration and make more things explicit but then we are sliding down the slope towards configuration hell soon to also roast in dependency hell.

[piyushk]

@bmagyar: I've done so.

In addition the workaround proposed by @progtologist has already been implemented in a couple of controllers:
https://github.com/ros-controls/ros_controllers/blob/kinetic-devel/gripper_action_controller/include/gripper_action_controller/gripper_action_controller_impl.h#L44
https://github.com/ros-controls/ros_controllers/blob/kinetic-devel/joint_trajectory_controller/include/joint_trajectory_controller/joint_trajectory_controller_impl.h#L79

The following controller seems to be doing it's own vairant:
https://github.com/ros-controls/ros_controllers/blob/kinetic-devel/diff_drive_controller/src/diff_drive_controller.cpp#L457

I can also unify all of these solutions to that proposed in PR #245, however I don't know if I have a clear way of testing them.

[wjwwood]

@bmagyar sorry I didn't respond right off.

I haven't had time to completely catch up with the issue here, so I'm not entirely clear on what "it" is in this case (that you'd like to add to urdf), but I gather it is changing or adding API to allow passing in a NodeHandle to some part of the URDF API to better handle namespacing? I think that might be ok, but honestly I don't have enough context to see whether or not it makes sense.

I'm sorry that I don't have better guidance for you guys, but you'll have to figure out what should be what on your own and propose a change to me if you think one is required. You're best bet for getting feedback on whether or not something could be put into URDF is to describe what you're thinking of in more detail for me, a preliminary pull request which demonstrates the API changes would be best.

[piyushk]

@wjwwood: Here's a summary of the changes in PR form:

ros/robot_model#168
#245

In short, NodeHandles constructed for controllers in Gazebo can be in a different namespace (/robot1/) than the default namespace (/), so the parameter search for robot_description in URDF fails.

[wjwwood]

Thanks @piyushk, sorry for not associating this issue with the robot model pr. I'm still trying to catch up my email. I'll comment on it asap.

[progtologist]

@wjwwood, @bmagyar don't get me wrong, I also prefer the "clean" change in the urdf::Model, however I am hesitant as I don't want to wait for a long time until this gets integrated. What @piyushk is suggesting is much better than my proposition, but it is also slower to get to the users (and benefit from this).

[sloretz]

I didn't see it mentioned here. What do you think about the JointPositionController calling resolveName("robot_description") and passing the fully qualified name into urdf::Model::initParam()?

[mathias-luedtke]

and passing the fully qualified name into urdf::Model::initParam()?

According to the code, this should be the default behaviour.

[sloretz]

According to the code, this should be the default behaviour.

The default behavior for urdf::Model is to use searchParam() to get the fully qualified name in the node's namespace.

Currently JointPositionController is asking urdf::Model to load "robot_description", but what it actually wants is to load "robot_description" from the namespace known only to the NodeHandle passed in via JointPositionController::init(). One way to accomplish that is to pass the NodeHandle to urdf::Model. Another is to use resolveName() on that NodeHandle and give urdf::Model the fully qualified name. The latter would get the fix out to users faster as it only requires a change to JointPositionController.

[mathias-luedtke]

@sloretz:
I don't think that calling resolveName() helps here, as it is meant for resolving remaps (?).
But you're right: JointPositionControllercould use searchParam() on its NodeHandle and pass the result to initParam() (here).

[shisha101]

I have been facing a the same problem on ros indigo and have created another issue #256 and PR #255.
Please let me know if it should stay separate or if you would like it to become part of this issue, as the urdf::Model::initParam() API change did not affect Indigo.