From cab960fe2ce52f610d5e9a8ca670c3e3e5331ef2 Mon Sep 17 00:00:00 2001
From: Joe Knapper <joe.knapper@glasgow.ac.uk>
Date: Fri, 8 May 2026 15:45:49 +0000
Subject: [PATCH] Apply suggestions from code review of branch fom-af

Co-authored-by: Julian Stirling <julian@julianstirling.co.uk>
---
 .../things/autofocus.py                       | 41 +++++++++++--------
 .../things/camera/picamera.py                 |  3 +-
 2 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/src/openflexure_microscope_server/things/autofocus.py b/src/openflexure_microscope_server/things/autofocus.py
index b7c92b5e..33207b41 100644
--- a/src/openflexure_microscope_server/things/autofocus.py
+++ b/src/openflexure_microscope_server/things/autofocus.py
@@ -37,8 +37,16 @@ class NotStreamingError(RuntimeError):
 class SharpnessMethod(enum.Enum):
     """The possible SharpnessMethods for autofocus."""
 
-    JPEG = enum.auto()
-    FOCUS_FOM = enum.auto()
+class SharpnessMethod(enum.Enum):
+    """The possible SharpnessMethods for autofocus.
+
+    Use powers of two for the methods so they can be selected bitwise. This allows choosing what to record
+    as the sum of methods.
+    """
+
+    JPEG = 1
+    FOCUS_FOM = 2
+    # Next method should be 4 not 3 for bitwise selection.
 
 
 class AutofocusParams(BaseModel):
@@ -260,6 +268,7 @@ class JPEGSharpnessMonitor:
         stage: BaseStage,
         camera: BaseCamera,
         method: SharpnessMethod = SharpnessMethod.JPEG,
+        record: Optional[int] = None,
     ) -> None:
         """Initialise a new JPEGSharpnessMonitor. The args are injected automatically.
 
@@ -270,6 +279,13 @@ class JPEGSharpnessMonitor:
         self.camera = camera
         self.stage = stage
         self.method = method
+        self.record = method if record is None else record
+
+        if not self.method & self.record:
+            raise ValueError(f"The sharpness metric `self.record` is not being recorded.")
+
+        if self.record & SharpnessMethod.FOCUS_FOM and not camera.supports_focus_fom:
+            raise ValueError("Cannot record focus FOM as this camera doesn't support it.")
         LOGGER.debug(f"Created sharpness monitor with {stage}, {camera}")
         self._stage_positions: list[Mapping[str, int]] = []
         self._stage_times: list[float] = []
@@ -311,14 +327,13 @@ class JPEGSharpnessMonitor:
             self._jpeg_times.append(time.time())
 
             # JPEG sharpness metric
-            self._jpeg_sizes.append(len(frame))
+            if self.record & SharpnessMethod.JPEG:
+                self._jpeg_sizes.append(len(frame))
 
             # FocusFoM metric
-            fom = getattr(self.camera, "_focus_fom", None)
-            if fom is not None:
-                self._focus_foms.append(float(fom))
-            else:
-                self._focus_foms.append(np.nan)
+            if self.record & SharpnessMethod.FOCUS_FOM:
+                self._focus_foms.append(self.camera._focus_fom)
+
             if not self.running:
                 break
 
@@ -381,15 +396,6 @@ class JPEGSharpnessMonitor:
         elif self.method == SharpnessMethod.FOCUS_FOM:
             sharpnesses = np.array(self.focus_foms)
 
-            # If no valid FocusFoM values were recorded, fall back to JPEG size
-            if np.all(np.isnan(self.focus_foms)):
-                LOGGER.warning(
-                    "Sharpness method FOCUS_FOM selected, but no FocusFoM values "
-                    "were recorded. Falling back to JPEG sharpness."
-                )
-                sharpnesses = np.array(self.jpeg_sizes)
-        else:
-            raise ValueError(f"Unknown sharpness method: {self.method}")
         stage_times: np.ndarray = np.array(self.stage_times)[istart:istop]
         stage_heights: np.ndarray = np.array(
             [p["z"] for p in self.stage_positions[istart:istop]]
@@ -458,6 +464,7 @@ class AutofocusThing(lt.Thing):
         dz: int = 2000,
         start: Literal["centre", "base"] = "centre",
         sharpness_metric: SharpnessMethod = SharpnessMethod.JPEG,
+    record: Optional[Int]
     ) -> SharpnessDataArrays:
         """Sweep the stage up and down, then move to the sharpest point.
 
diff --git a/src/openflexure_microscope_server/things/camera/picamera.py b/src/openflexure_microscope_server/things/camera/picamera.py
index 41c45313..d0b02f5f 100644
--- a/src/openflexure_microscope_server/things/camera/picamera.py
+++ b/src/openflexure_microscope_server/things/camera/picamera.py
@@ -123,7 +123,8 @@ class StreamingPiCamera2(BaseCamera):
     Currently the Thing only supports the PiCamera v2 board. This needs
     generalisation.
     """
-
+    _focus_fom: int
+    supports_focus_fom: bool = True
     tuning: dict = lt.setting(default_factory=dict, readonly=True)
     """The Raspberry PiCamera Tuning File JSON."""