genicam / harvesters

Image Acquisition Library for GenICam-based Machine Vision System
Apache License 2.0
500 stars 86 forks source link

Undocumented removal of sleep_duration property (held at small default) #429

Open erik-mansson opened 9 months ago

erik-mansson commented 9 months ago

Describe the Issue I'm in the process of upgrading from 1.3.2 to 1.4.2 and while trying to respect the change from Harvester.create_image_acquirer() to Harvester.create(), I noticed some changes and at least documentation issues about the parameter for sleeping in _NativeThread (after each image readout?). This seems related to the changes in pull request #320.

While the now deprecated create_image_acquirer() still has a sleep_duration argument defaulting to 1E-6 seconds, it is no longer used within the body of create_image_acquirer(). Although ImageAcquirer._supported_parameters mentions ParameterKey.THREAD_SLEEP_PERIOD, neither create_image_acquirer() nor ImageAcquirer.__init__() seems to get or use that parameter.

The docstring for ImageAcquirer.__init__() wrongly mentions the old sleep_duration : float argument, which it no longer accepts.

In the old version (e.g. 1.3.2), the sleep_duration was passed from ImageAcquirer via _ImageAcquisitionThread to _NativeThread.__init__() and _NativeThread.run() slept for that duration after each call to self._worker(). Now (e.g. 1.4.2), _NativeThread.__init__(... sleep=sys.float_info.min) has a very small default value (2E-308 seconds) and no other value is provided when constructed by _EventMonitor. I don't know whether time.sleep() actually sleeps at all when given the argument 2E-308 seconds, since this is way shorter than the Planck time (down in the range where no physical theories are established ;-).

Please enlighten me if ParameterKey.THREAD_SLEEP_PERIOD, which sounds like it should be the replacement, is in fact being used in some way I did not discover.

Expected Behavior

As I haven't finished converting my program or done any performance tests, I don't know whether I really need the sleep-configuration. My old code was varying the value between 1E-5 s (when acquiring at a few kHz) and 5E-3 s (when acquiring with long exposure time at low rate) -- with the idea that it seemed nice to avoid interrupting the CPU every few microseconds if I'm fine with longer latency.

The only real issue might be in the documentation: sleep_duration should be removed from the docstring of ImageAcquirer.__init__() the although Harvester.create_image_acquirer() is deprecated it seems worth mentioning that the behaviour has changed and it now ignores the sleep_duration parameter.

I see there are some other timeout parameters (ParameterKey.TIMEOUT_PERIOD_ON_UPDATE_EVENT_DATA_CALL and ParameterKey.TIMEOUT_PERIOD_ON_CLIENT_FETCH_CALL) which I don't use yet. In case it was concluded that sleep_duration was not needed, it would be nice to document it (at least I didn't find it).