search

PR2 / pr2_controllers

The controllers that run in realtime on the PR2 and supporting packages.
46 stars 34 forks source link

Publish PID integrator state for joint velocity and position controllers #375

Closed goretkin closed 2 years ago

goretkin commented 9 years ago

the value of the integrator is an important part of the state of a PID controller. I only tested this on groovy, but I don't know whether it's appropriate to add any new features to groovy at this point. Either way, this commit should merge cleanly with Hydro too.

This was useful is discovering https://github.com/PR2/pr2_controllers/issues/374

TheDash commented 9 years ago

Sorry I'm not very familiar with controls. What would change visually on the robot if this is implemented?

Also, the .msg file will have a different checksum which will cause big errors for users of this node. (Causing it to be non-backward compatible)

goretkin commented 9 years ago

This change, apart from breaking the "binary"-ish compatibility you mentioned, doesn't affect any behavior on the robot.

The joint/velocity controllers use the Pid class in the control_toolbox package, which is a PID controller. One of the things in the Pid class is a field which accumulates (integrates) the error, which is an important quantity for the PID controller. And to tune or debug a PID controller, it's nice to have access to its full state, including the value of the integrator.

As far as I know, the purpose of the JointControllerState is for debugging. This PR just makes available some extra debugging information. It is unnatural that the JointControllerState publishes every other single piece of state of the PID controller (all the gains, the derivative) but not the integral.

I don't know enough to know how bad it is to change .msg files the change-the-hash way. It's not the most backwards incompatible since no fields were removed or renamed. But it does require all distributions of ROS to have the same message file.

I just did a search and found some code using it for what appears to be something other than debugging http://ua-ros-pkg.googlecode.com/svn%27:%27ua-ros-pkg%27,-/trunk/arrg/ua_controllers/wubble_actions/nodes/smart_arm_action.py

https://gitlab.mech.kuleuven.be/rob-itasc/itasc_robots_objects/raw/e3b3824fb2109db8247d2243066bf05e7c03a456/itasc_pr2/src/pr2connect.hpp

v4hn commented 2 years ago

replaced by #405

Thank you for working on the controllers 7 years ago @goretkin :dog2:

goretkin commented 2 years ago

Thank you for applying these changes! The closure will help me sleep better at night :-P.

On Wed, Aug 10, 2022, 11:04 AM Michael Görner @.***> wrote:

replaced by #405 https://github.com/PR2/pr2_controllers/pull/405

Thank you for working on the controllers 7 years ago @goretkin https://github.com/goretkin 🐕

— Reply to this email directly, view it on GitHub https://github.com/PR2/pr2_controllers/pull/375#issuecomment-1210804806, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEN3M76O3N3BFFJGU6MY2DVYPAH5ANCNFSM4A57BYBA . You are receiving this because you were mentioned.Message ID: @.***>