Should this package be moved to ros-drivers?

[geoffviola]

This repository seems unmaintained. I'm using it right now. Unless one of the authors wants to maintain it, I can do a little maintenance and maybe set it up on bloom.

[muhrix]

Hi @geoffviola. I am (supposed to be) one of the maintainers of this repo, and I apologise for not being able to more actively maintain it.

I was only given write access to this repo; I cannot change any admin-related settings.

This repo is already set up on bloom and the package was released for hydro and indigo. Please see https://github.com/muhrix/phidgets_drivers-release.

If there is a possibility for us to migrate this package to ros-drivers, I would say it's a good idea (I am happy to be one of the maintainers there too -- that would motivate me to find my phidgets IMU and install post-indigo ROS distros as well).

Lastly, whether we migrate this repo to ros-drivers or keep it here, we should pick up from where I left off here: ipa320/cob_extern#44.

[mintar]

Yep, to actually move this repo to ros-drivers either @muhrix or me (or better still both) would need "admin" permissions on ccny-ros-pkg/phidgets_drivers . Since @idryanov has gone AWOL, and he's the only one that could give us those permissions, we're out of luck. We could of course deprecate this repo and fork it into ros-drivers, but that would be not as nice as actually moving it. A move is completely transparent to the user (they will be HTTP-forwarded to the new location if they didn't update their remote uris).

[muhrix]

We could move the phidgets_drivers-release repository to ros-drivers to avoid myself being such a bottleneck.

Regarding phidgets_drivers, ideally we should move the entire repo, but in the worst case scenario I would suggest we leave hydro and indigo here, and move newer distros development to another repo where we can have admin privileges so we can add/remove maintainers (avoiding situations like this).

[mintar]

Yeah, but that would be a worst case scenario. Let's wait first, perhaps @idryanov pops up again.

[geoffviola]

Cool. I wasn't sure if this package was maintained.

I was mostly hoping to get out of it kinetic support and changes from the high speed wheel encoders merged together. The big problem that I'm facing with kinetic support is that the phidgets drivers are not in the package manager in Ubuntu 16.04.

[mintar]

@muhrix : If you could fix ipa320/cob_extern#44 , that would be great. I think what needs to be done is:

• create a branch jade
• in that branch, copy cob_extern/libphidgets to libphidget21
• search & replace libphidgets => libphidget21
• release it
• remove libphidgets from cob_extern when they create a jade branch

I can't do step 1 because I don't have write access here. It would be nice if you could do step 2 & 3 as a PR so we can test it before you merge & release. If necessary, I can do that after you create the jade branch.

[muhrix]

@mintar could you please test branch jade? Please see the updated checklist below.

• branch jade created
• libphidgets moved from cob_extern into phidgets_drivers and renamed to libphidget21
• release phidgets_drivers (indigo onwards)
• remove libphidgets from cob_extern

The issue with releasing for indigo is that phidget21 library would be overwritten if cob_extern is also installed. That said, libphidgets and libphidget21 are the same, and they would both install the same version of phidget21. Everything should continue to work.

If cob_extern could drop libphidgets once phidgets_drivers is released for indigo, then we'd have no issues with phidget21.

[mintar]

@muhrix I've just tested your jade branch, and it works. However, you've squashed everything into one commit and added a couple of extra changes. This makes it extremely hard to review such a huge commit. So I've repeated the process myself in this branch: https://github.com/mintar/phidgets_drivers/tree/jade-mg .

There are a couple of differences between my branch and yours:

• You forgot to remove the include/libphidgets/phidget21.h file (it's now a duplicate).
• You added a lot of dependencies on catkin_EXPORTED_TARGETS lines. Why?
• You forgot a couple of renames.

It's a pity you pushed your changes directly to the jade branch instead of opening a PR instead. Could you reset the jade branch to indigo? (I know you shouldn't force-push, but let's make an exception just this once because nobody's using the jade branch yet):

git checkout jade
git status   # make sure you don't have any uncommitted changes, the next command will erase them
git reset --hard origin/indigo
git push --force-with-lease

Then I can open a PR for my jade-mg branch, or you can merge it directly if you like it.

[mintar]

BTW, I've posted a message to Discourse.

[muhrix]

@mintar thanks a lot for posting the message!!!

[mintar]

I wrote:

You added a lot of dependencies on catkin_EXPORTED_TARGETS lines. Why?

You were right, those should be added. I've added them to my branch.

[muhrix]

Sorry I didn't reply earlier. You figured why I added catkin_EXPORTED_TARGETS already.

I built the package on a clean, isolated environment to ensure the dependencies were all properly defined.

Thanks for pointing out the renaming of some files I missed, and also duplicate header. I'll get that sorted soon!

[mintar]

Can't you just force-push my branch to the jade branch? I already did all the work of splitting into smaller commits.

[mintar]

Ok, we now need to figure out how we manage the branches from now on. Here is my proposal.

Objectives:

• Avoid that the indigo branches on ros-drivers and here diverge! We must be careful that whenever we push something to indigo on one of the repos, we push it to the other as well.
• Finish merging everything we want into indigo before splitting of the jade branch to avoid having to merge a lot.

Next steps:

• Delete the jade branch from this repo to avoid confusion.
• Merge Added High Speed Encoders Phidgets Drivers #23 into indigo once it's ready.
• Merge indigo from here into jade on ros-drivers (should be a fast-forward).
• Add a .travis.yaml to jade (I already have it finished here and will create a PR then).
• Merge my jade-mg branch into jade.
• Merge Report mag data in Tesla, not Gauss #28 into jade.
• Create a PR for phidgets_imu should publish MagneticField, not Vector3 #29 and merge into jade.

Agreed?

[mintar]

I've just released phidgets_drivers into jade + kinetic. @muhrix : This issue can be closed.