Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
f209271
Add scint into beam + change Enum to be be more descriptive compared …
srishtysajeev Sep 24, 2025
1efa5fd
Get rid of Enum change as this has to match what the PV is expecting
srishtysajeev Sep 24, 2025
5b747e2
Add comments for what In and Out mean for the scint
srishtysajeev Sep 24, 2025
f61d124
small change
srishtysajeev Sep 26, 2025
542db18
add tests for scint in
srishtysajeev Sep 26, 2025
05453f9
get rid of comment!
srishtysajeev Sep 26, 2025
512a942
make changes to scintillator.py from comments
srishtysajeev Sep 26, 2025
aa826be
make changes from comments to test_scintillator.py
srishtysajeev Sep 26, 2025
ba13e02
fix typo
srishtysajeev Sep 26, 2025
9d71e54
make all functions hidden
srishtysajeev Sep 30, 2025
ec986c0
Change current_pos to a tuple because it was causing lint failing in …
srishtysajeev Sep 30, 2025
c5be74b
Add opencv docs
srishtysajeev Oct 30, 2025
2aaf02f
fix: add awaits
srishtysajeev Oct 31, 2025
fb5f26c
Better mock the aperture scatterguard
DominicOram Nov 6, 2025
01d6f02
fix: fix tests by changing expected type of current_pos in _check_pos…
srishtysajeev Nov 6, 2025
21b3eca
refactor: change error message so it's not specific to just out
srishtysajeev Nov 17, 2025
4ac32ae
fix: check if already In before attempting to move scint
srishtysajeev Nov 17, 2025
ebd1f6c
add test
srishtysajeev Nov 17, 2025
d4a74c1
fix: tests
srishtysajeev Nov 17, 2025
5fd13a6
Merge branch 'main' into 1567_move_scintillator_to_beam
DominicOram Nov 17, 2025
7d9e676
remove whitespace
srishtysajeev Nov 17, 2025
9479360
Merge branch 'main' into 1567_move_scintillator_to_beam
srishtysajeev Nov 17, 2025
78ba2f0
Merge branch 'main' into 1567_move_scintillator_to_beam
DominicOram Nov 17, 2025
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
6 changes: 6 additions & 0 deletions src/dodal/devices/oav/pin_image_recognition/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ class ScanDirections(Enum):
REVERSE = -1


"""
See https://opencv24-python-tutorials.readthedocs.io/en/latest/py_tutorials/py_imgproc/py_morphological_ops/py_morphological_ops.html
for description of functions below.
"""


def identity(*args, **kwargs) -> Callable[[np.ndarray], np.ndarray]:
return lambda arr: arr

Expand Down
50 changes: 36 additions & 14 deletions src/dodal/devices/scintillator.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@


class InOut(StrictEnum):
"""Currently Hyperion only needs to move the scintillator out for data collection."""
"""Moves scintillator in and out of the beam."""

OUT = "Out"
OUT = "Out" # Out of beam
IN = "In" # In to beam
UNKNOWN = "Unknown"


Expand Down Expand Up @@ -45,42 +46,63 @@ def __init__(
self._scintillator_out_yz_mm = [
float(beamline_parameters[f"scin_{axis}_SCIN_OUT"]) for axis in ("y", "z")
]
self._scintillator_in_yz_mm = [
float(beamline_parameters[f"scin_{axis}_SCIN_IN"]) for axis in ("y", "z")
]
self._yz_tolerance_mm = [
float(beamline_parameters[f"scin_{axis}_tolerance"]) for axis in ("y", "z")
]

super().__init__(name)

def _get_selected_position(self, y: float, z: float) -> InOut:
current_pos = [y, z]
if all(
def _check_position(self, current_pos: list[float], pos_to_check: list[float]):
return all(
isclose(axis_pos, axis_in_beam, abs_tol=axis_tolerance)
for axis_pos, axis_in_beam, axis_tolerance in zip(
current_pos,
self._scintillator_out_yz_mm,
pos_to_check,
self._yz_tolerance_mm,
strict=False,
)
):
)

def _get_selected_position(self, y: float, z: float) -> InOut:
current_pos = [y, z]
if self._check_position(current_pos, self._scintillator_out_yz_mm):
return InOut.OUT

elif self._check_position(current_pos, self._scintillator_in_yz_mm):
return InOut.IN

else:
return InOut.UNKNOWN

async def _check_aperture_parked(self):
if (
await self._aperture_scatterguard().selected_aperture.get_value()
!= ApertureValue.PARKED
):
raise ValueError(
f"Cannot move scintillator if aperture/scatterguard is not parked. Position is currently {await self._aperture_scatterguard().selected_aperture.get_value()}"
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is self._aperture_scatterguard().selected_aperture.get_value(), what we want here? I noticed that sometimes the error gives out Large or Small rather than an actual position - is that expected, and should I be testing that the correct error message is outputted?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's correct, the positions it can have are Large/Medium/Small

async def _set_selected_position(self, position: InOut) -> None:
match position:
case InOut.OUT:
current_y = await self.y_mm.user_readback.get_value()
current_z = await self.z_mm.user_readback.get_value()
if self._get_selected_position(current_y, current_z) == InOut.OUT:
return
if (
self._aperture_scatterguard().selected_aperture.get_value()
!= ApertureValue.PARKED
):
raise ValueError(
"Cannot move scintillator out if aperture/scatterguard is not parked"
)
await self._check_aperture_parked()
await self.y_mm.set(self._scintillator_out_yz_mm[0])
await self.z_mm.set(self._scintillator_out_yz_mm[1])
case InOut.IN:
current_y = await self.y_mm.user_readback.get_value()
current_z = await self.z_mm.user_readback.get_value()
if self._get_selected_position(current_y, current_z) == InOut.IN:
return
await self._check_aperture_parked()
await self.z_mm.set(self._scintillator_in_yz_mm[1])
await self.y_mm.set(self._scintillator_in_yz_mm[0])
case _:
raise ValueError(f"Cannot set scintillator to position {position}")
47 changes: 36 additions & 11 deletions tests/devices/test_scintillator.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
from collections.abc import AsyncGenerator
from contextlib import ExitStack
from unittest.mock import MagicMock
from unittest.mock import AsyncMock, MagicMock

import pytest
from ophyd_async.core import init_devices
from ophyd_async.testing import get_mock_put
from ophyd_async.testing import assert_value, get_mock_put

from dodal.common.beamlines.beamline_parameters import GDABeamlineParameters
from dodal.devices.aperturescatterguard import ApertureScatterguard, ApertureValue
Expand Down Expand Up @@ -32,6 +32,8 @@ async def scintillator_and_ap_sg(
) -> AsyncGenerator[tuple[Scintillator, MagicMock], None]:
async with init_devices(mock=True):
mock_ap_sg = MagicMock()
mock_ap_sg.return_value.selected_aperture.set = AsyncMock()
mock_ap_sg.return_value.selected_aperture.get_value = AsyncMock()
scintillator = Scintillator(
prefix="",
name="test_scin",
Expand All @@ -49,6 +51,7 @@ async def scintillator_and_ap_sg(
@pytest.mark.parametrize(
"y, z, expected_position",
[
(100.855, 101.5115, InOut.IN),
(-0.02, 0.1, InOut.OUT),
(0.1, 0.1, InOut.UNKNOWN),
(10.2, 15.6, InOut.UNKNOWN),
Expand Down Expand Up @@ -84,34 +87,56 @@ async def test_given_aperture_scatterguard_parked_when_set_to_out_position_then_

await scintillator.selected_pos.set(InOut.OUT)

assert await scintillator.y_mm.user_setpoint.get_value() == -0.02
assert await scintillator.z_mm.user_setpoint.get_value() == 0.1
await assert_value(scintillator.y_mm.user_setpoint, -0.02)
await assert_value(scintillator.z_mm.user_setpoint, 0.1)


async def test_given_aperture_scatterguard_not_parked_when_set_to_out_position_then_exception_raised(
async def test_given_aperture_scatterguard_parked_when_set_to_in_position_then_returns_expected(
scintillator_and_ap_sg: tuple[Scintillator, ApertureScatterguard],
):
scintillator, ap_sg = scintillator_and_ap_sg
ap_sg.return_value.selected_aperture.get_value.return_value = ApertureValue.PARKED # type: ignore

await scintillator.selected_pos.set(InOut.IN)

await assert_value(scintillator.y_mm.user_setpoint, 100.855)
await assert_value(scintillator.z_mm.user_setpoint, 101.5115)


@pytest.mark.parametrize("scint_pos", [InOut.OUT, InOut.IN])
async def test_given_aperture_scatterguard_not_parked_when_set_to_in_or_out_position_then_exception_raised(
scintillator_and_ap_sg: tuple[Scintillator, ApertureScatterguard], scint_pos
):
for position in ApertureValue:
if position != ApertureValue.PARKED:
scintillator, ap_sg = scintillator_and_ap_sg
ap_sg.return_value.selected_aperture.get_value.return_value = position # type: ignore

with pytest.raises(ValueError):
await scintillator.selected_pos.set(InOut.OUT)
await scintillator.selected_pos.set(scint_pos)


async def test_given_scintillator_already_out_when_moved_out_then_does_nothing(
@pytest.mark.parametrize(
"y, z, expected_position",
[
(100.855, 101.5115, InOut.IN),
(-0.02, 0.1, InOut.OUT),
],
)
async def test_given_scintillator_already_out_when_moved_in_or_out_then_does_nothing(
scintillator_and_ap_sg: tuple[Scintillator, ApertureScatterguard],
expected_position,
y,
z,
):
scintillator, ap_sg = scintillator_and_ap_sg
await scintillator.y_mm.set(0)
await scintillator.z_mm.set(0)
await scintillator.y_mm.set(y)
await scintillator.z_mm.set(z)

get_mock_put(scintillator.y_mm.user_setpoint).reset_mock()
get_mock_put(scintillator.z_mm.user_setpoint).reset_mock()

ap_sg.return_value.selected_aperture.get_value.return_value = ApertureValue.LARGE # type: ignore
await scintillator.selected_pos.set(InOut.OUT)
await scintillator.selected_pos.set(expected_position)

get_mock_put(scintillator.y_mm.user_setpoint).assert_not_called()
get_mock_put(scintillator.z_mm.user_setpoint).assert_not_called()