Skip to content

BUG: fix wrong DVF parameters in class constants #1242

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
xresende wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

BUG: fix wrong DVF parameters in class constants #1242

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions siriuspy/siriuspy/devices/dvf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""DVF devices."""

import numpy as _np

from mathphys.functions import get_namedtuple as _get_namedtuple
from mathphys.imgproc import (
FitGaussianScipy as _FitGaussianScipy,
Expand All @@ -25,8 +26,10 @@ class DEVICES:

_default_timeout = 10 # [s]

# should these parameters be moved to csconsts or a DVFSearch?
_dvfparam_fields = (
'MAX_INTENSITY_NR_BITS',
'MAX_INTENSITY_NR_BITS', # [nrbits]
'IMAGE_NR_PIXEL_MULTP', # roi params must be multip. of nr of pixels
'ACQUISITION_TIME_MIN', # [s]
'ACQUISITION_TIME_DEFAULT', # [s]
'EXPOSURE_TIME_DEFAULT', # [s]
Expand All @@ -38,19 +41,21 @@ class DEVICES:

_dev2params = {
DEVICES.CAX_DVF1: _get_namedtuple(
# Basler acA1300-75gm (version 106755-24)
'DVFParameters',
_dvfparam_fields,
(16, 0.5, 0.5, 0.100, 2064, 3088, 2.4, 5.0),
(16, 16, 0.5, 0.5, 0.100, 2064, 3088, 2.4, 5.0),
),
Comment on lines 43 to 48

@henriquesimoes henriquesimoes Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I mentioned in the deprecated PR, this camera (acA1300-75gm) has a $1024 \times 1280~\text{px}^2$ sensor, not $2064 \times 3088~\text{px}^2$ as defined here.

Can't we always use the existing cam_max_sizex and cam_max_sizey methods (which read the size of the sensor from the IOC) instead of redefining those constants? As of now, it seems dvfimgproc/csdev.py and dvfimgproc/meas.py consume these IMAGE_SIZE_Y and IMAGE_SIZE_X constants defined here.

I just noticed the pixel size is also wrong, because it was replicated from DVF2. It is $4.8\mu m$ for this model. Unfortunately, there is no way of reading this from the camera itself, so my suggestion for the sensor size can't be applied for this.

TL;DR: If we keep redefining the size constants, it should be at least updated to the following:

Suggested change
DEVICES.CAX_DVF1: _get_namedtuple(
# Basler acA1300-75gm (version 106755-24)
'DVFParameters',
_dvfparam_fields,
(16, 0.5, 0.5, 0.100, 2064, 3088, 2.4, 5.0),
(16, 16, 0.5, 0.5, 0.100, 2064, 3088, 2.4, 5.0),
),
DEVICES.CAX_DVF1: _get_namedtuple(
# Basler acA1300-75gm (version 106755-24)
'DVFParameters',
_dvfparam_fields,
(16, 16, 0.5, 0.5, 0.100, 1024, 1280, 4.8, 5.0),
),

DEVICES.CAX_DVF2: _get_namedtuple(
'DVFParameters',
_dvfparam_fields,
(16, 0.5, 0.5, 0.100, 2064, 3088, 2.4, 5.0),
(16, 16, 0.5, 0.5, 0.100, 2064, 3088, 2.4, 5.0),
),
Comment on lines 49 to 53

@henriquesimoes henriquesimoes Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This camera model is acA3088-16gm (version 107406-12), and its IMAGE_NR_PIXEL_MULTP equals to 4, not 16. (Yeah, that's the original one we used for developing the device, and where the constant 4 came from in cam_roi_calc).

Thus:

Suggested change
DEVICES.CAX_DVF2: _get_namedtuple(
'DVFParameters',
_dvfparam_fields,
(16, 0.5, 0.5, 0.100, 2064, 3088, 2.4, 5.0),
(16, 16, 0.5, 0.5, 0.100, 2064, 3088, 2.4, 5.0),
),
DEVICES.CAX_DVF2: _get_namedtuple(
'DVFParameters',
_dvfparam_fields,
(16, 4, 0.5, 0.5, 0.100, 2064, 3088, 2.4, 5.0),
),

DEVICES.BO_DVF: _get_namedtuple(
# Basler acA1300-75gm (version 106755-13)
'DVFParameters',
_dvfparam_fields,
(8, 0.5, 0.5, 0.005, 1024, 1280, 4.8, 5.0),
(8, 16, 0.5, 0.5, 0.005, 1024, 1280, 4.8, 5.0),
),
}

Expand Down Expand Up @@ -394,12 +399,20 @@ def cam_frames_failures(self):

def cmd_reset(self, timeout=None):
"""Reset DVF to a standard configuration."""
# TODO: is reseting BASLER roi necessary?

@henriquesimoes henriquesimoes Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to ensure it will always be in a valid state, I think so. Otherwise, the client of the DVF class will need to know the image might be cut to an arbitrary and possibly irrelevant portion of the sensor.

Why do you think it might not be needed?

# if not self.cmd_cam_roi_reset():
# return False
props_values = {
'cam1:ArrayCallbacks': 1, # Enable passing array
'cam1:ImageMode': 2, # Continuous
'cam1:PixelFormat': 1, # Mono12
# ROI1 takes images from camera driver
'ROI1:NDArrayPort': self['cam1:PortName_RBV'],
# TODO: check if these properties are necessary to be set on reset
# 'cam1:DataType': 1, # UInt16 (maybe unnecessary)
Comment on lines 408 to +412

@henriquesimoes henriquesimoes Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we set cam1:PixelFormat to Mono12, the IOC will automatically update the cam1:DataType to UInt16.

# 'cam1:NumImages': 1, # number of sequential acquired images

@henriquesimoes henriquesimoes Jun 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use ImageMode as Continuous, this PV is irrelevant. Is there any scenario which we use other acquisition mode?

# 'cam1:ExposureMode': 0, # TIMED

@henriquesimoes henriquesimoes Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems our cameras (both models relevant here) don't support any ExposureMode other than "timed". So this one in fact isn't currently needed.

# 'cam1:TriggerMode': 0, # Off

@henriquesimoes henriquesimoes Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we ever change the TriggerMode, it seems relevant to ensure this is off. Otherwise, the camera won't acquire because no external trigger will come, right?

I've never configured these cameras to use external trigger, but it seems that's one of the many parameters required to use that mode of operation.

'ROI1:EnableCallbacks': 1, # Enable getting from NDArrayPort
'ROI1:MinX': 0, # [pixel]
'ROI1:MinY': 0, # [pixel]
Expand Down
Loading