[refactor] updated cornercase + feedback

Author: yxkdejong

src/odemis/driver/andorshrk.py

@@ -1600,10 +1600,10 @@ def _getGratingChoices(self):
 
         return gchoices
 
-    def GetGoffset(self):
+    def GetGoffset(self) -> float:
         """
         Checks the current grating and flip-mirror positions.
-        Returns the grating offset, consisting of the grating_offset + the detector offset.
+        Returns the grating offset, consisting of the grating_offset + the detector offset (float).
 
         The detector offset is the equivalent grating offset that is needed to compensate for the change in
         optical path length, introduced by the flip-mirrors.
@@ -2097,21 +2097,17 @@ def _doSetFlipper(self, flipper: int, pos: int, check: bool):
             logging.warning("Failed to update turret position, detector offset might be incorrect", exc_info=True)
         self._updatePosition()
 
-
-    def _doSetGoffsetAbs(self, target_offset, *, allow_grating_offset=True, single_detector_mode = False):
-
-        """
-        Change grating offset, by either changing the grating offset or the detector offset.
-        :param target_offset (float): the new grating offset to set
-        :param allow_grating_offset (bool): check to allow changing the grating offset, if false, only change
-        detector offset.
-        :param single_detector_mode (bool): if true, it will always change the grating offset,
-        even if the output flipper is not in the direct port position.
+    def _doSetGoffsetAbs(self, target_offset: float) -> None:
         """
+        Sets the absolute grating offset (goffset) for the current configuration.
+        The goffset is the sum of the grating offset for the currently selected grating and
+        the detector offset associated with the current flip-mirror positions.
 
+        For the first detector to be calibrated (or if there's only one available) the grating offset is adjusted for
+        each grating. For the other detector, only the detector offset will be adjusted.
+        """
         target_offset = int(round(target_offset))  # ensure that we get integers for steps
         grating = self.GetGrating()
-        port_index = self.GetFlipperMirror(OUTPUT_FLIPPER)
 
         if "flip-in" in self.axes:
             flip_in_pos = self.GetFlipperMirror(INPUT_FLIPPER)
@@ -2123,38 +2119,55 @@ def _doSetGoffsetAbs(self, target_offset, *, allow_grating_offset=True, single_d
         else:
             flip_out_pos = DIRECT_PORT
 
-        single_detector = bool(single_detector_mode)
         current_grat_offset = self.GetGratingOffset(grating)
         current_det_offset = self.GetDetectorOffset(flip_in_pos, flip_out_pos)
+
         logging.debug("Current goffset: %d (Grat: %d, Det: %d)",
                       (current_grat_offset + current_det_offset),
                       current_grat_offset, current_det_offset)
 
-        # The detector offset compensates for small naccuracies introduced by the flip-mirror mechanism.
+        # The detector offset compensates for small inaccuracies introduced by the flip-mirror mechanism.
         # This value is normally stable and seldom requires re-adjustment.
 
-        if port_index == 0 or single_detector:
-            logging.debug(
-                "Choosing grating offset update (port_index=%s single_detector=%s)",
-                port_index, single_detector
-            )
+        # Get all detectors
+        input_ports = [DIRECT_PORT]
+        if "flip-in" in self.axes:
+            input_ports.append(self.GetFlipperMirror(INPUT_FLIPPER))
 
-            # primary detector -> modify grating offset
-            if not allow_grating_offset:
-                logging.debug("Grating offset update disabled (grating=1, target=%d)", target_offset, )
-            else:
-                grating_offset = target_offset - current_det_offset
-                self.SetGratingOffset(grating, grating_offset)
+        output_ports = [DIRECT_PORT]
+        if "flip-out" in self.axes:
+            output_ports.append(self.GetFlipperMirror(OUTPUT_FLIPPER))
+
+        all_detectors = [(i, o) for i in input_ports for o in output_ports]
+
+        current_detector = (flip_in_pos, flip_out_pos)
+        reference_detector = all_detectors[0]
+        is_multi_detector = len(all_detectors) > 1
+
+        # Apply offsets
+        # For the first detector (doesn't matter to which port it's connected), the grating offset is adjusted for
+        # all gratings. For the second detector, only the detector offset is adjusted.
+
+        if not is_multi_detector:
+            logging.debug("Single detector system -> updating grating offset")
+            self.SetGratingOffset(grating, target_offset)
+
+        elif current_detector == reference_detector:
+            logging.debug("Reference detector -> updating grating offset")
+            self.SetGratingOffset(grating, target_offset)
 
-        # secondary detector (if multiple detectors) -> modify detector offset
         else:
-            logging.debug(
-                "Choosing detector offset update (port_index=%s single_detector=%s)",
-                port_index, single_detector
-            )
+            logging.debug("Secondary detector -> updating detector offset")
             detector_offset = target_offset - current_grat_offset
             self.SetDetectorOffset(flip_in_pos, flip_out_pos, detector_offset)
 
+        new_grat_offset = self.GetGratingOffset(grating)
+        new_det_offset = self.GetDetectorOffset(flip_in_pos, flip_out_pos)
+
+        logging.info(
+            "Updated offsets -> Grating: %d, Detector: %d, Combined goffset: %d",
+            new_grat_offset, new_det_offset, new_grat_offset + new_det_offset)
+
         self._updatePosition()
 
     def _doSetGoffsetRel(self, shift):

src/odemis/driver/test/andorshrk_test.py

@@ -590,12 +590,16 @@ def test_iris(self):
         sp.moveAbsSync({"iris-in": orig_pos})
 
     def test_goffset(self):
+        """
+        Test relative and absolute moves of the goffset,
+        and check that it works for different gratings (if available).
+        """
         self.assertIn("goffset", self.spectrograph.axes)
         sp = self.spectrograph
         rng = sp.axes["goffset"].range
 
         orig_offsets = {}
-        orig_grating = sp.position.value["goffset"]
+        orig_grating = sp.position.value["grating"]
 
         for g in sp.axes["grating"].choices:
             sp.moveAbsSync({"grating": g})