Fixed PID destructor bug, cleaned up code #29
Conversation
|
|
||
| // Get URDF info about joint | ||
| urdf::Model urdf; | ||
| if (!urdf.initParam("robot_description")) |
There was a problem hiding this comment.
Why is it now required to have an URDF to use this controller? I also don't like the use of urdf::Model::initParam because you don't have a lot of control over where the parameter is coming from.
There was a problem hiding this comment.
It already required that... previously this
init(hardware_interface::EffortJointInterface *robot, ros::NodeHandle &n)
function called the first (now removed)
init(hardware_interface::EffortJointInterface *robot, const std::string &joint_name, const control_toolbox::Pid &pid)
function, which then required a URDF be used.
I agree it would be nice to have a little more control over the param location, although you have some based on your NodeHandle's namespace. But this is a ros_controller, so I don't think we should spend too much time removing URDF dependencies.
There was a problem hiding this comment.
That's really strange, I could have sworn I've used it without a node-scoped "robot_description" parameter. No worries, then.
Reviewed by @jbohren Fixed PID destructor bug, cleaned up code
No push_back in Real Time
…esired_param Add control_frequency_desired param
Fixed issue where the control_toolbox::Pid class was destructed during initialization here:
Also lots of documentation and cleanup.
This PR is required for ros-controls/control_toolbox#1 to work properly