Merge pull request #93 from DiamondLightSource/fix-misc-issues

- Fix broken offset plot
- Fix enabling of FOFB
- Improve "run topup?" question text
- Save offsets plot after each bba run as intended
- Fix GUI buttons to enable/disable saving of plots/rawdata/results
- Fix the ability to change the save location from the GUI
- Tidying of various bits of relevant code

Author: ptsOSL

src/dls_bba/algorithm.py

@@ -193,7 +193,12 @@ def use_bba_offsets(
             question_callback: The question function for the console or gui.
         """
         offsets_dict = self.reformat_and_save_offsets(results_list, save_location)
-        bba_offsets_plot(self._machine, offsets_dict, save_location)
+        bba_offsets_plot(
+            self._machine,
+            offsets_dict,
+            save_location,
+            self._machine.config["SAVE_PLOTS"],
+        )
         if question_callback("Apply these BBA offsets? (y / n) :"):
             self.apply_bba_offsets(offsets_dict)
             self._machine.apply_feedbacks()

src/dls_bba/beam_current.py

@@ -76,7 +76,8 @@ def topup_beam(self, minimum_topup: float) -> None:
         while True:
             log.error(f"Please topup current to > {minimum_topup}mA.")
             response = self.question_callback(
-                "Input y to continue after topup, or n to cancel:"
+                "Beam current dropped, please topup. Input y to continue after topup, "
+                "or n to cancel:"
             )
 
             if response:

src/dls_bba/defaults.json

@@ -8,7 +8,7 @@
     "CRITICAL_CURRENT_DROP": 100,
     "SOFB_RUN_TIME": 10,
     "FEEDBACK_WAIT_TIME": 3,
-    "FEEDBACK_RUN_TIME": 3,
+    "FOFB_RUN_TIME": 3,
     "MAX_ORBIT_CORRECTION_MICRONS": 7.5,
     "MIN_SLOPE_FRACTION": 0.25,
     "MIN_CURRENT": 295,

src/dls_bba/fbba_gui.ui

@@ -891,7 +891,7 @@
            <string>Feedbacks Wait time [s]</string>
           </property>
          </widget>
-         <widget class="QDoubleSpinBox" name="config_run_time">
+         <widget class="QDoubleSpinBox" name="config_fofb_run_time">
           <property name="geometry">
            <rect>
             <x>10</x>

src/dls_bba/gui.py

@@ -226,7 +226,7 @@ class MainWindow(QMainWindow):
     config_quad_step: QDoubleSpinBox
     config_warning_current: QDoubleSpinBox
     config_sofb_run_time: QDoubleSpinBox
-    config_run_time: QDoubleSpinBox
+    config_fofb_run_time: QDoubleSpinBox
     config_wait_time: QDoubleSpinBox
     config_sbba_min_frac: QDoubleSpinBox
     config_sbba_stdev: QDoubleSpinBox
@@ -288,7 +288,6 @@ def __init__(self, *args, **kwargs) -> None:
         self.selection_strings: list[str] | None = None
         self.selected: list[str] | str | None = None
         self.loadfolder: str | None = None
-        self.savepath: str | None = None
         self.loadfile: str | None = None
         self.selected_toggle: int = 0
         self.tmp_single_filepath: str | None = None
@@ -476,14 +475,17 @@ def create_worker(self) -> Worker:
         if isinstance(self.selected, str):
             self.selected = [self.selected]
         assert isinstance(self.selected, list) and isinstance(self.selected[0], str)
+
+        latest_config = self.get_config_from_gui()
+
         return Worker(
             method,
             self.selected,
             self.create_question_box,
             machine=self.machine,
-            folder_path=self.machine.config["SAVE_LOCATION"],
+            folder_path=latest_config["SAVE_LOCATION"],
             logger=self.logger,
-            additional_options=self.get_config_from_gui(),
+            additional_options=latest_config,
         )
 
     def reset_iocs(self) -> None:
@@ -551,8 +553,6 @@ def load_config_file(self) -> None:
         list_file = [file[0]]
         self.machine.update_config(extra_config_files=list_file)
         self.show_config()
-        # Allow for lattice reload.
-        cothread.Yield()  # TODO: Why does this need to yield?
         self.display_config_load.setText(f"Config File Applied at {get_isotime()}")
 
     def get_ringmode_options(self) -> list[str]:
@@ -574,36 +574,41 @@ def get_config_from_gui(self) -> dict[str, Any]:
             A dictionary of the new config.
         """
         config_dict = {
-            "SAVE_LOCATION": self.display_save_loc.toPlainText(),
+            # Main Page
             "USE_FEEDBACKS": self.config_use_feedbacks.isChecked(),
             "FOFB_FEEDBACKS": self.config_use_fofb.isChecked(),
             "MAX_ORBIT_CORRECTION_MICRONS": self.config_max_orbit.value(),
             "MIN_CURRENT": self.config_current_limit.value(),
+            ## Additional Options and Plotting#
+            # General Options
             "CORRECTOR_KICK_RADIANS": self.config_corr_kick.value() / RAD_TO_URAD_CONV,
             "QUADRUPOLE_STEP_PERCENT": self.config_quad_step.value(),
             "WARNING_CURRENT_DROP": self.config_warning_current.value(),
-            "FEEDBACK_WAIT_TIME": self.config_wait_time.value(),
-            "FEEDBACK_RUN_TIME": self.config_run_time.value(),
             "SOFB_RUN_TIME": self.config_sofb_run_time.value(),
-            "MIN_SLOPE_FRACTION": self.config_sbba_min_frac.value(),
-            "CENTER_OUTLIER_FACTOR": self.config_sbba_stdev.value(),
-            "OUTLIER_FACTOR": self.config_sbba_fit_diff.value(),
+            "FOFB_RUN_TIME": self.config_fofb_run_time.value(),
+            "FEEDBACK_WAIT_TIME": self.config_wait_time.value(),
             "DECIMATED": self.config_use_decimation.isChecked(),
             "X_CYCLES": self.config_x_cycles.value(),
             "X_FREQUENCY": self.config_x_freq.value(),
             "Y_CYCLES": self.config_y_cycles.value(),
             "Y_FREQUENCY": self.config_y_freq.value(),
+            "RESELECTION_LIMIT": self.config_reselection.value(),
             "SAVE_RAWDATA": self.save_rawdata.isChecked(),
             "SAVE_RESULTS": self.save_results.isChecked(),
             "SAVE_PLOTS": self.save_plots.isChecked(),
-            "RESELECTION_LIMIT": self.config_reselection.value(),
+            # Advanced Options
             "RINGMODE": self.config_ringmode.currentText(),
             "COTHREAD_CONTROL_SYSTEM_TIMEOUT": self.config_ccs_timeout.value(),
             "COTHREAD_CONTROL_SYSTEM_WAIT_FLAG": self.config_ccs_wait.isChecked(),
-            "FOFB_EXECUTABLE_PATH": self.config_fofb_executable_path.toPlainText(),
+            "MIN_SLOPE_FRACTION": self.config_sbba_min_frac.value(),
+            "OUTLIER_FACTOR": self.config_sbba_fit_diff.value(),
+            "CENTER_OUTLIER_FACTOR": self.config_sbba_stdev.value(),
             "FOFB_MAX_ORBIT_MICRONS": self.config_fofb_max_orbit.value(),
             "ORBIT_RESPONSE_MATRIX_PATH": self.config_orm_path.toPlainText(),
             "CORRECTORS_TXT_PATH": self.config_corrector_txt_path.toPlainText(),
+            "FOFB_EXECUTABLE_PATH": self.config_fofb_executable_path.toPlainText(),
+            # Save Location and Config
+            "SAVE_LOCATION": self.display_save_loc.toPlainText(),
         }
         return config_dict
 
@@ -629,7 +634,7 @@ def show_config(self) -> None:
         self.config_quad_step.setValue(config["QUADRUPOLE_STEP_PERCENT"])
         self.config_warning_current.setValue(config["WARNING_CURRENT_DROP"])
         self.config_wait_time.setValue(config["FEEDBACK_WAIT_TIME"])
-        self.config_run_time.setValue(config["FEEDBACK_RUN_TIME"])
+        self.config_fofb_run_time.setValue(config["FOFB_RUN_TIME"])
         self.config_sofb_run_time.setValue(config["SOFB_RUN_TIME"])
         self.config_sbba_min_frac.setValue(config["MIN_SLOPE_FRACTION"])
         self.config_sbba_stdev.setValue(config["CENTER_OUTLIER_FACTOR"])
@@ -703,11 +708,9 @@ def select_save_location_folder(self) -> None:
             self, "Select Folder to Save to", self.machine.config["SAVE_LOCATION"]
         )
         if folderpath == "":
-            self.display_save_loc.clear()
-            self.display_save_loc.setPlainText(self.machine.config["SAVE_LOCATION"])
-        else:
-            self.display_save_loc.setPlainText(folderpath)
-            self.savepath = folderpath
+            folderpath = self.machine.config["SAVE_LOCATION"]
+
+        self.display_save_loc.setPlainText(folderpath)
 
     def select_bba_folder(self) -> None:
         """Select a folder of old data to load."""

src/dls_bba/machine.py

@@ -503,8 +503,8 @@ def max_orbit_too_big_for_fofb(self) -> None:
         """Check if the maximum orbit is too large for FOFB and run SOFB if so."""
         fofb_max_orbit = self.config["FOFB_MAX_ORBIT_MICRONS"]
         max_value = self.get_largest_orbit()
-        while max_value >= fofb_max_orbit:
-            log.error("Orbit is too large for FOFB. Running SOFB.")
+        while max_value > fofb_max_orbit:
+            log.warning("Orbit is too large for FOFB. Running SOFB.")
             self.run_sofb()
             max_value = self.get_largest_orbit()
 
@@ -524,9 +524,9 @@ def run_sofb(self) -> None:
     def run_fofb(self) -> None:
         """Run FOFB and Tune feedbacks."""
         tune_trigger = self.config["FEEDBACK_PVS"]["Tune_Feedback"]
-        fofb_trigger = self.config["FOFB_NOGUI_PATH"]
+        fofb_trigger = self.config["FOFB_EXECUTABLE_PATH"]
         wait_time = self.config["FEEDBACK_WAIT_TIME"]
-        run_time = self.config["FEEDBACK_RUN_TIME"]
+        run_time = self.config["FOFB_RUN_TIME"]
         run(f"{fofb_trigger} start", check=True, shell=True)
         caput(tune_trigger, 1, wait=True)
 

src/dls_bba/plotting.py

@@ -58,12 +58,12 @@ def bba_offsets_plot(
     Returns:
         The figure object.
     """
-    x = np.arange(1, len(machine.bba_x_pvs) + 1)
+    x = np.arange(1, len(machine.bpms_names) + 1)
     change_in_x = []
     change_in_dx = []
-    for bpm_name in machine.bba_x_pvs:
-        if bpm_name.replace("-", "_").replace(":", "__") in offsets_dict.keys():
-            calc_offsets = offsets_dict[bpm_name.replace("-", "_").replace(":", "__")].x
+    for bpm_name in machine.bpms_names:
+        if bpm_name in offsets_dict.keys():
+            calc_offsets = offsets_dict[bpm_name].x
             change_in_x.append(calc_offsets.diff_value * MM_TO_UM_UNIT_CONV)  # type: ignore
             change_in_dx.append(abs(calc_offsets.diff_value * MM_TO_UM_UNIT_CONV))  # type: ignore
         else:
@@ -72,14 +72,15 @@ def bba_offsets_plot(
 
     change_in_y = []
     change_in_dy = []
-    for bpm_name in machine.bba_y_pvs:
-        if bpm_name.replace("-", "_").replace(":", "__") in offsets_dict.keys():
-            calc_offsets = offsets_dict[bpm_name.replace("-", "_").replace(":", "__")].y
+    for bpm_name in machine.bpms_names:
+        if bpm_name in offsets_dict.keys():
+            calc_offsets = offsets_dict[bpm_name].y
             change_in_y.append(calc_offsets.diff_value * MM_TO_UM_UNIT_CONV)  # type: ignore
             change_in_dy.append(abs(calc_offsets.diff_value * MM_TO_UM_UNIT_CONV))  # type: ignore
         else:
             change_in_y.append(0)
             change_in_dy.append(0)
+
     fig, (ax1, ax2) = plt.subplots(2, sharex=True, tight_layout=True)
     fig.suptitle("Change in BBA values")
     ax1.set_xlim(0, 174)

src/dls_bba/worker.py

@@ -95,7 +95,7 @@ def work(self) -> float:
                 break
 
         if rawdata is not None:
-            if self.save_rawdata is not None:
+            if self.save_rawdata:
                 rawdata.save(self.save_location)
 
             results = self.algorithm.analyse(rawdata)