Closed jproberge closed 5 years ago
jproberge
commented
5 years ago I'm not so sure about who could review this PR, but it would be great to eventually merge this (or a reviewed variant of this PR) in the not so far future :) Maybe @shaun-edwards or @AustinDeric could help? Any reviewer who wants to help out here is welcomed :) Thanks!
gavanderhoorn
commented
5 years ago @ all robotiq users that watch this repository: @jproberge has put in quite some effort to get this kinetic-devel candidate in place.
I would personally appreciate it very much if any of you would be willing to test out his proposed changes and let us know your experiences.
With the refactor of these packages quite a few things are changing, and before merging something like this it would be good to have a few reviewers from the community confirm that everything is ok and in working order.
gavanderhoorn
commented
5 years ago @jproberge: nice work with this PR.
Unfortunately I don't have any hw to test this against, so I can't be of any more help.
It would be nice if changes such as introduced in 2f12788 could be discussed first btw. Perhaps something for the future.
Again :100: for the PR.
jproberge
commented
5 years ago Hello all,
Three of my colleagues, me and even some of my students tested this PR on different grippers / FT sensors and they all reported that it worked right (and at least as well as the previous Jade version, which was also being used for Kinetic by many). It has been tested on all current Robotiq products. The fact that it is working properly is also not surprising since, as already pointed out by @etienne-roberge , this PR contains mostly cosmetic changes (mostly package name changes, as requested by Robotiq). Of course, it also passes all Travis-ci tests.
Now, a month and a half has already passed since this PR was first created. Only two reviewers commented and approved the PR, despite the "call to all" made by @gavanderhoorn a month ago. It is unfortunately a bottleneck to further improving the Kinetic branch of the Robotiq repo, since it was intended to be the first viable version for this branch. I'd like to start generating great and reliable documentation for this branch and also to address any potential issues that will arise in the future. I'd like to be able to maintain this branch properly, since a lot of users are using ros-kinetic, and I want to guarantee that they will have a high-quality repo at their disposal.
Since a lot of time has passed already, since two reviewers already approved the modifications (+many others have reported that it works properly), and since nobody will benefit in letting the current situation continue to last for a long time, I've decided to now merge this PR to the base branch. I hope that you'll all agree with this decision, as offering the best quality packages and improving Robotiq's Repo are really the only criteria on which I based that decision :).
Thanks to all!
wxmerkt
commented
5 years ago Hi, thank you very much for the update. Can we make the kinetic-devel branch the main branch of this repository? (Currently jade-devel).
jproberge
commented
5 years ago Hi @wxmerkt ! Thanks for your comment and I also think this should become the main default branch. However, since I'm only a member, I cannot make this happen.
@gavanderhoorn : If I'm not mistaken, as an owner you have the permission to change the default branch, right? Assuming that is the case, what are your thoughts about that possibility?
gavanderhoorn
commented
5 years ago @jproberge: I've made you a member of the admin team here, so you should be able to manage branches yourself now.
Remember: with great power .. ;)
I know this is a rather big PR, but this is because it's been a long time since Robotiq's repo was last significantly updated. This is a summary of what this Pull Request contains:
1) The .travis.yml file has been updated for ros-kinetic (reminder: in ros-kinetic, there's a debian package that solves the soem depency problem (i.e.: the ros-kinetic-soem package)
2) Robotiq asked me to update their package names and files so that they reflect their real product names. Consequently, all C++ / Python identifiers have been updated to match Robotiq's real product names. This is where the largest number of changes come from.
3) Minor package.xml file modifications to update the maintainer name and email address
4) Finally: I'v added the "robotiq_gripper_upload.launch" file to address issue #99 .
This is meant to superseed PR #101 , and to create the first viable kinetic release for the Robotiq repo. This has been successfully tested on hardware and works fine.