search

PR2 / pr2_common

46 stars 78 forks source link

Revert #255 #274

Closed knorth55 closed 5 years ago

knorth55 commented 5 years ago

related to https://github.com/PR2/pr2_common/issues/245 and https://github.com/PR2/pr2_common/pull/255

In #245 and #255, they try to work PR2 on Gazebo in Indigo version and make this change. However, this modification doesn't make sense, and it doesn't work on Kinetic and Melodic, which uses higher version Gazebo. I revert the PR and I checked it's working on Kinetic Gazebo.

knorth55 commented 5 years ago

I update to revert https://github.com/PR2/pr2_common/commit/d5c8f476c6882b74a03bdc39eccf36823da3ad97 and it works in Kinetic ! pr2-kinetic-gripper-gazebo

knorth55 commented 5 years ago

I add detailed explanation in this PR.

knorth55 commented 5 years ago

@k-okada can you review this again?

v4hn commented 5 years ago

Hi @knorth55

However, this modification doesn't make sense, and it doesn't work on Kinetic and Melodic, which uses higher version Gazebo.

Please give more details in pull-requests. "doesn't make sense".. Why? "doesn't work on kinetic and melodic".. What do you observe there?

Any idea why this factor of 10 seemed necessary in indigo and "doesn't work" in kinetic?

@k-okada given that both the original and the new patch come from your lab, I will leave this to you to resolve.

knorth55 commented 5 years ago

"doesn't make sense".. Why?

Originally, this parameter is set as 3141.6, which comes from PR2 hardware design. Willow garage set this parameter at first (we can see it from git log), but some people change the parameter to 314.16 in https://github.com/PR2/pr2_common/pull/255. They change this parameter from 3141.6 -> 314.16 to run PR2 gazebo indigo, but this parameter is defined from PR2 hardware, so 314.16 doesn't make sense. However, PR2 gazebo in Indigo does not work (it is reported in https://github.com/PR2/pr2_common/issues/245) with this original correct parameter 3141.6, and they solved the problem by changing this parameter to 314.16. This is my guess, but this problem is actually a bug in Gazebo, and it looks running correctly on Gazebo with the wrong parameter 314.16 somehow.

"doesn't work on kinetic and melodic".. What do you observe there?

However, when I tried to close PR2 gripper on PR2 Gazebo Kinetic, the gripper does not close. We read git log, and this is caused the parameter difference between https://github.com/PR2/pr2_common/blob/7a59197e881428a2748066f5d458449055a2664e/pr2_description/urdf/gripper_v0/gripper.transmission.xacro#L41 and https://github.com/PR2/pr2_common/blob/6f96993793230c81737478729c4e324c3574424a/pr2_description/urdf/gripper_v0/gripper.gazebo.xacro#L170 . So I change the parameter back to correct one 314.16 -> 3141.6, and now PR2 gazebo on kinetic works.

knorth55 commented 5 years ago

@v4hn I updated the comment to add more detailed information. My point is that

knorth55 commented 5 years ago

@v4hn kindly ping.