ros-controls / ros_controllers

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

Why don't position_controllers publish the controller state like effort_controllers do? #606

Open joaocandre opened 2 years ago

joaocandre commented 2 years ago

I was forced to migrate from effort_controllers/JointPositionController to position_controllers/JointPositionController and I noticed I no longer get topics publishing the controller state for each controller joint. Is this by design (is there any particular reason for it?) or there was just never the need to implement it?

bmagyar commented 2 years ago

Just a missing implementation AFAIK. Happy to review a PR ;)

On Sat, 27 Aug 2022, 23:34 João André, @.***> wrote:

I was forced to migrate from effort_controllers/JointPositionController to position_controllers/JointPositionController and I noticed I no longer get topics publishing the controller state for each controller joint. Is this by design (is there any particular reason for it?) or there was just never the need to implement it?

— Reply to this email directly, view it on GitHub https://github.com/ros-controls/ros_controllers/issues/606, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA24PYMP6L2NGVVE4O5AKOTV3KJW7ANCNFSM572AMPKQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

wxmerkt commented 2 years ago

I believe it's because it's a forward_controller rather than a PID controller. (I've implemented the controller state publishing for velocity_controllers/JointPositionController but it may not make sense for position_controller/JointPositionController - could PR that)

joaocandre commented 2 years ago

I assume internally there is some kind of PID loop though, considering the PID gains can be tuned through parameters in the controller namespace? In truth when setting publish_state parameter to true the state is published, but not as a control_msgs::JointController message.

wxmerkt commented 2 years ago

If we are speaking of a position_controller/JointPositionController, I don't think there are gains - it's a pure forward controller (cf. https://github.com/ros-controls/ros_controllers/blob/252d6584b17ae88e614a6c9cd30ba76ef258a193/position_controllers/src/joint_position_controller.cpp#L43-L49). Can you point me to the config where you are seeing the parameters/topics?

joaocandre commented 2 years ago

I haven't tried to tune these "PID" parameters to check whether they do anything at all, but I've seen some models/packages providing those parameters, and ROS complains about those missing parameters if not provided under gazebo_ros_control/pid_gains/[controller] (as error messages) .

For instance, we can see that in configuration files for simulating of a NAO humanoid with position controllers, whose hardware interface is defined in the URDF model as PositionJointInterface (albeit this is rather old repository, probably not actively maintained).

I've also seen it in some URDF tutorials that define PID gains for position-controlled joints. Again not sure if they are supposed to do anything because I've never tried to edit these values.

wxmerkt commented 2 years ago

Thank you for providing the references and links. The links above refer to an issue present in older versions of Gazebo (7, available with ROS Kinetic) for which a workaround was to set the PID gains for the PositionJointInterface. The position_controllers in ros_controllers do not use these gains - as thus, there wouldn't be modifications that could be made to publish the controller state

lprobsth commented 1 year ago

I haven't tried to tune these "PID" parameters to check whether they do anything at all, but I've seen some models/packages providing those parameters, and ROS complains about those missing parameters if not provided under gazebo_ros_control/pid_gains/[controller] (as error messages) .

For instance, we can see that in configuration files for simulating of a NAO humanoid with position controllers, whose hardware interface is defined in the URDF model as PositionJointInterface (albeit this is rather old repository, probably not actively maintained).

I've also seen it in some URDF tutorials that define PID gains for position-controlled joints. Again not sure if they are supposed to do anything because I've never tried to edit these values.

The PID gains for gazebo_ros_control are for setting either forces or velocities and positions in the simulation directly. If you omit the PID gains for gazebo_ros_control, the control_toolbox's PID controller throws an error during initialization (https://github.com/ros-simulation/gazebo_ros_pkgs/blob/noetic-devel/gazebo_ros_control/src/default_robot_hw_sim.cpp#L222). If the PID controller could be initialized, the hardware interface sets effort commands (https://github.com/ros-simulation/gazebo_ros_pkgs/blob/noetic-devel/gazebo_ros_control/src/default_robot_hw_sim.cpp#L351 and https://github.com/ros-simulation/gazebo_ros_pkgs/blob/noetic-devel/gazebo_ros_control/src/default_robot_hw_sim.cpp#L379). If the PID controller could not be initialized, the commanded position or velocity are set directly in the simulation, which sometimes results in weird behavior for mobile robots (sliding, wobbling).

(i think that multiple people are finding this issue when attempting to tune the gazebo controller from gazebo_ros_control - so I'm writing this here) Since the forward controllers in ros_control don't publish the state of the controller, the PID controllers from gazebo_ros_control are hard to tune (people migrating from other controllers from ros_control probably look first for the state topic). The other controllers (e.g. trajectory + velocity) publish the state including the set forces from gazebo correctly. Having the forward controllers publish the full state through a topic would be nice for both real and simulated robots (e.g. our rover also sends back efforts when controlling with velocities from a forward controller). But for gazebo the best approach is probably publishing the state directly in gazebo_ros_control since then we see the control error for the PID controller.

Edit

I just realized there is a parameter for the PID controller that is used in ros_gazebo_control publish_state (https://github.com/ros-controls/control_toolbox/blob/melodic-devel/src/pid.cpp#L141), that can be used to obtain the internal state of the controller. This includes the error and the individual P, I and D parts of the controller.

But still - having a publisher for the feed forward controllers from ROS control might be good for real robots that use external motor controllers that itself publish their internal controller states.