From 2780927fb8d380903681d3d4d71cba93c7e1ac98 Mon Sep 17 00:00:00 2001 From: Justin Fichtner Date: Tue, 17 Feb 2026 19:16:40 -0500 Subject: [PATCH 1/4] Fix cherk_errors typo and SSM enum mismatches - Fix misspelled `cherk_errors` keyword argument to `check_errors` in 6 locations across ssm_source_module.py and ssm_measure_module.py. The typo was silently ignored, leaving error checking enabled when the intent was to suppress it. - Fix set_resistance_mode() enum isinstance check: was checking ResistanceExcitationType instead of ResistanceMode. - Reconcile DataSourceMnemonic enum with data_source_types dict: add SSWeeping entry to data_source_types and SOURCE_IS_SETTLING member to DataSourceMnemonic. Co-Authored-By: Claude Opus 4.6 --- lakeshore/ssm_measure_module.py | 4 ++-- lakeshore/ssm_source_module.py | 10 +++++----- lakeshore/ssm_system.py | 1 + lakeshore/ssm_system_enums.py | 1 + 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/lakeshore/ssm_measure_module.py b/lakeshore/ssm_measure_module.py index a3dfe83..61e29f7 100644 --- a/lakeshore/ssm_measure_module.py +++ b/lakeshore/ssm_measure_module.py @@ -1029,7 +1029,7 @@ def set_identify_state(self, state): def get_dark_mode_state(self): """Returns the dark mode state for the given pod.""" - response = self.device.query(f'SENSe{self.module_number}:DMODe?', cherk_errors=False) + response = self.device.query(f'SENSe{self.module_number}:DMODe?', check_errors=False) return response def set_dark_mode_state(self, state): @@ -1234,7 +1234,7 @@ def set_resistance_mode(self, resistance_mode): Args: resistance_mode (ResistanceMode): The desired resistance optimization mode. """ - if isinstance(resistance_mode, SSMSystemEnums.ResistanceExcitationType): + if isinstance(resistance_mode, SSMSystemEnums.ResistanceMode): mode = resistance_mode.name else: mode = resistance_mode diff --git a/lakeshore/ssm_source_module.py b/lakeshore/ssm_source_module.py index 9b72915..1d450d6 100644 --- a/lakeshore/ssm_source_module.py +++ b/lakeshore/ssm_source_module.py @@ -1000,7 +1000,7 @@ def set_identify_state(self, state): def get_dark_mode_state(self): """Returns the dark mode state for the given pod.""" - response = self.device.query(f'SOURce{self.module_number}:DMODe?', cherk_errors=False) + response = self.device.query(f'SOURce{self.module_number}:DMODe?', check_errors=False) return response def set_dark_mode_state(self, state): @@ -1014,7 +1014,7 @@ def set_dark_mode_state(self, state): def get_voltage_output_limit_high(self): """Returns the present voltage high output limit.""" - response = float(self.device.query(f'SOURce{self.module_number}:VOLTage:LIMit:HIGH?', cherk_errors=False)) + response = float(self.device.query(f'SOURce{self.module_number}:VOLTage:LIMit:HIGH?', check_errors=False)) return response def set_voltage_output_limit_high(self, limit): @@ -1033,7 +1033,7 @@ def set_voltage_output_limit_high(self, limit): def get_voltage_output_limit_low(self): """Returns the present voltage low output limit.""" - response = float(self.device.query(f'SOURce{self.module_number}:VOLTage:LIMit:LOW?', cherk_errors=False)) + response = float(self.device.query(f'SOURce{self.module_number}:VOLTage:LIMit:LOW?', check_errors=False)) return response def set_voltage_output_limit_low(self, limit): @@ -1052,7 +1052,7 @@ def set_voltage_output_limit_low(self, limit): def get_current_output_limit_high(self): """Returns the present current high output limit.""" - response = float(self.device.query(f'SOURce{self.module_number}:CURRent:LIMit:HIGH?', cherk_errors=False)) + response = float(self.device.query(f'SOURce{self.module_number}:CURRent:LIMit:HIGH?', check_errors=False)) return response def set_current_output_limit_high(self, limit): @@ -1071,7 +1071,7 @@ def set_current_output_limit_high(self, limit): def get_current_output_limit_low(self): """Returns the present current low output limit.""" - response = float(self.device.query(f'SOURce{self.module_number}:CURRent:LIMit:LOW?', cherk_errors=False)) + response = float(self.device.query(f'SOURce{self.module_number}:CURRent:LIMit:LOW?', check_errors=False)) return response def set_disable_on_compliance(self, disable_on_compliance): diff --git a/lakeshore/ssm_system.py b/lakeshore/ssm_system.py index 70cdf5d..f6e94e6 100644 --- a/lakeshore/ssm_system.py +++ b/lakeshore/ssm_system.py @@ -218,6 +218,7 @@ def _locate_module_by_name(module_name, set_of_modules): 'SRANge': float, 'SVLimit': lambda s: bool(int(s)), 'SILimit': lambda s: bool(int(s)), + 'SSWeeping': lambda s: bool(int(s)), 'MDC': float, 'MRMs': float, 'MPPeak': float, diff --git a/lakeshore/ssm_system_enums.py b/lakeshore/ssm_system_enums.py index 2947ff2..1f42a80 100644 --- a/lakeshore/ssm_system_enums.py +++ b/lakeshore/ssm_system_enums.py @@ -30,6 +30,7 @@ class DataSourceMnemonic(str, Enum): MEASURE_SETTLING = 'MSETtling' MEASURE_UNLOCK = 'MUNLock' MEASURE_REFERENCE_FREQUENCY = 'MRFRequency' + SOURCE_IS_SETTLING = 'SRSettling' GENERAL_PURPOSE_INPUT_STATES = 'GPIStates' GENERAL_PURPOSE_OUTPUT_STATES = 'GPOStates' From cdba7b10b6fff5d5eae1a6bc6446d41dc5d2dfe1 Mon Sep 17 00:00:00 2001 From: Justin Fichtner Date: Tue, 17 Feb 2026 19:17:50 -0500 Subject: [PATCH 2/4] Fix stream_data generator robustness issues - Add try/finally to stream_data to send TRACe:STOp on generator abandonment or exception, ensuring instrument-side cleanup. - Add inner break when num_points reached to prevent yielding excess rows from a batch. - Validate row length before struct.unpack to skip incomplete rows from non-evenly-divisible byte buffers. - Add 10ms sleep backoff in polling loop to prevent busy-looping and flooding the instrument with queries. - Add length validation before datetime() construction in get_head_cal_datetime and get_head_self_cal_datetime to raise clear errors on malformed responses. - Handle wakepy import failure gracefully with a no-op context manager fallback, preventing NameError when wakepy is unavailable. Co-Authored-By: Claude Opus 4.6 --- lakeshore/ssm_system.py | 83 ++++++++++++++++++++++++++++------------- 1 file changed, 58 insertions(+), 25 deletions(-) diff --git a/lakeshore/ssm_system.py b/lakeshore/ssm_system.py index f6e94e6..35f2728 100644 --- a/lakeshore/ssm_system.py +++ b/lakeshore/ssm_system.py @@ -1,6 +1,7 @@ """Implements functionality unique to the Lake Shore M81.""" from datetime import datetime import struct +import time from base64 import b64decode from threading import Lock from warnings import warn @@ -14,10 +15,17 @@ try: from wakepy import keep -except NotImplementedError: - pass # Proceed without wakepy on linux without systemd -except KeyError: - pass # Proceed without wakepy on linux without dbus +except (NotImplementedError, KeyError, ImportError): + # Provide a no-op context manager fallback + from contextlib import contextmanager + + class _NoOpKeep: + @staticmethod + @contextmanager + def running(): + yield + + keep = _NoOpKeep() class SSMSystemOperationRegister(RegisterBase): @@ -293,25 +301,36 @@ def stream_data(self, rate, num_points, *data_sources): bytes_per_row = int(self.query('TRACe:FORMat:ENCOding:B64:BCOunt?')) binary_format = '<' + self.query('TRACe:FORMat:ENCOding:B64:BFORmat?').strip('\"') - if num_points is not None: - self.command(f'TRACe:STARt {num_points}') - else: - self.command('TRACe:STARt') - - num_collected = 0 - while num_points is None or num_collected < num_points: - b64_string = '' - while not b64_string: - b64_string = self.query('TRACe:DATA:ALL?', check_errors=False) - - new_bytes = b64decode(b64_string) - rows = [new_bytes[i:i + bytes_per_row] for i in range(0, len(new_bytes), bytes_per_row)] - - for row in rows: - data = struct.unpack(binary_format, row) - num_collected += 1 - - yield data + try: + if num_points is not None: + self.command(f'TRACe:STARt {num_points}') + else: + self.command('TRACe:STARt') + + num_collected = 0 + while num_points is None or num_collected < num_points: + b64_string = '' + while not b64_string: + b64_string = self.query('TRACe:DATA:ALL?', check_errors=False) + if not b64_string: + time.sleep(0.01) + + new_bytes = b64decode(b64_string) + rows = [new_bytes[i:i + bytes_per_row] for i in range(0, len(new_bytes), bytes_per_row)] + + for row in rows: + if len(row) < bytes_per_row: + break + if num_points is not None and num_collected >= num_points: + break + data = struct.unpack(binary_format, row) + num_collected += 1 + yield data + finally: + try: + self.command('TRACe:STOp', check_errors=False) + except Exception: + pass overflow_occurred = bool(int(self.query('TRACe:DATA:OVERflow?', check_errors=True))) if overflow_occurred: @@ -493,7 +512,14 @@ def get_head_cal_datetime(self): """Returns the date and time of the head calibration.""" response = self.query('CALibration:DATE?').split(',') - return datetime(int(response[0]), int(response[1]), int(response[2]), int(response[3]), int(response[4]), int(response[5])) + if len(response) < 6: + raise XIPInstrumentException( + f"Malformed calibration date response: expected 6 values, got {len(response)}") + try: + return datetime(int(response[0]), int(response[1]), int(response[2]), + int(response[3]), int(response[4]), int(response[5])) + except (ValueError, OverflowError) as ex: + raise XIPInstrumentException(f"Invalid calibration date values: {response}") from ex def get_head_cal_temperature(self): """Returns the temperature of the head calibration.""" @@ -509,7 +535,14 @@ def get_head_self_cal_datetime(self): """Returns the datetime of the last head self calibration.""" response = self.query('CALibration:SCALibration:DATE?').split(',') - return datetime(int(response[0]), int(response[1]), int(response[2]), int(response[3]), int(response[4]), int(response[5])) + if len(response) < 6: + raise XIPInstrumentException( + f"Malformed self-calibration date response: expected 6 values, got {len(response)}") + try: + return datetime(int(response[0]), int(response[1]), int(response[2]), + int(response[3]), int(response[4]), int(response[5])) + except (ValueError, OverflowError) as ex: + raise XIPInstrumentException(f"Invalid self-calibration date values: {response}") from ex def get_head_self_cal_temperature(self): """Returns the temperature of the last head self calibration.""" From 509ce8a655f38c311e7182e96a79d1767669429b Mon Sep 17 00:00:00 2001 From: Justin Fichtner Date: Tue, 17 Feb 2026 19:24:00 -0500 Subject: [PATCH 3/4] Address review findings: preserve exception compat and document lock ordering - Remove ValueError/OverflowError wrapping in datetime validation methods to preserve backward compatibility for callers catching those exception types. The length validation (XIPInstrumentException) is kept since it replaces a less informative IndexError. - Document the lock ordering invariant (stream_lock before dut_lock) to prevent future deadlock regressions. Co-Authored-By: Claude Opus 4.6 --- lakeshore/ssm_system.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/lakeshore/ssm_system.py b/lakeshore/ssm_system.py index 35f2728..4dcdcd4 100644 --- a/lakeshore/ssm_system.py +++ b/lakeshore/ssm_system.py @@ -136,6 +136,8 @@ def __init__(self, self.settings_profiles = SettingsProfiles(self) + # Lock ordering: stream_lock must be acquired before dut_lock (inherited from + # GenericInstrument) to avoid deadlock. Never acquire dut_lock then stream_lock. self.stream_lock = Lock() # Sweeping limits @@ -515,11 +517,8 @@ def get_head_cal_datetime(self): if len(response) < 6: raise XIPInstrumentException( f"Malformed calibration date response: expected 6 values, got {len(response)}") - try: - return datetime(int(response[0]), int(response[1]), int(response[2]), - int(response[3]), int(response[4]), int(response[5])) - except (ValueError, OverflowError) as ex: - raise XIPInstrumentException(f"Invalid calibration date values: {response}") from ex + return datetime(int(response[0]), int(response[1]), int(response[2]), + int(response[3]), int(response[4]), int(response[5])) def get_head_cal_temperature(self): """Returns the temperature of the head calibration.""" @@ -538,11 +537,8 @@ def get_head_self_cal_datetime(self): if len(response) < 6: raise XIPInstrumentException( f"Malformed self-calibration date response: expected 6 values, got {len(response)}") - try: - return datetime(int(response[0]), int(response[1]), int(response[2]), - int(response[3]), int(response[4]), int(response[5])) - except (ValueError, OverflowError) as ex: - raise XIPInstrumentException(f"Invalid self-calibration date values: {response}") from ex + return datetime(int(response[0]), int(response[1]), int(response[2]), + int(response[3]), int(response[4]), int(response[5])) def get_head_self_cal_temperature(self): """Returns the temperature of the last head self calibration.""" From 56590387650caf4398a6128db4f0d6329434f20c Mon Sep 17 00:00:00 2001 From: Justin Fichtner Date: Tue, 17 Feb 2026 19:32:41 -0500 Subject: [PATCH 4/4] Address remaining review findings - Add logging (DEBUG level) to stream_data cleanup instead of silently swallowing exceptions, and log skipped incomplete rows. - Wakepy no-op fallback now yields a mock Mode object with success=False for forward compatibility with code that inspects the yielded value. - Add docstring note documenting that TRACe:STOp is sent on generator exit and that the overflow check is skipped on early abandonment. - Add inline comments explaining the incomplete-row break, num_points break, potential blocking of TRACe:STOp, and overflow check unreachability on GeneratorExit. - Add tests verifying TRACe:STOp is sent on both normal completion and early generator abandonment (caller break). Co-Authored-By: Claude Opus 4.6 --- lakeshore/ssm_system.py | 28 ++++++++++++++-- tests/test_ssm_system.py | 71 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 3 deletions(-) diff --git a/lakeshore/ssm_system.py b/lakeshore/ssm_system.py index 4dcdcd4..c89de7b 100644 --- a/lakeshore/ssm_system.py +++ b/lakeshore/ssm_system.py @@ -1,5 +1,6 @@ """Implements functionality unique to the Lake Shore M81.""" from datetime import datetime +import logging import struct import time from base64 import b64decode @@ -13,17 +14,23 @@ from lakeshore.ssm_settings_profiles import SettingsProfiles from lakeshore.requires_firmware_version import requires_firmware_version +logger = logging.getLogger(__name__) + try: from wakepy import keep except (NotImplementedError, KeyError, ImportError): - # Provide a no-op context manager fallback + # Provide a no-op context manager fallback when wakepy is unavailable from contextlib import contextmanager + class _NoOpMode: + """Mimics wakepy's Mode object returned by keep.running().""" + success = False + class _NoOpKeep: @staticmethod @contextmanager def running(): - yield + yield _NoOpMode() keep = _NoOpKeep() @@ -281,6 +288,13 @@ def get_multiple_min_max_values(self, *data_sources): def stream_data(self, rate, num_points, *data_sources): """Generator object to stream data from the instrument. + A TRACe:STOp command is sent when the generator exits for any reason + (normal completion, caller break, exception, or garbage collection) + to ensure instrument-side cleanup. + + Note: the overflow check only runs on normal completion. If the + generator is abandoned early, overflow status is not checked. + Args: rate (int): Desired transfer rate in points/sec. @@ -322,18 +336,26 @@ def stream_data(self, rate, num_points, *data_sources): for row in rows: if len(row) < bytes_per_row: + # Incomplete trailing row from non-evenly-divisible buffer + logger.debug('Skipping incomplete row (%d/%d bytes)', len(row), bytes_per_row) break if num_points is not None and num_collected >= num_points: + # Batch contained more rows than needed to reach num_points break data = struct.unpack(binary_format, row) num_collected += 1 yield data finally: + # Best-effort cleanup: stop instrument-side streaming. + # May block briefly if dut_lock is held by another thread. try: self.command('TRACe:STOp', check_errors=False) except Exception: - pass + logger.debug('Failed to send TRACe:STOp during cleanup', exc_info=True) + # Note: this overflow check is only reached on normal generator + # completion. If the generator is abandoned (e.g. caller break), + # GeneratorExit bypasses this code via the context manager exits. overflow_occurred = bool(int(self.query('TRACe:DATA:OVERflow?', check_errors=True))) if overflow_occurred: raise XIPInstrumentException('Data loss occurred during this data stream.') diff --git a/tests/test_ssm_system.py b/tests/test_ssm_system.py index bafc605..d1f49ac 100644 --- a/tests/test_ssm_system.py +++ b/tests/test_ssm_system.py @@ -126,6 +126,77 @@ def test_stream_data(self): self.assertIn('TRACe:FORMat:ENCOding:B64:BFORmat?', self.fake_connection.get_outgoing_message()) self.assertIn('TRACe:STARt 3', self.fake_connection.get_outgoing_message()) + def test_stream_data_sends_stop_on_completion(self): + """Test that TRACe:STOp is sent when stream_data completes normally""" + + list_data = [(False, 45.6521), (True, 1.258), (False, 65.8974)] + + my_data = [] + for data in list_data: + for value in data: + my_data.append(value) + + list_format = '