Add optional state publishing to PID controller

[paulbovbel]

Also a minor change to computeCommand calls, to chain the two implementations rather than replicate code.

[adolfo-rt]

Per-changeset comments:

• 6dabd8f: In control_msgs there exists the JointControllerState, which is used in ros_controllers along with the control_toolbox::Pid class (example). If possible, my suggestion would be to leverage this existing message type. Also, if you prefer to incorporate the PID state publishing inside the control_toolbox::Pid class, it would be desirable to also propagate this change to the few other controllers that use it.
• d68c920: Great to reduce code duplication!. I would suggest to make this a separate PR, as it's orthogonal to the above changeset.
• 31dd5e5: What's wrong with the newline at the end of the file?. This is a good UNIX practice.

[paulbovbel]

6dabd8f: Thanks for pointing out that message, didn't spot it before. I'm a little skeptical of using it, since the PID library wouldn't always be used to control a joint, so the message name would be a little deceptive. It seems that in ROS the pattern is to create per-application message types for clarity. Additionally, from inside the controller I don't have access to the setpoint (only the error), so I can't fill that field.

I could move the PID message to control_msgs if you'd be amenable to that?

[paulbovbel]

Also, the JointStateController message doesn't include the three terms (p_term, i_term, d_term), which are the helpful bits for tuning a controller.

[adolfo-rt]

I get that the JointControllerState name might be misleading, if you're using a PID to control a non-joint plant. I find it sensible to have a more generally named PidState message. There is in fact nothing joint-specific in the JointControllerState spec, and it kind of makes the assumptions that joint controllers are PID-based, which in general is not true.

I find the message type you propose less informative than JointControllerState. I get that the setpoint is not available from within the PID class, but if we're to propose a new message type, I'd really want it to convey as much useful information as possible.

Also, it would be best if new messages made it to control_msgs, and not to control_toolbox. The former has much lighter dependencies, and are all message related; while the latter brings more (and non-message related) dependencies.

[paulbovbel]

Sounds good on all points. Any proposals for what additional information you'd like to see in the PidState message?

[paulbovbel]

Moved message to control_msgs, depends on ros-controls/control_msgs#14

[paulbovbel]

Depends on #44 or #42

[paulbovbel]

Updated with ros-controls/control_msgs#14, other dependencies met, ping for review.

[adolfo-rt]

Please rebase now that #46 has been merged, and we preserved the implementation in computeCommand.

Also, please make two commits: One with the style change, and another one with the enhancement.

[paulbovbel]

Ping for review. No style change here.

[adolfo-rt]

Hmmm, I realize just now that under certain RTOSes, like Xenomai 2.6.X, ros::Time::now is a no-no, as there's a Linux system call involved.

Since state publishing is optional and comes disabled by default, I think your proposal is an acceptable compromise.

Could you please document the publish_state parameter here, and mention that if enabled, it may break real-time guarantees on certain RTOS?. I think that with this, the PR can be merged.

[paulbovbel]

@adolfo-rt, added the documentation, thanks

[bmagyar]

Could you update this please?

[paulbovbel]

Rebased on kinetic-devel

[bmagyar]

Unfortunately you have to open a new PR pointing to kinetic-devel, github is rigid on the "from-to" relations of PRs, cannot edit this one...