Skip to content

Fix register bit_names mismatches and FastHall JSON parsing #17

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jfichtner wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix register bit_names mismatches and FastHall JSON parsing #17

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6efd0b1
Fix register bit_names mismatches causing TypeError crashes
jfichtner Feb 18, 2026
e29e920
Fix FastHall JSON parsing robustness and correctness issues
jfichtner Feb 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 66 additions & 55 deletions lakeshore/fast_hall_controller.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,20 @@

import json
from .xip_instrument import XIPInstrument, RegisterBase, StatusByteRegister, StandardEventRegister
from .generic_instrument import InstrumentException


class FastHallOperationRegister(RegisterBase):
"""Class object representing the operation status register."""

bit_names = [
"",
"",
"",
"",
"measuring_Done"
"settling",
"ranging",
"measurement_complete",
"waiting_for_trigger",
"field_control_ramping",
"field_measurement_enabled",
"transient"
]

def __init__(self,
Expand All @@ -37,20 +40,20 @@ class FastHallQuestionableRegister(RegisterBase):

bit_names = [
"source_in_compliance_or_at_current_limit",
"negative_resistivity",
"",
"field_control_slew_rate_limit",
"field_control_at_voltage_limit",
"",
"",
"current_measurement_overload",
"voltage_measurement_overload",
"",
"",
"invalid_probe",
"invalid_calibration",
"inter_processor_communication_error",
"",
"",
"field_measurement_communication_error",
"probe_eeprom_read_error",
"r2_less_than_minimum_allowable",
"f_value_out_of_acceptable_range",
"geometry_out_of_acceptable_range"
"",
""
]

def __init__(self,
Expand Down Expand Up @@ -90,7 +93,7 @@ def __init__(self,
measurement_range='AUTO',
min_r_squared=0.9999,
blanking_time=2e-3):
"""The constructor for ContackCheckManualParameters class.
"""The constructor for ContactCheckManualParameters class.

Args:
excitation_type (str):
Expand Down Expand Up @@ -378,7 +381,7 @@ def __init__(self,
self.blanking_time = blanking_time
self.max_samples = max_samples
self.min_snr = min_snr
self.excitation_reversal = str(int(excitation_reversal))
self.excitation_reversal = excitation_reversal


class DCHallParameters:
Expand Down Expand Up @@ -450,7 +453,7 @@ def __init__(self,
self.compliance_limit = compliance_limit
self.averaging_samples = averaging_samples
self.user_defined_field = user_defined_field
self.with_field_reversal = str(int(with_field_reversal))
self.with_field_reversal = with_field_reversal
self.resistivity = resistivity
self.blanking_time = blanking_time
self.sample_thickness = sample_thickness
Expand Down Expand Up @@ -538,7 +541,7 @@ def __init__(self,


class ResistivityLinkParameters:
"""Class object representing parameters used for running manual Resistivity measurements."""
"""Class object representing parameters used for running Resistivity Link measurements."""
def __init__(self,
measurement_range='AUTO',
sample_thickness=0,
Expand Down Expand Up @@ -594,6 +597,15 @@ def __init__(self,
self.operation_register = FastHallOperationRegister
self.questionable_register = FastHallQuestionableRegister

def _parse_json_response(self, response, context=""):
"""Parse a JSON response from the instrument with error handling."""
try:
return json.loads(response)
except json.JSONDecodeError as ex:
raise InstrumentException(
f"Failed to parse JSON response{' for ' + context if context else ''}: {ex}"
) from ex
Comment on lines +600 to +607

Copilot AI Feb 18, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new JSON parsing/error-wrapping behavior isn’t covered by unit tests. Please add tests that (a) exercise one of the get_*_setup_results/get_*_measurement_results methods with a representative JSON payload, and (b) verify malformed JSON raises InstrumentException via _parse_json_response() (to prevent regressions in the parsing/key fixes).

Copilot uses AI. Check for mistakes.

# Status Methods
def get_contact_check_running_status(self):
"""Indicates if the contact check measurement is running."""
Expand All @@ -616,7 +628,7 @@ def get_dc_hall_running_status(self):
return bool(int(self.query("HALL:DC:RUNNING?")))

def get_dc_hall_waiting_status(self):
"""Indicates if the DC hall measurement is running."""
"""Indicates if the DC hall measurement is waiting."""
return bool(int(self.query("HALL:DC:WAITING?")))

def continue_dc_hall(self):
Expand Down Expand Up @@ -737,7 +749,7 @@ def start_four_wire(self, settings):
f"{str(settings.blanking_time)}," +
f"{str(settings.max_samples)}," +
f"{str(settings.min_snr)}," +
f"{str(settings.excitation_reversal)}")
f"{str(int(settings.excitation_reversal))}")
self.command(command_string)

def start_dc_hall_vdp(self, settings):
Expand All @@ -757,7 +769,7 @@ def start_dc_hall_vdp(self, settings):
f"{str(settings.compliance_limit)}," +
f"{str(settings.averaging_samples)}," +
f"{str(settings.user_defined_field)}," +
f"{str(settings.with_field_reversal)}," +
f"{str(int(settings.with_field_reversal))}," +
f"{str(settings.resistivity)}," +
f"{str(settings.blanking_time)}," +
f"{str(settings.sample_thickness)}")
Expand All @@ -775,10 +787,10 @@ def start_dc_hall_hbar(self, settings):
f"{str(settings.excitation_range)}," +
f"{str(settings.excitation_measurement_range)}," +
f"{str(settings.measurement_range)}," +
f"{str(settings.compliance_limit)}," + \
f"{str(settings.compliance_limit)}," +
f"{str(settings.averaging_samples)}," +
f"{str(settings.user_defined_field)}," +
f"{str(settings.with_field_reversal)}," +
f"{str(int(settings.with_field_reversal))}," +
f"{str(settings.resistivity)}," +
f"{str(settings.blanking_time)}," +
f"{str(settings.sample_thickness)}")
Expand Down Expand Up @@ -847,7 +859,7 @@ def get_contact_check_setup_results(self):

# Parse the JSON query string into a dictionary with only the setup results
json_results = self.query('CCHECK:RESULT:JSON? 0')
setup_results = json.loads(json_results).get('Setup')
setup_results = self._parse_json_response(json_results, 'contact check setup').get('Setup', {})

# Generate a Contact Check settings object using the setup result values as the initialization parameters
settings = ContactCheckManualParameters(excitation_type=setup_results.get('ExcitationType'),
Expand All @@ -866,12 +878,12 @@ def get_contact_check_measurement_results(self):

# Parse the JSON query string into a dictionary
json_results = self.query('CCHECK:RESULT:JSON? 0')
measurement_results = json.loads(json_results)
measurement_results = self._parse_json_response(json_results, 'contact check measurement')

# Remove the setup data from the results dictionary
measurement_results.pop('Setup')
measurement_results.pop('OptimizationSetup')
measurement_results.pop('OptimizationDiagnostics')
measurement_results.pop('Setup', None)
measurement_results.pop('OptimizationSetup', None)
measurement_results.pop('OptimizationDiagnostics', None)

return measurement_results

Expand All @@ -880,22 +892,20 @@ def get_fasthall_setup_results(self):

# Parse the JSON query string into a dictionary with only the setup results
json_results = self.query('FASTHALL:RESULT:JSON? 0')
setup_results = json.loads(json_results).get('Setup')
setup_results = self._parse_json_response(json_results, 'FastHall setup').get('Setup', {})

# Generate a FastHall settings object using the setup result values as the initialization parameters
settings = FastHallManualParameters(excitation_type=setup_results.get('ExcitationType'),
excitation_value=setup_results.get('ExcitationValue'),
excitation_range=setup_results.get('ExcitationRange'),
excitation_measurement_range=setup_results.get('ExcitationMeasurementRange'
),
excitation_measurement_range=setup_results.get('ExcitationMeasurementRange'),
measurement_range=setup_results.get('MeasurementRange'),
compliance_limit=setup_results.get('ComplianceLimit'),
max_samples=setup_results.get('MeasurementRange'),
max_samples=setup_results.get('MaximumNumberOfSamples'),
user_defined_field=setup_results.get('UserDefinedFieldReadingInTesla'),
resistivity=setup_results.get('Resistivity'),
blanking_time=setup_results.get('BlankingTimeInSeconds'),
averaging_samples=setup_results.get('NumberOfVoltageCompensationSamplesTo\
Average'),
averaging_samples=setup_results.get('NumberOfVoltageCompensationSamplesToAverage'),
sample_thickness=setup_results.get('SampleThicknessInMeters'),
min_hall_voltage_snr=setup_results.get('HallVoltageSnr'))
return settings
Expand All @@ -905,10 +915,10 @@ def get_fasthall_measurement_results(self):

# Parse the JSON query string into a dictionary
json_results = self.query('FASTHALL:RESULT:JSON? 0')
measurement_results = json.loads(json_results)
measurement_results = self._parse_json_response(json_results, 'FastHall measurement')

# Remove the setup data from the results dictionary
measurement_results.pop('Setup')
measurement_results.pop('Setup', None)

return measurement_results

Expand All @@ -917,13 +927,15 @@ def get_four_wire_setup_results(self):

# Parse the JSON query string into a dictionary with only the setup results
json_results = self.query('FWIRE:RESULT:JSON? 0')
setup_results = json.loads(json_results).get('Setup')
setup_results = self._parse_json_response(json_results, 'four wire setup').get('Setup', {})

# Generate a Four Wire settings object using the setup result values as the initialization parameters
settings = FourWireParameters(contact_point1=setup_results.get('ContactPairExcitation').get('Point1'),
contact_point2=setup_results.get('ContactPairExcitation').get('Point2'),
contact_point3=setup_results.get('ContactPairSense').get('Point1'),
contact_point4=setup_results.get('ContactPairSense').get('Point2'),
contact_pair_excitation = setup_results.get('ContactPairExcitation', {})
contact_pair_sense = setup_results.get('ContactPairSense', {})
settings = FourWireParameters(contact_point1=contact_pair_excitation.get('Point1'),
contact_point2=contact_pair_excitation.get('Point2'),
contact_point3=contact_pair_sense.get('Point1'),
contact_point4=contact_pair_sense.get('Point2'),
excitation_type=setup_results.get('ExcitationType'),
excitation_value=setup_results.get('ExcitationValue'),
excitation_range=setup_results.get('ExcitationRange'),
Expand All @@ -941,10 +953,10 @@ def get_four_wire_measurement_results(self):

# Parse the JSON query string into a dictionary
json_results = self.query('FWIRE:RESULT:JSON? 0')
measurement_results = json.loads(json_results)
measurement_results = self._parse_json_response(json_results, 'four wire measurement')

# Remove the setup data from the results dictionary
measurement_results.pop('Setup')
measurement_results.pop('Setup', None)

return measurement_results

Expand All @@ -953,7 +965,7 @@ def get_dc_hall_setup_results(self):

# Parse the JSON query string into a dictionary with only the setup results
json_results = self.query('HALL:DC:RESULT:JSON? 0')
setup_results = json.loads(json_results).get('Setup')
setup_results = self._parse_json_response(json_results, 'DC Hall setup').get('Setup', {})

# Generate a DC Hall settings object using the setup result values as the initialization parameters
settings = DCHallParameters(excitation_type=setup_results.get('ExcitationType'),
Expand All @@ -975,10 +987,10 @@ def get_dc_hall_measurement_results(self):

# Parse the JSON query string into a dictionary
json_results = self.query('HALL:DC:RESULT:JSON? 0')
measurement_results = json.loads(json_results)
measurement_results = self._parse_json_response(json_results, 'DC Hall measurement')

# Remove the setup data from the results dictionary
measurement_results.pop('Setup')
measurement_results.pop('Setup', None)

return measurement_results

Expand All @@ -987,19 +999,18 @@ def get_resistivity_setup_results(self):

# Parse the JSON query string into a dictionary with only the setup results
json_results = self.query('RESISTIVITY:RESULT:JSON? 0')
setup_results = json.loads(json_results).get('Setup')
setup_results = self._parse_json_response(json_results, 'resistivity setup').get('Setup', {})

# Generate a Resistivity settings object using the setup result values as the initialization parameters
settings = ResistivityManualParameters(setup_results.get('ExcitationType'),
settings = ResistivityManualParameters(excitation_type=setup_results.get('ExcitationType'),
excitation_value=setup_results.get('ExcitationValue'),
excitation_range=setup_results.get('ExcitationRange'),
excitation_measurement_range=setup_results.get('Excitation\
MeasurementRange'),
excitation_measurement_range=setup_results.get('ExcitationMeasurementRange'),
measurement_range=setup_results.get('MeasurementRange'),
compliance_limit=setup_results.get('ComplianceLimit'),
width=setup_results.get('SampleWidthInMeters'),
separation=setup_results.get('SampleArmSeparationInMeters'),
max_samples=setup_results.get('MaxNumberOfSamples'),
max_samples=setup_results.get('MaxNumberOfSamples'), # TODO: verify key from M91 manual (other methods use 'MaximumNumberOfSamples')

Copilot AI Feb 18, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get_resistivity_setup_results() hard-codes MaxNumberOfSamples, but other setup parsers in this file use MaximumNumberOfSamples. To avoid returning None depending on firmware/manual differences, consider supporting both keys (e.g., try one then fall back to the other) instead of relying on the TODO.

Suggested change
max_samples=setup_results.get('MaxNumberOfSamples'), # TODO: verify key from M91 manual (other methods use 'MaximumNumberOfSamples')
max_samples=setup_results.get('MaxNumberOfSamples', setup_results.get('MaximumNumberOfSamples')),

Copilot uses AI. Check for mistakes.
blanking_time=setup_results.get('BlankingTimeInSeconds'),
sample_thickness=setup_results.get('SampleThicknessInMeters'),
min_snr=setup_results.get('MinimumSnr'))
Expand All @@ -1010,10 +1021,10 @@ def get_resistivity_measurement_results(self):

# Parse the JSON query string into a dictionary
json_results = self.query('RESISTIVITY:RESULT:JSON? 0')
measurement_results = json.loads(json_results)
measurement_results = self._parse_json_response(json_results, 'resistivity measurement')

# Remove the setup data from the results dictionary
measurement_results.pop('Setup')
measurement_results.pop('Setup', None)

return measurement_results

Expand Down Expand Up @@ -1059,7 +1070,7 @@ def run_complete_contact_check_manual(self, settings, sample_type):
self.start_contact_check_hbar(settings)
else:
raise ValueError('Invalid value for sample_type argument. '
'Sample type must be either "VDP" for a Van der Pauw sample or "HBAR for a Hall bar."')
'Sample type must be either "VDP" for a Van der Pauw sample or "HBAR" for a Hall bar.')

# Loop until measurement has stopped running
while self.get_contact_check_running_status():
Expand Down Expand Up @@ -1153,7 +1164,7 @@ def run_complete_dc_hall(self, settings, sample_type):
self.start_dc_hall_hbar(settings)
else:
raise ValueError('Invalid value for sample_type argument. '
'Sample type must be either "VDP" for a Van der Pauw sample or "HBAR for a Hall bar."')
'Sample type must be either "VDP" for a Van der Pauw sample or "HBAR" for a Hall bar.')

# Loop until measurement has stopped running or waiting
while self.get_dc_hall_running_status() or self.get_dc_hall_waiting_status():
Expand Down Expand Up @@ -1205,7 +1216,7 @@ def run_complete_resistivity_manual(self, settings, sample_type):
self.start_resistivity_hbar(settings)
else:
raise ValueError('Invalid value for sample_type argument. '
'Sample type must be either "VDP" for a Van der Pauw sample or "HBAR for a Hall bar."')
'Sample type must be either "VDP" for a Van der Pauw sample or "HBAR" for a Hall bar.')

# Loop until measurement has stopped running
while self.get_resistivity_running_status():
Expand Down
4 changes: 2 additions & 2 deletions lakeshore/model_224.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ class Model224ServiceRequestRegister(RegisterBase):
"",
"message_available",
"event_summary",
""
"",
"operation_summary"
]

Expand All @@ -135,7 +135,7 @@ class Model224StatusByteRegister(RegisterBase):
"",
"message_available",
"event_summary",
"master_summary_status"
"master_summary_status",
"operation_summary"
]

Expand Down
4 changes: 2 additions & 2 deletions lakeshore/model_335.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ class Model335(Model335Enums, TemperatureController):
"""A class object representing the Lake Shore Model 335 cryogenic temperature controller."""

# Initiate instrument specific registers
_status_byte_register = Model335StatusByteRegister
_service_request_enable = Model335ServiceRequestEnable
status_byte_register = Model335StatusByteRegister
service_request_enable = Model335ServiceRequestEnable

Copilot AI Feb 18, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change fixes register wiring for the TemperatureController base class, but it doesn’t appear to be covered by tests. Consider adding a small unit test that instantiates Model335 with a fake connection and calls get_status_byte()/get_service_request() to ensure these attributes are used and no AttributeError/type issues occur.

Suggested change
service_request_enable = Model335ServiceRequestEnable
service_request_enable_register = Model335ServiceRequestEnable

Copilot uses AI. Check for mistakes.

vid_pid = [(0x1FB9, 0x0300)]

Expand Down
4 changes: 2 additions & 2 deletions lakeshore/model_372.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,8 +330,8 @@ class Model372(Model372Enums, TemperatureController):
vid_pid = [(0x1FB9, 0x0305)]

# Initialize registers
_status_byte_register = Model372StatusByteRegister
_service_request_enable = Model372ServiceRequestEnable
status_byte_register = Model372StatusByteRegister
service_request_enable = Model372ServiceRequestEnable
Comment on lines 332 to +334

Copilot AI Feb 18, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change fixes register wiring for the TemperatureController base class, but it doesn’t appear to be covered by tests. Consider adding a small unit test that instantiates Model372 with a fake connection and calls get_status_byte()/get_service_request() to ensure these attributes are used and no AttributeError/type issues occur.

Copilot uses AI. Check for mistakes.

def __init__(self,
baud_rate,
Expand Down