ros-controls / ros_controllers

Generic robotic controllers to accompany ros_control
http://wiki.ros.org/ros_control
BSD 3-Clause "New" or "Revised" License
560 stars 526 forks source link

Adding an actionlib interface for Joint<Position/Velocity/Effort>Controllers #568

Open egordon opened 3 years ago

egordon commented 3 years ago

Hello there! Unsure if this is more a question or an enhancement request.

The JointTrajectoryController uses a control_msgs/FollowJointTrajectory action interface. And this is great.

However, all of the other basic forward-command controller types just subscribe to a simple topic with an array of float (sometimes with a separate publisher with the controller state if it involves a PID loop). In general, there is no way to provide feedback about the controller to the client. This seems to me like a significant reduction in flexibility.

Some features missing from the non-trajectory controller:

  1. Optional command durations or timeouts (e.g. set velocity 1 for 1s, only allow 5s to achieve the desired position)
  2. Notification when another client sends a command (via goal cancelling)
  3. Error / warning messages for malformed commands (e.g. outside limits), plus the ability to return an error code / status message for other controller errors (e.g. singularities)
  4. Goal-tied regular feedback (perhaps redundant with the joint state controller, but still useful)

I'm not saying that the generic controllers here have to implement all of these, but right now, if I build a custom controller with these features, then my client will not be compatible with the basic controllers, and I will probably have to re-write a lot of code I otherwise wouldn't have to.

See #549 , where a user feels the need to wrap the basic controller in an actionlib interface.

TL;DR: Basically, I guess I'm asking about having a more flexible interface for the basic forward command controllers that allow for more feedback to the client :sweat_smile: . If this is a useful enhancement, it is something I'm happy to put cycles into.

bmagyar commented 3 years ago

This sounds like a change that would be very welcome. What we could discuss is whether it's preferred to add this to the current ones on top of the topic interface, and have a similar setup as joint trajectory controller or an implementation where you create an alternative set of controllers instead with only the action interface.

egordon commented 3 years ago

I'm partial to the first approach to keep the interfaces consistent (and potentially

One question is whether we can co-opt existing control_msgs.

For example, JointJog is close and could work as the goal message type, but it doesn't provide Effort support. Do we want to potentially add effort support there, or just use a similar structure of base types?

egordon commented 3 years ago

Drafts up, they build. Need to test them on our end (and ideally build unit tests if those exist in this repo).

What are the policies here for PR reviews that I should adhere to?

egordon commented 3 years ago

@bmagyar The changes were implemented on tested on our lab's Kinova JACO2. Ready for review, I'm prepared to make any necessary modifications to adhere to any review policies.

Thank you!