Removed hardcoded strobe configuration for GPIO

ros-drivers / pointgrey_camera_driver

ROS driver for Pt. Grey cameras, based on the official FlyCapture2 SDK.

128 stars 180 forks source link

Removed hardcoded strobe configuration for GPIO #80

Closed evaldask closed 7 years ago

evaldask commented 7 years ago

Instead of having hardcoded GPIO pin number for strobe added same options as triggers.

mikepurvis commented 7 years ago

This seems reasonable, but I'm concerned that it will break anyone with a pre-existing configuration using the old parameter names. What do you think is the best way to proceed here?

evaldask commented 7 years ago

I see two ways of doing it:

leaving names as they are and just adding strobe_source parameter with default value GPIO1. That may lead to some misunderstanding but it will not break pre-existing configuration
another way is to do it like xacro. In new ROS release add to wiki "New in ..." and have these changes in new release

What do you think about it?

mikepurvis commented 7 years ago

The biggest issue is that the master branch is released to all distros, so there's no way to not change the behaviour on Indigo, short of just not making any more indigo releases, or creating an indigo branch at this juncture, and allowing the breaking change to go into master, targeting Jade and Kinetic.

BenBlumer commented 7 years ago

Check out pull request #91 -- I've added a hard option to strobe on GPIO2. It's not as classy as this, but it keeps backwards compatibility. It'd be nice to be able to set whichever GPIOs, but GPIO1 and GPIO2 are the only ones that PointGrey recommends in their literature anyways.

bdholt1 commented 7 years ago

If we use the FlyCapture software as a reference, then I'm inclined to suggest that we go with #91. It makes sense to me to make the dynamic reconfigure parameters mimic the FlyCapture demo software as closely as possible to

make it more intuitive for new users of this software to set parameters to achieve their goal
make it easier to get official support from Point Grey when something isn't working well

What do you think?

mikepurvis commented 7 years ago

Closed in favour of #91. Thanks!