From b315703faa52b41893f431f48a3ab8f07d8b8bb8 Mon Sep 17 00:00:00 2001
From: David Perl <david.perl@diamond.ac.uk>
Date: Tue, 14 Mar 2023 12:30:03 +0000
Subject: [PATCH 01/16] reorganise oav devices and rename snapshot to MJPG

but keep derivative devices named snapshot
---
 .../devices/areadetector/plugins/MJPG.py      | 42 +++++++++++++
 .../devices/areadetector/plugins/MXSC.py      | 59 ++++++++++++++++++
 src/dodal/devices/oav/grid_overlay.py         |  3 +-
 src/dodal/devices/oav/oav_detector.py         | 60 +------------------
 4 files changed, 103 insertions(+), 61 deletions(-)
 create mode 100644 src/dodal/devices/areadetector/plugins/MJPG.py
 create mode 100644 src/dodal/devices/areadetector/plugins/MXSC.py

diff --git a/src/dodal/devices/areadetector/plugins/MJPG.py b/src/dodal/devices/areadetector/plugins/MJPG.py
new file mode 100644
index 00000000000..d01240480f9
--- /dev/null
+++ b/src/dodal/devices/areadetector/plugins/MJPG.py
@@ -0,0 +1,42 @@
+import threading
+from pathlib import Path
+
+import requests
+from ophyd import Component, Device, DeviceStatus, EpicsSignal, EpicsSignalRO, Signal
+from PIL import Image
+
+
+class MJPG(Device):
+    filename: Signal = Component(Signal)
+    directory: Signal = Component(Signal)
+    url: EpicsSignal = Component(EpicsSignal, "JPG_URL_RBV", string=True)
+    x_size_pv: EpicsSignalRO = Component(EpicsSignalRO, "ArraySize1_RBV")
+    y_size_pv: EpicsSignalRO = Component(EpicsSignalRO, "ArraySize2_RBV")
+    input_rbpv: EpicsSignalRO = Component(EpicsSignalRO, "NDArrayPort_RBV")
+    input_pv: EpicsSignal = Component(EpicsSignal, "NDArrayPort")
+    KICKOFF_TIMEOUT: float = 10.0
+
+    def trigger(self):
+        st = DeviceStatus(device=self, timeout=self.KICKOFF_TIMEOUT)
+        url_str = self.url.get()
+        filename_str = self.filename.get()
+        directory_str = self.directory.get()
+
+        def get_snapshot():
+            try:
+                response = requests.get(url_str, stream=True)
+                response.raise_for_status()
+                image = Image.open(response.raw)
+                image_path = Path(f"{directory_str}/{filename_str}.png")
+                image.save(image_path)
+                self.post_processing(image)
+                st.set_finished()
+            except requests.HTTPError as e:
+                st.set_exception(e)
+
+        threading.Thread(target=get_snapshot, daemon=True).start()
+
+        return st
+
+    def post_processing(self, image: Image.Image):
+        pass
diff --git a/src/dodal/devices/areadetector/plugins/MXSC.py b/src/dodal/devices/areadetector/plugins/MXSC.py
new file mode 100644
index 00000000000..51bedbf38cf
--- /dev/null
+++ b/src/dodal/devices/areadetector/plugins/MXSC.py
@@ -0,0 +1,59 @@
+from dodal.devices.oav.grid_overlay import SnapshotWithGrid
+from dodal.devices.oav.oav_detector import ZoomController
+from ophyd import ADComponent as ADC
+from ophyd import (
+    AreaDetector,
+    CamBase,
+    Component,
+    Device,
+    EpicsSignal,
+    HDF5Plugin,
+    OverlayPlugin,
+    ProcessPlugin,
+    ROIPlugin,
+)
+
+
+class MXSC(Device):
+    """
+    Device for edge detection plugin.
+    """
+
+    input_plugin_pv: EpicsSignal = Component(EpicsSignal, "NDArrayPort")
+    enable_callbacks_pv: EpicsSignal = Component(EpicsSignal, "EnableCallbacks")
+    min_callback_time_pv: EpicsSignal = Component(EpicsSignal, "MinCallbackTime")
+    blocking_callbacks_pv: EpicsSignal = Component(EpicsSignal, "BlockingCallbacks")
+    read_file: EpicsSignal = Component(EpicsSignal, "ReadFile")
+    py_filename: EpicsSignal = Component(EpicsSignal, "Filename", string=True)
+    preprocess_operation: EpicsSignal = Component(EpicsSignal, "Preprocess")
+    preprocess_ksize: EpicsSignal = Component(EpicsSignal, "PpParam1")
+    canny_upper_threshold: EpicsSignal = Component(EpicsSignal, "CannyUpper")
+    canny_lower_threshold: EpicsSignal = Component(EpicsSignal, "CannyLower")
+    close_ksize: EpicsSignal = Component(EpicsSignal, "CloseKsize")
+    sample_detection_scan_direction: EpicsSignal = Component(
+        EpicsSignal, "ScanDirection"
+    )
+    sample_detection_min_tip_height: EpicsSignal = Component(
+        EpicsSignal, "MinTipHeight"
+    )
+    tip_x: EpicsSignal = Component(EpicsSignal, "TipX")
+    tip_y: EpicsSignal = Component(EpicsSignal, "TipY")
+    top: EpicsSignal = Component(EpicsSignal, "Top")
+    bottom: EpicsSignal = Component(EpicsSignal, "Bottom")
+    output_array: EpicsSignal = Component(EpicsSignal, "OutputArray")
+    draw_tip: EpicsSignal = Component(EpicsSignal, "DrawTip")
+    draw_edges: EpicsSignal = Component(EpicsSignal, "DrawEdges")
+    waveform_size_x: EpicsSignal = Component(EpicsSignal, "ArraySize1_RBV")
+    waveform_size_y: EpicsSignal = Component(EpicsSignal, "ArraySize2_RBV")
+
+
+class OAV(AreaDetector):
+    cam: CamBase = ADC(CamBase, "-DI-OAV-01:CAM:")
+    roi: ADC = ADC(ROIPlugin, "-DI-OAV-01:ROI:")
+    proc: ADC = ADC(ProcessPlugin, "-DI-OAV-01:PROC:")
+    over: ADC = ADC(OverlayPlugin, "-DI-OAV-01:OVER:")
+    tiff: ADC = ADC(OverlayPlugin, "-DI-OAV-01:TIFF:")
+    hdf5: ADC = ADC(HDF5Plugin, "-DI-OAV-01:HDF5:")
+    snapshot: SnapshotWithGrid = Component(SnapshotWithGrid, "-DI-OAV-01:MJPG:")
+    mxsc: MXSC = ADC(MXSC, "-DI-OAV-01:MXSC:")
+    zoom_controller: ZoomController = ADC(ZoomController, "-EA-OAV-01:FZOOM:")
diff --git a/src/dodal/devices/oav/grid_overlay.py b/src/dodal/devices/oav/grid_overlay.py
index 5f6885e24cc..e28063b7638 100644
--- a/src/dodal/devices/oav/grid_overlay.py
+++ b/src/dodal/devices/oav/grid_overlay.py
@@ -2,11 +2,10 @@
 from functools import partial
 from pathlib import Path
 
+from dodal.devices.areadetector.plugins.MJPG import MJPG as Snapshot
 from ophyd import Component, Signal
 from PIL import Image, ImageDraw
 
-from dodal.devices.oav.snapshot import Snapshot
-
 
 class Orientation(Enum):
     horizontal = 0
diff --git a/src/dodal/devices/oav/oav_detector.py b/src/dodal/devices/oav/oav_detector.py
index 6e13fd71855..0ee06c12aa1 100644
--- a/src/dodal/devices/oav/oav_detector.py
+++ b/src/dodal/devices/oav/oav_detector.py
@@ -1,19 +1,6 @@
 from enum import IntEnum
 
-from ophyd import ADComponent as ADC
-from ophyd import (
-    AreaDetector,
-    CamBase,
-    Component,
-    Device,
-    EpicsSignal,
-    HDF5Plugin,
-    OverlayPlugin,
-    ProcessPlugin,
-    ROIPlugin,
-)
-
-from dodal.devices.oav.grid_overlay import SnapshotWithGrid
+from ophyd import Component, Device, EpicsSignal
 
 
 class ColorMode(IntEnum):
@@ -72,48 +59,3 @@ class EdgeOutputArrayImageType(IntEnum):
     PREPROCESSED = 2
     CANNY_EDGES = 3
     CLOSED_EDGES = 4
-
-
-class MXSC(Device):
-    """
-    Device for edge detection plugin.
-    """
-
-    input_plugin_pv: EpicsSignal = Component(EpicsSignal, "NDArrayPort")
-    enable_callbacks_pv: EpicsSignal = Component(EpicsSignal, "EnableCallbacks")
-    min_callback_time_pv: EpicsSignal = Component(EpicsSignal, "MinCallbackTime")
-    blocking_callbacks_pv: EpicsSignal = Component(EpicsSignal, "BlockingCallbacks")
-    read_file: EpicsSignal = Component(EpicsSignal, "ReadFile")
-    py_filename: EpicsSignal = Component(EpicsSignal, "Filename", string=True)
-    preprocess_operation: EpicsSignal = Component(EpicsSignal, "Preprocess")
-    preprocess_ksize: EpicsSignal = Component(EpicsSignal, "PpParam1")
-    canny_upper_threshold: EpicsSignal = Component(EpicsSignal, "CannyUpper")
-    canny_lower_threshold: EpicsSignal = Component(EpicsSignal, "CannyLower")
-    close_ksize: EpicsSignal = Component(EpicsSignal, "CloseKsize")
-    sample_detection_scan_direction: EpicsSignal = Component(
-        EpicsSignal, "ScanDirection"
-    )
-    sample_detection_min_tip_height: EpicsSignal = Component(
-        EpicsSignal, "MinTipHeight"
-    )
-    tip_x: EpicsSignal = Component(EpicsSignal, "TipX")
-    tip_y: EpicsSignal = Component(EpicsSignal, "TipY")
-    top: EpicsSignal = Component(EpicsSignal, "Top")
-    bottom: EpicsSignal = Component(EpicsSignal, "Bottom")
-    output_array: EpicsSignal = Component(EpicsSignal, "OutputArray")
-    draw_tip: EpicsSignal = Component(EpicsSignal, "DrawTip")
-    draw_edges: EpicsSignal = Component(EpicsSignal, "DrawEdges")
-    waveform_size_x: EpicsSignal = Component(EpicsSignal, "ArraySize1_RBV")
-    waveform_size_y: EpicsSignal = Component(EpicsSignal, "ArraySize2_RBV")
-
-
-class OAV(AreaDetector):
-    cam: CamBase = ADC(CamBase, "-DI-OAV-01:CAM:")
-    roi: ADC = ADC(ROIPlugin, "-DI-OAV-01:ROI:")
-    proc: ADC = ADC(ProcessPlugin, "-DI-OAV-01:PROC:")
-    over: ADC = ADC(OverlayPlugin, "-DI-OAV-01:OVER:")
-    tiff: ADC = ADC(OverlayPlugin, "-DI-OAV-01:TIFF:")
-    hdf5: ADC = ADC(HDF5Plugin, "-DI-OAV-01:HDF5:")
-    snapshot: SnapshotWithGrid = Component(SnapshotWithGrid, "-DI-OAV-01:MJPG:")
-    mxsc: MXSC = ADC(MXSC, "-DI-OAV-01:MXSC:")
-    zoom_controller: ZoomController = ADC(ZoomController, "-EA-OAV-01:FZOOM:")

From 9210928d1b8b49e7c2e809a5a5ba777c7ccd2648 Mon Sep 17 00:00:00 2001
From: David Perl <david.perl@diamond.ac.uk>
Date: Tue, 14 Mar 2023 14:43:46 +0000
Subject: [PATCH 02/16] restructure oav devices

- move mjpg and mxsc to areadetector plugins
- separate out oav utils
- update i03
---
 .../devices/areadetector/plugins/MJPG.py      |  6 +--
 .../devices/areadetector/plugins/MXSC.py      | 27 +---------
 src/dodal/devices/oav/grid_overlay.py         | 23 ++++----
 src/dodal/devices/oav/oav_detector.py         | 53 +++++++++----------
 src/dodal/devices/oav/utils.py                | 29 ++++++++++
 src/dodal/i03.py                              |  2 +-
 tests/devices/system_tests/test_oav_system.py | 12 ++---
 tests/devices/unit_tests/test_oav.py          | 12 ++---
 tests/devices/unit_tests/test_oav_centring.py |  2 +-
 9 files changed, 84 insertions(+), 82 deletions(-)

diff --git a/src/dodal/devices/areadetector/plugins/MJPG.py b/src/dodal/devices/areadetector/plugins/MJPG.py
index d01240480f9..9bad808d6cd 100644
--- a/src/dodal/devices/areadetector/plugins/MJPG.py
+++ b/src/dodal/devices/areadetector/plugins/MJPG.py
@@ -10,10 +10,10 @@ class MJPG(Device):
     filename: Signal = Component(Signal)
     directory: Signal = Component(Signal)
     url: EpicsSignal = Component(EpicsSignal, "JPG_URL_RBV", string=True)
-    x_size_pv: EpicsSignalRO = Component(EpicsSignalRO, "ArraySize1_RBV")
-    y_size_pv: EpicsSignalRO = Component(EpicsSignalRO, "ArraySize2_RBV")
+    x_size: EpicsSignalRO = Component(EpicsSignalRO, "ArraySize1_RBV")
+    y_size: EpicsSignalRO = Component(EpicsSignalRO, "ArraySize2_RBV")
     input_rbpv: EpicsSignalRO = Component(EpicsSignalRO, "NDArrayPort_RBV")
-    input_pv: EpicsSignal = Component(EpicsSignal, "NDArrayPort")
+    input_plugin_pv: EpicsSignal = Component(EpicsSignal, "NDArrayPort")
     KICKOFF_TIMEOUT: float = 10.0
 
     def trigger(self):
diff --git a/src/dodal/devices/areadetector/plugins/MXSC.py b/src/dodal/devices/areadetector/plugins/MXSC.py
index 51bedbf38cf..9b62026fd28 100644
--- a/src/dodal/devices/areadetector/plugins/MXSC.py
+++ b/src/dodal/devices/areadetector/plugins/MXSC.py
@@ -1,17 +1,4 @@
-from dodal.devices.oav.grid_overlay import SnapshotWithGrid
-from dodal.devices.oav.oav_detector import ZoomController
-from ophyd import ADComponent as ADC
-from ophyd import (
-    AreaDetector,
-    CamBase,
-    Component,
-    Device,
-    EpicsSignal,
-    HDF5Plugin,
-    OverlayPlugin,
-    ProcessPlugin,
-    ROIPlugin,
-)
+from ophyd import Component, Device, EpicsSignal
 
 
 class MXSC(Device):
@@ -45,15 +32,3 @@ class MXSC(Device):
     draw_edges: EpicsSignal = Component(EpicsSignal, "DrawEdges")
     waveform_size_x: EpicsSignal = Component(EpicsSignal, "ArraySize1_RBV")
     waveform_size_y: EpicsSignal = Component(EpicsSignal, "ArraySize2_RBV")
-
-
-class OAV(AreaDetector):
-    cam: CamBase = ADC(CamBase, "-DI-OAV-01:CAM:")
-    roi: ADC = ADC(ROIPlugin, "-DI-OAV-01:ROI:")
-    proc: ADC = ADC(ProcessPlugin, "-DI-OAV-01:PROC:")
-    over: ADC = ADC(OverlayPlugin, "-DI-OAV-01:OVER:")
-    tiff: ADC = ADC(OverlayPlugin, "-DI-OAV-01:TIFF:")
-    hdf5: ADC = ADC(HDF5Plugin, "-DI-OAV-01:HDF5:")
-    snapshot: SnapshotWithGrid = Component(SnapshotWithGrid, "-DI-OAV-01:MJPG:")
-    mxsc: MXSC = ADC(MXSC, "-DI-OAV-01:MXSC:")
-    zoom_controller: ZoomController = ADC(ZoomController, "-EA-OAV-01:FZOOM:")
diff --git a/src/dodal/devices/oav/grid_overlay.py b/src/dodal/devices/oav/grid_overlay.py
index e28063b7638..f7e1ddf673d 100644
--- a/src/dodal/devices/oav/grid_overlay.py
+++ b/src/dodal/devices/oav/grid_overlay.py
@@ -2,10 +2,11 @@
 from functools import partial
 from pathlib import Path
 
-from dodal.devices.areadetector.plugins.MJPG import MJPG as Snapshot
 from ophyd import Component, Signal
 from PIL import Image, ImageDraw
 
+from dodal.devices.areadetector.plugins.MJPG import MJPG as Snapshot
+
 
 class Orientation(Enum):
     horizontal = 0
@@ -120,18 +121,18 @@ def add_grid_overlay_to_image(
 
 
 class SnapshotWithGrid(Snapshot):
-    top_left_x_signal: Signal = Component(Signal)
-    top_left_y_signal: Signal = Component(Signal)
-    box_width_signal: Signal = Component(Signal)
-    num_boxes_x_signal: Signal = Component(Signal)
-    num_boxes_y_signal: Signal = Component(Signal)
+    top_left_x: Signal = Component(Signal)
+    top_left_y: Signal = Component(Signal)
+    box_width: Signal = Component(Signal)
+    num_boxes_x: Signal = Component(Signal)
+    num_boxes_y: Signal = Component(Signal)
 
     def post_processing(self, image: Image.Image):
-        top_left_x = self.top_left_x_signal.get()
-        top_left_y = self.top_left_y_signal.get()
-        box_width = self.box_width_signal.get()
-        num_boxes_x = self.num_boxes_x_signal.get()
-        num_boxes_y = self.num_boxes_y_signal.get()
+        top_left_x = self.top_left_x.get()
+        top_left_y = self.top_left_y.get()
+        box_width = self.box_width.get()
+        num_boxes_x = self.num_boxes_x.get()
+        num_boxes_y = self.num_boxes_y.get()
         filename_str = self.filename.get()
         directory_str = self.directory.get()
         add_grid_border_overlay_to_image(
diff --git a/src/dodal/devices/oav/oav_detector.py b/src/dodal/devices/oav/oav_detector.py
index 0ee06c12aa1..9813d333ede 100644
--- a/src/dodal/devices/oav/oav_detector.py
+++ b/src/dodal/devices/oav/oav_detector.py
@@ -1,21 +1,18 @@
-from enum import IntEnum
-
-from ophyd import Component, Device, EpicsSignal
-
-
-class ColorMode(IntEnum):
-    """
-    Enum to store the various color modes of the camera. We use RGB1.
-    """
-
-    MONO = 0
-    BAYER = 1
-    RGB1 = 2
-    RGB2 = 3
-    RGB3 = 4
-    YUV444 = 5
-    YUV422 = 6
-    YUV421 = 7
+from ophyd import ADComponent as ADC
+from ophyd import (
+    AreaDetector,
+    CamBase,
+    Component,
+    Device,
+    EpicsSignal,
+    HDF5Plugin,
+    OverlayPlugin,
+    ProcessPlugin,
+    ROIPlugin,
+)
+
+from dodal.devices.areadetector.plugins.MXSC import MXSC
+from dodal.devices.oav.grid_overlay import SnapshotWithGrid
 
 
 class ZoomController(Device):
@@ -49,13 +46,13 @@ def allowed_zoom_levels(self):
         ]
 
 
-class EdgeOutputArrayImageType(IntEnum):
-    """
-    Enum to store the types of image to tweak the output array. We use Original.
-    """
-
-    ORIGINAL = 0
-    GREYSCALE = 1
-    PREPROCESSED = 2
-    CANNY_EDGES = 3
-    CLOSED_EDGES = 4
+class OAV(AreaDetector):
+    cam: CamBase = ADC(CamBase, "-DI-OAV-01:CAM:")
+    roi: ADC = ADC(ROIPlugin, "-DI-OAV-01:ROI:")
+    proc: ADC = ADC(ProcessPlugin, "-DI-OAV-01:PROC:")
+    over: ADC = ADC(OverlayPlugin, "-DI-OAV-01:OVER:")
+    tiff: ADC = ADC(OverlayPlugin, "-DI-OAV-01:TIFF:")
+    hdf5: ADC = ADC(HDF5Plugin, "-DI-OAV-01:HDF5:")
+    snapshot: SnapshotWithGrid = Component(SnapshotWithGrid, "-DI-OAV-01:MJPG:")
+    mxsc: MXSC = ADC(MXSC, "-DI-OAV-01:MXSC:")
+    zoom_controller: ZoomController = ADC(ZoomController, "-EA-OAV-01:FZOOM:")
diff --git a/src/dodal/devices/oav/utils.py b/src/dodal/devices/oav/utils.py
index 7753784e47b..3fc47ca3d05 100644
--- a/src/dodal/devices/oav/utils.py
+++ b/src/dodal/devices/oav/utils.py
@@ -1,3 +1,5 @@
+from enum import IntEnum
+
 from dodal.utils import Point2D
 
 
@@ -15,3 +17,30 @@ def bottom_right_from_top_left(
         int(steps_x * step_size_x * 1000 * pix_per_um_x + top_left.x),
         int(steps_y * step_size_y * 1000 * pix_per_um_y + top_left.y),
     )
+
+
+class ColorMode(IntEnum):
+    """
+    Enum to store the various color modes of the camera. We use RGB1.
+    """
+
+    MONO = 0
+    BAYER = 1
+    RGB1 = 2
+    RGB2 = 3
+    RGB3 = 4
+    YUV444 = 5
+    YUV422 = 6
+    YUV421 = 7
+
+
+class EdgeOutputArrayImageType(IntEnum):
+    """
+    Enum to store the types of image to tweak the output array. We use Original.
+    """
+
+    ORIGINAL = 0
+    GREYSCALE = 1
+    PREPROCESSED = 2
+    CANNY_EDGES = 3
+    CLOSED_EDGES = 4
diff --git a/src/dodal/i03.py b/src/dodal/i03.py
index 29ab6e1a79d..3afff4f5589 100644
--- a/src/dodal/i03.py
+++ b/src/dodal/i03.py
@@ -4,7 +4,7 @@
 from dodal.devices.detector import DetectorParams
 from dodal.devices.eiger import EigerDetector
 from dodal.devices.fast_grid_scan_composite import FGSComposite
-from dodal.devices.oav.oav_detector import OAV
+from dodal.devices.oav.zoom_controller import OAV
 from dodal.utils import BeamlinePrefix, get_beamline_name
 
 BL = get_beamline_name("s03")
diff --git a/tests/devices/system_tests/test_oav_system.py b/tests/devices/system_tests/test_oav_system.py
index a1a2656cfcb..01e7517e566 100644
--- a/tests/devices/system_tests/test_oav_system.py
+++ b/tests/devices/system_tests/test_oav_system.py
@@ -2,7 +2,7 @@
 import pytest
 from bluesky import RunEngine
 
-from dodal.devices.oav.oav_detector import OAV, ZoomController
+from dodal.devices.oav.zoom_controller import OAV, ZoomController
 
 TEST_GRID_TOP_LEFT_X = 100
 TEST_GRID_TOP_LEFT_Y = 100
@@ -13,11 +13,11 @@
 
 def take_snapshot_with_grid(oav: OAV, snapshot_filename, snapshot_directory):
     oav.wait_for_connection()
-    yield from bps.abs_set(oav.snapshot.top_left_x_signal, TEST_GRID_TOP_LEFT_X)
-    yield from bps.abs_set(oav.snapshot.top_left_y_signal, TEST_GRID_TOP_LEFT_Y)
-    yield from bps.abs_set(oav.snapshot.box_width_signal, TEST_GRID_BOX_WIDTH)
-    yield from bps.abs_set(oav.snapshot.num_boxes_x_signal, TEST_GRID_NUM_BOXES_X)
-    yield from bps.abs_set(oav.snapshot.num_boxes_y_signal, TEST_GRID_NUM_BOXES_Y)
+    yield from bps.abs_set(oav.snapshot.top_left_x, TEST_GRID_TOP_LEFT_X)
+    yield from bps.abs_set(oav.snapshot.top_left_y, TEST_GRID_TOP_LEFT_Y)
+    yield from bps.abs_set(oav.snapshot.box_width, TEST_GRID_BOX_WIDTH)
+    yield from bps.abs_set(oav.snapshot.num_boxes_x, TEST_GRID_NUM_BOXES_X)
+    yield from bps.abs_set(oav.snapshot.num_boxes_y, TEST_GRID_NUM_BOXES_Y)
     yield from bps.abs_set(oav.snapshot.filename, snapshot_filename)
     yield from bps.abs_set(oav.snapshot.directory, snapshot_directory)
     yield from bps.trigger(oav.snapshot, wait=True)
diff --git a/tests/devices/unit_tests/test_oav.py b/tests/devices/unit_tests/test_oav.py
index 57b18766e08..9dc2576ae26 100644
--- a/tests/devices/unit_tests/test_oav.py
+++ b/tests/devices/unit_tests/test_oav.py
@@ -7,7 +7,7 @@
 from requests import HTTPError, Response
 
 import dodal.devices.oav.utils as oav_utils
-from dodal.devices.oav.oav_detector import OAV
+from dodal.devices.oav.zoom_controller import OAV
 from dodal.utils import Point2D
 
 
@@ -19,11 +19,11 @@ def fake_oav() -> OAV:
     fake_oav.snapshot.url.sim_put("http://test.url")
     fake_oav.snapshot.filename.put("test filename")
     fake_oav.snapshot.directory.put("test directory")
-    fake_oav.snapshot.top_left_x_signal.put(100)
-    fake_oav.snapshot.top_left_y_signal.put(100)
-    fake_oav.snapshot.box_width_signal.put(50)
-    fake_oav.snapshot.num_boxes_x_signal.put(15)
-    fake_oav.snapshot.num_boxes_y_signal.put(10)
+    fake_oav.snapshot.top_left_x.put(100)
+    fake_oav.snapshot.top_left_y.put(100)
+    fake_oav.snapshot.box_width.put(50)
+    fake_oav.snapshot.num_boxes_x.put(15)
+    fake_oav.snapshot.num_boxes_y.put(10)
     return fake_oav
 
 
diff --git a/tests/devices/unit_tests/test_oav_centring.py b/tests/devices/unit_tests/test_oav_centring.py
index c93b13f4f96..99470bd3a91 100644
--- a/tests/devices/unit_tests/test_oav_centring.py
+++ b/tests/devices/unit_tests/test_oav_centring.py
@@ -17,7 +17,7 @@
     get_rotation_increment,
     keep_inside_bounds,
 )
-from dodal.devices.oav.oav_detector import OAV
+from dodal.devices.oav.zoom_controller import OAV
 from dodal.devices.oav.oav_errors import (
     OAVError_MissingRotations,
     OAVError_NoRotationsPassValidityTest,

From 7443325a769987e09d82d02e32d754b0e9e1df23 Mon Sep 17 00:00:00 2001
From: David Perl <david.perl@diamond.ac.uk>
Date: Tue, 14 Mar 2023 15:09:04 +0000
Subject: [PATCH 03/16] keep oav detector

---
 src/dodal/devices/oav/grid_overlay.py         | 4 ++--
 src/dodal/i03.py                              | 2 +-
 tests/devices/system_tests/test_oav_system.py | 2 +-
 tests/devices/unit_tests/test_oav.py          | 2 +-
 tests/devices/unit_tests/test_oav_centring.py | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/dodal/devices/oav/grid_overlay.py b/src/dodal/devices/oav/grid_overlay.py
index f7e1ddf673d..9820ee19e1a 100644
--- a/src/dodal/devices/oav/grid_overlay.py
+++ b/src/dodal/devices/oav/grid_overlay.py
@@ -5,7 +5,7 @@
 from ophyd import Component, Signal
 from PIL import Image, ImageDraw
 
-from dodal.devices.areadetector.plugins.MJPG import MJPG as Snapshot
+from dodal.devices.areadetector.plugins.MJPG import MJPG
 
 
 class Orientation(Enum):
@@ -120,7 +120,7 @@ def add_grid_overlay_to_image(
     )
 
 
-class SnapshotWithGrid(Snapshot):
+class SnapshotWithGrid(MJPG):
     top_left_x: Signal = Component(Signal)
     top_left_y: Signal = Component(Signal)
     box_width: Signal = Component(Signal)
diff --git a/src/dodal/i03.py b/src/dodal/i03.py
index 3afff4f5589..29ab6e1a79d 100644
--- a/src/dodal/i03.py
+++ b/src/dodal/i03.py
@@ -4,7 +4,7 @@
 from dodal.devices.detector import DetectorParams
 from dodal.devices.eiger import EigerDetector
 from dodal.devices.fast_grid_scan_composite import FGSComposite
-from dodal.devices.oav.zoom_controller import OAV
+from dodal.devices.oav.oav_detector import OAV
 from dodal.utils import BeamlinePrefix, get_beamline_name
 
 BL = get_beamline_name("s03")
diff --git a/tests/devices/system_tests/test_oav_system.py b/tests/devices/system_tests/test_oav_system.py
index 01e7517e566..aaa5a116477 100644
--- a/tests/devices/system_tests/test_oav_system.py
+++ b/tests/devices/system_tests/test_oav_system.py
@@ -2,7 +2,7 @@
 import pytest
 from bluesky import RunEngine
 
-from dodal.devices.oav.zoom_controller import OAV, ZoomController
+from dodal.devices.oav.oav_detector import OAV, ZoomController
 
 TEST_GRID_TOP_LEFT_X = 100
 TEST_GRID_TOP_LEFT_Y = 100
diff --git a/tests/devices/unit_tests/test_oav.py b/tests/devices/unit_tests/test_oav.py
index 9dc2576ae26..979718f7830 100644
--- a/tests/devices/unit_tests/test_oav.py
+++ b/tests/devices/unit_tests/test_oav.py
@@ -7,7 +7,7 @@
 from requests import HTTPError, Response
 
 import dodal.devices.oav.utils as oav_utils
-from dodal.devices.oav.zoom_controller import OAV
+from dodal.devices.oav.oav_detector import OAV
 from dodal.utils import Point2D
 
 
diff --git a/tests/devices/unit_tests/test_oav_centring.py b/tests/devices/unit_tests/test_oav_centring.py
index 99470bd3a91..c93b13f4f96 100644
--- a/tests/devices/unit_tests/test_oav_centring.py
+++ b/tests/devices/unit_tests/test_oav_centring.py
@@ -17,7 +17,7 @@
     get_rotation_increment,
     keep_inside_bounds,
 )
-from dodal.devices.oav.zoom_controller import OAV
+from dodal.devices.oav.oav_detector import OAV
 from dodal.devices.oav.oav_errors import (
     OAVError_MissingRotations,
     OAVError_NoRotationsPassValidityTest,

From 4c7494cce1ed89fbcaffec08c956a4e06ee86e8f Mon Sep 17 00:00:00 2001
From: David Perl <david.perl@diamond.ac.uk>
Date: Thu, 16 Mar 2023 09:16:23 +0000
Subject: [PATCH 04/16] fix oav test

---
 tests/devices/unit_tests/test_oav.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/devices/unit_tests/test_oav.py b/tests/devices/unit_tests/test_oav.py
index 979718f7830..46c4c89f83a 100644
--- a/tests/devices/unit_tests/test_oav.py
+++ b/tests/devices/unit_tests/test_oav.py
@@ -41,8 +41,10 @@ def test_snapshot_trigger_handles_request_with_bad_status_code_correctly(
 
 
 @patch("requests.get")
-@patch("dodal.devices.oav.snapshot.Image")
-def test_snapshot_trigger_loads_correct_url(mock_image, mock_get: MagicMock, fake_oav):
+@patch("dodal.devices.areadetector.plugins.MJPG.Image")
+def test_snapshot_trigger_loads_correct_url(
+    mock_image: MagicMock, mock_get: MagicMock, fake_oav: OAV
+):
     st = fake_oav.snapshot.trigger()
     st.wait()
     mock_get.assert_called_once_with("http://test.url", stream=True)

From 4524e504f4fd192a7a7de86a29c1209d2e40c211 Mon Sep 17 00:00:00 2001
From: David Perl <david.perl@diamond.ac.uk>
Date: Mon, 27 Mar 2023 16:58:35 +0100
Subject: [PATCH 05/16] tidy pv names

---
 src/dodal/devices/areadetector/plugins/MJPG.py |  2 +-
 src/dodal/devices/areadetector/plugins/MXSC.py | 10 +++++-----
 src/dodal/devices/oav/oav_parameters.py        | 15 +++++++++++++++
 src/dodal/devices/oav/snapshot.py              |  2 +-
 4 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/src/dodal/devices/areadetector/plugins/MJPG.py b/src/dodal/devices/areadetector/plugins/MJPG.py
index 9bad808d6cd..14aebe237ca 100644
--- a/src/dodal/devices/areadetector/plugins/MJPG.py
+++ b/src/dodal/devices/areadetector/plugins/MJPG.py
@@ -13,7 +13,7 @@ class MJPG(Device):
     x_size: EpicsSignalRO = Component(EpicsSignalRO, "ArraySize1_RBV")
     y_size: EpicsSignalRO = Component(EpicsSignalRO, "ArraySize2_RBV")
     input_rbpv: EpicsSignalRO = Component(EpicsSignalRO, "NDArrayPort_RBV")
-    input_plugin_pv: EpicsSignal = Component(EpicsSignal, "NDArrayPort")
+    input_plugin: EpicsSignal = Component(EpicsSignal, "NDArrayPort")
     KICKOFF_TIMEOUT: float = 10.0
 
     def trigger(self):
diff --git a/src/dodal/devices/areadetector/plugins/MXSC.py b/src/dodal/devices/areadetector/plugins/MXSC.py
index 9b62026fd28..aa214fa451b 100644
--- a/src/dodal/devices/areadetector/plugins/MXSC.py
+++ b/src/dodal/devices/areadetector/plugins/MXSC.py
@@ -6,12 +6,12 @@ class MXSC(Device):
     Device for edge detection plugin.
     """
 
-    input_plugin_pv: EpicsSignal = Component(EpicsSignal, "NDArrayPort")
-    enable_callbacks_pv: EpicsSignal = Component(EpicsSignal, "EnableCallbacks")
-    min_callback_time_pv: EpicsSignal = Component(EpicsSignal, "MinCallbackTime")
-    blocking_callbacks_pv: EpicsSignal = Component(EpicsSignal, "BlockingCallbacks")
+    input_plugin: EpicsSignal = Component(EpicsSignal, "NDArrayPort")
+    enable_callbacks: EpicsSignal = Component(EpicsSignal, "EnableCallbacks")
+    min_callback_time: EpicsSignal = Component(EpicsSignal, "MinCallbackTime")
+    blocking_callbacks: EpicsSignal = Component(EpicsSignal, "BlockingCallbacks")
     read_file: EpicsSignal = Component(EpicsSignal, "ReadFile")
-    py_filename: EpicsSignal = Component(EpicsSignal, "Filename", string=True)
+    filename: EpicsSignal = Component(EpicsSignal, "Filename", string=True)
     preprocess_operation: EpicsSignal = Component(EpicsSignal, "Preprocess")
     preprocess_ksize: EpicsSignal = Component(EpicsSignal, "PpParam1")
     canny_upper_threshold: EpicsSignal = Component(EpicsSignal, "CannyUpper")
diff --git a/src/dodal/devices/oav/oav_parameters.py b/src/dodal/devices/oav/oav_parameters.py
index ce5128256fa..9bb996809d2 100644
--- a/src/dodal/devices/oav/oav_parameters.py
+++ b/src/dodal/devices/oav/oav_parameters.py
@@ -8,20 +8,35 @@
 )
 from dodal.log import LOGGER
 
+OAV_CONFIG_FILE_DEFAULTS = {
+    "zoom_params_file": "/dls_sw/i03/software/gda_versions/gda_9_27/workspace_git/gda-mx.git/configurations/i03-config/xml/jCameraManZoomLevels.xml",
+    "oav_json": "/dls_sw/i03/software/gda_versions/gda_9_27/workspace_git/gda-mx.git/configurations/i03-config/etc/OAVCentring.json",
+    "display_config": "/dls_sw/i03/software/gda_versions/var/display.configuration",
+}
+
 
 class OAVParameters:
+    zoom_params_file: str
+    oav_json: str
+    display_config: str
+
     def __init__(
         self,
         centring_params_json: str,
         camera_zoom_levels_file: str,
         display_configuration_file: str,
         context="loopCentring",
+        config_files: dict[str, str] = OAV_CONFIG_FILE_DEFAULTS,
     ):
         self.centring_params_json = centring_params_json
         self.camera_zoom_levels_file = camera_zoom_levels_file
         self.display_configuration_file = display_configuration_file
         self.context = context
 
+        self.zoom_params_file = config_files["zoom_params_file"]
+        self.oav_json = config_files["oav_json"]
+        self.display_config = config_files["display_config"]
+
         self.load_parameters_from_json()
         self.load_microns_per_pixel()
         self._extract_beam_position()
diff --git a/src/dodal/devices/oav/snapshot.py b/src/dodal/devices/oav/snapshot.py
index 4574ddd53d2..a471e4eba05 100644
--- a/src/dodal/devices/oav/snapshot.py
+++ b/src/dodal/devices/oav/snapshot.py
@@ -13,7 +13,7 @@ class Snapshot(Device):
     x_size_pv: EpicsSignalRO = Component(EpicsSignalRO, "ArraySize1_RBV")
     y_size_pv: EpicsSignalRO = Component(EpicsSignalRO, "ArraySize2_RBV")
     input_rbpv: EpicsSignalRO = Component(EpicsSignalRO, "NDArrayPort_RBV")
-    input_pv: EpicsSignal = Component(EpicsSignal, "NDArrayPort")
+    input_plugin: EpicsSignal = Component(EpicsSignal, "NDArrayPort")
     KICKOFF_TIMEOUT: float = 10.0
 
     def trigger(self):

From c5ef21a80cda53e3e579b7a094d69a15acdae167 Mon Sep 17 00:00:00 2001
From: David Perl <david.perl@diamond.ac.uk>
Date: Thu, 6 Apr 2023 11:27:52 +0100
Subject: [PATCH 06/16] 3.8 compatible typing

---
 src/dodal/devices/oav/oav_parameters.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/dodal/devices/oav/oav_parameters.py b/src/dodal/devices/oav/oav_parameters.py
index 9bb996809d2..21dfd017fd7 100644
--- a/src/dodal/devices/oav/oav_parameters.py
+++ b/src/dodal/devices/oav/oav_parameters.py
@@ -1,6 +1,6 @@
 import json
 import xml.etree.cElementTree as et
-from typing import Tuple
+from typing import Dict, Tuple
 
 from dodal.devices.oav.oav_errors import (
     OAVError_BeamPositionNotFound,
@@ -26,7 +26,7 @@ def __init__(
         camera_zoom_levels_file: str,
         display_configuration_file: str,
         context="loopCentring",
-        config_files: dict[str, str] = OAV_CONFIG_FILE_DEFAULTS,
+        config_files: Dict[str, str] = OAV_CONFIG_FILE_DEFAULTS,
     ):
         self.centring_params_json = centring_params_json
         self.camera_zoom_levels_file = camera_zoom_levels_file

From 766285eb5906cf09ab3352df91f84c645ecf4204 Mon Sep 17 00:00:00 2001
From: David Perl <david.perl@diamond.ac.uk>
Date: Mon, 24 Apr 2023 17:10:02 +0100
Subject: [PATCH 07/16] add detector motion to i03 and change bad name

---
 src/dodal/devices/detector_motion.py |  2 +-
 src/dodal/i03.py                     | 17 +++++++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/src/dodal/devices/detector_motion.py b/src/dodal/devices/detector_motion.py
index 9e602b50a7c..8ed564061fd 100644
--- a/src/dodal/devices/detector_motion.py
+++ b/src/dodal/devices/detector_motion.py
@@ -2,7 +2,7 @@
 from ophyd import Device, EpicsMotor, EpicsSignal, EpicsSignalRO
 
 
-class Det(Device):
+class DetectorMotion(Device):
     """Physical motion and interlocks for detector travel"""
 
     upstream_x: EpicsMotor = Cpt(EpicsMotor, "-MO-DET-01:UPSTREAMX")
diff --git a/src/dodal/i03.py b/src/dodal/i03.py
index 6c59d34f138..2c797564f4f 100644
--- a/src/dodal/i03.py
+++ b/src/dodal/i03.py
@@ -7,6 +7,7 @@
 from dodal.devices.backlight import Backlight
 from dodal.devices.DCM import DCM
 from dodal.devices.detector import DetectorParams
+from dodal.devices.detector_motion import DetectorMotion
 from dodal.devices.eiger import EigerDetector
 from dodal.devices.fast_grid_scan import FastGridScan
 from dodal.devices.oav.oav_detector import OAV
@@ -128,6 +129,22 @@ def backlight(
     )
 
 
+@skip_device(lambda: BL == "s03")
+def detector_motion(
+    wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False
+) -> DetectorMotion:
+    """Get the i03 detector motion device, instantiate it if it hasn't already been.
+    If this is called when already instantiated in i03, it will return the existing object.
+    """
+    return device_instantiation(
+        device=DetectorMotion,
+        name="detector_motion",
+        prefix="",
+        wait=wait_for_connection,
+        fake=fake_with_ophyd_sim,
+    )
+
+
 @skip_device(lambda: BL == "s03")
 def eiger(
     wait_for_connection: bool = True,

From bec80353162d5bf54682c1539b61f4bc0243ad0d Mon Sep 17 00:00:00 2001
From: David Perl <david.perl@diamond.ac.uk>
Date: Mon, 24 Apr 2023 17:10:36 +0100
Subject: [PATCH 08/16] simplify oav params

---
 src/dodal/devices/oav/oav_parameters.py | 161 +++++++++++-------------
 1 file changed, 73 insertions(+), 88 deletions(-)

diff --git a/src/dodal/devices/oav/oav_parameters.py b/src/dodal/devices/oav/oav_parameters.py
index 21dfd017fd7..a8227e38c32 100644
--- a/src/dodal/devices/oav/oav_parameters.py
+++ b/src/dodal/devices/oav/oav_parameters.py
@@ -1,6 +1,7 @@
 import json
 import xml.etree.cElementTree as et
-from typing import Dict, Tuple
+from collections import ChainMap
+from typing import Any, Dict, Tuple
 
 from dodal.devices.oav.oav_errors import (
     OAVError_BeamPositionNotFound,
@@ -19,6 +20,26 @@ class OAVParameters:
     zoom_params_file: str
     oav_json: str
     display_config: str
+    global_params: dict[str, Any]
+    context_dicts: dict[str, dict]
+    active_params: ChainMap
+
+    exposure: float
+    acquire_period: float
+    gain: float
+    canny_edge_upper_threshold: float
+    canny_edge_lower_threshold: float
+    minimum_height: int
+    zoom: int
+    preprocess: int  # gets blur type, e.g. 8 = gaussianBlur, 9 = medianBlur
+    preprocess_K_size: int  # length scale for blur preprocessing
+    detection_script_filename: str
+    close_ksize: int
+    input_plugin: str
+    mxsc_input: str
+    min_callback_time: float
+    direction: int
+    max_tip_distance: int
 
     def __init__(
         self,
@@ -33,104 +54,68 @@ def __init__(
         self.display_configuration_file = display_configuration_file
         self.context = context
 
+        self.global_params, self.context_dicts = self.load_json(
+            self.centring_params_json
+        )
+        self.active_params = ChainMap(
+            {}, self.global_params, self.context_dicts[self.context]
+        )
+        self.update_self_from_current_context()
+
         self.zoom_params_file = config_files["zoom_params_file"]
         self.oav_json = config_files["oav_json"]
         self.display_config = config_files["display_config"]
 
-        self.load_parameters_from_json()
         self.load_microns_per_pixel()
         self._extract_beam_position()
 
-    def load_json(self):
-        """
-        Loads the json from the json file at self.centring_params_json and save it as a dictionary in the parameters attribute.
-        """
-        with open(f"{self.centring_params_json}") as f:
-            self.parameters = json.load(f)
-
-    def load_parameters_from_json(
-        self,
-    ) -> None:
-        """
-        Load required parameters on initialisation as an attribute variables. If a variable in the json is
-        liable to change throughout a run it can be reloaded when needed.
-        """
-
-        self.load_json()
-
-        self.exposure = self._extract_dict_parameter("exposure")
-        self.acquire_period = self._extract_dict_parameter("acqPeriod")
-        self.gain = self._extract_dict_parameter("gain")
-        self.canny_edge_upper_threshold = self._extract_dict_parameter(
-            "CannyEdgeUpperThreshold"
-        )
-        self.canny_edge_lower_threshold = self._extract_dict_parameter(
-            "CannyEdgeLowerThreshold", fallback_value=5.0
-        )
-        self.minimum_height = self._extract_dict_parameter("minheight")
-        self.zoom = self._extract_dict_parameter("zoom")
-        # gets blur type, e.g. 8 = gaussianBlur, 9 = medianBlur
-        self.preprocess = self._extract_dict_parameter("preprocess")
-        # length scale for blur preprocessing
-        self.preprocess_K_size = self._extract_dict_parameter("preProcessKSize")
-        self.filename = self._extract_dict_parameter("filename")
-        self.close_ksize = self._extract_dict_parameter(
-            "close_ksize", fallback_value=11
-        )
-
-        self.input_plugin = self._extract_dict_parameter("oav", fallback_value="OAV")
-        self.mxsc_input = self._extract_dict_parameter(
-            "mxsc_input", fallback_value="CAM"
-        )
-        self.min_callback_time = self._extract_dict_parameter(
-            "min_callback_time", fallback_value=0.08
-        )
-
-        self.direction = self._extract_dict_parameter("direction")
-
-        self.max_tip_distance = self._extract_dict_parameter("max_tip_distance")
-
-    def _extract_dict_parameter(self, key: str, fallback_value=None, reload_json=False):
+    @staticmethod
+    def load_json(filename: str) -> tuple[dict[str, Any], dict[str, dict]]:
         """
-        Designed to extract parameters from the json OAVParameters.json. This will hopefully be changed in
-        future, but currently we have to use the json passed in from GDA.
-
-        The json is of the form:
-            {
-                "parameter1": value1,
-                "parameter2": value2,
-                "context_name": {
-                    "parameter1": value3
-            }
-        When we extract the parameters we want to check if the given context (stored as a class variable)
-        contains a parameter, if it does we return it, if not we return the global value. If a parameter
-        is not found at all then the passed in fallback_value is returned. If that isn't found then an
-        error is raised.
-        Args:
-            key: the key of the value being extracted
-            fallback_value: a value to be returned if the key is not found
-            reload_json: reload the json from the file before searching for it, needed because some
-                parameters can change mid operation.
-        Returns: The extracted value corresponding to the key, or the fallback_value if none is found.
+        Loads the json from the specified file, and returns a dict with all the
+        individual top-level k-v pairs, and one with all the subdicts.
         """
+        with open(filename) as f:
+            raw_params: dict[str, Any] = json.load(f)
+        global_params = {k: raw_params.pop(v) for k, v in list(raw_params.items())}
+        context_dicts = raw_params
+        return global_params, context_dicts
+
+    def update_context(self, context: str) -> None:
+        self.active_params.maps.pop()
+        self.active_params = self.active_params.new_child(self.context_dicts[context])
+
+    def update_self_from_current_context(self) -> None:
+        def update(name, type, default=None):
+            try:
+                param = self.active_params.get(name, default)
+                assert isinstance(param, type)
+                return param
+            except AssertionError:
+                raise TypeError(
+                    f"OAV param {name} from the OAV centring params json file has the "
+                    f"wrong type, should be {type}."
+                )
 
-        if reload_json:
-            self.load_json()
-
-        if self.context in self.parameters:
-            if key in self.parameters[self.context]:
-                return self.parameters[self.context][key]
-
-        if key in self.parameters:
-            return self.parameters[key]
-
-        if fallback_value:
-            return fallback_value
-
-        # No fallback_value was given and the key wasn't found.
-        raise KeyError(
-            f"Searched in {self.centring_params_json} for key {key} in context {self.context} but no value was found. No fallback value was given."
+        self.exposure = update("exposure", float)
+        self.acquire_period = update("acqPeriod", float)
+        self.gain = update("gain", float)
+        self.canny_edge_upper_threshold = update("CannyEdgeUpperThreshold", float)
+        self.canny_edge_lower_threshold = update(
+            "CannyEdgeLowerThreshold", float, default=5.0
         )
+        self.minimum_height = update("minheight", int)
+        self.zoom = update("zoom", int)
+        self.preprocess = update("preprocess", int)
+        self.preprocess_K_size = update("preProcessKSize", int)
+        self.detection_script_filename = update("filename", str)
+        self.close_ksize = update("close_ksize", int, default=11)
+
+        self.input_plugin = update("oav", str, default="OAV")
+        self.mxsc_input = update("mxsc_input", str, default="CAM")
+        self.min_callback_time = update("min_callback_time", float, default=0.08)
+        self.direction = update("direction", int)
+        self.max_tip_distance = update("max_tip_distance", int)
 
     def load_microns_per_pixel(self, zoom=None):
         """

From f9ebe759cd6ff27696080f0e06ab5b31dbeeb51d Mon Sep 17 00:00:00 2001
From: David Perl <david.perl@diamond.ac.uk>
Date: Mon, 24 Apr 2023 17:31:51 +0100
Subject: [PATCH 09/16] tidy up

---
 src/dodal/devices/oav/oav_parameters.py | 43 +++++++++++--------------
 1 file changed, 18 insertions(+), 25 deletions(-)

diff --git a/src/dodal/devices/oav/oav_parameters.py b/src/dodal/devices/oav/oav_parameters.py
index a8227e38c32..bd891bea5d4 100644
--- a/src/dodal/devices/oav/oav_parameters.py
+++ b/src/dodal/devices/oav/oav_parameters.py
@@ -30,7 +30,7 @@ class OAVParameters:
     canny_edge_upper_threshold: float
     canny_edge_lower_threshold: float
     minimum_height: int
-    zoom: int
+    zoom: float
     preprocess: int  # gets blur type, e.g. 8 = gaussianBlur, 9 = medianBlur
     preprocess_K_size: int  # length scale for blur preprocessing
     detection_script_filename: str
@@ -39,33 +39,23 @@ class OAVParameters:
     mxsc_input: str
     min_callback_time: float
     direction: int
-    max_tip_distance: int
+    max_tip_distance: float
 
     def __init__(
         self,
-        centring_params_json: str,
-        camera_zoom_levels_file: str,
-        display_configuration_file: str,
         context="loopCentring",
         config_files: Dict[str, str] = OAV_CONFIG_FILE_DEFAULTS,
     ):
-        self.centring_params_json = centring_params_json
-        self.camera_zoom_levels_file = camera_zoom_levels_file
-        self.display_configuration_file = display_configuration_file
+        self.zoom_params_file = config_files["zoom_params_file"]
+        self.oav_config_json = config_files["oav_json"]
+        self.display_config = config_files["display_config"]
         self.context = context
 
-        self.global_params, self.context_dicts = self.load_json(
-            self.centring_params_json
-        )
+        self.global_params, self.context_dicts = self.load_json(self.oav_config_json)
         self.active_params = ChainMap(
             {}, self.global_params, self.context_dicts[self.context]
         )
         self.update_self_from_current_context()
-
-        self.zoom_params_file = config_files["zoom_params_file"]
-        self.oav_json = config_files["oav_json"]
-        self.display_config = config_files["display_config"]
-
         self.load_microns_per_pixel()
         self._extract_beam_position()
 
@@ -77,7 +67,11 @@ def load_json(filename: str) -> tuple[dict[str, Any], dict[str, dict]]:
         """
         with open(filename) as f:
             raw_params: dict[str, Any] = json.load(f)
-        global_params = {k: raw_params.pop(v) for k, v in list(raw_params.items())}
+        global_params = {
+            k: raw_params.pop(k)
+            for k, v in list(raw_params.items())
+            if not isinstance(v, dict)
+        }
         context_dicts = raw_params
         return global_params, context_dicts
 
@@ -105,27 +99,26 @@ def update(name, type, default=None):
             "CannyEdgeLowerThreshold", float, default=5.0
         )
         self.minimum_height = update("minheight", int)
-        self.zoom = update("zoom", int)
+        self.zoom = update("zoom", float)
         self.preprocess = update("preprocess", int)
         self.preprocess_K_size = update("preProcessKSize", int)
         self.detection_script_filename = update("filename", str)
         self.close_ksize = update("close_ksize", int, default=11)
-
         self.input_plugin = update("oav", str, default="OAV")
         self.mxsc_input = update("mxsc_input", str, default="CAM")
         self.min_callback_time = update("min_callback_time", float, default=0.08)
         self.direction = update("direction", int)
-        self.max_tip_distance = update("max_tip_distance", int)
+        self.max_tip_distance = update("max_tip_distance", float)
 
     def load_microns_per_pixel(self, zoom=None):
         """
-        Loads the microns per x pixel and y pixel for a given zoom level. These are currently generated by GDA, though artemis could generate them
-        in future.
+        Loads the microns per x pixel and y pixel for a given zoom level. These are
+        currently generated by GDA, though artemis could generate them in future.
         """
         if not zoom:
             zoom = self.zoom
 
-        tree = et.parse(self.camera_zoom_levels_file)
+        tree = et.parse(self.zoom_params_file)
         self.micronsPerXPixel = self.micronsPerYPixel = None
         root = tree.getroot()
         levels = root.findall(".//zoomLevel")
@@ -135,7 +128,7 @@ def load_microns_per_pixel(self, zoom=None):
                 self.micronsPerYPixel = float(node.find("micronsPerYPixel").text)
         if self.micronsPerXPixel is None or self.micronsPerYPixel is None:
             raise OAVError_ZoomLevelNotFound(
-                f"Could not find the micronsPer[X,Y]Pixel parameters in {self.camera_zoom_levels_file} for zoom level {zoom}."
+                f"Could not find the micronsPer[X,Y]Pixel parameters in {self.zoom_params_file} for zoom level {zoom}."
             )
 
         # get the max tip distance in pixels
@@ -147,7 +140,7 @@ def _extract_beam_position(self):
         stored in the file display.configuration. The beam location is manually inputted
         by the beamline operator GDA by clicking where on screen a scintillator ligths up.
         """
-        with open(self.display_configuration_file, "r") as f:
+        with open(self.display_config, "r") as f:
             file_lines = f.readlines()
             for i in range(len(file_lines)):
                 if file_lines[i].startswith("zoomLevel = " + str(self.zoom)):

From 8b1ecb444d79c11213539fc6a571ea8d4fd54a89 Mon Sep 17 00:00:00 2001
From: David Perl <david.perl@diamond.ac.uk>
Date: Mon, 24 Apr 2023 17:40:16 +0100
Subject: [PATCH 10/16] propagate type info in skip_device decorator

---
 src/dodal/utils.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/dodal/utils.py b/src/dodal/utils.py
index 195e53ffa33..84bca212279 100644
--- a/src/dodal/utils.py
+++ b/src/dodal/utils.py
@@ -7,7 +7,7 @@
 from inspect import signature
 from os import environ
 from types import ModuleType
-from typing import Any, Callable, Dict, Iterable, Optional, Type, Union
+from typing import Any, Callable, Dict, Iterable, Optional, Type, TypeVar, Union
 
 from bluesky.protocols import (
     Checkable,
@@ -48,6 +48,8 @@
 Point2D = namedtuple("Point2D", ["x", "y"])
 Point3D = namedtuple("Point3D", ["x", "y", "z"])
 
+T = TypeVar("T")
+
 
 def get_beamline_name(ixx: str) -> str:
     bl = environ.get("BEAMLINE")
@@ -73,9 +75,9 @@ def __post_init__(self):
 
 
 def skip_device(precondition=lambda: True):
-    def decorator(func: Callable[..., Any]):
+    def decorator(func: Callable[..., T]) -> Callable[..., T]:
         @wraps(func)
-        def wrapper(*args, **kwds):
+        def wrapper(*args, **kwds) -> T:
             return func(*args, **kwds)
 
         if precondition:

From 34ca0527c2545da60a937717233599141f7f9b44 Mon Sep 17 00:00:00 2001
From: David Perl <david.perl@diamond.ac.uk>
Date: Mon, 24 Apr 2023 18:09:30 +0100
Subject: [PATCH 11/16] more tidying

---
 src/dodal/devices/oav/oav_parameters.py | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/dodal/devices/oav/oav_parameters.py b/src/dodal/devices/oav/oav_parameters.py
index bd891bea5d4..af0e881a4a2 100644
--- a/src/dodal/devices/oav/oav_parameters.py
+++ b/src/dodal/devices/oav/oav_parameters.py
@@ -1,7 +1,7 @@
 import json
 import xml.etree.cElementTree as et
 from collections import ChainMap
-from typing import Any, Dict, Tuple
+from typing import Any, Tuple
 
 from dodal.devices.oav.oav_errors import (
     OAVError_BeamPositionNotFound,
@@ -44,11 +44,13 @@ class OAVParameters:
     def __init__(
         self,
         context="loopCentring",
-        config_files: Dict[str, str] = OAV_CONFIG_FILE_DEFAULTS,
+        zoom_params_file=OAV_CONFIG_FILE_DEFAULTS["zoom_params_file"],
+        oav_config_json=OAV_CONFIG_FILE_DEFAULTS["oav_json"],
+        display_config=OAV_CONFIG_FILE_DEFAULTS["display_config"],
     ):
-        self.zoom_params_file = config_files["zoom_params_file"]
-        self.oav_config_json = config_files["oav_json"]
-        self.display_config = config_files["display_config"]
+        self.zoom_params_file = zoom_params_file
+        self.oav_config_json = oav_config_json
+        self.display_config = display_config
         self.context = context
 
         self.global_params, self.context_dicts = self.load_json(self.oav_config_json)

From 4aa5c4504fa5b43780e8d446844f4ae093648663 Mon Sep 17 00:00:00 2001
From: David Perl <david.perl@diamond.ac.uk>
Date: Mon, 24 Apr 2023 18:24:26 +0100
Subject: [PATCH 12/16] fix tests

---
 src/dodal/devices/oav/oav_parameters.py       |  8 +++---
 tests/devices/unit_tests/test_oav_centring.py | 26 +++----------------
 2 files changed, 7 insertions(+), 27 deletions(-)

diff --git a/src/dodal/devices/oav/oav_parameters.py b/src/dodal/devices/oav/oav_parameters.py
index af0e881a4a2..ddcec6b74ad 100644
--- a/src/dodal/devices/oav/oav_parameters.py
+++ b/src/dodal/devices/oav/oav_parameters.py
@@ -82,15 +82,15 @@ def update_context(self, context: str) -> None:
         self.active_params = self.active_params.new_child(self.context_dicts[context])
 
     def update_self_from_current_context(self) -> None:
-        def update(name, type, default=None):
+        def update(name, param_type, default=None):
+            param = self.active_params.get(name, default)
             try:
-                param = self.active_params.get(name, default)
-                assert isinstance(param, type)
+                param = param_type(param)
                 return param
             except AssertionError:
                 raise TypeError(
                     f"OAV param {name} from the OAV centring params json file has the "
-                    f"wrong type, should be {type}."
+                    f"wrong type, should be {param_type} but is {type(param)}."
                 )
 
         self.exposure = update("exposure", float)
diff --git a/tests/devices/unit_tests/test_oav_centring.py b/tests/devices/unit_tests/test_oav_centring.py
index c93b13f4f96..33762668bcb 100644
--- a/tests/devices/unit_tests/test_oav_centring.py
+++ b/tests/devices/unit_tests/test_oav_centring.py
@@ -44,7 +44,9 @@ def mock_oav():
 
 @pytest.fixture
 def mock_parameters():
-    return OAVParameters(OAV_CENTRING_JSON, ZOOM_LEVELS_XML, DISPLAY_CONFIGURATION)
+    return OAVParameters(
+        "loopCentring", ZOOM_LEVELS_XML, OAV_CENTRING_JSON, DISPLAY_CONFIGURATION
+    )
 
 
 @pytest.fixture
@@ -91,31 +93,9 @@ def fake_run(
 def test_oav_parameters_load_parameters_from_json(
     parameter_name, expected_value, mock_parameters: OAVParameters
 ):
-    mock_parameters.load_parameters_from_json()
-
     assert mock_parameters.__dict__[parameter_name] == expected_value
 
 
-def test_oav__extract_dict_parameter_not_found_fallback_value_present(
-    mock_parameters: OAVParameters,
-):
-    mock_parameters.load_json()
-    assert (
-        mock_parameters._extract_dict_parameter(
-            "a_key_not_in_the_json", fallback_value=1
-        )
-        == 1
-    )
-
-
-def test_oav__extract_dict_parameter_not_found_fallback_value_not_present(
-    mock_parameters: OAVParameters,
-):
-    mock_parameters.load_json()
-    with pytest.raises(KeyError):
-        mock_parameters._extract_dict_parameter("a_key_not_in_the_json")
-
-
 def test_find_midpoint_symmetric_pin():
     x = np.arange(-15, 10, 25 / 1024)
     x2 = x**2

From 8b667a3e76e56394cb9cb5ceb51dc83ca3454a17 Mon Sep 17 00:00:00 2001
From: David Perl <david.perl@diamond.ac.uk>
Date: Thu, 11 May 2023 09:46:01 +0100
Subject: [PATCH 13/16] drop python 3.8 tests

---
 .github/workflows/code.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/code.yml b/.github/workflows/code.yml
index 67aaa2035fc..cd53ccbd751 100644
--- a/.github/workflows/code.yml
+++ b/.github/workflows/code.yml
@@ -35,12 +35,12 @@ jobs:
       fail-fast: false
       matrix:
         os: ["ubuntu-latest"] # can add windows-latest, macos-latest
-        python: ["3.9", "3.10", "3.11"]
+        python: ["3.10", "3.11"]
         install: ["-e .[dev]"]
         # Make one version be non-editable to test both paths of version code
         include:
           - os: "ubuntu-latest"
-            python: "3.8"
+            python: "3.9"
             install: ".[dev]"
 
     runs-on: ${{ matrix.os }}

From 127a03fb4c25fe81445ac229906923fffa27ecd1 Mon Sep 17 00:00:00 2001
From: David Perl <david.perl@diamond.ac.uk>
Date: Fri, 12 May 2023 09:29:58 +0100
Subject: [PATCH 14/16] remove redundant snapshot

---
 src/dodal/devices/oav/snapshot.py | 42 -------------------------------
 1 file changed, 42 deletions(-)
 delete mode 100644 src/dodal/devices/oav/snapshot.py

diff --git a/src/dodal/devices/oav/snapshot.py b/src/dodal/devices/oav/snapshot.py
deleted file mode 100644
index a471e4eba05..00000000000
--- a/src/dodal/devices/oav/snapshot.py
+++ /dev/null
@@ -1,42 +0,0 @@
-import threading
-from pathlib import Path
-
-import requests
-from ophyd import Component, Device, DeviceStatus, EpicsSignal, EpicsSignalRO, Signal
-from PIL import Image
-
-
-class Snapshot(Device):
-    filename: Signal = Component(Signal)
-    directory: Signal = Component(Signal)
-    url: EpicsSignal = Component(EpicsSignal, "JPG_URL_RBV", string=True)
-    x_size_pv: EpicsSignalRO = Component(EpicsSignalRO, "ArraySize1_RBV")
-    y_size_pv: EpicsSignalRO = Component(EpicsSignalRO, "ArraySize2_RBV")
-    input_rbpv: EpicsSignalRO = Component(EpicsSignalRO, "NDArrayPort_RBV")
-    input_plugin: EpicsSignal = Component(EpicsSignal, "NDArrayPort")
-    KICKOFF_TIMEOUT: float = 10.0
-
-    def trigger(self):
-        st = DeviceStatus(device=self, timeout=self.KICKOFF_TIMEOUT)
-        url_str = self.url.get()
-        filename_str = self.filename.get()
-        directory_str = self.directory.get()
-
-        def get_snapshot():
-            try:
-                response = requests.get(url_str, stream=True)
-                response.raise_for_status()
-                image = Image.open(response.raw)
-                image_path = Path(f"{directory_str}/{filename_str}.png")
-                image.save(image_path)
-                self.post_processing(image)
-                st.set_finished()
-            except requests.HTTPError as e:
-                st.set_exception(e)
-
-        threading.Thread(target=get_snapshot, daemon=True).start()
-
-        return st
-
-    def post_processing(self, image: Image.Image):
-        pass

From 4b4bf94f1d8bd107ad432ae1269e07e179c5c26d Mon Sep 17 00:00:00 2001
From: David Perl <david.perl@diamond.ac.uk>
Date: Fri, 12 May 2023 10:06:56 +0100
Subject: [PATCH 15/16] make keyword names consistent

---
 src/dodal/devices/oav/oav_parameters.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/dodal/devices/oav/oav_parameters.py b/src/dodal/devices/oav/oav_parameters.py
index ddcec6b74ad..9fd9412272e 100644
--- a/src/dodal/devices/oav/oav_parameters.py
+++ b/src/dodal/devices/oav/oav_parameters.py
@@ -11,14 +11,14 @@
 
 OAV_CONFIG_FILE_DEFAULTS = {
     "zoom_params_file": "/dls_sw/i03/software/gda_versions/gda_9_27/workspace_git/gda-mx.git/configurations/i03-config/xml/jCameraManZoomLevels.xml",
-    "oav_json": "/dls_sw/i03/software/gda_versions/gda_9_27/workspace_git/gda-mx.git/configurations/i03-config/etc/OAVCentring.json",
+    "oav_config_json": "/dls_sw/i03/software/gda_versions/gda_9_27/workspace_git/gda-mx.git/configurations/i03-config/etc/OAVCentring.json",
     "display_config": "/dls_sw/i03/software/gda_versions/var/display.configuration",
 }
 
 
 class OAVParameters:
     zoom_params_file: str
-    oav_json: str
+    oav_config_json: str
     display_config: str
     global_params: dict[str, Any]
     context_dicts: dict[str, dict]
@@ -45,7 +45,7 @@ def __init__(
         self,
         context="loopCentring",
         zoom_params_file=OAV_CONFIG_FILE_DEFAULTS["zoom_params_file"],
-        oav_config_json=OAV_CONFIG_FILE_DEFAULTS["oav_json"],
+        oav_config_json=OAV_CONFIG_FILE_DEFAULTS["oav_config_json"],
         display_config=OAV_CONFIG_FILE_DEFAULTS["display_config"],
     ):
         self.zoom_params_file = zoom_params_file

From bbf3b6b6add953561202a745d53b454cc72e30fd Mon Sep 17 00:00:00 2001
From: David Perl <david.perl@diamond.ac.uk>
Date: Fri, 12 May 2023 10:29:37 +0100
Subject: [PATCH 16/16] fix test patch location

---
 tests/devices/unit_tests/test_oav.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/devices/unit_tests/test_oav.py b/tests/devices/unit_tests/test_oav.py
index 46c4c89f83a..0d3a65785ab 100644
--- a/tests/devices/unit_tests/test_oav.py
+++ b/tests/devices/unit_tests/test_oav.py
@@ -51,7 +51,7 @@ def test_snapshot_trigger_loads_correct_url(
 
 
 @patch("requests.get")
-@patch("dodal.devices.oav.snapshot.Image.open")
+@patch("dodal.devices.areadetector.plugins.MJPG.Image.open")
 def test_snapshot_trigger_saves_to_correct_file(
     mock_open: MagicMock, mock_get, fake_oav
 ):
@@ -70,7 +70,7 @@ def test_snapshot_trigger_saves_to_correct_file(
 
 
 @patch("requests.get")
-@patch("dodal.devices.oav.snapshot.Image.open")
+@patch("dodal.devices.areadetector.plugins.MJPG.Image.open")
 @patch("dodal.devices.oav.grid_overlay.add_grid_overlay_to_image")
 @patch("dodal.devices.oav.grid_overlay.add_grid_border_overlay_to_image")
 def test_correct_grid_drawn_on_image(