-
Notifications
You must be signed in to change notification settings - Fork 8
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.
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
[#207] Add VMXm fast grid scan devices #211
base: main
Are you sure you want to change the base?
Changes from all commits
a516675
2380dfd
8cbc6e9
335ef4a
900467a
75e5928
429eb1d
9adc9ee
d96e13d
3908472
a869668
76c6391
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,142 @@ | ||
from typing import Optional | ||
|
||
from dodal.beamlines.beamline_utils import device_instantiation | ||
from dodal.beamlines.beamline_utils import set_beamline as set_utils_beamline | ||
from dodal.devices.backlight import VmxmBacklight | ||
from dodal.devices.detector import DetectorParams | ||
from dodal.devices.eiger import EigerDetector | ||
from dodal.devices.fast_grid_scan_2d import FastGridScan2D | ||
from dodal.devices.synchrotron import Synchrotron | ||
from dodal.devices.vmxm.vmxm_attenuator import VmxmAttenuator | ||
from dodal.devices.vmxm.vmxm_sample_motors import VmxmSampleMotors | ||
from dodal.devices.zebra import Zebra | ||
from dodal.log import set_beamline as set_log_beamline | ||
from dodal.utils import get_beamline_name, skip_device | ||
|
||
SIM_BEAMLINE_NAME = "S02-1" | ||
|
||
BL = get_beamline_name(SIM_BEAMLINE_NAME) | ||
set_log_beamline(BL) | ||
set_utils_beamline( | ||
BL, suffix="J", beamline_prefix="BL02J", insertion_prefix="SR-DI-J02" | ||
) | ||
|
||
|
||
@skip_device(lambda: BL == SIM_BEAMLINE_NAME) | ||
def eiger( | ||
wait_for_connection: bool = True, | ||
fake_with_ophyd_sim: bool = False, | ||
params: Optional[DetectorParams] = None, | ||
) -> EigerDetector: | ||
"""Get the i24 Eiger device, instantiate it if it hasn't already been. | ||
If this is called when already instantiated, it will return the existing object. | ||
If called with params, will update those params to the Eiger object. | ||
""" | ||
|
||
def set_params(eiger: EigerDetector): | ||
if params is not None: | ||
eiger.set_detector_parameters(params) | ||
|
||
return device_instantiation( | ||
device_factory=EigerDetector, | ||
name="eiger", | ||
prefix="-EA-EIGER-01:", | ||
wait=wait_for_connection, | ||
fake=fake_with_ophyd_sim, | ||
post_create=set_params, | ||
) | ||
|
||
|
||
@skip_device(lambda: BL == SIM_BEAMLINE_NAME) | ||
def fast_grid_scan( | ||
wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False | ||
) -> FastGridScan2D: | ||
"""Get the vmxm fast_grid_scan device, instantiate it if it hasn't already been. | ||
If this is called when already instantiated in vmxm, it will return the existing object. | ||
""" | ||
return device_instantiation( | ||
device_factory=FastGridScan2D, | ||
name="fast_grid_scan", | ||
prefix="-MO-SAMP-11:FGS:", | ||
wait=wait_for_connection, | ||
fake=fake_with_ophyd_sim, | ||
) | ||
|
||
|
||
@skip_device(lambda: BL == SIM_BEAMLINE_NAME) | ||
def zebra(wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False) -> Zebra: | ||
"""Get the vmxm zebra device, instantiate it if it hasn't already been. | ||
If this is called when already instantiated in vmxm, it will return the existing object. | ||
""" | ||
return device_instantiation( | ||
Zebra, | ||
"zebra", | ||
"-EA-ZEBRA-01:", | ||
wait_for_connection, | ||
fake_with_ophyd_sim, | ||
) | ||
|
||
|
||
@skip_device(lambda: BL == SIM_BEAMLINE_NAME) | ||
def attenuator( | ||
wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False | ||
) -> VmxmAttenuator: | ||
"""Get the vmxm attenuator device, instantiate it if it hasn't already been. | ||
If this is called when already instantiated in vmxm, it will return the existing object. | ||
""" | ||
return device_instantiation( | ||
VmxmAttenuator, | ||
"attenuator", | ||
"-OP-ATTN-01:", | ||
wait_for_connection, | ||
fake_with_ophyd_sim, | ||
) | ||
|
||
|
||
@skip_device(lambda: BL == SIM_BEAMLINE_NAME) | ||
def backlight( | ||
wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False | ||
) -> VmxmBacklight: | ||
"""Get the VMXm backlight device, instantiate it if it hasn't already been. | ||
If this is called when already instantiated in VMXm, it will return the existing object. | ||
""" | ||
return device_instantiation( | ||
device_factory=VmxmBacklight, | ||
name="backlight", | ||
prefix="", | ||
wait=wait_for_connection, | ||
fake=fake_with_ophyd_sim, | ||
) | ||
|
||
|
||
@skip_device(lambda: BL == SIM_BEAMLINE_NAME) | ||
def synchrotron( | ||
wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False | ||
) -> Synchrotron: | ||
"""Get the VMXm synchrotron device, instantiate it if it hasn't already been. | ||
If this is called when already instantiated in VMXm, it will return the existing object. | ||
""" | ||
return device_instantiation( | ||
Synchrotron, | ||
"synchrotron", | ||
"", | ||
wait_for_connection, | ||
fake_with_ophyd_sim, | ||
bl_prefix=False, | ||
) | ||
|
||
|
||
@skip_device(lambda: BL == SIM_BEAMLINE_NAME) | ||
def sample_motors( | ||
wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False | ||
) -> VmxmSampleMotors: | ||
"""Get the VMXm sample_motors device, instantiate it if it hasn't already been. | ||
If this is called when already instantiated in VMXm, it will return the existing object. | ||
""" | ||
return device_instantiation( | ||
VmxmSampleMotors, | ||
"sample_motors", | ||
"-MO-SAMP-01:", | ||
wait_for_connection, | ||
fake_with_ophyd_sim, | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
from enum import Enum | ||
from time import sleep | ||
from typing import Optional | ||
|
||
from ophyd import Component, Device, EpicsSignalRO, Signal | ||
|
@@ -35,7 +36,9 @@ def set(self, value, *, timeout=None, settle_time=None, **kwargs): | |
|
||
STALE_PARAMS_TIMEOUT = 60 | ||
GENERAL_STATUS_TIMEOUT = 10 | ||
ALL_FRAMES_TIMEOUT = 120 | ||
ALL_FRAMES_TIMEOUT = ( | ||
600 # FIXME: VMXm hack - we need to get this passed in a a param probably? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Per code comment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alternatively/in addition, we could make this variable default to a certain value based on the current beamline There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's really based on the experiment. It's how long do we expect the detector to be exposing for in total |
||
) | ||
ARMING_TIMEOUT = 60 | ||
|
||
filewriters_finished: SubscriptionStatus | ||
|
@@ -103,7 +106,9 @@ def stage(self): | |
self.async_stage().wait(timeout=self.ARMING_TIMEOUT) | ||
|
||
def stop_odin_when_all_frames_collected(self): | ||
LOGGER.info("Waiting on all frames") | ||
LOGGER.info( | ||
f"Waiting on all frames, expected {self.detector_params.full_number_of_images}" | ||
) | ||
try: | ||
await_value( | ||
self.odin.file_writer.num_captured, | ||
|
@@ -152,6 +157,10 @@ def change_roi_mode(self, enable: bool) -> Status: | |
else self.detector_params.detector_size_constants.det_size_pixels | ||
) | ||
|
||
LOGGER.info( | ||
f"Setting height and width on odin to {detector_dimensions.height}, {detector_dimensions.width}" | ||
) | ||
|
||
status = self.cam.roi_mode.set( | ||
1 if enable else 0, timeout=self.GENERAL_STATUS_TIMEOUT | ||
) | ||
|
@@ -242,7 +251,6 @@ def set_detector_threshold(self, energy: float, tolerance: float = 0.1) -> Statu | |
tolerance (float, optional): If the energy is already set to within | ||
this tolerance it is not set again. Defaults to 0.1eV. | ||
""" | ||
|
||
current_energy = self.cam.photon_energy.get() | ||
if abs(current_energy - energy) > tolerance: | ||
return self.cam.photon_energy.set( | ||
|
@@ -258,7 +266,6 @@ def set_num_triggers_and_captures(self) -> Status: | |
during the datacollection. The number of images is the number of images per | ||
trigger. | ||
""" | ||
|
||
assert self.detector_params is not None | ||
status = self.cam.num_images.set( | ||
self.detector_params.num_images_per_trigger, | ||
|
@@ -286,6 +293,9 @@ def set_num_triggers_and_captures(self) -> Status: | |
|
||
def _wait_for_odin_status(self) -> Status: | ||
self.forward_bit_depth_to_filewriter() | ||
sleep( | ||
2 | ||
) # FIXME - VMXm hack - on VMXm ODIN will *occasionally* fail to start without this sleep. Need a better solution. | ||
Comment on lines
+296
to
+298
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We might be able to do some kind of cleverer wait for ODIN to be ready to start capturing. Needs careful testing on VMXm if we adjust this logic as it was only failing to start ~10% of gridscans before this change was added. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any idea what was causing it to fail when the sleep wasn't there? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not really. Some kind of race condition at the ODIN level, I think. 2 seconds is a number plucked from thin air and we might be able to lower it or put in some better logic, but it'll need testing on VMXm if we do that. Should be possible on a Tuesday or in a shutdown if you ask the scientists - we shouldn't need beam to test this. For this issue, I'd maybe suggest doing some variant of:
and write an issue to investigate and fix this properly. |
||
status = self.odin.file_writer.capture.set( | ||
1, timeout=self.GENERAL_STATUS_TIMEOUT | ||
) | ||
|
@@ -306,16 +316,20 @@ def _finish_arm(self) -> Status: | |
|
||
def forward_bit_depth_to_filewriter(self): | ||
bit_depth = self.bit_depth.get() | ||
self.odin.file_writer.data_type.put(f"UInt{bit_depth}") | ||
self.odin.file_writer.data_type.set(f"UInt{bit_depth}").wait( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was this changed in attempt to fix the _wait_for_odin_status error? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but I believe from a discussion with Dom on the beamline this is actually the "more correct" option anyway as it waits for a completion callback. It wasn't sufficient to fix the issue on VMXm but I still think this is a good change to make. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I agree this is more correct |
||
self.GENERAL_STATUS_TIMEOUT | ||
) | ||
|
||
def disarm_detector(self): | ||
self.cam.acquire.put(0) | ||
|
||
def do_arming_chain(self) -> Status: | ||
functions_to_do_arm = [] | ||
detector_params: DetectorParams = self.detector_params | ||
if detector_params.use_roi_mode: | ||
functions_to_do_arm.append(lambda: self.change_roi_mode(enable=True)) | ||
|
||
functions_to_do_arm.append( | ||
lambda: self.change_roi_mode(detector_params.use_roi_mode) | ||
) | ||
|
||
functions_to_do_arm.extend( | ||
[ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This device is the same as I03's backlight other than the PV names. I think we should make a base
Backlight
device where the PV names are parametersThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, but in Ophyd V1 it's actually surprisingly tricky to do that due to some of the
Component
magic - I know because I had the exact same thought as you and then tried it...Suggest spinning that out into an issue if there isn't already one?