ros-drivers / pointgrey_camera_driver

ROS driver for Pt. Grey cameras, based on the official FlyCapture2 SDK.
128 stars 178 forks source link

No support for setting GigE packet size and delay #14

Open wnoise opened 10 years ago

wnoise commented 10 years ago

Setting these can really improve the number of images that are received with no problem.

Implementation wise, this is a bit hairy, and there are multiple possible approaches.

The Flycap library supports two camera classes: Camera and GigECamera. The first provides functionality for the FireWire (and USB, I believe) cameras, and some partial support for GigE cameras, mapping video mode request to appropriate GigE window and binning requests. They share a common abstract base class CameraBase, but it only has support for the intersection of common functionality.

Trying to use CameraBase pointers everywhere doesn't work because the driver uses Camera specific functionality to set the video modes, and setting the packet size and delay also needs GigECamera specific functionality.

Having both objects around, with only one actually active and connected should work (though be vary complicated in ensuring the use of the right object at the right times), but then the video-mode settings (exposed via the dynamic reconfigure / parameters) would need to be emulated by hand rather than via the Camera class in the GigE case.

Restricting the configuration to only supporting the mode 7 style settings in both cases would greatly simplify this emulation, but I expect this is unacceptable for compatibility reasons.

mikepurvis commented 10 years ago

Not something I can commit time to personally, but I'd happily review a pull request improving the situation.

What about having common functionality implemented in a class templated on Camera and GigECamera, and using traits to handle the differences in the two?

wnoise commented 10 years ago

Hmm. That might work, but it seems like it would push towards having the user specify what the camera type is, as the template would need to be instanced differently based on that.

I'll think about it a bit more and hopefully get a pull request together this week.