-
Notifications
You must be signed in to change notification settings - Fork 48
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
Adjust to Gazebo 8 API and fix gazebo_ros_pkgs#612 #12
Conversation
Note about the DisconnectWorldUpdateBegin: This function was deprecated in favor of resetting the ConnectionPtr, see here: https://bitbucket.org/osrf/gazebo/pull-requests/2329/deprecate-event-disconnect-connectionptr/diff
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
For a description, please see the extended individual commit messages. Tested on Gazebo 7 (the default in Kinetic) and Gazebo9. |
Also see ros-simulation/gazebo_ros_pkgs#688 . |
@mintar thanks for the PR. I will try to give it a look as soon as possible. Thanks for your patience. |
Sure, let me know if I can be of help. |
@@ -22,21 +22,23 @@ OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISE | |||
|
|||
#include <roboticsgroup_gazebo_plugins/mimic_joint_plugin.h> | |||
|
|||
#if GAZEBO_MAJOR_VERSION >= 8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer something like (there's no actual need to use the whole namespaces):
#if GAZEBO_MAJOR_VERSION >= 8
namespace math = ignition::math;
#else
namespace math = gazebo::math;
#endif
Then, you can use math::clamp
in the code..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! Thanks! I will test it in my machine and then accept it! Thanks again!
@mintar thanks for the PR! |
No description provided.