AravisProject / aravis

A vision library for genicam based cameras
GNU Lesser General Public License v2.1
872 stars 326 forks source link

Also test `ArvGevSCPSFireTestPacket` for auto packet size. #886

Closed feuerste closed 6 months ago

feuerste commented 6 months ago

Some cameras, e.g. the Allied Vision Alvium G1, don't have the GevSCPSFireTestPacket feature, but only the ArvGevSCPSFireTestPacket, so we now test both.

feuerste commented 6 months ago

@EmmanuelP I just saw a comment in e30bdaf064a5e12434f448b3cc61e707260d91bd about GevSCPSFireTestPacket. I guess my PR does not make sense, as the Arv prefixed version ArvGevSCPSFireTestPacket is only internal to aravis and always available, right? However, the test_packet_check function also uses the prefixed version. With the changes in this PR (basically resulting in the feature check to be always true) the Allied Vision Alvium G1 is able to do automatic packet size adjustments, without it it isn't. Would you mind to explain what exactly happens with the prefix? Thanks a lot!

feuerste commented 6 months ago

@EmmanuelP I just saw a comment in e30bdaf about GevSCPSFireTestPacket. I guess my PR does not make sense, as the Arv prefixed version ArvGevSCPSFireTestPacket is only internal to aravis and always available, right? However, the test_packet_check function also uses the prefixed version. With the changes in this PR (basically resulting in the feature check to be always true) the Allied Vision Alvium G1 is able to do automatic packet size adjustments, without it it isn't. Would you mind to explain what exactly happens with the prefix? Thanks a lot!

Thinking more about this, would a viable alternative to checking the availability of GevSCPSFireTestPacket be that we entirely remove the check and add a timeout instead, so the auto_packet_size function is not permitted to take more than a second or couple of seconds? So in case of a DMK 23G618, where one test packet takes 300 ms, we would just time out and return the current packet size...

feuerste commented 6 months ago

Closing this in favor of #888.