Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[gazebo_ros_control] Strange behaviour when adding gazebo_ros_control to a robot #612

Closed
Zabot opened this issue Aug 24, 2017 · 16 comments · Fixed by #688
Closed

[gazebo_ros_control] Strange behaviour when adding gazebo_ros_control to a robot #612

Zabot opened this issue Aug 24, 2017 · 16 comments · Fixed by #688

Comments

@Zabot
Copy link

Zabot commented Aug 24, 2017

I was modeling up an arm, and everything seemed to be working until I added the ros control plugin:

  <gazebo>
    <plugin filename="libgazebo_ros_control.so" name="gazebo_ros_control">
      <robotSimType>gazebo_ros_control/DefaultRobotHWSim</robotSimType>
    </plugin>
  </gazebo>

The only difference between these two arms is that the arm in front has gazebo_ros_control plugin enabled, and the one in back has those lines commented out.

comparison

I've verified the inertial properties of are all set right, tried it with collisions disabled completely and without, its the same both ways. I am still able to control the joints while the arm is falling, and they behave as expected, with normal speed, though sluggish acceleration.

Even more curious, when I manually change any one of the four revolute joints on the gazebo_ros_control arm to a fixed joint it behaves perfectly fine.

I was observing a similar issue with turtlebot_arm that I had originally attributed to inertial properties, but it seems like it may be a deeper seated issue.
turtlebot/turtlebot_arm/issues/22

@Zabot
Copy link
Author

Zabot commented Sep 1, 2017

After further investigation this appears to be related to #479 and specifically this PR to gazebo.

It looks like the function that was being used before to change the rotation of a frame used to preserve the velocity of the frame, and now it zeros the velocity of the frame. As such every time the position of a PositionJointInterface is updated, the velocity of the child frame is zeroed, preventing it from accelerating properly.

@mintar
Copy link

mintar commented Jan 23, 2018

Thanks @Zabot for writing this up so concisely. I just got bitten by this issue myself. I was working on a robot URDF consisting of a diffdrive mobile base with a robot arm on top. This issue caused the robot arm to act like a giant parachute, counteracting gravity, and causing some of the wheels to lose contact with the ground. It took me ages to figure out that an incorrect simulation of the arm was causing my diffdrive controller to fail...

Anyhow, for posterity here are the workarounds mentioned in #479 :

  • Add pid_gains for your Gazebo controllers when using the PositionJointInterface, like this:
gazebo_ros_control:
  pid_gains:
    joint2: {p: 100.0, i: 0.01, d: 10.0}
  • Alternatively, use the VelocityJointInterface or EffortJointInterface instead.

Both successfully work around this issue.

@vincentrou
Copy link

This issue can be closed since there is workaround and it now works with gazebo9.

@Zabot
Copy link
Author

Zabot commented Mar 1, 2018

The PR over on gazebo was merged, but don't we still need to change ros_control_plugin to use the new flag that was added? If I'm not mistaken wasn't it decided that the default behavior would remain unchanged?

@mintar
Copy link

mintar commented Mar 5, 2018

Yes, please don't close this issue. It's still there in ROS Kinetic + Gazebo7, which is what most users use. IMO, if this bug won't get fixed, it would be nice if at least a big fat warning would be printed. Usually this bug is very hard to track down, because it manifests in unintuitive problems like the mobile base isn't moving any more once you add an arm to the robot (because this bug causes the mobile base to lose contact with the ground).

There seems to be a large number of users affected - basically everyone who's using ros_control + Gazebo together. Every week, a couple of new questions pop up on ROS answers.

@mintar
Copy link

mintar commented Mar 5, 2018

The PR over on gazebo was merged, but don't we still need to change ros_control_plugin to use the new flag that was added? If I'm not mistaken wasn't it decided that the default behavior would remain unchanged?

I agree. The PR #2814 just added the option to specify preserveWorldVelocity, but the default is false. The ros_control_plugin still has to set it to true in the call to Joint::setPosition(). Unfortunately, that PR has only been merged into Gazebo9, not Gazebo7.

@mintar
Copy link

mintar commented Mar 6, 2018

In case somebody is interested, I've written a tutorial package that shows how to work around this bug:

https://github.com/mintar/mimic_joint_gazebo_tutorial

@j-rivero
Copy link
Contributor

j-rivero commented Mar 9, 2018

Thanks for all the debugging and information. If we need to backport a change from gazebo9 to previous versions, let me know. If we need to made changes in this repository, would you mind to create a PR with the needed changes?

mintar added a commit to mintar/roboticsgroup_gazebo_plugins that referenced this issue Mar 16, 2018
This issue also affects the mimic joint plugin:

    ros-simulation/gazebo_ros_pkgs#612

The commit here fixes that issue for Gazebo 9. We should change the
GAZEBO_MAJOR_VERSION check to >= 7 if the following PR gets backported
to Gazebo 7 and 8:

    https://bitbucket.org/osrf/gazebo/pull-requests/2814/fix-issue-2111-by-providing-options-to/diff
mintar added a commit to mintar/gazebo_ros_pkgs that referenced this issue Mar 16, 2018
This commit fixes ros-simulation#612, but only for Gazebo9. Fixing it for Gazebo7
(the version used in ROS Kinetic) requires the following PR to be
backported to Gazebo 7 and 8:

https://bitbucket.org/osrf/gazebo/pull-requests/2814/fix-issue-2111-by-providing-options-to/diff

Once that PR has been backported, we can remove the GAZEBO_MAJOR_VERSION
guards from this PR so that the fix is active for the older Gazebo
versions as well.

Tested on Gazebo7 (where it compiles, but doesn't change anything) and
Gazebo9 (where it compiles and fixes the bug). I've tested it using the
instructions I've put into this repo:

https://github.com/mintar/mimic_joint_gazebo_tutorial
@mintar
Copy link

mintar commented Mar 16, 2018

@j-rivero : I've added a pull request (#688) that fixes this issue for Gazebo 9. Could you please backport gazebo PR 2814 to Gazebo 7 and Gazebo 8, so that we can relax the GAZEBO_MAJOR_VERSION guards in #688 to bring this fix to Gazebo 7?

@scpeters
Copy link
Member

I'm going to see if it's possible to backport that bugfix to gazebo7 without breaking API/ABI

I'm testing a branch right now

@scpeters
Copy link
Member

@scpeters
Copy link
Member

I think we had talked about this at the time, but we couldn't figure out a way to backport a fix without breaking API/ABI

tried tweaking the const qualifiers, but that didn't help

Build Status https://build.osrfoundation.org/job/gazebo-abichecker-any_to_any-xenial-amd64/572

https://bitbucket.org/osrf/gazebo/branches/compare/issue_2111_7%0Dgazebo7#diff

scpeters pushed a commit that referenced this issue Mar 17, 2018
* Fix #612 for Gazebo9

This commit fixes #612, but only for Gazebo9. Fixing it for Gazebo7
(the version used in ROS Kinetic) requires the following PR to be
backported to Gazebo 7 and 8:

https://bitbucket.org/osrf/gazebo/pull-requests/2814/fix-issue-2111-by-providing-options-to/diff

Once that PR has been backported, we can remove the GAZEBO_MAJOR_VERSION
guards from this PR so that the fix is active for the older Gazebo
versions as well.

Tested on Gazebo7 (where it compiles, but doesn't change anything) and
Gazebo9 (where it compiles and fixes the bug). I've tested it using the
instructions I've put into this repo:

https://github.com/mintar/mimic_joint_gazebo_tutorial

* remove old ifdef
@mintar
Copy link

mintar commented Mar 19, 2018

@scpeters : Wouldn't something like this ABI compatible?

bool Joint::SetPosition(unsigned int _index, double _position)
{
   return this->SetPosition(_index, _position, false);
}

bool Joint::SetPosition(const unsigned int _index, const double _position,
                        const bool _preserveWorldVelocity)
{
  // ...
}

That way, you wouldn't be removing any symbols, only adding new ones. It would be really nice if we could backport this to Gazebo 7. Otherwise we should add a big fat warning whenever people call SetPosition without preserveWorldVelocity, since it will break their simulation in unexpected and hard to debug ways.

mintar added a commit to mintar/gazebo_ros_pkgs that referenced this issue Mar 20, 2018
mintar added a commit to mintar/gazebo_ros_pkgs that referenced this issue Mar 20, 2018
j-rivero pushed a commit that referenced this issue Apr 5, 2018
This commit fixes #612, but only for Gazebo9. Fixing it for Gazebo7
(the version used in ROS Kinetic) requires the following PR to be
backported to Gazebo 7 and 8:

https://bitbucket.org/osrf/gazebo/pull-requests/2814/fix-issue-2111-by-providing-options-to/diff

Once that PR has been backported, we can remove the GAZEBO_MAJOR_VERSION
guards from this PR so that the fix is active for the older Gazebo
versions as well.

Tested on Gazebo7 (where it compiles, but doesn't change anything) and
Gazebo9 (where it compiles and fixes the bug). I've tested it using the
instructions I've put into this repo:

https://github.com/mintar/mimic_joint_gazebo_tutorial
j-rivero added a commit that referenced this issue Apr 5, 2018
* Fix #612 for Gazebo9

This commit fixes #612, but only for Gazebo9. Fixing it for Gazebo7 (the version used in ROS Kinetic) requires the following PR to be backported to Gazebo 7 and 8:
@UltronDestroyer
Copy link

I'm also having this issue... except the pid_gains is not fixing it =(

Any other suggestions?

@mintar
Copy link

mintar commented Jun 11, 2018

I've added a warning message in #691 that should tell you if you're affected by exactly this issue or not. Unfortunately, that warning is only part of release 2.5.16, and kinetic is still on 2.5.14, so you'll have to build gazebo_ros_pkgs from source to get that warning.

@j-rivero
Copy link
Contributor

I've added a warning message in #691 that should tell you if you're affected by exactly this issue or not. Unfortunately, that warning is only part of release 2.5.16, and kinetic is still on 2.5.14, so you'll have to build gazebo_ros_pkgs from source to get that warning.

Waiting next sync in the shadow fixed repo: http://packages.ros.org/ros-shadow-fixed/ubuntu/pool/main/r/ros-kinetic-gazebo-ros-pkgs/ in the case you want to use .deb packages. Thanks @mintar for the contribution.

Roboterbastler pushed a commit to Roboterbastler/gazebo_ros_pkgs that referenced this issue Jul 6, 2018
* Fix ros-simulation#612 for Gazebo9

This commit fixes ros-simulation#612, but only for Gazebo9. Fixing it for Gazebo7
(the version used in ROS Kinetic) requires the following PR to be
backported to Gazebo 7 and 8:

https://bitbucket.org/osrf/gazebo/pull-requests/2814/fix-issue-2111-by-providing-options-to/diff

Once that PR has been backported, we can remove the GAZEBO_MAJOR_VERSION
guards from this PR so that the fix is active for the older Gazebo
versions as well.

Tested on Gazebo7 (where it compiles, but doesn't change anything) and
Gazebo9 (where it compiles and fixes the bug). I've tested it using the
instructions I've put into this repo:

https://github.com/mintar/mimic_joint_gazebo_tutorial

* remove old ifdef
Roboterbastler pushed a commit to Roboterbastler/gazebo_ros_pkgs that referenced this issue Jul 6, 2018
berktepebag added a commit to berktepebag/innomech_mobile_robotic_arm that referenced this issue Mar 8, 2019
Due to bug in Gazebo v 7.0 mentioned here ros-simulation/gazebo_ros_pkgs#612 changed the velocity type joints with effort joint type.
cohen39 pushed a commit to cohen39/gazebo_ros_pkgs that referenced this issue Nov 15, 2021
* Fix ros-simulation#612 for Gazebo9

This commit fixes ros-simulation#612, but only for Gazebo9. Fixing it for Gazebo7 (the version used in ROS Kinetic) requires the following PR to be backported to Gazebo 7 and 8:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants