search

robotology / gazebo-yarp-plugins

Plugins to interface Gazebo with YARP.
33 stars 49 forks source link

Modify the gazebo_yarp_controlboard plugin to ensure that the control law frequency can be set and is it not harded to the physics update step #69

Open arocchi opened 10 years ago

arocchi commented 10 years ago

At the moment, the control is hardcoded to be updated every _T_controller WorldUpdateBegin signals. This means, for .001 simulation step, a 100Hz control loop, and 1Khz control loop for 10Khz simulation (wrt simulated clock, not wall clock). Is this intended? What do you think about it?

EnricoMingo commented 10 years ago

Maybe the problems with the control thread sync (in the walking case) are due to these 100Hz?

MirkoFerrati commented 10 years ago

The T_controller was supposed to be used in position (and velocity and torque?) modes since joint PIDs were giving us problems due to a position step too little to generated a torque, i.e. joints were just vibrating instead of moving. For torque I think it was just a copy&paste error. For velocity we need to check if it works without this trick. But for position I recall lot of problems, you can just comment that if and see what happens.

EnricoMingo commented 10 years ago

With the position direct should work...torque also, for the velocity you should have a minimum admissible velocity that makes you move.

EnricoMingo commented 10 years ago

Actually, for the velocity case, do we use any feedback? Could be that the problem.

arocchi commented 10 years ago

today we did a commit with T=1, everything seems to work nicely. Sync + this working really means we should celebrate and call it a release. May once we fix also the bug on 12.04 can have a 12.04? Waiting for reports of this 790d1e25738abe7287a64e93b4e32b7853e7bc82 working in velocity mode and we can close the issue :)

traversaro commented 3 years ago

@GiulioRomualdi @paolo-viceconte this bug could be causing the strange changes in the position control perfomance when you reduce the timestep: the control board update loop should not be related to the physics update step, but rather be fixed to a specific value (probably 1 Khz can make sense for back compatibility, as 1 ms is the default physics timestep setting).

See also the related comment: https://github.com/robotology/gym-ignition/pull/18#issuecomment-485236545 .

traversaro commented 3 years ago

For reference, the part of the code that may need to be modified is https://github.com/robotology/gazebo-yarp-plugins/blob/master/plugins/controlboard/src/ControlBoardDriver.cpp#L528 . onUpdate runs at every physics update, but to ensure that the control law is always the same regardless of the physics timestep, there should be some logic that checks how much time passed since the last control execution, and only run the controller if the time has passed.

Note that given the peculiar semantics of the Joint::setForce method (see https://github.com/osrf/gazebo/issues/878), we probably need in any case to call setForce at any physics update step, so the best way to proceed is to have a buffer where the joint torque is written with the update frequency of the controller, but it is read and passed to setForce in every call to onUpdate .

traversaro commented 3 years ago

I modified the title of the PR to track the current problem that we would like to fix.

diegoferigo commented 3 years ago

@GiulioRomualdi @paolo-viceconte this bug could be causing the strange changes in the position control perfomance when you reduce the timestep: the control board update loop should not be related to the physics update step, but rather be fixed to a specific value (probably 1 Khz can make sense for back compatibility, as 1 ms is the default physics timestep setting).

See also the related comment: robotology/gym-ignition#18 (comment) .

Also: https://github.com/robotology/gym-ignition/pull/29