-
Notifications
You must be signed in to change notification settings - Fork 11
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
Create plans to arm the fast-cs eiger #1054
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1054 +/- ##
==========================================
- Coverage 97.68% 97.32% -0.36%
==========================================
Files 160 161 +1
Lines 6600 6624 +24
==========================================
Hits 6447 6447
- Misses 153 177 +24 ☔ View full report in Codecov by Sentry. |
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.
Thanks! Couple of comments, I think there are some additional things we need to set too:
num_frames_chunks
- The file name/directory e.g. everything in
set_odin_pvs
dev_shm
def load_metadata( | ||
eiger: EigerDetector, energy: float, enable: bool, detector_params: DetectorParams | ||
): | ||
def _inner_plan(): |
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.
Should: Why do you need an inner plan here?
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.
I think it was the first thing I wrote for structure and left in for no reason; removing as definitely not needed :0
def set_mx_settings_pvs(eiger: EigerDetector, detector_params: DetectorParams): | ||
beam_x_pixels, beam_y_pixels = detector_params.get_beam_position_pixels( | ||
detector_params.detector_distance | ||
) | ||
|
||
yield from bps.abs_set(eiger.drv.beam_centre_x, beam_x_pixels) | ||
yield from bps.abs_set(eiger.drv.beam_centre_y, beam_y_pixels) | ||
yield from bps.abs_set(eiger.drv.det_distance, detector_params.detector_distance) | ||
|
||
yield from bps.abs_set(eiger.drv.omega_start, detector_params.omega_start) | ||
yield from bps.abs_set(eiger.drv.omega_increment, detector_params.omega_increment) |
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.
Should: We probably want to group these things as it makes it more async (even if Tom/Gary assure us it's instantaneous). Then we will need to wait on them at some point so we should add that optionally too e.g.
def set_mx_settings_pvs(eiger: EigerDetector, detector_params: DetectorParams): | |
beam_x_pixels, beam_y_pixels = detector_params.get_beam_position_pixels( | |
detector_params.detector_distance | |
) | |
yield from bps.abs_set(eiger.drv.beam_centre_x, beam_x_pixels) | |
yield from bps.abs_set(eiger.drv.beam_centre_y, beam_y_pixels) | |
yield from bps.abs_set(eiger.drv.det_distance, detector_params.detector_distance) | |
yield from bps.abs_set(eiger.drv.omega_start, detector_params.omega_start) | |
yield from bps.abs_set(eiger.drv.omega_increment, detector_params.omega_increment) | |
def set_mx_settings_pvs(eiger: EigerDetector, detector_params: DetectorParams, group="mx_settings", wait=True): | |
beam_x_pixels, beam_y_pixels = detector_params.get_beam_position_pixels( | |
detector_params.detector_distance | |
) | |
yield from bps.abs_set(eiger.drv.beam_centre_x, beam_x_pixels, group) | |
yield from bps.abs_set(eiger.drv.beam_centre_y, beam_y_pixels, group) | |
yield from bps.abs_set(eiger.drv.det_distance, detector_params.detector_distance, group) | |
yield from bps.abs_set(eiger.drv.omega_start, detector_params.omega_start, group) | |
yield from bps.abs_set(eiger.drv.omega_increment, detector_params.omega_increment, group) | |
if wait: | |
yield from bps.wait(group) |
This is a pattern we've used in a few places e.g. https://github.com/DiamondLightSource/mx-bluesky/blob/11c8a31f05dfc8421b9ebf205cc49598cd8806be/src/mx_bluesky/beamlines/i24/serial/setup_beamline/setup_beamline.py#L58. It allows the caller to have some flexibility in timings.
We should do the same for other plans in this file too
Resolves #1053
This PR is not meant to be merged; kept as a draft to monitor changes to the dev branch.