Fix EM power supply correctness bugs and add safety validation

[jfichtner]

Summary

• Fix ERSTE argument order bug in set_operational_error_enable_mask — operational mask was incorrectly placed in the hardware mask position, causing cross-register corruption
• Add input validation to all safety-critical methods (set_current, set_ramp_rate, set_limits, set_ramp_segment, set_ieee_488, and others) with bounds checking for the Model 643/648 (up to 135A)
• Harden validation against edge cases — rejects bool, NaN, Inf, and non-numeric types via a shared _validate_number helper
• Add 58 new validation tests (50 → 108 total) covering boundary values, type rejection, and safety bounds

Commits

1. Fix ERSTE argument order and TOCTOU race — Corrects the register mask position and documents the read-modify-write race condition
2. Add bounds validation to safety-critical methods — set_current, set_ramp_rate, set_limits, set_ramp_segment
3. Harden validation against NaN/Inf — Adds _validate_number helper, adds current bounds to set_ramp_segment
4. Add validation to remaining methods — set_internal_water, set_magnet_water, set_display_brightness, set_front_panel_lock, set_programming_mode
5. Add comprehensive validation tests — 43 new tests, strengthen ERSTE mask test with distinguishable values
6. Fix remaining gaps and extract constants — Validate set_ieee_488, set_ieee_interface_mode, set_ramp_segments_enable; extract _MAX_CURRENT, _MIN_RAMP_RATE, _MAX_RAMP_RATE class constants
7. Complete test coverage — 15 additional tests for newly validated methods and edge cases

Test plan

• All 108 tests pass: python -m pytest tests/test_em_power_supply.py -v
• Verify on physical Model 643/648 hardware that validated commands still work within bounds
• Confirm ERSTE register behavior matches corrected argument order

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR fixes correctness issues in the Model 643/648 electromagnet power supply driver (notably ERSTE mask ordering) and adds safety-focused input validation across multiple setter APIs, along with expanded unit test coverage to exercise validation edge cases.

Changes:

• Fix ERSTE operational vs hardware mask argument ordering and update related tests.
• Add shared finite-number validation and apply bounds/type checks to current/ramp/limit and other safety-relevant setters.
• Add many new validation tests for boundary values and invalid types.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 12 comments.

File	Description
lakeshore/em_power_supply.py	Fixes ERSTE operational mask write ordering and introduces numeric and range validation for multiple safety-critical commands.
tests/test_em_power_supply.py	Updates ERSTE tests and adds a large suite of new validation tests for setter input bounds and invalid types.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

These tests run under TestWithFakeEMPowerSupply, which identifies as MODEL643 (see tests/utils.py). They currently assert that 135.1 A is accepted, but the Model 643 documented limit is 70.1 A. Either switch these validation tests to use a MODEL648 fixture or make the asserted max current depend on the detected model so the tests don’t lock in unsafe behavior for MODEL643.

Suggested change

	self.dut.set_current(135.2)
	def test_set_current_rejects_below_negative_max(self):
	with self.assertRaises(ValueError):
	self.dut.set_current(-135.2)
	def test_set_current_accepts_boundary_positive(self):
	self.fake_connection.setup_response("0")
	self.dut.set_current(135.1)
	self.assertIn("SETI 135.1", self.fake_connection.get_outgoing_message())
	def test_set_current_accepts_boundary_negative(self):
	self.fake_connection.setup_response("0")
	self.dut.set_current(-135.1)
	self.assertIn("SETI -135.1", self.fake_connection.get_outgoing_message())
	self.dut.set_current(70.2)
	def test_set_current_rejects_below_negative_max(self):
	with self.assertRaises(ValueError):
	self.dut.set_current(-70.2)
	def test_set_current_accepts_boundary_positive(self):
	self.fake_connection.setup_response("0")
	self.dut.set_current(70.1)
	self.assertIn("SETI 70.1", self.fake_connection.get_outgoing_message())
	def test_set_current_accepts_boundary_negative(self):
	self.fake_connection.setup_response("0")
	self.dut.set_current(-70.1)
	self.assertIn("SETI -70.1", self.fake_connection.get_outgoing_message())

[Copilot]

The new safety/mode validation tests don’t cover the bool edge case for integer-coded parameters (e.g., set_internal_water(True), set_magnet_water(True), etc.). Since bools currently compare equal to 0/1, add explicit tests to ensure bool inputs are rejected (or correctly coerced) for these methods.

[Copilot]

_MAX_CURRENT is hard-coded to the Model 648 limit (135.1 A) even though this class is used for MODEL643 as well (tests instantiate Model643). This contradicts the docstrings (MODEL643 max 70.1 A) and can allow unsafe currents/limits on a 643. Consider deriving the max current from the connected instrument model (e.g., based on self.model_number) or making it configurable per instance, and update validation accordingly.

Suggested change

	_MAX_CURRENT = 135.1 # Model 648 maximum; Model 643 is 70.1 A
	# Use the more restrictive of the two model limits (643: 70.1 A, 648: 135.1 A) for safety.
	_MAX_CURRENT = 70.1 # A

[Copilot]

state not in (True, False, 0, 1) will also accept 0.0/1.0 (and other numeric types equal to 0/1) due to equality semantics, even though the docstring says bool or 0/1. If you want strict input validation, check type explicitly (bool or int) and reject floats/other numerics.

Suggested change

	if state not in (True, False, 0, 1):
	# Enforce strict type checking: accept only bool or int 0/1, reject floats and other numerics.
	if isinstance(state, bool):
	valid = True
	elif isinstance(state, int) and state in (0, 1):
	valid = True
	else:
	valid = False
	if not valid:

[Copilot]

mode not in (0, 1, 2, 3) will allow bools (True == 1 / False == 0). Passing True currently results in INTWTR True being sent, which is not a valid instrument command. Explicitly reject bool (and/or enforce isinstance(mode, int) and format with int(mode)).

[Copilot]

The segment validation allows bool values because True/False compare equal to 1/0 and will pass segment in range(1, 6). That would format into the command as RSEGS True,..., which the instrument will not accept. Add an explicit type check to require an int (and reject bool) before the range check.

[Copilot]

mode not in (0, 1, 2, 3) will allow bools (True == 1 / False == 0). Passing True will send MAGWTR True, which is not valid for the instrument. Add an explicit bool/type check and/or cast to int when formatting the command.

[Copilot]

brightness_level not in (0, 1, 2, 3) allows bools (True == 1 / False == 0), and f-string formatting would send DISP True/False. Add an explicit check to reject bool and require an int, and/or format the command with int(brightness_level).

Suggested change

	if brightness_level not in (0, 1, 2, 3):
	raise ValueError(f"Brightness level must be 0, 1, 2, or 3, got {brightness_level}")
	self.command(f"DISP {brightness_level}")
	if isinstance(brightness_level, bool):
	raise ValueError("Brightness level must be an int between 0 and 3, got bool")
	if not isinstance(brightness_level, int):
	raise ValueError(
	f"Brightness level must be an int between 0 and 3, got {type(brightness_level).__name__}"
	)
	if brightness_level not in (0, 1, 2, 3):
	raise ValueError(f"Brightness level must be 0, 1, 2, or 3, got {brightness_level}")
	self.command(f"DISP {int(brightness_level)}")

[Copilot]

lock_state not in (0, 1, 2) allows bools (True == 1 / False == 0), which would result in LOCK True,<code> being sent. Add an explicit isinstance(lock_state, bool) rejection (or require int and cast with int(lock_state) when formatting).

Suggested change

	if lock_state not in (0, 1, 2):
	raise ValueError(f"Lock state must be 0, 1, or 2, got {lock_state}")
	if not isinstance(code, int) or isinstance(code, bool):
	raise ValueError(f"Lock code must be an integer, got {type(code).__name__}")
	self.command(f"LOCK {lock_state},{code}")
	if isinstance(lock_state, bool) or not isinstance(lock_state, int) or lock_state not in (0, 1, 2):
	raise ValueError(f"Lock state must be 0, 1, or 2, got {lock_state}")
	if not isinstance(code, int) or isinstance(code, bool):
	raise ValueError(f"Lock code must be an integer, got {type(code).__name__}")
	self.command(f"LOCK {int(lock_state)},{code}")

[Copilot]

mode not in (0, 1, 2) allows bools, and f-string formatting would send XPGM True/False. Add an explicit bool/type check and/or cast to int when formatting the command.

Suggested change

	if mode not in (0, 1, 2):
	raise ValueError(f"Programming mode must be 0, 1, or 2, got {mode}")
	self.command(f"XPGM {mode}")
	if not isinstance(mode, int) or isinstance(mode, bool):
	raise ValueError(f"Programming mode must be an integer, got {type(mode).__name__}")
	if mode not in (0, 1, 2):
	raise ValueError(f"Programming mode must be 0, 1, or 2, got {mode}")
	self.command(f"XPGM {int(mode)}")