search

ros-drivers / pointgrey_camera_driver

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

Added possibility to set GigE packet size and packet delay. #18

Closed mmunaro closed 9 years ago

mmunaro commented 9 years ago

Added possibility to set GigE packet size and packet delay from launch/yaml file. Also, the raw and mono pixel formats (raw8, raw16, mono8, mono16) can be selected from dynamic reconfigure with every video mode (while before it was hard coded that only mono pixel formats could be used with mode1 and mode2).

chadrockey commented 9 years ago

I tested this today and the gige configuration seems to work well. Thanks @mmunaro !

I didn't test the pixel formats though, just rostopic hz.

mikepurvis commented 9 years ago

I don't have the bandwidth to test with hardware right now. Are you satisfied to merge based on your tests, @chadrockey?

chadrockey commented 9 years ago

Yeah, @mikepurvis I think so. Just checked in RVIZ and the raw image on my gige cameras looks fine (but I think I have only mono).

I would be more confident (and we can have cleaner pull requests) if we pulled commit 08d3f4b from this pull request and made it a separate request until someone with color cameras could test. What do you think @mmunaro ?

mmunaro commented 9 years ago

I tested both USB3 and GigE color cameras, but tomorrow I will separate commit 08d3f4b279aed9414f0b4ad16558793515f24efc from the rest.

Anyway, the problem was that the getFormat7PixelFormatFromString method seemed to be targeted to a specific camera and was imposing mono pixel format for video mode 1 (half of resolution). Do you agree that it would be better to allow to choose any pixel format from the dynamic reconfigure gui?

chadrockey commented 9 years ago

@mmunaro Probably. It was written to exercise most of the features of the USB chameleons and later a bumblebee for test purposes. It's very likely that those pixel formats are valid.

It will just be a good idea to keep the pull request separate so its easier for someone to find this commit if they are looking for that feature.

@mikepurvis I think once these are separated you can merge both. Thanks!

mmunaro commented 9 years ago

Closing this pull request because it has been split into pull requests #20 and #21.