This repository has been archived by the owner on Sep 2, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 5
Cache lower gonio positions before robot load and restore them afterwards #1467
Merged
Merged
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
6f03b2e
First draft of caching lower gonios before robot load
DominicOram d01425c
Add test for returning lower_gonio to correct position
DominicOram fa0ee27
Merge branch 'main' into 1439_save_restore_lower_gonio_pos
DominicOram bdf3dec
Merge branch 'main' into 1439_save_restore_lower_gonio_pos
DominicOram 2962cc7
Update dodal
DominicOram b0d75a4
Fix unit tests
DominicOram 5a423c7
Specify argument for readability
DominicOram e49c812
Changes from review comments
DominicOram 039099e
Update dodal
DominicOram File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ | |
from dodal.devices.fast_grid_scan import PandAFastGridScan, ZebraFastGridScan | ||
from dodal.devices.flux import Flux | ||
from dodal.devices.focusing_mirror import FocusingMirrorWithStripes, VFMMirrorVoltages | ||
from dodal.devices.motors import XYZPositioner | ||
from dodal.devices.oav.oav_detector import OAV | ||
from dodal.devices.oav.pin_image_recognition import PinTipDetection | ||
from dodal.devices.robot import BartRobot, SampleLocation | ||
|
@@ -30,6 +31,7 @@ | |
from dodal.devices.xbpm_feedback import XBPMFeedback | ||
from dodal.devices.zebra import Zebra | ||
from dodal.devices.zocalo import ZocaloResults | ||
from dodal.plans.motor_util_plans import MoveTooLarge, home_and_reset_wrapper | ||
from ophyd_async.panda import HDFPanda | ||
|
||
from hyperion.device_setup_plans.utils import ( | ||
|
@@ -85,6 +87,7 @@ class RobotLoadThenCentreComposite: | |
# RobotLoad fields | ||
robot: BartRobot | ||
webcam: Webcam | ||
lower_gonio: XYZPositioner | ||
|
||
|
||
def create_devices(context: BlueskyContext) -> RobotLoadThenCentreComposite: | ||
|
@@ -146,6 +149,36 @@ def prepare_for_robot_load(composite: RobotLoadThenCentreComposite): | |
yield from bps.wait("prepare_robot_load") | ||
|
||
|
||
def robot_load_and_energy_change( | ||
composite: RobotLoadThenCentreComposite, | ||
sample_location: SampleLocation, | ||
demand_energy_ev: float | None, | ||
): | ||
yield from bps.abs_set( | ||
composite.robot, | ||
sample_location, | ||
group="robot_load", | ||
) | ||
|
||
if demand_energy_ev: | ||
yield from set_energy_plan( | ||
demand_energy_ev / 1000, | ||
cast(SetEnergyComposite, composite), | ||
) | ||
|
||
yield from bps.wait("robot_load") | ||
|
||
|
||
def raise_exception_if_moved_out_of_cryojet(exception): | ||
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. this is nice |
||
yield from bps.null() | ||
if isinstance(exception, MoveTooLarge): | ||
raise Exception( | ||
f"Moving {exception.axis} back to {exception.position} after \ | ||
robot load would move it out of the cryojet. The max safe \ | ||
distance is {exception.maximum_move}" | ||
) | ||
|
||
|
||
def robot_load_then_centre_plan( | ||
composite: RobotLoadThenCentreComposite, | ||
params: RobotLoadThenCentre, | ||
|
@@ -166,23 +199,32 @@ def robot_load_then_centre_plan( | |
], | ||
} | ||
) | ||
def robot_load(): | ||
def robot_load_and_snapshots(): | ||
# TODO: get these from one source of truth #1347 | ||
assert params.sample_puck is not None | ||
assert params.sample_pin is not None | ||
yield from bps.abs_set( | ||
composite.robot, | ||
|
||
robot_load_plan = robot_load_and_energy_change( | ||
composite, | ||
SampleLocation(params.sample_puck, params.sample_pin), | ||
group="robot_load", | ||
params.demand_energy_ev, | ||
) | ||
|
||
if params.demand_energy_ev: | ||
yield from set_energy_plan( | ||
params.demand_energy_ev / 1000, | ||
cast(SetEnergyComposite, composite), | ||
) | ||
|
||
yield from bps.wait("robot_load") | ||
# The lower gonio must be in the correct position for the robot load and we | ||
# want to put it back afterwards. Note we don't wait as we assume the lower | ||
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. would be good to add explanation of why this is safe here |
||
# gonio move is quicker than the robot takes to get to the load position. | ||
yield from bpp.contingency_wrapper( | ||
home_and_reset_wrapper( | ||
robot_load_plan, | ||
composite.lower_gonio, | ||
CONST.HARDWARE.LOWER_GONIO_ROBOT_LOAD_POSITION, | ||
BartRobot.LOAD_TOLERANCE_MM, | ||
CONST.HARDWARE.CRYOJET_MARGIN_MM, | ||
"lower_gonio", | ||
wait_for_all=False, | ||
), | ||
except_plan=raise_exception_if_moved_out_of_cryojet, | ||
) | ||
|
||
yield from bps.abs_set(composite.thawer.thaw_for_time_s, params.thawing_time) | ||
yield from wait_for_smargon_not_disabled(composite.smargon) | ||
|
@@ -197,7 +239,9 @@ def robot_load(): | |
yield from bps.read(composite.webcam) | ||
yield from bps.save() | ||
|
||
yield from robot_load() | ||
yield from bps.wait("reset-lower_gonio") | ||
|
||
yield from robot_load_and_snapshots() | ||
|
||
yield from pin_centre_then_xray_centre_plan( | ||
cast(GridDetectThenXRayCentreComposite, composite), | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
bit tangential, but does the set energy plan always have to block during robot load or could we wait on it later?
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.
No, we want https://github.com/DiamondLightSource/hyperion/issues/1495 really but needs the RE changes