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
69 stars 41 forks source link

Ximea camera (and others) dont allow arbitrary ROI values #293

Open juliomateoslangerak opened 11 months ago

juliomateoslangerak commented 11 months ago

Trying to change the roi on the ximea camera to anything containing an uneven value throws a ximea.xiapi.Xi_error: ERROR 101: Wrong parameter value

>>> # camera is a Ximea camera instance
>>> camera.get_binning()
Binning(h=1, v=1)
>>> camera.get_roi()
ROI(left=0, top=0, width=2048, height=2048)
>>> camera.set_roi((512, 512, 1024, 1024))
True
>>> camera.set_roi((513, 512, 1024, 1024))
Traceback (most recent call last):
  File "/snap/pycharm-professional/354/plugins/python/helpers/pydev/pydevconsole.py", line 364, in runcode
    coro = func()
           ^^^^^^
  File "<input>", line 1, in <module>
  File "/home/julio/PycharmProjects/microscope/microscope/abc.py", line 980, in set_roi
    return self._set_roi(roi)
           ^^^^^^^^^^^^^^^^^^
  File "/home/julio/PycharmProjects/microscope/microscope/cameras/ximea.py", line 416, in _set_roi
    self._handle.set_offsetY(roi.top)
  File "/home/julio/PycharmProjects/microscope/ximea/xiapi.py", line 1444, in set_offsetY
    self.set_param('offsetY', offsetY)
  File "/home/julio/PycharmProjects/microscope/ximea/xiapi.py", line 403, in set_param
    raise Xi_error(stat)
ximea.xiapi.Xi_error: ERROR 101: Wrong parameter value
>>> camera.get_roi()
ROI(left=512, top=512, width=1024, height=1024)
>>> camera.set_roi((512, 512, 1023, 1024))
...
ximea.xiapi.Xi_error: ERROR 101: Wrong parameter value
iandobbie commented 11 months ago

I vaguely remember another camera which had specific sizes that ROIs could be set to. Obviously we need to sanitize the input to ensure it is even and sensible eg not width 0, or outside the sensor edge.

Maybe the set ROI functions should return the ROI that is actually set as this would mean that the calling function and have the real ROI rather than what it set, but was not valid on that camera.

juliomateoslangerak commented 11 months ago

Or rather return False and log/raise the error (IncompatibleDeviceState?).

Do we specify if left and top are zero based? Ximea uses 0 as the first pixel position and andor uses 1.

iandobbie commented 11 months ago

I think returning an error is the wrong behavior as a random user is unlikely to know you need even positions. I think it would be better to force the ROI to a valid value and then return that.

As to whether we count from 1 or 0, as you say we should probably specify this and then let the specific device code transform it to whichever. I would suggest counting from 0.

juliomateoslangerak commented 11 months ago

Should we, for the sake of consistency, do the same for other settings (binning, exposure-time,...)

As for the first pixel of the camera, I would also go for 0.

iandobbie commented 11 months ago

Ok, I think these are two separate issues and we should have separate discussions.

1) Forcing parameters to valid values and returning the value set, I propose we keep this discussion here, maybe change the issue name.

2) Defining the start value for camera pixels as 0, and if a camera counts from 1 then the hardware specific device code needs to deal with the translating this. I have opened issue #294 to cover this.

iandobbie commented 11 months ago

As Julio comments binning is a functionality that often has restrictions on what values are possible and hence valid, maybe both x and y must be equal, or must be powers of two. Another ares where this can be significant is exposure times. There are frequently restrictions on what exposures are valid.

I think it would be reasonable to change the code so that setting values would always return the true value set, with the expectation that this MAY be different than the passed value. With the hardware specific code expected to massage the values if needed (eg ROI on Ximea cameras) or pass on to lower level code which does this, eg the raspberry PIcam exposure time setting will set something near what you ask for but often not exactly that.

juliomateoslangerak commented 11 months ago

Looking into Hamamatsu's docs, this is some of a complex issue to handle and depends on the camera model. ie: the orca.spark is very much like the ximea:

DCAM_IDPROP_SUBARRAYHPOS  
0 to 1918         , step 2         , default 0
DCAM_IDPROP_SUBARRAYHSIZE  
2 to 1920         , step 2         , default 1920
DCAM_IDPROP_SUBARRAYVPOS  
0 to 1198         , step 2         , default 0
DCAM_IDPROP_SUBARRAYVSIZE  
2 to 1200         , step 2         , default 1200

and the orca.flash (I think)

DCAM_IDPROP_SUBARRAYHPOS
0 to 2300         , step 4         , default 0 | @ DCAMPROP_SENSORMODE__AREA
0 to 2303         , step 1         , default 0 | @ DCAMPROP_SENSORMODE__PROGRESSIVE
DCAM_IDPROP_SUBARRAYHSIZE
4 to 2304         , step 4         , default 2304 | @ DCAMPROP_SENSORMODE__AREA
1 to 2304         , step 1         , default 2304 | @ DCAMPROP_SENSORMODE__PROGRESSIVE
DCAM_IDPROP_SUBARRAYVPOS
0 to 2300         , step 4         , default 0 |  
DCAM_IDPROP_SUBARRAYVSIZE
4 to 2304         , step 4         , default 2304 |  

I presume that for ximea cameras it is a bit of the same or even more complex as the number of different chip types are more diverse.