CombinedRobotHW implementation

[toliver]

(Includes changes in PR #220)

This is related to some of the issues discussed in #75.

CombinedRobotHW allows to dynamically load a list of RobotHW objects defined in the parameter server (using pluginlib à la controller manager) and combines them into a single RobotHW. A single controller manager can then access resources from any robot.

Configuration example:

robot_hardware:
- my_robot_hw_1
- my_robot_hw_2
- right_arm
- left_arm
my_robot_hw_1:
  type: combined_robot_hw_tests/MyRobotHW1
my_robot_hw_2:
  type: combined_robot_hw_tests/MyRobotHW2
  joints:
  - test_joint4
  - test_joint5
right_arm:
  type: my_arm/MyArmHW
  address: 192.168.1.100
  robot_description: robot_description
  joint_name_filter: ra_
left_arm:
  type: my_arm/MyArmHW
  address: 192.168.1.101
  robot_description: robot_description
  joint_name_filter: la_

This is a similar approach to #151 but removing the need for a separate DeviceHW class, and for a URDF to be passed. It uses #220 to combine the resources of the different RobotHWs.

Only one change in this PR is not backwards compatible: All the classes derived from RobotHW will have to declare a parameterless constructor in order to build correctly. (See #231 (comment))

If a RobotHW derived class is intended to be loaded by CombinedRobotHW then it needs to implement at least:

• Parameterless constructor
• bool init(ros::NodeHandle& root_nh, ros::NodeHandle &robot_hw_nh) function that sets up and initializes the RobotHW (potentially reading ROS parameters)
• Parameterless read() and/or write()

Our motivation to write this class is to have an easy way to combine the Shadow Robot hand with different types of arms, so that we can have access to all the robot hardware resources from a single controller (e.g. to implement an impedance controller for arm+hand).

[carlosjoserg]

It sounds good, like I would want to use it a lot... my humble opinion as an user, if I may...

• Naming is always one of the hardest tasks. I think I prefer the class name to be RobotHWGroup from RobotHWGroup with aliased interfaces #138 or something like RobotHWCollector or RobotHWManager? Though I'm sure the community can propose more interesting names.
• I never liked the joint_name_filter: parameter. Perhaps first time I saw it was here for a gazebo plugin. I typically write the hw for a particular robot which I link to an urdf model. So the joint names are known by design. Thus, I think each hwiface should be aware of its own joints from a combined robot_description parameter using the robot name, or namespace, or prefix (used in ROS-I). But probably is my way of thinking...

Parameterless read() and/or write()

• If one uses the joint_limits_interface, one needs to pass the period, or elapsed time, computed from the control loop, as it is done e.g. in the gazebo_ros_control plugin here or in the (unofficial but useful) ros_control_boilerplate template here

[carlosjoserg]

[minor] PAL or Shadow?

[toliver]

Thanks, I didn't spot that.

[bmagyar]

Hi,
I will have to take some time to give this a shot. Would you consider adding a Gazebo example too?

I understand that this would be very desired to use with modular robot products such as Shadow's robots or PAL's TIAGo.

Let me begin with the quick and simple but annoying notes.

• All commits on the PR should compile. Travis gives a weird dependency error with the first one here.
• Please refrain from merging remotes on top of the PR branch. The best practice is to rebase your changes upon the current state of the repository, so that your modifications are added in a clean way rather than being sewn into the history of the repository.

Are you sure that API changes are utmost necessary? I'm not sure how the jade release would tolerate breaking API. Is it still young enough to allow for it, @davetcoleman ?

[mathias-luedtke]

All commits on the PR should compile. Travis gives a weird dependency error with the first one here.

The dependency error results from the broken travis config.

The best practice is to rebase

This should solve the dependency error ;)

[toliver]

@carlosjoserg

• I agree that the name is not the nicest. I like RobotHWGroup, although CombinedRobotHW kind of stresses the fact that it is itself a RobotHW child. Let's see if somebody comes up with an even better option. Otherwise changing to RobotHWGroup sounds good to me.
• Regarding joint_name_filter, this is just a configuration example. The CombinedRobotHW doesn't impose any constraint on the parameters that a particular RobotHW would use for its set up. The only parameter that is necessary is type for obvious reasons.
• About read() and write(), if that is the case maybe I should change the signature of read and write to:

read(ros::Time time, ros::Duration period)
write(ros::Time time, ros::Duration period)

[toliver]

@bmagyar

• I've just checked and the changes actually don't break the API. The constructor without parameters would only be necessary if you want to publish your MyRobotHW class as a plugin by including:

PLUGINLIB_EXPORT_CLASS( my_robot_hw::MyRobotHW, hardware_interface::RobotHW)

in your cpp file. But that is of course acceptable as it is a requirement of pluginlib.

• Thanks for the merging tips. I will rebase this PR (and also Allow the InterfaceManager class to register other InterfaceManagers. #220) to comply with the best practice.
• I can try to write a gazebo example. The problem is that gazebo loads plugins that inherit from gazebo_ros_control::RobotHWSim, so I should maybe write a CombinedRobotHWSim version of CombinedRobotHW in order to test it with gazebo.

[davetcoleman]

should HARDWARE be HW?

[davetcoleman]

The changes to interface_manager.h are very complex, maybe @adolfo-rt can review those?

[toliver]

Most of the changes in interface_manager.h belong to PR #220. It's probably easier to review that one first.

I'm not sure if it's good practice to submit a PR that includes the commits of a previous unmerged PR, but for this PR I needed to build on that.

[toliver]

I have fixed some of the comments and also changed read() and write() to

read(const ros::Time& time, const ros::Duration& period)
write(const ros::Time& time, const ros::Duration& period)

[bmagyar]

Building on a different PR is fine, but risky since that one needs to be cleared first!

[toliver]

Reopen against kinetic-devel as #235

[piyush2307]

Is it possible that we can control different hardware interface with different frequency in control loop.