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

Hotfixes found from testing on the beamline #477

Merged
merged 7 commits into from
Sep 18, 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
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
from dodal.devices.webcam import Webcam
from dodal.devices.xbpm_feedback import XBPMFeedback
from dodal.devices.zebra import Zebra
from dodal.devices.zebra_controlled_shutter import ZebraShutter
from dodal.devices.zocalo import ZocaloResults
from dodal.plans.motor_util_plans import MoveTooLarge, home_and_reset_wrapper
from ophyd_async.fastcs.panda import HDFPanda
Expand Down Expand Up @@ -77,6 +78,7 @@ class RobotLoadThenCentreComposite:
panda: HDFPanda
panda_fast_grid_scan: PandAFastGridScan
thawer: Thawer
sample_shutter: ZebraShutter
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should use python protocols more then we wouldn't run into this kind of issue as much as it would be picked up by static analysis

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yh, we need a better way of managing it. If you have an idea can you add it to #287?


# SetEnergyComposite fields
vfm: FocusingMirrorWithStripes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,8 @@ def rotation_scan(
md={
"subplan_name": CONST.PLAN.ROTATION_OUTER,
CONST.TRIGGER.ZOCALO: CONST.PLAN.ROTATION_MAIN,
"hyperion_parameters": parameters.model_dump_json,
"zocalo_environment": parameters.zocalo_environment,
"hyperion_parameters": parameters.model_dump_json(),
"activate_callbacks": [
"RotationISPyBCallback",
"RotationNexusFileCallback",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,8 @@ def populate_info_for_update(
def activity_gated_stop(self, doc: RunStop) -> RunStop:
"""Subclasses must check that they are recieving a stop document for the correct
uid to use this method!"""
assert isinstance(
self.ispyb, StoreInIspyb
assert (
self.ispyb is not None
), "ISPyB handler received stop document, but deposition object doesn't exist!"
ISPYB_LOGGER.debug("ISPyB handler received stop document.")
exit_status = (
Expand All @@ -184,7 +184,7 @@ def activity_gated_stop(self, doc: RunStop) -> RunStop:
return self._tag_doc(doc)

def _append_to_comment(self, id: int, comment: str) -> None:
assert isinstance(self.ispyb, StoreInIspyb)
assert self.ispyb is not None
try:
self.ispyb.append_to_comment(id, comment)
except TypeError:
Expand Down
8 changes: 6 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,8 +293,7 @@ def smargon(RE: RunEngine) -> Generator[Smargon, None, None]:


@pytest.fixture
def zebra():
RunEngine()
def zebra(RE):
zebra = i03.zebra(fake_with_ophyd_sim=True)

def mock_side(*args, **kwargs):
Expand Down Expand Up @@ -691,6 +690,11 @@ def mock_gridscan_kickoff_complete(gridscan: FastGridScanCommon):
gridscan.complete = MagicMock(return_value=async_status_done)


@pytest.fixture
def panda_fast_grid_scan(RE):
return i03.panda_fast_grid_scan(fake_with_ophyd_sim=True)


@pytest.fixture
async def fake_fgs_composite(
smargon: Smargon,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@
rotation_scan,
rotation_scan_plan,
)
from mx_bluesky.hyperion.external_interaction.callbacks.rotation.ispyb_callback import (
RotationISPyBCallback,
)
from mx_bluesky.hyperion.external_interaction.callbacks.zocalo_callback import (
ZocaloCallback,
)
from mx_bluesky.hyperion.external_interaction.ispyb.ispyb_store import IspybIds
from mx_bluesky.hyperion.parameters.constants import CONST, DocDescriptorNames
from mx_bluesky.hyperion.parameters.rotation import RotationScan

Expand Down Expand Up @@ -592,3 +599,69 @@ def test_rotation_scan_arms_detector_and_takes_snapshots_whilst_arming(
lambda msg: msg.command == "wait"
and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC,
)


@patch(
"mx_bluesky.hyperion.external_interaction.callbacks.rotation.ispyb_callback.StoreInIspyb"
)
def test_rotation_scan_correctly_triggers_ispyb_callback(
mock_store_in_ispyb,
RE: RunEngine,
test_rotation_params: RotationScan,
fake_create_rotation_devices: RotationScanComposite,
oav_parameters_for_rotation: OAVParameters,
):
mock_ispyb_callback = RotationISPyBCallback()
RE.subscribe(mock_ispyb_callback)
with (
patch("bluesky.plan_stubs.wait", autospec=True),
patch(
"bluesky.preprocessors.__read_and_stash_a_motor",
fake_read,
),
):
RE(
rotation_scan(
fake_create_rotation_devices,
test_rotation_params,
oav_parameters_for_rotation,
),
)
mock_store_in_ispyb.assert_called()


@patch(
"mx_bluesky.hyperion.external_interaction.callbacks.zocalo_callback.ZocaloTrigger"
)
@patch(
"mx_bluesky.hyperion.external_interaction.callbacks.rotation.ispyb_callback.StoreInIspyb"
)
def test_rotation_scan_correctly_triggers_zocalo_callback(
mock_store_in_ispyb,
mock_zocalo_interactor,
RE: RunEngine,
test_rotation_params: RotationScan,
fake_create_rotation_devices: RotationScanComposite,
oav_parameters_for_rotation: OAVParameters,
):
mock_zocalo_callback = ZocaloCallback()
mock_ispyb_callback = RotationISPyBCallback(emit=mock_zocalo_callback)
mock_store_in_ispyb.return_value.update_deposition.return_value = IspybIds(
data_collection_ids=(0, 1)
)
RE.subscribe(mock_ispyb_callback)
with (
patch("bluesky.plan_stubs.wait", autospec=True),
patch(
"bluesky.preprocessors.__read_and_stash_a_motor",
fake_read,
),
):
RE(
rotation_scan(
fake_create_rotation_devices,
test_rotation_params,
oav_parameters_for_rotation,
),
)
mock_zocalo_interactor.return_value.run_start.assert_called_once()
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
from ophyd.sim import NullStatus
from ophyd_async.core import set_mock_value

from mx_bluesky.hyperion.experiment_plans.grid_detect_then_xray_centre_plan import (
GridDetectThenXRayCentreComposite,
)
from mx_bluesky.hyperion.experiment_plans.robot_load_then_centre_plan import (
RobotLoadThenCentreComposite,
prepare_for_robot_load,
Expand All @@ -32,21 +35,67 @@

@pytest.fixture
def robot_load_composite(
smargon, dcm, robot, aperture_scatterguard, oav, webcam, thawer, lower_gonio, eiger
smargon,
dcm,
robot,
aperture_scatterguard,
oav,
webcam,
thawer,
lower_gonio,
eiger,
xbpm_feedback,
flux,
zocalo,
panda,
backlight,
attenuator,
pin_tip,
fast_grid_scan,
detector_motion,
synchrotron,
s4_slit_gaps,
undulator,
zebra,
panda_fast_grid_scan,
vfm,
vfm_mirror_voltages,
undulator_dcm,
sample_shutter,
) -> RobotLoadThenCentreComposite:
composite: RobotLoadThenCentreComposite = MagicMock()
composite.smargon = smargon
composite.dcm = dcm
composite: RobotLoadThenCentreComposite = RobotLoadThenCentreComposite(
smargon=smargon,
dcm=dcm,
robot=robot,
aperture_scatterguard=aperture_scatterguard,
oav=oav,
webcam=webcam,
lower_gonio=lower_gonio,
thawer=thawer,
eiger=eiger,
xbpm_feedback=xbpm_feedback,
flux=flux,
zocalo=zocalo,
panda=panda,
backlight=backlight,
attenuator=attenuator,
pin_tip_detection=pin_tip,
zebra_fast_grid_scan=fast_grid_scan,
detector_motion=detector_motion,
synchrotron=synchrotron,
s4_slit_gaps=s4_slit_gaps,
undulator=undulator,
zebra=zebra,
panda_fast_grid_scan=panda_fast_grid_scan,
vfm=vfm,
vfm_mirror_voltages=vfm_mirror_voltages,
undulator_dcm=undulator_dcm,
sample_shutter=sample_shutter,
)
set_mock_value(composite.dcm.energy_in_kev.user_readback, 11.105)
composite.robot = robot
composite.aperture_scatterguard = aperture_scatterguard
composite.smargon.stub_offsets.set = MagicMock(return_value=NullStatus())
composite.aperture_scatterguard.set = MagicMock(return_value=NullStatus())
composite.oav = oav
composite.webcam = webcam
composite.lower_gonio = lower_gonio
composite.thawer = thawer
composite.eiger = eiger
return composite


Expand Down Expand Up @@ -89,6 +138,9 @@ def test_when_plan_run_then_centring_plan_run_with_expected_parameters(
for name, value in vars(composite_passed).items():
assert value == getattr(robot_load_composite, name)

for name in GridDetectThenXRayCentreComposite.__dataclass_fields__.keys():
assert getattr(composite_passed, name), f"{name} not in composite"

assert isinstance(params_passed, PinTipCentreThenXrayCentre)
assert params_passed.detector_params.expected_energy_ev == 11100

Expand Down
17 changes: 13 additions & 4 deletions utility_scripts/dls_dev_env.sh
Original file line number Diff line number Diff line change
@@ -1,9 +1,18 @@
#!/bin/bash

# Check we're in the right place
dir_name=${PWD##*/}
if [ "$dir_name" != "mx-bluesky" ]; then
echo "This script should be run from the 'mx-bluesky' directory"
# Check we're in the right place (assuming the location of this script in the repo)

# Get the directory where the script is located
script_dir=$(dirname "$(readlink -f "$0")")

# Get the current working directory
current_dir=$(pwd)

# Get the directory up from the script's location
two_levels_up=$(dirname "$script_dir")

if [ "$current_dir" != "$two_levels_up" ]; then
echo "This script should be run from the top directory of the repo"
exit 1
fi

Expand Down
Loading