From b2d7b1bb9819a9a1aa79afc52cdf15d5ef09d127 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Wed, 16 Oct 2024 15:36:15 +0100 Subject: [PATCH 01/53] Set up skeleton of async mjpg --- src/dodal/devices/oav/snapshots/MJPG.py | 63 +++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 src/dodal/devices/oav/snapshots/MJPG.py diff --git a/src/dodal/devices/oav/snapshots/MJPG.py b/src/dodal/devices/oav/snapshots/MJPG.py new file mode 100644 index 0000000000..0cd4800fa8 --- /dev/null +++ b/src/dodal/devices/oav/snapshots/MJPG.py @@ -0,0 +1,63 @@ +from abc import ABC, abstractmethod +from pathlib import Path + +from bluesky.protocols import Triggerable +from ophyd_async.core import AsyncStatus, StandardReadable, soft_signal_rw +from ophyd_async.epics.signal import epics_signal_r, epics_signal_rw +from PIL import Image # , ImageDraw + +from dodal.log import LOGGER + + +class MJPG(StandardReadable, Triggerable, ABC): + """The MJPG areadetector plugin creates an MJPG video stream of the camera's output. + + This devices uses that stream to grab images. When it is triggered it will send the + latest image from the stream to the `post_processing` method for child classes to handle. + """ + + def __init__(self, prefix: str, name: str = "") -> None: + self.filename = soft_signal_rw(str) + self.directory = soft_signal_rw(str) + self.last_saved_path = soft_signal_rw(str) + + self.url = epics_signal_rw(str, prefix + "JPG_URL_RBV") + + self.x_size = epics_signal_r(int, prefix + "ArraySize1_RBV") + self.y_size = epics_signal_r(int, prefix + "ArraySize2_RBV") + + # TODO check type of these two + self.input_rbpv = epics_signal_r(str, prefix + "NDArrayPort_RBV") + self.input_plugin = epics_signal_rw(str, prefix + "NDArrayPort") + + # scaling factors for the snapshot at the time it was triggered + self.microns_per_pixel_x = soft_signal_rw(float) + self.microns_per_pixel_y = soft_signal_rw(float) + + # May not need this one + # self.oav_params = None (OAVConfig) + + self.KICKOFF_TIMEOUT = 30.0 + + super().__init__(name) + + async def _save_image(self, image: Image.Image): + filename_str = await self.filename.get_value() + directory_str = await self.directory.get_value() + + path = Path(f"{directory_str}/{filename_str}.png").as_posix() + if not Path(directory_str).is_dir(): + LOGGER.info(f"Snapshot folder {directory_str} does not exist, creating...") + Path(directory_str).mkdir(parents=True) + + LOGGER.info(f"Saving image to {path}") + image.save(path) + await self.last_saved_path.set(path, wait=True) + + @AsyncStatus.wrap + async def trigger(self): + pass + + @abstractmethod + def post_processing(self, image: Image.Image): + pass From bf59ccebfac498ba07b0a7b2bc9964189c94098e Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Wed, 16 Oct 2024 16:07:22 +0100 Subject: [PATCH 02/53] Start filling things in --- src/dodal/devices/oav/snapshots/MJPG.py | 39 ++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/src/dodal/devices/oav/snapshots/MJPG.py b/src/dodal/devices/oav/snapshots/MJPG.py index 0cd4800fa8..aa374a1e55 100644 --- a/src/dodal/devices/oav/snapshots/MJPG.py +++ b/src/dodal/devices/oav/snapshots/MJPG.py @@ -1,8 +1,16 @@ +import threading from abc import ABC, abstractmethod +from io import BytesIO from pathlib import Path +import requests from bluesky.protocols import Triggerable -from ophyd_async.core import AsyncStatus, StandardReadable, soft_signal_rw +from ophyd_async.core import ( + AsyncStatus, + StandardReadable, + completed_status, + soft_signal_rw, +) from ophyd_async.epics.signal import epics_signal_r, epics_signal_rw from PIL import Image # , ImageDraw @@ -42,6 +50,10 @@ def __init__(self, prefix: str, name: str = "") -> None: super().__init__(name) async def _save_image(self, image: Image.Image): + """A helper function to save a given image to the path supplied by the \ + directory and filename signals. The full resultant path is put on the \ + last_saved_path signal + """ filename_str = await self.filename.get_value() directory_str = await self.directory.get_value() @@ -56,6 +68,31 @@ async def _save_image(self, image: Image.Image): @AsyncStatus.wrap async def trigger(self): + """This takes a snapshot image from the MJPG stream and send it to the + post_processing method, expected to be implemented by a child of this class. + + It is the responsibility of the child class to save any resulting images. + """ + # TODO Figure out status!!! + # status = AsyncStatus(self) + url_str = await self.url.get_value() + + # For now, let's assume I set microns_per_pixels in the oav from those values + def get_snapshot(): + try: + response = requests.get(url_str, stream=True) + response.raise_for_status() + with Image.open(BytesIO(response.content)) as image: + self.post_processing(image) + completed_status() + # st.set_finished() + except requests.HTTPError as e: + print(e) + # st.set_exception(e) + + threading.Thread(target=get_snapshot, daemon=True).start() + + # return st pass @abstractmethod From 579bac1a3562cd577319c75919eeecca4a9c5208 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Wed, 16 Oct 2024 18:28:35 +0100 Subject: [PATCH 03/53] Try using completed_status --- src/dodal/devices/oav/snapshots/MJPG.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/dodal/devices/oav/snapshots/MJPG.py b/src/dodal/devices/oav/snapshots/MJPG.py index aa374a1e55..c4a7147669 100644 --- a/src/dodal/devices/oav/snapshots/MJPG.py +++ b/src/dodal/devices/oav/snapshots/MJPG.py @@ -87,13 +87,12 @@ def get_snapshot(): completed_status() # st.set_finished() except requests.HTTPError as e: - print(e) + completed_status(exception=e) # st.set_exception(e) threading.Thread(target=get_snapshot, daemon=True).start() # return st - pass @abstractmethod def post_processing(self, image: Image.Image): From 67e774c875d0fb1ca431272c9db4b883e466e5c9 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Wed, 16 Oct 2024 19:22:33 +0100 Subject: [PATCH 04/53] Add SnapshotWithBeamCentre --- src/dodal/devices/oav/snapshots/MJPG.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/dodal/devices/oav/snapshots/MJPG.py b/src/dodal/devices/oav/snapshots/MJPG.py index c4a7147669..61bf27fd0b 100644 --- a/src/dodal/devices/oav/snapshots/MJPG.py +++ b/src/dodal/devices/oav/snapshots/MJPG.py @@ -1,3 +1,4 @@ +import asyncio import threading from abc import ABC, abstractmethod from io import BytesIO @@ -12,7 +13,7 @@ soft_signal_rw, ) from ophyd_async.epics.signal import epics_signal_r, epics_signal_rw -from PIL import Image # , ImageDraw +from PIL import Image from dodal.log import LOGGER @@ -83,7 +84,7 @@ def get_snapshot(): response = requests.get(url_str, stream=True) response.raise_for_status() with Image.open(BytesIO(response.content)) as image: - self.post_processing(image) + asyncio.run(self.post_processing(image)) completed_status() # st.set_finished() except requests.HTTPError as e: @@ -95,5 +96,5 @@ def get_snapshot(): # return st @abstractmethod - def post_processing(self, image: Image.Image): + async def post_processing(self, image: Image.Image): pass From dd0e7079a592a505ab2d0dfcd44e58e1f9389037 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Wed, 16 Oct 2024 19:22:47 +0100 Subject: [PATCH 05/53] Add SnapshotWithBeamCentre --- src/dodal/devices/oav/snapshots/snapshot.py | 57 +++++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 src/dodal/devices/oav/snapshots/snapshot.py diff --git a/src/dodal/devices/oav/snapshots/snapshot.py b/src/dodal/devices/oav/snapshots/snapshot.py new file mode 100644 index 0000000000..7b3a7c7a77 --- /dev/null +++ b/src/dodal/devices/oav/snapshots/snapshot.py @@ -0,0 +1,57 @@ +from ophyd_async.core import soft_signal_rw +from PIL import Image, ImageDraw + +from dodal.devices.oav.snapshots.MJPG import MJPG + + +class SnapshotWithBeamCentre(MJPG): + """A child of MJPG which, when triggered, draws an outlined crosshair at the beam + centre in the image and saves the image to disk.""" + + CROSSHAIR_LENGTH_PX = 20 + CROSSHAIR_OUTLINE_COLOUR = "Black" + CROSSHAIR_FILL_COLOUR = "White" + + def __init__(self, prefix: str, name: str = "") -> None: + self.beam_centre_i = soft_signal_rw(int) + self.beam_centre_j = soft_signal_rw(int) + super().__init__(prefix, name) + + async def post_processing(self, image: Image.Image): + beam_x = await self.beam_centre_i.get_value() + beam_y = await self.beam_centre_j.get_value() + SnapshotWithBeamCentre.draw_crosshair(image, beam_x, beam_y) + + await self._save_image(image) + + @classmethod + def draw_crosshair(cls, image: Image.Image, beam_x: int, beam_y: int): + draw = ImageDraw.Draw(image) + OUTLINE_WIDTH = 1 + HALF_LEN = cls.CROSSHAIR_LENGTH_PX / 2 + draw.rectangle( + [ + beam_x - OUTLINE_WIDTH, + beam_y - HALF_LEN - OUTLINE_WIDTH, + beam_x + OUTLINE_WIDTH, + beam_y + HALF_LEN + OUTLINE_WIDTH, + ], + fill=cls.CROSSHAIR_OUTLINE_COLOUR, + ) + draw.rectangle( + [ + beam_x - HALF_LEN - OUTLINE_WIDTH, + beam_y - OUTLINE_WIDTH, + beam_x + HALF_LEN + OUTLINE_WIDTH, + beam_y + OUTLINE_WIDTH, + ], + fill=cls.CROSSHAIR_OUTLINE_COLOUR, + ) + draw.line( + ((beam_x, beam_y - HALF_LEN), (beam_x, beam_y + HALF_LEN)), + fill=cls.CROSSHAIR_FILL_COLOUR, + ) + draw.line( + ((beam_x - HALF_LEN, beam_y), (beam_x + HALF_LEN, beam_y)), + fill=cls.CROSSHAIR_FILL_COLOUR, + ) From 2a1ed7fe7086bd886395891d784c323936d4e17e Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Wed, 16 Oct 2024 19:30:49 +0100 Subject: [PATCH 06/53] A first try at adding snapshot to oav --- src/dodal/devices/oav/oav_async.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/dodal/devices/oav/oav_async.py b/src/dodal/devices/oav/oav_async.py index bc7f12962f..e93e0f88d1 100644 --- a/src/dodal/devices/oav/oav_async.py +++ b/src/dodal/devices/oav/oav_async.py @@ -8,6 +8,7 @@ from dodal.common.signal_utils import create_hardware_backed_soft_signal from dodal.devices.oav.oav_parameters import DEFAULT_OAV_WINDOW, OAVConfig +from dodal.devices.oav.snapshots.snapshot import SnapshotWithBeamCentre class Coords(IntEnum): @@ -47,6 +48,7 @@ async def set(self, level_to_set: str): class OAV(StandardReadable): def __init__(self, prefix: str, config: OAVConfig, name: str = ""): + self.snapshot = SnapshotWithBeamCentre(f"{prefix}-DI-OAV-01:MJPG:", name) _bl_prefix = prefix.split("-")[0] self.zoom_controller = ZoomController(f"{_bl_prefix}-EA-OAV-01:FZOOM:", name) @@ -75,8 +77,16 @@ def __init__(self, prefix: str, config: OAVConfig, name: str = ""): int, lambda: self._get_beam_position(Coords.Y) ) + self._set_up_snapshot() + super().__init__(name) + def _set_up_snapshot(self): + self.snapshot.microns_per_pixel_x = self.microns_per_pixel_x + self.snapshot.microns_per_pixel_y = self.microns_per_pixel_y + self.snapshot.beam_centre_i = self.beam_centre_i + self.snapshot.beam_centre_j = self.beam_centre_j + async def _read_current_zoom(self) -> str: _zoom = await self.zoom_controller.level.get_value() return _get_correct_zoom_string(_zoom) From 75e83b55cae84a219e27e75b6244d6ee36fd7833 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Thu, 17 Oct 2024 10:51:44 +0100 Subject: [PATCH 07/53] Rename --- src/dodal/devices/oav/oav_async.py | 2 +- .../oav/snapshots/{snapshot.py => snapshot_with_beam_centre.py} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename src/dodal/devices/oav/snapshots/{snapshot.py => snapshot_with_beam_centre.py} (100%) diff --git a/src/dodal/devices/oav/oav_async.py b/src/dodal/devices/oav/oav_async.py index e93e0f88d1..d69987d3ee 100644 --- a/src/dodal/devices/oav/oav_async.py +++ b/src/dodal/devices/oav/oav_async.py @@ -8,7 +8,7 @@ from dodal.common.signal_utils import create_hardware_backed_soft_signal from dodal.devices.oav.oav_parameters import DEFAULT_OAV_WINDOW, OAVConfig -from dodal.devices.oav.snapshots.snapshot import SnapshotWithBeamCentre +from dodal.devices.oav.snapshots.snapshot_with_beam_centre import SnapshotWithBeamCentre class Coords(IntEnum): diff --git a/src/dodal/devices/oav/snapshots/snapshot.py b/src/dodal/devices/oav/snapshots/snapshot_with_beam_centre.py similarity index 100% rename from src/dodal/devices/oav/snapshots/snapshot.py rename to src/dodal/devices/oav/snapshots/snapshot_with_beam_centre.py From e47699b0bc10d40698a83a7ec729e280c32829a7 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Thu, 17 Oct 2024 11:22:03 +0100 Subject: [PATCH 08/53] Start adding async grid overlay --- src/dodal/devices/oav/oav_async.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/dodal/devices/oav/oav_async.py b/src/dodal/devices/oav/oav_async.py index d69987d3ee..02d2a2a791 100644 --- a/src/dodal/devices/oav/oav_async.py +++ b/src/dodal/devices/oav/oav_async.py @@ -9,6 +9,7 @@ from dodal.common.signal_utils import create_hardware_backed_soft_signal from dodal.devices.oav.oav_parameters import DEFAULT_OAV_WINDOW, OAVConfig from dodal.devices.oav.snapshots.snapshot_with_beam_centre import SnapshotWithBeamCentre +from dodal.devices.oav.snapshots.snapshot_with_grid import SnapshotWithGrid class Coords(IntEnum): @@ -49,6 +50,7 @@ async def set(self, level_to_set: str): class OAV(StandardReadable): def __init__(self, prefix: str, config: OAVConfig, name: str = ""): self.snapshot = SnapshotWithBeamCentre(f"{prefix}-DI-OAV-01:MJPG:", name) + self.grid_snapshot = SnapshotWithGrid(f"{prefix}-DI-OAV-01:MJPG:", name) _bl_prefix = prefix.split("-")[0] self.zoom_controller = ZoomController(f"{_bl_prefix}-EA-OAV-01:FZOOM:", name) @@ -77,15 +79,16 @@ def __init__(self, prefix: str, config: OAVConfig, name: str = ""): int, lambda: self._get_beam_position(Coords.Y) ) - self._set_up_snapshot() + await self._set_up_snapshots() super().__init__(name) - def _set_up_snapshot(self): - self.snapshot.microns_per_pixel_x = self.microns_per_pixel_x - self.snapshot.microns_per_pixel_y = self.microns_per_pixel_y - self.snapshot.beam_centre_i = self.beam_centre_i - self.snapshot.beam_centre_j = self.beam_centre_j + async def _set_up_snapshots(self): + for snapshot in [self.snapshot, self.grid_snapshot]: + await snapshot.microns_per_pixel_x.set(self.microns_per_pixel_x, wait=True) + await snapshot.microns_per_pixel_y.set(self.microns_per_pixel_y, wait=True) + await snapshot.beam_centre_i.set(self.beam_centre_i, wait=True) + await snapshot.beam_centre_j.set(self.beam_centre_j, wait=True) async def _read_current_zoom(self) -> str: _zoom = await self.zoom_controller.level.get_value() From 6d28b69f500d7813d3df2c3b6ce0495e995c84ee Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Thu, 17 Oct 2024 11:22:53 +0100 Subject: [PATCH 09/53] Actually add file --- .../oav/snapshots/snapshot_with_grid.py | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 src/dodal/devices/oav/snapshots/snapshot_with_grid.py diff --git a/src/dodal/devices/oav/snapshots/snapshot_with_grid.py b/src/dodal/devices/oav/snapshots/snapshot_with_grid.py new file mode 100644 index 0000000000..865a970217 --- /dev/null +++ b/src/dodal/devices/oav/snapshots/snapshot_with_grid.py @@ -0,0 +1,56 @@ +from os.path import join as path_join + +from ophyd_async.core import soft_signal_rw +from PIL.Image import Image + +from dodal.devices.oav.grid_overlay import ( + add_grid_border_overlay_to_image, + add_grid_overlay_to_image, +) +from dodal.devices.oav.snapshots.MJPG import MJPG +from dodal.log import LOGGER + + +class SnapshotWithGrid(MJPG): + def __init__(self, prefix: str, name: str = "") -> None: + super().__init__(prefix, name) + + self.top_left_x = soft_signal_rw(int) + self.top_left_y = soft_signal_rw(int) + self.box_width = soft_signal_rw(int) + self.num_boxes_x = soft_signal_rw(int) + self.num_boxes_y = soft_signal_rw(int) + + self.last_path_outer = soft_signal_rw(str) + self.last_path_full_overlay = soft_signal_rw(str) + + async def post_processing(self, image: Image): + # Save an unmodified image with no suffix + await self._save_image(image) + + top_left_x = await self.top_left_x.get_value() + top_left_y = await self.top_left_y.get_value() + box_witdh = await self.box_width.get_value() + num_boxes_x = await self.num_boxes_x.get_value() + num_boxes_y = await self.num_boxes_y.get_value() + + assert isinstance(filename_str := await self.filename.get_value(), str) + assert isinstance(directory_str := await self.directory.get_value(), str) + + add_grid_border_overlay_to_image( + image, top_left_x, top_left_y, box_witdh, num_boxes_x, num_boxes_y + ) + + path = path_join(directory_str, f"{filename_str}_outer_overlay.png") + await self.last_path_outer.set(path, wait=True) + LOGGER.info(f"Saving grid outer edge at {path}") + image.save(path) + + add_grid_overlay_to_image( + image, top_left_x, top_left_y, box_witdh, num_boxes_x, num_boxes_y + ) + + path = path_join(directory_str, f"{filename_str}_grid_overlay.png") + await self.last_path_full_overlay.set(path, wait=True) + LOGGER.info(f"Saving full grid overlay at {path}") + image.save(path) From 866addbb0dd05253ef4a08ad4265af38ad070226 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Thu, 17 Oct 2024 16:11:19 +0100 Subject: [PATCH 10/53] Start adding a test - might be deleted --- src/dodal/devices/oav/oav_async.py | 7 +++---- tests/devices/unit_tests/oav/test_oav_async.py | 12 ++++++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/dodal/devices/oav/oav_async.py b/src/dodal/devices/oav/oav_async.py index 02d2a2a791..af99e48f63 100644 --- a/src/dodal/devices/oav/oav_async.py +++ b/src/dodal/devices/oav/oav_async.py @@ -4,7 +4,7 @@ AsyncStatus, StandardReadable, ) -from ophyd_async.epics.signal import epics_signal_r, epics_signal_rw +from ophyd_async.epics.signal import epics_signal_rw from dodal.common.signal_utils import create_hardware_backed_soft_signal from dodal.devices.oav.oav_parameters import DEFAULT_OAV_WINDOW, OAVConfig @@ -54,9 +54,8 @@ def __init__(self, prefix: str, config: OAVConfig, name: str = ""): _bl_prefix = prefix.split("-")[0] self.zoom_controller = ZoomController(f"{_bl_prefix}-EA-OAV-01:FZOOM:", name) - # TODO See https://github.com/DiamondLightSource/dodal/issues/824 - self.x_size = epics_signal_r(int, prefix + "CAM:ArraySizeX_RBV") - self.y_size = epics_signal_r(int, prefix + "CAM:ArraySizeY_RBV") + self.x_size = await self.grid_snapshot.x_size.get_value() + self.y_size = await self.grid_snapshot.y_size.get_value() self.sizes = [self.x_size, self.y_size] diff --git a/tests/devices/unit_tests/oav/test_oav_async.py b/tests/devices/unit_tests/oav/test_oav_async.py index fe31680b8b..0669b3848f 100644 --- a/tests/devices/unit_tests/oav/test_oav_async.py +++ b/tests/devices/unit_tests/oav/test_oav_async.py @@ -13,8 +13,8 @@ async def oav() -> OAV: oav_config = OAVConfig(ZOOM_LEVELS_XML, DISPLAY_CONFIGURATION) async with DeviceCollector(mock=True, connect=True): oav = OAV("", config=oav_config, name="fake_oav") - set_mock_value(oav.x_size, 1024) - set_mock_value(oav.y_size, 768) + set_mock_value(oav.grid_snapshot.x_size, 1024) + set_mock_value(oav.grid_snapshot.y_size, 768) return oav @@ -68,8 +68,8 @@ async def test_extract_beam_position_given_different_zoom_levels( async def test_oav_returns_rescaled_beam_position_and_microns_per_pixel_correctly( oav: OAV, ): - set_mock_value(oav.x_size, 1292) - set_mock_value(oav.y_size, 964) + set_mock_value(oav.grid_snapshot.x_size, 1292) + set_mock_value(oav.grid_snapshot.y_size, 964) set_mock_value(oav.zoom_controller.level, "1.0") @@ -99,3 +99,7 @@ async def test_calculate_beam_distance(h, v, expected_x, expected_y, oav: OAV): h, v, ) == (expected_x, expected_y) + + +async def test_when_oav_created_microns_and_beam_centre_set_in_snapshots(oav: OAV): + pass From 07ffa05c596f0b7f8cc01451bdc248ee8ffc3f1a Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Thu, 17 Oct 2024 18:19:35 +0100 Subject: [PATCH 11/53] Move mjpg to plugins and get trigger working with async --- .../plugins/MJPG_async.py} | 38 ++++++------------- 1 file changed, 11 insertions(+), 27 deletions(-) rename src/dodal/devices/{oav/snapshots/MJPG.py => areadetector/plugins/MJPG_async.py} (75%) diff --git a/src/dodal/devices/oav/snapshots/MJPG.py b/src/dodal/devices/areadetector/plugins/MJPG_async.py similarity index 75% rename from src/dodal/devices/oav/snapshots/MJPG.py rename to src/dodal/devices/areadetector/plugins/MJPG_async.py index 61bf27fd0b..8d863ac041 100644 --- a/src/dodal/devices/oav/snapshots/MJPG.py +++ b/src/dodal/devices/areadetector/plugins/MJPG_async.py @@ -1,17 +1,10 @@ -import asyncio -import threading from abc import ABC, abstractmethod from io import BytesIO from pathlib import Path -import requests +from aiohttp import ClientSession from bluesky.protocols import Triggerable -from ophyd_async.core import ( - AsyncStatus, - StandardReadable, - completed_status, - soft_signal_rw, -) +from ophyd_async.core import AsyncStatus, StandardReadable, soft_signal_rw from ophyd_async.epics.signal import epics_signal_r, epics_signal_rw from PIL import Image @@ -74,26 +67,17 @@ async def trigger(self): It is the responsibility of the child class to save any resulting images. """ - # TODO Figure out status!!! - # status = AsyncStatus(self) url_str = await self.url.get_value() - # For now, let's assume I set microns_per_pixels in the oav from those values - def get_snapshot(): - try: - response = requests.get(url_str, stream=True) - response.raise_for_status() - with Image.open(BytesIO(response.content)) as image: - asyncio.run(self.post_processing(image)) - completed_status() - # st.set_finished() - except requests.HTTPError as e: - completed_status(exception=e) - # st.set_exception(e) - - threading.Thread(target=get_snapshot, daemon=True).start() - - # return st + async with ClientSession() as session: + async with session.get(url_str) as response: + try: + response.raise_for_status() + data = await response.read() + with Image.open(BytesIO(data)) as image: + await self.post_processing(image) + except Exception as e: + LOGGER.warning(f"Failed to create snapshot. \n {e}") @abstractmethod async def post_processing(self, image: Image.Image): From 4d9c4857515fec52c3a4a95fe9bad9661fb18632 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Thu, 17 Oct 2024 18:20:22 +0100 Subject: [PATCH 12/53] Remove microns per pixels from mjpg --- src/dodal/devices/areadetector/plugins/MJPG_async.py | 7 ------- src/dodal/devices/oav/oav_async.py | 7 +------ 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/src/dodal/devices/areadetector/plugins/MJPG_async.py b/src/dodal/devices/areadetector/plugins/MJPG_async.py index 8d863ac041..e2712007f8 100644 --- a/src/dodal/devices/areadetector/plugins/MJPG_async.py +++ b/src/dodal/devices/areadetector/plugins/MJPG_async.py @@ -32,13 +32,6 @@ def __init__(self, prefix: str, name: str = "") -> None: self.input_rbpv = epics_signal_r(str, prefix + "NDArrayPort_RBV") self.input_plugin = epics_signal_rw(str, prefix + "NDArrayPort") - # scaling factors for the snapshot at the time it was triggered - self.microns_per_pixel_x = soft_signal_rw(float) - self.microns_per_pixel_y = soft_signal_rw(float) - - # May not need this one - # self.oav_params = None (OAVConfig) - self.KICKOFF_TIMEOUT = 30.0 super().__init__(name) diff --git a/src/dodal/devices/oav/oav_async.py b/src/dodal/devices/oav/oav_async.py index af99e48f63..310ac4588e 100644 --- a/src/dodal/devices/oav/oav_async.py +++ b/src/dodal/devices/oav/oav_async.py @@ -1,9 +1,6 @@ from enum import IntEnum -from ophyd_async.core import ( - AsyncStatus, - StandardReadable, -) +from ophyd_async.core import AsyncStatus, StandardReadable from ophyd_async.epics.signal import epics_signal_rw from dodal.common.signal_utils import create_hardware_backed_soft_signal @@ -84,8 +81,6 @@ def __init__(self, prefix: str, config: OAVConfig, name: str = ""): async def _set_up_snapshots(self): for snapshot in [self.snapshot, self.grid_snapshot]: - await snapshot.microns_per_pixel_x.set(self.microns_per_pixel_x, wait=True) - await snapshot.microns_per_pixel_y.set(self.microns_per_pixel_y, wait=True) await snapshot.beam_centre_i.set(self.beam_centre_i, wait=True) await snapshot.beam_centre_j.set(self.beam_centre_j, wait=True) From 76eecae52d3872fafcf98eb8728ead0617ea0eb6 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Thu, 17 Oct 2024 18:25:15 +0100 Subject: [PATCH 13/53] Pass an instance of soft signal beam_centre and fix imports --- src/dodal/devices/oav/oav_async.py | 12 ++++-------- .../oav/snapshots/snapshot_with_beam_centre.py | 16 +++++++++++----- .../devices/oav/snapshots/snapshot_with_grid.py | 2 +- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/dodal/devices/oav/oav_async.py b/src/dodal/devices/oav/oav_async.py index 310ac4588e..c91292fc88 100644 --- a/src/dodal/devices/oav/oav_async.py +++ b/src/dodal/devices/oav/oav_async.py @@ -46,8 +46,6 @@ async def set(self, level_to_set: str): class OAV(StandardReadable): def __init__(self, prefix: str, config: OAVConfig, name: str = ""): - self.snapshot = SnapshotWithBeamCentre(f"{prefix}-DI-OAV-01:MJPG:", name) - self.grid_snapshot = SnapshotWithGrid(f"{prefix}-DI-OAV-01:MJPG:", name) _bl_prefix = prefix.split("-")[0] self.zoom_controller = ZoomController(f"{_bl_prefix}-EA-OAV-01:FZOOM:", name) @@ -75,15 +73,13 @@ def __init__(self, prefix: str, config: OAVConfig, name: str = ""): int, lambda: self._get_beam_position(Coords.Y) ) - await self._set_up_snapshots() + self.snapshot = SnapshotWithBeamCentre( + f"{prefix}-DI-OAV-01:MJPG:", self.beam_centre_i, self.beam_centre_j, name + ) + self.grid_snapshot = SnapshotWithGrid(f"{prefix}-DI-OAV-01:MJPG:", name) super().__init__(name) - async def _set_up_snapshots(self): - for snapshot in [self.snapshot, self.grid_snapshot]: - await snapshot.beam_centre_i.set(self.beam_centre_i, wait=True) - await snapshot.beam_centre_j.set(self.beam_centre_j, wait=True) - async def _read_current_zoom(self) -> str: _zoom = await self.zoom_controller.level.get_value() return _get_correct_zoom_string(_zoom) diff --git a/src/dodal/devices/oav/snapshots/snapshot_with_beam_centre.py b/src/dodal/devices/oav/snapshots/snapshot_with_beam_centre.py index 7b3a7c7a77..e29e40f876 100644 --- a/src/dodal/devices/oav/snapshots/snapshot_with_beam_centre.py +++ b/src/dodal/devices/oav/snapshots/snapshot_with_beam_centre.py @@ -1,7 +1,7 @@ -from ophyd_async.core import soft_signal_rw +from ophyd_async.core import SignalRW from PIL import Image, ImageDraw -from dodal.devices.oav.snapshots.MJPG import MJPG +from dodal.devices.areadetector.plugins.MJPG_async import MJPG class SnapshotWithBeamCentre(MJPG): @@ -12,9 +12,15 @@ class SnapshotWithBeamCentre(MJPG): CROSSHAIR_OUTLINE_COLOUR = "Black" CROSSHAIR_FILL_COLOUR = "White" - def __init__(self, prefix: str, name: str = "") -> None: - self.beam_centre_i = soft_signal_rw(int) - self.beam_centre_j = soft_signal_rw(int) + def __init__( + self, + prefix: str, + beam_x_signal: SignalRW, + beam_y_signal: SignalRW, + name: str = "", + ) -> None: + self.beam_centre_i = beam_x_signal + self.beam_centre_j = beam_y_signal super().__init__(prefix, name) async def post_processing(self, image: Image.Image): diff --git a/src/dodal/devices/oav/snapshots/snapshot_with_grid.py b/src/dodal/devices/oav/snapshots/snapshot_with_grid.py index 865a970217..6ca430d486 100644 --- a/src/dodal/devices/oav/snapshots/snapshot_with_grid.py +++ b/src/dodal/devices/oav/snapshots/snapshot_with_grid.py @@ -3,11 +3,11 @@ from ophyd_async.core import soft_signal_rw from PIL.Image import Image +from dodal.devices.areadetector.plugins.MJPG_async import MJPG from dodal.devices.oav.grid_overlay import ( add_grid_border_overlay_to_image, add_grid_overlay_to_image, ) -from dodal.devices.oav.snapshots.MJPG import MJPG from dodal.log import LOGGER From 3553bfc3bf007eac51f775ea0947f42f3e4fe936 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Thu, 17 Oct 2024 19:06:10 +0100 Subject: [PATCH 14/53] Fix some typing and signals --- src/dodal/devices/oav/oav_async.py | 5 +---- .../devices/oav/snapshots/snapshot_with_beam_centre.py | 6 +++--- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/dodal/devices/oav/oav_async.py b/src/dodal/devices/oav/oav_async.py index c91292fc88..50222d3d38 100644 --- a/src/dodal/devices/oav/oav_async.py +++ b/src/dodal/devices/oav/oav_async.py @@ -49,10 +49,7 @@ def __init__(self, prefix: str, config: OAVConfig, name: str = ""): _bl_prefix = prefix.split("-")[0] self.zoom_controller = ZoomController(f"{_bl_prefix}-EA-OAV-01:FZOOM:", name) - self.x_size = await self.grid_snapshot.x_size.get_value() - self.y_size = await self.grid_snapshot.y_size.get_value() - - self.sizes = [self.x_size, self.y_size] + self.sizes = [self.grid_snapshot.x_size, self.grid_snapshot.y_size] self.parameters = config.get_parameters() diff --git a/src/dodal/devices/oav/snapshots/snapshot_with_beam_centre.py b/src/dodal/devices/oav/snapshots/snapshot_with_beam_centre.py index e29e40f876..a96318459e 100644 --- a/src/dodal/devices/oav/snapshots/snapshot_with_beam_centre.py +++ b/src/dodal/devices/oav/snapshots/snapshot_with_beam_centre.py @@ -1,4 +1,4 @@ -from ophyd_async.core import SignalRW +from ophyd_async.core import SignalR from PIL import Image, ImageDraw from dodal.devices.areadetector.plugins.MJPG_async import MJPG @@ -15,8 +15,8 @@ class SnapshotWithBeamCentre(MJPG): def __init__( self, prefix: str, - beam_x_signal: SignalRW, - beam_y_signal: SignalRW, + beam_x_signal: SignalR, + beam_y_signal: SignalR, name: str = "", ) -> None: self.beam_centre_i = beam_x_signal From a15765c36f9fd085c35388cb7b90eaf2ab7ea8d1 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Fri, 18 Oct 2024 11:16:44 +0100 Subject: [PATCH 15/53] A bunch of fixmes --- src/dodal/devices/areadetector/plugins/MJPG_async.py | 6 +++++- tests/devices/unit_tests/oav/test_oav_async.py | 4 ---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/dodal/devices/areadetector/plugins/MJPG_async.py b/src/dodal/devices/areadetector/plugins/MJPG_async.py index e2712007f8..267be7427c 100644 --- a/src/dodal/devices/areadetector/plugins/MJPG_async.py +++ b/src/dodal/devices/areadetector/plugins/MJPG_async.py @@ -2,6 +2,7 @@ from io import BytesIO from pathlib import Path +import aiofiles from aiohttp import ClientSession from bluesky.protocols import Triggerable from ophyd_async.core import AsyncStatus, StandardReadable, soft_signal_rw @@ -50,7 +51,10 @@ async def _save_image(self, image: Image.Image): Path(directory_str).mkdir(parents=True) LOGGER.info(f"Saving image to {path}") - image.save(path) + async with aiofiles.open(path, "wb") as fh: + # FIXME + await fh.write(image) + # image.save(path) await self.last_saved_path.set(path, wait=True) @AsyncStatus.wrap diff --git a/tests/devices/unit_tests/oav/test_oav_async.py b/tests/devices/unit_tests/oav/test_oav_async.py index 0669b3848f..923bd043d1 100644 --- a/tests/devices/unit_tests/oav/test_oav_async.py +++ b/tests/devices/unit_tests/oav/test_oav_async.py @@ -99,7 +99,3 @@ async def test_calculate_beam_distance(h, v, expected_x, expected_y, oav: OAV): h, v, ) == (expected_x, expected_y) - - -async def test_when_oav_created_microns_and_beam_centre_set_in_snapshots(oav: OAV): - pass From a0092a3a85a65e0136a8c4c114b7ed42b16d9413 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Fri, 18 Oct 2024 11:17:07 +0100 Subject: [PATCH 16/53] A bunch of fixmes with test --- .../devices/unit_tests/oav/test_snapshots.py | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 tests/devices/unit_tests/oav/test_snapshots.py diff --git a/tests/devices/unit_tests/oav/test_snapshots.py b/tests/devices/unit_tests/oav/test_snapshots.py new file mode 100644 index 0000000000..7a583fa79e --- /dev/null +++ b/tests/devices/unit_tests/oav/test_snapshots.py @@ -0,0 +1,44 @@ +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +from ophyd_async.core import DeviceCollector, MockSignalBackend, SignalR, set_mock_value + +from dodal.devices.oav.snapshots.snapshot_with_beam_centre import SnapshotWithBeamCentre + + +def create_and_set_mock_signal_r(dtype, name, value): + sig = SignalR(MockSignalBackend(dtype), name=name) + set_mock_value(sig, value) + return sig + + +@pytest.fixture +async def snapshot() -> SnapshotWithBeamCentre: + mock_beam_x = create_and_set_mock_signal_r(int, "moxk_beam_x", 510) + mock_beam_y = create_and_set_mock_signal_r(int, "mock_beam_y", 380) + async with DeviceCollector(mock=True): + snapshot = SnapshotWithBeamCentre("", mock_beam_x, mock_beam_y, "fake_snapshot") + return snapshot + + +@patch("dodal.devices.oav.snapshots.snapshot_with_beam_centre.Image") +@patch("dodal.devices.oav.snapshots.snapshot_with_beam_centre.ImageDraw") +@patch( + "dodal.devices.areadetector.plugins.MJPG_async.ClientSession.get", + autospec=True, +) +async def test_given_snapshot_triggered_then_crosshair_drawn( + mock_get, patch_image_draw, patch_image, snapshot +): + mock_get.return_value.__aenter__.return_value = AsyncMock() + patch_line = MagicMock() + patch_image_draw.Draw.return_value.line = patch_line + + await snapshot.directory.set("/tmp/") + await snapshot.filename.set("test") + + # FIXME This will fail because "function never awaited" + # Need to find it though. + # await snapshot.trigger() + + # assert len(patch_line.mock_calls) == 2 From b5f6e7c30d3cfed33c3277372db0cd7b10e15709 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Fri, 18 Oct 2024 11:34:33 +0100 Subject: [PATCH 17/53] Improving file saving to make async and looking for the problem --- src/dodal/devices/areadetector/plugins/MJPG_async.py | 11 +++++++++-- .../oav/snapshots/snapshot_with_beam_centre.py | 3 +++ tests/devices/unit_tests/oav/test_snapshots.py | 7 +++++-- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/dodal/devices/areadetector/plugins/MJPG_async.py b/src/dodal/devices/areadetector/plugins/MJPG_async.py index 267be7427c..4215b37f69 100644 --- a/src/dodal/devices/areadetector/plugins/MJPG_async.py +++ b/src/dodal/devices/areadetector/plugins/MJPG_async.py @@ -44,6 +44,7 @@ async def _save_image(self, image: Image.Image): """ filename_str = await self.filename.get_value() directory_str = await self.directory.get_value() + print("HERE 6") path = Path(f"{directory_str}/{filename_str}.png").as_posix() if not Path(directory_str).is_dir(): @@ -51,9 +52,12 @@ async def _save_image(self, image: Image.Image): Path(directory_str).mkdir(parents=True) LOGGER.info(f"Saving image to {path}") + print("HERE 7") + + buffer = BytesIO() + image.save(buffer, format="png") async with aiofiles.open(path, "wb") as fh: - # FIXME - await fh.write(image) + await fh.write(buffer.getbuffer()) # image.save(path) await self.last_saved_path.set(path, wait=True) @@ -70,8 +74,11 @@ async def trigger(self): async with session.get(url_str) as response: try: response.raise_for_status() + print("HERE 1") data = await response.read() + print("HERE1.5") with Image.open(BytesIO(data)) as image: + print("HERE 2") await self.post_processing(image) except Exception as e: LOGGER.warning(f"Failed to create snapshot. \n {e}") diff --git a/src/dodal/devices/oav/snapshots/snapshot_with_beam_centre.py b/src/dodal/devices/oav/snapshots/snapshot_with_beam_centre.py index a96318459e..044456af6a 100644 --- a/src/dodal/devices/oav/snapshots/snapshot_with_beam_centre.py +++ b/src/dodal/devices/oav/snapshots/snapshot_with_beam_centre.py @@ -24,9 +24,12 @@ def __init__( super().__init__(prefix, name) async def post_processing(self, image: Image.Image): + print("HERE 3") beam_x = await self.beam_centre_i.get_value() beam_y = await self.beam_centre_j.get_value() + print("HERE 4") SnapshotWithBeamCentre.draw_crosshair(image, beam_x, beam_y) + print("HERE 5") await self._save_image(image) diff --git a/tests/devices/unit_tests/oav/test_snapshots.py b/tests/devices/unit_tests/oav/test_snapshots.py index 7a583fa79e..aac518fb6d 100644 --- a/tests/devices/unit_tests/oav/test_snapshots.py +++ b/tests/devices/unit_tests/oav/test_snapshots.py @@ -31,14 +31,17 @@ async def test_given_snapshot_triggered_then_crosshair_drawn( mock_get, patch_image_draw, patch_image, snapshot ): mock_get.return_value.__aenter__.return_value = AsyncMock() + mock_get.return_value.read.return_value = MagicMock() patch_line = MagicMock() patch_image_draw.Draw.return_value.line = patch_line + # patch_image.open.return_value = MagicMock() + await snapshot.directory.set("/tmp/") await snapshot.filename.set("test") - # FIXME This will fail because "function never awaited" - # Need to find it though. + # FIXME Now failing because data is not Bytes but asyncmock + # Not sure how to fix tbh # await snapshot.trigger() # assert len(patch_line.mock_calls) == 2 From 51d45f73b1244fe6c8634a8bf653d00e68857557 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Mon, 21 Oct 2024 10:33:30 +0100 Subject: [PATCH 18/53] A passing test --- .../areadetector/plugins/MJPG_async.py | 10 +++++-- .../devices/unit_tests/oav/test_snapshots.py | 29 ++++++++++++------- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/dodal/devices/areadetector/plugins/MJPG_async.py b/src/dodal/devices/areadetector/plugins/MJPG_async.py index 4215b37f69..b03144b519 100644 --- a/src/dodal/devices/areadetector/plugins/MJPG_async.py +++ b/src/dodal/devices/areadetector/plugins/MJPG_async.py @@ -42,6 +42,7 @@ async def _save_image(self, image: Image.Image): directory and filename signals. The full resultant path is put on the \ last_saved_path signal """ + print("HERE 5.5") filename_str = await self.filename.get_value() directory_str = await self.directory.get_value() print("HERE 6") @@ -72,14 +73,17 @@ async def trigger(self): async with ClientSession() as session: async with session.get(url_str) as response: + if not response.ok: + LOGGER.warning( + f"OAV responded with {response.status}: {response.reason}." + ) try: - response.raise_for_status() print("HERE 1") data = await response.read() print("HERE1.5") with Image.open(BytesIO(data)) as image: - print("HERE 2") - await self.post_processing(image) + print("HERE 2") + await self.post_processing(image) except Exception as e: LOGGER.warning(f"Failed to create snapshot. \n {e}") diff --git a/tests/devices/unit_tests/oav/test_snapshots.py b/tests/devices/unit_tests/oav/test_snapshots.py index aac518fb6d..c87fd5e6d4 100644 --- a/tests/devices/unit_tests/oav/test_snapshots.py +++ b/tests/devices/unit_tests/oav/test_snapshots.py @@ -21,27 +21,36 @@ async def snapshot() -> SnapshotWithBeamCentre: return snapshot -@patch("dodal.devices.oav.snapshots.snapshot_with_beam_centre.Image") +@patch("dodal.devices.areadetector.plugins.MJPG_async.Path.mkdir") +@patch("dodal.devices.areadetector.plugins.MJPG_async.Image") @patch("dodal.devices.oav.snapshots.snapshot_with_beam_centre.ImageDraw") @patch( "dodal.devices.areadetector.plugins.MJPG_async.ClientSession.get", autospec=True, ) -async def test_given_snapshot_triggered_then_crosshair_drawn( - mock_get, patch_image_draw, patch_image, snapshot +# @patch("dodal.devices.areadetector.plugins.MJPG_async.aiofiles", autospec=True) +async def test_given_snapshot_triggered_then_crosshair_drawn_and_file_saved( + # mock_aiofiles, + mock_get, patch_image_draw, patch_image, mock_mkdir, snapshot ): - mock_get.return_value.__aenter__.return_value = AsyncMock() - mock_get.return_value.read.return_value = MagicMock() + mock_get.return_value.__aenter__.return_value = (mock_response := AsyncMock()) + mock_response.ok = MagicMock(return_value=True) + mock_response.read.return_value = (test_data := b"TEST") + + # mock_open = mock_aiofiles.open + # mock_open.return_value.__aenter__.return_value = (mock_file := AsyncMock()) + patch_line = MagicMock() patch_image_draw.Draw.return_value.line = patch_line - # patch_image.open.return_value = MagicMock() + mock_open = patch_image.open + mock_open.return_value.__aenter__.return_value = test_data await snapshot.directory.set("/tmp/") await snapshot.filename.set("test") - # FIXME Now failing because data is not Bytes but asyncmock - # Not sure how to fix tbh - # await snapshot.trigger() + await snapshot.trigger() - # assert len(patch_line.mock_calls) == 2 + assert len(patch_line.mock_calls) == 2 + # mock_open.assert_called_once_with("/tmp/file.png", "wb") + # mock_file.write.assert_called_once_with(test_data) From 46c4a33e23fa3ba616854b848024d98878a6c58f Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Mon, 21 Oct 2024 10:43:53 +0100 Subject: [PATCH 19/53] get test to pass --- .../devices/areadetector/plugins/MJPG_async.py | 9 +-------- .../oav/snapshots/snapshot_with_beam_centre.py | 3 --- tests/devices/unit_tests/oav/test_snapshots.py | 14 +++++++------- 3 files changed, 8 insertions(+), 18 deletions(-) diff --git a/src/dodal/devices/areadetector/plugins/MJPG_async.py b/src/dodal/devices/areadetector/plugins/MJPG_async.py index b03144b519..dd759cdd6b 100644 --- a/src/dodal/devices/areadetector/plugins/MJPG_async.py +++ b/src/dodal/devices/areadetector/plugins/MJPG_async.py @@ -42,10 +42,8 @@ async def _save_image(self, image: Image.Image): directory and filename signals. The full resultant path is put on the \ last_saved_path signal """ - print("HERE 5.5") filename_str = await self.filename.get_value() directory_str = await self.directory.get_value() - print("HERE 6") path = Path(f"{directory_str}/{filename_str}.png").as_posix() if not Path(directory_str).is_dir(): @@ -53,13 +51,11 @@ async def _save_image(self, image: Image.Image): Path(directory_str).mkdir(parents=True) LOGGER.info(f"Saving image to {path}") - print("HERE 7") buffer = BytesIO() image.save(buffer, format="png") async with aiofiles.open(path, "wb") as fh: await fh.write(buffer.getbuffer()) - # image.save(path) await self.last_saved_path.set(path, wait=True) @AsyncStatus.wrap @@ -78,12 +74,9 @@ async def trigger(self): f"OAV responded with {response.status}: {response.reason}." ) try: - print("HERE 1") data = await response.read() - print("HERE1.5") with Image.open(BytesIO(data)) as image: - print("HERE 2") - await self.post_processing(image) + await self.post_processing(image) except Exception as e: LOGGER.warning(f"Failed to create snapshot. \n {e}") diff --git a/src/dodal/devices/oav/snapshots/snapshot_with_beam_centre.py b/src/dodal/devices/oav/snapshots/snapshot_with_beam_centre.py index 044456af6a..a96318459e 100644 --- a/src/dodal/devices/oav/snapshots/snapshot_with_beam_centre.py +++ b/src/dodal/devices/oav/snapshots/snapshot_with_beam_centre.py @@ -24,12 +24,9 @@ def __init__( super().__init__(prefix, name) async def post_processing(self, image: Image.Image): - print("HERE 3") beam_x = await self.beam_centre_i.get_value() beam_y = await self.beam_centre_j.get_value() - print("HERE 4") SnapshotWithBeamCentre.draw_crosshair(image, beam_x, beam_y) - print("HERE 5") await self._save_image(image) diff --git a/tests/devices/unit_tests/oav/test_snapshots.py b/tests/devices/unit_tests/oav/test_snapshots.py index c87fd5e6d4..e8f5543825 100644 --- a/tests/devices/unit_tests/oav/test_snapshots.py +++ b/tests/devices/unit_tests/oav/test_snapshots.py @@ -28,17 +28,16 @@ async def snapshot() -> SnapshotWithBeamCentre: "dodal.devices.areadetector.plugins.MJPG_async.ClientSession.get", autospec=True, ) -# @patch("dodal.devices.areadetector.plugins.MJPG_async.aiofiles", autospec=True) +@patch("dodal.devices.areadetector.plugins.MJPG_async.aiofiles", autospec=True) async def test_given_snapshot_triggered_then_crosshair_drawn_and_file_saved( - # mock_aiofiles, - mock_get, patch_image_draw, patch_image, mock_mkdir, snapshot + mock_aiofiles, mock_get, patch_image_draw, patch_image, mock_mkdir, snapshot ): mock_get.return_value.__aenter__.return_value = (mock_response := AsyncMock()) mock_response.ok = MagicMock(return_value=True) mock_response.read.return_value = (test_data := b"TEST") - # mock_open = mock_aiofiles.open - # mock_open.return_value.__aenter__.return_value = (mock_file := AsyncMock()) + mock_aio_open = mock_aiofiles.open + mock_aio_open.return_value.__aenter__.return_value = (mock_file := AsyncMock()) patch_line = MagicMock() patch_image_draw.Draw.return_value.line = patch_line @@ -52,5 +51,6 @@ async def test_given_snapshot_triggered_then_crosshair_drawn_and_file_saved( await snapshot.trigger() assert len(patch_line.mock_calls) == 2 - # mock_open.assert_called_once_with("/tmp/file.png", "wb") - # mock_file.write.assert_called_once_with(test_data) + assert await snapshot.last_saved_path.get_value() == "/tmp/test.png" + mock_aio_open.assert_called_once_with("/tmp/test.png", "wb") + mock_file.write.assert_called_once() From f6af9677c0ff528c739b62d9c7b89241ef66e6b8 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Mon, 21 Oct 2024 10:57:00 +0100 Subject: [PATCH 20/53] Tidy up and fix tests --- .../devices/unit_tests/oav/test_snapshots.py | 53 +++++++++++++++---- 1 file changed, 43 insertions(+), 10 deletions(-) diff --git a/tests/devices/unit_tests/oav/test_snapshots.py b/tests/devices/unit_tests/oav/test_snapshots.py index e8f5543825..7d504c9c25 100644 --- a/tests/devices/unit_tests/oav/test_snapshots.py +++ b/tests/devices/unit_tests/oav/test_snapshots.py @@ -1,7 +1,9 @@ +from pathlib import Path from unittest.mock import AsyncMock, MagicMock, patch import pytest from ophyd_async.core import DeviceCollector, MockSignalBackend, SignalR, set_mock_value +from PIL import Image from dodal.devices.oav.snapshots.snapshot_with_beam_centre import SnapshotWithBeamCentre @@ -18,39 +20,70 @@ async def snapshot() -> SnapshotWithBeamCentre: mock_beam_y = create_and_set_mock_signal_r(int, "mock_beam_y", 380) async with DeviceCollector(mock=True): snapshot = SnapshotWithBeamCentre("", mock_beam_x, mock_beam_y, "fake_snapshot") + await snapshot.directory.set("/tmp/") + await snapshot.filename.set("test") return snapshot -@patch("dodal.devices.areadetector.plugins.MJPG_async.Path.mkdir") @patch("dodal.devices.areadetector.plugins.MJPG_async.Image") @patch("dodal.devices.oav.snapshots.snapshot_with_beam_centre.ImageDraw") @patch( "dodal.devices.areadetector.plugins.MJPG_async.ClientSession.get", autospec=True, ) -@patch("dodal.devices.areadetector.plugins.MJPG_async.aiofiles", autospec=True) -async def test_given_snapshot_triggered_then_crosshair_drawn_and_file_saved( - mock_aiofiles, mock_get, patch_image_draw, patch_image, mock_mkdir, snapshot +async def test_given_snapshot_triggered_then_crosshair_drawn_and( + mock_get, patch_image_draw, patch_image, snapshot ): mock_get.return_value.__aenter__.return_value = (mock_response := AsyncMock()) mock_response.ok = MagicMock(return_value=True) mock_response.read.return_value = (test_data := b"TEST") - mock_aio_open = mock_aiofiles.open - mock_aio_open.return_value.__aenter__.return_value = (mock_file := AsyncMock()) - patch_line = MagicMock() patch_image_draw.Draw.return_value.line = patch_line + snapshot._save_image = (mock_save := AsyncMock()) + mock_open = patch_image.open mock_open.return_value.__aenter__.return_value = test_data - await snapshot.directory.set("/tmp/") - await snapshot.filename.set("test") - await snapshot.trigger() assert len(patch_line.mock_calls) == 2 + mock_save.assert_called_once() + + +@patch("dodal.devices.areadetector.plugins.MJPG_async.Path.mkdir") +@patch("dodal.devices.areadetector.plugins.MJPG_async.Image") +@patch( + "dodal.devices.areadetector.plugins.MJPG_async.ClientSession.get", + autospec=True, +) +@patch("dodal.devices.areadetector.plugins.MJPG_async.aiofiles", autospec=True) +async def test_snapshot_correctly_triggered_and_saved( + mock_aiofiles, mock_get, patch_image, mock_mkdir, snapshot +): + mock_get.return_value.__aenter__.return_value = (mock_response := AsyncMock()) + mock_response.ok = MagicMock(return_value=True) + mock_response.read.return_value = (test_data := b"TEST") + + mock_aio_open = mock_aiofiles.open + mock_aio_open.return_value.__aenter__.return_value = (mock_file := AsyncMock()) + + mock_open = patch_image.open + mock_open.return_value.__aenter__.return_value = test_data + + await snapshot.trigger() + assert await snapshot.last_saved_path.get_value() == "/tmp/test.png" mock_aio_open.assert_called_once_with("/tmp/test.png", "wb") mock_file.write.assert_called_once() + + +def test_snapshot_draws_expected_crosshair(tmp_path: Path): + image = Image.open("tests/test_data/test_images/oav_snapshot_test.png") + SnapshotWithBeamCentre.draw_crosshair(image, 510, 380) + image.save(tmp_path / "output_image.png") + expected_image = Image.open("tests/test_data/test_images/oav_snapshot_expected.png") + image_bytes = image.tobytes() + expected_bytes = expected_image.tobytes() + assert image_bytes == expected_bytes, "Actual and expected images differ" From d321c169ccc445c9512a98e6ae1e7c3f712cfc67 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Mon, 21 Oct 2024 13:06:25 +0100 Subject: [PATCH 21/53] Add a test for snapshot with grid --- .../oav/snapshots/snapshot_with_grid.py | 12 +++- .../devices/unit_tests/oav/test_snapshots.py | 59 ++++++++++++++++++- 2 files changed, 66 insertions(+), 5 deletions(-) diff --git a/src/dodal/devices/oav/snapshots/snapshot_with_grid.py b/src/dodal/devices/oav/snapshots/snapshot_with_grid.py index 6ca430d486..c21c849199 100644 --- a/src/dodal/devices/oav/snapshots/snapshot_with_grid.py +++ b/src/dodal/devices/oav/snapshots/snapshot_with_grid.py @@ -1,5 +1,7 @@ +from io import BytesIO from os.path import join as path_join +import aiofiles from ophyd_async.core import soft_signal_rw from PIL.Image import Image @@ -24,6 +26,12 @@ def __init__(self, prefix: str, name: str = "") -> None: self.last_path_outer = soft_signal_rw(str) self.last_path_full_overlay = soft_signal_rw(str) + async def _save_grid_to_image(self, image: Image, path: str): + buffer = BytesIO() + image.save(buffer, format="png") + async with aiofiles.open(path, "wb") as fh: + await fh.write(buffer.getbuffer()) + async def post_processing(self, image: Image): # Save an unmodified image with no suffix await self._save_image(image) @@ -44,7 +52,7 @@ async def post_processing(self, image: Image): path = path_join(directory_str, f"{filename_str}_outer_overlay.png") await self.last_path_outer.set(path, wait=True) LOGGER.info(f"Saving grid outer edge at {path}") - image.save(path) + await self._save_grid_to_image(image, path) add_grid_overlay_to_image( image, top_left_x, top_left_y, box_witdh, num_boxes_x, num_boxes_y @@ -53,4 +61,4 @@ async def post_processing(self, image: Image): path = path_join(directory_str, f"{filename_str}_grid_overlay.png") await self.last_path_full_overlay.set(path, wait=True) LOGGER.info(f"Saving full grid overlay at {path}") - image.save(path) + await self._save_grid_to_image(image, path) diff --git a/tests/devices/unit_tests/oav/test_snapshots.py b/tests/devices/unit_tests/oav/test_snapshots.py index 7d504c9c25..0a04dd41ee 100644 --- a/tests/devices/unit_tests/oav/test_snapshots.py +++ b/tests/devices/unit_tests/oav/test_snapshots.py @@ -6,6 +6,7 @@ from PIL import Image from dodal.devices.oav.snapshots.snapshot_with_beam_centre import SnapshotWithBeamCentre +from dodal.devices.oav.snapshots.snapshot_with_grid import SnapshotWithGrid def create_and_set_mock_signal_r(dtype, name, value): @@ -25,13 +26,29 @@ async def snapshot() -> SnapshotWithBeamCentre: return snapshot +@pytest.fixture +async def grid_snapshot() -> SnapshotWithGrid: + async with DeviceCollector(mock=True): + grid_snapshot = SnapshotWithGrid("", "fake_grid") + + await grid_snapshot.top_left_x.set(5) + await grid_snapshot.top_left_y.set(5) + await grid_snapshot.box_width.set(5) + await grid_snapshot.num_boxes_y.set(5) + await grid_snapshot.num_boxes_y.set(5) + + await grid_snapshot.directory.set("/tmp/") + await grid_snapshot.filename.set("test") + return grid_snapshot + + @patch("dodal.devices.areadetector.plugins.MJPG_async.Image") @patch("dodal.devices.oav.snapshots.snapshot_with_beam_centre.ImageDraw") @patch( "dodal.devices.areadetector.plugins.MJPG_async.ClientSession.get", autospec=True, ) -async def test_given_snapshot_triggered_then_crosshair_drawn_and( +async def test_snapshot_with_beam_centre_triggered_then_crosshair_drawn_and( mock_get, patch_image_draw, patch_image, snapshot ): mock_get.return_value.__aenter__.return_value = (mock_response := AsyncMock()) @@ -49,7 +66,7 @@ async def test_given_snapshot_triggered_then_crosshair_drawn_and( await snapshot.trigger() assert len(patch_line.mock_calls) == 2 - mock_save.assert_called_once() + mock_save.assert_awaited_once() @patch("dodal.devices.areadetector.plugins.MJPG_async.Path.mkdir") @@ -59,7 +76,7 @@ async def test_given_snapshot_triggered_then_crosshair_drawn_and( autospec=True, ) @patch("dodal.devices.areadetector.plugins.MJPG_async.aiofiles", autospec=True) -async def test_snapshot_correctly_triggered_and_saved( +async def test_snapshot_with_beam_centre_correctly_triggered_and_saved( mock_aiofiles, mock_get, patch_image, mock_mkdir, snapshot ): mock_get.return_value.__aenter__.return_value = (mock_response := AsyncMock()) @@ -87,3 +104,39 @@ def test_snapshot_draws_expected_crosshair(tmp_path: Path): image_bytes = image.tobytes() expected_bytes = expected_image.tobytes() assert image_bytes == expected_bytes, "Actual and expected images differ" + + +@patch("dodal.devices.areadetector.plugins.MJPG_async.Image") +@patch( + "dodal.devices.oav.snapshots.snapshot_with_grid.add_grid_border_overlay_to_image" +) +@patch("dodal.devices.oav.snapshots.snapshot_with_grid.add_grid_overlay_to_image") +@patch( + "dodal.devices.areadetector.plugins.MJPG_async.ClientSession.get", + autospec=True, +) +async def test_snapshot_with_grid_triggered_saves_image_and_draws_grid( + mock_get, patch_add_grid, patch_add_border, patch_image, grid_snapshot +): + mock_get.return_value.__aenter__.return_value = (mock_response := AsyncMock()) + mock_response.ok = MagicMock(return_value=True) + mock_response.read.return_value = (test_data := b"TEST") + + mock_open = patch_image.open + mock_open.return_value.__aenter__.return_value = test_data + + grid_snapshot._save_image = (mock_save := AsyncMock()) + grid_snapshot._save_grid_to_image = (mock_save_grid := AsyncMock()) + + await grid_snapshot.trigger() + mock_save.assert_awaited_once() + patch_add_border.assert_called_once() + patch_add_grid.assert_called_once() + assert mock_save_grid.await_count == 2 + assert ( + await grid_snapshot.last_path_outer.get_value() == "/tmp/test_outer_overlay.png" + ) + assert ( + await grid_snapshot.last_path_full_overlay.get_value() + == "/tmp/test_grid_overlay.png" + ) From 13edeca14c6ffb322407231a42caeae658a3c203 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Mon, 21 Oct 2024 13:11:43 +0100 Subject: [PATCH 22/53] Fix oav --- src/dodal/devices/oav/oav_async.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dodal/devices/oav/oav_async.py b/src/dodal/devices/oav/oav_async.py index 50222d3d38..39d752b76e 100644 --- a/src/dodal/devices/oav/oav_async.py +++ b/src/dodal/devices/oav/oav_async.py @@ -49,8 +49,6 @@ def __init__(self, prefix: str, config: OAVConfig, name: str = ""): _bl_prefix = prefix.split("-")[0] self.zoom_controller = ZoomController(f"{_bl_prefix}-EA-OAV-01:FZOOM:", name) - self.sizes = [self.grid_snapshot.x_size, self.grid_snapshot.y_size] - self.parameters = config.get_parameters() self.microns_per_pixel_x = create_hardware_backed_soft_signal( @@ -75,6 +73,8 @@ def __init__(self, prefix: str, config: OAVConfig, name: str = ""): ) self.grid_snapshot = SnapshotWithGrid(f"{prefix}-DI-OAV-01:MJPG:", name) + self.sizes = [self.grid_snapshot.x_size, self.grid_snapshot.y_size] + super().__init__(name) async def _read_current_zoom(self) -> str: From 126f6875db9f92a46f5c2233931281ca52b32fee Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Mon, 21 Oct 2024 13:43:01 +0100 Subject: [PATCH 23/53] Start breaking things: remove old OAV --- src/dodal/devices/oav/oav_async.py | 2 + src/dodal/devices/oav/oav_detector.py | 92 ---------------- tests/devices/unit_tests/oav/test_oav.py | 133 ----------------------- 3 files changed, 2 insertions(+), 225 deletions(-) delete mode 100644 src/dodal/devices/oav/oav_detector.py delete mode 100644 tests/devices/unit_tests/oav/test_oav.py diff --git a/src/dodal/devices/oav/oav_async.py b/src/dodal/devices/oav/oav_async.py index 39d752b76e..643e89727f 100644 --- a/src/dodal/devices/oav/oav_async.py +++ b/src/dodal/devices/oav/oav_async.py @@ -73,6 +73,8 @@ def __init__(self, prefix: str, config: OAVConfig, name: str = ""): ) self.grid_snapshot = SnapshotWithGrid(f"{prefix}-DI-OAV-01:MJPG:", name) + self._snapshot_trigger_subscription_id = None + self.sizes = [self.grid_snapshot.x_size, self.grid_snapshot.y_size] super().__init__(name) diff --git a/src/dodal/devices/oav/oav_detector.py b/src/dodal/devices/oav/oav_detector.py deleted file mode 100644 index e6c37610aa..0000000000 --- a/src/dodal/devices/oav/oav_detector.py +++ /dev/null @@ -1,92 +0,0 @@ -# type: ignore # OAV will soon be ophyd-async, see https://github.com/DiamondLightSource/dodal/issues/716 -from functools import partial - -from ophyd import ADComponent as ADC -from ophyd import ( - AreaDetector, - CamBase, - Component, - Device, - EpicsSignal, - HDF5Plugin, - OverlayPlugin, - ProcessPlugin, - ROIPlugin, - StatusBase, -) - -from dodal.devices.areadetector.plugins.MJPG import SnapshotWithBeamCentre -from dodal.devices.oav.grid_overlay import SnapshotWithGrid -from dodal.devices.oav.oav_parameters import OAVConfigParams - - -class ZoomController(Device): - """ - Device to control the zoom level. This should be set like - o = OAV(name="oav") - oav.zoom_controller.set("1.0x") - - Note that changing the zoom may change the AD wiring on the associated OAV, as such - you should wait on any zoom changs to finish before changing the OAV wiring. - """ - - percentage = Component(EpicsSignal, "ZOOMPOSCMD") - - # Level is the string description of the zoom level e.g. "1.0x" - level = Component(EpicsSignal, "MP:SELECT", string=True) - - zrst = Component(EpicsSignal, "MP:SELECT.ZRST") - onst = Component(EpicsSignal, "MP:SELECT.ONST") - twst = Component(EpicsSignal, "MP:SELECT.TWST") - thst = Component(EpicsSignal, "MP:SELECT.THST") - frst = Component(EpicsSignal, "MP:SELECT.FRST") - fvst = Component(EpicsSignal, "MP:SELECT.FVST") - sxst = Component(EpicsSignal, "MP:SELECT.SXST") - - @property - def allowed_zoom_levels(self): - return [ - self.zrst.get(), - self.onst.get(), - self.twst.get(), - self.thst.get(), - self.frst.get(), - self.fvst.get(), - self.sxst.get(), - ] - - def set(self, level_to_set: str) -> StatusBase: - return self.level.set(level_to_set) - - -class OAV(AreaDetector): - cam = ADC(CamBase, "-DI-OAV-01:CAM:") - roi = ADC(ROIPlugin, "-DI-OAV-01:ROI:") - proc = ADC(ProcessPlugin, "-DI-OAV-01:PROC:") - over = ADC(OverlayPlugin, "-DI-OAV-01:OVER:") - tiff = ADC(OverlayPlugin, "-DI-OAV-01:TIFF:") - hdf5 = ADC(HDF5Plugin, "-DI-OAV-01:HDF5:") - grid_snapshot = Component(SnapshotWithGrid, "-DI-OAV-01:MJPG:") - snapshot = Component(SnapshotWithBeamCentre, "-DI-OAV-01:MJPG:") - zoom_controller = Component(ZoomController, "-EA-OAV-01:FZOOM:") - - def __init__(self, *args, params: OAVConfigParams, **kwargs): - super().__init__(*args, **kwargs) - self.parameters = params - self.grid_snapshot.oav_params = params - self.snapshot.oav_params = params - self.subscription_id = None - self._snapshot_trigger_subscription_id = None - - def wait_for_connection(self, all_signals=False, timeout=2): - connected = super().wait_for_connection(all_signals, timeout) - x = self.grid_snapshot.x_size.get() - y = self.grid_snapshot.y_size.get() - - cb = partial(self.parameters.update_on_zoom, xsize=x, ysize=y) - - if self.subscription_id is not None: - self.zoom_controller.level.unsubscribe(self.subscription_id) - self.subscription_id = self.zoom_controller.level.subscribe(cb) - - return connected diff --git a/tests/devices/unit_tests/oav/test_oav.py b/tests/devices/unit_tests/oav/test_oav.py deleted file mode 100644 index ebaa6eb228..0000000000 --- a/tests/devices/unit_tests/oav/test_oav.py +++ /dev/null @@ -1,133 +0,0 @@ -import pytest -from ophyd.sim import instantiate_fake_device - -from dodal.devices.oav.oav_detector import OAV, OAVConfigParams -from dodal.devices.oav.oav_errors import ( - OAVError_BeamPositionNotFound, - OAVError_ZoomLevelNotFound, -) - -DISPLAY_CONFIGURATION = "tests/devices/unit_tests/test_display.configuration" -ZOOM_LEVELS_XML = "tests/devices/unit_tests/test_jCameraManZoomLevels.xml" - - -@pytest.fixture -def oav() -> OAV: - oav_params = OAVConfigParams(ZOOM_LEVELS_XML, DISPLAY_CONFIGURATION) - oav: OAV = instantiate_fake_device(OAV, params=oav_params) - oav.proc.port_name.sim_put("proc") # type: ignore - oav.cam.port_name.sim_put("CAM") # type: ignore - - oav.grid_snapshot.x_size.sim_put("1024") # type: ignore - oav.grid_snapshot.y_size.sim_put("768") # type: ignore - - oav.zoom_controller.zrst.set("1.0x") - oav.zoom_controller.onst.set("2.0x") - oav.zoom_controller.twst.set("3.0x") - oav.zoom_controller.thst.set("5.0x") - oav.zoom_controller.frst.set("7.0x") - oav.zoom_controller.fvst.set("9.0x") - - oav.wait_for_connection() - - return oav - - -def test_load_microns_per_pixel_entry_not_found(oav: OAV): - with pytest.raises(OAVError_ZoomLevelNotFound): - oav.parameters.load_microns_per_pixel(0.000001, 0, 0) - - -@pytest.mark.parametrize( - "zoom_level,expected_microns_x,expected_microns_y", - [ - ("1.0x", 2.87, 2.87), - ("2.5", 2.31, 2.31), - ("5.0x", 1.58, 1.58), - ("15.0", 0.302, 0.302), - ], -) -def test_get_micronsperpixel_from_oav( - zoom_level, expected_microns_x, expected_microns_y, oav: OAV -): - oav.zoom_controller.level.sim_put(zoom_level) # type: ignore - - assert oav.parameters.micronsPerXPixel == pytest.approx( - expected_microns_x, abs=1e-2 - ) - assert oav.parameters.micronsPerYPixel == pytest.approx( - expected_microns_y, abs=1e-2 - ) - - -def test_beam_position_not_found_for_wrong_entry(oav: OAV): - with pytest.raises(OAVError_BeamPositionNotFound): - oav.parameters.get_beam_position_from_zoom(2.0, 0, 0) - - -def test_get_beam_position(oav: OAV): - expected_beam_position = (493, 355) - beam_position = oav.parameters.get_beam_position_from_zoom(2.5, 1024, 768) - - assert beam_position[0] == expected_beam_position[0] - assert beam_position[1] == expected_beam_position[1] - - -@pytest.mark.parametrize( - "zoom_level,expected_xCentre,expected_yCentre", - [("1.0", 477, 359), ("5.0", 517, 350), ("10.0x", 613, 344)], -) -def test_extract_beam_position_given_different_zoom_levels( - zoom_level, - expected_xCentre, - expected_yCentre, - oav: OAV, -): - oav.zoom_controller.level.sim_put(zoom_level) # type: ignore - - assert oav.parameters.beam_centre_i == expected_xCentre - assert oav.parameters.beam_centre_j == expected_yCentre - - -def test_extract_rescaled_micronsperpixel(oav: OAV): - oav.grid_snapshot.x_size.sim_put("1292") # type: ignore - oav.grid_snapshot.y_size.sim_put("964") # type: ignore - oav.wait_for_connection() - - oav.zoom_controller.level.sim_put("1.0") # type: ignore - - assert oav.parameters.micronsPerXPixel == pytest.approx(2.27, abs=1e-2) - assert oav.parameters.micronsPerYPixel == pytest.approx(2.28, abs=1e-2) - - -def test_extract_rescaled_beam_position(oav: OAV): - oav.grid_snapshot.x_size.sim_put("1292") # type: ignore - oav.grid_snapshot.y_size.sim_put("964") # type: ignore - oav.wait_for_connection() - - oav.zoom_controller.level.sim_put("1.0") # type: ignore - - assert oav.parameters.beam_centre_i == 601 - assert oav.parameters.beam_centre_j == 450 - - -@pytest.mark.parametrize( - "h, v, expected_x, expected_y", - [ - (54, 100, 517 - 54, 350 - 100), - (0, 0, 517, 350), - (500, 500, 517 - 500, 350 - 500), - ], -) -def test_calculate_beam_distance(h, v, expected_x, expected_y, oav: OAV): - oav.zoom_controller.level.sim_put("5.0x") # type: ignore - - assert oav.parameters.calculate_beam_distance( - h, - v, - ) == (expected_x, expected_y) - - -def test_when_oav_created_then_snapshot_parameters_set(oav: OAV): - assert oav.snapshot.oav_params is not None - assert oav.grid_snapshot.oav_params is not None From 25166a4e3aec7bb53b1229b8149821e54a627f1a Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Mon, 21 Oct 2024 13:46:58 +0100 Subject: [PATCH 24/53] Rename oav async file --- .../oav/{oav_async.py => oav_detector.py} | 0 src/dodal/devices/oav/utils.py | 18 +++++++++--------- tests/devices/unit_tests/oav/test_oav_async.py | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) rename src/dodal/devices/oav/{oav_async.py => oav_detector.py} (100%) diff --git a/src/dodal/devices/oav/oav_async.py b/src/dodal/devices/oav/oav_detector.py similarity index 100% rename from src/dodal/devices/oav/oav_async.py rename to src/dodal/devices/oav/oav_detector.py diff --git a/src/dodal/devices/oav/utils.py b/src/dodal/devices/oav/utils.py index 24071502fe..9abeac6a2e 100644 --- a/src/dodal/devices/oav/utils.py +++ b/src/dodal/devices/oav/utils.py @@ -6,7 +6,7 @@ from bluesky.utils import Msg from dodal.devices.oav.oav_calculations import camera_coordinates_to_xyz -from dodal.devices.oav.oav_detector import OAVConfigParams +from dodal.devices.oav.oav_detector import OAV from dodal.devices.oav.pin_image_recognition import PinTipDetection from dodal.devices.smargon import Smargon @@ -64,7 +64,7 @@ class EdgeOutputArrayImageType(IntEnum): def get_move_required_so_that_beam_is_at_pixel( - smargon: Smargon, pixel: Pixel, oav_params: OAVConfigParams + smargon: Smargon, pixel: Pixel, oav: OAV ) -> Generator[Msg, None, np.ndarray]: """Calculate the required move so that the given pixel is in the centre of the beam.""" @@ -78,22 +78,22 @@ def get_move_required_so_that_beam_is_at_pixel( ) current_angle = yield from bps.rd(smargon.omega) - return calculate_x_y_z_of_pixel(current_motor_xyz, current_angle, pixel, oav_params) + return calculate_x_y_z_of_pixel(current_motor_xyz, current_angle, pixel, oav) def calculate_x_y_z_of_pixel( - current_x_y_z, current_omega, pixel: Pixel, oav_params: OAVConfigParams + current_x_y_z, current_omega, pixel: Pixel, oav: OAV ) -> np.ndarray: - beam_distance_px: Pixel = oav_params.calculate_beam_distance(*pixel) + beam_distance_px: Pixel = oav.calculate_beam_distance(*pixel) - assert oav_params.micronsPerXPixel - assert oav_params.micronsPerYPixel + assert oav.microns_per_pixel_x + assert oav.microns_per_pixel_y return current_x_y_z + camera_coordinates_to_xyz( beam_distance_px[0], beam_distance_px[1], current_omega, - oav_params.micronsPerXPixel, - oav_params.micronsPerYPixel, + oav.microns_per_pixel_x, + oav.microns_per_pixel_y, ) diff --git a/tests/devices/unit_tests/oav/test_oav_async.py b/tests/devices/unit_tests/oav/test_oav_async.py index 923bd043d1..1d1646a879 100644 --- a/tests/devices/unit_tests/oav/test_oav_async.py +++ b/tests/devices/unit_tests/oav/test_oav_async.py @@ -1,7 +1,7 @@ import pytest from ophyd_async.core import DeviceCollector, set_mock_value -from dodal.devices.oav.oav_async import OAV, ZoomController +from dodal.devices.oav.oav_detector import OAV, ZoomController from dodal.devices.oav.oav_parameters import OAVConfig DISPLAY_CONFIGURATION = "tests/devices/unit_tests/test_display.configuration" From a5b800b4c1aadd387802f858fc8d1c241d570ebe Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Mon, 21 Oct 2024 13:58:51 +0100 Subject: [PATCH 25/53] Remove old MJPG code and move around some files --- src/dodal/beamlines/i24.py | 2 +- .../devices/areadetector/plugins/MJPG.py | 138 ------------------ .../oav/{ => snapshots}/grid_overlay.py | 42 ------ .../oav/snapshots/snapshot_with_grid.py | 2 +- .../areadetector/plugins/test_MJPG.py | 45 ------ .../unit_tests/{ => oav}/test_grid_overlay.py | 0 .../devices/unit_tests/oav/test_snapshots.py | 10 +- 7 files changed, 7 insertions(+), 232 deletions(-) delete mode 100644 src/dodal/devices/areadetector/plugins/MJPG.py rename src/dodal/devices/oav/{ => snapshots}/grid_overlay.py (69%) delete mode 100644 tests/devices/unit_tests/areadetector/plugins/test_MJPG.py rename tests/devices/unit_tests/{ => oav}/test_grid_overlay.py (100%) diff --git a/src/dodal/beamlines/i24.py b/src/dodal/beamlines/i24.py index 7cba0a0e64..353eab6063 100644 --- a/src/dodal/beamlines/i24.py +++ b/src/dodal/beamlines/i24.py @@ -13,7 +13,7 @@ from dodal.devices.i24.i24_detector_motion import DetectorMotion from dodal.devices.i24.i24_vgonio import VGonio from dodal.devices.i24.pmac import PMAC -from dodal.devices.oav.oav_async import OAV +from dodal.devices.oav.oav_detector import OAV from dodal.devices.oav.oav_parameters import OAVConfig from dodal.devices.zebra import Zebra from dodal.log import set_beamline as set_log_beamline diff --git a/src/dodal/devices/areadetector/plugins/MJPG.py b/src/dodal/devices/areadetector/plugins/MJPG.py deleted file mode 100644 index b82d5dd10f..0000000000 --- a/src/dodal/devices/areadetector/plugins/MJPG.py +++ /dev/null @@ -1,138 +0,0 @@ -import os -import threading -from abc import ABC, abstractmethod -from io import BytesIO -from pathlib import Path - -import requests -from ophyd import Component, Device, DeviceStatus, EpicsSignal, EpicsSignalRO, Signal -from PIL import Image, ImageDraw - -from dodal.devices.oav.oav_parameters import OAVConfigParams -from dodal.log import LOGGER - - -class MJPG(Device, ABC): - """The MJPG areadetector plugin creates an MJPG video stream of the camera's output. - - This devices uses that stream to grab images. When it is triggered it will send the - latest image from the stream to the `post_processing` method for child classes to handle. - """ - - filename = Component(Signal) - directory = Component(Signal) - last_saved_path = Component(Signal) - url = Component(EpicsSignal, "JPG_URL_RBV", string=True) - x_size = Component(EpicsSignalRO, "ArraySize1_RBV") - y_size = Component(EpicsSignalRO, "ArraySize2_RBV") - input_rbpv = Component(EpicsSignalRO, "NDArrayPort_RBV") - input_plugin = Component(EpicsSignal, "NDArrayPort") - - # scaling factors for the snapshot at the time it was triggered - microns_per_pixel_x = Component(Signal) - microns_per_pixel_y = Component(Signal) - - oav_params: OAVConfigParams | None = None - - KICKOFF_TIMEOUT: float = 30.0 - - def _save_image(self, image: Image.Image): - """A helper function to save a given image to the path supplied by the directory - and filename signals. The full resultant path is put on the last_saved_path signal - """ - filename_str = self.filename.get() - directory_str: str = self.directory.get() # type: ignore - - path = Path(f"{directory_str}/{filename_str}.png").as_posix() - if not os.path.isdir(Path(directory_str)): - LOGGER.info(f"Snapshot folder {directory_str} does not exist, creating...") - os.mkdir(directory_str) - - LOGGER.info(f"Saving image to {path}") - image.save(path) - self.last_saved_path.put(path) - - def trigger(self): - """This takes a snapshot image from the MJPG stream and send it to the - post_processing method, expected to be implemented by a child of this class. - - It is the responsibility of the child class to save any resulting images. - """ - st = DeviceStatus(device=self, timeout=self.KICKOFF_TIMEOUT) - url_str = self.url.get() - - assert isinstance( - self.oav_params, OAVConfigParams - ), "MJPG does not have valid OAV parameters" - self.microns_per_pixel_x.set(self.oav_params.micronsPerXPixel) - self.microns_per_pixel_y.set(self.oav_params.micronsPerYPixel) - - def get_snapshot(): - try: - response = requests.get(url_str, stream=True) - response.raise_for_status() - with Image.open(BytesIO(response.content)) as image: - self.post_processing(image) - st.set_finished() - except requests.HTTPError as e: - st.set_exception(e) - - threading.Thread(target=get_snapshot, daemon=True).start() - - return st - - @abstractmethod - def post_processing(self, image: Image.Image): - pass - - -class SnapshotWithBeamCentre(MJPG): - """A child of MJPG which, when triggered, draws an outlined crosshair at the beam - centre in the image and saves the image to disk.""" - - CROSSHAIR_LENGTH_PX = 20 - CROSSHAIR_OUTLINE_COLOUR = "Black" - CROSSHAIR_FILL_COLOUR = "White" - - def post_processing(self, image: Image.Image): - assert ( - self.oav_params is not None - ), "Snapshot device does not have valid OAV parameters" - beam_x = self.oav_params.beam_centre_i - beam_y = self.oav_params.beam_centre_j - - SnapshotWithBeamCentre.draw_crosshair(image, beam_x, beam_y) - - self._save_image(image) - - @classmethod - def draw_crosshair(cls, image: Image.Image, beam_x: int, beam_y: int): - draw = ImageDraw.Draw(image) - OUTLINE_WIDTH = 1 - HALF_LEN = cls.CROSSHAIR_LENGTH_PX / 2 - draw.rectangle( - [ - beam_x - OUTLINE_WIDTH, - beam_y - HALF_LEN - OUTLINE_WIDTH, - beam_x + OUTLINE_WIDTH, - beam_y + HALF_LEN + OUTLINE_WIDTH, - ], - fill=cls.CROSSHAIR_OUTLINE_COLOUR, - ) - draw.rectangle( - [ - beam_x - HALF_LEN - OUTLINE_WIDTH, - beam_y - OUTLINE_WIDTH, - beam_x + HALF_LEN + OUTLINE_WIDTH, - beam_y + OUTLINE_WIDTH, - ], - fill=cls.CROSSHAIR_OUTLINE_COLOUR, - ) - draw.line( - ((beam_x, beam_y - HALF_LEN), (beam_x, beam_y + HALF_LEN)), - fill=cls.CROSSHAIR_FILL_COLOUR, - ) - draw.line( - ((beam_x - HALF_LEN, beam_y), (beam_x + HALF_LEN, beam_y)), - fill=cls.CROSSHAIR_FILL_COLOUR, - ) diff --git a/src/dodal/devices/oav/grid_overlay.py b/src/dodal/devices/oav/snapshots/grid_overlay.py similarity index 69% rename from src/dodal/devices/oav/grid_overlay.py rename to src/dodal/devices/oav/snapshots/grid_overlay.py index 24215e72c2..1f82312e5b 100644 --- a/src/dodal/devices/oav/grid_overlay.py +++ b/src/dodal/devices/oav/snapshots/grid_overlay.py @@ -1,14 +1,9 @@ # type: ignore # OAV will soon be ophyd-async, see https://github.com/DiamondLightSource/dodal/issues/716 from enum import Enum from functools import partial -from os.path import join as path_join -from ophyd import Component, Signal from PIL import Image, ImageDraw -from dodal.devices.areadetector.plugins.MJPG import MJPG -from dodal.log import LOGGER - class Orientation(Enum): horizontal = 0 @@ -122,40 +117,3 @@ def add_grid_overlay_to_image( spacing=box_width, num_lines=num_boxes_y - 1, ) - - -class SnapshotWithGrid(MJPG): - top_left_x = Component(Signal) - top_left_y = Component(Signal) - box_width = Component(Signal) - num_boxes_x = Component(Signal) - num_boxes_y = Component(Signal) - - last_path_outer = Component(Signal) - last_path_full_overlay = Component(Signal) - - def post_processing(self, image: Image.Image): - # Save an unmodified image with no suffix - self._save_image(image) - - top_left_x = self.top_left_x.get() - top_left_y = self.top_left_y.get() - box_width = self.box_width.get() - num_boxes_x = self.num_boxes_x.get() - num_boxes_y = self.num_boxes_y.get() - assert isinstance(filename_str := self.filename.get(), str) - assert isinstance(directory_str := self.directory.get(), str) - add_grid_border_overlay_to_image( - image, top_left_x, top_left_y, box_width, num_boxes_x, num_boxes_y - ) - path = path_join(directory_str, f"{filename_str}_outer_overlay.png") - self.last_path_outer.put(path) - LOGGER.info(f"Saving grid outer edge at {path}") - image.save(path) - add_grid_overlay_to_image( - image, top_left_x, top_left_y, box_width, num_boxes_x, num_boxes_y - ) - path = path_join(directory_str, f"{filename_str}_grid_overlay.png") - self.last_path_full_overlay.put(path) - LOGGER.info(f"Saving full grid overlay at {path}") - image.save(path) diff --git a/src/dodal/devices/oav/snapshots/snapshot_with_grid.py b/src/dodal/devices/oav/snapshots/snapshot_with_grid.py index c21c849199..4612e29803 100644 --- a/src/dodal/devices/oav/snapshots/snapshot_with_grid.py +++ b/src/dodal/devices/oav/snapshots/snapshot_with_grid.py @@ -6,7 +6,7 @@ from PIL.Image import Image from dodal.devices.areadetector.plugins.MJPG_async import MJPG -from dodal.devices.oav.grid_overlay import ( +from dodal.devices.oav.snapshots.grid_overlay import ( add_grid_border_overlay_to_image, add_grid_overlay_to_image, ) diff --git a/tests/devices/unit_tests/areadetector/plugins/test_MJPG.py b/tests/devices/unit_tests/areadetector/plugins/test_MJPG.py deleted file mode 100644 index 2bc8d993fd..0000000000 --- a/tests/devices/unit_tests/areadetector/plugins/test_MJPG.py +++ /dev/null @@ -1,45 +0,0 @@ -from pathlib import Path -from unittest.mock import MagicMock, patch - -from ophyd.sim import instantiate_fake_device -from PIL import Image - -from dodal.devices.areadetector.plugins.MJPG import SnapshotWithBeamCentre -from dodal.devices.oav.oav_detector import OAVConfigParams - -DISPLAY_CONFIGURATION = "tests/devices/unit_tests/test_display.configuration" -ZOOM_LEVELS_XML = "tests/devices/unit_tests/test_jCameraManZoomLevels.xml" - - -@patch("dodal.devices.areadetector.plugins.MJPG.Image") -@patch("dodal.devices.areadetector.plugins.MJPG.os") -@patch("dodal.devices.areadetector.plugins.MJPG.ImageDraw") -@patch("dodal.devices.areadetector.plugins.MJPG.requests") -def test_given_snapshot_triggered_then_crosshair_drawn( - patch_requests, patch_image_draw, patch_os, patch_image -): - patch_line = MagicMock() - patch_requests.get.return_value.content = b"" - params = OAVConfigParams(ZOOM_LEVELS_XML, DISPLAY_CONFIGURATION) - params.update_on_zoom(1.0, 100, 100) - - patch_image_draw.Draw.return_value.line = patch_line - snapshot: SnapshotWithBeamCentre = instantiate_fake_device(SnapshotWithBeamCentre) - snapshot.oav_params = params - snapshot.directory.set("/tmp/") - snapshot.filename.set("test") - - status = snapshot.trigger() - status.wait() - - assert len(patch_line.mock_calls) == 2 - - -def test_snapshot_draws_expected_crosshair(tmp_path: Path): - image = Image.open("tests/test_data/test_images/oav_snapshot_test.png") - SnapshotWithBeamCentre.draw_crosshair(image, 510, 380) - image.save(tmp_path / "output_image.png") - expected_image = Image.open("tests/test_data/test_images/oav_snapshot_expected.png") - image_bytes = image.tobytes() - expected_bytes = expected_image.tobytes() - assert image_bytes == expected_bytes, "Actual and expected images differ" diff --git a/tests/devices/unit_tests/test_grid_overlay.py b/tests/devices/unit_tests/oav/test_grid_overlay.py similarity index 100% rename from tests/devices/unit_tests/test_grid_overlay.py rename to tests/devices/unit_tests/oav/test_grid_overlay.py diff --git a/tests/devices/unit_tests/oav/test_snapshots.py b/tests/devices/unit_tests/oav/test_snapshots.py index 0a04dd41ee..de561550dd 100644 --- a/tests/devices/unit_tests/oav/test_snapshots.py +++ b/tests/devices/unit_tests/oav/test_snapshots.py @@ -31,11 +31,11 @@ async def grid_snapshot() -> SnapshotWithGrid: async with DeviceCollector(mock=True): grid_snapshot = SnapshotWithGrid("", "fake_grid") - await grid_snapshot.top_left_x.set(5) - await grid_snapshot.top_left_y.set(5) - await grid_snapshot.box_width.set(5) - await grid_snapshot.num_boxes_y.set(5) - await grid_snapshot.num_boxes_y.set(5) + await grid_snapshot.top_left_x.set(100) + await grid_snapshot.top_left_y.set(100) + await grid_snapshot.box_width.set(50) + await grid_snapshot.num_boxes_y.set(15) + await grid_snapshot.num_boxes_y.set(10) await grid_snapshot.directory.set("/tmp/") await grid_snapshot.filename.set("test") From 83429ac87fbd6abe85832d7cb731760bbd396c35 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Mon, 21 Oct 2024 14:17:02 +0100 Subject: [PATCH 26/53] Rename --- .../plugins/{MJPG_async.py => MJPG.py} | 0 .../oav/snapshots/snapshot_with_beam_centre.py | 2 +- .../devices/oav/snapshots/snapshot_with_grid.py | 2 +- .../oav/{test_oav_async.py => test_oav.py} | 0 tests/devices/unit_tests/oav/test_snapshots.py | 16 ++++++++-------- 5 files changed, 10 insertions(+), 10 deletions(-) rename src/dodal/devices/areadetector/plugins/{MJPG_async.py => MJPG.py} (100%) rename tests/devices/unit_tests/oav/{test_oav_async.py => test_oav.py} (100%) diff --git a/src/dodal/devices/areadetector/plugins/MJPG_async.py b/src/dodal/devices/areadetector/plugins/MJPG.py similarity index 100% rename from src/dodal/devices/areadetector/plugins/MJPG_async.py rename to src/dodal/devices/areadetector/plugins/MJPG.py diff --git a/src/dodal/devices/oav/snapshots/snapshot_with_beam_centre.py b/src/dodal/devices/oav/snapshots/snapshot_with_beam_centre.py index a96318459e..d312789fb3 100644 --- a/src/dodal/devices/oav/snapshots/snapshot_with_beam_centre.py +++ b/src/dodal/devices/oav/snapshots/snapshot_with_beam_centre.py @@ -1,7 +1,7 @@ from ophyd_async.core import SignalR from PIL import Image, ImageDraw -from dodal.devices.areadetector.plugins.MJPG_async import MJPG +from dodal.devices.areadetector.plugins.MJPG import MJPG class SnapshotWithBeamCentre(MJPG): diff --git a/src/dodal/devices/oav/snapshots/snapshot_with_grid.py b/src/dodal/devices/oav/snapshots/snapshot_with_grid.py index 4612e29803..8259d1de73 100644 --- a/src/dodal/devices/oav/snapshots/snapshot_with_grid.py +++ b/src/dodal/devices/oav/snapshots/snapshot_with_grid.py @@ -5,7 +5,7 @@ from ophyd_async.core import soft_signal_rw from PIL.Image import Image -from dodal.devices.areadetector.plugins.MJPG_async import MJPG +from dodal.devices.areadetector.plugins.MJPG import MJPG from dodal.devices.oav.snapshots.grid_overlay import ( add_grid_border_overlay_to_image, add_grid_overlay_to_image, diff --git a/tests/devices/unit_tests/oav/test_oav_async.py b/tests/devices/unit_tests/oav/test_oav.py similarity index 100% rename from tests/devices/unit_tests/oav/test_oav_async.py rename to tests/devices/unit_tests/oav/test_oav.py diff --git a/tests/devices/unit_tests/oav/test_snapshots.py b/tests/devices/unit_tests/oav/test_snapshots.py index de561550dd..e6724a96d1 100644 --- a/tests/devices/unit_tests/oav/test_snapshots.py +++ b/tests/devices/unit_tests/oav/test_snapshots.py @@ -42,10 +42,10 @@ async def grid_snapshot() -> SnapshotWithGrid: return grid_snapshot -@patch("dodal.devices.areadetector.plugins.MJPG_async.Image") +@patch("dodal.devices.areadetector.plugins.MJPG.Image") @patch("dodal.devices.oav.snapshots.snapshot_with_beam_centre.ImageDraw") @patch( - "dodal.devices.areadetector.plugins.MJPG_async.ClientSession.get", + "dodal.devices.areadetector.plugins.MJPG.ClientSession.get", autospec=True, ) async def test_snapshot_with_beam_centre_triggered_then_crosshair_drawn_and( @@ -69,13 +69,13 @@ async def test_snapshot_with_beam_centre_triggered_then_crosshair_drawn_and( mock_save.assert_awaited_once() -@patch("dodal.devices.areadetector.plugins.MJPG_async.Path.mkdir") -@patch("dodal.devices.areadetector.plugins.MJPG_async.Image") +@patch("dodal.devices.areadetector.plugins.MJPG.Path.mkdir") +@patch("dodal.devices.areadetector.plugins.MJPG.Image") @patch( - "dodal.devices.areadetector.plugins.MJPG_async.ClientSession.get", + "dodal.devices.areadetector.plugins.MJPG.ClientSession.get", autospec=True, ) -@patch("dodal.devices.areadetector.plugins.MJPG_async.aiofiles", autospec=True) +@patch("dodal.devices.areadetector.plugins.MJPG.aiofiles", autospec=True) async def test_snapshot_with_beam_centre_correctly_triggered_and_saved( mock_aiofiles, mock_get, patch_image, mock_mkdir, snapshot ): @@ -106,13 +106,13 @@ def test_snapshot_draws_expected_crosshair(tmp_path: Path): assert image_bytes == expected_bytes, "Actual and expected images differ" -@patch("dodal.devices.areadetector.plugins.MJPG_async.Image") +@patch("dodal.devices.areadetector.plugins.MJPG.Image") @patch( "dodal.devices.oav.snapshots.snapshot_with_grid.add_grid_border_overlay_to_image" ) @patch("dodal.devices.oav.snapshots.snapshot_with_grid.add_grid_overlay_to_image") @patch( - "dodal.devices.areadetector.plugins.MJPG_async.ClientSession.get", + "dodal.devices.areadetector.plugins.MJPG.ClientSession.get", autospec=True, ) async def test_snapshot_with_grid_triggered_saves_image_and_draws_grid( From c6a518bb4127e0b480f7076c839d6cb3afe0e5b3 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Mon, 21 Oct 2024 15:16:05 +0100 Subject: [PATCH 27/53] Instantiate async oav on beamlines --- src/dodal/beamlines/i03.py | 7 ++++--- src/dodal/beamlines/i04.py | 5 +++-- src/dodal/beamlines/i04_1.py | 5 +++-- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/dodal/beamlines/i03.py b/src/dodal/beamlines/i03.py index ec87595145..95847e2f95 100644 --- a/src/dodal/beamlines/i03.py +++ b/src/dodal/beamlines/i03.py @@ -25,7 +25,8 @@ 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, OAVConfigParams +from dodal.devices.oav.oav_detector import OAV +from dodal.devices.oav.oav_parameters import OAVConfig from dodal.devices.oav.pin_image_recognition import PinTipDetection from dodal.devices.qbpm import QBPM from dodal.devices.robot import BartRobot @@ -227,7 +228,7 @@ def panda_fast_grid_scan( def oav( wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False, - params: OAVConfigParams | None = None, + params: OAVConfig | None = None, ) -> OAV: """Get the i03 OAV device, instantiate it if it hasn't already been. If this is called when already instantiated in i03, it will return the existing object. @@ -238,7 +239,7 @@ def oav( "", wait_for_connection, fake_with_ophyd_sim, - params=params or OAVConfigParams(ZOOM_PARAMS_FILE, DISPLAY_CONFIG), + config=params or OAVConfig(ZOOM_PARAMS_FILE, DISPLAY_CONFIG), ) diff --git a/src/dodal/beamlines/i04.py b/src/dodal/beamlines/i04.py index 97f53e16c7..05be54208c 100644 --- a/src/dodal/beamlines/i04.py +++ b/src/dodal/beamlines/i04.py @@ -18,7 +18,8 @@ from dodal.devices.i04.transfocator import Transfocator from dodal.devices.ipin import IPin from dodal.devices.motors import XYZPositioner -from dodal.devices.oav.oav_detector import OAV, OAVConfigParams +from dodal.devices.oav.oav_detector import OAV +from dodal.devices.oav.oav_parameters import OAVConfig from dodal.devices.oav.oav_to_redis_forwarder import OAVToRedisForwarder from dodal.devices.robot import BartRobot from dodal.devices.s4_slit_gaps import S4SlitGaps @@ -354,7 +355,7 @@ def oav(wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False) -> "", wait_for_connection, fake_with_ophyd_sim, - params=OAVConfigParams(ZOOM_PARAMS_FILE, DISPLAY_CONFIG), + config=OAVConfig(ZOOM_PARAMS_FILE, DISPLAY_CONFIG), ) diff --git a/src/dodal/beamlines/i04_1.py b/src/dodal/beamlines/i04_1.py index 2341d9a033..ed3784ec0c 100644 --- a/src/dodal/beamlines/i04_1.py +++ b/src/dodal/beamlines/i04_1.py @@ -3,7 +3,8 @@ from dodal.devices.backlight import Backlight from dodal.devices.detector import DetectorParams from dodal.devices.eiger import EigerDetector -from dodal.devices.oav.oav_detector import OAV, OAVConfigParams +from dodal.devices.oav.oav_detector import OAV +from dodal.devices.oav.oav_parameters import OAVConfig from dodal.devices.s4_slit_gaps import S4SlitGaps from dodal.devices.synchrotron import Synchrotron from dodal.devices.undulator import Undulator @@ -75,7 +76,7 @@ def oav(wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False) -> "", wait_for_connection, fake_with_ophyd_sim, - params=OAVConfigParams(ZOOM_PARAMS_FILE, DISPLAY_CONFIG), + config=OAVConfig(ZOOM_PARAMS_FILE, DISPLAY_CONFIG), ) From 32944048419ca81fa91018dff87a29a3dbb1bf74 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Mon, 21 Oct 2024 17:03:13 +0100 Subject: [PATCH 28/53] Tidy up some tests --- .../devices/areadetector/plugins/MJPG.py | 8 +- src/dodal/devices/oav/oav_calculations.py | 22 ++++ src/dodal/devices/oav/utils.py | 34 ++++-- tests/devices/unit_tests/oav/conftest.py | 18 ++++ tests/devices/unit_tests/oav/test_oav.py | 32 +----- .../devices/unit_tests/oav/test_oav_utils.py | 80 ++++++++++++++ .../devices/unit_tests/oav/test_snapshots.py | 34 +++--- tests/devices/unit_tests/test_oav.py | 101 +----------------- 8 files changed, 178 insertions(+), 151 deletions(-) create mode 100644 tests/devices/unit_tests/oav/conftest.py create mode 100644 tests/devices/unit_tests/oav/test_oav_utils.py diff --git a/src/dodal/devices/areadetector/plugins/MJPG.py b/src/dodal/devices/areadetector/plugins/MJPG.py index dd759cdd6b..8a9589d56a 100644 --- a/src/dodal/devices/areadetector/plugins/MJPG.py +++ b/src/dodal/devices/areadetector/plugins/MJPG.py @@ -3,7 +3,7 @@ from pathlib import Path import aiofiles -from aiohttp import ClientSession +from aiohttp import ClientConnectionError, ClientSession from bluesky.protocols import Triggerable from ophyd_async.core import AsyncStatus, StandardReadable, soft_signal_rw from ophyd_async.epics.signal import epics_signal_r, epics_signal_rw @@ -69,8 +69,12 @@ async def trigger(self): async with ClientSession() as session: async with session.get(url_str) as response: + print(response.ok) if not response.ok: - LOGGER.warning( + LOGGER.error( + f"OAV responded with {response.status}: {response.reason}." + ) + raise ClientConnectionError( f"OAV responded with {response.status}: {response.reason}." ) try: diff --git a/src/dodal/devices/oav/oav_calculations.py b/src/dodal/devices/oav/oav_calculations.py index 6796e3a411..122a2cd68c 100644 --- a/src/dodal/devices/oav/oav_calculations.py +++ b/src/dodal/devices/oav/oav_calculations.py @@ -37,3 +37,25 @@ def camera_coordinates_to_xyz( z = vertical * sine return np.array([x, y, z], dtype=np.float64) + + +def calculate_beam_distance( + beam_centre: tuple[int, int], + horizontal_pixels: int, + vertical_pixels: int, +) -> tuple[int, int]: + """ + Calculates the distance between the beam centre and the given (horizontal, vertical). + + Args: + horizontal_pixels (int): The x (camera coordinates) value in pixels. + vertical_pixels (int): The y (camera coordinates) value in pixels. + Returns: + The distance between the beam centre and the (horizontal, vertical) point in pixels as a tuple + (horizontal_distance, vertical_distance). + """ + beam_x, beam_y = beam_centre + return ( + beam_x - horizontal_pixels, + beam_y - vertical_pixels, + ) diff --git a/src/dodal/devices/oav/utils.py b/src/dodal/devices/oav/utils.py index 9abeac6a2e..f1269f5657 100644 --- a/src/dodal/devices/oav/utils.py +++ b/src/dodal/devices/oav/utils.py @@ -5,7 +5,10 @@ import numpy as np from bluesky.utils import Msg -from dodal.devices.oav.oav_calculations import camera_coordinates_to_xyz +from dodal.devices.oav.oav_calculations import ( + calculate_beam_distance, + camera_coordinates_to_xyz, +) from dodal.devices.oav.oav_detector import OAV from dodal.devices.oav.pin_image_recognition import PinTipDetection from dodal.devices.smargon import Smargon @@ -78,22 +81,37 @@ def get_move_required_so_that_beam_is_at_pixel( ) current_angle = yield from bps.rd(smargon.omega) - return calculate_x_y_z_of_pixel(current_motor_xyz, current_angle, pixel, oav) + beam_x = yield from bps.rd(oav.beam_centre_i) + beam_y = yield from bps.rd(oav.beam_centre_j) + microns_per_pixel_x = yield from bps.rd(oav.microns_per_pixel_x) + microns_per_pixel_y = yield from bps.rd(oav.microns_per_pixel_y) + + return calculate_x_y_z_of_pixel( + current_motor_xyz, + current_angle, + pixel, + (beam_x, beam_y), + (microns_per_pixel_x, microns_per_pixel_y), + ) def calculate_x_y_z_of_pixel( - current_x_y_z, current_omega, pixel: Pixel, oav: OAV + current_x_y_z, + current_omega, + pixel: Pixel, + beam_centre: tuple[int, int], + microns_per_pixel: tuple[float, float], ) -> np.ndarray: - beam_distance_px: Pixel = oav.calculate_beam_distance(*pixel) + print(beam_centre) + print(microns_per_pixel) + beam_distance_px: Pixel = calculate_beam_distance((beam_centre), *pixel) - assert oav.microns_per_pixel_x - assert oav.microns_per_pixel_y return current_x_y_z + camera_coordinates_to_xyz( beam_distance_px[0], beam_distance_px[1], current_omega, - oav.microns_per_pixel_x, - oav.microns_per_pixel_y, + microns_per_pixel[0], + microns_per_pixel[1], ) diff --git a/tests/devices/unit_tests/oav/conftest.py b/tests/devices/unit_tests/oav/conftest.py new file mode 100644 index 0000000000..70f24b9efa --- /dev/null +++ b/tests/devices/unit_tests/oav/conftest.py @@ -0,0 +1,18 @@ +import pytest +from ophyd_async.core import DeviceCollector, set_mock_value + +from dodal.devices.oav.oav_detector import OAV +from dodal.devices.oav.oav_parameters import OAVConfig + +DISPLAY_CONFIGURATION = "tests/devices/unit_tests/test_display.configuration" +ZOOM_LEVELS_XML = "tests/devices/unit_tests/test_jCameraManZoomLevels.xml" + + +@pytest.fixture +async def oav() -> OAV: + oav_config = OAVConfig(ZOOM_LEVELS_XML, DISPLAY_CONFIGURATION) + async with DeviceCollector(mock=True, connect=True): + oav = OAV("", config=oav_config, name="fake_oav") + set_mock_value(oav.grid_snapshot.x_size, 1024) + set_mock_value(oav.grid_snapshot.y_size, 768) + return oav diff --git a/tests/devices/unit_tests/oav/test_oav.py b/tests/devices/unit_tests/oav/test_oav.py index 1d1646a879..810387714f 100644 --- a/tests/devices/unit_tests/oav/test_oav.py +++ b/tests/devices/unit_tests/oav/test_oav.py @@ -1,21 +1,7 @@ import pytest -from ophyd_async.core import DeviceCollector, set_mock_value +from ophyd_async.core import set_mock_value from dodal.devices.oav.oav_detector import OAV, ZoomController -from dodal.devices.oav.oav_parameters import OAVConfig - -DISPLAY_CONFIGURATION = "tests/devices/unit_tests/test_display.configuration" -ZOOM_LEVELS_XML = "tests/devices/unit_tests/test_jCameraManZoomLevels.xml" - - -@pytest.fixture -async def oav() -> OAV: - oav_config = OAVConfig(ZOOM_LEVELS_XML, DISPLAY_CONFIGURATION) - async with DeviceCollector(mock=True, connect=True): - oav = OAV("", config=oav_config, name="fake_oav") - set_mock_value(oav.grid_snapshot.x_size, 1024) - set_mock_value(oav.grid_snapshot.y_size, 768) - return oav async def test_zoom_controller(): @@ -84,18 +70,4 @@ async def test_oav_returns_rescaled_beam_position_and_microns_per_pixel_correctl assert beam_y == 450 -@pytest.mark.parametrize( - "h, v, expected_x, expected_y", - [ - (54, 100, 517 - 54, 350 - 100), - (0, 0, 517, 350), - (500, 500, 517 - 500, 350 - 500), - ], -) -async def test_calculate_beam_distance(h, v, expected_x, expected_y, oav: OAV): - set_mock_value(oav.zoom_controller.level, "5.0x") # type: ignore - - assert await oav.calculate_beam_distance( - h, - v, - ) == (expected_x, expected_y) +# TODO add test for bad response of snapshot diff --git a/tests/devices/unit_tests/oav/test_oav_utils.py b/tests/devices/unit_tests/oav/test_oav_utils.py new file mode 100644 index 0000000000..38fc556ba9 --- /dev/null +++ b/tests/devices/unit_tests/oav/test_oav_utils.py @@ -0,0 +1,80 @@ +from unittest.mock import AsyncMock + +import numpy as np +import pytest +from bluesky.run_engine import RunEngine +from ophyd.sim import instantiate_fake_device + +from dodal.devices.oav.oav_calculations import calculate_beam_distance +from dodal.devices.oav.oav_detector import OAV +from dodal.devices.oav.pin_image_recognition import PinTipDetection +from dodal.devices.oav.pin_image_recognition.utils import SampleLocation +from dodal.devices.oav.utils import ( + PinNotFoundException, + bottom_right_from_top_left, + wait_for_tip_to_be_found, +) + + +def test_bottom_right_from_top_left(): + top_left = np.array([123, 123]) + bottom_right = bottom_right_from_top_left( + top_left, 20, 30, 0.1, 0.15, 2.7027, 2.7027 + ) + assert bottom_right[0] == 863 and bottom_right[1] == 1788 + bottom_right = bottom_right_from_top_left(top_left, 15, 20, 0.005, 0.007, 1, 1) + assert bottom_right[0] == 198 and bottom_right[1] == 263 + + +@pytest.mark.parametrize( + "h, v, expected_x, expected_y", + [ + (54, 100, 517 - 54, 350 - 100), + (0, 0, 517, 350), + (500, 500, 517 - 500, 350 - 500), + ], +) +def test_calculate_beam_distance(h, v, expected_x, expected_y, oav: OAV): + # Beam center from test files for zoom_level = 5.0x + beam_centre = (517, 350) + + assert calculate_beam_distance( + beam_centre, + h, + v, + ) == (expected_x, expected_y) + + +# TODO add test_values_for_move_so_that_beam_is_at_pixel + + +@pytest.mark.asyncio +async def test_given_tip_found_when_wait_for_tip_to_be_found_called_then_tip_immediately_returned(): + mock_pin_tip_detect: PinTipDetection = instantiate_fake_device( + PinTipDetection, name="pin_detect" + ) + await mock_pin_tip_detect.connect(mock=True) + mock_pin_tip_detect._get_tip_and_edge_data = AsyncMock( + return_value=SampleLocation(100, 100, np.array([]), np.array([])) + ) + RE = RunEngine(call_returns_result=True) + result = RE(wait_for_tip_to_be_found(mock_pin_tip_detect)) + assert result.plan_result == (100, 100) # type: ignore + mock_pin_tip_detect._get_tip_and_edge_data.assert_called_once() + + +@pytest.mark.asyncio +async def test_given_no_tip_when_wait_for_tip_to_be_found_called_then_exception_thrown(): + mock_pin_tip_detect: PinTipDetection = instantiate_fake_device( + PinTipDetection, name="pin_detect" + ) + await mock_pin_tip_detect.connect(mock=True) + await mock_pin_tip_detect.validity_timeout.set(0.2) + mock_pin_tip_detect._get_tip_and_edge_data = AsyncMock( + return_value=SampleLocation( + *PinTipDetection.INVALID_POSITION, np.array([]), np.array([]) + ) + ) + RE = RunEngine(call_returns_result=True) + with pytest.raises(PinNotFoundException): + RE(wait_for_tip_to_be_found(mock_pin_tip_detect)) diff --git a/tests/devices/unit_tests/oav/test_snapshots.py b/tests/devices/unit_tests/oav/test_snapshots.py index e6724a96d1..eba915d2cc 100644 --- a/tests/devices/unit_tests/oav/test_snapshots.py +++ b/tests/devices/unit_tests/oav/test_snapshots.py @@ -1,5 +1,5 @@ from pathlib import Path -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import ANY, AsyncMock, MagicMock, patch import pytest from ophyd_async.core import DeviceCollector, MockSignalBackend, SignalR, set_mock_value @@ -21,8 +21,9 @@ async def snapshot() -> SnapshotWithBeamCentre: mock_beam_y = create_and_set_mock_signal_r(int, "mock_beam_y", 380) async with DeviceCollector(mock=True): snapshot = SnapshotWithBeamCentre("", mock_beam_x, mock_beam_y, "fake_snapshot") - await snapshot.directory.set("/tmp/") - await snapshot.filename.set("test") + set_mock_value(snapshot.directory, "/tmp/") + set_mock_value(snapshot.filename, "test") + set_mock_value(snapshot.url, "http://test.url") return snapshot @@ -31,14 +32,15 @@ async def grid_snapshot() -> SnapshotWithGrid: async with DeviceCollector(mock=True): grid_snapshot = SnapshotWithGrid("", "fake_grid") - await grid_snapshot.top_left_x.set(100) - await grid_snapshot.top_left_y.set(100) - await grid_snapshot.box_width.set(50) - await grid_snapshot.num_boxes_y.set(15) - await grid_snapshot.num_boxes_y.set(10) + set_mock_value(grid_snapshot.top_left_x, 100) + set_mock_value(grid_snapshot.top_left_y, 100) + set_mock_value(grid_snapshot.box_width, 50) + set_mock_value(grid_snapshot.num_boxes_x, 15) + set_mock_value(grid_snapshot.num_boxes_y, 10) - await grid_snapshot.directory.set("/tmp/") - await grid_snapshot.filename.set("test") + set_mock_value(grid_snapshot.directory, "/tmp/") + set_mock_value(grid_snapshot.filename, "test") + set_mock_value(grid_snapshot.url, "http://test.url") return grid_snapshot @@ -129,9 +131,17 @@ async def test_snapshot_with_grid_triggered_saves_image_and_draws_grid( grid_snapshot._save_grid_to_image = (mock_save_grid := AsyncMock()) await grid_snapshot.trigger() + + test_url = await grid_snapshot.url.get_value() + # Get called with an instance of the session and correct url + mock_get.assert_called_once_with(ANY, test_url) mock_save.assert_awaited_once() - patch_add_border.assert_called_once() - patch_add_grid.assert_called_once() + patch_add_border.assert_called_once_with( + mock_open.return_value.__enter__.return_value, 100, 100, 50, 15, 10 + ) + patch_add_grid.assert_called_once_with( + mock_open.return_value.__enter__.return_value, 100, 100, 50, 15, 10 + ) assert mock_save_grid.await_count == 2 assert ( await grid_snapshot.last_path_outer.get_value() == "/tmp/test_outer_overlay.png" diff --git a/tests/devices/unit_tests/test_oav.py b/tests/devices/unit_tests/test_oav.py index 46efa5516d..78a37056df 100644 --- a/tests/devices/unit_tests/test_oav.py +++ b/tests/devices/unit_tests/test_oav.py @@ -1,23 +1,11 @@ -from unittest.mock import AsyncMock, MagicMock, call, patch +from unittest.mock import MagicMock, call, patch -import numpy as np import pytest -from bluesky.run_engine import RunEngine -from ophyd.sim import instantiate_fake_device, make_fake_device -from ophyd_async.core import set_mock_value +from ophyd.sim import make_fake_device from PIL import Image from requests import HTTPError, Response -import dodal.devices.oav.utils as oav_utils from dodal.devices.oav.oav_detector import OAV, OAVConfigParams -from dodal.devices.oav.pin_image_recognition import PinTipDetection -from dodal.devices.oav.pin_image_recognition.utils import SampleLocation -from dodal.devices.oav.utils import ( - PinNotFoundException, - get_move_required_so_that_beam_is_at_pixel, - wait_for_tip_to_be_found, -) -from dodal.devices.smargon import Smargon DISPLAY_CONFIGURATION = "tests/devices/unit_tests/test_display.configuration" ZOOM_LEVELS_XML = "tests/devices/unit_tests/test_jCameraManZoomLevels.xml" @@ -152,18 +140,6 @@ def test_correct_grid_drawn_on_image( assert actual_grid_calls == expected_grid_calls -def test_bottom_right_from_top_left(): - top_left = np.array([123, 123]) - bottom_right = oav_utils.bottom_right_from_top_left( - top_left, 20, 30, 0.1, 0.15, 2.7027, 2.7027 - ) - assert bottom_right[0] == 863 and bottom_right[1] == 1788 - bottom_right = oav_utils.bottom_right_from_top_left( - top_left, 15, 20, 0.005, 0.007, 1, 1 - ) - assert bottom_right[0] == 198 and bottom_right[1] == 263 - - def test_get_beam_position_from_zoom_only_called_once_on_multiple_connects( fake_oav: OAV, ): @@ -187,76 +163,3 @@ def test_get_beam_position_from_zoom_only_called_once_on_multiple_connects( ): fake_oav.zoom_controller.level.sim_put("2.0x") # type: ignore assert mock_get_beam_position_from_zoom.call_count == 1 - - -@pytest.mark.parametrize( - "px_per_um, beam_centre, angle, pixel_to_move_to, expected_xyz", - [ - # Simple case of beam being in the top left and each pixel being 1 mm - ([1000, 1000], [0, 0], 0, [100, 190], [100, 190, 0]), - ([1000, 1000], [0, 0], -90, [50, 250], [50, 0, 250]), - ([1000, 1000], [0, 0], 90, [-60, 450], [-60, 0, -450]), - # Beam offset - ([1000, 1000], [100, 100], 0, [100, 100], [0, 0, 0]), - ([1000, 1000], [100, 100], -90, [50, 250], [-50, 0, 150]), - # Pixels_per_micron different - ([10, 50], [0, 0], 0, [100, 190], [1, 9.5, 0]), - ([60, 80], [0, 0], -90, [50, 250], [3, 0, 20]), - ], -) -def test_values_for_move_so_that_beam_is_at_pixel( - smargon: Smargon, - fake_oav: OAV, - px_per_um, - beam_centre, - angle, - pixel_to_move_to, - expected_xyz, -): - fake_oav.parameters.micronsPerXPixel = px_per_um[0] - fake_oav.parameters.micronsPerYPixel = px_per_um[1] - fake_oav.parameters.beam_centre_i = beam_centre[0] - fake_oav.parameters.beam_centre_j = beam_centre[1] - - set_mock_value(smargon.omega.user_readback, angle) - - RE = RunEngine(call_returns_result=True) - pos = RE( - get_move_required_so_that_beam_is_at_pixel( - smargon, pixel_to_move_to, fake_oav.parameters - ) - ).plan_result # type: ignore - - assert pos == pytest.approx(expected_xyz) - - -@pytest.mark.asyncio -async def test_given_tip_found_when_wait_for_tip_to_be_found_called_then_tip_immediately_returned(): - mock_pin_tip_detect: PinTipDetection = instantiate_fake_device( - PinTipDetection, name="pin_detect" - ) - await mock_pin_tip_detect.connect(mock=True) - mock_pin_tip_detect._get_tip_and_edge_data = AsyncMock( - return_value=SampleLocation(100, 100, np.array([]), np.array([])) - ) - RE = RunEngine(call_returns_result=True) - result = RE(wait_for_tip_to_be_found(mock_pin_tip_detect)) - assert result.plan_result == (100, 100) # type: ignore - mock_pin_tip_detect._get_tip_and_edge_data.assert_called_once() - - -@pytest.mark.asyncio -async def test_given_no_tip_when_wait_for_tip_to_be_found_called_then_exception_thrown(): - mock_pin_tip_detect: PinTipDetection = instantiate_fake_device( - PinTipDetection, name="pin_detect" - ) - await mock_pin_tip_detect.connect(mock=True) - await mock_pin_tip_detect.validity_timeout.set(0.2) - mock_pin_tip_detect._get_tip_and_edge_data = AsyncMock( - return_value=SampleLocation( - *PinTipDetection.INVALID_POSITION, np.array([]), np.array([]) - ) - ) - RE = RunEngine(call_returns_result=True) - with pytest.raises(PinNotFoundException): - RE(wait_for_tip_to_be_found(mock_pin_tip_detect)) From 1899d5de04915a4efca23ae8dfa2158caffcd46d Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Mon, 21 Oct 2024 17:19:57 +0100 Subject: [PATCH 29/53] Add commented out test --- tests/devices/unit_tests/oav/conftest.py | 1 + .../devices/unit_tests/oav/test_oav_utils.py | 47 ++++++++++++++++++- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/tests/devices/unit_tests/oav/conftest.py b/tests/devices/unit_tests/oav/conftest.py index 70f24b9efa..af6aafebed 100644 --- a/tests/devices/unit_tests/oav/conftest.py +++ b/tests/devices/unit_tests/oav/conftest.py @@ -15,4 +15,5 @@ async def oav() -> OAV: oav = OAV("", config=oav_config, name="fake_oav") set_mock_value(oav.grid_snapshot.x_size, 1024) set_mock_value(oav.grid_snapshot.y_size, 768) + set_mock_value(oav.zoom_controller.level, "1.0x") return oav diff --git a/tests/devices/unit_tests/oav/test_oav_utils.py b/tests/devices/unit_tests/oav/test_oav_utils.py index 38fc556ba9..54c46e2c68 100644 --- a/tests/devices/unit_tests/oav/test_oav_utils.py +++ b/tests/devices/unit_tests/oav/test_oav_utils.py @@ -5,6 +5,7 @@ from bluesky.run_engine import RunEngine from ophyd.sim import instantiate_fake_device +# from ophyd_async.core import set_mock_value from dodal.devices.oav.oav_calculations import calculate_beam_distance from dodal.devices.oav.oav_detector import OAV from dodal.devices.oav.pin_image_recognition import PinTipDetection @@ -13,8 +14,11 @@ PinNotFoundException, bottom_right_from_top_left, wait_for_tip_to_be_found, + # get_move_required_so_that_beam_is_at_pixel, ) +# from dodal.devices.smargon import Smargon + def test_bottom_right_from_top_left(): top_left = np.array([123, 123]) @@ -45,7 +49,48 @@ def test_calculate_beam_distance(h, v, expected_x, expected_y, oav: OAV): ) == (expected_x, expected_y) -# TODO add test_values_for_move_so_that_beam_is_at_pixel +# TODO, can't set beam center and micron as I want, will need to calculate +# the right values. +# @pytest.mark.parametrize( +# "px_per_um, beam_centre, angle, pixel_to_move_to, expected_xyz", +# [ +# # Simple case of beam being in the top left and each pixel being 1 mm +# ([1000, 1000], [0, 0], 0, [100, 190], [100, 190, 0]), +# ([1000, 1000], [0, 0], -90, [50, 250], [50, 0, 250]), +# ([1000, 1000], [0, 0], 90, [-60, 450], [-60, 0, -450]), +# # Beam offset +# ([1000, 1000], [100, 100], 0, [100, 100], [0, 0, 0]), +# ([1000, 1000], [100, 100], -90, [50, 250], [-50, 0, 150]), +# # Pixels_per_micron different +# ([10, 50], [0, 0], 0, [100, 190], [1, 9.5, 0]), +# ([60, 80], [0, 0], -90, [50, 250], [3, 0, 20]), +# ], +# ) +# async def test_values_for_move_so_that_beam_is_at_pixel( +# smargon: Smargon, +# oav: OAV, +# px_per_um, +# beam_centre, +# angle, +# pixel_to_move_to, +# expected_xyz, +# ): +# await oav.microns_per_pixel_x._backend.put(px_per_um[0]) +# # set_mock_value(oav.microns_per_pixel_x, px_per_um[0]) +# set_mock_value(oav.microns_per_pixel_y, px_per_um[1]) +# set_mock_value(oav.beam_centre_i, beam_centre[0]) +# set_mock_value(oav.beam_centre_j, beam_centre[1]) + +# set_mock_value(smargon.omega.user_readback, angle) + +# RE = RunEngine(call_returns_result=True) +# pos = RE( +# get_move_required_so_that_beam_is_at_pixel( +# smargon, pixel_to_move_to, oav +# ) +# ).plan_result # type: ignore + +# assert pos == pytest.approx(expected_xyz) @pytest.mark.asyncio From 35d4af70ef3091cb853ba949da857c7a16e3a66c Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Wed, 23 Oct 2024 09:58:56 +0100 Subject: [PATCH 30/53] Add a sort of similar test for utils --- src/dodal/devices/oav/utils.py | 2 + .../devices/unit_tests/oav/test_oav_utils.py | 75 ++++++++----------- 2 files changed, 32 insertions(+), 45 deletions(-) diff --git a/src/dodal/devices/oav/utils.py b/src/dodal/devices/oav/utils.py index f1269f5657..d0311e680b 100644 --- a/src/dodal/devices/oav/utils.py +++ b/src/dodal/devices/oav/utils.py @@ -79,6 +79,7 @@ def get_move_required_so_that_beam_is_at_pixel( ], dtype=np.float64, ) + print(current_motor_xyz) current_angle = yield from bps.rd(smargon.omega) beam_x = yield from bps.rd(oav.beam_centre_i) @@ -105,6 +106,7 @@ def calculate_x_y_z_of_pixel( print(beam_centre) print(microns_per_pixel) beam_distance_px: Pixel = calculate_beam_distance((beam_centre), *pixel) + print(beam_distance_px) return current_x_y_z + camera_coordinates_to_xyz( beam_distance_px[0], diff --git a/tests/devices/unit_tests/oav/test_oav_utils.py b/tests/devices/unit_tests/oav/test_oav_utils.py index 54c46e2c68..e3d576016c 100644 --- a/tests/devices/unit_tests/oav/test_oav_utils.py +++ b/tests/devices/unit_tests/oav/test_oav_utils.py @@ -4,8 +4,8 @@ import pytest from bluesky.run_engine import RunEngine from ophyd.sim import instantiate_fake_device +from ophyd_async.core import set_mock_value -# from ophyd_async.core import set_mock_value from dodal.devices.oav.oav_calculations import calculate_beam_distance from dodal.devices.oav.oav_detector import OAV from dodal.devices.oav.pin_image_recognition import PinTipDetection @@ -13,11 +13,10 @@ from dodal.devices.oav.utils import ( PinNotFoundException, bottom_right_from_top_left, + get_move_required_so_that_beam_is_at_pixel, wait_for_tip_to_be_found, - # get_move_required_so_that_beam_is_at_pixel, ) - -# from dodal.devices.smargon import Smargon +from dodal.devices.smargon import Smargon def test_bottom_right_from_top_left(): @@ -49,48 +48,34 @@ def test_calculate_beam_distance(h, v, expected_x, expected_y, oav: OAV): ) == (expected_x, expected_y) -# TODO, can't set beam center and micron as I want, will need to calculate -# the right values. -# @pytest.mark.parametrize( -# "px_per_um, beam_centre, angle, pixel_to_move_to, expected_xyz", -# [ -# # Simple case of beam being in the top left and each pixel being 1 mm -# ([1000, 1000], [0, 0], 0, [100, 190], [100, 190, 0]), -# ([1000, 1000], [0, 0], -90, [50, 250], [50, 0, 250]), -# ([1000, 1000], [0, 0], 90, [-60, 450], [-60, 0, -450]), -# # Beam offset -# ([1000, 1000], [100, 100], 0, [100, 100], [0, 0, 0]), -# ([1000, 1000], [100, 100], -90, [50, 250], [-50, 0, 150]), -# # Pixels_per_micron different -# ([10, 50], [0, 0], 0, [100, 190], [1, 9.5, 0]), -# ([60, 80], [0, 0], -90, [50, 250], [3, 0, 20]), -# ], -# ) -# async def test_values_for_move_so_that_beam_is_at_pixel( -# smargon: Smargon, -# oav: OAV, -# px_per_um, -# beam_centre, -# angle, -# pixel_to_move_to, -# expected_xyz, -# ): -# await oav.microns_per_pixel_x._backend.put(px_per_um[0]) -# # set_mock_value(oav.microns_per_pixel_x, px_per_um[0]) -# set_mock_value(oav.microns_per_pixel_y, px_per_um[1]) -# set_mock_value(oav.beam_centre_i, beam_centre[0]) -# set_mock_value(oav.beam_centre_j, beam_centre[1]) - -# set_mock_value(smargon.omega.user_readback, angle) - -# RE = RunEngine(call_returns_result=True) -# pos = RE( -# get_move_required_so_that_beam_is_at_pixel( -# smargon, pixel_to_move_to, oav -# ) -# ).plan_result # type: ignore +@pytest.mark.parametrize( + "zoom_level, angle, pixel_to_move_to, expected_xyz", + [ + # Different zoom levels -> different um_per_pix and beam_centre + ("5.0x", 0, (100, 190), (-0.659, -0.253, 0)), + ("1.0x", 0, (100, 190), (-1.082, -0.485, 0)), + # Different position to reach, same zoom level + ("1.0x", 0, (50, 250), (-1.226, -0.313, 0)), + # Change angle + ("5.0x", 45, (100, 190), (-0.659, -0.179, 0.179)), + ], +) +async def test_values_for_move_so_that_beam_is_at_pixel( + zoom_level: str, + angle: int, + pixel_to_move_to: tuple, + expected_xyz: tuple, + oav: OAV, + smargon: Smargon, +): + set_mock_value(oav.zoom_controller.level, zoom_level) + set_mock_value(smargon.omega.user_readback, angle) + RE = RunEngine(call_returns_result=True) + pos = RE( + get_move_required_so_that_beam_is_at_pixel(smargon, pixel_to_move_to, oav) + ).plan_result # type: ignore -# assert pos == pytest.approx(expected_xyz) + assert pos == pytest.approx(expected_xyz, abs=1e-3) @pytest.mark.asyncio From e0546c4d8f3bfb058110c8c4fcaed06eff7a967a Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Wed, 23 Oct 2024 10:00:03 +0100 Subject: [PATCH 31/53] Remove some prints --- src/dodal/devices/oav/utils.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/dodal/devices/oav/utils.py b/src/dodal/devices/oav/utils.py index d0311e680b..f1ac168739 100644 --- a/src/dodal/devices/oav/utils.py +++ b/src/dodal/devices/oav/utils.py @@ -79,7 +79,6 @@ def get_move_required_so_that_beam_is_at_pixel( ], dtype=np.float64, ) - print(current_motor_xyz) current_angle = yield from bps.rd(smargon.omega) beam_x = yield from bps.rd(oav.beam_centre_i) @@ -103,10 +102,7 @@ def calculate_x_y_z_of_pixel( beam_centre: tuple[int, int], microns_per_pixel: tuple[float, float], ) -> np.ndarray: - print(beam_centre) - print(microns_per_pixel) beam_distance_px: Pixel = calculate_beam_distance((beam_centre), *pixel) - print(beam_distance_px) return current_x_y_z + camera_coordinates_to_xyz( beam_distance_px[0], From 84a709841757b8620ebc521948d12a2280c44e0b Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Wed, 23 Oct 2024 10:30:29 +0100 Subject: [PATCH 32/53] Use async oav in system test --- src/dodal/devices/oav/oav_detector.py | 21 ---------------- .../devices/oav/snapshots/grid_overlay.py | 1 - system_tests/test_oav_system.py | 20 ++++++++------- .../unit_tests/oav/test_oav_parameters.py | 15 ----------- tests/devices/unit_tests/test_oav.py | 25 ------------------- 5 files changed, 11 insertions(+), 71 deletions(-) diff --git a/src/dodal/devices/oav/oav_detector.py b/src/dodal/devices/oav/oav_detector.py index 643e89727f..3b10d1c345 100644 --- a/src/dodal/devices/oav/oav_detector.py +++ b/src/dodal/devices/oav/oav_detector.py @@ -97,24 +97,3 @@ async def _get_beam_position(self, coord: int) -> int: value = self.parameters[_zoom].crosshair[coord] size = await self.sizes[coord].get_value() return int(value * size / DEFAULT_OAV_WINDOW[coord]) - - async def calculate_beam_distance( - self, horizontal_pixels: int, vertical_pixels: int - ) -> tuple[int, int]: - """ - Calculates the distance between the beam centre and the given (horizontal, vertical). - - Args: - horizontal_pixels (int): The x (camera coordinates) value in pixels. - vertical_pixels (int): The y (camera coordinates) value in pixels. - Returns: - The distance between the beam centre and the (horizontal, vertical) point in pixels as a tuple - (horizontal_distance, vertical_distance). - """ - beam_x = await self.beam_centre_i.get_value() - beam_y = await self.beam_centre_j.get_value() - - return ( - beam_x - horizontal_pixels, - beam_y - vertical_pixels, - ) diff --git a/src/dodal/devices/oav/snapshots/grid_overlay.py b/src/dodal/devices/oav/snapshots/grid_overlay.py index 1f82312e5b..56eb048727 100644 --- a/src/dodal/devices/oav/snapshots/grid_overlay.py +++ b/src/dodal/devices/oav/snapshots/grid_overlay.py @@ -1,4 +1,3 @@ -# type: ignore # OAV will soon be ophyd-async, see https://github.com/DiamondLightSource/dodal/issues/716 from enum import Enum from functools import partial diff --git a/system_tests/test_oav_system.py b/system_tests/test_oav_system.py index 8768594416..e722823129 100644 --- a/system_tests/test_oav_system.py +++ b/system_tests/test_oav_system.py @@ -2,7 +2,7 @@ import pytest from bluesky.run_engine import RunEngine -from dodal.devices.oav.oav_detector import OAV, OAVConfigParams, ZoomController +from dodal.devices.oav.oav_detector import OAV, OAVConfig, ZoomController TEST_GRID_TOP_LEFT_X = 100 TEST_GRID_TOP_LEFT_Y = 100 @@ -15,8 +15,8 @@ ZOOM_LEVELS_XML = "tests/devices/unit_tests/test_jCameraManZoomLevels.xml" -def take_snapshot_with_grid(oav: OAV, snapshot_filename, snapshot_directory): - oav.wait_for_connection() +async def take_snapshot_with_grid(oav: OAV, snapshot_filename, snapshot_directory): + await oav.connect() yield from bps.abs_set(oav.grid_snapshot.top_left_x, TEST_GRID_TOP_LEFT_X) # type: ignore # See: https://github.com/DiamondLightSource/dodal/issues/827 yield from bps.abs_set(oav.grid_snapshot.top_left_y, TEST_GRID_TOP_LEFT_Y) # type: ignore # See: https://github.com/DiamondLightSource/dodal/issues/827 yield from bps.abs_set(oav.grid_snapshot.box_width, TEST_GRID_BOX_WIDTH) # type: ignore # See: https://github.com/DiamondLightSource/dodal/issues/827 @@ -29,10 +29,11 @@ def take_snapshot_with_grid(oav: OAV, snapshot_filename, snapshot_directory): # We need to find a better way of integrating this, see https://github.com/DiamondLightSource/mx-bluesky/issues/183 @pytest.mark.skip(reason="Don't want to actually take snapshots during testing.") -def test_grid_overlay(RE: RunEngine): +async def test_grid_overlay(RE: RunEngine): beamline = "BL03I" - oav_params = OAVConfigParams(ZOOM_LEVELS_XML, DISPLAY_CONFIGURATION) - oav = OAV(name="oav", prefix=f"{beamline}", params=oav_params) + oav_params = OAVConfig(ZOOM_LEVELS_XML, DISPLAY_CONFIGURATION) + oav = OAV(name="oav", prefix=f"{beamline}", config=oav_params) + await oav.connect() snapshot_filename = "snapshot" snapshot_directory = "." RE(take_snapshot_with_grid(oav, snapshot_filename, snapshot_directory)) @@ -40,7 +41,8 @@ def test_grid_overlay(RE: RunEngine): @pytest.mark.skip(reason="No OAV in S03") @pytest.mark.s03 -def test_get_zoom_levels(): +async def test_get_zoom_levels(): my_zoom_controller = ZoomController("BL03S-EA-OAV-01:FZOOM:", name="test_zoom") - my_zoom_controller.wait_for_connection() - assert my_zoom_controller.allowed_zoom_levels[0] == "1.0x" + await my_zoom_controller.connect() + description = await my_zoom_controller.level.describe() + assert description["zoom_controller-level"]["choices"][0] == "1.0x" # type: ignore diff --git a/tests/devices/unit_tests/oav/test_oav_parameters.py b/tests/devices/unit_tests/oav/test_oav_parameters.py index 421906baaf..aba5771d4d 100644 --- a/tests/devices/unit_tests/oav/test_oav_parameters.py +++ b/tests/devices/unit_tests/oav/test_oav_parameters.py @@ -2,7 +2,6 @@ from dodal.devices.oav.oav_parameters import ( OAVConfig, - OAVConfigParams, OAVParameters, ZoomParams, ) @@ -43,20 +42,6 @@ def test_given_key_in_context_and_default_when_load_parameters_then_value_found_ assert mock_parameters.minimum_height == 10 -def test_given_context_and_microns_per_pixel_get_max_tip_distance_in_pixels( - mock_parameters: OAVParameters, -): - zoom_level = mock_parameters.zoom - config_params = OAVConfigParams(ZOOM_LEVELS_XML, DISPLAY_CONFIGURATION) - config_params.update_on_zoom(str(zoom_level), 1024, 768) - - assert mock_parameters.max_tip_distance == 300 - assert config_params.micronsPerXPixel - assert mock_parameters.get_max_tip_distance_in_pixels( - config_params.micronsPerXPixel - ) == pytest.approx(189.873, abs=1e-3) - - @pytest.mark.parametrize( "zoom_level, expected_microns, expected_crosshair", [ diff --git a/tests/devices/unit_tests/test_oav.py b/tests/devices/unit_tests/test_oav.py index 78a37056df..c9e88d0166 100644 --- a/tests/devices/unit_tests/test_oav.py +++ b/tests/devices/unit_tests/test_oav.py @@ -138,28 +138,3 @@ def test_correct_grid_drawn_on_image( actual_grid_calls = mock_grid_overlay.mock_calls assert actual_border_calls == expected_border_calls assert actual_grid_calls == expected_grid_calls - - -def test_get_beam_position_from_zoom_only_called_once_on_multiple_connects( - fake_oav: OAV, -): - fake_oav.wait_for_connection() - fake_oav.wait_for_connection() - fake_oav.wait_for_connection() - - with ( - patch( - "dodal.devices.oav.oav_detector.OAVConfigParams.update_on_zoom", - MagicMock(), - ), - patch( - "dodal.devices.oav.oav_detector.OAVConfigParams.get_beam_position_from_zoom", - MagicMock(), - ) as mock_get_beam_position_from_zoom, - patch( - "dodal.devices.oav.oav_detector.OAVConfigParams.load_microns_per_pixel", - MagicMock(), - ), - ): - fake_oav.zoom_controller.level.sim_put("2.0x") # type: ignore - assert mock_get_beam_position_from_zoom.call_count == 1 From 0f44f68ba6d708984c9c0242535da32800827d19 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Wed, 23 Oct 2024 11:59:03 +0100 Subject: [PATCH 33/53] Remove some old tests and add to new ones --- .../devices/areadetector/plugins/MJPG.py | 1 - tests/devices/unit_tests/oav/test_oav.py | 14 +++- .../devices/unit_tests/oav/test_snapshots.py | 9 ++- tests/devices/unit_tests/test_oav.py | 77 +------------------ 4 files changed, 21 insertions(+), 80 deletions(-) diff --git a/src/dodal/devices/areadetector/plugins/MJPG.py b/src/dodal/devices/areadetector/plugins/MJPG.py index 8a9589d56a..ee4c753547 100644 --- a/src/dodal/devices/areadetector/plugins/MJPG.py +++ b/src/dodal/devices/areadetector/plugins/MJPG.py @@ -69,7 +69,6 @@ async def trigger(self): async with ClientSession() as session: async with session.get(url_str) as response: - print(response.ok) if not response.ok: LOGGER.error( f"OAV responded with {response.status}: {response.reason}." diff --git a/tests/devices/unit_tests/oav/test_oav.py b/tests/devices/unit_tests/oav/test_oav.py index 810387714f..1ff32e46c1 100644 --- a/tests/devices/unit_tests/oav/test_oav.py +++ b/tests/devices/unit_tests/oav/test_oav.py @@ -1,3 +1,6 @@ +from unittest.mock import AsyncMock, patch + +import aiohttp import pytest from ophyd_async.core import set_mock_value @@ -70,4 +73,13 @@ async def test_oav_returns_rescaled_beam_position_and_microns_per_pixel_correctl assert beam_y == 450 -# TODO add test for bad response of snapshot +@patch( + "dodal.devices.areadetector.plugins.MJPG.ClientSession.get", + autospec=True, +) +async def test_snapshot_trigger_handles_bad_request_status(mock_get, oav: OAV): + mock_get.return_value.__aenter__.return_value = (mock_response := AsyncMock()) + mock_response.ok = False + + with pytest.raises(aiohttp.ClientConnectionError): + await oav.grid_snapshot.trigger() diff --git a/tests/devices/unit_tests/oav/test_snapshots.py b/tests/devices/unit_tests/oav/test_snapshots.py index eba915d2cc..d882e5c375 100644 --- a/tests/devices/unit_tests/oav/test_snapshots.py +++ b/tests/devices/unit_tests/oav/test_snapshots.py @@ -1,5 +1,5 @@ from pathlib import Path -from unittest.mock import ANY, AsyncMock, MagicMock, patch +from unittest.mock import ANY, AsyncMock, MagicMock, call, patch import pytest from ophyd_async.core import DeviceCollector, MockSignalBackend, SignalR, set_mock_value @@ -117,7 +117,7 @@ def test_snapshot_draws_expected_crosshair(tmp_path: Path): "dodal.devices.areadetector.plugins.MJPG.ClientSession.get", autospec=True, ) -async def test_snapshot_with_grid_triggered_saves_image_and_draws_grid( +async def test_snapshot_with_grid_triggered_saves_image_and_draws_correct_grid( mock_get, patch_add_grid, patch_add_border, patch_image, grid_snapshot ): mock_get.return_value.__aenter__.return_value = (mock_response := AsyncMock()) @@ -143,6 +143,11 @@ async def test_snapshot_with_grid_triggered_saves_image_and_draws_grid( mock_open.return_value.__enter__.return_value, 100, 100, 50, 15, 10 ) assert mock_save_grid.await_count == 2 + expected_grid_save_calls = [ + call(ANY, f"/tmp/test_{suffix}.png") + for suffix in ["outer_overlay", "grid_overlay"] + ] + assert mock_save_grid.mock_calls == expected_grid_save_calls assert ( await grid_snapshot.last_path_outer.get_value() == "/tmp/test_outer_overlay.png" ) diff --git a/tests/devices/unit_tests/test_oav.py b/tests/devices/unit_tests/test_oav.py index c9e88d0166..0481d4b1b1 100644 --- a/tests/devices/unit_tests/test_oav.py +++ b/tests/devices/unit_tests/test_oav.py @@ -1,9 +1,7 @@ -from unittest.mock import MagicMock, call, patch +from unittest.mock import MagicMock, patch import pytest from ophyd.sim import make_fake_device -from PIL import Image -from requests import HTTPError, Response from dodal.devices.oav.oav_detector import OAV, OAVConfigParams @@ -46,19 +44,6 @@ def mock_get_with_valid_response(): patcher.stop() -@patch("requests.get") -def test_snapshot_trigger_handles_request_with_bad_status_code_correctly( - mock_get, fake_oav: OAV -): - response = Response() - response.status_code = 404 - mock_get.return_value = response - - st = fake_oav.grid_snapshot.trigger() - with pytest.raises(HTTPError): - st.wait() - - @patch("dodal.devices.areadetector.plugins.MJPG.Image") @patch("dodal.devices.areadetector.plugins.MJPG.os", new=MagicMock()) def test_snapshot_trigger_loads_correct_url( @@ -69,24 +54,6 @@ def test_snapshot_trigger_loads_correct_url( mock_get_with_valid_response.assert_called_once_with("http://test.url", stream=True) -@patch("dodal.devices.areadetector.plugins.MJPG.Image.open") -@patch("dodal.devices.areadetector.plugins.MJPG.os", new=MagicMock()) -def test_snapshot_trigger_saves_to_correct_file( - mock_open: MagicMock, mock_get_with_valid_response, fake_oav -): - image = Image.open("test") - mock_open.return_value.__enter__.return_value = image - with patch.object(image, "save") as mock_save: - st = fake_oav.grid_snapshot.trigger() - st.wait() - expected_calls_to_save = [ - call(f"test directory/test filename{addition}.png") - for addition in ["", "_outer_overlay", "_grid_overlay"] - ] - calls_to_save = mock_save.mock_calls - assert calls_to_save == expected_calls_to_save - - @patch("dodal.devices.areadetector.plugins.MJPG.Image.open") @patch("dodal.devices.areadetector.plugins.MJPG.os") def test_given_directory_not_existing_when_snapshot_triggered_then_directory_created( @@ -96,45 +63,3 @@ def test_given_directory_not_existing_when_snapshot_triggered_then_directory_cre st = fake_oav.grid_snapshot.trigger() st.wait() mock_os.mkdir.assert_called_once_with("test directory") - - -@patch("dodal.devices.areadetector.plugins.MJPG.Image.open") -@patch("dodal.devices.areadetector.plugins.MJPG.os", new=MagicMock()) -def test_snapshot_trigger_applies_current_microns_per_pixel_to_snapshot( - mock_open: MagicMock, mock_get_with_valid_response, fake_oav -): - image = Image.open("test") # type: ignore - mock_open.return_value.__enter__.return_value = image - - expected_mpp_x = fake_oav.parameters.micronsPerXPixel - expected_mpp_y = fake_oav.parameters.micronsPerYPixel - with patch.object(image, "save"): - st = fake_oav.grid_snapshot.trigger() - st.wait() - assert fake_oav.grid_snapshot.microns_per_pixel_x.get() == expected_mpp_x - assert fake_oav.grid_snapshot.microns_per_pixel_y.get() == expected_mpp_y - - -@patch("dodal.devices.areadetector.plugins.MJPG.Image.open") -@patch("dodal.devices.oav.grid_overlay.add_grid_overlay_to_image") -@patch("dodal.devices.oav.grid_overlay.add_grid_border_overlay_to_image") -@patch("dodal.devices.areadetector.plugins.MJPG.os", new=MagicMock()) -def test_correct_grid_drawn_on_image( - mock_border_overlay: MagicMock, - mock_grid_overlay: MagicMock, - mock_open: MagicMock, - mock_get_with_valid_response: MagicMock, - fake_oav: OAV, -): - st = fake_oav.grid_snapshot.trigger() - st.wait() - expected_border_calls = [ - call(mock_open.return_value.__enter__.return_value, 100, 100, 50, 15, 10) - ] - expected_grid_calls = [ - call(mock_open.return_value.__enter__.return_value, 100, 100, 50, 15, 10) - ] - actual_border_calls = mock_border_overlay.mock_calls - actual_grid_calls = mock_grid_overlay.mock_calls - assert actual_border_calls == expected_border_calls - assert actual_grid_calls == expected_grid_calls From a27afb9b88e8c171eb9e0562499f2f8757cc13f9 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Wed, 23 Oct 2024 12:02:33 +0100 Subject: [PATCH 34/53] Remove tests for old oav --- tests/devices/unit_tests/test_oav.py | 65 ---------------------------- 1 file changed, 65 deletions(-) delete mode 100644 tests/devices/unit_tests/test_oav.py diff --git a/tests/devices/unit_tests/test_oav.py b/tests/devices/unit_tests/test_oav.py deleted file mode 100644 index 0481d4b1b1..0000000000 --- a/tests/devices/unit_tests/test_oav.py +++ /dev/null @@ -1,65 +0,0 @@ -from unittest.mock import MagicMock, patch - -import pytest -from ophyd.sim import make_fake_device - -from dodal.devices.oav.oav_detector import OAV, OAVConfigParams - -DISPLAY_CONFIGURATION = "tests/devices/unit_tests/test_display.configuration" -ZOOM_LEVELS_XML = "tests/devices/unit_tests/test_jCameraManZoomLevels.xml" - - -@pytest.fixture -def fake_oav() -> OAV: - oav_params = OAVConfigParams(ZOOM_LEVELS_XML, DISPLAY_CONFIGURATION) - FakeOAV = make_fake_device(OAV) - fake_oav: OAV = FakeOAV(name="test fake OAV", params=oav_params) - - fake_oav.grid_snapshot.url.sim_put("http://test.url") # type: ignore - fake_oav.grid_snapshot.filename.put("test filename") - fake_oav.grid_snapshot.directory.put("test directory") - fake_oav.grid_snapshot.top_left_x.put(100) - fake_oav.grid_snapshot.top_left_y.put(100) - fake_oav.grid_snapshot.box_width.put(50) - fake_oav.grid_snapshot.num_boxes_x.put(15) - fake_oav.grid_snapshot.num_boxes_y.put(10) - fake_oav.grid_snapshot.x_size.sim_put(1024) # type: ignore - fake_oav.grid_snapshot.y_size.sim_put(768) # type: ignore - - fake_oav.cam.port_name.sim_put("CAM") # type: ignore - fake_oav.proc.port_name.sim_put("PROC") # type: ignore - - fake_oav.wait_for_connection() - fake_oav.zoom_controller.set("1.0x").wait() - - return fake_oav - - -@pytest.fixture -def mock_get_with_valid_response(): - patcher = patch("requests.get") - mock_get = patcher.start() - mock_get.return_value.content = b"" - yield mock_get - patcher.stop() - - -@patch("dodal.devices.areadetector.plugins.MJPG.Image") -@patch("dodal.devices.areadetector.plugins.MJPG.os", new=MagicMock()) -def test_snapshot_trigger_loads_correct_url( - mock_image: MagicMock, mock_get_with_valid_response: MagicMock, fake_oav: OAV -): - st = fake_oav.grid_snapshot.trigger() - st.wait() - mock_get_with_valid_response.assert_called_once_with("http://test.url", stream=True) - - -@patch("dodal.devices.areadetector.plugins.MJPG.Image.open") -@patch("dodal.devices.areadetector.plugins.MJPG.os") -def test_given_directory_not_existing_when_snapshot_triggered_then_directory_created( - mock_os, mock_open: MagicMock, mock_get_with_valid_response, fake_oav -): - mock_os.path.isdir.return_value = False - st = fake_oav.grid_snapshot.trigger() - st.wait() - mock_os.mkdir.assert_called_once_with("test directory") From dab56979b57aba7dc689ed63ac877d1ad332a6fc Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Wed, 23 Oct 2024 12:05:31 +0100 Subject: [PATCH 35/53] Remove old OAVConfigParams --- src/dodal/devices/oav/oav_parameters.py | 112 ------------------------ 1 file changed, 112 deletions(-) diff --git a/src/dodal/devices/oav/oav_parameters.py b/src/dodal/devices/oav/oav_parameters.py index ac4c246473..c8d1eb047c 100644 --- a/src/dodal/devices/oav/oav_parameters.py +++ b/src/dodal/devices/oav/oav_parameters.py @@ -5,12 +5,6 @@ from typing import Any from xml.etree.ElementTree import Element -from dodal.devices.oav.oav_errors import ( - OAVError_BeamPositionNotFound, - OAVError_ZoomLevelNotFound, -) -from dodal.log import LOGGER - # GDA currently assumes this aspect ratio for the OAV window size. # For some beamline this doesn't affect anything as the actual OAV aspect ratio # matches. Others need to take it into account to rescale the values stored in @@ -110,118 +104,12 @@ def get_max_tip_distance_in_pixels(self, micronsPerPixel: float) -> float: return self.max_tip_distance / micronsPerPixel -class OAVConfigParams: - """ - The OAV parameters which may update depending on settings such as the zoom level. - """ - - def __init__( - self, - zoom_params_file, - display_config, - ): - self.zoom_params_file: str = zoom_params_file - self.display_config: str = display_config - - def update_on_zoom(self, value, xsize, ysize, *args, **kwargs): - xsize, ysize = int(xsize), int(ysize) - if isinstance(value, str) and value.endswith("x"): - value = value.strip("x") - zoom = float(value) - self.load_microns_per_pixel(zoom, xsize, ysize) - self.beam_centre_i, self.beam_centre_j = self.get_beam_position_from_zoom( - zoom, xsize, ysize - ) - - def load_microns_per_pixel(self, zoom: float, xsize: int, ysize: int) -> None: - """ - Loads the microns per x pixel and y pixel for a given zoom level. These are - currently generated by GDA, though hyperion could generate them in future. - """ - tree = et.parse(self.zoom_params_file) - self.micronsPerXPixel = self.micronsPerYPixel = None - root = tree.getroot() - levels = root.findall(".//zoomLevel") - for node in levels: - if _get_element_as_float(node, "level") == zoom: - self.micronsPerXPixel = ( - _get_element_as_float(node, "micronsPerXPixel") - * DEFAULT_OAV_WINDOW[0] - / xsize - ) - self.micronsPerYPixel = ( - _get_element_as_float(node, "micronsPerYPixel") - * DEFAULT_OAV_WINDOW[1] - / ysize - ) - if self.micronsPerXPixel is None or self.micronsPerYPixel is None: - raise OAVError_ZoomLevelNotFound( - f""" - Could not find the micronsPer[X,Y]Pixel parameters in - {self.zoom_params_file} for zoom level {zoom}. - """ - ) - - def get_beam_position_from_zoom( - self, zoom: float, xsize: int, ysize: int - ) -> tuple[int, int]: - """ - Extracts the beam location in pixels `xCentre` `yCentre`, for a requested zoom \ - level. The beam location is manually inputted by the beamline operator on GDA \ - by clicking where on screen a scintillator lights up, and stored in the \ - display.configuration file. - """ - crosshair_x_line = None - crosshair_y_line = None - with open(self.display_config) as f: - file_lines = f.readlines() - for i in range(len(file_lines)): - if file_lines[i].startswith("zoomLevel = " + str(zoom)): - crosshair_x_line = file_lines[i + 1] - crosshair_y_line = file_lines[i + 2] - break - - if crosshair_x_line is None or crosshair_y_line is None: - raise OAVError_BeamPositionNotFound( - f"Could not extract beam position at zoom level {zoom}" - ) - - beam_centre_i = ( - int(crosshair_x_line.split(" = ")[1]) * xsize / DEFAULT_OAV_WINDOW[0] - ) - beam_centre_j = ( - int(crosshair_y_line.split(" = ")[1]) * ysize / DEFAULT_OAV_WINDOW[1] - ) - LOGGER.info(f"Beam centre: {beam_centre_i, beam_centre_j}") - return int(beam_centre_i), int(beam_centre_j) - - def calculate_beam_distance( - self, horizontal_pixels: int, vertical_pixels: int - ) -> tuple[int, int]: - """ - Calculates the distance between the beam centre and the given (horizontal, vertical). - - Args: - horizontal_pixels (int): The x (camera coordinates) value in pixels. - vertical_pixels (int): The y (camera coordinates) value in pixels. - Returns: - The distance between the beam centre and the (horizontal, vertical) point in pixels as a tuple - (horizontal_distance, vertical_distance). - """ - - return ( - self.beam_centre_i - horizontal_pixels, - self.beam_centre_j - vertical_pixels, - ) - - @dataclass class ZoomParams: microns_per_pixel: tuple[float, float] crosshair: tuple[int, int] -# Once all moved to async this should replace OAVConfigParams class OAVConfig: """ Read the OAV config files and return a dictionary of {'zoom_level': ZoomParams}\ with information about microns per pixels and crosshairs. From dc3650bd7c570668e3dffa44669dfedb295610fe Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Wed, 23 Oct 2024 12:53:31 +0100 Subject: [PATCH 36/53] Fix import --- tests/devices/unit_tests/oav/test_grid_overlay.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/devices/unit_tests/oav/test_grid_overlay.py b/tests/devices/unit_tests/oav/test_grid_overlay.py index fdde1bc423..b675afedea 100644 --- a/tests/devices/unit_tests/oav/test_grid_overlay.py +++ b/tests/devices/unit_tests/oav/test_grid_overlay.py @@ -2,7 +2,7 @@ import pytest -from dodal.devices.oav.grid_overlay import ( +from dodal.devices.oav.snapshots.grid_overlay import ( add_grid_border_overlay_to_image, add_grid_overlay_to_image, ) From fe599eddeb3c94b9c28b52cc661a7b303759df0f Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Wed, 23 Oct 2024 13:09:55 +0100 Subject: [PATCH 37/53] Hopefully fix system test --- system_tests/test_oav_system.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/system_tests/test_oav_system.py b/system_tests/test_oav_system.py index e722823129..38c4afb8d3 100644 --- a/system_tests/test_oav_system.py +++ b/system_tests/test_oav_system.py @@ -1,6 +1,7 @@ import bluesky.plan_stubs as bps import pytest from bluesky.run_engine import RunEngine +from ophyd_async.core import DeviceCollector from dodal.devices.oav.oav_detector import OAV, OAVConfig, ZoomController @@ -15,8 +16,16 @@ ZOOM_LEVELS_XML = "tests/devices/unit_tests/test_jCameraManZoomLevels.xml" -async def take_snapshot_with_grid(oav: OAV, snapshot_filename, snapshot_directory): +@pytest.fixture +async def oav() -> OAV: + oav_config = OAVConfig(ZOOM_LEVELS_XML, DISPLAY_CONFIGURATION) + async with DeviceCollector(): + oav = OAV("", config=oav_config, name="oav") await oav.connect() + return oav + + +def take_snapshot_with_grid(oav: OAV, snapshot_filename, snapshot_directory): yield from bps.abs_set(oav.grid_snapshot.top_left_x, TEST_GRID_TOP_LEFT_X) # type: ignore # See: https://github.com/DiamondLightSource/dodal/issues/827 yield from bps.abs_set(oav.grid_snapshot.top_left_y, TEST_GRID_TOP_LEFT_Y) # type: ignore # See: https://github.com/DiamondLightSource/dodal/issues/827 yield from bps.abs_set(oav.grid_snapshot.box_width, TEST_GRID_BOX_WIDTH) # type: ignore # See: https://github.com/DiamondLightSource/dodal/issues/827 @@ -29,11 +38,10 @@ async def take_snapshot_with_grid(oav: OAV, snapshot_filename, snapshot_director # We need to find a better way of integrating this, see https://github.com/DiamondLightSource/mx-bluesky/issues/183 @pytest.mark.skip(reason="Don't want to actually take snapshots during testing.") -async def test_grid_overlay(RE: RunEngine): +def test_grid_overlay(RE: RunEngine): beamline = "BL03I" oav_params = OAVConfig(ZOOM_LEVELS_XML, DISPLAY_CONFIGURATION) oav = OAV(name="oav", prefix=f"{beamline}", config=oav_params) - await oav.connect() snapshot_filename = "snapshot" snapshot_directory = "." RE(take_snapshot_with_grid(oav, snapshot_filename, snapshot_directory)) From caef8184d51173efb068cd16ca504715e52dbb06 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Wed, 23 Oct 2024 13:15:41 +0100 Subject: [PATCH 38/53] Fix more imports --- tests/devices/unit_tests/oav/test_grid_overlay.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/devices/unit_tests/oav/test_grid_overlay.py b/tests/devices/unit_tests/oav/test_grid_overlay.py index b675afedea..1d194b09b7 100644 --- a/tests/devices/unit_tests/oav/test_grid_overlay.py +++ b/tests/devices/unit_tests/oav/test_grid_overlay.py @@ -47,7 +47,7 @@ def _test_expected_calls_to_image_draw_line(mock_image_draw: MagicMock, expected ), ], ) -@patch("dodal.devices.oav.grid_overlay.ImageDraw.Draw") +@patch("dodal.devices.oav.snapshots.grid_overlay.ImageDraw.Draw") def test_add_grid_border_overlay_to_image_makes_correct_calls_to_imagedraw( mock_imagedraw: MagicMock, top_left_x, @@ -97,7 +97,7 @@ def test_add_grid_border_overlay_to_image_makes_correct_calls_to_imagedraw( ), ], ) -@patch("dodal.devices.oav.grid_overlay.ImageDraw.Draw") +@patch("dodal.devices.oav.snapshots.grid_overlay.ImageDraw.Draw") def test_add_grid_overlay_to_image_makes_correct_calls_to_imagedraw( mock_imagedraw: MagicMock, top_left_x, From acc1bd08d70b6f3072fd50fe8a8a18ad002417fb Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Wed, 23 Oct 2024 14:00:40 +0100 Subject: [PATCH 39/53] Add small missing test --- .../devices/unit_tests/oav/test_snapshots.py | 36 ++++++++++++++----- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/tests/devices/unit_tests/oav/test_snapshots.py b/tests/devices/unit_tests/oav/test_snapshots.py index d882e5c375..3fb5d0e19c 100644 --- a/tests/devices/unit_tests/oav/test_snapshots.py +++ b/tests/devices/unit_tests/oav/test_snapshots.py @@ -54,7 +54,7 @@ async def test_snapshot_with_beam_centre_triggered_then_crosshair_drawn_and( mock_get, patch_image_draw, patch_image, snapshot ): mock_get.return_value.__aenter__.return_value = (mock_response := AsyncMock()) - mock_response.ok = MagicMock(return_value=True) + mock_response.ok = True mock_response.read.return_value = (test_data := b"TEST") patch_line = MagicMock() @@ -82,7 +82,7 @@ async def test_snapshot_with_beam_centre_correctly_triggered_and_saved( mock_aiofiles, mock_get, patch_image, mock_mkdir, snapshot ): mock_get.return_value.__aenter__.return_value = (mock_response := AsyncMock()) - mock_response.ok = MagicMock(return_value=True) + mock_response.ok = True mock_response.read.return_value = (test_data := b"TEST") mock_aio_open = mock_aiofiles.open @@ -91,10 +91,19 @@ async def test_snapshot_with_beam_centre_correctly_triggered_and_saved( mock_open = patch_image.open mock_open.return_value.__aenter__.return_value = test_data + # Set new directory and test that it's created + set_mock_value(snapshot.directory, "new_dir") + await snapshot.trigger() - assert await snapshot.last_saved_path.get_value() == "/tmp/test.png" - mock_aio_open.assert_called_once_with("/tmp/test.png", "wb") + mock_mkdir.assert_called_once() + + test_url = await snapshot.url.get_value() + # Get called with an instance of the session and correct url + mock_get.assert_called_once_with(ANY, test_url) + + assert await snapshot.last_saved_path.get_value() == "new_dir/test.png" + mock_aio_open.assert_called_once_with("new_dir/test.png", "wb") mock_file.write.assert_called_once() @@ -121,7 +130,7 @@ async def test_snapshot_with_grid_triggered_saves_image_and_draws_correct_grid( mock_get, patch_add_grid, patch_add_border, patch_image, grid_snapshot ): mock_get.return_value.__aenter__.return_value = (mock_response := AsyncMock()) - mock_response.ok = MagicMock(return_value=True) + mock_response.ok = True mock_response.read.return_value = (test_data := b"TEST") mock_open = patch_image.open @@ -132,9 +141,6 @@ async def test_snapshot_with_grid_triggered_saves_image_and_draws_correct_grid( await grid_snapshot.trigger() - test_url = await grid_snapshot.url.get_value() - # Get called with an instance of the session and correct url - mock_get.assert_called_once_with(ANY, test_url) mock_save.assert_awaited_once() patch_add_border.assert_called_once_with( mock_open.return_value.__enter__.return_value, 100, 100, 50, 15, 10 @@ -155,3 +161,17 @@ async def test_snapshot_with_grid_triggered_saves_image_and_draws_correct_grid( await grid_snapshot.last_path_full_overlay.get_value() == "/tmp/test_grid_overlay.png" ) + + +@patch("dodal.devices.oav.snapshots.snapshot_with_grid.Image") +@patch("dodal.devices.oav.snapshots.snapshot_with_grid.aiofiles", autospec=True) +async def test_save_grid_to_image(mock_aiofiles, patch_image, grid_snapshot): + mock_aio_open = mock_aiofiles.open + mock_aio_open.return_value.__aenter__.return_value = (mock_file := AsyncMock()) + + test_path = MagicMock(return_value="some_path/test_grid.png") + await grid_snapshot._save_grid_to_image(patch_image, test_path) + + patch_image.save.assert_called_once() + mock_aio_open.assert_called_once_with(test_path, "wb") + mock_file.write.assert_called_once() From c7b78cd62baaf4617fc7be34e42a1fe07e191cd1 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Wed, 23 Oct 2024 14:04:39 +0100 Subject: [PATCH 40/53] Fix snapshot prefixes --- src/dodal/devices/oav/oav_detector.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dodal/devices/oav/oav_detector.py b/src/dodal/devices/oav/oav_detector.py index 3b10d1c345..621381c309 100644 --- a/src/dodal/devices/oav/oav_detector.py +++ b/src/dodal/devices/oav/oav_detector.py @@ -69,9 +69,9 @@ def __init__(self, prefix: str, config: OAVConfig, name: str = ""): ) self.snapshot = SnapshotWithBeamCentre( - f"{prefix}-DI-OAV-01:MJPG:", self.beam_centre_i, self.beam_centre_j, name + f"{prefix}:MJPG:", self.beam_centre_i, self.beam_centre_j, name ) - self.grid_snapshot = SnapshotWithGrid(f"{prefix}-DI-OAV-01:MJPG:", name) + self.grid_snapshot = SnapshotWithGrid(f"{prefix}:MJPG:", name) self._snapshot_trigger_subscription_id = None From 1b03b35520c80e7d3e8be83f414a3ecc51c776a1 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Wed, 23 Oct 2024 14:05:37 +0100 Subject: [PATCH 41/53] Fix snapshot prefixes - part 2 --- src/dodal/devices/oav/oav_detector.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dodal/devices/oav/oav_detector.py b/src/dodal/devices/oav/oav_detector.py index 621381c309..b252b577c1 100644 --- a/src/dodal/devices/oav/oav_detector.py +++ b/src/dodal/devices/oav/oav_detector.py @@ -69,9 +69,9 @@ def __init__(self, prefix: str, config: OAVConfig, name: str = ""): ) self.snapshot = SnapshotWithBeamCentre( - f"{prefix}:MJPG:", self.beam_centre_i, self.beam_centre_j, name + f"{prefix}MJPG:", self.beam_centre_i, self.beam_centre_j, name ) - self.grid_snapshot = SnapshotWithGrid(f"{prefix}:MJPG:", name) + self.grid_snapshot = SnapshotWithGrid(f"{prefix}MJPG:", name) self._snapshot_trigger_subscription_id = None From dcaa8fd8034525874f31a19a4e2a2848fd3b0545 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Wed, 23 Oct 2024 14:40:47 +0100 Subject: [PATCH 42/53] Add small test --- tests/devices/unit_tests/oav/test_oav.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/devices/unit_tests/oav/test_oav.py b/tests/devices/unit_tests/oav/test_oav.py index 1ff32e46c1..3f553da399 100644 --- a/tests/devices/unit_tests/oav/test_oav.py +++ b/tests/devices/unit_tests/oav/test_oav.py @@ -83,3 +83,22 @@ async def test_snapshot_trigger_handles_bad_request_status(mock_get, oav: OAV): with pytest.raises(aiohttp.ClientConnectionError): await oav.grid_snapshot.trigger() + + +@patch( + "dodal.devices.areadetector.plugins.MJPG.ClientSession.get", + autospec=True, +) +@patch("dodal.devices.areadetector.plugins.MJPG.Image") +async def test_snapshot_trigger_fails_in_post_processing_withouth_raising_error( + patch_image, mock_get, oav: OAV +): + mock_get.return_value.__aenter__.return_value = (mock_response := AsyncMock()) + mock_response.ok = True + mock_response.read.return_value = "invalid read" + + mock_open = patch_image.open + + await oav.snapshot.trigger() + + mock_open.assert_not_called() From b4ec7a23d457288f701b7270e117f59a89b661c8 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Wed, 23 Oct 2024 15:42:30 +0100 Subject: [PATCH 43/53] Try to make codecov happy --- tests/devices/unit_tests/oav/test_oav.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/devices/unit_tests/oav/test_oav.py b/tests/devices/unit_tests/oav/test_oav.py index 3f553da399..9aaffc6ad8 100644 --- a/tests/devices/unit_tests/oav/test_oav.py +++ b/tests/devices/unit_tests/oav/test_oav.py @@ -73,6 +73,28 @@ async def test_oav_returns_rescaled_beam_position_and_microns_per_pixel_correctl assert beam_y == 450 +@patch( + "dodal.devices.areadetector.plugins.MJPG.ClientSession.get", + autospec=True, +) +@patch("dodal.devices.areadetector.plugins.MJPG.Image") +async def test_when_snapshot_triggered_post_processing_called_correctly( + patch_image, mock_get, oav: OAV +): + mock_get.return_value.__aenter__.return_value = (mock_response := AsyncMock()) + mock_response.ok = True + mock_response.read.return_value = (test_data := b"TEST") + + mock_open = patch_image.open + mock_open.return_value.__aenter__.return_value = test_data + + oav.snapshot.post_processing = (mock_proc := AsyncMock()) + + await oav.snapshot.trigger() + + mock_proc.assert_awaited_once_with(test_data) + + @patch( "dodal.devices.areadetector.plugins.MJPG.ClientSession.get", autospec=True, From fe259d7a77016245cbb4d0117d379213b910dc5e Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Wed, 23 Oct 2024 16:03:42 +0100 Subject: [PATCH 44/53] Fix test --- tests/devices/unit_tests/oav/test_oav.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/devices/unit_tests/oav/test_oav.py b/tests/devices/unit_tests/oav/test_oav.py index 9aaffc6ad8..6d8df19f15 100644 --- a/tests/devices/unit_tests/oav/test_oav.py +++ b/tests/devices/unit_tests/oav/test_oav.py @@ -92,7 +92,7 @@ async def test_when_snapshot_triggered_post_processing_called_correctly( await oav.snapshot.trigger() - mock_proc.assert_awaited_once_with(test_data) + mock_proc.assert_awaited_once() @patch( From eb32c5a47287f59c8f557ac86fbcc2ca78c50dd4 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Wed, 23 Oct 2024 16:41:52 +0100 Subject: [PATCH 45/53] Add a small cam plugin for oav since we use it --- src/dodal/devices/areadetector/plugins/CAM.py | 28 +++++++++++++++++++ src/dodal/devices/oav/oav_detector.py | 3 ++ src/dodal/devices/oav/utils.py | 15 ---------- tests/devices/unit_tests/oav/test_oav.py | 14 +++++++++- 4 files changed, 44 insertions(+), 16 deletions(-) create mode 100644 src/dodal/devices/areadetector/plugins/CAM.py diff --git a/src/dodal/devices/areadetector/plugins/CAM.py b/src/dodal/devices/areadetector/plugins/CAM.py new file mode 100644 index 0000000000..c1b894ab09 --- /dev/null +++ b/src/dodal/devices/areadetector/plugins/CAM.py @@ -0,0 +1,28 @@ +from enum import IntEnum + +from ophyd_async.core import StandardReadable +from ophyd_async.epics.signal import epics_signal_rw + + +class ColorMode(IntEnum): + """ + Enum to store the various color modes of the camera. We use RGB1. + """ + + MONO = 0 + BAYER = 1 + RGB1 = 2 + RGB2 = 3 + RGB3 = 4 + YUV444 = 5 + YUV422 = 6 + YUV421 = 7 + + +class Cam(StandardReadable): + def __init__(self, prefix: str, name: str = "") -> None: + self.color_mode = epics_signal_rw(ColorMode, "GC_BalRatioSelector") + self.acquire_period = epics_signal_rw(float, "AcquirePeriod") + self.acquire_time = epics_signal_rw(float, "AcquireTime") + self.gain = epics_signal_rw(float, "Gain") + super().__init__(name) diff --git a/src/dodal/devices/oav/oav_detector.py b/src/dodal/devices/oav/oav_detector.py index b252b577c1..9f06632a29 100644 --- a/src/dodal/devices/oav/oav_detector.py +++ b/src/dodal/devices/oav/oav_detector.py @@ -4,6 +4,7 @@ from ophyd_async.epics.signal import epics_signal_rw from dodal.common.signal_utils import create_hardware_backed_soft_signal +from dodal.devices.areadetector.plugins.CAM import Cam from dodal.devices.oav.oav_parameters import DEFAULT_OAV_WINDOW, OAVConfig from dodal.devices.oav.snapshots.snapshot_with_beam_centre import SnapshotWithBeamCentre from dodal.devices.oav.snapshots.snapshot_with_grid import SnapshotWithGrid @@ -49,6 +50,8 @@ def __init__(self, prefix: str, config: OAVConfig, name: str = ""): _bl_prefix = prefix.split("-")[0] self.zoom_controller = ZoomController(f"{_bl_prefix}-EA-OAV-01:FZOOM:", name) + self.cam = Cam(f"{prefix}CAM:", name=name) + self.parameters = config.get_parameters() self.microns_per_pixel_x = create_hardware_backed_soft_signal( diff --git a/src/dodal/devices/oav/utils.py b/src/dodal/devices/oav/utils.py index f1ac168739..9f918feaa7 100644 --- a/src/dodal/devices/oav/utils.py +++ b/src/dodal/devices/oav/utils.py @@ -39,21 +39,6 @@ def bottom_right_from_top_left( ) -class ColorMode(IntEnum): - """ - Enum to store the various color modes of the camera. We use RGB1. - """ - - MONO = 0 - BAYER = 1 - RGB1 = 2 - RGB2 = 3 - RGB3 = 4 - YUV444 = 5 - YUV422 = 6 - YUV421 = 7 - - class EdgeOutputArrayImageType(IntEnum): """ Enum to store the types of image to tweak the output array. We use Original. diff --git a/tests/devices/unit_tests/oav/test_oav.py b/tests/devices/unit_tests/oav/test_oav.py index 6d8df19f15..cf855e7e77 100644 --- a/tests/devices/unit_tests/oav/test_oav.py +++ b/tests/devices/unit_tests/oav/test_oav.py @@ -4,7 +4,7 @@ import pytest from ophyd_async.core import set_mock_value -from dodal.devices.oav.oav_detector import OAV, ZoomController +from dodal.devices.oav.oav_detector import OAV, Cam, ZoomController async def test_zoom_controller(): @@ -16,6 +16,18 @@ async def test_zoom_controller(): assert await zoom_controller.level.get_value() == "3.0x" +async def test_cam(): + cam = Cam("", "fake cam") + await cam.connect(mock=True) + status = cam.acquire_period.set(0.01) + await status + assert status.success + assert await cam.acquire_period.get_value() == 0.01 + + cam.acquire_time.set(0.01) + assert await cam.acquire_time.get_value() == 0.01 + + @pytest.mark.parametrize( "zoom_level,expected_microns_x,expected_microns_y", [ From 5a032133ddae754451074b01bad94ee208873d84 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Wed, 23 Oct 2024 16:46:11 +0100 Subject: [PATCH 46/53] Try to fix test --- tests/devices/unit_tests/oav/test_oav.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/devices/unit_tests/oav/test_oav.py b/tests/devices/unit_tests/oav/test_oav.py index cf855e7e77..ac57b36021 100644 --- a/tests/devices/unit_tests/oav/test_oav.py +++ b/tests/devices/unit_tests/oav/test_oav.py @@ -24,7 +24,9 @@ async def test_cam(): assert status.success assert await cam.acquire_period.get_value() == 0.01 - cam.acquire_time.set(0.01) + status = cam.acquire_time.set(0.01) + await status + assert status.success assert await cam.acquire_time.get_value() == 0.01 From 6541cddb4793a9d8cbbf435af3382b29c3fa83fb Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Thu, 24 Oct 2024 08:45:23 +0100 Subject: [PATCH 47/53] Add test for parameters --- tests/devices/unit_tests/oav/test_oav_parameters.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/devices/unit_tests/oav/test_oav_parameters.py b/tests/devices/unit_tests/oav/test_oav_parameters.py index aba5771d4d..6f04fc1f29 100644 --- a/tests/devices/unit_tests/oav/test_oav_parameters.py +++ b/tests/devices/unit_tests/oav/test_oav_parameters.py @@ -69,3 +69,10 @@ def test_given_oav_config_get_max_tip_distance_in_pixels( assert mock_parameters.get_max_tip_distance_in_pixels( microns_per_pixel_x ) == pytest.approx(189.873, abs=1e-3) + + +def test_given_new_context_parameters_are_updated(mock_parameters: OAVParameters): + mock_parameters.update_context("xrayCentring") + + assert mock_parameters.active_params.get("zoom") == 7.5 + assert mock_parameters.active_params.get("brightness") == 80 From 9f13205439eb9fa22483611579236df690214bed Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Thu, 24 Oct 2024 09:18:28 +0100 Subject: [PATCH 48/53] Add array sizes --- src/dodal/devices/areadetector/plugins/CAM.py | 5 ++++- tests/devices/unit_tests/oav/test_oav.py | 5 +++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/dodal/devices/areadetector/plugins/CAM.py b/src/dodal/devices/areadetector/plugins/CAM.py index c1b894ab09..52c5414bc5 100644 --- a/src/dodal/devices/areadetector/plugins/CAM.py +++ b/src/dodal/devices/areadetector/plugins/CAM.py @@ -1,7 +1,7 @@ from enum import IntEnum from ophyd_async.core import StandardReadable -from ophyd_async.epics.signal import epics_signal_rw +from ophyd_async.epics.signal import epics_signal_r, epics_signal_rw class ColorMode(IntEnum): @@ -25,4 +25,7 @@ def __init__(self, prefix: str, name: str = "") -> None: self.acquire_period = epics_signal_rw(float, "AcquirePeriod") self.acquire_time = epics_signal_rw(float, "AcquireTime") self.gain = epics_signal_rw(float, "Gain") + + self.array_size_x = epics_signal_r(int, "ArraySizeX_RBV") + self.array_size_y = epics_signal_r(int, "ArraySizeY_RBV") super().__init__(name) diff --git a/tests/devices/unit_tests/oav/test_oav.py b/tests/devices/unit_tests/oav/test_oav.py index ac57b36021..c70b8b49b5 100644 --- a/tests/devices/unit_tests/oav/test_oav.py +++ b/tests/devices/unit_tests/oav/test_oav.py @@ -19,6 +19,9 @@ async def test_zoom_controller(): async def test_cam(): cam = Cam("", "fake cam") await cam.connect(mock=True) + set_mock_value(cam.array_size_x, 1024) + set_mock_value(cam.array_size_y, 768) + status = cam.acquire_period.set(0.01) await status assert status.success @@ -29,6 +32,8 @@ async def test_cam(): assert status.success assert await cam.acquire_time.get_value() == 0.01 + assert await cam.array_size_x.get_value() == 1024 + @pytest.mark.parametrize( "zoom_level,expected_microns_x,expected_microns_y", From b2d0dbe4c63fb0ca1313be5155c6a57832a446ad Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Thu, 24 Oct 2024 14:29:25 +0100 Subject: [PATCH 49/53] Add check for zoom level and test --- src/dodal/devices/oav/oav_detector.py | 11 +++++++++++ .../devices/oav/snapshots/snapshot_with_grid.py | 2 +- tests/devices/unit_tests/oav/conftest.py | 6 ++++++ tests/devices/unit_tests/oav/test_oav.py | 16 +++++++++++++++- 4 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/dodal/devices/oav/oav_detector.py b/src/dodal/devices/oav/oav_detector.py index 9f06632a29..b98d26a49b 100644 --- a/src/dodal/devices/oav/oav_detector.py +++ b/src/dodal/devices/oav/oav_detector.py @@ -5,6 +5,7 @@ from dodal.common.signal_utils import create_hardware_backed_soft_signal from dodal.devices.areadetector.plugins.CAM import Cam +from dodal.devices.oav.oav_errors import OAVError_ZoomLevelNotFound from dodal.devices.oav.oav_parameters import DEFAULT_OAV_WINDOW, OAVConfig from dodal.devices.oav.snapshots.snapshot_with_beam_centre import SnapshotWithBeamCentre from dodal.devices.oav.snapshots.snapshot_with_grid import SnapshotWithGrid @@ -40,8 +41,18 @@ def __init__(self, prefix: str, name: str = "") -> None: # Level is the string description of the zoom level e.g. "1.0x" or "1.0" self.level = epics_signal_rw(str, f"{prefix}MP:SELECT") + async def _get_allowed_zoom_levels(self) -> list: + zoom_levels = await self.level.describe() + print(zoom_levels) + return zoom_levels["level"]["choices"] # type: ignore + @AsyncStatus.wrap async def set(self, level_to_set: str): + allowed_zoom_levels = await self._get_allowed_zoom_levels() + if level_to_set not in allowed_zoom_levels: + raise OAVError_ZoomLevelNotFound( + f"{level_to_set} not found, expected one of {allowed_zoom_levels}" + ) await self.level.set(level_to_set, wait=True) diff --git a/src/dodal/devices/oav/snapshots/snapshot_with_grid.py b/src/dodal/devices/oav/snapshots/snapshot_with_grid.py index 8259d1de73..b981f8831b 100644 --- a/src/dodal/devices/oav/snapshots/snapshot_with_grid.py +++ b/src/dodal/devices/oav/snapshots/snapshot_with_grid.py @@ -19,7 +19,7 @@ def __init__(self, prefix: str, name: str = "") -> None: self.top_left_x = soft_signal_rw(int) self.top_left_y = soft_signal_rw(int) - self.box_width = soft_signal_rw(int) + self.box_width = soft_signal_rw(float) self.num_boxes_x = soft_signal_rw(int) self.num_boxes_y = soft_signal_rw(int) diff --git a/tests/devices/unit_tests/oav/conftest.py b/tests/devices/unit_tests/oav/conftest.py index af6aafebed..c23a1dab8d 100644 --- a/tests/devices/unit_tests/oav/conftest.py +++ b/tests/devices/unit_tests/oav/conftest.py @@ -1,3 +1,5 @@ +from unittest.mock import AsyncMock + import pytest from ophyd_async.core import DeviceCollector, set_mock_value @@ -13,6 +15,10 @@ async def oav() -> OAV: oav_config = OAVConfig(ZOOM_LEVELS_XML, DISPLAY_CONFIGURATION) async with DeviceCollector(mock=True, connect=True): oav = OAV("", config=oav_config, name="fake_oav") + zoom_levels_list = ["1.0x", "3.0x", "5.0x", "7.5x", "10.0x"] + oav.zoom_controller._get_allowed_zoom_levels = AsyncMock( + return_value=zoom_levels_list + ) set_mock_value(oav.grid_snapshot.x_size, 1024) set_mock_value(oav.grid_snapshot.y_size, 768) set_mock_value(oav.zoom_controller.level, "1.0x") diff --git a/tests/devices/unit_tests/oav/test_oav.py b/tests/devices/unit_tests/oav/test_oav.py index c70b8b49b5..3c5ea404c6 100644 --- a/tests/devices/unit_tests/oav/test_oav.py +++ b/tests/devices/unit_tests/oav/test_oav.py @@ -4,18 +4,32 @@ import pytest from ophyd_async.core import set_mock_value -from dodal.devices.oav.oav_detector import OAV, Cam, ZoomController +from dodal.devices.oav.oav_detector import ( + OAV, + Cam, + ZoomController, +) +from dodal.devices.oav.oav_errors import OAVError_ZoomLevelNotFound async def test_zoom_controller(): zoom_controller = ZoomController("", "fake zoom controller") await zoom_controller.connect(mock=True) + zoom_controller._get_allowed_zoom_levels = AsyncMock(return_value=["1.0x", "3.0x"]) status = zoom_controller.set("3.0x") await status assert status.success assert await zoom_controller.level.get_value() == "3.0x" +async def test_zoom_controller_set_raises_error_for_wrong_level(): + zoom_controller = ZoomController("", "fake zoom controller") + await zoom_controller.connect(mock=True) + zoom_controller._get_allowed_zoom_levels = AsyncMock(return_value=["1.0x", "3.0x"]) + with pytest.raises(OAVError_ZoomLevelNotFound): + await zoom_controller.set("5.0x") + + async def test_cam(): cam = Cam("", "fake cam") await cam.connect(mock=True) From 9c8a3cc3f07ee56e05f65b16537b3b2bceb15368 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Thu, 24 Oct 2024 14:35:25 +0100 Subject: [PATCH 50/53] Fix linting --- src/dodal/devices/oav/snapshots/grid_overlay.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/dodal/devices/oav/snapshots/grid_overlay.py b/src/dodal/devices/oav/snapshots/grid_overlay.py index 56eb048727..707d47f4b3 100644 --- a/src/dodal/devices/oav/snapshots/grid_overlay.py +++ b/src/dodal/devices/oav/snapshots/grid_overlay.py @@ -13,8 +13,8 @@ def _add_parallel_lines_to_image( image: Image.Image, start_x: int, start_y: int, - line_length: int, - spacing: int, + line_length: float, + spacing: float, num_lines: int, orientation=Orientation.horizontal, ): @@ -31,8 +31,8 @@ def _add_parallel_lines_to_image( image (PIL.Image): The image to be drawn upon. start_x (int): The x coordinate (in pixels) of the start of the initial line. start_y (int): The y coordinate (in pixels) of the start of the initial line. - line_length (int): The length of each of the parallel lines in pixels. - spacing (int): The spacing, in pixels, between each parallel line. Strictly, \ + line_length (float): The length of each of the parallel lines in pixels. + spacing (float): The spacing, in pixels, between each parallel line. Strictly, \ there are spacing-1 pixels between each line num_lines (int): The total number of parallel lines to draw. orientation (Orientation): The orientation (horizontal or vertical) of the \ @@ -70,7 +70,7 @@ def add_grid_border_overlay_to_image( image: Image.Image, top_left_x: int, top_left_y: int, - box_width: int, + box_width: float, num_boxes_x: int, num_boxes_y: int, ): @@ -96,7 +96,7 @@ def add_grid_overlay_to_image( image: Image.Image, top_left_x: int, top_left_y: int, - box_width: int, + box_width: float, num_boxes_x: int, num_boxes_y: int, ): From c0914a6f9b0d5c9efa50cda036d61e68f14e7a8f Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Thu, 24 Oct 2024 14:39:21 +0100 Subject: [PATCH 51/53] Really fix linting --- src/dodal/devices/oav/snapshots/grid_overlay.py | 8 ++++---- src/dodal/devices/oav/snapshots/snapshot_with_grid.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/dodal/devices/oav/snapshots/grid_overlay.py b/src/dodal/devices/oav/snapshots/grid_overlay.py index 707d47f4b3..b0b7fafbef 100644 --- a/src/dodal/devices/oav/snapshots/grid_overlay.py +++ b/src/dodal/devices/oav/snapshots/grid_overlay.py @@ -13,8 +13,8 @@ def _add_parallel_lines_to_image( image: Image.Image, start_x: int, start_y: int, - line_length: float, - spacing: float, + line_length: int, + spacing: int, num_lines: int, orientation=Orientation.horizontal, ): @@ -70,7 +70,7 @@ def add_grid_border_overlay_to_image( image: Image.Image, top_left_x: int, top_left_y: int, - box_width: float, + box_width: int, num_boxes_x: int, num_boxes_y: int, ): @@ -96,7 +96,7 @@ def add_grid_overlay_to_image( image: Image.Image, top_left_x: int, top_left_y: int, - box_width: float, + box_width: int, num_boxes_x: int, num_boxes_y: int, ): diff --git a/src/dodal/devices/oav/snapshots/snapshot_with_grid.py b/src/dodal/devices/oav/snapshots/snapshot_with_grid.py index b981f8831b..8259d1de73 100644 --- a/src/dodal/devices/oav/snapshots/snapshot_with_grid.py +++ b/src/dodal/devices/oav/snapshots/snapshot_with_grid.py @@ -19,7 +19,7 @@ def __init__(self, prefix: str, name: str = "") -> None: self.top_left_x = soft_signal_rw(int) self.top_left_y = soft_signal_rw(int) - self.box_width = soft_signal_rw(float) + self.box_width = soft_signal_rw(int) self.num_boxes_x = soft_signal_rw(int) self.num_boxes_y = soft_signal_rw(int) From 6f1928165e14e2236ee84787b4311ee79a2e376f Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Thu, 24 Oct 2024 17:00:07 +0100 Subject: [PATCH 52/53] Fix docstring --- src/dodal/devices/oav/snapshots/grid_overlay.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dodal/devices/oav/snapshots/grid_overlay.py b/src/dodal/devices/oav/snapshots/grid_overlay.py index b0b7fafbef..56eb048727 100644 --- a/src/dodal/devices/oav/snapshots/grid_overlay.py +++ b/src/dodal/devices/oav/snapshots/grid_overlay.py @@ -31,8 +31,8 @@ def _add_parallel_lines_to_image( image (PIL.Image): The image to be drawn upon. start_x (int): The x coordinate (in pixels) of the start of the initial line. start_y (int): The y coordinate (in pixels) of the start of the initial line. - line_length (float): The length of each of the parallel lines in pixels. - spacing (float): The spacing, in pixels, between each parallel line. Strictly, \ + line_length (int): The length of each of the parallel lines in pixels. + spacing (int): The spacing, in pixels, between each parallel line. Strictly, \ there are spacing-1 pixels between each line num_lines (int): The total number of parallel lines to draw. orientation (Orientation): The orientation (horizontal or vertical) of the \ From f30a5198cb97616a5dd742289fc3c5f738979f14 Mon Sep 17 00:00:00 2001 From: Noemi Frisina Date: Fri, 25 Oct 2024 10:15:35 +0100 Subject: [PATCH 53/53] Change i04 oav instatiation --- src/dodal/beamlines/i04.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/dodal/beamlines/i04.py b/src/dodal/beamlines/i04.py index 05be54208c..ea435cb32d 100644 --- a/src/dodal/beamlines/i04.py +++ b/src/dodal/beamlines/i04.py @@ -345,7 +345,11 @@ def zebra(wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False) - @skip_device(lambda: BL == "s04") -def oav(wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False) -> OAV: +def oav( + wait_for_connection: bool = True, + fake_with_ophyd_sim: bool = False, + params: OAVConfig | None = None, +) -> OAV: """Get the i04 OAV device, instantiate it if it hasn't already been. If this is called when already instantiated in i04, it will return the existing object. """ @@ -355,7 +359,7 @@ def oav(wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False) -> "", wait_for_connection, fake_with_ophyd_sim, - config=OAVConfig(ZOOM_PARAMS_FILE, DISPLAY_CONFIG), + config=params or OAVConfig(ZOOM_PARAMS_FILE, DISPLAY_CONFIG), )