Skip to content
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

Do not reload sample if its already been loaded #480

Merged
merged 5 commits into from
Sep 13, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ dependencies = [
"ophyd == 1.9.0",
"ophyd-async >= 0.3a5",
"bluesky >= 1.13.0a4",
"dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@5e8b6fc2933a2b2b2e788d432c8408317ac4a5cc",
"dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@638ab7a0c814ed3f61833e8135c28fd6903003f3",
]


Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
from __future__ import annotations

import dataclasses
from collections.abc import Generator
from datetime import datetime
from pathlib import Path
from typing import cast

import bluesky.plan_stubs as bps
import bluesky.preprocessors as bpp
from blueapi.core import BlueskyContext, MsgGenerator
from bluesky.utils import Msg
from dodal.devices.aperturescatterguard import ApertureScatterguard, ApertureValue
from dodal.devices.attenuator import Attenuator
from dodal.devices.backlight import Backlight
Expand Down Expand Up @@ -185,68 +187,90 @@ def raise_exception_if_moved_out_of_cryojet(exception):
)


def robot_load_then_centre_plan(
def _pin_already_loaded(
robot: BartRobot, pin_to_load: int, puck_to_load: int
) -> Generator[Msg, None, bool]:
current_puck = yield from bps.rd(robot.current_puck)
current_pin = yield from bps.rd(robot.current_pin)
return int(current_puck) == puck_to_load and int(current_pin) == pin_to_load


def robot_load_and_snapshots(
composite: RobotLoadThenCentreComposite,
params: RobotLoadThenCentre,
location: SampleLocation,
):
yield from prepare_for_robot_load(composite)

@bpp.run_decorator(
md={
"subplan_name": CONST.PLAN.ROBOT_LOAD,
"metadata": {
"visit_path": str(params.visit_directory),
"sample_id": params.sample_id,
"sample_puck": params.sample_puck,
"sample_pin": params.sample_pin,
},
"activate_callbacks": [
"RobotLoadISPyBCallback",
],
}
robot_load_plan = do_robot_load(
composite,
location,
params.demand_energy_ev,
params.thawing_time,
)
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

robot_load_plan = do_robot_load(
composite,
SampleLocation(params.sample_puck, params.sample_pin),
params.demand_energy_ev,
params.thawing_time,
)

# 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 the robot is interlocked
# to the lower gonio and the 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,
BartRobot.LOAD_TOLERANCE_MM,
CONST.HARDWARE.CRYOJET_MARGIN_MM,
"lower_gonio",
wait_for_all=False,
),
except_plan=raise_exception_if_moved_out_of_cryojet,
)
# 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 the robot is interlocked
# to the lower gonio and the 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,
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 take_robot_snapshots(
composite.oav, composite.webcam, params.snapshot_directory
)
yield from take_robot_snapshots(
composite.oav, composite.webcam, params.snapshot_directory
)

yield from bps.create(name=CONST.DESCRIPTORS.ROBOT_LOAD)
yield from bps.read(composite.robot.barcode)
yield from bps.read(composite.oav.snapshot)
yield from bps.read(composite.webcam)
yield from bps.save()
yield from bps.create(name=CONST.DESCRIPTORS.ROBOT_LOAD)
yield from bps.read(composite.robot.barcode)
yield from bps.read(composite.oav.snapshot)
yield from bps.read(composite.webcam)
yield from bps.save()

yield from bps.wait("reset-lower_gonio")
yield from bps.wait("reset-lower_gonio")

yield from robot_load_and_snapshots()

def robot_load_then_centre_plan(
composite: RobotLoadThenCentreComposite,
params: RobotLoadThenCentre,
):
# TODO: get these from one source of truth #254
assert params.sample_puck is not None
assert params.sample_pin is not None

if not (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should consider moving robot_load_and_snapshots() out of the function similar to prepare_for_robot_load() - it's quite long I think it's affecting readbility.

yield from _pin_already_loaded(
composite.robot, params.sample_pin, params.sample_puck
)
):
yield from prepare_for_robot_load(composite)
yield from bpp.run_wrapper(
robot_load_and_snapshots(
composite, params, SampleLocation(params.sample_puck, params.sample_pin)
),
md={
"subplan_name": CONST.PLAN.ROBOT_LOAD,
"metadata": {
"visit_path": str(params.visit_directory),
"sample_id": params.sample_id,
"sample_puck": params.sample_puck,
"sample_pin": params.sample_pin,
},
"activate_callbacks": [
"RobotLoadISPyBCallback",
],
},
)
else:
LOGGER.info(
f"Pin/puck {params.sample_pin}/{params.sample_puck} already loaded, will not reload."
)
yield from pin_centre_then_xray_centre_plan(
cast(GridDetectThenXRayCentreComposite, composite),
params.pin_centre_then_xray_centre_params(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
RobotLoadThenCentreComposite,
prepare_for_robot_load,
robot_load_then_centre,
robot_load_then_centre_plan,
take_robot_snapshots,
)
from mx_bluesky.hyperion.external_interaction.callbacks.robot_load.ispyb_callback import (
Expand Down Expand Up @@ -491,3 +492,35 @@ def test_when_plan_run_then_thawing_turned_on_for_expected_time(
and msg.obj.name == "thawer-thaw_for_time_s"
and msg.args[0] == thaw_time,
)


@patch(
"mx_bluesky.hyperion.experiment_plans.robot_load_then_centre_plan.pin_centre_then_xray_centre_plan"
)
@patch("mx_bluesky.hyperion.experiment_plans.robot_load_then_centre_plan.do_robot_load")
@patch(
"mx_bluesky.hyperion.experiment_plans.robot_load_then_centre_plan.prepare_for_robot_load"
)
def test_given_sample_already_loaded_when_plan_run_then_sample_not_loaded(
mock_prepare_robot: MagicMock,
mock_do_robot_load: MagicMock,
mock_centring_plan: MagicMock,
robot_load_composite: RobotLoadThenCentreComposite,
robot_load_then_centre_params_no_energy: RobotLoadThenCentre,
RE: RunEngine,
):
set_mock_value(robot_load_composite.robot.current_pin, 1)
set_mock_value(robot_load_composite.robot.current_puck, 2)

robot_load_then_centre_params_no_energy.sample_pin = 1
robot_load_then_centre_params_no_energy.sample_puck = 2

RE(
robot_load_then_centre_plan(
robot_load_composite,
robot_load_then_centre_params_no_energy,
)
)
mock_prepare_robot.assert_not_called()
mock_do_robot_load.assert_not_called()
mock_centring_plan.assert_called()
Loading