Fix temperature controller, XIP, and Model 155 safety issues

lakeshore/em_power_supply.py

@@ -499,11 +499,18 @@ def get_ieee_interface_mode(self):
         """
         return int(self.query("MODE?"))
 
-    def set_factory_defaults(self):
+    def set_factory_defaults(self, confirm=False):
         """Sets all configuration values to factory defaults and resets the instrument.
 
             The instrument must be at zero amps for this command to work.
+
+            Args:
+                confirm (bool): Must be True to execute. Prevents accidental factory reset.
         """
+        if not confirm:
+            raise ValueError(
+                "Factory reset will clear all settings including safety limits. "
+                "Pass confirm=True to proceed.")
         self.command("DFLT 99")
 
     def reset_instrument(self):

lakeshore/model_121.py

@@ -110,8 +110,15 @@ def get_compliance_limit_status(self):
         """
         return bool(int(self.query("COMP?")))
 
-    def set_factory_defaults(self):
-        """Sets all configuration values to factory defaults and resets the instrument."""
+    def set_factory_defaults(self, confirm=False):
+        """Sets all configuration values to factory defaults and resets the instrument.
+
+            Args:
+                confirm (bool): Must be True to execute. Prevents accidental factory reset.
+        """
+        if not confirm:
+            raise ValueError(
+                "Factory reset will clear all settings. Pass confirm=True to proceed.")
         self.command("DFLT 99")
 
     def lock_front_panel(self):

lakeshore/model_155.py

@@ -2,6 +2,8 @@
 
 from time import sleep
 import itertools
+import math
+import numbers
 
 from .xip_instrument import XIPInstrument, RegisterBase, StatusByteRegister, StandardEventRegister
 
@@ -231,6 +233,14 @@ def route_terminals(self, output_connections_location="REAR"):
         """
         self.command("ROUTE:TERMINALS " + output_connections_location)
 
+    @staticmethod
+    def _validate_numeric(value, name):
+        """Validate that a value is a finite real number (not bool, NaN, or inf)."""
+        if isinstance(value, bool) or not isinstance(value, numbers.Real):
+            raise ValueError(f"{name} must be a number, got {type(value).__name__}")
+        if not math.isfinite(value):
+            raise ValueError(f"{name} must be finite, got {value}")
+
     def output_sine_current(self, amplitude, frequency, offset=0.0, phase=0.0):
         """Configures and enables the source output to be a sine wave current source.
 
@@ -245,6 +255,9 @@ def output_sine_current(self, amplitude, frequency, offset=0.0, phase=0.0):
                     Shifts the phase of the output relative to the reference out. Must be between -180 and 180 degrees.
 
         """
+        for val, name in [(amplitude, "Amplitude"), (frequency, "Frequency"),
+                          (offset, "Offset"), (phase, "Phase")]:
+            self._validate_numeric(val, name)
 
         # Change the output mode to source current instead of voltage
         self.command("SOURCE:FUNCTION:MODE CURRENT")
@@ -281,6 +294,9 @@ def output_sine_voltage(self, amplitude, frequency, offset=0.0, phase=0.0):
                     Shifts the phase of the output relative to the reference out. Must be between -180 and 180 degrees.
 
         """
+        for val, name in [(amplitude, "Amplitude"), (frequency, "Frequency"),
+                          (offset, "Offset"), (phase, "Phase")]:
+            self._validate_numeric(val, name)
 
         # Change the output mode to source voltage instead of current
         self.command("SOURCE:FUNCTION:MODE VOLTAGE")
@@ -311,6 +327,7 @@ def output_dc_current(self, current_level):
                     The output current level in amps.
 
         """
+        self._validate_numeric(current_level, "Current level")
 
         # Change the output mode to source current instead of voltage
         self.command("SOURCE:FUNCTION:MODE CURRENT")
@@ -332,6 +349,7 @@ def output_dc_voltage(self, voltage_level):
                     The output voltage level in volts.
 
         """
+        self._validate_numeric(voltage_level, "Voltage level")
 
         # Change the output mode to source voltage instead of current
         self.command("SOURCE:FUNCTION:MODE VOLTAGE")
@@ -401,16 +419,22 @@ def set_current_limit(self, current_limit):
                     The maximum settable current in amps. Must be between 0 and 100 milli-amps.
 
         """
+        self._validate_numeric(current_limit, "Current limit")
+        if current_limit < 0 or current_limit > 0.1:
+            raise ValueError(f"Current limit must be between 0 and 100 milli-amps (0.1 A), got {current_limit}")
         self.command("SOURCE:CURRENT:LIMIT " + str(current_limit))
 
     def set_voltage_limit(self, voltage_limit):
         """Sets the highest settable voltage output value when in voltage mode.
 
             Args:
                 voltage_limit (float):
-                    The maximum settable voltage in amps. Must be between 0 and 100 volts.
+                    The maximum settable voltage in volts. Must be between 0 and 100 volts.
 
         """
+        self._validate_numeric(voltage_limit, "Voltage limit")
+        if voltage_limit < 0 or voltage_limit > 100:
+            raise ValueError(f"Voltage limit must be between 0 and 100 volts, got {voltage_limit}")
         self.command("SOURCE:VOLTAGE:LIMIT " + str(voltage_limit))
 
     def set_current_mode_voltage_protection(self, max_voltage):
@@ -421,16 +445,22 @@ def set_current_mode_voltage_protection(self, max_voltage):
                     The maximum permissible voltage. Must be between 1 and 100 volts.
 
         """
+        self._validate_numeric(max_voltage, "Max voltage")
+        if max_voltage < 1 or max_voltage > 100:
+            raise ValueError(f"Max voltage must be between 1 and 100 volts, got {max_voltage}")
         self.command("SOURCE:CURRENT:PROTECTION " + str(max_voltage))
 
     def set_voltage_mode_current_protection(self, max_current):
         """Sets the maximum current level permitted by the instrument when sourcing voltage.
 
             Args:
                 max_current (float):
-                    The maximum permissible voltage. Must be between 1 and 100 volts.
+                    The maximum permissible current in amps.
 
         """
+        self._validate_numeric(max_current, "Max current")
+        if max_current < 0:
+            raise ValueError(f"Max current must be non-negative, got {max_current}")
         self.command("SOURCE:VOLTAGE:PROTECTION " + str(max_current))
 
     def enable_ac_high_voltage_compliance(self):

lakeshore/model_224.py

@@ -346,8 +346,15 @@ def set_wait_to_continue(self):
 
         self.command("*WAI")
 
-    def set_to_factory_defaults(self):
-        """Sets all the settings and configurations to their factory default values."""
+    def set_to_factory_defaults(self, confirm=False):
+        """Sets all the settings and configurations to their factory default values.
+
+            Args:
+                confirm (bool): Must be True to execute. Prevents accidental factory reset.
+        """
+        if not confirm:
+            raise ValueError(
+                "Factory reset will clear all settings. Pass confirm=True to proceed.")
         self.command("DFLT 99")
 
     def get_reading_status(self, input_channel):
@@ -873,7 +880,7 @@ def get_curve(self, curve):
         """
 
         data_points = []
-        true_point_index = 0
+        true_point_index = -1
         for i in range(0, 200):
             point = self.get_curve_data_point(curve, i + 1)
             data_points.append(point)
@@ -899,7 +906,7 @@ def set_curve(self, curve, data_points):
 
         self.delete_curve(curve)
 
-        for index, point in data_points:
+        for index, point in enumerate(data_points):
             self.set_curve_data_point(curve, index + 1, point[0], point[1])
 
     def get_relay_status(self, relay_channel):

lakeshore/model_240.py

@@ -147,8 +147,15 @@ def get_celsius_reading(self, channel):
         """
         return self.query(f"CRDG? {channel}")
 
-    def set_factory_defaults(self):
-        """Sets all configuration values to factory defaults and resets the instrument."""
+    def set_factory_defaults(self, confirm=False):
+        """Sets all configuration values to factory defaults and resets the instrument.
+
+            Args:
+                confirm (bool): Must be True to execute. Prevents accidental factory reset.
+        """
+        if not confirm:
+            raise ValueError(
+                "Factory reset will clear all settings. Pass confirm=True to proceed.")
         self.command("DFLT 99")
 
     def get_kelvin_reading(self, channel):

lakeshore/model_335.py

@@ -364,7 +364,7 @@ def get_input_sensor(self, channel):
             sensor_range = 0
         elif input_sensor_type == self.InputSensorType.DIODE:
             sensor_range = self.DiodeRange(int(sensor_configuration[2]))
-        elif input_sensor_type in (self.InputSensorType.PLATINUM_RTD or self.InputSensorType.NTC_RTD):
+        elif input_sensor_type in (self.InputSensorType.PLATINUM_RTD, self.InputSensorType.NTC_RTD):
             sensor_range = self.RTDRange(int(sensor_configuration[2]))
         elif input_sensor_type == self.InputSensorType.THERMOCOUPLE:
             sensor_range = self.ThermocoupleRange(int(sensor_configuration[2]))

lakeshore/temperature_controllers.py

@@ -1,6 +1,9 @@
 """Implements a parent class for temperature controllers that contains shared methods between similar instruments."""
 
+import math
+import numbers
 import serial
+from warnings import warn
 from .generic_instrument import GenericInstrument, InstrumentException, RegisterBase
 from .temperature_controllers_enums import TemperatureControllerEnums
 
@@ -139,6 +142,14 @@ def __init__(self, serial_number, com_port, baud_rate, timeout, ip_address, tcp_
         GenericInstrument.__init__(self, serial_number, com_port, baud_rate, 7, 1, serial.PARITY_ODD,
                                    False, False, timeout, ip_address, tcp_port, **kwargs)
 
+    @staticmethod
+    def _validate_numeric(value, name):
+        """Validate that a value is a finite real number (not bool, NaN, or inf)."""
+        if isinstance(value, bool) or not isinstance(value, numbers.Real):
+            raise ValueError(f"{name} must be a number, got {type(value).__name__}")
+        if not math.isfinite(value):
+            raise ValueError(f"{name} must be finite, got {value}")
+
     @staticmethod
     def _error_check(error_code):
         event_register = StandardEventRegister.from_integer(error_code)
@@ -366,7 +377,7 @@ def set_curve_data_point(self, curve, index, sensor_units, temperature, curvatur
                     The curvature value scale used to calculate spindle coefficients to 6 digits. Optional parameter.
 
         """
-        if curvature:
+        if curvature is not None:
             command_string = f"CRVPT {curve},{index},{sensor_units},{temperature},{curvature}"
         else:
             command_string = f"CRVPT {curve},{index},{sensor_units},{temperature}"
@@ -403,7 +414,7 @@ def get_curve(self, curve):
                     (sensor_units: float, temp_value: float, curvature_value: float (optional)).
 
         """
-        true_point_index = 200
+        true_point_index = -1
         data_points = []
         for i in range(0, 200):
             point = self.get_curve_data_point(curve, i + 1)
@@ -412,7 +423,7 @@ def get_curve(self, curve):
                 true_point_index = i
 
         # Remove all extraneous points
-        return data_points[:true_point_index]
+        return data_points[:true_point_index + 1]
 
     def set_curve(self, curve, data_points):
         """Method to define a user curve using a list of data points.
@@ -427,7 +438,7 @@ def set_curve(self, curve, data_points):
         """
         self.delete_curve(curve)
 
-        for index, point in data_points:
+        for index, point in enumerate(data_points):
             if len(point) > 2:
                 self.set_curve_data_point(curve, index + 1, point[0], point[1], point[2])
             else:
@@ -1043,6 +1054,15 @@ def set_heater_pid(self, output, gain, integral, derivative):
                     The ramp rate is configured in field units per second.
 
         """
+        self._validate_numeric(gain, "Gain")
+        self._validate_numeric(integral, "Integral")
+        self._validate_numeric(derivative, "Derivative")
+        if gain < 0:
+            raise ValueError(f"Gain must be non-negative, got {gain}")
+        if integral < 0:
+            raise ValueError(f"Integral must be non-negative, got {integral}")
+        if derivative < 0:
+            raise ValueError(f"Derivative must be non-negative, got {derivative}")
         self.command(f"PID {output},{gain},{integral},{derivative}")
 
     def get_heater_pid(self, output):
@@ -1054,17 +1074,23 @@ def get_heater_pid(self, output):
 
             Returns:
                 (dict):
-                    {"gain": float, "integral": float, "ramp_rate": float}
+                    {"gain": float, "integral": float, "derivative": float, "ramp_rate": float}
                         gain: Proportional term in PID control.
                         integral: Integral term in PID control.
-                        ramp_rate: Derivative term in PID control.
+                        derivative: Derivative term in PID control.
+                        ramp_rate: Deprecated alias for derivative. Use "derivative" instead.
 
         """
         pid_values = self.query(f"PID? {output}")
         pid_values = pid_values.split(",")
+        derivative = float(pid_values[2])
+        # "ramp_rate" was the original (incorrect) key name for the derivative PID term.
+        # Kept for backward compatibility — new code should use "derivative" to match
+        # the set_heater_pid() parameter name.
         return {"gain": float(pid_values[0]),
                 "integral": float(pid_values[1]),
-                "ramp_rate": float(pid_values[2])}
+                "derivative": derivative,
+                "ramp_rate": derivative}
 
     def set_setpoint_ramp_parameter(self, output, ramp_enable, rate_value):
         """Sets the control loop of a particular output.
@@ -1308,6 +1334,7 @@ def set_control_setpoint(self, output, value):
                     The value for the set-point (in the preferred units of the control loop sensor).
 
         """
+        self._validate_numeric(value, "Setpoint")
         self.command(f"SETP {output},{value}")
 
     def get_control_setpoint(self, output):
@@ -1345,6 +1372,9 @@ def set_temperature_limit(self, input_channel, limit):
                     A limit of zero will turn the feature off.
 
         """
+        self._validate_numeric(limit, "Temperature limit")
+        if limit < 0:
+            raise ValueError(f"Temperature limit must be non-negative, got {limit}")
         self.command(f"TLIMIT {input_channel},{limit}")
 
     def get_temperature_limit(self, input_channel):

lakeshore/xip_instrument.py

@@ -324,7 +324,7 @@ def set_questionable_event_enable_mask(self, register_mask):
                     An instrument specific QuestionableRegister class object with all bits configured true or false.
         """
 
-        integer_representation = self.questionable_register.to_integer(register_mask)
+        integer_representation = register_mask.to_integer()
         self.command(f"STATus:QUEStionable:ENABle {str(integer_representation)}", check_errors=False)
 
     def reset_status_register_masks(self):
@@ -403,8 +403,15 @@ def reset_measurement_settings(self):
         """Resets measurement settings to their default values."""
         self.command("SYSTEM:PRESET")
 
-    def factory_reset(self):
-        """Resets all system information such as settings, wi-fi connections, date and time, etc."""
+    def factory_reset(self, confirm=False):
+        """Resets all system information such as settings, wi-fi connections, date and time, etc.
+
+            Args:
+                confirm (bool): Must be True to execute. Prevents accidental factory reset.
+        """
+        if not confirm:
+            raise ValueError(
+                "Factory reset will clear all settings. Pass confirm=True to proceed.")
         self.command("SYSTEM:FACTORYRESET")
 
     def _get_identity(self):

tests/test_121.py

@@ -71,9 +71,13 @@ def test_compliance_limit_status(self):
 
     def test_factory_defaults(self):
         self.fake_connection.setup_response('')
-        self.dut.set_factory_defaults()
+        self.dut.set_factory_defaults(confirm=True)
         self.assertIn('DFLT 99', self.fake_connection.get_outgoing_message())
 
+    def test_factory_defaults_raises_without_confirm(self):
+        with self.assertRaises(ValueError):
+            self.dut.set_factory_defaults()
+
     def test_enable_keypad(self):
         self.fake_connection.setup_response('')
         self.dut.unlock_front_panel()

tests/test_224.py

@@ -354,3 +354,18 @@ def test_get_display_configuration_without_number_of_fields(self):
         response = self.dut.get_display_configuration()
         self.assertDictEqual(response, expected_response)
         self.assertIn("DISPLAY?", self.fake_connection.get_outgoing_message())
+
+
+class TestFactoryDefaults(TestWithFakeModel224):
+    def test_set_to_factory_defaults(self):
+        self.fake_connection.setup_response('0')
+        self.dut.set_to_factory_defaults(confirm=True)
+        self.assertIn('DFLT 99', self.fake_connection.get_outgoing_message())
+
+    def test_set_to_factory_defaults_raises_without_confirm(self):
+        with self.assertRaises(ValueError):
+            self.dut.set_to_factory_defaults()
+
+    def test_set_to_factory_defaults_raises_with_confirm_false(self):
+        with self.assertRaises(ValueError):
+            self.dut.set_to_factory_defaults(confirm=False)