mabuchilab / Instrumental

Python-based instrumentation library from the Mabuchi Lab.
http://instrumental-lib.readthedocs.org/
GNU General Public License v3.0
120 stars 80 forks source link

Broken behaviour of UC480 Thorlabs camera driver #150

Closed LMSC-NTappy closed 2 years ago

LMSC-NTappy commented 2 years ago

Hello,

I have some troubles with the current behaviour of the thorlabs UC480 camera driver.

from instrumental import list_instruments,Q_
instruments = list_instruments()
instruments
# [<ParamSet[UC480_Camera] serial=b'4102795822' model=b'C1285R12M' id=1>]
i = instruments[0]
cam = i.create()
cam._get_exposure()
# 10.012500000000001 millisecond
cam._set_exposure(Q_(0.1,'ms'))
cam._get_exposure()
# 0.1335 millisecond
cam.grab_image()
# array([[255, 255, 255, ..., 255, 255, 255],
#        [255, 255, 255, ..., 255, 255, 255],
#        [255, 255, 255, ..., 255, 255, 255],
#        ...,
#        [255, 255, 255, ..., 255, 255, 255],
#        [255, 255, 255, ..., 255, 255, 255],
#        [255, 255, 255, ..., 255, 255, 255]], dtype=uint8)
cam._get_exposure()
# 10.012500000000001 millisecond

Obviously, if one applies a new exposure time to the camera and asks to grab a frame, she expects the exposure time to be applied in the acquisition and not re-set to default. This makes the _set_exposure as well as some facet parameters useless.

This is due to a faulty behaviour of the _handle_keywords function. https://github.com/mabuchilab/Instrumental/blob/fe8f3b4be7cd25976b6187fe4cfcf0dc0f2cbb63/instrumental/drivers/cameras/__init__.py#L208

If no args are passed, the parameters are re-set to the default values. Since I saw that you are currently changing this in #140 & #144 I thought I would share.

seb5g commented 2 years ago

In fact grab_image is requesting parameters as argument for them to be applied. This is kind of awkward. In my PR #144 I added functionalities for that reason.

natezb commented 2 years ago

This behavior is not "broken", it is as originally designed. Note that in your example code you're using private functions not intended for external use. There is currently no public interface for setting things like exposure in a persistent way.

We can consider adding persistent setting of values like this, but we'll have to be careful in thinking how this interacts with the existing behavior. For example, if we pass an exposure to grab_image(), does that persistently set the exposure, or only set it for that image? I'm thinking probably the latter.

cam.grab_image()  # Uses default value

cam.exposure = '100 ms'
cam.grab_image()  # 100 ms

cam.grab_image(exposure='20 ms')  # 20 ms

cam.grab_image()  # 100 ms
LMSC-NTappy commented 2 years ago

Ok, I get it now. I didn't think of it that way, sorry.

In any case I agree with your suggestion and example.

Many thanks for adressing this. I'll close this issue