search

zivid / zivid-ros

Official ROS driver for Zivid 3D cameras
BSD 3-Clause "New" or "Revised" License
60 stars 45 forks source link

Support for sdk 2.2 #38

Closed apartridge closed 3 years ago

runenordmo commented 3 years ago

This is likely not introduced in this PR, but when I was running the sample I got reflection removal enabled by default in the dynamic reconfigure, which should likely not be the case: image

Edit: Reflection removal enabled to true is correct, as the default false is set to true in the sample: https://github.com/zivid/zivid-ros/blob/6266868c81d6997db60f882a208c6c50f5ea5405/zivid_samples/src/sample_capture.cpp#L60

runenordmo commented 3 years ago

The Stripe engine option will now be visible in the dynamic reconfigure for Zivid One, although it's supported on Zivid One+. I guess we are restricted by the way that we generate the enum setting in ROS, which will have to contain all possible enum values from the start (here two enum values).

runenordmo commented 3 years ago

Review done for this round, added some comments. Looks good :+1:

apartridge commented 3 years ago

Added 1 commit that increases the test timeout (hopefully helps CI stability)

runenordmo commented 3 years ago

Added 1 commit that increases the test timeout (hopefully helps CI stability)

Any idea why that was needed (as opposed to before)?

apartridge commented 3 years ago

Added 1 commit that increases the test timeout (hopefully helps CI stability)

Any idea why that was needed (as opposed to before)?

We added 1 test and it looks like sometimes the CI is slower and we hit the old limit, so we should bump it for now. To me it looks like the Azure CI machines are slow in general, in the CI it takes like 4-5 times as long as my local machine.

runenordmo commented 3 years ago

Added 1 commit that increases the test timeout (hopefully helps CI stability)

Any idea why that was needed (as opposed to before)?

We added 1 test and it looks like sometimes the CI is slower and we hit the old limit, so we should bump it for now. To me it looks like the Azure CI machines are slow in general, in the CI it takes like 4-5 times as long as my local machine.

I see. I'll re-approve now.