-
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
Changes to support hyperion 1283 use pixels per micron from file not gda #423
Changes to support hyperion 1283 use pixels per micron from file not gda #423
Conversation
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.
Just one nitpick, up to you
src/dodal/beamlines/i03.py
Outdated
def oav( | ||
wait_for_connection: bool = True, | ||
fake_with_ophyd_sim: bool = False, | ||
params=OAVConfigParams(ZOOM_PARAMS_FILE, DISPLAY_CONFIG), |
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.
Nit: the default arg will be evaluated at definition time (i.e. on import). I know that just stores the paths, but I'd still rather do something like:
@skip_device(lambda: BL == "s03")
def oav(
wait_for_connection: bool = True,
fake_with_ophyd_sim: bool = False,
params: OAVConfigParams | None = None,
) -> OAV:
"""Get the i03 OAV device, instantiate it if it hasn't already been.
If this is called when already instantiated in i03, it will return the existing object.
"""
return device_instantiation(
OAV,
"oav",
"",
wait_for_connection,
fake_with_ophyd_sim,
params=params or OAVConfigParams(ZOOM_PARAMS_FILE, DISPLAY_CONFIG),
)
In case the OAVConfigParams
ever changes to do something in its init
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'll make the change, although I think most classes should not do anything interesting in constructors, especially parameter classes.
… generation for ispyb
…litate testability
…icrons_per_pixel when the snapshot is triggered instead of requiring the plan to apply it manually.
00388e4
to
0aeff28
Compare
Fixes (DiamondLightSource/hyperion#1283)
Add microns-per-pixel to the snapshot so that oav snapshot events can contain data needed for ispyb
A small change to OAV initialiser to allow parameters to be specified on construction for system tests
Checks for reviewer