search

python-microscope / microscope

Python library for control of microscope devices, supporting hardware triggers and distribution of devices over the network for performance and flexibility.
https://www.python-microscope.org
GNU General Public License v3.0
67 stars 39 forks source link

Andor EMCCD (SDK2) binning settings #161

Closed iandobbie closed 3 years ago

iandobbie commented 3 years ago

The EMCCD andor driver has setting for both "binning" and "Binning" which is correct and can we get rid of the other one.

Ian

carandraug commented 3 years ago

The "binning" setting comes from the Camera abstract base class, while "Binning" is set on the AndorAtmcd class. I believe the point of having it on Camera is to have something that all devices will share, while on the concrete classes we have something with the exact name (and quirks) of the device. For example, AndorSDK3 also has the AOIBinning setting which is an enum and PVCamera has "BINNING_SER" and "BINNING_PAR" (if I read the code correctly).

However, in the base class we already have the get_binning and set_binning methods so I guess we could just remove the "binning" setting from the Camera abstract base class since it's redundant. The same for the "roi" setting. How does that sound?

iandobbie commented 3 years ago

I wonder if the idea was to have a generic binning function that add adjacent pixels, which can be overridden on specific cameras (ie the CCDs rater than CMOS) which sum the electrons on chip and read out once? If this is so the abc functions should take the binning and sum pixels by the x,y binning factors, whereas the cameras which can actually bin on chip should override this function with the relevant api settings to enable the hardware functionality.

carandraug commented 3 years ago

We can have a software fallback for binning, if we want (do we? Is there any point of doing binning after acquisition other than reducing size?). The issue here is that given an AndorAtmcd instance, we have 3 ways to set binning and ROI, all doing exactly the same behind the scenes:

camera.set_binning(Binning(1,1)) # a method from Camera
camera.set_setting("binning", (1,1)) # setting added by Camera class
camera.set_binning("Binning", (1,1)) # setting added by AndorAtmcd class

I propose we:

  1. remove the "binning" setting (added by Camera)
  2. keep the Camera.set_binning as the official approved method to set binning in the camera (which does whatever it is needed to do it on the hardware if possible)
  3. keep the "Binning" as a setting that maps one to one to a setting on the device hardware (for those that know the specifics of the device and want to make use of them).

This also removes the "binning" setting from AndorSDK3, PVCamera, and XimeaCamera, but all have Camera.set_binning and some sort of device specific binning related setting.

iandobbie commented 3 years ago

We should definitely enable the camera provided binning as this is the only way to do it properly on a CCD camera, it also might significantly increase throughput if it is done on the FPGA of a CMOS camera.

David approach seems reasonable, and then the camera abc might enforce the existence of a set_binning function.

carandraug commented 3 years ago

Done as suggested (removed the "binning" setting from the Camera ABC). Closing as fixed.