Port velocity_controllers/joint_position_controller from ROS1

[tonynajjar]

I know you probably have it on your roadmap somehwere but I didn't find it in the issues (found similar) so I'm writing this feature request to formalize it and track it

I might take this on in the near future.

[bmagyar]

Volunteer accepted! :D

[destogl]

Wait! We maybe don't need that! :)

This is basically a PID controller. The question is does this separate name adds any value to it. Or should we stick with more basic names and parameterize controller accordingly.

Copy from the meeting notes

This controller is actually a PID controller – in ROS 1 we had to have those because of templated interfaces Now we could do relatively simple specialization of PID controller - #434

Parameters set:

position_reference_velocity_commands_controller:
  ros__parameters:
    dof_names:
      - joint1
      - joint2

    command_interface: velocity

    reference_and_state_interfaces:
      - position

    gains:
      joint1: {p: 10.0, i: 1.0, d: 0.1}
      joint2: {p: 10.0, i: 1.0, d: 0.1}

[tonynajjar]

Thanks for the heads up, how far away is the PR from being testable? I could help test it

[destogl]

The PR is testable and workable (almost in production already :D) But I didn't have a chance to finish up the tests.

[muttistefano]

Dear @destogl , i'm trying to test and use your PR. In my case, I have a joint position controlled hardware interface for a robot, and I want to use the PID controller to feed positions to the hardware interface while receiving velocity commands. Up to now my config looks like :

controller_manager:
  ros__parameters:
    update_rate: 125  # Hz

    pid:
      type: "pid_controller/PidController"

    forward_velocity_controller_pid:
      type: forward_command_controller/ForwardCommandController

pid:
  ros__parameters:
    dof_names:
      - shoulder_pan_joint
      - shoulder_lift_joint
      - elbow_joint
      - wrist_1_joint
      - wrist_2_joint
      - wrist_3_joint

    command_interface: position

    reference_and_state_interfaces:
      # - position
      - velocity

    update_rate: 125

    gains:
      shoulder_pan_joint:  {"p": 1.0, "i": 1.0, "d": 0.0, "i_clamp_min": -7.0, "i_clamp_max": 7.0, "antiwindup": true}
      shoulder_lift_joint: {"p": 1.0, "i": 1.0, "d": 0.0, "i_clamp_min": -7.0, "i_clamp_max": 7.0, "antiwindup": true}
      elbow_joint:         {"p": 1.0, "i": 1.0, "d": 0.0, "i_clamp_min": -7.0, "i_clamp_max": 7.0, "antiwindup": true}
      wrist_1_joint:       {"p": 1.0, "i": 1.0, "d": 0.0, "i_clamp_min": -7.0, "i_clamp_max": 7.0, "antiwindup": true}
      wrist_2_joint:       {"p": 1.0, "i": 1.0, "d": 0.0, "i_clamp_min": -7.0, "i_clamp_max": 7.0, "antiwindup": true}
      wrist_3_joint:       {"p": 1.0, "i": 1.0, "d": 0.0, "i_clamp_min": -7.0, "i_clamp_max": 7.0, "antiwindup": true}

# preceeding_forward_velocity_controller_pid:
#   ros__parameters:
#     joints:
#       - pid/shoulder_pan_joint
#       - pid/shoulder_lift_joint
#       - pid/elbow_joint
#       - pid/wrist_1_joint
#       - pid/wrist_2_joint
#       - pid/wrist_3_joint
#     interface_name: velocity


forward_velocity_controller_pid:
  ros__parameters:
    joints:
      - pid/shoulder_pan_joint
      - pid/shoulder_lift_joint
      - pid/elbow_joint
      - pid/wrist_1_joint
      - pid/wrist_2_joint
      - pid/wrist_3_joint
    interface_name: velocity

Apart from the PID values that have to be tuned, it seems like the controller still receives position reference rather than velocity. Am I doing something wrong?

This configuration is a modified version of the one provided in your pull request. I had to comment out the preceding controller otherwise it wont work.

Thanks