From c66ac6596a92e76a1b9fc533d1981c15da128c6d Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 28 Jul 2023 16:59:01 +0100 Subject: [PATCH 01/30] Add edge detection to dodal --- pyproject.toml | 1 + .../oav/edge_detection/edge_detect_utils.py | 201 ++++++++++++++++++ .../edge_detection/oav_with_edge_detection.py | 6 + .../edge_detection/test_edge_detect_utils.py | 103 +++++++++ 4 files changed, 311 insertions(+) create mode 100644 src/dodal/devices/oav/edge_detection/edge_detect_utils.py create mode 100644 src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py create mode 100644 tests/devices/unit_tests/oav/edge_detection/test_edge_detect_utils.py diff --git a/pyproject.toml b/pyproject.toml index e1c22449bf..a0bcce0583 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -22,6 +22,7 @@ dependencies = [ "requests", "graypy", "pydantic<2.0", + "opencv-python-headless", ] dynamic = ["version"] license.file = "LICENSE" diff --git a/src/dodal/devices/oav/edge_detection/edge_detect_utils.py b/src/dodal/devices/oav/edge_detection/edge_detect_utils.py new file mode 100644 index 0000000000..50db399534 --- /dev/null +++ b/src/dodal/devices/oav/edge_detection/edge_detect_utils.py @@ -0,0 +1,201 @@ +from dataclasses import dataclass +from typing import Callable, Optional, Final +import numpy as np +import cv2 + + +class ArrayProcessingFunctions(): + """ + Utility class for creating array preprocessing functions (arr -> arr with no additional parameters) + for some common operations. + """ + @staticmethod + def erode(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: + element = cv2.getStructuringElement(cv2.MORPH_RECT, (ksize, ksize)) + return lambda arr: cv2.erode(arr, element, iterations=iterations) + + @staticmethod + def dilate(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: + element = cv2.getStructuringElement(cv2.MORPH_RECT, (ksize, ksize)) + return lambda arr: cv2.dilate(arr, element, iterations=iterations) + + @staticmethod + def _morph(ksize: int, iterations: int, morph_type: int) -> Callable[[np.ndarray], np.ndarray]: + element = cv2.getStructuringElement(cv2.MORPH_RECT, (ksize, ksize)) + return lambda arr: cv2.morphologyEx( + arr, morph_type, element, iterations=iterations) + + @staticmethod + def open_morph(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: + return ArrayProcessingFunctions._morph(ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_OPEN) + + @staticmethod + def close(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: + return ArrayProcessingFunctions._morph(ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_CLOSE) + + @staticmethod + def gradient(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: + return ArrayProcessingFunctions._morph(ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_GRADIENT) + + @staticmethod + def top_hat(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: + return ArrayProcessingFunctions._morph(ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_TOPHAT) + + @staticmethod + def black_hat(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: + return ArrayProcessingFunctions._morph(ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_BLACKHAT) + + @staticmethod + def blur(ksize: int) -> Callable[[np.ndarray], np.ndarray]: + return lambda arr: cv2.blur(arr, ksize=(ksize, ksize)) + + @staticmethod + def gaussian_blur(ksize: int) -> Callable[[np.ndarray], np.ndarray]: + # Kernel size should be odd. + if not ksize % 2: + ksize += 1 + return lambda arr: cv2.GaussianBlur(arr, (ksize, ksize), 0) + + @staticmethod + def median_blur(ksize: int) -> Callable[[np.ndarray], np.ndarray]: + if not ksize % 2: + ksize += 1 + return lambda arr: cv2.medianBlur(arr, ksize) + + +# A substitute for "None" which can fit into an np.int32 array/waveform record. +# EDM plot can't handle negative integers, so best to use 0 rather than -1. +NONE_VALUE: Final[int] = 0 + + +@dataclass +class SampleLocation(): + """ + Holder type for results from sample detection. + """ + tip_y: Optional[int] + tip_x: Optional[int] + edge_top: Optional[np.ndarray] + edge_bottom: Optional[np.ndarray] + + +class MxSampleDetect(object): + + def __init__( + self, + *, + preprocess: Callable[[np.ndarray], np.ndarray] = lambda arr: arr, + canny_upper: int = 100, + canny_lower: int = 50, + close_ksize: int = 5, + close_iterations: int = 5, + scan_direction: int = 1, + min_tip_height: int = 5, + ): + """ + Configures sample detection parameters. + + Args: + preprocess: A preprocessing function applied to the array after conversion to grayscale. + See implementations of common functions in ArrayProcessingFunctions for predefined conversions + Defaults to a no-op (i.e. no preprocessing) + canny_upper: upper threshold for canny edge detection + canny_lower: lower threshold for canny edge detection + close_ksize: kernel size for "close" operation + close_iterations: number of iterations for "close" operation + scan_direction: +1 for left-to-right, -1 for right-to-left + min_tip_height: minimum height of pin tip + """ + + self.preprocess = preprocess + self.canny_upper = canny_upper + self.canny_lower = canny_lower + self.close_ksize = close_ksize + self.close_iterations = close_iterations + + if scan_direction not in [1, -1]: + raise ValueError("Invalid scan direction, expected +1 for left-to-right or -1 for right-to-left") + self.scan_direction = scan_direction + + self.min_tip_height = min_tip_height + + def processArray(self, arr: np.ndarray) -> SampleLocation: + # Get a greyscale version of the input. + if arr.ndim == 3: + gray_arr = cv2.cvtColor(arr, cv2.COLOR_BGR2GRAY) + else: + assert arr.ndim == 2 + gray_arr = arr + + # Preprocess the array. (Use the greyscale one.) + pp_arr = self.preprocess(gray_arr) + + # (Could do a remove_dirt step here if wanted.) + + # Find some edges. + edge_arr = cv2.Canny(pp_arr, self.canny_upper, self.canny_lower) + + # Do a "close" image operation. (Add other options?) + closed_arr = ArrayProcessingFunctions.close(self.close_ksize, self.close_iterations)(edge_arr) + + # Find the sample. + return self._locate_sample(closed_arr) + + def _locate_sample(self, edge_arr: np.ndarray) -> SampleLocation: + # Straight port of Tom Cobb's algorithm from the original (adOpenCV) + # mxSampleDetect. + + # Index into edges_arr like [y, x], not [x, y]! + height, width = edge_arr.shape + + tip_y, tip_x = None, None + + # TODO: use np instead? + top = [None]*width + bottom = [None]*width + + columns = range(width)[::self.scan_direction] + + n: np.ndarray = np.transpose(np.nonzero(edge_arr)) + + # TODO: inefficient + for y, x in n: + if top[x] is None: + top[x] = y + bottom[x] = y + + for x in columns: + # Look for the first non-narrow region between top and bottom edges. + if tip_x is None and top[x] is not None and abs(top[x] - bottom[x]) > self.min_tip_height: + + # Move backwards to where there were no edges at all... + while top[x] is not None: + x += -self.scan_direction + if x == -1 or x == width: + # (In this case the sample is off the edge of the picture.) + break + x += self.scan_direction # ...and forward one step. x is now at the tip. + + tip_x = x + tip_y = int(round(0.5*(top[x] + bottom[x]))) + + # Zero the edge arrays to the left (right) of the tip. + # TODO: inefficient + if self.scan_direction == 1: + top[:x] = [None for _ in range(x)] + bottom[:x] = [None for _ in range(x)] + else: + top[x+1:] = [None for _ in range(len(columns) - x - 1)] + bottom[x+1:] = [None for _ in range(len(columns) - x - 1)] + + # Prepare for export to PVs. + # TODO: inefficient + edge_top = np.asarray( + [NONE_VALUE if t is None else t for t in top], dtype=np.int32) + edge_bottom = np.asarray( + [NONE_VALUE if b is None else b for b in bottom], dtype=np.int32) + + if tip_y is None or tip_x is None: + tip_y, tip_x = -1, -1 + + return SampleLocation(tip_y=tip_y, tip_x=tip_x, edge_bottom=edge_bottom, edge_top=edge_top) diff --git a/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py b/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py new file mode 100644 index 0000000000..51214c1774 --- /dev/null +++ b/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py @@ -0,0 +1,6 @@ +from dodal.devices.oav.oav_detector import OAV + + +class OAVWithEdgeDetection(OAV): + def get_edge(self): + pass diff --git a/tests/devices/unit_tests/oav/edge_detection/test_edge_detect_utils.py b/tests/devices/unit_tests/oav/edge_detection/test_edge_detect_utils.py new file mode 100644 index 0000000000..c512b5f7f6 --- /dev/null +++ b/tests/devices/unit_tests/oav/edge_detection/test_edge_detect_utils.py @@ -0,0 +1,103 @@ +from dodal.devices.oav.edge_detection.edge_detect_utils import MxSampleDetect + +import numpy as np +import pytest + + +def test_locate_sample_simple_forward(): + test_arr = np.array( + [ + [0, 0, 0, 0, 0], + [0, 1, 1, 1, 0], + [0, 1, 0, 1, 1], + [0, 1, 1, 1, 0], + [0, 0, 0, 0, 0], + ], + dtype=np.int32, + ) + + location = MxSampleDetect(min_tip_height=1)._locate_sample(test_arr) + + assert location.edge_top is not None + assert location.edge_bottom is not None + + np.testing.assert_array_equal(location.edge_top, np.array([0, 1, 1, 1, 2], dtype=np.int32)) + np.testing.assert_array_equal(location.edge_bottom, np.array([0, 3, 3, 3, 2], dtype=np.int32)) + + assert location.tip_x == 1 + assert location.tip_y == 2 + + +def test_locate_sample_simple_reverse(): + test_arr = np.array( + [ + [0, 0, 0, 0, 0], + [0, 1, 1, 1, 0], + [1, 1, 0, 1, 0], + [0, 1, 1, 1, 0], + [0, 0, 0, 0, 0], + ], + dtype=np.int32, + ) + + location = MxSampleDetect(min_tip_height=1, scan_direction=-1)._locate_sample(test_arr) + + assert location.edge_top is not None + assert location.edge_bottom is not None + + np.testing.assert_array_equal(location.edge_top, np.array([2, 1, 1, 1, 0], dtype=np.int32)) + np.testing.assert_array_equal(location.edge_bottom, np.array([2, 3, 3, 3, 0], dtype=np.int32)) + + assert location.tip_x == 3 + assert location.tip_y == 2 + + +def test_locate_sample_no_edges(): + test_arr = np.array( + [ + [0, 0, 0, 0, 0], + [0, 0, 0, 0, 0], + [0, 0, 0, 0, 0], + [0, 0, 0, 0, 0], + [0, 0, 0, 0, 0], + ], + dtype=np.int32, + ) + + location = MxSampleDetect(min_tip_height=1)._locate_sample(test_arr) + + assert location.edge_top is not None + assert location.edge_bottom is not None + + np.testing.assert_array_equal(location.edge_top, np.array([0, 0, 0, 0, 0], dtype=np.int32)) + np.testing.assert_array_equal(location.edge_bottom, np.array([0, 0, 0, 0, 0], dtype=np.int32)) + + assert location.tip_x == -1 + assert location.tip_y == -1 + + +@pytest.mark.parametrize("direction,x_centre", [(1, 0), (-1, 4)]) +def test_locate_sample_tip_off_screen(direction, x_centre): + test_arr = np.array( + [ + [0, 0, 0, 0, 0], + [1, 1, 1, 1, 1], + [0, 0, 0, 0, 0], + [1, 1, 1, 1, 1], + [0, 0, 0, 0, 0], + ], + dtype=np.int32, + ) + + location = MxSampleDetect(min_tip_height=1, scan_direction=direction)._locate_sample(test_arr) + + assert location.edge_top is not None + assert location.edge_bottom is not None + + np.testing.assert_array_equal(location.edge_top, np.array([1, 1, 1, 1, 1], dtype=np.int32)) + np.testing.assert_array_equal(location.edge_bottom, np.array([3, 3, 3, 3, 3], dtype=np.int32)) + + # Currently sample "off screen" is not considered an error so a centre is still returned. + # Maybe it should be? + assert location.tip_x == x_centre + assert location.tip_y == 2 From 79c580387d6f3f0e678886555f62bf686e256cc9 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Tue, 8 Aug 2023 10:59:54 +0100 Subject: [PATCH 02/30] refactor to use numpy for most operations --- .../oav/edge_detection/edge_detect_utils.py | 197 +++++++++++------- .../edge_detection/test_edge_detect_utils.py | 109 ++++++++-- 2 files changed, 214 insertions(+), 92 deletions(-) diff --git a/src/dodal/devices/oav/edge_detection/edge_detect_utils.py b/src/dodal/devices/oav/edge_detection/edge_detect_utils.py index 50db399534..2abe49104d 100644 --- a/src/dodal/devices/oav/edge_detection/edge_detect_utils.py +++ b/src/dodal/devices/oav/edge_detection/edge_detect_utils.py @@ -1,14 +1,16 @@ from dataclasses import dataclass -from typing import Callable, Optional, Final -import numpy as np +from typing import Callable, Final, Optional, Tuple + import cv2 +import numpy as np -class ArrayProcessingFunctions(): +class ArrayProcessingFunctions: """ - Utility class for creating array preprocessing functions (arr -> arr with no additional parameters) + Utility class for creating array preprocessing functions (arr -> arr with no additional parameters) for some common operations. """ + @staticmethod def erode(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: element = cv2.getStructuringElement(cv2.MORPH_RECT, (ksize, ksize)) @@ -18,61 +20,74 @@ def erode(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: def dilate(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: element = cv2.getStructuringElement(cv2.MORPH_RECT, (ksize, ksize)) return lambda arr: cv2.dilate(arr, element, iterations=iterations) - + @staticmethod - def _morph(ksize: int, iterations: int, morph_type: int) -> Callable[[np.ndarray], np.ndarray]: + def _morph( + ksize: int, iterations: int, morph_type: int + ) -> Callable[[np.ndarray], np.ndarray]: element = cv2.getStructuringElement(cv2.MORPH_RECT, (ksize, ksize)) return lambda arr: cv2.morphologyEx( - arr, morph_type, element, iterations=iterations) + arr, morph_type, element, iterations=iterations + ) @staticmethod def open_morph(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: - return ArrayProcessingFunctions._morph(ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_OPEN) + return ArrayProcessingFunctions._morph( + ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_OPEN + ) @staticmethod def close(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: - return ArrayProcessingFunctions._morph(ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_CLOSE) + return ArrayProcessingFunctions._morph( + ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_CLOSE + ) @staticmethod def gradient(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: - return ArrayProcessingFunctions._morph(ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_GRADIENT) + return ArrayProcessingFunctions._morph( + ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_GRADIENT + ) @staticmethod def top_hat(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: - return ArrayProcessingFunctions._morph(ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_TOPHAT) + return ArrayProcessingFunctions._morph( + ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_TOPHAT + ) @staticmethod def black_hat(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: - return ArrayProcessingFunctions._morph(ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_BLACKHAT) + return ArrayProcessingFunctions._morph( + ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_BLACKHAT + ) @staticmethod def blur(ksize: int) -> Callable[[np.ndarray], np.ndarray]: return lambda arr: cv2.blur(arr, ksize=(ksize, ksize)) - + @staticmethod def gaussian_blur(ksize: int) -> Callable[[np.ndarray], np.ndarray]: # Kernel size should be odd. - if not ksize % 2: + if not ksize % 2: ksize += 1 return lambda arr: cv2.GaussianBlur(arr, (ksize, ksize), 0) - + @staticmethod def median_blur(ksize: int) -> Callable[[np.ndarray], np.ndarray]: - if not ksize % 2: + if not ksize % 2: ksize += 1 return lambda arr: cv2.medianBlur(arr, ksize) -# A substitute for "None" which can fit into an np.int32 array/waveform record. -# EDM plot can't handle negative integers, so best to use 0 rather than -1. -NONE_VALUE: Final[int] = 0 +# A substitute for "None" which can fit into an np.int32 array. +NONE_VALUE: Final[int] = -1 @dataclass -class SampleLocation(): +class SampleLocation: """ Holder type for results from sample detection. """ + tip_y: Optional[int] tip_x: Optional[int] edge_top: Optional[np.ndarray] @@ -80,7 +95,6 @@ class SampleLocation(): class MxSampleDetect(object): - def __init__( self, *, @@ -96,7 +110,7 @@ def __init__( Configures sample detection parameters. Args: - preprocess: A preprocessing function applied to the array after conversion to grayscale. + preprocess: A preprocessing function applied to the array after conversion to grayscale. See implementations of common functions in ArrayProcessingFunctions for predefined conversions Defaults to a no-op (i.e. no preprocessing) canny_upper: upper threshold for canny edge detection @@ -114,7 +128,9 @@ def __init__( self.close_iterations = close_iterations if scan_direction not in [1, -1]: - raise ValueError("Invalid scan direction, expected +1 for left-to-right or -1 for right-to-left") + raise ValueError( + "Invalid scan direction, expected +1 for left-to-right or -1 for right-to-left" + ) self.scan_direction = scan_direction self.min_tip_height = min_tip_height @@ -136,66 +152,87 @@ def processArray(self, arr: np.ndarray) -> SampleLocation: edge_arr = cv2.Canny(pp_arr, self.canny_upper, self.canny_lower) # Do a "close" image operation. (Add other options?) - closed_arr = ArrayProcessingFunctions.close(self.close_ksize, self.close_iterations)(edge_arr) + closed_arr = ArrayProcessingFunctions.close( + self.close_ksize, self.close_iterations + )(edge_arr) # Find the sample. return self._locate_sample(closed_arr) - - def _locate_sample(self, edge_arr: np.ndarray) -> SampleLocation: - # Straight port of Tom Cobb's algorithm from the original (adOpenCV) - # mxSampleDetect. - # Index into edges_arr like [y, x], not [x, y]! + @staticmethod + def _first_and_last_nonzero_by_columns( + arr: np.ndarray, + ) -> Tuple[np.ndarray, np.ndarray]: + """ + Finds the indexes of the first & last non-zero values by column in a 2d array. + + Outputs will contain NONE_VALUE if no non-zero values exist in a column. + + i.e. for input: + [ + [0, 0, 0, 1], + [1, 1, 0, 0], + [0, 1, 0, 1], + ] + + first_nonzero will be: [1, 1, NONE_VALUE, 0] + last_nonzero will be [1, 2, NONE_VALUE, 2] + """ + mask = arr != 0 + first_nonzero = np.where(mask.any(axis=0), mask.argmax(axis=0), NONE_VALUE) + + flipped = arr.shape[0] - np.flip(mask, axis=0).argmax(axis=0) - 1 + last_nonzero = np.where(mask.any(axis=0), flipped, NONE_VALUE) + return first_nonzero, last_nonzero + + def _locate_sample(self, edge_arr: np.ndarray) -> SampleLocation: height, width = edge_arr.shape - tip_y, tip_x = None, None - - # TODO: use np instead? - top = [None]*width - bottom = [None]*width - - columns = range(width)[::self.scan_direction] - - n: np.ndarray = np.transpose(np.nonzero(edge_arr)) - - # TODO: inefficient - for y, x in n: - if top[x] is None: - top[x] = y - bottom[x] = y - - for x in columns: - # Look for the first non-narrow region between top and bottom edges. - if tip_x is None and top[x] is not None and abs(top[x] - bottom[x]) > self.min_tip_height: - - # Move backwards to where there were no edges at all... - while top[x] is not None: - x += -self.scan_direction - if x == -1 or x == width: - # (In this case the sample is off the edge of the picture.) - break - x += self.scan_direction # ...and forward one step. x is now at the tip. - - tip_x = x - tip_y = int(round(0.5*(top[x] + bottom[x]))) - - # Zero the edge arrays to the left (right) of the tip. - # TODO: inefficient - if self.scan_direction == 1: - top[:x] = [None for _ in range(x)] - bottom[:x] = [None for _ in range(x)] - else: - top[x+1:] = [None for _ in range(len(columns) - x - 1)] - bottom[x+1:] = [None for _ in range(len(columns) - x - 1)] - - # Prepare for export to PVs. - # TODO: inefficient - edge_top = np.asarray( - [NONE_VALUE if t is None else t for t in top], dtype=np.int32) - edge_bottom = np.asarray( - [NONE_VALUE if b is None else b for b in bottom], dtype=np.int32) - - if tip_y is None or tip_x is None: - tip_y, tip_x = -1, -1 - - return SampleLocation(tip_y=tip_y, tip_x=tip_x, edge_bottom=edge_bottom, edge_top=edge_top) + top, bottom = MxSampleDetect._first_and_last_nonzero_by_columns(edge_arr) + + # Calculate widths. In general if bottom == top this has width 1. + # special case for bottom == top == NONE_VALUE (i.e. no edge at all), that has width 0. + widths = np.where(top != NONE_VALUE, bottom - top + 1, 0) + + # Generate the indices of columns with widths larger than the specified min tip height. + column_indices_with_non_narrow_widths = np.nonzero( + widths >= self.min_tip_height + )[0] + + if column_indices_with_non_narrow_widths.shape[0] == 0: + # No non-narrow locations - sample not in picture? + # Or wrong parameters etc. + return SampleLocation( + tip_y=NONE_VALUE, tip_x=NONE_VALUE, edge_bottom=bottom, edge_top=top + ) + + # Choose our starting point - i.e. first column with non-narrow width for positive scan, last one for negative scan. + if self.scan_direction == 1: + start_column = int(column_indices_with_non_narrow_widths[0]) + else: + start_column = int(column_indices_with_non_narrow_widths[-1]) + + x = start_column + + # Move backwards to where there were no edges at all... + while top[x] != NONE_VALUE: + x += -self.scan_direction + if x == -1 or x == width: + # (In this case the sample is off the edge of the picture.) + break + x += self.scan_direction # ...and forward one step. x is now at the tip. + + tip_x = x + tip_y = int(round(0.5 * (top[x] + bottom[x]))) + + # clear edges to the left (right) of the tip. + if self.scan_direction == 1: + top[:x] = NONE_VALUE + bottom[:x] = NONE_VALUE + else: + top[x + 1 :] = NONE_VALUE + bottom[x + 1 :] = NONE_VALUE + + return SampleLocation( + tip_y=tip_y, tip_x=tip_x, edge_bottom=bottom, edge_top=top + ) diff --git a/tests/devices/unit_tests/oav/edge_detection/test_edge_detect_utils.py b/tests/devices/unit_tests/oav/edge_detection/test_edge_detect_utils.py index c512b5f7f6..396156bfcc 100644 --- a/tests/devices/unit_tests/oav/edge_detection/test_edge_detect_utils.py +++ b/tests/devices/unit_tests/oav/edge_detection/test_edge_detect_utils.py @@ -1,8 +1,11 @@ -from dodal.devices.oav.edge_detection.edge_detect_utils import MxSampleDetect - import numpy as np import pytest +from dodal.devices.oav.edge_detection.edge_detect_utils import ( + NONE_VALUE, + MxSampleDetect, +) + def test_locate_sample_simple_forward(): test_arr = np.array( @@ -21,8 +24,12 @@ def test_locate_sample_simple_forward(): assert location.edge_top is not None assert location.edge_bottom is not None - np.testing.assert_array_equal(location.edge_top, np.array([0, 1, 1, 1, 2], dtype=np.int32)) - np.testing.assert_array_equal(location.edge_bottom, np.array([0, 3, 3, 3, 2], dtype=np.int32)) + np.testing.assert_array_equal( + location.edge_top, np.array([NONE_VALUE, 1, 1, 1, 2], dtype=np.int32) + ) + np.testing.assert_array_equal( + location.edge_bottom, np.array([NONE_VALUE, 3, 3, 3, 2], dtype=np.int32) + ) assert location.tip_x == 1 assert location.tip_y == 2 @@ -40,13 +47,19 @@ def test_locate_sample_simple_reverse(): dtype=np.int32, ) - location = MxSampleDetect(min_tip_height=1, scan_direction=-1)._locate_sample(test_arr) + location = MxSampleDetect(min_tip_height=1, scan_direction=-1)._locate_sample( + test_arr + ) assert location.edge_top is not None assert location.edge_bottom is not None - np.testing.assert_array_equal(location.edge_top, np.array([2, 1, 1, 1, 0], dtype=np.int32)) - np.testing.assert_array_equal(location.edge_bottom, np.array([2, 3, 3, 3, 0], dtype=np.int32)) + np.testing.assert_array_equal( + location.edge_top, np.array([2, 1, 1, 1, NONE_VALUE], dtype=np.int32) + ) + np.testing.assert_array_equal( + location.edge_bottom, np.array([2, 3, 3, 3, NONE_VALUE], dtype=np.int32) + ) assert location.tip_x == 3 assert location.tip_y == 2 @@ -69,8 +82,14 @@ def test_locate_sample_no_edges(): assert location.edge_top is not None assert location.edge_bottom is not None - np.testing.assert_array_equal(location.edge_top, np.array([0, 0, 0, 0, 0], dtype=np.int32)) - np.testing.assert_array_equal(location.edge_bottom, np.array([0, 0, 0, 0, 0], dtype=np.int32)) + np.testing.assert_array_equal( + location.edge_top, + np.broadcast_to(np.array([NONE_VALUE], dtype=np.int32), (5,)), + ) + np.testing.assert_array_equal( + location.edge_bottom, + np.broadcast_to(np.array([NONE_VALUE], dtype=np.int32), (5,)), + ) assert location.tip_x == -1 assert location.tip_y == -1 @@ -89,15 +108,81 @@ def test_locate_sample_tip_off_screen(direction, x_centre): dtype=np.int32, ) - location = MxSampleDetect(min_tip_height=1, scan_direction=direction)._locate_sample(test_arr) + location = MxSampleDetect( + min_tip_height=1, scan_direction=direction + )._locate_sample(test_arr) assert location.edge_top is not None assert location.edge_bottom is not None - np.testing.assert_array_equal(location.edge_top, np.array([1, 1, 1, 1, 1], dtype=np.int32)) - np.testing.assert_array_equal(location.edge_bottom, np.array([3, 3, 3, 3, 3], dtype=np.int32)) + np.testing.assert_array_equal( + location.edge_top, np.array([1, 1, 1, 1, 1], dtype=np.int32) + ) + np.testing.assert_array_equal( + location.edge_bottom, np.array([3, 3, 3, 3, 3], dtype=np.int32) + ) # Currently sample "off screen" is not considered an error so a centre is still returned. # Maybe it should be? assert location.tip_x == x_centre assert location.tip_y == 2 + + +@pytest.mark.parametrize( + "min_tip_width,sample_x_location,expected_top_edge,expected_bottom_edge", + [ + (1, 0, [2, NONE_VALUE, 1, 0, 0], [2, NONE_VALUE, 3, 4, 4]), + (3, 2, [NONE_VALUE, NONE_VALUE, 1, 0, 0], [NONE_VALUE, NONE_VALUE, 3, 4, 4]), + ], +) +def test_locate_sample_with_min_tip_height( + min_tip_width, sample_x_location, expected_top_edge, expected_bottom_edge +): + # "noise" at (0, 2) should be ignored by "min tip width" mechanism if it has min width >1 + # Tip should therefore be detected at (2, 2) for min_tip_width = 3, or (0, 2) for min_tip_width = 1 + test_arr = np.array( + [ + [0, 0, 0, 1, 1], + [0, 0, 1, 0, 0], + [1, 0, 1, 0, 0], + [0, 0, 1, 0, 0], + [0, 0, 0, 1, 1], + ], + dtype=np.int32, + ) + + location = MxSampleDetect( + min_tip_height=min_tip_width, scan_direction=1 + )._locate_sample(test_arr) + + assert location.edge_top is not None + assert location.edge_bottom is not None + + # Even though an edge is detected at x=4, it should be explicitly set to NONE_VALUE + # as this is past the detected "tip" + np.testing.assert_array_equal( + location.edge_top, np.array(expected_top_edge, dtype=np.int32) + ) + np.testing.assert_array_equal( + location.edge_bottom, + np.array(expected_bottom_edge, dtype=np.int32), + ) + + assert location.tip_x == sample_x_location + assert location.tip_y == 2 + + +def test_first_and_last_nonzero_by_columns(): + test_arr = np.array( + [ + [0, 0, 0, 1], + [1, 1, 0, 0], + [0, 1, 0, 1], + ], + dtype=np.int32, + ) + + first, last = MxSampleDetect._first_and_last_nonzero_by_columns(test_arr) + + np.testing.assert_array_equal(first, np.array([1, 1, NONE_VALUE, 0])) + np.testing.assert_array_equal(last, np.array([1, 2, NONE_VALUE, 2])) From 0fe133cf094bef3e3b91a3ed472beabfad946a8a Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Tue, 8 Aug 2023 13:17:34 +0100 Subject: [PATCH 03/30] Minor tidying/commenting --- .../devices/oav/edge_detection/edge_detect_utils.py | 10 +++++----- .../oav/edge_detection/test_edge_detect_utils.py | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/dodal/devices/oav/edge_detection/edge_detect_utils.py b/src/dodal/devices/oav/edge_detection/edge_detect_utils.py index 2abe49104d..17ca802ecc 100644 --- a/src/dodal/devices/oav/edge_detection/edge_detect_utils.py +++ b/src/dodal/devices/oav/edge_detection/edge_detect_utils.py @@ -79,6 +79,7 @@ def median_blur(ksize: int) -> Callable[[np.ndarray], np.ndarray]: # A substitute for "None" which can fit into an np.int32 array. +# Also used as a substitute for a not-found sample position. NONE_VALUE: Final[int] = -1 @@ -87,11 +88,10 @@ class SampleLocation: """ Holder type for results from sample detection. """ - tip_y: Optional[int] tip_x: Optional[int] - edge_top: Optional[np.ndarray] - edge_bottom: Optional[np.ndarray] + edge_top: np.ndarray + edge_bottom: np.ndarray class MxSampleDetect(object): @@ -201,9 +201,9 @@ def _locate_sample(self, edge_arr: np.ndarray) -> SampleLocation: if column_indices_with_non_narrow_widths.shape[0] == 0: # No non-narrow locations - sample not in picture? - # Or wrong parameters etc. + # Or wrong parameters for edge-finding, ... return SampleLocation( - tip_y=NONE_VALUE, tip_x=NONE_VALUE, edge_bottom=bottom, edge_top=top + tip_y=None, tip_x=None, edge_bottom=bottom, edge_top=top ) # Choose our starting point - i.e. first column with non-narrow width for positive scan, last one for negative scan. diff --git a/tests/devices/unit_tests/oav/edge_detection/test_edge_detect_utils.py b/tests/devices/unit_tests/oav/edge_detection/test_edge_detect_utils.py index 396156bfcc..08ff3c11a4 100644 --- a/tests/devices/unit_tests/oav/edge_detection/test_edge_detect_utils.py +++ b/tests/devices/unit_tests/oav/edge_detection/test_edge_detect_utils.py @@ -91,8 +91,8 @@ def test_locate_sample_no_edges(): np.broadcast_to(np.array([NONE_VALUE], dtype=np.int32), (5,)), ) - assert location.tip_x == -1 - assert location.tip_y == -1 + assert location.tip_x is None + assert location.tip_y is None @pytest.mark.parametrize("direction,x_centre", [(1, 0), (-1, 4)]) From d3541950e08754addc309eb2f08c9b3775c68486 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Wed, 9 Aug 2023 15:32:54 +0100 Subject: [PATCH 04/30] pre v2 refactor --- .../oav/edge_detection/edge_detect_utils.py | 3 + .../edge_detection/oav_with_edge_detection.py | 116 +++++++++++++++++- 2 files changed, 117 insertions(+), 2 deletions(-) diff --git a/src/dodal/devices/oav/edge_detection/edge_detect_utils.py b/src/dodal/devices/oav/edge_detection/edge_detect_utils.py index 17ca802ecc..dc92baaf44 100644 --- a/src/dodal/devices/oav/edge_detection/edge_detect_utils.py +++ b/src/dodal/devices/oav/edge_detection/edge_detect_utils.py @@ -10,6 +10,9 @@ class ArrayProcessingFunctions: Utility class for creating array preprocessing functions (arr -> arr with no additional parameters) for some common operations. """ + @staticmethod + def identity() -> Callable[[np.ndarray], np.ndarray]: + return lambda arr: arr @staticmethod def erode(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: diff --git a/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py b/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py index 51214c1774..f6888534de 100644 --- a/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py +++ b/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py @@ -1,6 +1,118 @@ from dodal.devices.oav.oav_detector import OAV +from ophyd.signal import EpicsSignalRO, Signal, Kind +from ophyd.status import SubscriptionStatus +from ophyd import Component, Device +from dodal.devices.oav.edge_detection.edge_detect_utils import MxSampleDetect, ArrayProcessingFunctions, NONE_VALUE as INVALID_POSITION_VALUE, SampleLocation +from dodal.log import LOGGER +from typing import TYPE_CHECKING, Callable, Final, Tuple + +from ophyd.v2.core import StandardReadable, DeviceCollector +from ophyd.v2.epics import epics_signal_r + +import numpy as np +import numpy.typing + + +class AreaDetectorPvaPluginArray(StandardReadable): + """ + An Ophyd v2 device wrapping the areadetector PVA plugin. Will fetch array data via PVA. + """ + + def __init__(self, prefix: str, name: str = ""): + + self.array_data = epics_signal_r(numpy.typing.NDArray, "pva://{}ARR:ArrayData".format(prefix)) + + self.set_readable_signals( + read=[self.array_data], + config=[], + ) + super().__init__(name=name) + + +class EdgeDetection(StandardReadable): + + INVALID_POSITION: Final[Tuple[int, int]] = (INVALID_POSITION_VALUE, INVALID_POSITION_VALUE) + + def __init__(self, prefix, name: str = ""): + + self.array_data = epics_signal_r(numpy.typing.NDArray, "pva://{}ARR:ArrayData".format(prefix)) + + with DeviceCollector(): + self.pva_array_data: StandardReadable = AreaDetectorPvaPluginArray(prefix=prefix) + + self.triggered_tip: Signal = Component(Signal, kind=Kind.hinted, value=INVALID_POSITION) # type: ignore + + self.oav_width: int = 1024 + self.oav_height: int = 768 + + self.timeout: float = 10.0 + + self.preprocess: Callable[[np.ndarray], np.ndarray] = ArrayProcessingFunctions.identity() + self.canny_upper: int = 100 + self.canny_lower: int = 50 + self.close_ksize: int = 5 + self.close_iterations: int = 5 + self.scan_direction: int = 1 + self.min_tip_height: int = 5 + + super().__init__(name=name) + + def update_tip_position(self, *, value, **_): + + sample_detection = MxSampleDetect( + preprocess=self.preprocess, + canny_lower=self.canny_lower, + canny_upper=self.canny_upper, + close_ksize=self.close_ksize, + close_iterations=self.close_iterations, + scan_direction=self.scan_direction, + min_tip_height=self.min_tip_height, + ) + + try: + num_pixels = self.oav_height * self.oav_width + value_len = value.shape[0] + if value_len == num_pixels * 3: + # RGB data + value = value.reshape(self.oav_height, self.oav_width, 3) + elif value_len == num_pixels: + # Grayscale data + value = value.reshape(self.oav_height, self.oav_width) + else: + # Something else? + raise ValueError("Unexpected data array size: expected {} (grayscale data) or {} (rgb data), got {}", num_pixels, num_pixels * 3, value_len) + + location = sample_detection.processArray(value) + tip_x = location.tip_x + tip_y = location.tip_y + except Exception as e: + LOGGER.error("Failed to detect sample position: ", e) + tip_x = INVALID_POSITION_VALUE + tip_y = INVALID_POSITION_VALUE + + self.triggered_tip.put((tip_x, tip_y)) + + def trigger(self) -> SubscriptionStatus: + # Clear last value + self.triggered_tip.put(INVALID_POSITION_VALUE) + + subscription_status = SubscriptionStatus( + self.pva_array_data, self.update_tip_position, run=True, timeout=self.timeout + ) + return subscription_status class OAVWithEdgeDetection(OAV): - def get_edge(self): - pass + edge_detect: Component[EdgeDetection] = Component(EdgeDetection, "-DI-OAV-01:") + + +if __name__ == "__main__": + x = OAVWithEdgeDetection(name="oav", prefix="BL03I") + x.wait_for_connection() + img = x.edge_detect.array_data.get() + with open("/scratch/beamline_i03_oav_image_temp", "wb") as f: + np.save(f, img) + + import matplotlib.pyplot as plt + plt.imshow(np.transpose(img.reshape(80, 80, 3), axes=[0, 1, 2])) + plt.show() From 096e4288071e26c8054b9045e822f2ef9c1459c9 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Wed, 9 Aug 2023 16:13:25 +0100 Subject: [PATCH 05/30] Begin ophyd v2 refactor --- .../edge_detection/oav_with_edge_detection.py | 91 +++++++++---------- 1 file changed, 45 insertions(+), 46 deletions(-) diff --git a/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py b/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py index f6888534de..c1127e3fcd 100644 --- a/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py +++ b/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py @@ -1,32 +1,34 @@ from dodal.devices.oav.oav_detector import OAV -from ophyd.signal import EpicsSignalRO, Signal, Kind -from ophyd.status import SubscriptionStatus -from ophyd import Component, Device from dodal.devices.oav.edge_detection.edge_detect_utils import MxSampleDetect, ArrayProcessingFunctions, NONE_VALUE as INVALID_POSITION_VALUE, SampleLocation from dodal.log import LOGGER -from typing import TYPE_CHECKING, Callable, Final, Tuple +from typing import TYPE_CHECKING, Callable, Final, Tuple, TypeVar, Optional -from ophyd.v2.core import StandardReadable, DeviceCollector -from ophyd.v2.epics import epics_signal_r +from ophyd.v2.core import AsyncStatus, StandardReadable, DeviceCollector +from ophyd.v2.epics import epics_signal_r, SignalR, SignalRW import numpy as np -import numpy.typing +from numpy.typing import NDArray +import asyncio -class AreaDetectorPvaPluginArray(StandardReadable): - """ - An Ophyd v2 device wrapping the areadetector PVA plugin. Will fetch array data via PVA. - """ - - def __init__(self, prefix: str, name: str = ""): - self.array_data = epics_signal_r(numpy.typing.NDArray, "pva://{}ARR:ArrayData".format(prefix)) +T = TypeVar('T') - self.set_readable_signals( - read=[self.array_data], - config=[], - ) - super().__init__(name=name) +class _SoftSignal(SignalRW[T]): + def __init__(self, initial_value: T): + self.value: T = initial_value + + async def _set(self, value: T): + self.value = value + + def set(self, value: T, wait=True, timeout=None) -> AsyncStatus: + coro = self._set(value) + return AsyncStatus(coro) + + def read(self) -> T: + if self.value is None: + raise ValueError("Soft signal read before being set") + return self.value class EdgeDetection(StandardReadable): @@ -34,13 +36,10 @@ class EdgeDetection(StandardReadable): INVALID_POSITION: Final[Tuple[int, int]] = (INVALID_POSITION_VALUE, INVALID_POSITION_VALUE) def __init__(self, prefix, name: str = ""): - - self.array_data = epics_signal_r(numpy.typing.NDArray, "pva://{}ARR:ArrayData".format(prefix)) - with DeviceCollector(): - self.pva_array_data: StandardReadable = AreaDetectorPvaPluginArray(prefix=prefix) + self.array_data: SignalR[NDArray] = epics_signal_r(NDArray, "pva://{}ARR:ArrayData".format(prefix)) - self.triggered_tip: Signal = Component(Signal, kind=Kind.hinted, value=INVALID_POSITION) # type: ignore + self.triggered_tip: SignalRW[Tuple[int | None, int | None]] = _SoftSignal(initial_value=EdgeDetection.INVALID_POSITION) self.oav_width: int = 1024 self.oav_height: int = 768 @@ -55,6 +54,15 @@ def __init__(self, prefix, name: str = ""): self.scan_direction: int = 1 self.min_tip_height: int = 5 + self.set_readable_signals( + read=[ + self.array_data + ], + config=[ + self.triggered_tip + ], + ) + super().__init__(name=name) def update_tip_position(self, *, value, **_): @@ -87,32 +95,23 @@ def update_tip_position(self, *, value, **_): tip_y = location.tip_y except Exception as e: LOGGER.error("Failed to detect sample position: ", e) - tip_x = INVALID_POSITION_VALUE - tip_y = INVALID_POSITION_VALUE + tip_x = None + tip_y = None - self.triggered_tip.put((tip_x, tip_y)) + self.triggered_tip.set((tip_x, tip_y)) - def trigger(self) -> SubscriptionStatus: + async def read(self) -> Tuple[int | None, int | None]: # Clear last value - self.triggered_tip.put(INVALID_POSITION_VALUE) - - subscription_status = SubscriptionStatus( - self.pva_array_data, self.update_tip_position, run=True, timeout=self.timeout - ) - return subscription_status + self.triggered_tip.set(EdgeDetection.INVALID_POSITION) - -class OAVWithEdgeDetection(OAV): - edge_detect: Component[EdgeDetection] = Component(EdgeDetection, "-DI-OAV-01:") + self.update_tip_position(value=self.array_data.get_value()) + return await self.triggered_tip.get_value() if __name__ == "__main__": - x = OAVWithEdgeDetection(name="oav", prefix="BL03I") - x.wait_for_connection() - img = x.edge_detect.array_data.get() - with open("/scratch/beamline_i03_oav_image_temp", "wb") as f: - np.save(f, img) - - import matplotlib.pyplot as plt - plt.imshow(np.transpose(img.reshape(80, 80, 3), axes=[0, 1, 2])) - plt.show() + # with DeviceCollector(): + x = EdgeDetection(prefix="BL03I-DI-OAV-01:") + + img = asyncio.get_event_loop().run_until_complete(x.array_data.read()) + + print(img) From df955cad3fc7b2b496f32c6e1108eae5f2f6fe6c Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Wed, 9 Aug 2023 17:19:41 +0100 Subject: [PATCH 06/30] HACK (but it can get pva data) --- .../edge_detection/oav_with_edge_detection.py | 59 +++++++++++++++++-- 1 file changed, 53 insertions(+), 6 deletions(-) diff --git a/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py b/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py index c1127e3fcd..0d05537b4f 100644 --- a/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py +++ b/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py @@ -1,7 +1,7 @@ from dodal.devices.oav.oav_detector import OAV from dodal.devices.oav.edge_detection.edge_detect_utils import MxSampleDetect, ArrayProcessingFunctions, NONE_VALUE as INVALID_POSITION_VALUE, SampleLocation from dodal.log import LOGGER -from typing import TYPE_CHECKING, Callable, Final, Tuple, TypeVar, Optional +from typing import TYPE_CHECKING, Callable, Final, Tuple, TypeVar, Optional, Type, Dict, Any from ophyd.v2.core import AsyncStatus, StandardReadable, DeviceCollector from ophyd.v2.epics import epics_signal_r, SignalR, SignalRW @@ -11,6 +11,48 @@ import asyncio +from ophyd.v2 import _p4p +from ophyd.v2._p4p import make_converter as default_make_converter, get_unique, PvaConverter, PvaArrayConverter, get_dtype + + +class PvaNDArrayConverter(PvaConverter): + def write_value(self, value): + return value + + def value(self, value): + return value["value"] + + def reading(self, value): + return dict( + value=self.value(value), + timestamp=0, + alarm_severity=0, + ) + +# TODO: HACK! +def make_converter(datatype: Optional[Type], values: Dict[str, Any]) -> PvaConverter: + pv = list(values)[0] + typeid = get_unique({k: v.getID() for k, v in values.items()}, "typeids") + typ = get_unique({k: type(v["value"]) for k, v in values.items()}, "value types") + + if "NTNDArray" in typeid: + pv_dtype = get_unique( + {k: v["value"].dtype for k, v in values.items()}, "dtypes" + ) + # This is an array + if datatype: + # Check we wanted an array of this type + dtype = get_dtype(datatype) + if not dtype: + raise TypeError(f"{pv} has type [{pv_dtype}] not {datatype.__name__}") + if dtype != pv_dtype: + raise TypeError(f"{pv} has type [{pv_dtype}] not [{dtype}]") + return PvaNDArrayConverter() + + return default_make_converter(datatype, values) + +_p4p.make_converter = make_converter + T = TypeVar('T') @@ -36,10 +78,9 @@ class EdgeDetection(StandardReadable): INVALID_POSITION: Final[Tuple[int, int]] = (INVALID_POSITION_VALUE, INVALID_POSITION_VALUE) def __init__(self, prefix, name: str = ""): + self.array_data: SignalR[NDArray] = epics_signal_r(NDArray[np.uint8], "pva://{}PVA:ARRAY".format(prefix)) - self.array_data: SignalR[NDArray] = epics_signal_r(NDArray, "pva://{}ARR:ArrayData".format(prefix)) - - self.triggered_tip: SignalRW[Tuple[int | None, int | None]] = _SoftSignal(initial_value=EdgeDetection.INVALID_POSITION) + # self.triggered_tip: SignalRW[Tuple[int | None, int | None]] = _SoftSignal(initial_value=EdgeDetection.INVALID_POSITION) self.oav_width: int = 1024 self.oav_height: int = 768 @@ -59,7 +100,7 @@ def __init__(self, prefix, name: str = ""): self.array_data ], config=[ - self.triggered_tip + # self.triggered_tip ], ) @@ -112,6 +153,12 @@ async def read(self) -> Tuple[int | None, int | None]: # with DeviceCollector(): x = EdgeDetection(prefix="BL03I-DI-OAV-01:") - img = asyncio.get_event_loop().run_until_complete(x.array_data.read()) + async def doit(): + await x.connect() + data = await x.array_data.read() + print(data) + return data + + img = asyncio.get_event_loop().run_until_complete(doit()) print(img) From 0f47def784cc5de0e046fd2c985a016399900ea5 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 11 Aug 2023 10:23:46 +0100 Subject: [PATCH 07/30] Refactor to use Ophyd v2 --- pyproject.toml | 2 + .../edge_detection/oav_with_edge_detection.py | 102 ++++++------------ 2 files changed, 32 insertions(+), 72 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index a0bcce0583..7c6f0130c4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -23,6 +23,8 @@ dependencies = [ "graypy", "pydantic<2.0", "opencv-python-headless", + "aioca", + "p4p", ] dynamic = ["version"] license.file = "LICENSE" diff --git a/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py b/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py index 0d05537b4f..6bac748023 100644 --- a/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py +++ b/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py @@ -1,9 +1,8 @@ -from dodal.devices.oav.oav_detector import OAV -from dodal.devices.oav.edge_detection.edge_detect_utils import MxSampleDetect, ArrayProcessingFunctions, NONE_VALUE as INVALID_POSITION_VALUE, SampleLocation +from dodal.devices.oav.edge_detection.edge_detect_utils import MxSampleDetect, ArrayProcessingFunctions, NONE_VALUE as INVALID_POSITION_VALUE from dodal.log import LOGGER -from typing import TYPE_CHECKING, Callable, Final, Tuple, TypeVar, Optional, Type, Dict, Any +from typing import Callable, Final, Tuple, TypeVar, Optional -from ophyd.v2.core import AsyncStatus, StandardReadable, DeviceCollector +from ophyd.v2.core import AsyncStatus, StandardReadable, Reading from ophyd.v2.epics import epics_signal_r, SignalR, SignalRW import numpy as np @@ -11,48 +10,6 @@ import asyncio -from ophyd.v2 import _p4p -from ophyd.v2._p4p import make_converter as default_make_converter, get_unique, PvaConverter, PvaArrayConverter, get_dtype - - -class PvaNDArrayConverter(PvaConverter): - def write_value(self, value): - return value - - def value(self, value): - return value["value"] - - def reading(self, value): - return dict( - value=self.value(value), - timestamp=0, - alarm_severity=0, - ) - -# TODO: HACK! -def make_converter(datatype: Optional[Type], values: Dict[str, Any]) -> PvaConverter: - pv = list(values)[0] - typeid = get_unique({k: v.getID() for k, v in values.items()}, "typeids") - typ = get_unique({k: type(v["value"]) for k, v in values.items()}, "value types") - - if "NTNDArray" in typeid: - pv_dtype = get_unique( - {k: v["value"].dtype for k, v in values.items()}, "dtypes" - ) - # This is an array - if datatype: - # Check we wanted an array of this type - dtype = get_dtype(datatype) - if not dtype: - raise TypeError(f"{pv} has type [{pv_dtype}] not {datatype.__name__}") - if dtype != pv_dtype: - raise TypeError(f"{pv} has type [{pv_dtype}] not [{dtype}]") - return PvaNDArrayConverter() - - return default_make_converter(datatype, values) - -_p4p.make_converter = make_converter - T = TypeVar('T') @@ -78,12 +35,10 @@ class EdgeDetection(StandardReadable): INVALID_POSITION: Final[Tuple[int, int]] = (INVALID_POSITION_VALUE, INVALID_POSITION_VALUE) def __init__(self, prefix, name: str = ""): - self.array_data: SignalR[NDArray] = epics_signal_r(NDArray[np.uint8], "pva://{}PVA:ARRAY".format(prefix)) - - # self.triggered_tip: SignalRW[Tuple[int | None, int | None]] = _SoftSignal(initial_value=EdgeDetection.INVALID_POSITION) + self.array_data: SignalR[NDArray[np.uint8]] = epics_signal_r(NDArray[np.uint8], "pva://{}PVA:ARRAY".format(prefix)) - self.oav_width: int = 1024 - self.oav_height: int = 768 + self.oav_width: SignalR[int] = epics_signal_r(int, "{}PVA:ArraySize1_RBV".format(prefix)) + self.oav_height: SignalR[int] = epics_signal_r(int, "{}PVA:ArraySize2_RBV".format(prefix)) self.timeout: float = 10.0 @@ -97,16 +52,17 @@ def __init__(self, prefix, name: str = ""): self.set_readable_signals( read=[ - self.array_data + self.array_data, + self.oav_width, + self.oav_height, ], config=[ - # self.triggered_tip ], ) super().__init__(name=name) - def update_tip_position(self, *, value, **_): + async def _get_tip_position(self) -> Tuple[Optional[int], Optional[int]]: sample_detection = MxSampleDetect( preprocess=self.preprocess, @@ -119,14 +75,18 @@ def update_tip_position(self, *, value, **_): ) try: - num_pixels = self.oav_height * self.oav_width - value_len = value.shape[0] + array_data: NDArray[np.uint8] = await self.array_data.get_value() + height: int = await self.oav_height.get_value() + width: int = await self.oav_width.get_value() + + num_pixels: int = height * width # type: ignore + value_len = array_data.shape[0] if value_len == num_pixels * 3: # RGB data - value = value.reshape(self.oav_height, self.oav_width, 3) + value = array_data.reshape(height, width, 3) elif value_len == num_pixels: # Grayscale data - value = value.reshape(self.oav_height, self.oav_width) + value = array_data.reshape(height, width) else: # Something else? raise ValueError("Unexpected data array size: expected {} (grayscale data) or {} (rgb data), got {}", num_pixels, num_pixels * 3, value_len) @@ -139,26 +99,24 @@ def update_tip_position(self, *, value, **_): tip_x = None tip_y = None - self.triggered_tip.set((tip_x, tip_y)) + return (tip_x, tip_y) async def read(self) -> Tuple[int | None, int | None]: - # Clear last value - self.triggered_tip.set(EdgeDetection.INVALID_POSITION) - - self.update_tip_position(value=self.array_data.get_value()) - return await self.triggered_tip.get_value() + return await self._get_tip_position() if __name__ == "__main__": # with DeviceCollector(): x = EdgeDetection(prefix="BL03I-DI-OAV-01:") - async def doit(): + async def acquire(): await x.connect() - data = await x.array_data.read() - print(data) - return data - - img = asyncio.get_event_loop().run_until_complete(doit()) - - print(img) + img = await x.array_data.read() + tip = await x.read() + return img, tip + + img, tip = asyncio.get_event_loop().run_until_complete(acquire()) + print("Tip: {}".format(tip)) + import matplotlib.pyplot as plt + plt.imshow(img[""]["value"].reshape(768, 1024, 3)) + plt.show() From 9b5fed24766d5147eab3e7b0df582b359a08e4e0 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 11 Aug 2023 10:52:54 +0100 Subject: [PATCH 08/30] Tidy ups --- src/dodal/devices/oav/edge_detection/edge_detect_utils.py | 4 ++++ .../devices/oav/edge_detection/oav_with_edge_detection.py | 6 +++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/dodal/devices/oav/edge_detection/edge_detect_utils.py b/src/dodal/devices/oav/edge_detection/edge_detect_utils.py index dc92baaf44..c9e8a4d663 100644 --- a/src/dodal/devices/oav/edge_detection/edge_detect_utils.py +++ b/src/dodal/devices/oav/edge_detection/edge_detect_utils.py @@ -3,6 +3,7 @@ import cv2 import numpy as np +from dodal.log import LOGGER class ArrayProcessingFunctions: @@ -205,6 +206,7 @@ def _locate_sample(self, edge_arr: np.ndarray) -> SampleLocation: if column_indices_with_non_narrow_widths.shape[0] == 0: # No non-narrow locations - sample not in picture? # Or wrong parameters for edge-finding, ... + LOGGER.warn("pin-tip detection: No non-narrow edges found - cannot locate pin tip") return SampleLocation( tip_y=None, tip_x=None, edge_bottom=bottom, edge_top=top ) @@ -222,6 +224,7 @@ def _locate_sample(self, edge_arr: np.ndarray) -> SampleLocation: x += -self.scan_direction if x == -1 or x == width: # (In this case the sample is off the edge of the picture.) + LOGGER.warn("pin-tip detection: Pin tip may be outside image area - assuming at edge.") break x += self.scan_direction # ...and forward one step. x is now at the tip. @@ -236,6 +239,7 @@ def _locate_sample(self, edge_arr: np.ndarray) -> SampleLocation: top[x + 1 :] = NONE_VALUE bottom[x + 1 :] = NONE_VALUE + LOGGER.debug("Successfully located pin tip at (x={}, y={})".format(tip_x, tip_y)) return SampleLocation( tip_y=tip_y, tip_x=tip_x, edge_bottom=bottom, edge_top=top ) diff --git a/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py b/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py index 6bac748023..d9dc9c2585 100644 --- a/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py +++ b/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py @@ -95,14 +95,14 @@ async def _get_tip_position(self) -> Tuple[Optional[int], Optional[int]]: tip_x = location.tip_x tip_y = location.tip_y except Exception as e: - LOGGER.error("Failed to detect sample position: ", e) + LOGGER.error("Failed to detect pin-tip location due to exception: ", e) tip_x = None tip_y = None return (tip_x, tip_y) async def read(self) -> Tuple[int | None, int | None]: - return await self._get_tip_position() + return await asyncio.wait_for(self._get_tip_position(), timeout=self.timeout) if __name__ == "__main__": @@ -115,7 +115,7 @@ async def acquire(): tip = await x.read() return img, tip - img, tip = asyncio.get_event_loop().run_until_complete(acquire()) + img, tip = asyncio.get_event_loop().run_until_complete(asyncio.wait_for(acquire(), timeout=10)) print("Tip: {}".format(tip)) import matplotlib.pyplot as plt plt.imshow(img[""]["value"].reshape(768, 1024, 3)) From 3d04cdeb55ec41d3343d5fe95e803e379cf7e708 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 11 Aug 2023 15:34:58 +0100 Subject: [PATCH 09/30] refactoring --- .../oav/edge_detection/edge_detect_utils.py | 33 ++++--- .../edge_detection/oav_with_edge_detection.py | 94 +++++++++++-------- 2 files changed, 75 insertions(+), 52 deletions(-) diff --git a/src/dodal/devices/oav/edge_detection/edge_detect_utils.py b/src/dodal/devices/oav/edge_detection/edge_detect_utils.py index c9e8a4d663..8791779a7c 100644 --- a/src/dodal/devices/oav/edge_detection/edge_detect_utils.py +++ b/src/dodal/devices/oav/edge_detection/edge_detect_utils.py @@ -3,6 +3,7 @@ import cv2 import numpy as np + from dodal.log import LOGGER @@ -11,6 +12,7 @@ class ArrayProcessingFunctions: Utility class for creating array preprocessing functions (arr -> arr with no additional parameters) for some common operations. """ + @staticmethod def identity() -> Callable[[np.ndarray], np.ndarray]: return lambda arr: arr @@ -82,7 +84,7 @@ def median_blur(ksize: int) -> Callable[[np.ndarray], np.ndarray]: return lambda arr: cv2.medianBlur(arr, ksize) -# A substitute for "None" which can fit into an np.int32 array. +# A substitute for "None" which can fit into an numpy int array. # Also used as a substitute for a not-found sample position. NONE_VALUE: Final[int] = -1 @@ -92,6 +94,7 @@ class SampleLocation: """ Holder type for results from sample detection. """ + tip_y: Optional[int] tip_x: Optional[int] edge_top: np.ndarray @@ -182,11 +185,14 @@ def _first_and_last_nonzero_by_columns( first_nonzero will be: [1, 1, NONE_VALUE, 0] last_nonzero will be [1, 2, NONE_VALUE, 2] """ - mask = arr != 0 - first_nonzero = np.where(mask.any(axis=0), mask.argmax(axis=0), NONE_VALUE) + nonzero = arr.astype(dtype=bool, copy=False) + any_nonzero_in_column = nonzero.any(axis=0) + + first_nonzero = np.where(any_nonzero_in_column, nonzero.argmax(axis=0), NONE_VALUE) + + flipped = nonzero.shape[0] - np.flip(nonzero, axis=0).argmax(axis=0) - 1 + last_nonzero = np.where(any_nonzero_in_column, flipped, NONE_VALUE) - flipped = arr.shape[0] - np.flip(mask, axis=0).argmax(axis=0) - 1 - last_nonzero = np.where(mask.any(axis=0), flipped, NONE_VALUE) return first_nonzero, last_nonzero def _locate_sample(self, edge_arr: np.ndarray) -> SampleLocation: @@ -199,14 +205,15 @@ def _locate_sample(self, edge_arr: np.ndarray) -> SampleLocation: widths = np.where(top != NONE_VALUE, bottom - top + 1, 0) # Generate the indices of columns with widths larger than the specified min tip height. - column_indices_with_non_narrow_widths = np.nonzero( - widths >= self.min_tip_height - )[0] + non_narrow_widths = widths >= self.min_tip_height + column_indices_with_non_narrow_widths = np.flatnonzero(non_narrow_widths) if column_indices_with_non_narrow_widths.shape[0] == 0: # No non-narrow locations - sample not in picture? # Or wrong parameters for edge-finding, ... - LOGGER.warn("pin-tip detection: No non-narrow edges found - cannot locate pin tip") + LOGGER.warning( + "pin-tip detection: No non-narrow edges found - cannot locate pin tip" + ) return SampleLocation( tip_y=None, tip_x=None, edge_bottom=bottom, edge_top=top ) @@ -224,7 +231,9 @@ def _locate_sample(self, edge_arr: np.ndarray) -> SampleLocation: x += -self.scan_direction if x == -1 or x == width: # (In this case the sample is off the edge of the picture.) - LOGGER.warn("pin-tip detection: Pin tip may be outside image area - assuming at edge.") + LOGGER.warning( + "pin-tip detection: Pin tip may be outside image area - assuming at edge." + ) break x += self.scan_direction # ...and forward one step. x is now at the tip. @@ -239,7 +248,9 @@ def _locate_sample(self, edge_arr: np.ndarray) -> SampleLocation: top[x + 1 :] = NONE_VALUE bottom[x + 1 :] = NONE_VALUE - LOGGER.debug("Successfully located pin tip at (x={}, y={})".format(tip_x, tip_y)) + LOGGER.info( + "pin-tip detection: Successfully located pin tip at (x={}, y={})".format(tip_x, tip_y) + ) return SampleLocation( tip_y=tip_y, tip_x=tip_x, edge_bottom=bottom, edge_top=top ) diff --git a/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py b/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py index d9dc9c2585..6beb52bba7 100644 --- a/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py +++ b/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py @@ -1,48 +1,47 @@ -from dodal.devices.oav.edge_detection.edge_detect_utils import MxSampleDetect, ArrayProcessingFunctions, NONE_VALUE as INVALID_POSITION_VALUE -from dodal.log import LOGGER -from typing import Callable, Final, Tuple, TypeVar, Optional - -from ophyd.v2.core import AsyncStatus, StandardReadable, Reading -from ophyd.v2.epics import epics_signal_r, SignalR, SignalRW +import asyncio +import time +from typing import Callable, Final, Optional, Tuple, TypeVar import numpy as np from numpy.typing import NDArray +from ophyd.v2.core import AsyncStatus, StandardReadable +from ophyd.v2.epics import SignalR, SignalRW, epics_signal_r + +from dodal.devices.oav.edge_detection.edge_detect_utils import ( + NONE_VALUE as INVALID_POSITION_VALUE, +) +from dodal.devices.oav.edge_detection.edge_detect_utils import ( + ArrayProcessingFunctions, + MxSampleDetect, +) +from dodal.log import LOGGER -import asyncio - - -T = TypeVar('T') - -class _SoftSignal(SignalRW[T]): - def __init__(self, initial_value: T): - self.value: T = initial_value - - async def _set(self, value: T): - self.value = value - - def set(self, value: T, wait=True, timeout=None) -> AsyncStatus: - coro = self._set(value) - return AsyncStatus(coro) - - def read(self) -> T: - if self.value is None: - raise ValueError("Soft signal read before being set") - return self.value +T = TypeVar("T") class EdgeDetection(StandardReadable): - - INVALID_POSITION: Final[Tuple[int, int]] = (INVALID_POSITION_VALUE, INVALID_POSITION_VALUE) + INVALID_POSITION: Final[Tuple[int, int]] = ( + INVALID_POSITION_VALUE, + INVALID_POSITION_VALUE, + ) def __init__(self, prefix, name: str = ""): - self.array_data: SignalR[NDArray[np.uint8]] = epics_signal_r(NDArray[np.uint8], "pva://{}PVA:ARRAY".format(prefix)) + self.array_data: SignalR[NDArray[np.uint8]] = epics_signal_r( + NDArray[np.uint8], "pva://{}PVA:ARRAY".format(prefix) + ) - self.oav_width: SignalR[int] = epics_signal_r(int, "{}PVA:ArraySize1_RBV".format(prefix)) - self.oav_height: SignalR[int] = epics_signal_r(int, "{}PVA:ArraySize2_RBV".format(prefix)) + self.oav_width: SignalR[int] = epics_signal_r( + int, "{}PVA:ArraySize1_RBV".format(prefix) + ) + self.oav_height: SignalR[int] = epics_signal_r( + int, "{}PVA:ArraySize2_RBV".format(prefix) + ) self.timeout: float = 10.0 - self.preprocess: Callable[[np.ndarray], np.ndarray] = ArrayProcessingFunctions.identity() + self.preprocess: Callable[ + [np.ndarray], np.ndarray + ] = ArrayProcessingFunctions.identity() self.canny_upper: int = 100 self.canny_lower: int = 50 self.close_ksize: int = 5 @@ -56,14 +55,12 @@ def __init__(self, prefix, name: str = ""): self.oav_width, self.oav_height, ], - config=[ - ], + config=[], ) super().__init__(name=name) async def _get_tip_position(self) -> Tuple[Optional[int], Optional[int]]: - sample_detection = MxSampleDetect( preprocess=self.preprocess, canny_lower=self.canny_lower, @@ -89,9 +86,21 @@ async def _get_tip_position(self) -> Tuple[Optional[int], Optional[int]]: value = array_data.reshape(height, width) else: # Something else? - raise ValueError("Unexpected data array size: expected {} (grayscale data) or {} (rgb data), got {}", num_pixels, num_pixels * 3, value_len) - + raise ValueError( + "Unexpected data array size: expected {} (grayscale data) or {} (rgb data), got {}", + num_pixels, + num_pixels * 3, + value_len, + ) + + start_time = time.time() location = sample_detection.processArray(value) + end_time = time.time() + LOGGER.warning( + "Sample location detection took {}ms".format( + (end_time - start_time) * 1000.0 + ) + ) tip_x = location.tip_x tip_y = location.tip_y except Exception as e: @@ -104,10 +113,10 @@ async def _get_tip_position(self) -> Tuple[Optional[int], Optional[int]]: async def read(self) -> Tuple[int | None, int | None]: return await asyncio.wait_for(self._get_tip_position(), timeout=self.timeout) - + if __name__ == "__main__": # with DeviceCollector(): - x = EdgeDetection(prefix="BL03I-DI-OAV-01:") + x = EdgeDetection(prefix="BL02J-DI-OAV-01:") async def acquire(): await x.connect() @@ -115,8 +124,11 @@ async def acquire(): tip = await x.read() return img, tip - img, tip = asyncio.get_event_loop().run_until_complete(asyncio.wait_for(acquire(), timeout=10)) + img, tip = asyncio.get_event_loop().run_until_complete( + asyncio.wait_for(acquire(), timeout=10) + ) print("Tip: {}".format(tip)) import matplotlib.pyplot as plt - plt.imshow(img[""]["value"].reshape(768, 1024, 3)) + + plt.imshow(img[""]["value"].reshape(2176, 2112, 3)) plt.show() From 8fd023cb32d8cc119196224aea8598c8fb72d8c7 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Mon, 14 Aug 2023 10:15:02 +0100 Subject: [PATCH 10/30] Minor refactor --- pyproject.toml | 8 +- .../oav/edge_detection/edge_detect_utils.py | 8 +- .../edge_detection/oav_with_edge_detection.py | 92 ++++++++++++------- 3 files changed, 70 insertions(+), 38 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 7c6f0130c4..634a503571 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -14,7 +14,7 @@ classifiers = [ ] description = "Ophyd devices and other utils that could be used across DLS beamlines" dependencies = [ - "ophyd", + "ophyd@git+https://github.com/Tom-Willemsen/ophyd#ntndarray", # Switch back to just "ophyd" once merged. "bluesky", "pyepics", "dataclasses-json", @@ -22,9 +22,9 @@ dependencies = [ "requests", "graypy", "pydantic<2.0", - "opencv-python-headless", - "aioca", - "p4p", + "opencv-python-headless", # For pin-tip detection. + "aioca", # Required for CA support with Ophyd V2. + "p4p", # Required for PVA support with Ophyd V2. ] dynamic = ["version"] license.file = "LICENSE" diff --git a/src/dodal/devices/oav/edge_detection/edge_detect_utils.py b/src/dodal/devices/oav/edge_detection/edge_detect_utils.py index 8791779a7c..9b46b35ccd 100644 --- a/src/dodal/devices/oav/edge_detection/edge_detect_utils.py +++ b/src/dodal/devices/oav/edge_detection/edge_detect_utils.py @@ -188,7 +188,9 @@ def _first_and_last_nonzero_by_columns( nonzero = arr.astype(dtype=bool, copy=False) any_nonzero_in_column = nonzero.any(axis=0) - first_nonzero = np.where(any_nonzero_in_column, nonzero.argmax(axis=0), NONE_VALUE) + first_nonzero = np.where( + any_nonzero_in_column, nonzero.argmax(axis=0), NONE_VALUE + ) flipped = nonzero.shape[0] - np.flip(nonzero, axis=0).argmax(axis=0) - 1 last_nonzero = np.where(any_nonzero_in_column, flipped, NONE_VALUE) @@ -249,7 +251,9 @@ def _locate_sample(self, edge_arr: np.ndarray) -> SampleLocation: bottom[x + 1 :] = NONE_VALUE LOGGER.info( - "pin-tip detection: Successfully located pin tip at (x={}, y={})".format(tip_x, tip_y) + "pin-tip detection: Successfully located pin tip at (x={}, y={})".format( + tip_x, tip_y + ) ) return SampleLocation( tip_y=tip_y, tip_x=tip_x, edge_bottom=bottom, edge_top=top diff --git a/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py b/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py index 6beb52bba7..0837a30b74 100644 --- a/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py +++ b/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py @@ -1,11 +1,12 @@ import asyncio import time -from typing import Callable, Final, Optional, Tuple, TypeVar +from typing import Callable, Dict, Final, Optional, Tuple, TypeVar +from bluesky.protocols import Descriptor, SyncOrAsync import numpy as np from numpy.typing import NDArray -from ophyd.v2.core import AsyncStatus, StandardReadable -from ophyd.v2.epics import SignalR, SignalRW, epics_signal_r +from ophyd.v2.core import Readable, Reading +from ophyd.v2.epics import SignalR, epics_signal_r from dodal.devices.oav.edge_detection.edge_detect_utils import ( NONE_VALUE as INVALID_POSITION_VALUE, @@ -15,17 +16,18 @@ MxSampleDetect, ) from dodal.log import LOGGER +from collections import OrderedDict T = TypeVar("T") -class EdgeDetection(StandardReadable): - INVALID_POSITION: Final[Tuple[int, int]] = ( - INVALID_POSITION_VALUE, - INVALID_POSITION_VALUE, - ) +class EdgeDetection(Readable): + + def __init__(self, prefix: str, name: str = ""): + + self._name: str = name + self._prefix: str = prefix - def __init__(self, prefix, name: str = ""): self.array_data: SignalR[NDArray[np.uint8]] = epics_signal_r( NDArray[np.uint8], "pva://{}PVA:ARRAY".format(prefix) ) @@ -49,18 +51,20 @@ def __init__(self, prefix, name: str = ""): self.scan_direction: int = 1 self.min_tip_height: int = 5 - self.set_readable_signals( - read=[ - self.array_data, - self.oav_width, - self.oav_height, - ], - config=[], - ) + async def connect(self): + for signal in [self.array_data, self.oav_width, self.oav_height]: + await signal.connect() + + def name(self) -> str: + return self._name - super().__init__(name=name) + async def _get_tip_position(self) -> Tuple[Tuple[Optional[int], Optional[int]], float]: + """ + Gets the location of the pin tip. - async def _get_tip_position(self) -> Tuple[Optional[int], Optional[int]]: + Returns tuple of: + ((tip_x, tip_y), timestamp) + """ sample_detection = MxSampleDetect( preprocess=self.preprocess, canny_lower=self.canny_lower, @@ -72,7 +76,10 @@ async def _get_tip_position(self) -> Tuple[Optional[int], Optional[int]]: ) try: - array_data: NDArray[np.uint8] = await self.array_data.get_value() + array_reading: dict[str, Reading] = await self.array_data.read() + array_data: NDArray[np.uint8] = array_reading[""]["value"] + timestamp: float = array_reading[""]["timestamp"] + height: int = await self.oav_height.get_value() width: int = await self.oav_width.get_value() @@ -96,7 +103,7 @@ async def _get_tip_position(self) -> Tuple[Optional[int], Optional[int]]: start_time = time.time() location = sample_detection.processArray(value) end_time = time.time() - LOGGER.warning( + LOGGER.debug( "Sample location detection took {}ms".format( (end_time - start_time) * 1000.0 ) @@ -107,16 +114,33 @@ async def _get_tip_position(self) -> Tuple[Optional[int], Optional[int]]: LOGGER.error("Failed to detect pin-tip location due to exception: ", e) tip_x = None tip_y = None - - return (tip_x, tip_y) - - async def read(self) -> Tuple[int | None, int | None]: - return await asyncio.wait_for(self._get_tip_position(), timeout=self.timeout) + timestamp = time.time() # Fallback only. + + return (tip_x, tip_y), timestamp + + async def read(self) -> dict[str, Reading]: + tip_pos, timestamp = await asyncio.wait_for(self._get_tip_position(), timeout=self.timeout) + + return OrderedDict([ + (self._name, { + "value": tip_pos, + "timestamp": timestamp + }), + ]) + + async def describe(self) -> dict[str, Descriptor]: + return OrderedDict( + [(self._name, + { + 'source': "pva://{}PVA:ARRAY".format(self._prefix), + 'dtype': "number", + 'shape': [2], # Tuple of (x, y) tip position + })], + ) if __name__ == "__main__": - # with DeviceCollector(): - x = EdgeDetection(prefix="BL02J-DI-OAV-01:") + x = EdgeDetection(prefix="BL02J-DI-OAV-01:", name="edgeDetect") async def acquire(): await x.connect() @@ -127,8 +151,12 @@ async def acquire(): img, tip = asyncio.get_event_loop().run_until_complete( asyncio.wait_for(acquire(), timeout=10) ) - print("Tip: {}".format(tip)) - import matplotlib.pyplot as plt + print("Tip: {}".format(tip["edgeDetect"]["value"])) + + try: + import matplotlib.pyplot as plt - plt.imshow(img[""]["value"].reshape(2176, 2112, 3)) - plt.show() + plt.imshow(img[""]["value"].reshape(2176, 2112, 3)) + plt.show() + except ImportError as e: + print("matplotlib not available; cannot show acquired image") From 58493d666b9ce37968783eca317be8802051763c Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Mon, 14 Aug 2023 10:23:46 +0100 Subject: [PATCH 11/30] Placate linters --- .../edge_detection/oav_with_edge_detection.py | 52 ++++++++++--------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py b/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py index 0837a30b74..2bb442f1da 100644 --- a/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py +++ b/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py @@ -1,30 +1,25 @@ import asyncio import time -from typing import Callable, Dict, Final, Optional, Tuple, TypeVar -from bluesky.protocols import Descriptor, SyncOrAsync +from collections import OrderedDict +from typing import Callable, Optional, Tuple, TypeVar import numpy as np +from bluesky.protocols import Descriptor from numpy.typing import NDArray from ophyd.v2.core import Readable, Reading from ophyd.v2.epics import SignalR, epics_signal_r -from dodal.devices.oav.edge_detection.edge_detect_utils import ( - NONE_VALUE as INVALID_POSITION_VALUE, -) from dodal.devices.oav.edge_detection.edge_detect_utils import ( ArrayProcessingFunctions, MxSampleDetect, ) from dodal.log import LOGGER -from collections import OrderedDict T = TypeVar("T") class EdgeDetection(Readable): - def __init__(self, prefix: str, name: str = ""): - self._name: str = name self._prefix: str = prefix @@ -58,7 +53,9 @@ async def connect(self): def name(self) -> str: return self._name - async def _get_tip_position(self) -> Tuple[Tuple[Optional[int], Optional[int]], float]: + async def _get_tip_position( + self, + ) -> Tuple[Tuple[Optional[int], Optional[int]], float]: """ Gets the location of the pin tip. @@ -119,23 +116,28 @@ async def _get_tip_position(self) -> Tuple[Tuple[Optional[int], Optional[int]], return (tip_x, tip_y), timestamp async def read(self) -> dict[str, Reading]: - tip_pos, timestamp = await asyncio.wait_for(self._get_tip_position(), timeout=self.timeout) - - return OrderedDict([ - (self._name, { - "value": tip_pos, - "timestamp": timestamp - }), - ]) - + tip_pos, timestamp = await asyncio.wait_for( + self._get_tip_position(), timeout=self.timeout + ) + + return OrderedDict( + [ + (self._name, {"value": tip_pos, "timestamp": timestamp}), + ] + ) + async def describe(self) -> dict[str, Descriptor]: return OrderedDict( - [(self._name, - { - 'source': "pva://{}PVA:ARRAY".format(self._prefix), - 'dtype': "number", - 'shape': [2], # Tuple of (x, y) tip position - })], + [ + ( + self._name, + { + "source": "pva://{}PVA:ARRAY".format(self._prefix), + "dtype": "number", + "shape": [2], # Tuple of (x, y) tip position + }, + ) + ], ) @@ -158,5 +160,5 @@ async def acquire(): plt.imshow(img[""]["value"].reshape(2176, 2112, 3)) plt.show() - except ImportError as e: + except ImportError: print("matplotlib not available; cannot show acquired image") From aa39ed653975c1948e9bddfe029efc5a4a40d850 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Tue, 15 Aug 2023 13:22:01 +0100 Subject: [PATCH 12/30] Initial test with v1/v2 ophyd devices --- src/dodal/beamlines/beamline_utils.py | 41 ++++++++++++---- src/dodal/beamlines/i23.py | 13 +++++ .../edge_detection/oav_with_edge_detection.py | 46 ++++++++---------- src/dodal/utils.py | 48 ++++++++++++++----- tests/system_tests/test_i23_system.py | 11 ++++- 5 files changed, 112 insertions(+), 47 deletions(-) diff --git a/src/dodal/beamlines/beamline_utils.py b/src/dodal/beamlines/beamline_utils.py index 5eebee3858..6898cd0ee0 100644 --- a/src/dodal/beamlines/beamline_utils.py +++ b/src/dodal/beamlines/beamline_utils.py @@ -1,11 +1,16 @@ -from typing import Callable, Dict, List, Optional +from typing import Callable, Dict, List, Optional, TypeVar, cast from ophyd import Device +from ophyd.v2.core import Device as OphydV2Device, wait_for_connection as v2_device_wait_for_connection from ophyd.sim import make_fake_device from dodal.utils import BeamlinePrefix, skip_device -ACTIVE_DEVICES: Dict[str, Device] = {} +from bluesky.run_engine import call_in_bluesky_event_loop +import asyncio + + +ACTIVE_DEVICES: Dict[str, Device | OphydV2Device] = {} BL = "" @@ -34,17 +39,30 @@ def active_device_is_same_type(active_device, device): return type(active_device) == device +T = TypeVar("T", bound=Device | OphydV2Device) + + +def _wait_for_connection(device: Device | OphydV2Device) -> None: + match device: + case Device(): + device.wait_for_connection() + case OphydV2Device(): + call_in_bluesky_event_loop(asyncio.wait_for(v2_device_wait_for_connection(coros=device.connect()), timeout=30)) + case _: + raise TypeError("Invalid type {} in _wait_for_connection".format(device.__class__.__name__)) + + @skip_device() def device_instantiation( - device: Callable, + device: Callable[..., T], name: str, prefix: str, wait: bool, fake: bool, - post_create: Optional[Callable] = None, + post_create: Optional[Callable[[T], None]] = None, bl_prefix: bool = True, **kwargs, -) -> Device: +) -> T: """Method to allow generic creation of singleton devices. Meant to be used to easily define lists of devices in beamline files. Additional keyword arguments are passed directly to the device constructor. @@ -66,15 +84,17 @@ def device_instantiation( if fake: device = make_fake_device(device) if active_device is None: - ACTIVE_DEVICES[name] = device( + device_instance: T = device( name=name, prefix=f"{(BeamlinePrefix(BL).beamline_prefix)}{prefix}" if bl_prefix else prefix, **kwargs, ) + ACTIVE_DEVICES[name] = device_instance if wait: - ACTIVE_DEVICES[name].wait_for_connection() + _wait_for_connection(device_instance) + else: if not active_device_is_same_type(active_device, device): raise TypeError( @@ -82,6 +102,9 @@ def device_instantiation( f"name as an existing device. Device name '{name}' already used for " f"a(n) {device}." ) + else: + # We have manually checked types + device_instance: T = cast(T, ACTIVE_DEVICES[name]) if post_create: - post_create(ACTIVE_DEVICES[name]) - return ACTIVE_DEVICES[name] + post_create(device_instance) + return device_instance diff --git a/src/dodal/beamlines/i23.py b/src/dodal/beamlines/i23.py index c65b11043b..13a35c518c 100644 --- a/src/dodal/beamlines/i23.py +++ b/src/dodal/beamlines/i23.py @@ -1,6 +1,7 @@ from dodal.beamlines.beamline_utils import device_instantiation from dodal.beamlines.beamline_utils import set_beamline as set_utils_beamline from dodal.devices.i23.gonio import Gonio +from dodal.devices.oav.edge_detection.oav_with_edge_detection import EdgeDetection from dodal.log import set_beamline as set_log_beamline from dodal.utils import get_beamline_name @@ -18,3 +19,15 @@ def gonio(wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False) - wait_for_connection, fake_with_ophyd_sim, ) + + +def oav_pin_tip_detection(wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False) -> EdgeDetection: + """Get the i23 OAV pin-tip detection device""" + return device_instantiation( + EdgeDetection, + "PinTipDetection", + "BL03I-DI-OAV-01:", # TODO: FIXME + wait_for_connection, + fake_with_ophyd_sim, + bl_prefix=False, + ) diff --git a/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py b/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py index 2bb442f1da..35e5893fac 100644 --- a/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py +++ b/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py @@ -1,12 +1,12 @@ import asyncio import time from collections import OrderedDict -from typing import Callable, Optional, Tuple, TypeVar +from typing import Callable, Optional, Tuple, TypeVar, Type import numpy as np from bluesky.protocols import Descriptor from numpy.typing import NDArray -from ophyd.v2.core import Readable, Reading +from ophyd.v2.core import Readable, Reading, Device from ophyd.v2.epics import SignalR, epics_signal_r from dodal.devices.oav.edge_detection.edge_detect_utils import ( @@ -18,20 +18,20 @@ T = TypeVar("T") -class EdgeDetection(Readable): +class EdgeDetection(Readable, Device): def __init__(self, prefix: str, name: str = ""): - self._name: str = name self._prefix: str = prefix + self._name = name self.array_data: SignalR[NDArray[np.uint8]] = epics_signal_r( - NDArray[np.uint8], "pva://{}PVA:ARRAY".format(prefix) + NDArray[np.uint8], f"pva://{prefix}PVA:ARRAY" ) self.oav_width: SignalR[int] = epics_signal_r( - int, "{}PVA:ArraySize1_RBV".format(prefix) + int, f"{prefix}PVA:ArraySize1_RBV" ) self.oav_height: SignalR[int] = epics_signal_r( - int, "{}PVA:ArraySize2_RBV".format(prefix) + int, f"{prefix}PVA:ArraySize2_RBV" ) self.timeout: float = 10.0 @@ -46,12 +46,7 @@ def __init__(self, prefix: str, name: str = ""): self.scan_direction: int = 1 self.min_tip_height: int = 5 - async def connect(self): - for signal in [self.array_data, self.oav_width, self.oav_height]: - await signal.connect() - - def name(self) -> str: - return self._name + super().__init__(name=name) async def _get_tip_position( self, @@ -72,16 +67,17 @@ async def _get_tip_position( min_tip_height=self.min_tip_height, ) - try: - array_reading: dict[str, Reading] = await self.array_data.read() - array_data: NDArray[np.uint8] = array_reading[""]["value"] - timestamp: float = array_reading[""]["timestamp"] + array_reading: dict[str, Reading] = await self.array_data.read() + array_data: NDArray[np.uint8] = array_reading[""]["value"] + timestamp: float = array_reading[""]["timestamp"] + + height: int = await self.oav_height.get_value() + width: int = await self.oav_width.get_value() - height: int = await self.oav_height.get_value() - width: int = await self.oav_width.get_value() + num_pixels: int = height * width # type: ignore + value_len = array_data.shape[0] - num_pixels: int = height * width # type: ignore - value_len = array_data.shape[0] + try: if value_len == num_pixels * 3: # RGB data value = array_data.reshape(height, width, 3) @@ -96,7 +92,7 @@ async def _get_tip_position( num_pixels * 3, value_len, ) - + start_time = time.time() location = sample_detection.processArray(value) end_time = time.time() @@ -111,7 +107,6 @@ async def _get_tip_position( LOGGER.error("Failed to detect pin-tip location due to exception: ", e) tip_x = None tip_y = None - timestamp = time.time() # Fallback only. return (tip_x, tip_y), timestamp @@ -142,7 +137,7 @@ async def describe(self) -> dict[str, Descriptor]: if __name__ == "__main__": - x = EdgeDetection(prefix="BL02J-DI-OAV-01:", name="edgeDetect") + x = EdgeDetection(prefix="BL03I-DI-OAV-01:", name="edgeDetect") async def acquire(): await x.connect() @@ -153,12 +148,13 @@ async def acquire(): img, tip = asyncio.get_event_loop().run_until_complete( asyncio.wait_for(acquire(), timeout=10) ) + print(tip) print("Tip: {}".format(tip["edgeDetect"]["value"])) try: import matplotlib.pyplot as plt - plt.imshow(img[""]["value"].reshape(2176, 2112, 3)) + plt.imshow(img[""]["value"].reshape(768, 1024, 3)) plt.show() except ImportError: print("matplotlib not available; cannot show acquired image") diff --git a/src/dodal/utils.py b/src/dodal/utils.py index 075f9bfc95..439b62612b 100644 --- a/src/dodal/utils.py +++ b/src/dodal/utils.py @@ -11,6 +11,7 @@ Callable, Dict, Iterable, + Final, Mapping, Optional, Type, @@ -35,6 +36,8 @@ WritesExternalAssets, ) +from ophyd.v2.core import Device as OphydV2Device + #: Protocols defining interface to hardware BLUESKY_PROTOCOLS = [ Checkable, @@ -106,8 +109,14 @@ def make_all_devices( """ if isinstance(module, str) or module is None: module = import_module(module or __name__) - factories = collect_factories(module) - return invoke_factories(factories, **kwargs) + v1_factories, v2_factories = collect_factories(module) + print(f"v1: {v1_factories}, v2: {v2_factories}") + devices = invoke_factories(v1_factories, **kwargs) + + for name, factory in v2_factories.items(): + devices[name] = factory(**kwargs) + + return devices def invoke_factories( @@ -143,12 +152,17 @@ def extract_dependencies( yield name -def collect_factories(module: ModuleType) -> Mapping[str, Callable[..., Any]]: - factories = {} +def collect_factories(module: ModuleType) -> tuple[Mapping[str, Callable[..., Any]], Mapping[str, Callable[..., OphydV2Device]]]: + v1_factories = {} + v2_factories = {} for var in module.__dict__.values(): - if callable(var) and _is_device_factory(var) and not _is_device_skipped(var): - factories[var.__name__] = var - return factories + if callable(var) and not _is_device_skipped(var): + if _is_v1_device_factory(var): + v1_factories[var.__name__] = var + elif _is_v2_device_factory(var): + v2_factories[var.__name__] = var + + return v1_factories, v2_factories def _is_device_skipped(func: Callable[..., Any]) -> bool: @@ -157,17 +171,29 @@ def _is_device_skipped(func: Callable[..., Any]) -> bool: return func.__skip__ # type: ignore -def _is_device_factory(func: Callable[..., Any]) -> bool: +def _is_v1_device_factory(func: Callable[..., Any]) -> bool: + try: + return_type = signature(func).return_annotation + return _is_v1_device_type(return_type) + except ValueError: + return False + + +def _is_v2_device_factory(func: Callable[..., Any]) -> bool: try: return_type = signature(func).return_annotation - return _is_device_type(return_type) + return _is_v2_device_type(return_type) except ValueError: return False + + +def _is_v2_device_type(obj: Type[Any]) -> bool: + return issubclass(obj, OphydV2Device) -def _is_device_type(obj: Type[Any]) -> bool: +def _is_v1_device_type(obj: Type[Any]) -> bool: is_class = inspect.isclass(obj) follows_protocols = any( map(lambda protocol: isinstance(obj, protocol), BLUESKY_PROTOCOLS) ) - return is_class and follows_protocols + return is_class and follows_protocols and not _is_v2_device_type(obj) diff --git a/tests/system_tests/test_i23_system.py b/tests/system_tests/test_i23_system.py index 8a35e9cc05..063353156b 100644 --- a/tests/system_tests/test_i23_system.py +++ b/tests/system_tests/test_i23_system.py @@ -1,11 +1,14 @@ import os from dodal.utils import make_all_devices +from bluesky.run_engine import RunEngine if __name__ == "__main__": """This test runs against the real beamline and confirms that all the devices connect - i.e. that we match the beamline PVs. Obviously this must be run on the DLS network - and could be flaky if parts of the beamline are down for maintenance. + i.e. that we match the beamline PVs. + + THIS TEST IS ONLY EXPECTED TO WORK FROM AN I23 WORKSTATION! This is because i23 uses an + EPICS v4 (pva) device, and no PVA gateways currently exist. This is not implemented as a normal pytest test as those tests run using the S03 EPICS ports and switching ports at runtime is non-trivial @@ -13,6 +16,10 @@ os.environ["BEAMLINE"] = "i23" from dodal.beamlines import i23 + # For i23, need a RunEngine started, as it uses some Ophyd V2 devices which need a bluesky + # event loop started in order to connect. + RE = RunEngine() + print("Making all i23 devices") make_all_devices(i23) print("Successfully connected") From 3c26bd49a8fd9dc6a51835720f1e0fac2b8844eb Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Tue, 15 Aug 2023 15:35:29 +0100 Subject: [PATCH 13/30] Refactor --- src/dodal/beamlines/beamline_utils.py | 54 +++++++++------ src/dodal/beamlines/i23.py | 14 ++-- .../oav_with_pin_tip_detection.py} | 6 +- .../pin_tip_detect_utils.py} | 0 src/dodal/utils.py | 68 +++++++++++-------- .../edge_detection/test_edge_detect_utils.py | 2 +- tests/system_tests/test_i23_system.py | 3 +- 7 files changed, 86 insertions(+), 61 deletions(-) rename src/dodal/devices/oav/{edge_detection/oav_with_edge_detection.py => pin_tip_detection/oav_with_pin_tip_detection.py} (96%) rename src/dodal/devices/oav/{edge_detection/edge_detect_utils.py => pin_tip_detection/pin_tip_detect_utils.py} (100%) diff --git a/src/dodal/beamlines/beamline_utils.py b/src/dodal/beamlines/beamline_utils.py index 6898cd0ee0..4d3ffb7010 100644 --- a/src/dodal/beamlines/beamline_utils.py +++ b/src/dodal/beamlines/beamline_utils.py @@ -1,16 +1,17 @@ -from typing import Callable, Dict, List, Optional, TypeVar, cast +import asyncio +from typing import Callable, Dict, Final, List, Optional, TypeVar, cast -from ophyd import Device -from ophyd.v2.core import Device as OphydV2Device, wait_for_connection as v2_device_wait_for_connection +from bluesky.run_engine import call_in_bluesky_event_loop +from ophyd import Device as OphydV1Device from ophyd.sim import make_fake_device +from ophyd.v2.core import Device as OphydV2Device +from ophyd.v2.core import wait_for_connection as v2_device_wait_for_connection -from dodal.utils import BeamlinePrefix, skip_device - -from bluesky.run_engine import call_in_bluesky_event_loop -import asyncio +from dodal.utils import AnyDevice, BeamlinePrefix, skip_device +DEFAULT_CONNECTION_TIMEOUT: Final[float] = 5.0 -ACTIVE_DEVICES: Dict[str, Device | OphydV2Device] = {} +ACTIVE_DEVICES: Dict[str, AnyDevice] = {} BL = "" @@ -35,21 +36,31 @@ def list_active_devices() -> List[str]: return list(ACTIVE_DEVICES.keys()) -def active_device_is_same_type(active_device, device): +def active_device_is_same_type( + active_device: AnyDevice, device: Callable[..., AnyDevice] +) -> bool: return type(active_device) == device -T = TypeVar("T", bound=Device | OphydV2Device) +def _wait_for_connection( + device: AnyDevice, timeout: float = DEFAULT_CONNECTION_TIMEOUT +) -> None: + if isinstance(device, OphydV1Device): + device.wait_for_connection(timeout=timeout) + elif isinstance(device, OphydV2Device): + call_in_bluesky_event_loop( + asyncio.wait_for( + v2_device_wait_for_connection(coros=device.connect()), + timeout=timeout, + ) + ) + else: + raise TypeError( + "Invalid type {} in _wait_for_connection".format(device.__class__.__name__) + ) -def _wait_for_connection(device: Device | OphydV2Device) -> None: - match device: - case Device(): - device.wait_for_connection() - case OphydV2Device(): - call_in_bluesky_event_loop(asyncio.wait_for(v2_device_wait_for_connection(coros=device.connect()), timeout=30)) - case _: - raise TypeError("Invalid type {} in _wait_for_connection".format(device.__class__.__name__)) +T = TypeVar("T", bound=AnyDevice) @skip_device() @@ -80,11 +91,12 @@ def device_instantiation( Returns: The instance of the device. """ - active_device = ACTIVE_DEVICES.get(name) + device_instance: T + active_device: Optional[AnyDevice] = ACTIVE_DEVICES.get(name) if fake: device = make_fake_device(device) if active_device is None: - device_instance: T = device( + device_instance = device( name=name, prefix=f"{(BeamlinePrefix(BL).beamline_prefix)}{prefix}" if bl_prefix @@ -104,7 +116,7 @@ def device_instantiation( ) else: # We have manually checked types - device_instance: T = cast(T, ACTIVE_DEVICES[name]) + device_instance = cast(T, ACTIVE_DEVICES[name]) if post_create: post_create(device_instance) return device_instance diff --git a/src/dodal/beamlines/i23.py b/src/dodal/beamlines/i23.py index 13a35c518c..bb140041cd 100644 --- a/src/dodal/beamlines/i23.py +++ b/src/dodal/beamlines/i23.py @@ -1,7 +1,9 @@ from dodal.beamlines.beamline_utils import device_instantiation from dodal.beamlines.beamline_utils import set_beamline as set_utils_beamline from dodal.devices.i23.gonio import Gonio -from dodal.devices.oav.edge_detection.oav_with_edge_detection import EdgeDetection +from dodal.devices.oav.pin_tip_detection.oav_with_pin_tip_detection import ( + PinTipDetection, +) from dodal.log import set_beamline as set_log_beamline from dodal.utils import get_beamline_name @@ -21,13 +23,15 @@ def gonio(wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False) - ) -def oav_pin_tip_detection(wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False) -> EdgeDetection: +def oav_pin_tip_detection( + wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False +) -> PinTipDetection: """Get the i23 OAV pin-tip detection device""" return device_instantiation( - EdgeDetection, + PinTipDetection, "PinTipDetection", - "BL03I-DI-OAV-01:", # TODO: FIXME + "BL03I-DI-OAV-01:", # TODO: FIXME - pointing at i03 for test, should point at i23 wait_for_connection, fake_with_ophyd_sim, - bl_prefix=False, + bl_prefix=False, # TODO: FIXME ) diff --git a/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py b/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py similarity index 96% rename from src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py rename to src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py index 35e5893fac..7cbb7d159a 100644 --- a/src/dodal/devices/oav/edge_detection/oav_with_edge_detection.py +++ b/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py @@ -9,7 +9,7 @@ from ophyd.v2.core import Readable, Reading, Device from ophyd.v2.epics import SignalR, epics_signal_r -from dodal.devices.oav.edge_detection.edge_detect_utils import ( +from dodal.devices.oav.pin_tip_detection.pin_tip_detect_utils import ( ArrayProcessingFunctions, MxSampleDetect, ) @@ -18,7 +18,7 @@ T = TypeVar("T") -class EdgeDetection(Readable, Device): +class PinTipDetection(Readable, Device): def __init__(self, prefix: str, name: str = ""): self._prefix: str = prefix self._name = name @@ -137,7 +137,7 @@ async def describe(self) -> dict[str, Descriptor]: if __name__ == "__main__": - x = EdgeDetection(prefix="BL03I-DI-OAV-01:", name="edgeDetect") + x = PinTipDetection(prefix="BL03I-DI-OAV-01:", name="edgeDetect") async def acquire(): await x.connect() diff --git a/src/dodal/devices/oav/edge_detection/edge_detect_utils.py b/src/dodal/devices/oav/pin_tip_detection/pin_tip_detect_utils.py similarity index 100% rename from src/dodal/devices/oav/edge_detection/edge_detect_utils.py rename to src/dodal/devices/oav/pin_tip_detection/pin_tip_detect_utils.py diff --git a/src/dodal/utils.py b/src/dodal/utils.py index 439b62612b..a9ae3ae0de 100644 --- a/src/dodal/utils.py +++ b/src/dodal/utils.py @@ -11,10 +11,10 @@ Callable, Dict, Iterable, - Final, Mapping, Optional, Type, + TypeAlias, TypeVar, Union, ) @@ -35,7 +35,7 @@ Triggerable, WritesExternalAssets, ) - +from ophyd.device import Device as OphydV1Device from ophyd.v2.core import Device as OphydV2Device #: Protocols defining interface to hardware @@ -56,8 +56,10 @@ Triggerable, ] - -T = TypeVar("T") +AnyDevice: TypeAlias = OphydV1Device | OphydV2Device +V1DeviceFactory: TypeAlias = Callable[..., OphydV1Device] +V2DeviceFactory: TypeAlias = Callable[..., OphydV2Device] +AnyDeviceFactory: TypeAlias = V1DeviceFactory | V2DeviceFactory def get_beamline_name(default: str) -> str: @@ -79,6 +81,9 @@ def __post_init__(self): self.insertion_prefix = f"SR{self.ixx[1:3]}{self.suffix}" +T = TypeVar("T", bound=AnyDevice) + + def skip_device(precondition=lambda: True): def decorator(func: Callable[..., T]) -> Callable[..., T]: @wraps(func) @@ -94,7 +99,7 @@ def wrapper(*args, **kwds) -> T: def make_all_devices( module: Union[str, ModuleType, None] = None, **kwargs -) -> Dict[str, HasName]: +) -> dict[str, AnyDevice]: """Makes all devices in the given beamline module. In cases of device interdependencies it ensures a device is created before any which @@ -109,25 +114,23 @@ def make_all_devices( """ if isinstance(module, str) or module is None: module = import_module(module or __name__) - v1_factories, v2_factories = collect_factories(module) - print(f"v1: {v1_factories}, v2: {v2_factories}") - devices = invoke_factories(v1_factories, **kwargs) - - for name, factory in v2_factories.items(): - devices[name] = factory(**kwargs) + factories = collect_factories(module) + devices: dict[str, AnyDevice] = invoke_factories(factories, **kwargs) return devices def invoke_factories( - factories: Mapping[str, Callable[..., Any]], + factories: Mapping[str, AnyDeviceFactory], **kwargs, -) -> Dict[str, HasName]: - devices: Dict[str, HasName] = {} +) -> Dict[str, AnyDevice]: + devices: dict[str, AnyDevice] = {} + dependencies = { factory_name: set(extract_dependencies(factories, factory_name)) for factory_name in factories.keys() } + while len(devices) < len(factories): leaves = [ device @@ -145,50 +148,55 @@ def invoke_factories( def extract_dependencies( - factories: Mapping[str, Callable[..., Any]], factory_name: str + factories: Mapping[str, AnyDeviceFactory], factory_name: str ) -> Iterable[str]: for name, param in inspect.signature(factories[factory_name]).parameters.items(): if param.default is inspect.Parameter.empty and name in factories: yield name -def collect_factories(module: ModuleType) -> tuple[Mapping[str, Callable[..., Any]], Mapping[str, Callable[..., OphydV2Device]]]: - v1_factories = {} - v2_factories = {} +def collect_factories(module: ModuleType) -> dict[str, AnyDeviceFactory]: + factories: dict[str, AnyDeviceFactory] = {} + for var in module.__dict__.values(): - if callable(var) and not _is_device_skipped(var): - if _is_v1_device_factory(var): - v1_factories[var.__name__] = var - elif _is_v2_device_factory(var): - v2_factories[var.__name__] = var + if ( + callable(var) + and _is_any_device_factory(var) + and not _is_device_skipped(var) + ): + factories[var.__name__] = var - return v1_factories, v2_factories + return factories -def _is_device_skipped(func: Callable[..., Any]) -> bool: +def _is_device_skipped(func: AnyDeviceFactory) -> bool: if not hasattr(func, "__skip__"): return False return func.__skip__ # type: ignore -def _is_v1_device_factory(func: Callable[..., Any]) -> bool: +def _is_v1_device_factory(func: AnyDeviceFactory) -> bool: try: return_type = signature(func).return_annotation return _is_v1_device_type(return_type) except ValueError: return False - -def _is_v2_device_factory(func: Callable[..., Any]) -> bool: + +def _is_v2_device_factory(func: AnyDeviceFactory) -> bool: try: return_type = signature(func).return_annotation return _is_v2_device_type(return_type) except ValueError: return False - + + +def _is_any_device_factory(func: AnyDeviceFactory) -> bool: + return _is_v1_device_factory(func) or _is_v2_device_factory(func) + def _is_v2_device_type(obj: Type[Any]) -> bool: - return issubclass(obj, OphydV2Device) + return inspect.isclass(obj) and issubclass(obj, OphydV2Device) def _is_v1_device_type(obj: Type[Any]) -> bool: diff --git a/tests/devices/unit_tests/oav/edge_detection/test_edge_detect_utils.py b/tests/devices/unit_tests/oav/edge_detection/test_edge_detect_utils.py index 08ff3c11a4..35510f2bda 100644 --- a/tests/devices/unit_tests/oav/edge_detection/test_edge_detect_utils.py +++ b/tests/devices/unit_tests/oav/edge_detection/test_edge_detect_utils.py @@ -1,7 +1,7 @@ import numpy as np import pytest -from dodal.devices.oav.edge_detection.edge_detect_utils import ( +from dodal.devices.oav.pin_tip_detection.pin_tip_detect_utils import ( NONE_VALUE, MxSampleDetect, ) diff --git a/tests/system_tests/test_i23_system.py b/tests/system_tests/test_i23_system.py index 063353156b..476c63e1dd 100644 --- a/tests/system_tests/test_i23_system.py +++ b/tests/system_tests/test_i23_system.py @@ -1,8 +1,9 @@ import os -from dodal.utils import make_all_devices from bluesky.run_engine import RunEngine +from dodal.utils import make_all_devices + if __name__ == "__main__": """This test runs against the real beamline and confirms that all the devices connect i.e. that we match the beamline PVs. From 498ce53d41ad02d9f2b4621229268273548f01b8 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Tue, 15 Aug 2023 16:11:48 +0100 Subject: [PATCH 14/30] Correct type hints --- .../oav/pin_tip_detection/oav_with_pin_tip_detection.py | 6 +++--- src/dodal/utils.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py b/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py index 7cbb7d159a..40db8c8242 100644 --- a/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py +++ b/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py @@ -1,12 +1,12 @@ import asyncio import time from collections import OrderedDict -from typing import Callable, Optional, Tuple, TypeVar, Type +from typing import Callable, Optional, Tuple, Type, TypeVar import numpy as np from bluesky.protocols import Descriptor from numpy.typing import NDArray -from ophyd.v2.core import Readable, Reading, Device +from ophyd.v2.core import Device, Readable, Reading from ophyd.v2.epics import SignalR, epics_signal_r from dodal.devices.oav.pin_tip_detection.pin_tip_detect_utils import ( @@ -92,7 +92,7 @@ async def _get_tip_position( num_pixels * 3, value_len, ) - + start_time = time.time() location = sample_detection.processArray(value) end_time = time.time() diff --git a/src/dodal/utils.py b/src/dodal/utils.py index a9ae3ae0de..d5c101a9d4 100644 --- a/src/dodal/utils.py +++ b/src/dodal/utils.py @@ -175,7 +175,7 @@ def _is_device_skipped(func: AnyDeviceFactory) -> bool: return func.__skip__ # type: ignore -def _is_v1_device_factory(func: AnyDeviceFactory) -> bool: +def _is_v1_device_factory(func: Callable) -> bool: try: return_type = signature(func).return_annotation return _is_v1_device_type(return_type) @@ -183,7 +183,7 @@ def _is_v1_device_factory(func: AnyDeviceFactory) -> bool: return False -def _is_v2_device_factory(func: AnyDeviceFactory) -> bool: +def _is_v2_device_factory(func: Callable) -> bool: try: return_type = signature(func).return_annotation return _is_v2_device_type(return_type) @@ -191,7 +191,7 @@ def _is_v2_device_factory(func: AnyDeviceFactory) -> bool: return False -def _is_any_device_factory(func: AnyDeviceFactory) -> bool: +def _is_any_device_factory(func: Callable) -> bool: return _is_v1_device_factory(func) or _is_v2_device_factory(func) From 08ac04b780c89c646041cfef27d30c094c8a523b Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Wed, 16 Aug 2023 09:48:39 +0100 Subject: [PATCH 15/30] Remove unused import --- .../devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py b/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py index 40db8c8242..80a7372a25 100644 --- a/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py +++ b/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py @@ -1,7 +1,7 @@ import asyncio import time from collections import OrderedDict -from typing import Callable, Optional, Tuple, Type, TypeVar +from typing import Callable, Optional, Tuple, TypeVar import numpy as np from bluesky.protocols import Descriptor From e782d4d8a3c8e21ab319bf17b887633c0ded44e1 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Wed, 16 Aug 2023 10:44:11 +0100 Subject: [PATCH 16/30] Make parameters settable externally --- .../oav_with_pin_tip_detection.py | 76 +++++++++++++------ 1 file changed, 54 insertions(+), 22 deletions(-) diff --git a/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py b/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py index 80a7372a25..03f2a2a395 100644 --- a/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py +++ b/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py @@ -1,13 +1,13 @@ import asyncio import time from collections import OrderedDict -from typing import Callable, Optional, Tuple, TypeVar +from typing import Callable, Optional, Tuple, Type, TypeVar import numpy as np from bluesky.protocols import Descriptor from numpy.typing import NDArray -from ophyd.v2.core import Device, Readable, Reading -from ophyd.v2.epics import SignalR, epics_signal_r +from ophyd.v2.core import Device, Readable, Reading, SimConverter, SimSignalBackend +from ophyd.v2.epics import SignalR, SignalRW, epics_signal_r from dodal.devices.oav.pin_tip_detection.pin_tip_detect_utils import ( ArrayProcessingFunctions, @@ -18,6 +18,28 @@ T = TypeVar("T") +def _create_soft_signal( + name: str, datatype: T | Type[T], initial_value: T +) -> SignalRW[T]: + class _Converter(SimConverter): + def make_initial_value(self, *args, **kwargs) -> T: + return initial_value + + class _SimSignalBackend(SimSignalBackend): + async def connect(self) -> None: + self.converter = _Converter() + self._initial_value = self.converter.make_initial_value() + self._severity = 0 + + await self.put(None) + + backend = _SimSignalBackend( + datatype, f"sim://oav_with_pin_tip_detection_soft_params:{name}" + ) + signal: SignalRW[T] = SignalRW(backend) + return signal + + class PinTipDetection(Readable, Device): def __init__(self, prefix: str, name: str = ""): self._prefix: str = prefix @@ -34,17 +56,27 @@ def __init__(self, prefix: str, name: str = ""): int, f"{prefix}PVA:ArraySize2_RBV" ) - self.timeout: float = 10.0 - - self.preprocess: Callable[ - [np.ndarray], np.ndarray - ] = ArrayProcessingFunctions.identity() - self.canny_upper: int = 100 - self.canny_lower: int = 50 - self.close_ksize: int = 5 - self.close_iterations: int = 5 - self.scan_direction: int = 1 - self.min_tip_height: int = 5 + # Soft parameters for pin-tip detection. + self.timeout: SignalRW[float] = _create_soft_signal("timeout", float, 10.0) + self.preprocess: SignalRW[ + Callable[[np.ndarray], np.ndarray] + ] = _create_soft_signal( + "preprocess", + Callable[[np.ndarray], np.ndarray], + ArrayProcessingFunctions.identity(), + ) + self.canny_upper: SignalRW[int] = _create_soft_signal("canny_upper", int, 100) + self.canny_lower: SignalRW[int] = _create_soft_signal("canny_lower", int, 50) + self.close_ksize: SignalRW[int] = _create_soft_signal("close_ksize", int, 5) + self.close_iterations: SignalRW[int] = _create_soft_signal( + "close_iterations", int, 5 + ) + self.scan_direction: SignalRW[int] = _create_soft_signal( + "scan_direction", int, 1 + ) + self.min_tip_height: SignalRW[int] = _create_soft_signal( + "min_tip_height", int, 5 + ) super().__init__(name=name) @@ -58,13 +90,13 @@ async def _get_tip_position( ((tip_x, tip_y), timestamp) """ sample_detection = MxSampleDetect( - preprocess=self.preprocess, - canny_lower=self.canny_lower, - canny_upper=self.canny_upper, - close_ksize=self.close_ksize, - close_iterations=self.close_iterations, - scan_direction=self.scan_direction, - min_tip_height=self.min_tip_height, + preprocess=await self.preprocess.get_value(), + canny_lower=await self.canny_lower.get_value(), + canny_upper=await self.canny_upper.get_value(), + close_ksize=await self.close_ksize.get_value(), + close_iterations=await self.close_iterations.get_value(), + scan_direction=await self.scan_direction.get_value(), + min_tip_height=await self.min_tip_height.get_value(), ) array_reading: dict[str, Reading] = await self.array_data.read() @@ -112,7 +144,7 @@ async def _get_tip_position( async def read(self) -> dict[str, Reading]: tip_pos, timestamp = await asyncio.wait_for( - self._get_tip_position(), timeout=self.timeout + self._get_tip_position(), timeout=await self.timeout.get_value() ) return OrderedDict( From cd881124f299347060e78659f314795f75f4154d Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Wed, 16 Aug 2023 13:18:33 +0100 Subject: [PATCH 17/30] Add further unit tests --- pyproject.toml | 1 + src/dodal/beamlines/i23.py | 3 +- .../devices/oav/pin_tip_detection/__init__.py | 0 .../oav_with_pin_tip_detection.py | 7 + .../pin_tip_detection/test_pin_tip_detect.py | 128 ++++++++++++++++++ .../test_pin_tip_detect_utils.py} | 0 6 files changed, 137 insertions(+), 2 deletions(-) create mode 100644 src/dodal/devices/oav/pin_tip_detection/__init__.py create mode 100644 tests/devices/unit_tests/oav/pin_tip_detection/test_pin_tip_detect.py rename tests/devices/unit_tests/oav/{edge_detection/test_edge_detect_utils.py => pin_tip_detection/test_pin_tip_detect_utils.py} (100%) diff --git a/pyproject.toml b/pyproject.toml index 634a503571..b2defa9cad 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,6 +41,7 @@ dev = [ "pipdeptree", "pre-commit", "pydata-sphinx-theme>=0.12", + "pytest-asyncio", "pytest-cov", "pytest-random-order", "sphinx-autobuild", diff --git a/src/dodal/beamlines/i23.py b/src/dodal/beamlines/i23.py index bb140041cd..d88f35c37d 100644 --- a/src/dodal/beamlines/i23.py +++ b/src/dodal/beamlines/i23.py @@ -30,8 +30,7 @@ def oav_pin_tip_detection( return device_instantiation( PinTipDetection, "PinTipDetection", - "BL03I-DI-OAV-01:", # TODO: FIXME - pointing at i03 for test, should point at i23 + "-DI-OAV-01:", wait_for_connection, fake_with_ophyd_sim, - bl_prefix=False, # TODO: FIXME ) diff --git a/src/dodal/devices/oav/pin_tip_detection/__init__.py b/src/dodal/devices/oav/pin_tip_detection/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py b/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py index 03f2a2a395..d3232ebb61 100644 --- a/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py +++ b/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py @@ -80,6 +80,13 @@ def __init__(self, prefix: str, name: str = ""): super().__init__(name=name) + async def connect(self, sim: bool = False): + for signal in [self.array_data, self.oav_width, self.oav_height]: + await signal.connect(sim=sim) + + for soft_signal in [self.timeout, self.preprocess, self.canny_upper, self.canny_lower, self.close_ksize, self.close_iterations, self.scan_direction, self.min_tip_height]: + await soft_signal.connect(sim=False) # Soft signals don't need simulating. + async def _get_tip_position( self, ) -> Tuple[Tuple[Optional[int], Optional[int]], float]: diff --git a/tests/devices/unit_tests/oav/pin_tip_detection/test_pin_tip_detect.py b/tests/devices/unit_tests/oav/pin_tip_detection/test_pin_tip_detect.py new file mode 100644 index 0000000000..f0203a9787 --- /dev/null +++ b/tests/devices/unit_tests/oav/pin_tip_detection/test_pin_tip_detect.py @@ -0,0 +1,128 @@ +import pytest +import numpy as np +from numpy.typing import NDArray +import asyncio +from dodal.devices.oav.pin_tip_detection.oav_with_pin_tip_detection import PinTipDetection, MxSampleDetect +from dodal.devices.oav.pin_tip_detection.pin_tip_detect_utils import SampleLocation, ArrayProcessingFunctions +from ophyd.v2.core import set_sim_value +from unittest.mock import MagicMock + + +EVENT_LOOP = asyncio.new_event_loop() + + +pytest_plugins = ('pytest_asyncio', ) + + +async def _get_pin_tip_detection_device() -> PinTipDetection: + device = PinTipDetection("-DI-OAV-01") + await device.connect(sim=True) + return device + + +@pytest.mark.asyncio +async def test_pin_tip_detect_can_be_connected_in_sim_mode(): + device = await _get_pin_tip_detection_device() + await device.connect(sim=True) + + +@pytest.mark.asyncio +async def test_soft_parameter_defaults_are_correct(): + device = await _get_pin_tip_detection_device() + + assert await device.timeout.get_value() == 10. + assert await device.canny_lower.get_value() == 50 + assert await device.canny_upper.get_value() == 100 + assert await device.close_ksize.get_value() == 5 + assert await device.close_iterations.get_value() == 5 + assert await device.min_tip_height.get_value() == 5 + assert await device.scan_direction.get_value() == 1 + + +@pytest.mark.asyncio +async def test_numeric_soft_parameters_can_be_changed(): + device = await _get_pin_tip_detection_device() + + await device.timeout.set(100.) + await device.canny_lower.set(5) + await device.canny_upper.set(10) + await device.close_ksize.set(15) + await device.close_iterations.set(20) + await device.min_tip_height.set(25) + await device.scan_direction.set(-1) + + assert await device.timeout.get_value() == 100. + assert await device.canny_lower.get_value() == 5 + assert await device.canny_upper.get_value() == 10 + assert await device.close_ksize.get_value() == 15 + assert await device.close_iterations.get_value() == 20 + assert await device.min_tip_height.get_value() == 25 + assert await device.scan_direction.get_value() == -1 + + +@pytest.mark.asyncio +async def test_preprocessing_soft_parameter_can_be_changed(): + device = await _get_pin_tip_detection_device() + + my_func_called = False + + def my_preprocessing_func(arr: NDArray) -> NDArray: + nonlocal my_func_called + my_func_called = True + return arr + + await device.preprocess.set(my_preprocessing_func) + + f = (await device.preprocess.read())[""]["value"] + + f(None) + + assert my_func_called + + +@pytest.mark.parametrize("input_array,height,width,reshaped", [ + (np.zeros(shape=(1, )), 1, 1, np.zeros(shape=(1, 1))), + (np.zeros(shape=(3, )), 1, 1, np.zeros(shape=(1, 1, 3))), + (np.zeros(shape=(1920*1080)), 1080, 1920, np.zeros(shape=(1080, 1920))), + (np.zeros(shape=(1920*1080*3)), 1080, 1920, np.zeros(shape=(1080, 1920, 3))), +]) +def test_when_data_supplied_THEN_reshaped_correctly_before_call_to_process_array(input_array: NDArray, height: int, width: int, reshaped: NDArray): + device = EVENT_LOOP.run_until_complete(_get_pin_tip_detection_device()) + + device.array_data._backend._set_value(input_array) # type: ignore + set_sim_value(device.oav_height, height) + set_sim_value(device.oav_width, width) + + MxSampleDetect.processArray = MagicMock( + autospec=True, + return_value=SampleLocation(tip_x=10, tip_y=20, edge_bottom=np.array([]), edge_top=np.array([])), + ) + + result = EVENT_LOOP.run_until_complete(device.read()) + + MxSampleDetect.processArray.assert_called_once() + np.testing.assert_array_equal(MxSampleDetect.processArray.call_args[0][0], reshaped) + + assert result[""]["value"] == (10, 20) + + +@pytest.mark.parametrize("input_array,height,width", [ + (np.zeros(shape=(0,)), 1080, 1920), + (np.zeros(shape=(1920*1080*2)), 1080, 1920), +]) +def test_when_invalid_data_length_supplied_THEN_no_call_to_process_array(input_array: NDArray, height: int, width: int): + device = EVENT_LOOP.run_until_complete(_get_pin_tip_detection_device()) + + set_sim_value(device.array_data, input_array) + set_sim_value(device.oav_height, height) + set_sim_value(device.oav_width, width) + + MxSampleDetect.processArray = MagicMock( + autospec=True, + ) + + result = EVENT_LOOP.run_until_complete(device.read()) + + MxSampleDetect.processArray.assert_not_called() + + assert result[""]["value"] == (None, None) diff --git a/tests/devices/unit_tests/oav/edge_detection/test_edge_detect_utils.py b/tests/devices/unit_tests/oav/pin_tip_detection/test_pin_tip_detect_utils.py similarity index 100% rename from tests/devices/unit_tests/oav/edge_detection/test_edge_detect_utils.py rename to tests/devices/unit_tests/oav/pin_tip_detection/test_pin_tip_detect_utils.py From 732a194029ead80d687c28a1147b8dd960d984dd Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Wed, 16 Aug 2023 13:51:12 +0100 Subject: [PATCH 18/30] Placate linters --- pyproject.toml | 2 +- .../oav_with_pin_tip_detection.py | 11 +++- .../pin_tip_detection/test_pin_tip_detect.py | 65 ++++++++++++------- 3 files changed, 54 insertions(+), 24 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index b2defa9cad..c88492595e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -14,7 +14,7 @@ classifiers = [ ] description = "Ophyd devices and other utils that could be used across DLS beamlines" dependencies = [ - "ophyd@git+https://github.com/Tom-Willemsen/ophyd#ntndarray", # Switch back to just "ophyd" once merged. + "ophyd@git+https://github.com/Tom-Willemsen/ophyd@40e4b72c582e65e63d13a1650f76de709e5c70bb", # Switch back to just "ophyd" once merged. "bluesky", "pyepics", "dataclasses-json", diff --git a/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py b/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py index d3232ebb61..c8e7223827 100644 --- a/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py +++ b/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py @@ -84,7 +84,16 @@ async def connect(self, sim: bool = False): for signal in [self.array_data, self.oav_width, self.oav_height]: await signal.connect(sim=sim) - for soft_signal in [self.timeout, self.preprocess, self.canny_upper, self.canny_lower, self.close_ksize, self.close_iterations, self.scan_direction, self.min_tip_height]: + for soft_signal in [ + self.timeout, + self.preprocess, + self.canny_upper, + self.canny_lower, + self.close_ksize, + self.close_iterations, + self.scan_direction, + self.min_tip_height, + ]: await soft_signal.connect(sim=False) # Soft signals don't need simulating. async def _get_tip_position( diff --git a/tests/devices/unit_tests/oav/pin_tip_detection/test_pin_tip_detect.py b/tests/devices/unit_tests/oav/pin_tip_detection/test_pin_tip_detect.py index f0203a9787..2855691bda 100644 --- a/tests/devices/unit_tests/oav/pin_tip_detection/test_pin_tip_detect.py +++ b/tests/devices/unit_tests/oav/pin_tip_detection/test_pin_tip_detect.py @@ -1,17 +1,21 @@ -import pytest +import asyncio +from unittest.mock import MagicMock + import numpy as np +import pytest from numpy.typing import NDArray -import asyncio -from dodal.devices.oav.pin_tip_detection.oav_with_pin_tip_detection import PinTipDetection, MxSampleDetect -from dodal.devices.oav.pin_tip_detection.pin_tip_detect_utils import SampleLocation, ArrayProcessingFunctions from ophyd.v2.core import set_sim_value -from unittest.mock import MagicMock +from dodal.devices.oav.pin_tip_detection.oav_with_pin_tip_detection import ( + MxSampleDetect, + PinTipDetection, +) +from dodal.devices.oav.pin_tip_detection.pin_tip_detect_utils import SampleLocation EVENT_LOOP = asyncio.new_event_loop() -pytest_plugins = ('pytest_asyncio', ) +pytest_plugins = ("pytest_asyncio",) async def _get_pin_tip_detection_device() -> PinTipDetection: @@ -30,7 +34,7 @@ async def test_pin_tip_detect_can_be_connected_in_sim_mode(): async def test_soft_parameter_defaults_are_correct(): device = await _get_pin_tip_detection_device() - assert await device.timeout.get_value() == 10. + assert await device.timeout.get_value() == 10.0 assert await device.canny_lower.get_value() == 50 assert await device.canny_upper.get_value() == 100 assert await device.close_ksize.get_value() == 5 @@ -43,7 +47,7 @@ async def test_soft_parameter_defaults_are_correct(): async def test_numeric_soft_parameters_can_be_changed(): device = await _get_pin_tip_detection_device() - await device.timeout.set(100.) + await device.timeout.set(100.0) await device.canny_lower.set(5) await device.canny_upper.set(10) await device.close_ksize.set(15) @@ -51,7 +55,7 @@ async def test_numeric_soft_parameters_can_be_changed(): await device.min_tip_height.set(25) await device.scan_direction.set(-1) - assert await device.timeout.get_value() == 100. + assert await device.timeout.get_value() == 100.0 assert await device.canny_lower.get_value() == 5 assert await device.canny_upper.get_value() == 10 assert await device.close_ksize.get_value() == 15 @@ -80,13 +84,23 @@ def my_preprocessing_func(arr: NDArray) -> NDArray: assert my_func_called -@pytest.mark.parametrize("input_array,height,width,reshaped", [ - (np.zeros(shape=(1, )), 1, 1, np.zeros(shape=(1, 1))), - (np.zeros(shape=(3, )), 1, 1, np.zeros(shape=(1, 1, 3))), - (np.zeros(shape=(1920*1080)), 1080, 1920, np.zeros(shape=(1080, 1920))), - (np.zeros(shape=(1920*1080*3)), 1080, 1920, np.zeros(shape=(1080, 1920, 3))), -]) -def test_when_data_supplied_THEN_reshaped_correctly_before_call_to_process_array(input_array: NDArray, height: int, width: int, reshaped: NDArray): +@pytest.mark.parametrize( + "input_array,height,width,reshaped", + [ + (np.zeros(shape=(1,)), 1, 1, np.zeros(shape=(1, 1))), + (np.zeros(shape=(3,)), 1, 1, np.zeros(shape=(1, 1, 3))), + (np.zeros(shape=(1920 * 1080)), 1080, 1920, np.zeros(shape=(1080, 1920))), + ( + np.zeros(shape=(1920 * 1080 * 3)), + 1080, + 1920, + np.zeros(shape=(1080, 1920, 3)), + ), + ], +) +def test_when_data_supplied_THEN_reshaped_correctly_before_call_to_process_array( + input_array: NDArray, height: int, width: int, reshaped: NDArray +): device = EVENT_LOOP.run_until_complete(_get_pin_tip_detection_device()) device.array_data._backend._set_value(input_array) # type: ignore @@ -95,7 +109,9 @@ def test_when_data_supplied_THEN_reshaped_correctly_before_call_to_process_array MxSampleDetect.processArray = MagicMock( autospec=True, - return_value=SampleLocation(tip_x=10, tip_y=20, edge_bottom=np.array([]), edge_top=np.array([])), + return_value=SampleLocation( + tip_x=10, tip_y=20, edge_bottom=np.array([]), edge_top=np.array([]) + ), ) result = EVENT_LOOP.run_until_complete(device.read()) @@ -106,11 +122,16 @@ def test_when_data_supplied_THEN_reshaped_correctly_before_call_to_process_array assert result[""]["value"] == (10, 20) -@pytest.mark.parametrize("input_array,height,width", [ - (np.zeros(shape=(0,)), 1080, 1920), - (np.zeros(shape=(1920*1080*2)), 1080, 1920), -]) -def test_when_invalid_data_length_supplied_THEN_no_call_to_process_array(input_array: NDArray, height: int, width: int): +@pytest.mark.parametrize( + "input_array,height,width", + [ + (np.zeros(shape=(0,)), 1080, 1920), + (np.zeros(shape=(1920 * 1080 * 2)), 1080, 1920), + ], +) +def test_when_invalid_data_length_supplied_THEN_no_call_to_process_array( + input_array: NDArray, height: int, width: int +): device = EVENT_LOOP.run_until_complete(_get_pin_tip_detection_device()) set_sim_value(device.array_data, input_array) From 10f1fb2236dcd9ada9aff3e9f5ce0342368548b9 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 18 Aug 2023 10:01:36 +0100 Subject: [PATCH 19/30] Fix first batch of review comments --- src/dodal/beamlines/beamline_utils.py | 40 +++---- src/dodal/beamlines/i03.py | 12 +- src/dodal/beamlines/i23.py | 10 +- src/dodal/beamlines/i24.py | 6 +- .../oav_with_pin_tip_detection.py | 113 +++++++++--------- .../pin_tip_detection/pin_tip_detect_utils.py | 28 +++-- src/dodal/utils.py | 24 ++-- .../pin_tip_detection/test_pin_tip_detect.py | 23 +--- .../test_pin_tip_detect_utils.py | 1 - 9 files changed, 131 insertions(+), 126 deletions(-) diff --git a/src/dodal/beamlines/beamline_utils.py b/src/dodal/beamlines/beamline_utils.py index 4d3ffb7010..7512b25daa 100644 --- a/src/dodal/beamlines/beamline_utils.py +++ b/src/dodal/beamlines/beamline_utils.py @@ -43,14 +43,14 @@ def active_device_is_same_type( def _wait_for_connection( - device: AnyDevice, timeout: float = DEFAULT_CONNECTION_TIMEOUT + device: AnyDevice, timeout: float = DEFAULT_CONNECTION_TIMEOUT, sim: bool = False ) -> None: if isinstance(device, OphydV1Device): device.wait_for_connection(timeout=timeout) elif isinstance(device, OphydV2Device): call_in_bluesky_event_loop( asyncio.wait_for( - v2_device_wait_for_connection(coros=device.connect()), + v2_device_wait_for_connection(coros=device.connect(sim=sim)), timeout=timeout, ) ) @@ -65,7 +65,7 @@ def _wait_for_connection( @skip_device() def device_instantiation( - device: Callable[..., T], + device_factory: Callable[..., T], name: str, prefix: str, wait: bool, @@ -79,24 +79,24 @@ def device_instantiation( directly to the device constructor. Arguments: - device: Callable the device class - name: str the name for ophyd - prefix: str the PV prefix for the most (usually all) components - wait: bool whether to run .wait_for_connection() - fake: bool whether to fake with ophyd.sim - post_create: Callable (optional) a function to be run on the device after - creation - bl_prefix: bool if true, add the beamline prefix when instantiating, if - false the complete PV prefix must be supplied. + device_factory: Callable the device class + name: str the name for ophyd + prefix: str the PV prefix for the most (usually all) components + wait: bool whether to run .wait_for_connection() + fake: bool whether to fake with ophyd.sim + post_create: Callable (optional) a function to be run on the device after + creation + bl_prefix: bool if true, add the beamline prefix when instantiating, if + false the complete PV prefix must be supplied. Returns: The instance of the device. """ device_instance: T - active_device: Optional[AnyDevice] = ACTIVE_DEVICES.get(name) + already_existing_device: Optional[AnyDevice] = ACTIVE_DEVICES.get(name) if fake: - device = make_fake_device(device) - if active_device is None: - device_instance = device( + device_factory = make_fake_device(device_factory) + if already_existing_device is None: + device_instance = device_factory( name=name, prefix=f"{(BeamlinePrefix(BL).beamline_prefix)}{prefix}" if bl_prefix @@ -105,14 +105,14 @@ def device_instantiation( ) ACTIVE_DEVICES[name] = device_instance if wait: - _wait_for_connection(device_instance) + _wait_for_connection(device_instance, sim=fake) else: - if not active_device_is_same_type(active_device, device): + if not active_device_is_same_type(already_existing_device, device_factory): raise TypeError( - f"Can't instantiate device of type {type(active_device)} with the same " + f"Can't instantiate device of type {device_factory} with the same " f"name as an existing device. Device name '{name}' already used for " - f"a(n) {device}." + f"a(n) {type(already_existing_device)}." ) else: # We have manually checked types diff --git a/src/dodal/beamlines/i03.py b/src/dodal/beamlines/i03.py index 2b5d497604..8d021e1fc2 100644 --- a/src/dodal/beamlines/i03.py +++ b/src/dodal/beamlines/i03.py @@ -33,7 +33,7 @@ def dcm(wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False) -> If this is called when already instantiated in i03, it will return the existing object. """ return device_instantiation( - device=DCM, + device_factory=DCM, name="dcm", prefix="", wait=wait_for_connection, @@ -56,7 +56,7 @@ def load_positions(a_s: ApertureScatterguard): a_s.load_aperture_positions(aperture_positions) return device_instantiation( - device=ApertureScatterguard, + device_factory=ApertureScatterguard, name="aperture_scatterguard", prefix="", wait=wait_for_connection, @@ -72,7 +72,7 @@ def backlight( If this is called when already instantiated in i03, it will return the existing object. """ return device_instantiation( - device=Backlight, + device_factory=Backlight, name="backlight", prefix="-EA-BL-01:", wait=wait_for_connection, @@ -88,7 +88,7 @@ def detector_motion( If this is called when already instantiated in i03, it will return the existing object. """ return device_instantiation( - device=DetectorMotion, + device_factory=DetectorMotion, name="detector_motion", prefix="", wait=wait_for_connection, @@ -112,7 +112,7 @@ def set_params(eiger: EigerDetector): eiger.set_detector_parameters(params) return device_instantiation( - device=EigerDetector, + device_factory=EigerDetector, name="eiger", prefix="-EA-EIGER-01:", wait=wait_for_connection, @@ -128,7 +128,7 @@ def fast_grid_scan( If this is called when already instantiated in i03, it will return the existing object. """ return device_instantiation( - device=FastGridScan, + device_factory=FastGridScan, name="fast_grid_scan", prefix="-MO-SGON-01:FGS:", wait=wait_for_connection, diff --git a/src/dodal/beamlines/i23.py b/src/dodal/beamlines/i23.py index d88f35c37d..dbb913cc3c 100644 --- a/src/dodal/beamlines/i23.py +++ b/src/dodal/beamlines/i23.py @@ -4,6 +4,7 @@ from dodal.devices.oav.pin_tip_detection.oav_with_pin_tip_detection import ( PinTipDetection, ) +from dodal.log import LOGGER from dodal.log import set_beamline as set_log_beamline from dodal.utils import get_beamline_name @@ -27,9 +28,16 @@ def oav_pin_tip_detection( wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False ) -> PinTipDetection: """Get the i23 OAV pin-tip detection device""" + + if get_beamline_name("") != "i23": + LOGGER.warning( + "Not running on i23 - forcing pin tip detection into simulation mode" + ) + fake_with_ophyd_sim = True + return device_instantiation( PinTipDetection, - "PinTipDetection", + "pin_tip_detection", "-DI-OAV-01:", wait_for_connection, fake_with_ophyd_sim, diff --git a/src/dodal/beamlines/i24.py b/src/dodal/beamlines/i24.py index 6aa1934ad0..7fa7229cb3 100644 --- a/src/dodal/beamlines/i24.py +++ b/src/dodal/beamlines/i24.py @@ -24,7 +24,7 @@ def backlight( If this is called when already instantiated in i24, it will return the existing object. """ return device_instantiation( - device=Backlight, + device_factory=Backlight, name="backlight", prefix="-MO-BL-01:", wait=wait_for_connection, @@ -40,7 +40,7 @@ def detector_motion( If this is called when already instantiated in i24, it will return the existing object. """ return device_instantiation( - device=DetectorMotion, + device_factory=DetectorMotion, name="detector_motion", prefix="-MO-DET-01:", wait=wait_for_connection, @@ -64,7 +64,7 @@ def set_params(eiger: EigerDetector): eiger.set_detector_parameters(params) return device_instantiation( - device=EigerDetector, + device_factory=EigerDetector, name="eiger", prefix="-EA-EIGER-01:", wait=wait_for_connection, diff --git a/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py b/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py index c8e7223827..0e6c9682b0 100644 --- a/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py +++ b/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py @@ -1,15 +1,16 @@ import asyncio import time from collections import OrderedDict -from typing import Callable, Optional, Tuple, Type, TypeVar +from typing import Optional, Tuple, Type, TypeVar import numpy as np from bluesky.protocols import Descriptor from numpy.typing import NDArray -from ophyd.v2.core import Device, Readable, Reading, SimConverter, SimSignalBackend +from ophyd.v2.core import Device, Readable, Reading, SimSignalBackend from ophyd.v2.epics import SignalR, SignalRW, epics_signal_r from dodal.devices.oav.pin_tip_detection.pin_tip_detect_utils import ( + ARRAY_PROCESSING_FUNCTIONS_MAP, ArrayProcessingFunctions, MxSampleDetect, ) @@ -18,29 +19,25 @@ T = TypeVar("T") -def _create_soft_signal( - name: str, datatype: T | Type[T], initial_value: T -) -> SignalRW[T]: - class _Converter(SimConverter): - def make_initial_value(self, *args, **kwargs) -> T: - return initial_value +def _create_soft_signal(name: str, datatype: Type[T]) -> SignalRW[T]: + return SignalRW( + SimSignalBackend( + datatype, f"sim://oav_with_pin_tip_detection_soft_params:{name}" + ) + ) - class _SimSignalBackend(SimSignalBackend): - async def connect(self) -> None: - self.converter = _Converter() - self._initial_value = self.converter.make_initial_value() - self._severity = 0 - await self.put(None) +class PinTipDetection(Readable, Device): + """ + A device which will read a single frame from an on-axis view and use that frame + to calculate the pin-tip offset (in pixels) of that frame. - backend = _SimSignalBackend( - datatype, f"sim://oav_with_pin_tip_detection_soft_params:{name}" - ) - signal: SignalRW[T] = SignalRW(backend) - return signal + Used for pin tip centring workflow. + Note that if the sample is off-screen, this class will return the centre as the "edge" + of the image. + """ -class PinTipDetection(Readable, Device): def __init__(self, prefix: str, name: str = ""): self._prefix: str = prefix self._name = name @@ -57,45 +54,25 @@ def __init__(self, prefix: str, name: str = ""): ) # Soft parameters for pin-tip detection. - self.timeout: SignalRW[float] = _create_soft_signal("timeout", float, 10.0) - self.preprocess: SignalRW[ - Callable[[np.ndarray], np.ndarray] - ] = _create_soft_signal( - "preprocess", - Callable[[np.ndarray], np.ndarray], - ArrayProcessingFunctions.identity(), + self.timeout: SignalRW[float] = _create_soft_signal("timeout", float) + self.preprocess: SignalRW[int] = _create_soft_signal("preprocess", int) + self.preprocess_ksize: SignalRW[int] = _create_soft_signal( + "preprocess_ksize", int ) - self.canny_upper: SignalRW[int] = _create_soft_signal("canny_upper", int, 100) - self.canny_lower: SignalRW[int] = _create_soft_signal("canny_lower", int, 50) - self.close_ksize: SignalRW[int] = _create_soft_signal("close_ksize", int, 5) - self.close_iterations: SignalRW[int] = _create_soft_signal( - "close_iterations", int, 5 - ) - self.scan_direction: SignalRW[int] = _create_soft_signal( - "scan_direction", int, 1 + self.preprocess_iterations: SignalRW[int] = _create_soft_signal( + "preprocess_iterations", int ) - self.min_tip_height: SignalRW[int] = _create_soft_signal( - "min_tip_height", int, 5 + self.canny_upper: SignalRW[int] = _create_soft_signal("canny_upper", int) + self.canny_lower: SignalRW[int] = _create_soft_signal("canny_lower", int) + self.close_ksize: SignalRW[int] = _create_soft_signal("close_ksize", int) + self.close_iterations: SignalRW[int] = _create_soft_signal( + "close_iterations", int ) + self.scan_direction: SignalRW[int] = _create_soft_signal("scan_direction", int) + self.min_tip_height: SignalRW[int] = _create_soft_signal("min_tip_height", int) super().__init__(name=name) - async def connect(self, sim: bool = False): - for signal in [self.array_data, self.oav_width, self.oav_height]: - await signal.connect(sim=sim) - - for soft_signal in [ - self.timeout, - self.preprocess, - self.canny_upper, - self.canny_lower, - self.close_ksize, - self.close_iterations, - self.scan_direction, - self.min_tip_height, - ]: - await soft_signal.connect(sim=False) # Soft signals don't need simulating. - async def _get_tip_position( self, ) -> Tuple[Tuple[Optional[int], Optional[int]], float]: @@ -105,8 +82,21 @@ async def _get_tip_position( Returns tuple of: ((tip_x, tip_y), timestamp) """ + + preprocess_key = await self.preprocess.get_value() + preprocess_iter = await self.preprocess_iterations.get_value() + preprocess_ksize = await self.preprocess_ksize.get_value() + + try: + preprocess_func = ARRAY_PROCESSING_FUNCTIONS_MAP[preprocess_key]( + iter=preprocess_iter, ksize=preprocess_ksize + ) + except KeyError: + LOGGER.error("Invalid preprocessing function, using identity") + preprocess_func = ArrayProcessingFunctions.identity() + sample_detection = MxSampleDetect( - preprocess=await self.preprocess.get_value(), + preprocess=preprocess_func, canny_lower=await self.canny_lower.get_value(), canny_upper=await self.canny_upper.get_value(), close_ksize=await self.close_ksize.get_value(), @@ -158,6 +148,21 @@ async def _get_tip_position( return (tip_x, tip_y), timestamp + async def connect(self, sim: bool = False): + await super().connect(sim) + + # Set defaults for soft parameters + await self.timeout.set(10.0) + await self.canny_upper.set(100) + await self.canny_lower.set(50) + await self.close_iterations.set(5) + await self.close_ksize.set(5) + await self.scan_direction.set(1) + await self.min_tip_height.set(5) + await self.preprocess.set(10) # Identity function + await self.preprocess_iterations.set(5) + await self.preprocess_ksize.set(5) + async def read(self) -> dict[str, Reading]: tip_pos, timestamp = await asyncio.wait_for( self._get_tip_position(), timeout=await self.timeout.get_value() diff --git a/src/dodal/devices/oav/pin_tip_detection/pin_tip_detect_utils.py b/src/dodal/devices/oav/pin_tip_detection/pin_tip_detect_utils.py index 9b46b35ccd..9651740cab 100644 --- a/src/dodal/devices/oav/pin_tip_detection/pin_tip_detect_utils.py +++ b/src/dodal/devices/oav/pin_tip_detection/pin_tip_detect_utils.py @@ -14,7 +14,7 @@ class ArrayProcessingFunctions: """ @staticmethod - def identity() -> Callable[[np.ndarray], np.ndarray]: + def identity(*args, **kwargs) -> Callable[[np.ndarray], np.ndarray]: return lambda arr: arr @staticmethod @@ -67,23 +67,40 @@ def black_hat(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray] ) @staticmethod - def blur(ksize: int) -> Callable[[np.ndarray], np.ndarray]: + def blur(ksize: int, *args, **kwargs) -> Callable[[np.ndarray], np.ndarray]: return lambda arr: cv2.blur(arr, ksize=(ksize, ksize)) @staticmethod - def gaussian_blur(ksize: int) -> Callable[[np.ndarray], np.ndarray]: + def gaussian_blur( + ksize: int, *args, **kwargs + ) -> Callable[[np.ndarray], np.ndarray]: # Kernel size should be odd. if not ksize % 2: ksize += 1 return lambda arr: cv2.GaussianBlur(arr, (ksize, ksize), 0) @staticmethod - def median_blur(ksize: int) -> Callable[[np.ndarray], np.ndarray]: + def median_blur(ksize: int, *args, **kwargs) -> Callable[[np.ndarray], np.ndarray]: if not ksize % 2: ksize += 1 return lambda arr: cv2.medianBlur(arr, ksize) +ARRAY_PROCESSING_FUNCTIONS_MAP = { + 0: ArrayProcessingFunctions.erode, + 1: ArrayProcessingFunctions.dilate, + 2: ArrayProcessingFunctions.open_morph, + 3: ArrayProcessingFunctions.close, + 4: ArrayProcessingFunctions.gradient, + 5: ArrayProcessingFunctions.top_hat, + 6: ArrayProcessingFunctions.black_hat, + 7: ArrayProcessingFunctions.blur, + 8: ArrayProcessingFunctions.gaussian_blur, + 9: ArrayProcessingFunctions.median_blur, + 10: ArrayProcessingFunctions.identity, +} + + # A substitute for "None" which can fit into an numpy int array. # Also used as a substitute for a not-found sample position. NONE_VALUE: Final[int] = -1 @@ -153,12 +170,9 @@ def processArray(self, arr: np.ndarray) -> SampleLocation: # Preprocess the array. (Use the greyscale one.) pp_arr = self.preprocess(gray_arr) - # (Could do a remove_dirt step here if wanted.) - # Find some edges. edge_arr = cv2.Canny(pp_arr, self.canny_upper, self.canny_lower) - # Do a "close" image operation. (Add other options?) closed_arr = ArrayProcessingFunctions.close( self.close_ksize, self.close_iterations )(edge_arr) diff --git a/src/dodal/utils.py b/src/dodal/utils.py index d5c101a9d4..0016f78948 100644 --- a/src/dodal/utils.py +++ b/src/dodal/utils.py @@ -159,11 +159,7 @@ def collect_factories(module: ModuleType) -> dict[str, AnyDeviceFactory]: factories: dict[str, AnyDeviceFactory] = {} for var in module.__dict__.values(): - if ( - callable(var) - and _is_any_device_factory(var) - and not _is_device_skipped(var) - ): + if callable(var) and is_any_device_factory(var) and not _is_device_skipped(var): factories[var.__name__] = var return factories @@ -175,33 +171,33 @@ def _is_device_skipped(func: AnyDeviceFactory) -> bool: return func.__skip__ # type: ignore -def _is_v1_device_factory(func: Callable) -> bool: +def is_v1_device_factory(func: Callable) -> bool: try: return_type = signature(func).return_annotation - return _is_v1_device_type(return_type) + return is_v1_device_type(return_type) except ValueError: return False -def _is_v2_device_factory(func: Callable) -> bool: +def is_v2_device_factory(func: Callable) -> bool: try: return_type = signature(func).return_annotation - return _is_v2_device_type(return_type) + return is_v2_device_type(return_type) except ValueError: return False -def _is_any_device_factory(func: Callable) -> bool: - return _is_v1_device_factory(func) or _is_v2_device_factory(func) +def is_any_device_factory(func: Callable) -> bool: + return is_v1_device_factory(func) or is_v2_device_factory(func) -def _is_v2_device_type(obj: Type[Any]) -> bool: +def is_v2_device_type(obj: Type[Any]) -> bool: return inspect.isclass(obj) and issubclass(obj, OphydV2Device) -def _is_v1_device_type(obj: Type[Any]) -> bool: +def is_v1_device_type(obj: Type[Any]) -> bool: is_class = inspect.isclass(obj) follows_protocols = any( map(lambda protocol: isinstance(obj, protocol), BLUESKY_PROTOCOLS) ) - return is_class and follows_protocols and not _is_v2_device_type(obj) + return is_class and follows_protocols and not is_v2_device_type(obj) diff --git a/tests/devices/unit_tests/oav/pin_tip_detection/test_pin_tip_detect.py b/tests/devices/unit_tests/oav/pin_tip_detection/test_pin_tip_detect.py index 2855691bda..d97f75ce8a 100644 --- a/tests/devices/unit_tests/oav/pin_tip_detection/test_pin_tip_detect.py +++ b/tests/devices/unit_tests/oav/pin_tip_detection/test_pin_tip_detect.py @@ -41,6 +41,9 @@ async def test_soft_parameter_defaults_are_correct(): assert await device.close_iterations.get_value() == 5 assert await device.min_tip_height.get_value() == 5 assert await device.scan_direction.get_value() == 1 + assert await device.preprocess.get_value() == 10 + assert await device.preprocess_iterations.get_value() == 5 + assert await device.preprocess_ksize.get_value() == 5 @pytest.mark.asyncio @@ -64,26 +67,6 @@ async def test_numeric_soft_parameters_can_be_changed(): assert await device.scan_direction.get_value() == -1 -@pytest.mark.asyncio -async def test_preprocessing_soft_parameter_can_be_changed(): - device = await _get_pin_tip_detection_device() - - my_func_called = False - - def my_preprocessing_func(arr: NDArray) -> NDArray: - nonlocal my_func_called - my_func_called = True - return arr - - await device.preprocess.set(my_preprocessing_func) - - f = (await device.preprocess.read())[""]["value"] - - f(None) - - assert my_func_called - - @pytest.mark.parametrize( "input_array,height,width,reshaped", [ diff --git a/tests/devices/unit_tests/oav/pin_tip_detection/test_pin_tip_detect_utils.py b/tests/devices/unit_tests/oav/pin_tip_detection/test_pin_tip_detect_utils.py index 35510f2bda..803d944995 100644 --- a/tests/devices/unit_tests/oav/pin_tip_detection/test_pin_tip_detect_utils.py +++ b/tests/devices/unit_tests/oav/pin_tip_detection/test_pin_tip_detect_utils.py @@ -123,7 +123,6 @@ def test_locate_sample_tip_off_screen(direction, x_centre): ) # Currently sample "off screen" is not considered an error so a centre is still returned. - # Maybe it should be? assert location.tip_x == x_centre assert location.tip_y == 2 From ee7c73284b8e2cbbc5296b37c39bc6d5c7a4ba5b Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 18 Aug 2023 10:23:32 +0100 Subject: [PATCH 20/30] Add enum --- .../pin_tip_detection/oav_with_pin_tip_detection.py | 3 ++- .../oav/pin_tip_detection/pin_tip_detect_utils.py | 13 +++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py b/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py index 0e6c9682b0..1a06ddfe52 100644 --- a/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py +++ b/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py @@ -13,6 +13,7 @@ ARRAY_PROCESSING_FUNCTIONS_MAP, ArrayProcessingFunctions, MxSampleDetect, + ScanDirections, ) from dodal.log import LOGGER @@ -157,7 +158,7 @@ async def connect(self, sim: bool = False): await self.canny_lower.set(50) await self.close_iterations.set(5) await self.close_ksize.set(5) - await self.scan_direction.set(1) + await self.scan_direction.set(ScanDirections.FORWARD.value) await self.min_tip_height.set(5) await self.preprocess.set(10) # Identity function await self.preprocess_iterations.set(5) diff --git a/src/dodal/devices/oav/pin_tip_detection/pin_tip_detect_utils.py b/src/dodal/devices/oav/pin_tip_detection/pin_tip_detect_utils.py index 9651740cab..fc49b5a62f 100644 --- a/src/dodal/devices/oav/pin_tip_detection/pin_tip_detect_utils.py +++ b/src/dodal/devices/oav/pin_tip_detection/pin_tip_detect_utils.py @@ -1,4 +1,5 @@ from dataclasses import dataclass +from enum import Enum from typing import Callable, Final, Optional, Tuple import cv2 @@ -7,6 +8,11 @@ from dodal.log import LOGGER +class ScanDirections(Enum): + FORWARD = 1 + REVERSE = -1 + + class ArrayProcessingFunctions: """ Utility class for creating array preprocessing functions (arr -> arr with no additional parameters) @@ -151,7 +157,10 @@ def __init__( self.close_ksize = close_ksize self.close_iterations = close_iterations - if scan_direction not in [1, -1]: + if scan_direction not in [ + ScanDirections.FORWARD.value, + ScanDirections.REVERSE.value, + ]: raise ValueError( "Invalid scan direction, expected +1 for left-to-right or -1 for right-to-left" ) @@ -235,7 +244,7 @@ def _locate_sample(self, edge_arr: np.ndarray) -> SampleLocation: ) # Choose our starting point - i.e. first column with non-narrow width for positive scan, last one for negative scan. - if self.scan_direction == 1: + if self.scan_direction == ScanDirections.FORWARD.value: start_column = int(column_indices_with_non_narrow_widths[0]) else: start_column = int(column_indices_with_non_narrow_widths[-1]) From 2d8f2de6e7958f891cb502c5942f910ce0c3ef75 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 18 Aug 2023 16:57:57 +0100 Subject: [PATCH 21/30] Address review comments --- src/dodal/beamlines/beamline_utils.py | 10 +- src/dodal/beamlines/i23.py | 4 +- .../__init__.py} | 54 ++----- .../oav/pin_image_recognition/manual_test.py | 32 ++++ .../utils.py} | 149 +++++++++--------- .../devices/oav/pin_tip_detection/__init__.py | 0 src/dodal/utils.py | 6 +- .../test_pin_tip_detect.py | 4 +- .../test_pin_tip_detect_utils.py | 2 +- 9 files changed, 129 insertions(+), 132 deletions(-) rename src/dodal/devices/oav/{pin_tip_detection/oav_with_pin_tip_detection.py => pin_image_recognition/__init__.py} (79%) create mode 100644 src/dodal/devices/oav/pin_image_recognition/manual_test.py rename src/dodal/devices/oav/{pin_tip_detection/pin_tip_detect_utils.py => pin_image_recognition/utils.py} (64%) delete mode 100644 src/dodal/devices/oav/pin_tip_detection/__init__.py rename tests/devices/unit_tests/oav/{pin_tip_detection => image_recognition}/test_pin_tip_detect.py (96%) rename tests/devices/unit_tests/oav/{pin_tip_detection => image_recognition}/test_pin_tip_detect_utils.py (98%) diff --git a/src/dodal/beamlines/beamline_utils.py b/src/dodal/beamlines/beamline_utils.py index 7512b25daa..b6e0471e64 100644 --- a/src/dodal/beamlines/beamline_utils.py +++ b/src/dodal/beamlines/beamline_utils.py @@ -1,4 +1,5 @@ import asyncio +import inspect from typing import Callable, Dict, Final, List, Optional, TypeVar, cast from bluesky.run_engine import call_in_bluesky_event_loop @@ -39,7 +40,7 @@ def list_active_devices() -> List[str]: def active_device_is_same_type( active_device: AnyDevice, device: Callable[..., AnyDevice] ) -> bool: - return type(active_device) == device + return inspect.isclass(device) and isinstance(active_device, device) def _wait_for_connection( @@ -91,10 +92,9 @@ def device_instantiation( Returns: The instance of the device. """ - device_instance: T already_existing_device: Optional[AnyDevice] = ACTIVE_DEVICES.get(name) if fake: - device_factory = make_fake_device(device_factory) + device_factory = cast(Callable[..., T], make_fake_device(device_factory)) if already_existing_device is None: device_instance = device_factory( name=name, @@ -114,9 +114,7 @@ def device_instantiation( f"name as an existing device. Device name '{name}' already used for " f"a(n) {type(already_existing_device)}." ) - else: - # We have manually checked types - device_instance = cast(T, ACTIVE_DEVICES[name]) + device_instance = cast(T, already_existing_device) if post_create: post_create(device_instance) return device_instance diff --git a/src/dodal/beamlines/i23.py b/src/dodal/beamlines/i23.py index dbb913cc3c..a1922f1954 100644 --- a/src/dodal/beamlines/i23.py +++ b/src/dodal/beamlines/i23.py @@ -1,9 +1,7 @@ from dodal.beamlines.beamline_utils import device_instantiation from dodal.beamlines.beamline_utils import set_beamline as set_utils_beamline from dodal.devices.i23.gonio import Gonio -from dodal.devices.oav.pin_tip_detection.oav_with_pin_tip_detection import ( - PinTipDetection, -) +from dodal.devices.oav.pin_image_recognition import PinTipDetection from dodal.log import LOGGER from dodal.log import set_beamline as set_log_beamline from dodal.utils import get_beamline_name diff --git a/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py b/src/dodal/devices/oav/pin_image_recognition/__init__.py similarity index 79% rename from src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py rename to src/dodal/devices/oav/pin_image_recognition/__init__.py index 1a06ddfe52..45ed6c7899 100644 --- a/src/dodal/devices/oav/pin_tip_detection/oav_with_pin_tip_detection.py +++ b/src/dodal/devices/oav/pin_image_recognition/__init__.py @@ -9,9 +9,9 @@ from ophyd.v2.core import Device, Readable, Reading, SimSignalBackend from ophyd.v2.epics import SignalR, SignalRW, epics_signal_r -from dodal.devices.oav.pin_tip_detection.pin_tip_detect_utils import ( +from dodal.devices.oav.pin_image_recognition.utils import ( ARRAY_PROCESSING_FUNCTIONS_MAP, - ArrayProcessingFunctions, + identity, MxSampleDetect, ScanDirections, ) @@ -20,7 +20,7 @@ T = TypeVar("T") -def _create_soft_signal(name: str, datatype: Type[T]) -> SignalRW[T]: +def _create_soft_signal(datatype: Type[T], name: str) -> SignalRW[T]: return SignalRW( SimSignalBackend( datatype, f"sim://oav_with_pin_tip_detection_soft_params:{name}" @@ -55,22 +55,22 @@ def __init__(self, prefix: str, name: str = ""): ) # Soft parameters for pin-tip detection. - self.timeout: SignalRW[float] = _create_soft_signal("timeout", float) - self.preprocess: SignalRW[int] = _create_soft_signal("preprocess", int) + self.timeout: SignalRW[float] = _create_soft_signal(float, "timeout") + self.preprocess: SignalRW[int] = _create_soft_signal(int, "preprocess") self.preprocess_ksize: SignalRW[int] = _create_soft_signal( - "preprocess_ksize", int + int, "preprocess_ksize" ) self.preprocess_iterations: SignalRW[int] = _create_soft_signal( - "preprocess_iterations", int + int, "preprocess_iterations" ) - self.canny_upper: SignalRW[int] = _create_soft_signal("canny_upper", int) - self.canny_lower: SignalRW[int] = _create_soft_signal("canny_lower", int) - self.close_ksize: SignalRW[int] = _create_soft_signal("close_ksize", int) + self.canny_upper: SignalRW[int] = _create_soft_signal(int, "canny_upper") + self.canny_lower: SignalRW[int] = _create_soft_signal(int, "canny_lower") + self.close_ksize: SignalRW[int] = _create_soft_signal(int, "close_ksize") self.close_iterations: SignalRW[int] = _create_soft_signal( - "close_iterations", int + int, "close_iterations" ) - self.scan_direction: SignalRW[int] = _create_soft_signal("scan_direction", int) - self.min_tip_height: SignalRW[int] = _create_soft_signal("min_tip_height", int) + self.scan_direction: SignalRW[int] = _create_soft_signal(int, "scan_direction") + self.min_tip_height: SignalRW[int] = _create_soft_signal(int, "min_tip_height") super().__init__(name=name) @@ -94,7 +94,7 @@ async def _get_tip_position( ) except KeyError: LOGGER.error("Invalid preprocessing function, using identity") - preprocess_func = ArrayProcessingFunctions.identity() + preprocess_func = identity() sample_detection = MxSampleDetect( preprocess=preprocess_func, @@ -181,34 +181,10 @@ async def describe(self) -> dict[str, Descriptor]: ( self._name, { - "source": "pva://{}PVA:ARRAY".format(self._prefix), + "source": f"pva://{self._prefix}PVA:ARRAY", "dtype": "number", "shape": [2], # Tuple of (x, y) tip position }, ) ], ) - - -if __name__ == "__main__": - x = PinTipDetection(prefix="BL03I-DI-OAV-01:", name="edgeDetect") - - async def acquire(): - await x.connect() - img = await x.array_data.read() - tip = await x.read() - return img, tip - - img, tip = asyncio.get_event_loop().run_until_complete( - asyncio.wait_for(acquire(), timeout=10) - ) - print(tip) - print("Tip: {}".format(tip["edgeDetect"]["value"])) - - try: - import matplotlib.pyplot as plt - - plt.imshow(img[""]["value"].reshape(768, 1024, 3)) - plt.show() - except ImportError: - print("matplotlib not available; cannot show acquired image") diff --git a/src/dodal/devices/oav/pin_image_recognition/manual_test.py b/src/dodal/devices/oav/pin_image_recognition/manual_test.py new file mode 100644 index 0000000000..6b5bd70442 --- /dev/null +++ b/src/dodal/devices/oav/pin_image_recognition/manual_test.py @@ -0,0 +1,32 @@ +""" +Note: this file exists exclusively to make it easier to manually run +image recognition on a beamline without setting up all of the +bluesky infrastructure. + +It is otherwise unused. +""" +import asyncio +from dodal.devices.oav.pin_image_recognition import PinTipDetection + +if __name__ == "__main__": + x = PinTipDetection(prefix="BL03I-DI-OAV-01:", name="edgeDetect") + + async def acquire(): + await x.connect() + img = await x.array_data.read() + tip = await x.read() + return img, tip + + img, tip = asyncio.get_event_loop().run_until_complete( + asyncio.wait_for(acquire(), timeout=10) + ) + print(tip) + print("Tip: {}".format(tip["edgeDetect"]["value"])) + + try: + import matplotlib.pyplot as plt + + plt.imshow(img[""]["value"].reshape(768, 1024, 3)) + plt.show() + except ImportError: + print("matplotlib not available; cannot show acquired image") \ No newline at end of file diff --git a/src/dodal/devices/oav/pin_tip_detection/pin_tip_detect_utils.py b/src/dodal/devices/oav/pin_image_recognition/utils.py similarity index 64% rename from src/dodal/devices/oav/pin_tip_detection/pin_tip_detect_utils.py rename to src/dodal/devices/oav/pin_image_recognition/utils.py index fc49b5a62f..8b561b25b0 100644 --- a/src/dodal/devices/oav/pin_tip_detection/pin_tip_detect_utils.py +++ b/src/dodal/devices/oav/pin_image_recognition/utils.py @@ -13,97 +13,90 @@ class ScanDirections(Enum): REVERSE = -1 -class ArrayProcessingFunctions: - """ - Utility class for creating array preprocessing functions (arr -> arr with no additional parameters) - for some common operations. - """ +def identity(*args, **kwargs) -> Callable[[np.ndarray], np.ndarray]: + return lambda arr: arr - @staticmethod - def identity(*args, **kwargs) -> Callable[[np.ndarray], np.ndarray]: - return lambda arr: arr - @staticmethod - def erode(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: - element = cv2.getStructuringElement(cv2.MORPH_RECT, (ksize, ksize)) - return lambda arr: cv2.erode(arr, element, iterations=iterations) +def erode(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: + element = cv2.getStructuringElement(cv2.MORPH_RECT, (ksize, ksize)) + return lambda arr: cv2.erode(arr, element, iterations=iterations) - @staticmethod - def dilate(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: - element = cv2.getStructuringElement(cv2.MORPH_RECT, (ksize, ksize)) - return lambda arr: cv2.dilate(arr, element, iterations=iterations) - @staticmethod - def _morph( - ksize: int, iterations: int, morph_type: int - ) -> Callable[[np.ndarray], np.ndarray]: - element = cv2.getStructuringElement(cv2.MORPH_RECT, (ksize, ksize)) - return lambda arr: cv2.morphologyEx( - arr, morph_type, element, iterations=iterations - ) +def dilate(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: + element = cv2.getStructuringElement(cv2.MORPH_RECT, (ksize, ksize)) + return lambda arr: cv2.dilate(arr, element, iterations=iterations) - @staticmethod - def open_morph(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: - return ArrayProcessingFunctions._morph( - ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_OPEN - ) - @staticmethod - def close(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: - return ArrayProcessingFunctions._morph( - ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_CLOSE - ) +def _morph( + ksize: int, iterations: int, morph_type: int +) -> Callable[[np.ndarray], np.ndarray]: + element = cv2.getStructuringElement(cv2.MORPH_RECT, (ksize, ksize)) + return lambda arr: cv2.morphologyEx( + arr, morph_type, element, iterations=iterations + ) - @staticmethod - def gradient(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: - return ArrayProcessingFunctions._morph( - ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_GRADIENT - ) - @staticmethod - def top_hat(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: - return ArrayProcessingFunctions._morph( - ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_TOPHAT - ) +def open_morph(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: + return _morph( + ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_OPEN + ) - @staticmethod - def black_hat(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: - return ArrayProcessingFunctions._morph( - ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_BLACKHAT - ) - @staticmethod - def blur(ksize: int, *args, **kwargs) -> Callable[[np.ndarray], np.ndarray]: - return lambda arr: cv2.blur(arr, ksize=(ksize, ksize)) +def close(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: + return _morph( + ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_CLOSE + ) - @staticmethod - def gaussian_blur( - ksize: int, *args, **kwargs - ) -> Callable[[np.ndarray], np.ndarray]: - # Kernel size should be odd. - if not ksize % 2: - ksize += 1 - return lambda arr: cv2.GaussianBlur(arr, (ksize, ksize), 0) - @staticmethod - def median_blur(ksize: int, *args, **kwargs) -> Callable[[np.ndarray], np.ndarray]: - if not ksize % 2: - ksize += 1 - return lambda arr: cv2.medianBlur(arr, ksize) +def gradient(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: + return _morph( + ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_GRADIENT + ) + + +def top_hat(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: + return _morph( + ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_TOPHAT + ) + + +def black_hat(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: + return _morph( + ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_BLACKHAT + ) + + +def blur(ksize: int, *args, **kwargs) -> Callable[[np.ndarray], np.ndarray]: + return lambda arr: cv2.blur(arr, ksize=(ksize, ksize)) + + +def gaussian_blur( + ksize: int, *args, **kwargs +) -> Callable[[np.ndarray], np.ndarray]: + # Kernel size should be odd. + if not ksize % 2: + ksize += 1 + return lambda arr: cv2.GaussianBlur(arr, (ksize, ksize), 0) + + +def median_blur(ksize: int, *args, **kwargs) -> Callable[[np.ndarray], np.ndarray]: + if not ksize % 2: + ksize += 1 + return lambda arr: cv2.medianBlur(arr, ksize) ARRAY_PROCESSING_FUNCTIONS_MAP = { - 0: ArrayProcessingFunctions.erode, - 1: ArrayProcessingFunctions.dilate, - 2: ArrayProcessingFunctions.open_morph, - 3: ArrayProcessingFunctions.close, - 4: ArrayProcessingFunctions.gradient, - 5: ArrayProcessingFunctions.top_hat, - 6: ArrayProcessingFunctions.black_hat, - 7: ArrayProcessingFunctions.blur, - 8: ArrayProcessingFunctions.gaussian_blur, - 9: ArrayProcessingFunctions.median_blur, - 10: ArrayProcessingFunctions.identity, + 0: erode, + 1: dilate, + 2: open_morph, + 3: close, + 4: gradient, + 5: top_hat, + 6: black_hat, + 7: blur, + 8: gaussian_blur, + 9: median_blur, + 10: identity, } @@ -141,7 +134,7 @@ def __init__( Args: preprocess: A preprocessing function applied to the array after conversion to grayscale. - See implementations of common functions in ArrayProcessingFunctions for predefined conversions + See implementations of common functions above for predefined conversions Defaults to a no-op (i.e. no preprocessing) canny_upper: upper threshold for canny edge detection canny_lower: lower threshold for canny edge detection @@ -182,7 +175,7 @@ def processArray(self, arr: np.ndarray) -> SampleLocation: # Find some edges. edge_arr = cv2.Canny(pp_arr, self.canny_upper, self.canny_lower) - closed_arr = ArrayProcessingFunctions.close( + closed_arr = close( self.close_ksize, self.close_iterations )(edge_arr) diff --git a/src/dodal/devices/oav/pin_tip_detection/__init__.py b/src/dodal/devices/oav/pin_tip_detection/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/src/dodal/utils.py b/src/dodal/utils.py index 0016f78948..b4ea595855 100644 --- a/src/dodal/utils.py +++ b/src/dodal/utils.py @@ -56,10 +56,10 @@ Triggerable, ] -AnyDevice: TypeAlias = OphydV1Device | OphydV2Device +AnyDevice: TypeAlias = Union[OphydV1Device, OphydV2Device] V1DeviceFactory: TypeAlias = Callable[..., OphydV1Device] V2DeviceFactory: TypeAlias = Callable[..., OphydV2Device] -AnyDeviceFactory: TypeAlias = V1DeviceFactory | V2DeviceFactory +AnyDeviceFactory: TypeAlias = Union[V1DeviceFactory, V2DeviceFactory] def get_beamline_name(default: str) -> str: @@ -99,7 +99,7 @@ def wrapper(*args, **kwds) -> T: def make_all_devices( module: Union[str, ModuleType, None] = None, **kwargs -) -> dict[str, AnyDevice]: +) -> Dict[str, AnyDevice]: """Makes all devices in the given beamline module. In cases of device interdependencies it ensures a device is created before any which diff --git a/tests/devices/unit_tests/oav/pin_tip_detection/test_pin_tip_detect.py b/tests/devices/unit_tests/oav/image_recognition/test_pin_tip_detect.py similarity index 96% rename from tests/devices/unit_tests/oav/pin_tip_detection/test_pin_tip_detect.py rename to tests/devices/unit_tests/oav/image_recognition/test_pin_tip_detect.py index d97f75ce8a..8fed78ae70 100644 --- a/tests/devices/unit_tests/oav/pin_tip_detection/test_pin_tip_detect.py +++ b/tests/devices/unit_tests/oav/image_recognition/test_pin_tip_detect.py @@ -6,11 +6,11 @@ from numpy.typing import NDArray from ophyd.v2.core import set_sim_value -from dodal.devices.oav.pin_tip_detection.oav_with_pin_tip_detection import ( +from dodal.devices.oav.pin_image_recognition import ( MxSampleDetect, PinTipDetection, ) -from dodal.devices.oav.pin_tip_detection.pin_tip_detect_utils import SampleLocation +from dodal.devices.oav.pin_image_recognition.utils import SampleLocation EVENT_LOOP = asyncio.new_event_loop() diff --git a/tests/devices/unit_tests/oav/pin_tip_detection/test_pin_tip_detect_utils.py b/tests/devices/unit_tests/oav/image_recognition/test_pin_tip_detect_utils.py similarity index 98% rename from tests/devices/unit_tests/oav/pin_tip_detection/test_pin_tip_detect_utils.py rename to tests/devices/unit_tests/oav/image_recognition/test_pin_tip_detect_utils.py index 803d944995..c15d05f1e7 100644 --- a/tests/devices/unit_tests/oav/pin_tip_detection/test_pin_tip_detect_utils.py +++ b/tests/devices/unit_tests/oav/image_recognition/test_pin_tip_detect_utils.py @@ -1,7 +1,7 @@ import numpy as np import pytest -from dodal.devices.oav.pin_tip_detection.pin_tip_detect_utils import ( +from dodal.devices.oav.pin_image_recognition.utils import ( NONE_VALUE, MxSampleDetect, ) From 67a119ed5a833d316a5d717437b85db933e35c9c Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 18 Aug 2023 17:07:48 +0100 Subject: [PATCH 22/30] Linters --- .../oav/pin_image_recognition/__init__.py | 2 +- .../oav/pin_image_recognition/manual_test.py | 3 +- .../oav/pin_image_recognition/utils.py | 32 +++++-------------- 3 files changed, 11 insertions(+), 26 deletions(-) diff --git a/src/dodal/devices/oav/pin_image_recognition/__init__.py b/src/dodal/devices/oav/pin_image_recognition/__init__.py index 45ed6c7899..d20ccbfb1f 100644 --- a/src/dodal/devices/oav/pin_image_recognition/__init__.py +++ b/src/dodal/devices/oav/pin_image_recognition/__init__.py @@ -11,9 +11,9 @@ from dodal.devices.oav.pin_image_recognition.utils import ( ARRAY_PROCESSING_FUNCTIONS_MAP, - identity, MxSampleDetect, ScanDirections, + identity, ) from dodal.log import LOGGER diff --git a/src/dodal/devices/oav/pin_image_recognition/manual_test.py b/src/dodal/devices/oav/pin_image_recognition/manual_test.py index 6b5bd70442..2ca77b1c82 100644 --- a/src/dodal/devices/oav/pin_image_recognition/manual_test.py +++ b/src/dodal/devices/oav/pin_image_recognition/manual_test.py @@ -6,6 +6,7 @@ It is otherwise unused. """ import asyncio + from dodal.devices.oav.pin_image_recognition import PinTipDetection if __name__ == "__main__": @@ -29,4 +30,4 @@ async def acquire(): plt.imshow(img[""]["value"].reshape(768, 1024, 3)) plt.show() except ImportError: - print("matplotlib not available; cannot show acquired image") \ No newline at end of file + print("matplotlib not available; cannot show acquired image") diff --git a/src/dodal/devices/oav/pin_image_recognition/utils.py b/src/dodal/devices/oav/pin_image_recognition/utils.py index 8b561b25b0..42f798e7ff 100644 --- a/src/dodal/devices/oav/pin_image_recognition/utils.py +++ b/src/dodal/devices/oav/pin_image_recognition/utils.py @@ -31,48 +31,34 @@ def _morph( ksize: int, iterations: int, morph_type: int ) -> Callable[[np.ndarray], np.ndarray]: element = cv2.getStructuringElement(cv2.MORPH_RECT, (ksize, ksize)) - return lambda arr: cv2.morphologyEx( - arr, morph_type, element, iterations=iterations - ) + return lambda arr: cv2.morphologyEx(arr, morph_type, element, iterations=iterations) def open_morph(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: - return _morph( - ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_OPEN - ) + return _morph(ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_OPEN) def close(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: - return _morph( - ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_CLOSE - ) + return _morph(ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_CLOSE) def gradient(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: - return _morph( - ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_GRADIENT - ) + return _morph(ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_GRADIENT) def top_hat(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: - return _morph( - ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_TOPHAT - ) + return _morph(ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_TOPHAT) def black_hat(ksize: int, iterations: int) -> Callable[[np.ndarray], np.ndarray]: - return _morph( - ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_BLACKHAT - ) + return _morph(ksize=ksize, iterations=iterations, morph_type=cv2.MORPH_BLACKHAT) def blur(ksize: int, *args, **kwargs) -> Callable[[np.ndarray], np.ndarray]: return lambda arr: cv2.blur(arr, ksize=(ksize, ksize)) -def gaussian_blur( - ksize: int, *args, **kwargs -) -> Callable[[np.ndarray], np.ndarray]: +def gaussian_blur(ksize: int, *args, **kwargs) -> Callable[[np.ndarray], np.ndarray]: # Kernel size should be odd. if not ksize % 2: ksize += 1 @@ -175,9 +161,7 @@ def processArray(self, arr: np.ndarray) -> SampleLocation: # Find some edges. edge_arr = cv2.Canny(pp_arr, self.canny_upper, self.canny_lower) - closed_arr = close( - self.close_ksize, self.close_iterations - )(edge_arr) + closed_arr = close(self.close_ksize, self.close_iterations)(edge_arr) # Find the sample. return self._locate_sample(closed_arr) From 72a6b62e9bdcd05dd5d07da2bac326af747202d1 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 18 Aug 2023 17:10:53 +0100 Subject: [PATCH 23/30] Linters, again. --- .../unit_tests/oav/image_recognition/test_pin_tip_detect.py | 5 +---- .../oav/image_recognition/test_pin_tip_detect_utils.py | 5 +---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/tests/devices/unit_tests/oav/image_recognition/test_pin_tip_detect.py b/tests/devices/unit_tests/oav/image_recognition/test_pin_tip_detect.py index 8fed78ae70..f03f746aee 100644 --- a/tests/devices/unit_tests/oav/image_recognition/test_pin_tip_detect.py +++ b/tests/devices/unit_tests/oav/image_recognition/test_pin_tip_detect.py @@ -6,10 +6,7 @@ from numpy.typing import NDArray from ophyd.v2.core import set_sim_value -from dodal.devices.oav.pin_image_recognition import ( - MxSampleDetect, - PinTipDetection, -) +from dodal.devices.oav.pin_image_recognition import MxSampleDetect, PinTipDetection from dodal.devices.oav.pin_image_recognition.utils import SampleLocation EVENT_LOOP = asyncio.new_event_loop() diff --git a/tests/devices/unit_tests/oav/image_recognition/test_pin_tip_detect_utils.py b/tests/devices/unit_tests/oav/image_recognition/test_pin_tip_detect_utils.py index c15d05f1e7..953623bde9 100644 --- a/tests/devices/unit_tests/oav/image_recognition/test_pin_tip_detect_utils.py +++ b/tests/devices/unit_tests/oav/image_recognition/test_pin_tip_detect_utils.py @@ -1,10 +1,7 @@ import numpy as np import pytest -from dodal.devices.oav.pin_image_recognition.utils import ( - NONE_VALUE, - MxSampleDetect, -) +from dodal.devices.oav.pin_image_recognition.utils import NONE_VALUE, MxSampleDetect def test_locate_sample_simple_forward(): From 11475dc689287510ffa13dc87a8880fa39fe757e Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 18 Aug 2023 17:18:27 +0100 Subject: [PATCH 24/30] Support older versions of python --- src/dodal/utils.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/dodal/utils.py b/src/dodal/utils.py index b4ea595855..2484e292d4 100644 --- a/src/dodal/utils.py +++ b/src/dodal/utils.py @@ -14,7 +14,6 @@ Mapping, Optional, Type, - TypeAlias, TypeVar, Union, ) @@ -38,6 +37,12 @@ from ophyd.device import Device as OphydV1Device from ophyd.v2.core import Device as OphydV2Device +try: + from typing import TypeAlias +except ImportError: + from typing_extensions import TypeAlias + + #: Protocols defining interface to hardware BLUESKY_PROTOCOLS = [ Checkable, From d400d81c1dd4685262c9ce496fd1e85b13190ea8 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 18 Aug 2023 17:24:03 +0100 Subject: [PATCH 25/30] Fix test --- src/dodal/devices/oav/pin_image_recognition/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dodal/devices/oav/pin_image_recognition/__init__.py b/src/dodal/devices/oav/pin_image_recognition/__init__.py index d20ccbfb1f..7ba04f4558 100644 --- a/src/dodal/devices/oav/pin_image_recognition/__init__.py +++ b/src/dodal/devices/oav/pin_image_recognition/__init__.py @@ -143,7 +143,7 @@ async def _get_tip_position( tip_x = location.tip_x tip_y = location.tip_y except Exception as e: - LOGGER.error("Failed to detect pin-tip location due to exception: ", e) + LOGGER.error("Failed to detect pin-tip location due to exception: {}", e) tip_x = None tip_y = None From be9399879fea28663a378c6f7e1528746c34c65a Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 18 Aug 2023 19:01:58 +0100 Subject: [PATCH 26/30] use f-string --- src/dodal/devices/oav/pin_image_recognition/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dodal/devices/oav/pin_image_recognition/__init__.py b/src/dodal/devices/oav/pin_image_recognition/__init__.py index 7ba04f4558..613c9a80f9 100644 --- a/src/dodal/devices/oav/pin_image_recognition/__init__.py +++ b/src/dodal/devices/oav/pin_image_recognition/__init__.py @@ -143,7 +143,7 @@ async def _get_tip_position( tip_x = location.tip_x tip_y = location.tip_y except Exception as e: - LOGGER.error("Failed to detect pin-tip location due to exception: {}", e) + LOGGER.error(f"Failed to detect pin-tip location due to exception: {e}") tip_x = None tip_y = None From 927f3de0d15f1d7ec2e277cb75d593b6648daa80 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Mon, 21 Aug 2023 10:05:31 +0100 Subject: [PATCH 27/30] Add requested test for passing through connection timeouts. --- .../unit_tests/test_beamline_utils.py | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/beamlines/unit_tests/test_beamline_utils.py b/tests/beamlines/unit_tests/test_beamline_utils.py index e2516aa99f..e949c3b8a9 100644 --- a/tests/beamlines/unit_tests/test_beamline_utils.py +++ b/tests/beamlines/unit_tests/test_beamline_utils.py @@ -1,6 +1,10 @@ +from unittest.mock import ANY, MagicMock, patch + import pytest from ophyd import Device +from ophyd.device import Device as OphydV1Device from ophyd.sim import FakeEpicsSignal +from ophyd.v2.core import Device as OphydV2Device from dodal.beamlines import beamline_utils, i03 from dodal.devices.aperturescatterguard import ApertureScatterguard @@ -69,3 +73,31 @@ def _make_devices_and_get_id(): beamline_utils.clear_devices() ids_3 = [_make_devices_and_get_id()] assert ids_1 != ids_3 + + +@pytest.mark.parametrize( + "kwargs,expected_timeout", [({}, 5.0), ({"timeout": 15.0}, 15.0)] +) +def test_wait_for_v1_device_connection_passes_through_timeout(kwargs, expected_timeout): + device = OphydV1Device(name="") + device.wait_for_connection = MagicMock() + + beamline_utils._wait_for_connection(device, **kwargs) + + device.wait_for_connection.assert_called_once_with(timeout=expected_timeout) + + +@pytest.mark.parametrize( + "kwargs,expected_timeout", [({}, 5.0), ({"timeout": 15.0}, 15.0)] +) +@patch("dodal.beamlines.beamline_utils.call_in_bluesky_event_loop", autospec=True) +@patch("asyncio.wait_for", autospec=True) +def test_wait_for_v2_device_connection_passes_through_timeout( + wait_for, call_in_bluesky_el, kwargs, expected_timeout +): + device = OphydV2Device() + + beamline_utils._wait_for_connection(device, **kwargs) + + wait_for.assert_called_once_with(ANY, expected_timeout) + call_in_bluesky_el.assert_called_once() From bfeac3528df89fa5fc31bb6aa73a172d6a6cfa6d Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Mon, 21 Aug 2023 11:21:38 +0100 Subject: [PATCH 28/30] Address further review comments --- src/dodal/beamlines/i23.py | 10 ++----- .../oav/pin_image_recognition/__init__.py | 5 ++-- .../oav/pin_image_recognition/manual_test.py | 1 + .../image_recognition/test_pin_tip_detect.py | 30 ++++++++++++++++++- 4 files changed, 35 insertions(+), 11 deletions(-) diff --git a/src/dodal/beamlines/i23.py b/src/dodal/beamlines/i23.py index a1922f1954..5a00015ae2 100644 --- a/src/dodal/beamlines/i23.py +++ b/src/dodal/beamlines/i23.py @@ -2,9 +2,8 @@ from dodal.beamlines.beamline_utils import set_beamline as set_utils_beamline from dodal.devices.i23.gonio import Gonio from dodal.devices.oav.pin_image_recognition import PinTipDetection -from dodal.log import LOGGER from dodal.log import set_beamline as set_log_beamline -from dodal.utils import get_beamline_name +from dodal.utils import get_beamline_name, get_hostname, skip_device BL = get_beamline_name("i23") set_log_beamline(BL) @@ -22,17 +21,12 @@ def gonio(wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False) - ) +@skip_device(lambda: not get_hostname().startswith("i23-ws")) def oav_pin_tip_detection( wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False ) -> PinTipDetection: """Get the i23 OAV pin-tip detection device""" - if get_beamline_name("") != "i23": - LOGGER.warning( - "Not running on i23 - forcing pin tip detection into simulation mode" - ) - fake_with_ophyd_sim = True - return device_instantiation( PinTipDetection, "pin_tip_detection", diff --git a/src/dodal/devices/oav/pin_image_recognition/__init__.py b/src/dodal/devices/oav/pin_image_recognition/__init__.py index 613c9a80f9..a83f8f8b3e 100644 --- a/src/dodal/devices/oav/pin_image_recognition/__init__.py +++ b/src/dodal/devices/oav/pin_image_recognition/__init__.py @@ -35,8 +35,9 @@ class PinTipDetection(Readable, Device): Used for pin tip centring workflow. - Note that if the sample is off-screen, this class will return the centre as the "edge" - of the image. + Note that if the tip of the sample is off-screen, this class will return the centre as the "edge" + of the image. If the entire sample if off-screen (i.e. no suitable edges were detected at all) + then it will return (None, None). """ def __init__(self, prefix: str, name: str = ""): diff --git a/src/dodal/devices/oav/pin_image_recognition/manual_test.py b/src/dodal/devices/oav/pin_image_recognition/manual_test.py index 2ca77b1c82..16dd32b636 100644 --- a/src/dodal/devices/oav/pin_image_recognition/manual_test.py +++ b/src/dodal/devices/oav/pin_image_recognition/manual_test.py @@ -5,6 +5,7 @@ It is otherwise unused. """ +# pragma: no cover import asyncio from dodal.devices.oav.pin_image_recognition import PinTipDetection diff --git a/tests/devices/unit_tests/oav/image_recognition/test_pin_tip_detect.py b/tests/devices/unit_tests/oav/image_recognition/test_pin_tip_detect.py index f03f746aee..1990b7703d 100644 --- a/tests/devices/unit_tests/oav/image_recognition/test_pin_tip_detect.py +++ b/tests/devices/unit_tests/oav/image_recognition/test_pin_tip_detect.py @@ -1,5 +1,5 @@ import asyncio -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch import numpy as np import pytest @@ -54,6 +54,9 @@ async def test_numeric_soft_parameters_can_be_changed(): await device.close_iterations.set(20) await device.min_tip_height.set(25) await device.scan_direction.set(-1) + await device.preprocess.set(2) + await device.preprocess_ksize.set(3) + await device.preprocess_iterations.set(4) assert await device.timeout.get_value() == 100.0 assert await device.canny_lower.get_value() == 5 @@ -62,6 +65,31 @@ async def test_numeric_soft_parameters_can_be_changed(): assert await device.close_iterations.get_value() == 20 assert await device.min_tip_height.get_value() == 25 assert await device.scan_direction.get_value() == -1 + assert await device.preprocess.get_value() == 2 + assert await device.preprocess_ksize.get_value() == 3 + assert await device.preprocess_iterations.get_value() == 4 + + +@pytest.mark.asyncio +async def test_invalid_processing_func_uses_identity_function(): + device = await _get_pin_tip_detection_device() + + set_sim_value(device.preprocess, 50) # Invalid index + + with patch.object( + MxSampleDetect, "__init__", return_value=None + ) as mock_init, patch.object( + MxSampleDetect, "processArray", return_value=((None, None), None) + ): + await device.read() + + mock_init.assert_called_once() + + captured_func = mock_init.call_args[1]["preprocess"] + + # Assert captured preprocess function is the identitiy function + arg = object() + assert arg == captured_func(arg) @pytest.mark.parametrize( From 655255741efef858469384477a96f57cd986c4f3 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Tue, 22 Aug 2023 09:20:29 +0100 Subject: [PATCH 29/30] Address straggling review comments --- src/dodal/beamlines/i23.py | 11 ++++++++++- .../devices/oav/pin_image_recognition/manual_test.py | 1 - 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/dodal/beamlines/i23.py b/src/dodal/beamlines/i23.py index 5a00015ae2..3f12a82c00 100644 --- a/src/dodal/beamlines/i23.py +++ b/src/dodal/beamlines/i23.py @@ -10,6 +10,15 @@ set_utils_beamline(BL) +def _is_i23_machine(): + """ + Devices using PVA can only connect from i23 machines, due to the absence of + PVA gateways at present. + """ + hostname = get_hostname() + return hostname.startswith("i23-ws") or hostname.startswith("i23-control") + + def gonio(wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False) -> Gonio: """Get the i23 goniometer device""" return device_instantiation( @@ -21,7 +30,7 @@ def gonio(wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False) - ) -@skip_device(lambda: not get_hostname().startswith("i23-ws")) +@skip_device(lambda: not _is_i23_machine()) def oav_pin_tip_detection( wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False ) -> PinTipDetection: diff --git a/src/dodal/devices/oav/pin_image_recognition/manual_test.py b/src/dodal/devices/oav/pin_image_recognition/manual_test.py index 16dd32b636..2ca77b1c82 100644 --- a/src/dodal/devices/oav/pin_image_recognition/manual_test.py +++ b/src/dodal/devices/oav/pin_image_recognition/manual_test.py @@ -5,7 +5,6 @@ It is otherwise unused. """ -# pragma: no cover import asyncio from dodal.devices.oav.pin_image_recognition import PinTipDetection From be318f98681b4de38e01ea2de261de7a27f2eb06 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Tue, 22 Aug 2023 09:58:23 +0100 Subject: [PATCH 30/30] Start runengine to prevent test flakiness --- tests/beamlines/unit_tests/test_beamline_utils.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/beamlines/unit_tests/test_beamline_utils.py b/tests/beamlines/unit_tests/test_beamline_utils.py index e949c3b8a9..8f444ad43e 100644 --- a/tests/beamlines/unit_tests/test_beamline_utils.py +++ b/tests/beamlines/unit_tests/test_beamline_utils.py @@ -1,6 +1,7 @@ from unittest.mock import ANY, MagicMock, patch import pytest +from bluesky.run_engine import RunEngine as RE from ophyd import Device from ophyd.device import Device as OphydV1Device from ophyd.sim import FakeEpicsSignal @@ -95,6 +96,7 @@ def test_wait_for_v1_device_connection_passes_through_timeout(kwargs, expected_t def test_wait_for_v2_device_connection_passes_through_timeout( wait_for, call_in_bluesky_el, kwargs, expected_timeout ): + RE() device = OphydV2Device() beamline_utils._wait_for_connection(device, **kwargs)