[JTC] Implement effort-only command interface

[Ace314159]

This addresses the effort part of #135 and #171. I've tested this on our team's robot and it works identically to the ROS1 version from our testing.

I just realized that the trajectory tolerance parameters fix (the second commit) is also addressed by #224. I can remove the commit if you'd like to merge that first instead.

[Ace314159]

The test failures seem to be in force_torque_sensor_broadcaster, but I didn't modify that part of the code. Any ideas for what might be happening?

[destogl]

The test failures seem to be in force_torque_sensor_broadcaster, but I didn't modify that part of the code. Any ideas for what might be happening?

Don't worry about that. The tests are flaky.

Could you rebase the code since we changed the formatting?

[Ace314159]

@destogl Does it look good now?

[bmagyar]

The changes look good in general, thanks for keeping it up to date!
Do you think this should be called effort though? I think that the values you are using are basically acceleration or strongly derived from it. Does it relate to forces in any way? Effort in ros_control was used somewhat as a catch-all for "the third value" next to Position and Velocity

[Ace314159]

@bmagyar I'm not too sure either, but I just tried to mimic the behavior of ros_control.

However, I don't think this is acceleration. My team is using the effort value as the power output for the motor, so perhaps a better name might be power?

[Ace314159]

@livanov93 Does this look good?

Do you know if the test failures are just caused by flaky tests or if my changes caused it? There seem to be test failures on master as well.

[livanov93]

@livanov93 Does this look good?

Do you know if the test failures are just caused by flaky tests or if my changes caused it? There seem to be test failures on master as well.

I think it is flaky test. The rest looks fine.

[destogl]

However, I don't think this is acceleration. My team is using the effort value as the power output for the motor, so perhaps a better name might be power?

My two cents about this. I think that you are actually using acceleration, which results in moment/power of the motor. The only thing what is intransparently happening in PID-part is conversion between acceleration and power by gain of the PID.

IMHO, the proper way for this would be to use acceleration interface and convert in robot driver acceleration to power. then if you are using other controllers you don't have to adjust gain of it for your specific robot.

(I am delighted if someone gives me a reason that this "proper" was is not so "proper") :)

[Ace314159]

@destogl Perhaps power is not the right term. In the motor code, the type is called percent output. The difference between what we supply to the motor and acceleration is that when we supply a value of 0, the motor completely stops, but an acceleration of 0 would still preserve the velocity. However, I don't think this is velocity either since the value we output doesn't map linearly to position

How would you convert acceleration to power? I was able to find some formulas online, but they required knowing the mass and a time for the system.

Please correct me if I am mistaken. It's been a while since I've taken physics.

[destogl]

@Ace314159 what I can remember circular acceleration of a motor is proportional to its moment. But this discussion is probably not sensible because what you are saying you have some kind of abstract value. So we can go with the term "EFFORT" in this case. What do you think?

P.S. For personal interest, I am wondering how are you tuning your PID then if you don't have any idea about the physics of the value the motor is accepting.

[Ace314159]

@destogl Yeah, I think that sounds good.

We don't really know the physics of what the motor is accepting, but we are assuming/hoping it is consistent (which is probably not true). We can read values like the position and velocity of the motor, and we know that a positive value turns the motor in direction and a negative value turns it in the other direction. Then, we just tuned our PID following the guidelines we found from StackExchange. I'm not sure if a PID is the best way to do what we want, but from our cursory testing it seems to work well enough. Does this answer your question?

[Ace314159]

@destogl Is there anything else you'd like me to change to get this merged?

[bmagyar]

The PR is good to go!

Two follow-up issues I created for refactoring:
#325
#326

[Ace314159]

@bmagyar Would it be possible to backport this to foxy? Someone else also requested this and this would be useful for my team as well.

[theunkn0wn1]

Yeah, a foxy backport would be extremely useful for my team.

[destogl]

Sorry guys but that is not possible because of too many changes in the mean time. you can try to resolve conflicting PRs and then we are happy to merge. We don't have resources to do this.

[GeorgeVe]

Hee, I quickly created a WIP backport to foxy for the joint_trajectory_controller with an effort-only command interface. It works for my team in simulation with Gazebo. It is on our forked GitLab repository: (https://gitlab.com/project-march/libraries/control/ros2_controllers/-/tree/mergify/bp/foxy/pr-225/joint_trajectory_controller). Feel free to check it out, but I can't guarantee that it is 100% correct and finished. For example, the feature of 'allow_integration_in_goal_trajectories_' is also added but only partially implemented.

[rohitmenon86]

Hi, I have created a backport for galactic on my fork here https://github.com/rohitmenon86/ros2_controllers.git for the joint_trajectory_controller ONLY with an effort-only command interface. I am still testing it

[bmagyar]

PRs are welcome