From caa31e504acaca316e6ceb5f5b8af536109a2cd4 Mon Sep 17 00:00:00 2001
From: Johannes Schabbauer <johannes.schabbauer@tuwien.ac.at>
Date: Tue, 3 Dec 2024 14:10:24 +0100
Subject: [PATCH 1/6] Added starting value to ADwin PID controls

---
 ADwinProII/blacs_workers.py                   |  1 +
 ADwinProII/labscript_devices_ADwin_modules.py | 23 ++++++++++++++++---
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/ADwinProII/blacs_workers.py b/ADwinProII/blacs_workers.py
index 8271e02..d69de1f 100644
--- a/ADwinProII/blacs_workers.py
+++ b/ADwinProII/blacs_workers.py
@@ -169,6 +169,7 @@ class ADwinProIIWorker(Worker):
                 self.adw.SetData_Float(PIDs["PID_D"], 27, 1, PIDs.shape[0])
                 self.adw.SetData_Long(PIDs["PID_min"], 28, 1, PIDs.shape[0])
                 self.adw.SetData_Long(PIDs["PID_max"], 29, 1, PIDs.shape[0])
+                self.adw.SetData_Long(PIDs["PID_start"], 30, 1, PIDs.shape[0])
             AIN = group["ANALOG_IN/TIMES"]
             if fresh or not np.array_equal(AIN[:],self.smart_cache["AIN"]):
                 print("AIN programmed.")
diff --git a/ADwinProII/labscript_devices_ADwin_modules.py b/ADwinProII/labscript_devices_ADwin_modules.py
index 3872563..a73cb8f 100644
--- a/ADwinProII/labscript_devices_ADwin_modules.py
+++ b/ADwinProII/labscript_devices_ADwin_modules.py
@@ -97,7 +97,7 @@ class ADwinAnalogOut(AnalogOut):
         self.PID = {} # instructions for PID settings
 
 
-    def set_PID(self,t,PID_AnalogIn,P=0,I=0,D=0,limits=None):
+    def set_PID(self,t,PID_AnalogIn,P=0,I=0,D=0,limits=None, start=0):
         """(De-)activate PID for analog output, or change settings
         
         Parameters
@@ -115,12 +115,20 @@ class ADwinAnalogOut(AnalogOut):
             Differential parameter
         limits : tuple of float, optional
             Limits for output voltage, defaults to output limits
+        start : float or "last"
+            Inital value of the output when the PID is turned on, assigned to the I part.
+            If start="last" when the PID is turned on, the I value from the PID earlier 
+            in the shot is kept. If start="last" when the PID is turned off, the last output
+            value is kept as "set_target".
         """
 
         if limits is None:
             # Use the Output limits if there are none specified here.
             limits = self.limits
-
+        if limits[0]<self.limits[0] or limits[1]>self.limits[1]:
+            raise LabscriptError(f"Limits of {self.name} with PID must not be larger than channel limits {self.limits}!")
+        
+        # TURN OFF PID
         if PID_AnalogIn is None:
             self.PID[t] = {
                 "PID_channel": 0,
@@ -128,7 +136,9 @@ class ADwinAnalogOut(AnalogOut):
                 "I": I,
                 "D": D,
                 "limits": limits,
+                "start": start,
             }
+        # TURN ON PID
         elif (isinstance(PID_AnalogIn,AnalogIn) and isinstance(PID_AnalogIn.parent_device,_ADwinCard)) \
                 or isinstance(PID_AnalogIn,int):
             self.PID[t] = {
@@ -137,6 +147,7 @@ class ADwinAnalogOut(AnalogOut):
                 "I": I,
                 "D": D,
                 "limits": limits,
+                "start": start,
             }
             # TODO: Do we need scale factors for setting a PID with integer?
         else:
@@ -344,7 +355,7 @@ class ADwinAO8(_ADwinCard):
         output_dtypes = [("n_cycles",np.int32),("channel",np.int32),("value",np.int32)]
         PID_dtypes = [
             ("n_cycles",np.int32),("AOUT_channel",np.int32),("PID_channel",np.int32),
-            ("PID_P",np.float64),("PID_I",np.float64),("PID_D",np.float64),("PID_min",np.int32),("PID_max",np.int32)
+            ("PID_P",np.float64),("PID_I",np.float64),("PID_D",np.float64),("PID_min",np.int32),("PID_max",np.int32),("PID_start",np.int32)
             ]
         outputs = []
         PID_channels = []
@@ -380,6 +391,12 @@ class ADwinAO8(_ADwinCard):
                     limits = output.PID[t]['limits']
                     PID_array["PID_min"][i] = ADC(limits[0],self.resolution_bits,self.min_V,self.max_V)
                     PID_array["PID_max"][i] = ADC(limits[1],self.resolution_bits,self.min_V,self.max_V)
+                    if output.PID[t]["start"]=="last":
+                        # When we want to use the previous value during the shot,
+                        # we use a value that's out of the 16 bit ADC range to identify.
+                        PID_array["PID_start"][i] = 100_000
+                    else:
+                        PID_array["PID_start"][i] = ADC(output.PID[t]['start'],self.resolution_bits,self.min_V,self.max_V)
             # The ADwin has 16 bit output resolution, so we quantize the Voltage to the right values
             quantized_output = ADC(output.raw_output,self.resolution_bits,self.min_V,self.max_V)
             out = np.empty(quantized_output.size,dtype=output_dtypes)
-- 
GitLab


From 6e745c9a932f6089b22b483b117fee3a28bf277e Mon Sep 17 00:00:00 2001
From: Johannes Schabbauer <johannes.schabbauer@tuwien.ac.at>
Date: Fri, 6 Dec 2024 09:41:08 +0100
Subject: [PATCH 2/6] ADwin PIDs: Changed default start value to keep
 'backwards compatibility'

---
 ADwinProII/labscript_devices_ADwin_modules.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ADwinProII/labscript_devices_ADwin_modules.py b/ADwinProII/labscript_devices_ADwin_modules.py
index a73cb8f..29ef0f9 100644
--- a/ADwinProII/labscript_devices_ADwin_modules.py
+++ b/ADwinProII/labscript_devices_ADwin_modules.py
@@ -97,7 +97,7 @@ class ADwinAnalogOut(AnalogOut):
         self.PID = {} # instructions for PID settings
 
 
-    def set_PID(self,t,PID_AnalogIn,P=0,I=0,D=0,limits=None, start=0):
+    def set_PID(self,t,PID_AnalogIn,P=0,I=0,D=0,limits=None, start="last"):
         """(De-)activate PID for analog output, or change settings
         
         Parameters
-- 
GitLab


From 7d94255f870a48937eb0c3b1dec0023a684bfeb0 Mon Sep 17 00:00:00 2001
From: Runner PC Cavity Lab <johannes.schabbauer@tuwien.ac.at>
Date: Mon, 9 Dec 2024 13:12:43 +0100
Subject: [PATCH 3/6] Adwin: Set PID gains and limits only once during init and
 not later when it's activated. Added extra dataset in shot file containing
 thise PID config.

---
 ADwinProII/blacs_workers.py                   |  19 +++-
 ADwinProII/labscript_devices.py               |   7 +-
 ADwinProII/labscript_devices_ADwin_modules.py | 104 +++++++++++-------
 3 files changed, 83 insertions(+), 47 deletions(-)

diff --git a/ADwinProII/blacs_workers.py b/ADwinProII/blacs_workers.py
index d69de1f..5b8cec3 100644
--- a/ADwinProII/blacs_workers.py
+++ b/ADwinProII/blacs_workers.py
@@ -29,7 +29,7 @@ class ADwinProIIWorker(Worker):
     def init(self):
         self.timing = None
         self.h5file = None
-        self.smart_cache = {"AOUT":None, "PIDs":None, "AIN":None}
+        self.smart_cache = {"AOUT":None, "PIDs":None, "PID_CONFIG":None, "AIN":None}
         self.smart_cache.update({DIO:None for DIO in self.DIO_ADwin_DataNo})
         self.process_number_buffered = int(self.process_buffered[-1])
         self.process_number_manual = int(self.process_manual[-1])
@@ -164,12 +164,19 @@ class ADwinProIIWorker(Worker):
                 self.adw.SetData_Long(PIDs["n_cycles"], 4, 1, PIDs.shape[0])
                 self.adw.SetData_Long(PIDs["AOUT_channel"], 5, 1, PIDs.shape[0])
                 self.adw.SetData_Long(PIDs["PID_channel"], 6, 1, PIDs.shape[0])
-                self.adw.SetData_Float(PIDs["PID_P"], 25, 1, PIDs.shape[0])
-                self.adw.SetData_Float(PIDs["PID_I"], 26, 1, PIDs.shape[0])
-                self.adw.SetData_Float(PIDs["PID_D"], 27, 1, PIDs.shape[0])
-                self.adw.SetData_Long(PIDs["PID_min"], 28, 1, PIDs.shape[0])
-                self.adw.SetData_Long(PIDs["PID_max"], 29, 1, PIDs.shape[0])
                 self.adw.SetData_Long(PIDs["PID_start"], 30, 1, PIDs.shape[0])
+            PID_config = group["ANALOG_OUT/PID_CONFIG"]
+            if fresh or not np.array_equal(PID_config[:],self.smart_cache["PID_CONFIG"]):
+                print("PID_CONFIG programmed.")
+                self.smart_cache["PID_CONFIG"] = PID_config[:]
+                n_PID = PID_config.shape[0]
+                self.adw.Set_Par(22,n_PID)
+                self.adw.SetData_Long(PID_config["PID_channel"], 24, 1, n_PID)
+                self.adw.SetData_Float(PID_config["PID_P"], 25, 1, n_PID)
+                self.adw.SetData_Float(PID_config["PID_I"], 26, 1, n_PID)
+                self.adw.SetData_Float(PID_config["PID_D"], 27, 1, n_PID)
+                self.adw.SetData_Long(PID_config["PID_min"], 28, 1, n_PID)
+                self.adw.SetData_Long(PID_config["PID_max"], 29, 1, n_PID)
             AIN = group["ANALOG_IN/TIMES"]
             if fresh or not np.array_equal(AIN[:],self.smart_cache["AIN"]):
                 print("AIN programmed.")
diff --git a/ADwinProII/labscript_devices.py b/ADwinProII/labscript_devices.py
index d1bee26..f74a85b 100644
--- a/ADwinProII/labscript_devices.py
+++ b/ADwinProII/labscript_devices.py
@@ -232,12 +232,14 @@ class ADwinProII(PseudoclockDevice):
         # Lists to collect instructions of analog channels
         analog_output = []
         PID_channels = []
+        PID_config = []
         analog_input = []
  
         for device in self.modules: 
             if isinstance(device,ADwinAO8):
                 analog_output.append(device.outputs)
-                PID_channels.append(device.PID_channels)
+                PID_channels.append(device.PID_table)
+                PID_config.append(device.PID_config)
             elif isinstance(device,ADwinDIO32):
                 group.create_dataset("DIGITAL_OUT/"+device.name, data=device.digital_data)
             elif isinstance(device,ADwinAI8):
@@ -252,14 +254,17 @@ class ADwinProII(PseudoclockDevice):
         PID_channels.append(np.full(1,last_values,dtype=PID_channels[0].dtype))
         # Concatenate arrays
         PID_channels = np.concatenate(PID_channels)
+        PID_config = np.concatenate(PID_config)
         analog_output = np.concatenate(analog_output)
         analog_input = np.concatenate(analog_input)
         # Sort analog outputs and PID settings
         analog_output = np.sort(analog_output, axis=0, order="n_cycles")
         PID_channels = np.sort(PID_channels, axis=0, order="n_cycles")
+        PID_config = np.sort(PID_config, axis=0, order="PID_channel")
         # Save datasets
         AO_group.create_dataset('VALUES', compression=config.compression, data=analog_output)
         AO_group.create_dataset('PID_CHANNELS', compression=config.compression, data=PID_channels)
+        AO_group.create_dataset('PID_CONFIG', compression=config.compression, data=PID_config)
         AI_group.create_dataset('TIMES', data=analog_input)
 
         AI_datapoints = analog_input["stop_time"] - analog_input["start_time"]
diff --git a/ADwinProII/labscript_devices_ADwin_modules.py b/ADwinProII/labscript_devices_ADwin_modules.py
index 29ef0f9..46385ab 100644
--- a/ADwinProII/labscript_devices_ADwin_modules.py
+++ b/ADwinProII/labscript_devices_ADwin_modules.py
@@ -96,17 +96,13 @@ class ADwinAnalogOut(AnalogOut):
         )
         self.PID = {} # instructions for PID settings
 
-
-    def set_PID(self,t,PID_AnalogIn,P=0,I=0,D=0,limits=None, start="last"):
-        """(De-)activate PID for analog output, or change settings
+    def init_PID(self,pid_no,P=0,I=0,D=0,limits=None):
+        """Set parameters for PID once at beginning of shot.
         
         Parameters
         ----------
-        t : float
-            Time when to apply the PID settings
-        PID_AnalogIn : int or `AnalogIn` or None
+        pid_no : int or `AnalogIn`
             Channel of analog input for error siganl of PID feedback.
-            If `None` PID is deactivated.
         P : float
             Proportional parameter
         I : float
@@ -115,38 +111,54 @@ class ADwinAnalogOut(AnalogOut):
             Differential parameter
         limits : tuple of float, optional
             Limits for output voltage, defaults to output limits
-        start : float or "last"
-            Inital value of the output when the PID is turned on, assigned to the I part.
-            If start="last" when the PID is turned on, the I value from the PID earlier 
-            in the shot is kept. If start="last" when the PID is turned off, the last output
-            value is kept as "set_target".
         """
+        if hasattr(self,"pid_no"):
+            raise NotImplementedError("Only one set of PID parameters per channel is implemented.")
 
         if limits is None:
             # Use the Output limits if there are none specified here.
             limits = self.limits
         if limits[0]<self.limits[0] or limits[1]>self.limits[1]:
             raise LabscriptError(f"Limits of {self.name} with PID must not be larger than channel limits {self.limits}!")
+
+        if isinstance(pid_no,AnalogIn) and isinstance(pid_no.parent_device,_ADwinCard):
+            pid_no = int(pid_no.connection) + pid_no.parent_device.start_index
+        self.pid_no = pid_no
+        self.P = P
+        self.I = I
+        self.D = D
+        self.PID_min = limits[0]
+        self.PID_max = limits[1]
+
+
+    def set_PID(self,t,pid_no,start="last"):
+        """(De-)activate PID for analog output, or change settings
+        
+        Parameters
+        ----------
+        t : float
+            Time when to apply the PID settings
+        pid_no : int or `AnalogIn` or None
+            Channel of analog input for error siganl of PID feedback.
+            If `None` PID is deactivated.
+        start : float or "last"
+            Inital value of the output when the PID is turned on, assigned to the I part.
+            If start="last" when the PID is turned on, the I value from the PID earlier 
+            in the shot is kept. If start="last" when the PID is turned off, the last output
+            value is kept as "set_target".
+        """
         
         # TURN OFF PID
-        if PID_AnalogIn is None:
+        if pid_no is None:
             self.PID[t] = {
                 "PID_channel": 0,
-                "P": P,
-                "I": I,
-                "D": D,
-                "limits": limits,
                 "start": start,
             }
         # TURN ON PID
-        elif (isinstance(PID_AnalogIn,AnalogIn) and isinstance(PID_AnalogIn.parent_device,_ADwinCard)) \
-                or isinstance(PID_AnalogIn,int):
+        elif (isinstance(pid_no,AnalogIn) and isinstance(pid_no.parent_device,_ADwinCard)) \
+                or isinstance(pid_no,int):
             self.PID[t] = {
-                "PID_channel": PID_AnalogIn,
-                "P": P,
-                "I": I,
-                "D": D,
-                "limits": limits,
+                "PID_channel": pid_no,
                 "start": start,
             }
             # TODO: Do we need scale factors for setting a PID with integer?
@@ -321,6 +333,7 @@ class ADwinAO8(_ADwinCard):
     def __init__(self, name, parent_device, module_address, num_AO=8, **kwargs):
         self.num_AO = num_AO
         self.start_index = module_start_index[int(module_address)]
+
         super().__init__(name, parent_device, module_address, **kwargs)
 
 
@@ -341,10 +354,9 @@ class ADwinAO8(_ADwinCard):
                         f"The ramp sample rate ({rate_kHz:.0f}kHz) of {output.name} must not be faster than ADwin ({ADwin_rate_kHz:.0f}kHz)."
                     )
         # Check if the PID channel is allowed
-        if np.any(self.PID_channels["PID_channel"] > PIDNO):
-            max_PID_channel = self.PID_channels["PID_channel"].max()
+        if np.any(self.PID_table["PID_channel"] > PIDNO):
             raise LabscriptError(f"ADwin: Setting the PID channel to more than {PIDNO} is not possible!")
-        if np.any(self.PID_channels["PID_channel"] < 0):
+        if np.any(self.PID_table["PID_channel"] < 0):
             raise LabscriptError("ADwin: Setting the PID channel to less than 0 is not possible!")
 
     def generate_code(self,hdf5_file):
@@ -353,25 +365,36 @@ class ADwinAO8(_ADwinCard):
         pseudoclock = clockline.parent_device
 
         output_dtypes = [("n_cycles",np.int32),("channel",np.int32),("value",np.int32)]
-        PID_dtypes = [
-            ("n_cycles",np.int32),("AOUT_channel",np.int32),("PID_channel",np.int32),
-            ("PID_P",np.float64),("PID_I",np.float64),("PID_D",np.float64),("PID_min",np.int32),("PID_max",np.int32),("PID_start",np.int32)
+        PID_config_dtypes = [
+            ("PID_channel",np.int32),("PID_P",np.float64),("PID_I",np.float64),("PID_D",np.float64),("PID_min",np.int32),("PID_max",np.int32)
+        ]
+        PID_table_dtypes = [
+            ("n_cycles",np.int32),("AOUT_channel",np.int32),("PID_channel",np.int32),("PID_start",np.int32)
             ]
         outputs = []
-        PID_channels = []
+        PID_table = []
+        PID_config = []
 
         for output in sorted(self.child_devices, key=lambda dev:int(dev.connection)):
             output.expand_output()
 
             # Get input channels for PID, collect changed for time table and store bare channels as dataset
             if output.PID:
-                PID_array = np.zeros(len(output.PID),dtype=PID_dtypes)
+                # Get PID parameters
+                if not hasattr(output,"pid_no"):
+                    raise LabscriptError(f"For {self.name} you try to use a PID, but never set the parameters via {self.name}.init_PID().")
+                PID_config. append(
+                    np.array([
+                        (output.pid_no,output.P,output.I,output.D,ADC(output.PID_min,self.resolution_bits,self.min_V,self.max_V),ADC(output.PID_max,self.resolution_bits,self.min_V,self.max_V))
+                        ], dtype=PID_config_dtypes)
+                )
+                PID_array = np.zeros(len(output.PID),dtype=PID_table_dtypes)
                 PID_times = np.array(list(output.PID.keys()))
                 PID_times.sort()
                 PID_array["n_cycles"] = np.round(PID_times * pseudoclock.clock_limit)
                 # PID_array["PID_channel"] = list(output.PID_channel.values())
                 PID_array["AOUT_channel"] = int(output.connection) + self.start_index
-                PID_channels.append(PID_array)
+                PID_table.append(PID_array)
                 # If a PID is enabled, the set values are not the actual voltage values of the 
                 # Output, but those measured at the input. If the input has a gain enabled, the
                 # set values have the be scaled too, to have the PID stabilized to the right values.
@@ -385,12 +408,12 @@ class ADwinAO8(_ADwinCard):
                         PID_array["PID_channel"][i] = output.PID[t]['PID_channel']
                     # If PID_channel[t]=None, there are zeros in the PID_array
 
-                    PID_array["PID_P"][i] = output.PID[t]['P']
-                    PID_array["PID_I"][i] = output.PID[t]['I']
-                    PID_array["PID_D"][i] = output.PID[t]['D']
-                    limits = output.PID[t]['limits']
-                    PID_array["PID_min"][i] = ADC(limits[0],self.resolution_bits,self.min_V,self.max_V)
-                    PID_array["PID_max"][i] = ADC(limits[1],self.resolution_bits,self.min_V,self.max_V)
+                    # PID_array["PID_P"][i] = output.PID[t]['P']
+                    # PID_array["PID_I"][i] = output.PID[t]['I']
+                    # PID_array["PID_D"][i] = output.PID[t]['D']
+                    # limits = output.PID[t]['limits']
+                    # PID_array["PID_min"][i] = ADC(limits[0],self.resolution_bits,self.min_V,self.max_V)
+                    # PID_array["PID_max"][i] = ADC(limits[1],self.resolution_bits,self.min_V,self.max_V)
                     if output.PID[t]["start"]=="last":
                         # When we want to use the previous value during the shot,
                         # we use a value that's out of the 16 bit ADC range to identify.
@@ -406,7 +429,8 @@ class ADwinAO8(_ADwinCard):
             outputs.append(out)
 
         self.outputs = np.concatenate(outputs) if outputs else np.array([],dtype=output_dtypes)
-        self.PID_channels = np.concatenate(PID_channels) if PID_channels else np.array([],dtype=PID_dtypes)
+        self.PID_table = np.concatenate(PID_table) if PID_table else np.array([],dtype=PID_table_dtypes)
+        self.PID_config = np.concatenate(PID_config) if PID_config else np.array([],dtype=PID_config_dtypes)
 
 
 
-- 
GitLab


From 189a35f7798d12c3e2a60a9f3c17e562a19ceb59 Mon Sep 17 00:00:00 2001
From: Runner PC Cavity Lab <johannes.schabbauer@tuwien.ac.at>
Date: Tue, 10 Dec 2024 10:41:24 +0100
Subject: [PATCH 4/6] ADwin: Added limit checks for AOUT Added limit check only
 when PID is off and set output when PID is turned off.

---
 ADwinProII/labscript_devices_ADwin_modules.py | 75 +++++++++++++------
 1 file changed, 53 insertions(+), 22 deletions(-)

diff --git a/ADwinProII/labscript_devices_ADwin_modules.py b/ADwinProII/labscript_devices_ADwin_modules.py
index 46385ab..60a86ec 100644
--- a/ADwinProII/labscript_devices_ADwin_modules.py
+++ b/ADwinProII/labscript_devices_ADwin_modules.py
@@ -131,7 +131,7 @@ class ADwinAnalogOut(AnalogOut):
         self.PID_max = limits[1]
 
 
-    def set_PID(self,t,pid_no,start="last"):
+    def set_PID(self,t,pid_no,set_output=0):
         """(De-)activate PID for analog output, or change settings
         
         Parameters
@@ -141,25 +141,34 @@ class ADwinAnalogOut(AnalogOut):
         pid_no : int or `AnalogIn` or None
             Channel of analog input for error siganl of PID feedback.
             If `None` PID is deactivated.
-        start : float or "last"
-            Inital value of the output when the PID is turned on, assigned to the I part.
-            If start="last" when the PID is turned on, the I value from the PID earlier 
-            in the shot is kept. If start="last" when the PID is turned off, the last output
-            value is kept as "set_target".
+        set_value : float or "last"
+            When the PID is turned on, 'set_value' is the initially chosen output value,
+            'last' means that the I value from the previous PID is taken.
+            When the PID is turned off, 'set_value' is programmed as the new output/target value,
+            'last' means that the output from the PID loop is kept as 'set_target'.
         """
+        # Error check of bounds for set_output
+        if set_output!="last":
+            if set_output<self.PID_min or set_output>self.PID_max:
+                raise LabscriptError(
+                    f"{self.name}: PID 'set_output={set_output}' must be within ({self.PID_min},{self.PID_max})"
+                )
         
         # TURN OFF PID
         if pid_no is None:
             self.PID[t] = {
                 "PID_channel": 0,
-                "start": start,
+                "start": set_output,
             }
+            # If we don't keep the PID output, set the output to the set_value
+            if set_output!="last":
+                self.constant(t,set_output)
         # TURN ON PID
         elif (isinstance(pid_no,AnalogIn) and isinstance(pid_no.parent_device,_ADwinCard)) \
                 or isinstance(pid_no,int):
             self.PID[t] = {
                 "PID_channel": pid_no,
-                "start": start,
+                "start": set_output,
             }
             # TODO: Do we need scale factors for setting a PID with integer?
         else:
@@ -203,19 +212,17 @@ class ADwinAnalogOut(AnalogOut):
 
     def add_instruction(self,time,instruction,units=None):
         # Overwrite Output.add_instruction without limit check, becasue the value can be off-limits when this is the target value of the PID
-        # TODO / WARNING: THIS IS QUITE HACKY AND COULD LEAD TO OFF-LIMIT VALUES NOT NOTICED
-        # (the actual limits are also checked in the ADwin, so the actual output should be always within limits!)
         limits_temp = self.limits
-        self.limits = (-10,10)
+        if hasattr(self,"pid_no"):
+            self.limits = (-10,10)
         super().add_instruction(time,instruction,units)
         self.limits = limits_temp
 
     def expand_timeseries(self,all_times,flat_all_times_len):
         # Overwrite Output.add_instruction without limit check, becasue the value can be off-limits when this is the target value of the PID
-        # TODO / WARNING: THIS IS QUITE HACKY AND COULD LEAD TO OFF-LIMIT VALUES NOT NOTICED
-        # (the actual limits are also checked in the ADwin, so the actual output should be always within limits!)
         limits_temp = self.limits
-        self.limits = (-10,10)
+        if hasattr(self,"pid_no"):
+            self.limits = (-10,10)
         super().expand_timeseries(all_times,flat_all_times_len)
         self.limits = limits_temp
 
@@ -353,6 +360,38 @@ class ADwinAO8(_ADwinCard):
                     raise LabscriptError(
                         f"The ramp sample rate ({rate_kHz:.0f}kHz) of {output.name} must not be faster than ADwin ({ADwin_rate_kHz:.0f}kHz)."
                     )
+
+            # Check limits of output, but only when PID is NOT enabled (becasue then the target can be out of limits)
+            # Get all times when PID is not enabled
+            PID = output.PID.copy()
+            if 0 not in PID:
+                # Because 'np.digitize' determines the bins, we also have to make sure t=0 is included.
+                # If output.PID does not have the key t=0, then the PID is disabled in the beginning.
+                PID[0] = {"PID_channel":0,"start":0}
+            PID_times = np.array(list(PID.keys()))
+            PID_times.sort()
+            PID_off_times = []
+            # For each output value, digitize gets the next highest time in  PID_times.
+            # Using '-1' to get next lowest time.
+            for i_out,i_PID in enumerate(np.digitize(output.all_times, PID_times)-1):
+                t = PID_times[i_PID]
+                if PID[t]["PID_channel"]==0:
+                    # When we turn the PID off but keep the last output, we make sure that
+                    #  - at least once in the end the output is (re)set, otherwise the get_final_values() in the Worker is wrong,
+                    #  - don't try to also set the target value at the same time, as this would overwrite the target in the ADwin with the PID output.
+                    if PID[t]["start"]=="last":
+                        if i_out+1==len(output.all_times):
+                            raise LabscriptError(f"{output.name}: You must set the output at the end after turning off PID with 'last'.")
+                        elif output.all_times[i_out] == t:
+                            raise LabscriptError(f"{output.name}: Don't turn off PID with persitent value ('last') and also set new value.")
+                    PID_off_times.append(i_out)
+            PID_off_outputs = output.raw_output[PID_off_times]
+            if np.any(PID_off_outputs < output.limits[0]) or np.any(PID_off_outputs > output.limits[1]):
+                raise LabscriptError(
+                    f"Limits of {output.name} (when PID is off) must be in {output.limits}, " +
+                    f"you try to set ({PID_off_outputs.min()},{PID_off_outputs.max()}) " +
+                    f"or turning off the PID with target value beyond limits.")
+            
         # Check if the PID channel is allowed
         if np.any(self.PID_table["PID_channel"] > PIDNO):
             raise LabscriptError(f"ADwin: Setting the PID channel to more than {PIDNO} is not possible!")
@@ -406,14 +445,6 @@ class ADwinAO8(_ADwinCard):
                         output.raw_output[indices==i+1] *= output.PID[t]['PID_channel'].scale_factor
                     elif isinstance(output.PID[t]['PID_channel'],int):
                         PID_array["PID_channel"][i] = output.PID[t]['PID_channel']
-                    # If PID_channel[t]=None, there are zeros in the PID_array
-
-                    # PID_array["PID_P"][i] = output.PID[t]['P']
-                    # PID_array["PID_I"][i] = output.PID[t]['I']
-                    # PID_array["PID_D"][i] = output.PID[t]['D']
-                    # limits = output.PID[t]['limits']
-                    # PID_array["PID_min"][i] = ADC(limits[0],self.resolution_bits,self.min_V,self.max_V)
-                    # PID_array["PID_max"][i] = ADC(limits[1],self.resolution_bits,self.min_V,self.max_V)
                     if output.PID[t]["start"]=="last":
                         # When we want to use the previous value during the shot,
                         # we use a value that's out of the 16 bit ADC range to identify.
-- 
GitLab


From 5188425a456cde400eee7967c426a61003d57d88 Mon Sep 17 00:00:00 2001
From: Runner PC Cavity Lab <johannes.schabbauer@tuwien.ac.at>
Date: Tue, 10 Dec 2024 12:07:44 +0100
Subject: [PATCH 5/6] ADwin: Fix rounding error during limit check

---
 ADwinProII/labscript_devices_ADwin_modules.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ADwinProII/labscript_devices_ADwin_modules.py b/ADwinProII/labscript_devices_ADwin_modules.py
index 60a86ec..0225ecf 100644
--- a/ADwinProII/labscript_devices_ADwin_modules.py
+++ b/ADwinProII/labscript_devices_ADwin_modules.py
@@ -373,7 +373,7 @@ class ADwinAO8(_ADwinCard):
             PID_off_times = []
             # For each output value, digitize gets the next highest time in  PID_times.
             # Using '-1' to get next lowest time.
-            for i_out,i_PID in enumerate(np.digitize(output.all_times, PID_times)-1):
+            for i_out,i_PID in enumerate(np.digitize(np.round(output.all_times,9), np.round(PID_times))-1):
                 t = PID_times[i_PID]
                 if PID[t]["PID_channel"]==0:
                     # When we turn the PID off but keep the last output, we make sure that
@@ -387,10 +387,11 @@ class ADwinAO8(_ADwinCard):
                     PID_off_times.append(i_out)
             PID_off_outputs = output.raw_output[PID_off_times]
             if np.any(PID_off_outputs < output.limits[0]) or np.any(PID_off_outputs > output.limits[1]):
+                error_times = output.all_times[PID_off_times][(PID_off_outputs < output.limits[0]) < (PID_off_outputs > output.limits[1])]
                 raise LabscriptError(
                     f"Limits of {output.name} (when PID is off) must be in {output.limits}, " +
                     f"you try to set ({PID_off_outputs.min()},{PID_off_outputs.max()}) " +
-                    f"or turning off the PID with target value beyond limits.")
+                    f"or turning off the PID with target value beyond limits at times {error_times}.")
             
         # Check if the PID channel is allowed
         if np.any(self.PID_table["PID_channel"] > PIDNO):
-- 
GitLab


From 3eb5f2186e3a926c5c6839fc66ddbffa00f077e5 Mon Sep 17 00:00:00 2001
From: Runner PC Cavity Lab <johannes.schabbauer@tuwien.ac.at>
Date: Tue, 10 Dec 2024 14:30:43 +0100
Subject: [PATCH 6/6] Bugfix

---
 ADwinProII/labscript_devices_ADwin_modules.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/ADwinProII/labscript_devices_ADwin_modules.py b/ADwinProII/labscript_devices_ADwin_modules.py
index 0225ecf..48ff106 100644
--- a/ADwinProII/labscript_devices_ADwin_modules.py
+++ b/ADwinProII/labscript_devices_ADwin_modules.py
@@ -153,7 +153,6 @@ class ADwinAnalogOut(AnalogOut):
                 raise LabscriptError(
                     f"{self.name}: PID 'set_output={set_output}' must be within ({self.PID_min},{self.PID_max})"
                 )
-        
         # TURN OFF PID
         if pid_no is None:
             self.PID[t] = {
@@ -373,7 +372,7 @@ class ADwinAO8(_ADwinCard):
             PID_off_times = []
             # For each output value, digitize gets the next highest time in  PID_times.
             # Using '-1' to get next lowest time.
-            for i_out,i_PID in enumerate(np.digitize(np.round(output.all_times,9), np.round(PID_times))-1):
+            for i_out,i_PID in enumerate(np.digitize(np.round(output.all_times,6), np.round(PID_times,6))-1):
                 t = PID_times[i_PID]
                 if PID[t]["PID_channel"]==0:
                     # When we turn the PID off but keep the last output, we make sure that
-- 
GitLab