-
Notifications
You must be signed in to change notification settings - Fork 59
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
add preset support for SQR1 #1255
base: master
Are you sure you want to change the base?
Conversation
fix isort
|
||
if preset_pos is not None: | ||
axis_list = [self.x, self.y, self.z, self.rz, self.ry, self.rz] | ||
if any(hasattr(a.presets.positions, preset_pos) for a in axis_list): |
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.
Is this intended to be all
instead of any
, to force all the presets to exist before moving?
ry_sp = (getattr(self.ry.presets.positions, preset_pos)).pos | ||
rz_sp = (getattr(self.rz.presets.positions, preset_pos)).pos | ||
else: | ||
logger.error('One of the axes is missing the desired state.') |
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 this return
or raise
as well in the error state to prevent the rest of the function from running?
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 like this, two small observations/questions above
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.
Side note: the new functionality you've added here is unit-testable if you have the time
@@ -0,0 +1,30 @@ | |||
IssueNumber Title |
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.
did you run the script to generate this file? I'm surprised the filename is standard but the title here isn't
Description
add preset support for SQR1 device
Motivation and Context
James Cryan asked for preset feature so he can move lamp sqr1 to predefined positions where beam focus is the best
How Has This Been Tested?
tested using hutch-python with TMO LAMP SQR1
Where Has This Been Documented?
Pre-merge checklist