ros-industrial / robotiq

Robotiq packages (http://wiki.ros.org/robotiq)
BSD 2-Clause "Simplified" License
232 stars 382 forks source link

Bringing forward old changes, an urdf bug fix. #71

Closed TheDash closed 7 years ago

TheDash commented 8 years ago

-Bringing forward Gazebo plugin work, an articulated, underactuated gripper from Groovy devel

Please see the code notes please. Also, referencing #70

shaun-edwards commented 8 years ago

@TheDash, thanks for this. I'm in favor of merging this, but I'd like to figure out why this was missed (most likely my fault). These same set of changes will likely need to be merged into indigo-devel.

TheDash commented 8 years ago

Maybe it was my fault for pushing to Groovy. Not sure. Anyways, we can merge it into Jade. I'm about to make a PR soon adding the FT300 model to the ft sensor description.

athackst commented 8 years ago

@TheDash would it be possible to remove the whole "side" notion from the gazebo plugin? It looks like it is enforcing some assumptions on the prefix/naming etc that is not necessarily true.

Also, it looks like the input/output messages have been copied and renamed to their own package so now we will have 2 of essentially the same thing. I recommend picking one.

TheDash commented 8 years ago

@athackst Can you comment on the locations in the code you're talking about? I'm not too familiar with this code base as it was just an integration project.

We can definitely use existing messages barring they're internally the same and maintains similar functionality, I'm impartial.

shaun-edwards commented 8 years ago

@TheDash, @athackst, sorry for the delay on my end. I'm going to merge this change into indigo-devel (if needed) as is, since it was present in groovy.

The items that @athackst pointed out above can be fixed in jade-devel if we think they are needed. We don't use gazebo so I'd rather rely on your guys judgement.

wxmerkt commented 7 years ago

Hey, what's the status of this PR and can we help/assist in getting it merged? Thanks, Wolfgang

shaun-edwards commented 7 years ago

@wxmerkt, Thanks for the ping. I merged #71 and will sync the jade branch soon.

shaun-edwards commented 7 years ago

Merged in #71, closing.

wxmerkt commented 7 years ago

Awesome thanks @shaun-edwards :)