Position controller implementation seems odd

[ijnek]

Thank you to the developers/maintainers of this library, and I appreciate the good foundation to build upon.

In #29, the position control was modified from using a JointPositionReset to using a JointVelocityCmd, with the description:

Position control was not working from Citadel, instead of using JointPositionReset I used a velocity controller.

I'm wondering what the "was not working" part means, because reverting the changes, I see the joint positions updating more in a way I expect. Does the "not working" refer to physics not taking place becauser the JointPositions "jump" when resetting?

The current implementation seems like a simple proportional position controller, where:

velocity = error * dt where

• error = expected - current
• dt = update rate

but the PID-behaviours of the motor are hardcoded and cannot be tuned to reflect what the actual motor would act like.

I've got three alternative suggestions:

1. Implement a PID controller that sends a JointForceCmd. We could perhaps use ignition/gazebo/systems/JointPositionController to do this. This would more accurately reflect the motors movement and all the physics, but users would have to know the specifics of the underlying PID values used in the actuator.
2. Revert to use JointPositionReset - JointPosition controllers are intended to be used when the physical actuator (eg. motor) can deal with joint position commands. This is useful if the specific controllers in the joints are unknown, but it is still possible to move the joints of the robot to where you're asking it to move to.
3. Ditch implementation of controller inside this package. Let the users add ignition::gazebo::systems::JointPositionController and ignition::gazebo::systems::JointController to their sdf/urdf files, and simply publish messages for those plugins to pick up and act on appropriately.

My preference is 3.

@ahcorde What are your thoughts?

[destogl]

My preference is 3.

If you got this way, why do you need ros2_control here at all? You can then simply use state output from a controller and publish it directly into the controllers. What I understand is you propose to not use this plugin at all, is this correct?

The second question would be, how do those controllers work internally? Isn't it possible to do the same here?

[ijnek]

@destogl Before going into discussing alternatives, are you in agreeance that the implementation is a simple proportional position controller (where P gain = dt) which seems... odd? (or at least isn't what people would expect)

What I understand is you propose to not use this plugin at all, is this correct?

I think I am slowly reaching that conclusion. I'm still in the process of learning ros2 control, and the reason I'm using this package is because it sounds like an obvious choice when working with ignnition gazebo and ros2 control. If this package turns out to be the wrong tool for what I'm trying to achieve I'll decide not to use it, but I haven't reached that conclusion yet.

In Ignition Gazebo, the strongly encouraged way of writing plugins is to write an ignition plugin that listens to ignition topics, have a ros node that publishes ros messages, and have a ros_ign_bridge that converts the messages between the two. The main reason this is encouraged instead of writing a ros-specific ignition plugin directly, was that a lot of code otherwise gets duplicated, for general ignition plugins and ros-specific ignition plugins. From what I can tell, this package doesn't follow that recommendation. I'm not saying the packages here are bad, I just want to know if I'm getting this correct or not, so that I can look into designing something that follows the recommended way, by using the ignition controllers (where the PID values are configurable).

[ijnek]

I've gone ahead and created a ros2 controller that interfaces gazebo, and a demo package with an example robot that uses it:

• Hardware Interface for Ignition Gazebo and ROS2 Control
• Demo Package

[AndyZe]

I've spent some time debugging an issue with this plugin where the joints which are not actuated tend to droop with gravity over time. In other words, I'll command a new position for the wrist joint, but the elbow joint also slowly sags with gravity.

I think the problem is, every joint is updated, all the time. Even if the new command does not actuate that joint.

[ijnek]

debugging an issue with this plugin where the joints which are not actuated tend to droop with gravity over time.

I'd expect this to happen (and actually saw it happen) with the current implementation (unless it's been changed), if the arm has non-negligible mass.