From fed80b8ab710ae924292a52fa035b8426b9edee7 Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 23 Apr 2026 20:19:21 -0700 Subject: [PATCH 01/33] feat: add modbus RTU client helper for turret controller --- software/control/modbus_rtu.py | 458 +++++++++++++++++++++++++++++++++ 1 file changed, 458 insertions(+) create mode 100644 software/control/modbus_rtu.py diff --git a/software/control/modbus_rtu.py b/software/control/modbus_rtu.py new file mode 100644 index 000000000..2fd00d82f --- /dev/null +++ b/software/control/modbus_rtu.py @@ -0,0 +1,458 @@ +import logging +import struct +import threading +import time +from typing import Optional + +import serial + +logger = logging.getLogger(__name__) + +# CRC-16 Modbus lookup table (polynomial: 0xA001, initial: 0xFFFF) +CRC16_TABLE = [ + 0x0000, + 0xC0C1, + 0xC181, + 0x0140, + 0xC301, + 0x03C0, + 0x0280, + 0xC241, + 0xC601, + 0x06C0, + 0x0780, + 0xC741, + 0x0500, + 0xC5C1, + 0xC481, + 0x0440, + 0xCC01, + 0x0CC0, + 0x0D80, + 0xCD41, + 0x0F00, + 0xCFC1, + 0xCE81, + 0x0E40, + 0x0A00, + 0xCAC1, + 0xCB81, + 0x0B40, + 0xC901, + 0x09C0, + 0x0880, + 0xC841, + 0xD801, + 0x18C0, + 0x1980, + 0xD941, + 0x1B00, + 0xDBC1, + 0xDA81, + 0x1A40, + 0x1E00, + 0xDEC1, + 0xDF81, + 0x1F40, + 0xDD01, + 0x1DC0, + 0x1C80, + 0xDC41, + 0x1400, + 0xD4C1, + 0xD581, + 0x1540, + 0xD701, + 0x17C0, + 0x1680, + 0xD641, + 0xD201, + 0x12C0, + 0x1380, + 0xD341, + 0x1100, + 0xD1C1, + 0xD081, + 0x1040, + 0xF001, + 0x30C0, + 0x3180, + 0xF141, + 0x3300, + 0xF3C1, + 0xF281, + 0x3240, + 0x3600, + 0xF6C1, + 0xF781, + 0x3740, + 0xF501, + 0x35C0, + 0x3480, + 0xF441, + 0x3C00, + 0xFCC1, + 0xFD81, + 0x3D40, + 0xFF01, + 0x3FC0, + 0x3E80, + 0xFE41, + 0xFA01, + 0x3AC0, + 0x3B80, + 0xFB41, + 0x3900, + 0xF9C1, + 0xF881, + 0x3840, + 0x2800, + 0xE8C1, + 0xE981, + 0x2940, + 0xEB01, + 0x2BC0, + 0x2A80, + 0xEA41, + 0xEE01, + 0x2EC0, + 0x2F80, + 0xEF41, + 0x2D00, + 0xEDC1, + 0xEC81, + 0x2C40, + 0xE401, + 0x24C0, + 0x2580, + 0xE541, + 0x2700, + 0xE7C1, + 0xE681, + 0x2640, + 0x2200, + 0xE2C1, + 0xE381, + 0x2340, + 0xE101, + 0x21C0, + 0x2080, + 0xE041, + 0xA001, + 0x60C0, + 0x6180, + 0xA141, + 0x6300, + 0xA3C1, + 0xA281, + 0x6240, + 0x6600, + 0xA6C1, + 0xA781, + 0x6740, + 0xA501, + 0x65C0, + 0x6480, + 0xA441, + 0x6C00, + 0xACC1, + 0xAD81, + 0x6D40, + 0xAF01, + 0x6FC0, + 0x6E80, + 0xAE41, + 0xAA01, + 0x6AC0, + 0x6B80, + 0xAB41, + 0x6900, + 0xA9C1, + 0xA881, + 0x6840, + 0x7800, + 0xB8C1, + 0xB981, + 0x7940, + 0xBB01, + 0x7BC0, + 0x7A80, + 0xBA41, + 0xBE01, + 0x7EC0, + 0x7F80, + 0xBF41, + 0x7D00, + 0xBDC1, + 0xBC81, + 0x7C40, + 0xB401, + 0x74C0, + 0x7580, + 0xB541, + 0x7700, + 0xB7C1, + 0xB681, + 0x7640, + 0x7200, + 0xB2C1, + 0xB381, + 0x7340, + 0xB101, + 0x71C0, + 0x7080, + 0xB041, + 0x5000, + 0x90C1, + 0x9181, + 0x5140, + 0x9301, + 0x53C0, + 0x5280, + 0x9241, + 0x9601, + 0x56C0, + 0x5780, + 0x9741, + 0x5500, + 0x95C1, + 0x9481, + 0x5440, + 0x9C01, + 0x5CC0, + 0x5D80, + 0x9D41, + 0x5F00, + 0x9FC1, + 0x9E81, + 0x5E40, + 0x5A00, + 0x9AC1, + 0x9B81, + 0x5B40, + 0x9901, + 0x59C0, + 0x5880, + 0x9841, + 0x8801, + 0x48C0, + 0x4980, + 0x8941, + 0x4B00, + 0x8BC1, + 0x8A81, + 0x4A40, + 0x4E00, + 0x8EC1, + 0x8F81, + 0x4F40, + 0x8D01, + 0x4DC0, + 0x4C80, + 0x8C41, + 0x4400, + 0x84C1, + 0x8581, + 0x4540, + 0x8701, + 0x47C0, + 0x4680, + 0x8641, + 0x8201, + 0x42C0, + 0x4380, + 0x8341, + 0x4100, + 0x81C1, + 0x8081, + 0x4040, +] + +FRAME_INTERVAL = 0.003 + + +def calculate_crc(data: bytes | bytearray) -> int: + crc = 0xFFFF + for byte in data: + crc = (crc >> 8) ^ CRC16_TABLE[(crc ^ byte) & 0xFF] + return crc + + +def _append_crc(data: bytes | bytearray) -> bytes: + crc = calculate_crc(data) + return bytes(data) + bytes([crc & 0xFF, (crc >> 8) & 0xFF]) + + +def _verify_crc(data: bytes | bytearray) -> bool: + if len(data) < 3: + return False + payload = data[:-2] + received_crc = data[-2] | (data[-1] << 8) + return calculate_crc(payload) == received_crc + + +def build_read_registers_frame(slave_id: int, address: int, count: int) -> bytes: + frame = struct.pack(">BBHH", slave_id, 0x03, address, count) + return _append_crc(frame) + + +def build_write_register_frame(slave_id: int, address: int, value: int) -> bytes: + frame = struct.pack(">BBHH", slave_id, 0x06, address, value) + return _append_crc(frame) + + +def build_write_multiple_registers_frame(slave_id: int, address: int, values: list[int]) -> bytes: + count = len(values) + byte_count = count * 2 + frame = struct.pack(">BBHHB", slave_id, 0x10, address, count, byte_count) + for v in values: + frame += struct.pack(">H", v) + return _append_crc(frame) + + +class ModbusError(Exception): + def __init__(self, message: str, slave_id: Optional[int] = None): + super().__init__(message) + self.message = message + self.slave_id = slave_id + + def __str__(self) -> str: + return self.message + + +class ModbusRTUClient: + def __init__( + self, + port: Optional[str] = None, + baudrate: int = 115200, + timeout: float = 0.5, + retries: int = 3, + ): + self._port = port + self._baudrate = baudrate + self._timeout = timeout + self._retries = retries + self._serial: Optional[serial.Serial] = None + self._lock = threading.Lock() + + @property + def is_connected(self) -> bool: + return self._serial is not None and self._serial.is_open + + def connect(self, port: Optional[str] = None, baudrate: Optional[int] = None): + if port is not None: + self._port = port + if baudrate is not None: + self._baudrate = baudrate + if self._port is None: + raise ModbusError("No serial port specified") + if self._serial is not None: + self._serial.close() + self._serial = None + try: + self._serial = serial.Serial(self._port, baudrate=self._baudrate, timeout=self._timeout) + except (serial.SerialException, OSError) as e: + raise ModbusError(str(e)) from e + logger.info(f"Modbus RTU connected: {self._port}") + + def disconnect(self): + if self._serial is not None: + try: + self._serial.close() + finally: + self._serial = None + logger.info("Modbus RTU disconnected") + + def _require_connected(self): + if not self.is_connected: + raise ModbusError("Client is not connected") + + def read_register(self, slave_id: int, address: int) -> int: + self._require_connected() + frame = build_read_registers_frame(slave_id, address, 1) + # Response: slave(1) + fc(1) + byte_count(1) + data(2) + crc(2) = 7 + response = self._send_receive(frame, expected_response_len=7) + return (response[3] << 8) | response[4] + + def read_register_32bit(self, slave_id: int, address: int, signed: bool = False) -> int: + self._require_connected() + frame = build_read_registers_frame(slave_id, address, 2) + # Response: slave(1) + fc(1) + byte_count(1) + data(4) + crc(2) = 9 + response = self._send_receive(frame, expected_response_len=9) + high = (response[3] << 8) | response[4] + low = (response[5] << 8) | response[6] + value = (high << 16) | low + if signed and value >= 0x80000000: + value -= 0x100000000 + return value + + def write_register(self, slave_id: int, address: int, value: int): + self._require_connected() + frame = build_write_register_frame(slave_id, address, value) + # Response: slave(1) + fc(1) + address(2) + value(2) + crc(2) = 8 + self._send_receive(frame, expected_response_len=8) + + def write_register_32bit(self, slave_id: int, address: int, value: int, signed: bool = False): + self._require_connected() + if signed and value < 0: + value += 0x100000000 + high = (value >> 16) & 0xFFFF + low = value & 0xFFFF + frame = build_write_multiple_registers_frame(slave_id, address, [high, low]) + # Response: slave(1) + fc(1) + address(2) + quantity(2) + crc(2) = 8 + self._send_receive(frame, expected_response_len=8) + + def _send_receive(self, frame: bytes, expected_response_len: int) -> bytes: + with self._lock: + last_error: Optional[Exception] = None + for attempt in range(self._retries + 1): + try: + self._serial.reset_input_buffer() + self._serial.write(frame) + time.sleep(FRAME_INTERVAL) + response = self._serial.read(expected_response_len) + except (serial.SerialException, OSError) as e: + last_error = ModbusError(str(e), slave_id=frame[0]) + logger.warning(f"Modbus request failed (attempt {attempt + 1}/" f"{self._retries + 1}): {e}") + if attempt < self._retries: + time.sleep(FRAME_INTERVAL * 2) + continue + + # Exception responses are 5 bytes — check before incomplete check + if len(response) >= 5 and (response[1] & 0x80) and _verify_crc(response[:5]): + exception_code = response[2] + raise ModbusError( + f"Modbus exception response: FC=0x{response[1]:02X}, " f"code=0x{exception_code:02X}", + slave_id=response[0], + ) + + if len(response) < expected_response_len: + last_error = ModbusError( + f"Incomplete response: expected {expected_response_len} " f"bytes, got {len(response)}", + slave_id=frame[0], + ) + logger.warning( + f"Modbus request failed (attempt {attempt + 1}/" f"{self._retries + 1}): {last_error}" + ) + if attempt < self._retries: + time.sleep(FRAME_INTERVAL * 2) + continue + + if not _verify_crc(response): + last_error = ModbusError("CRC verification failed", slave_id=frame[0]) + logger.warning( + f"Modbus request failed (attempt {attempt + 1}/" f"{self._retries + 1}): {last_error}" + ) + if attempt < self._retries: + time.sleep(FRAME_INTERVAL * 2) + continue + + return response + + raise last_error + + def __enter__(self) -> "ModbusRTUClient": + return self + + def __exit__(self, exc_type, exc_val, exc_tb) -> None: + self.disconnect() From 776ff4a64bf79a7eb69f86442c742d4547fe0e5b Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 23 Apr 2026 20:24:05 -0700 Subject: [PATCH 02/33] fix: use __future__ annotations in modbus_rtu for py38/py39 --- software/control/modbus_rtu.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/software/control/modbus_rtu.py b/software/control/modbus_rtu.py index 2fd00d82f..9aa610216 100644 --- a/software/control/modbus_rtu.py +++ b/software/control/modbus_rtu.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import logging import struct import threading From 56de368fa321d2ff1b3e8545d4a75a8ed0412521 Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 23 Apr 2026 20:26:03 -0700 Subject: [PATCH 03/33] feat: add objective turret _def constants and mutual-exclusion guard Co-Authored-By: Claude Sonnet 4.6 --- software/control/_def.py | 19 +++++++++++++ .../control/test_objective_changer_config.py | 27 +++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 software/tests/control/test_objective_changer_config.py diff --git a/software/control/_def.py b/software/control/_def.py index 182a0683e..a20e482e6 100644 --- a/software/control/_def.py +++ b/software/control/_def.py @@ -1221,6 +1221,22 @@ def get_wellplate_settings(wellplate_format): XERYON_OBJECTIVE_SWITCHER_POS_2 = ["20x", "40x", "60x"] XERYON_OBJECTIVE_SWITCHER_POS_2_OFFSET_MM = 2 +# Motorized 4-position objective turret (NiMotion RS-485 stepper, Modbus-RTU) +USE_OBJECTIVE_TURRET = False +OBJECTIVE_TURRET_SERIAL_NUMBER = "" +OBJECTIVE_TURRET_SLAVE_ID = 1 +OBJECTIVE_TURRET_BAUDRATE = 115200 +# Objective name -> turret slot index (1..4). Override per machine in .ini. +OBJECTIVE_TURRET_POSITIONS = {"4x": 1, "10x": 2, "20x": 3, "40x": 4} + + +def _validate_objective_changer_flags(use_xeryon: bool, use_turret: bool) -> None: + if use_xeryon and use_turret: + raise ValueError( + "USE_XERYON and USE_OBJECTIVE_TURRET are mutually exclusive " "(set only one to True in the machine .ini)" + ) + + # fluidics RUN_FLUIDICS = False FLUIDICS_CONFIG_PATH = "./merfish_config/MERFISH_config.json" @@ -1325,6 +1341,8 @@ class SlackNotifications: myclass = locals()[classkey] populate_class_from_dict(myclass, pop_items) + _validate_objective_changer_flags(USE_XERYON, USE_OBJECTIVE_TURRET) + with open("cache/config_file_path.txt", "w") as file: file.write(config_files[0]) CACHED_CONFIG_FILE_PATH = config_files[0] @@ -1337,6 +1355,7 @@ class SlackNotifications: sys.exit(1) log.info("load machine-specific configuration") exec(open(config_files[0]).read()) + _validate_objective_changer_flags(USE_XERYON, USE_OBJECTIVE_TURRET) else: log.error("machine-specific configuration not present, the program will exit") sys.exit(1) diff --git a/software/tests/control/test_objective_changer_config.py b/software/tests/control/test_objective_changer_config.py new file mode 100644 index 000000000..452e1aefa --- /dev/null +++ b/software/tests/control/test_objective_changer_config.py @@ -0,0 +1,27 @@ +"""Unit tests for mutual-exclusion of Xeryon and turret flags.""" + +import pytest + +from control._def import _validate_objective_changer_flags, OBJECTIVE_TURRET_POSITIONS + + +def test_mutual_exclusion_raises_when_both_true(): + with pytest.raises(ValueError, match="mutually exclusive"): + _validate_objective_changer_flags(use_xeryon=True, use_turret=True) + + +def test_mutual_exclusion_allows_xeryon_only(): + _validate_objective_changer_flags(use_xeryon=True, use_turret=False) + + +def test_mutual_exclusion_allows_turret_only(): + _validate_objective_changer_flags(use_xeryon=False, use_turret=True) + + +def test_mutual_exclusion_allows_neither(): + _validate_objective_changer_flags(use_xeryon=False, use_turret=False) + + +def test_objective_turret_positions_shape(): + assert len(OBJECTIVE_TURRET_POSITIONS) == 4 + assert sorted(OBJECTIVE_TURRET_POSITIONS.values()) == [1, 2, 3, 4] From 6bb036cdb5de47cf2b826c654d65fb063f2c58b5 Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 23 Apr 2026 20:29:41 -0700 Subject: [PATCH 04/33] test: add failing tests for ObjectiveTurret4PosControllerSimulation --- .../test_objective_turret_controller.py | 147 ++++++++++++++++++ 1 file changed, 147 insertions(+) create mode 100644 software/tests/control/test_objective_turret_controller.py diff --git a/software/tests/control/test_objective_turret_controller.py b/software/tests/control/test_objective_turret_controller.py new file mode 100644 index 000000000..ce15ce4ea --- /dev/null +++ b/software/tests/control/test_objective_turret_controller.py @@ -0,0 +1,147 @@ +"""Tests for ObjectiveTurret4PosControllerSimulation (no hardware required).""" + +from __future__ import annotations + +import pytest + +import control._def +from control._def import OBJECTIVE_TURRET_POSITIONS, OBJECTIVE_RETRACTED_POS_MM +from control.objective_turret_controller import ( + ObjectiveTurret4PosControllerSimulation, +) + + +class FakeStage: + """Records move_z_to calls and reports a preset Z position.""" + + def __init__(self, z_mm: float = 3.5): + self._z_mm = z_mm + self.z_moves: list[float] = [] + + def move_z_to(self, abs_mm: float, blocking: bool = True): + self.z_moves.append(abs_mm) + self._z_mm = abs_mm + + def get_pos(self): + class _Pos: + pass + + p = _Pos() + p.z_mm = self._z_mm + return p + + +def _make_sim(stage=None): + return ObjectiveTurret4PosControllerSimulation( + serial_number="SIM-001", + positions=OBJECTIVE_TURRET_POSITIONS, + stage=stage, + ) + + +def test_init_opens_controller(): + sim = _make_sim() + assert sim.is_open + assert sim.current_objective is None + sim.close() + + +def test_home_clears_current_objective(): + sim = _make_sim() + sim.move_to_objective("10x") + sim.home() + assert sim.current_objective is None + sim.close() + + +@pytest.mark.parametrize("name", list(OBJECTIVE_TURRET_POSITIONS)) +def test_move_to_each_known_objective(name): + sim = _make_sim() + sim.move_to_objective(name) + assert sim.current_objective == name + sim.close() + + +def test_move_unknown_objective_raises_key_error(): + sim = _make_sim() + with pytest.raises(KeyError): + sim.move_to_objective("1000x") + sim.close() + + +def test_clear_alarm_is_callable(): + sim = _make_sim() + sim.clear_alarm() + assert sim.is_open + sim.close() + + +def test_enable_is_callable(): + sim = _make_sim() + sim.enable() + assert sim.is_open + sim.close() + + +def test_operations_after_close_raise(): + sim = _make_sim() + sim.close() + with pytest.raises(RuntimeError): + sim.home() + with pytest.raises(RuntimeError): + sim.move_to_objective("10x") + with pytest.raises(RuntimeError): + sim.clear_alarm() + with pytest.raises(RuntimeError): + sim.enable() + + +def test_close_is_idempotent(): + sim = _make_sim() + sim.close() + sim.close() + assert not sim.is_open + + +def test_context_manager_closes_on_exit(): + with _make_sim() as sim: + sim.move_to_objective("20x") + assert sim.is_open + assert not sim.is_open + + +def test_move_to_objective_retracts_and_restores_z(monkeypatch): + monkeypatch.setattr(control._def, "HOMING_ENABLED_Z", True) + stage = FakeStage(z_mm=3.5) + sim = _make_sim(stage=stage) + + sim.move_to_objective("40x") + + # First switch: retract to OBJECTIVE_RETRACTED_POS_MM, then restore captured z. + assert stage.z_moves == [OBJECTIVE_RETRACTED_POS_MM, 3.5] + assert sim.current_objective == "40x" + + # Second call with same objective: no-op (early exit), no new z motion. + stage.z_moves.clear() + sim.move_to_objective("40x") + assert stage.z_moves == [] + + sim.close() + + +def test_move_to_objective_skips_z_retract_when_no_stage(monkeypatch): + monkeypatch.setattr(control._def, "HOMING_ENABLED_Z", True) + sim = _make_sim(stage=None) + sim.move_to_objective("10x") # must not raise even without a stage + assert sim.current_objective == "10x" + sim.close() + + +def test_move_to_objective_skips_z_retract_when_homing_z_disabled(monkeypatch): + monkeypatch.setattr(control._def, "HOMING_ENABLED_Z", False) + stage = FakeStage(z_mm=3.5) + sim = _make_sim(stage=stage) + sim.move_to_objective("10x") + assert stage.z_moves == [] # retract is gated on HOMING_ENABLED_Z + assert sim.current_objective == "10x" + sim.close() From 6603b3692326605a6b584686c12fb7d221f956ca Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 23 Apr 2026 20:42:12 -0700 Subject: [PATCH 05/33] feat: add ObjectiveTurret4PosControllerSimulation with Z retract/restore Co-Authored-By: Claude Sonnet 4.6 --- .../control/objective_turret_controller.py | 190 ++++++++++++++++++ 1 file changed, 190 insertions(+) create mode 100644 software/control/objective_turret_controller.py diff --git a/software/control/objective_turret_controller.py b/software/control/objective_turret_controller.py new file mode 100644 index 000000000..8c23b1f8d --- /dev/null +++ b/software/control/objective_turret_controller.py @@ -0,0 +1,190 @@ +"""Controller for a motorized 4-position objective turret (NiMotion RS-485 stepper). + +The real controller talks Modbus-RTU to the motor. A simulation twin mirrors +the public API for CI and offline development. +""" + +from __future__ import annotations + +import logging +import time +from typing import Optional + +from serial.tools import list_ports + +from control.modbus_rtu import ModbusRTUClient + +logger = logging.getLogger(__name__) + +# Turret mechanics +GEAR_RATIO = 132 / 48 +MOTOR_STEPS_PER_REV = 200 +POSITIONS_PER_REV = 4 # 90 degrees per objective +POSITION_TOLERANCE_PULSES = 50 + +# NiMotion Modbus register map +REG_SAVE_PARAMS = 0x0008 +REG_DI_FUNCTION = 0x002C +REG_MICROSTEP = 0x001A +REG_STATUS_WORD = 0x001F +REG_CURRENT_POSITION = 0x0021 +REG_RUN_MODE = 0x0039 +REG_CONTROL_WORD = 0x0051 +REG_TARGET_POSITION = 0x0053 +REG_MAX_SPEED = 0x005B +REG_ACCEL = 0x005F +REG_DECEL = 0x0061 +REG_HOMING_OFFSET = 0x0069 +REG_HOMING_METHOD = 0x006B +REG_ZERO_RETURN = 0x0072 +REG_CLEAR_ERROR_STORAGE = 0x0073 + +# Control word values +CW_DISABLE = 0x0000 +CW_STARTUP = 0x0006 +CW_ENABLE = 0x0007 +CW_RUN_ABSOLUTE = 0x000F +CW_TRIGGER_ABSOLUTE = 0x001F +CW_CLEAR_FAULT = 0x0080 + +# Magic values +SAVE_PARAMS_MAGIC = 0x7376 +CLEAR_ERROR_STORAGE_MAGIC = 0x6C64 + +# Run modes +MODE_POSITION = 1 +MODE_HOMING = 3 + +# Status word bits +STATUS_BIT_FAULT = 1 << 3 +STATUS_BIT_RUNNING = 1 << 12 + +# Motion parameter defaults (auto-calibrated on first connect) +EXPECTED_ACCEL = 200 +EXPECTED_DECEL = 200 +EXPECTED_MAX_SPEED = 250 + +# Homing defaults (auto-calibrated on first connect) +HOMING_METHOD = 17 +HOMING_ORIGIN_OFFSET = 500 +HOMING_ZERO_RETURN = 1 +DI1_FUNCTION_NEG_LIMIT = 1 + +# Polling +POLL_INTERVAL_S = 0.05 +DEFAULT_MOVE_TIMEOUT_S = 10.0 +DEFAULT_HOME_TIMEOUT_S = 30.0 + + +def _resolve_position(objective_name: str, positions: dict) -> int: + try: + return positions[objective_name] + except KeyError: + raise KeyError(f"Unknown objective '{objective_name}'. Valid names: {sorted(positions)}") from None + + +def _find_port(serial_number: str) -> str: + matches = [p.device for p in list_ports.comports() if p.serial_number == serial_number] + if not matches: + raise ValueError(f"No serial device found with serial number: {serial_number}") + if len(matches) > 1: + logger.warning( + "Multiple devices match serial number %s: %s. Using %s.", + serial_number, + matches, + matches[0], + ) + return matches[0] + + +class ObjectiveTurret4PosControllerSimulation: + """In-memory stand-in for ObjectiveTurret4PosController. + + Mirrors the real controller's public API for tests and offline use. + Implements the Z retract/restore dance when a stage reference is provided. + """ + + def __init__( + self, + serial_number: Optional[str] = None, + slave_id: int = 1, + baudrate: int = 115200, + timeout: float = 0.5, + positions: Optional[dict] = None, + stage=None, + ): + from control._def import OBJECTIVE_TURRET_POSITIONS + + self._is_open = True + self._current_objective: Optional[str] = None + self._positions = dict(positions) if positions is not None else dict(OBJECTIVE_TURRET_POSITIONS) + self._stage = stage + logger.info("Simulated turret opened (sn=%s)", serial_number) + + def home(self, timeout_s: float = DEFAULT_HOME_TIMEOUT_S) -> None: + self._require_open() + self._current_objective = None + logger.info("Simulated turret homed") + + def enable(self) -> None: + """Mirror of the real controller's disable -> startup -> enable state-machine cycle.""" + self._require_open() + logger.info("Simulated turret enabled") + + def move_to_objective(self, objective_name: str, timeout_s: float = DEFAULT_MOVE_TIMEOUT_S) -> None: + self._require_open() + _resolve_position(objective_name, self._positions) + if self._current_objective == objective_name: + return + + captured_z = self._retract_z_if_possible() + self._current_objective = objective_name + self._restore_z_if_captured(captured_z) + + logger.info( + "Simulated turret moved to %s (position %d)", + objective_name, + self._positions[objective_name], + ) + + def clear_alarm(self) -> None: + self._require_open() + logger.info("Simulated turret alarm cleared") + + def close(self) -> None: + if self._is_open: + self._is_open = False + logger.info("Simulated turret closed") + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_val, exc_tb): + self.close() + + @property + def current_objective(self) -> Optional[str]: + return self._current_objective + + @property + def is_open(self) -> bool: + return self._is_open + + def _require_open(self) -> None: + if not self._is_open: + raise RuntimeError("Turret controller is closed") + + def _retract_z_if_possible(self) -> Optional[float]: + """If stage + Z homing are usable, capture Z and move to safe retract. Return captured z, else None.""" + from control._def import HOMING_ENABLED_Z, OBJECTIVE_RETRACTED_POS_MM + + if self._stage is None or not HOMING_ENABLED_Z: + return None + z_mm = self._stage.get_pos().z_mm + self._stage.move_z_to(OBJECTIVE_RETRACTED_POS_MM) + return z_mm + + def _restore_z_if_captured(self, captured_z: Optional[float]) -> None: + if captured_z is None or self._stage is None: + return + self._stage.move_z_to(captured_z) From 62a91be0861f5e557bd9e1c1c30b244acc5b2b6a Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 23 Apr 2026 20:44:46 -0700 Subject: [PATCH 06/33] refactor: drop unused imports and add stage type hint on turret sim --- software/control/objective_turret_controller.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/software/control/objective_turret_controller.py b/software/control/objective_turret_controller.py index 8c23b1f8d..0cc6aa0f4 100644 --- a/software/control/objective_turret_controller.py +++ b/software/control/objective_turret_controller.py @@ -7,12 +7,11 @@ from __future__ import annotations import logging -import time from typing import Optional from serial.tools import list_ports -from control.modbus_rtu import ModbusRTUClient +import squid.abc logger = logging.getLogger(__name__) @@ -111,7 +110,7 @@ def __init__( baudrate: int = 115200, timeout: float = 0.5, positions: Optional[dict] = None, - stage=None, + stage: Optional[squid.abc.AbstractStage] = None, ): from control._def import OBJECTIVE_TURRET_POSITIONS @@ -159,7 +158,7 @@ def close(self) -> None: def __enter__(self): return self - def __exit__(self, exc_type, exc_val, exc_tb): + def __exit__(self, exc_type, exc_val, exc_tb) -> None: self.close() @property From 4190b58c328c8548a60193195ee223961c02e7a0 Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 23 Apr 2026 20:46:57 -0700 Subject: [PATCH 07/33] feat: add ObjectiveTurret4PosController (real Modbus-RTU controller) Co-Authored-By: Claude Sonnet 4.6 --- .../control/objective_turret_controller.py | 261 ++++++++++++++++++ 1 file changed, 261 insertions(+) diff --git a/software/control/objective_turret_controller.py b/software/control/objective_turret_controller.py index 0cc6aa0f4..24a71eceb 100644 --- a/software/control/objective_turret_controller.py +++ b/software/control/objective_turret_controller.py @@ -7,9 +7,11 @@ from __future__ import annotations import logging +import time from typing import Optional from serial.tools import list_ports +from control.modbus_rtu import ModbusRTUClient import squid.abc @@ -187,3 +189,262 @@ def _restore_z_if_captured(self, captured_z: Optional[float]) -> None: if captured_z is None or self._stage is None: return self._stage.move_z_to(captured_z) + + +class ObjectiveTurret4PosController: + """Synchronous controller for a 4-position objective turret over Modbus-RTU.""" + + def __init__( + self, + serial_number: str, + slave_id: int = 1, + baudrate: int = 115200, + timeout: float = 0.5, + positions: Optional[dict] = None, + stage: Optional[squid.abc.AbstractStage] = None, + ) -> None: + from control._def import OBJECTIVE_TURRET_POSITIONS + + self._slave_id = slave_id + self._positions = dict(positions) if positions is not None else dict(OBJECTIVE_TURRET_POSITIONS) + self._stage = stage + self._current_objective: Optional[str] = None + self._is_open = False + + port = _find_port(serial_number) + self._modbus = ModbusRTUClient(port=port, baudrate=baudrate, timeout=timeout) + self._modbus.connect() + try: + self.clear_alarm() + + microstep_raw = self._modbus.read_register(self._slave_id, REG_MICROSTEP) + if not 0 <= microstep_raw <= 7: + raise ValueError(f"Invalid microstep register value {microstep_raw} (expected 0..7)") + self._microstep = 2**microstep_raw + self._pulses_per_position = int(MOTOR_STEPS_PER_REV * self._microstep * GEAR_RATIO / POSITIONS_PER_REV) + + changed = [self._calibrate_motion_params(), self._calibrate_homing_config()] + if any(changed): + self._save_to_eeprom() + + logger.info( + "Turret controller ready: port=%s microstep=%d pulses/position=%d calibrated=%s", + port, + self._microstep, + self._pulses_per_position, + any(changed), + ) + + self.enable() + self._is_open = True + except Exception: + self._modbus.disconnect() + raise + + def home(self, timeout_s: float = DEFAULT_HOME_TIMEOUT_S) -> None: + self._require_open() + self._write_control(CW_DISABLE) + self._write_holding(REG_RUN_MODE, MODE_HOMING) + self._write_control(CW_STARTUP) + self._write_control(CW_ENABLE) + self._write_control(CW_RUN_ABSOLUTE) + self._write_control(CW_TRIGGER_ABSOLUTE) + self._wait_until_idle(timeout_s) + self._current_objective = None + + def enable(self) -> None: + """Run the disable -> startup -> enable state-machine cycle.""" + self._write_control(CW_DISABLE) + self._write_control(CW_STARTUP) + self._write_control(CW_ENABLE) + + def move_to_objective(self, objective_name: str, timeout_s: float = DEFAULT_MOVE_TIMEOUT_S) -> None: + self._require_open() + _resolve_position(objective_name, self._positions) + if self._current_objective == objective_name: + return + + captured_z = self._retract_z_if_possible() + self._rotate_to(objective_name, timeout_s) + self._current_objective = objective_name + self._restore_z_if_captured(captured_z) + + def clear_alarm(self) -> None: + self._write_control(CW_CLEAR_FAULT) + self._write_holding(REG_CLEAR_ERROR_STORAGE, CLEAR_ERROR_STORAGE_MAGIC) + + def close(self) -> None: + if not self._is_open and not self._modbus.is_connected: + return + if self._modbus.is_connected: + try: + self._write_control(CW_DISABLE) + except Exception as exc: + logger.warning("Failed to disable motor during close: %s", exc) + self._modbus.disconnect() + self._is_open = False + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_val, exc_tb) -> None: + self.close() + + @property + def pulses_per_position(self) -> int: + return self._pulses_per_position + + @property + def current_position_pulses(self) -> int: + return self._modbus.read_register_32bit(self._slave_id, REG_CURRENT_POSITION, signed=True) + + @property + def current_objective(self) -> Optional[str]: + return self._current_objective + + @property + def is_open(self) -> bool: + return self._is_open + + # --- internal helpers --- + + def _require_open(self) -> None: + if not self._is_open: + raise RuntimeError("Turret controller is closed") + + def _rotate_to(self, objective_name: str, timeout_s: float) -> None: + position_index = _resolve_position(objective_name, self._positions) + target_pulses = (position_index - 1) * self._pulses_per_position + + self._write_control(CW_DISABLE) + self._write_holding(REG_RUN_MODE, MODE_POSITION) + self._modbus.write_register_32bit(self._slave_id, REG_TARGET_POSITION, target_pulses, signed=True) + self._write_control(CW_STARTUP) + self._write_control(CW_ENABLE) + self._write_control(CW_RUN_ABSOLUTE) + self._write_control(CW_TRIGGER_ABSOLUTE) + self._wait_for_position(target_pulses, timeout_s) + + def _retract_z_if_possible(self) -> Optional[float]: + from control._def import HOMING_ENABLED_Z, OBJECTIVE_RETRACTED_POS_MM + + if self._stage is None or not HOMING_ENABLED_Z: + return None + z_mm = self._stage.get_pos().z_mm + self._stage.move_z_to(OBJECTIVE_RETRACTED_POS_MM) + return z_mm + + def _restore_z_if_captured(self, captured_z: Optional[float]) -> None: + if captured_z is None or self._stage is None: + return + self._stage.move_z_to(captured_z) + + def _calibrate_one( + self, + addr: int, + expected: int, + label: str, + *, + is_32bit: bool = False, + signed: bool = False, + mask: Optional[int] = None, + ) -> bool: + if is_32bit: + current = self._modbus.read_register_32bit(self._slave_id, addr, signed=signed) + else: + current = self._modbus.read_register(self._slave_id, addr) + desired = (current & ~mask) | expected if mask is not None else expected + if current == desired: + return False + if is_32bit: + self._modbus.write_register_32bit(self._slave_id, addr, desired, signed=signed) + else: + self._modbus.write_register(self._slave_id, addr, desired) + fmt = "0x%08X" if mask is not None else "%d" + logger.info(f"Calibrated %s: {fmt} -> {fmt}", label, current, desired) + return True + + def _calibrate_motion_params(self) -> bool: + return any( + [ + self._calibrate_one(REG_ACCEL, EXPECTED_ACCEL, "accel", is_32bit=True), + self._calibrate_one(REG_DECEL, EXPECTED_DECEL, "decel", is_32bit=True), + self._calibrate_one(REG_MAX_SPEED, EXPECTED_MAX_SPEED, "max_speed", is_32bit=True), + ] + ) + + def _calibrate_homing_config(self) -> bool: + return any( + [ + self._calibrate_one(REG_HOMING_METHOD, HOMING_METHOD, "homing_method"), + self._calibrate_one( + REG_HOMING_OFFSET, + HOMING_ORIGIN_OFFSET, + "homing_offset", + is_32bit=True, + signed=True, + ), + self._calibrate_one(REG_ZERO_RETURN, HOMING_ZERO_RETURN, "zero_return"), + self._calibrate_one( + REG_DI_FUNCTION, + DI1_FUNCTION_NEG_LIMIT, + "DI1_function", + is_32bit=True, + mask=0xF, + ), + ] + ) + + def _save_to_eeprom(self) -> None: + self._write_holding(REG_SAVE_PARAMS, SAVE_PARAMS_MAGIC) + logger.info("Saved parameters to EEPROM") + + def _write_control(self, value: int) -> None: + self._modbus.write_register(self._slave_id, REG_CONTROL_WORD, value) + + def _write_holding(self, address: int, value: int) -> None: + self._modbus.write_register(self._slave_id, address, value) + + def _read_status_word(self) -> int: + return self._modbus.read_register(self._slave_id, REG_STATUS_WORD) + + @staticmethod + def _check_fault(status_word: int) -> None: + if status_word & STATUS_BIT_FAULT: + raise RuntimeError(f"Motor reported fault (status word=0x{status_word:04X})") + + def _wait_until_idle(self, timeout_s: float) -> None: + deadline = time.monotonic() + timeout_s + time.sleep(POLL_INTERVAL_S) + while time.monotonic() < deadline: + status = self._read_status_word() + self._check_fault(status) + if not (status & STATUS_BIT_RUNNING): + return + time.sleep(POLL_INTERVAL_S) + raise TimeoutError(f"Motion did not finish within {timeout_s:.1f}s") + + def _wait_for_position(self, target_pulses: int, timeout_s: float) -> None: + deadline = time.monotonic() + timeout_s + seen_running = False + last_pos: Optional[int] = None + while time.monotonic() < deadline: + status = self._read_status_word() + self._check_fault(status) + running = bool(status & STATUS_BIT_RUNNING) + last_pos = self.current_position_pulses + in_tolerance = abs(last_pos - target_pulses) <= POSITION_TOLERANCE_PULSES + + if running: + seen_running = True + if in_tolerance and not running: + return + if seen_running and not running and not in_tolerance: + raise RuntimeError( + f"Motor stopped at {last_pos} pulses, target {target_pulses} " + f"(tolerance ±{POSITION_TOLERANCE_PULSES})" + ) + time.sleep(POLL_INTERVAL_S) + raise TimeoutError( + f"Move to {target_pulses} pulses timed out after {timeout_s:.1f}s " f"(last position={last_pos})" + ) From 7cab3a004a1371056c587adf1696ad5eb4363764 Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 23 Apr 2026 20:58:06 -0700 Subject: [PATCH 08/33] docs: note why _wait_for_position has no leading sleep --- software/control/objective_turret_controller.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/software/control/objective_turret_controller.py b/software/control/objective_turret_controller.py index 24a71eceb..cc2300eca 100644 --- a/software/control/objective_turret_controller.py +++ b/software/control/objective_turret_controller.py @@ -425,6 +425,8 @@ def _wait_until_idle(self, timeout_s: float) -> None: raise TimeoutError(f"Motion did not finish within {timeout_s:.1f}s") def _wait_for_position(self, target_pulses: int, timeout_s: float) -> None: + # No leading sleep: seen_running prevents stall detection before the motor asserts RUNNING. + # (Contrast _wait_until_idle which has a leading sleep because it lacks a seen_running guard.) deadline = time.monotonic() + timeout_s seen_running = False last_pos: Optional[int] = None From b2a3f6ea723c068948e418c42522a7624c6331e0 Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 23 Apr 2026 20:59:40 -0700 Subject: [PATCH 09/33] feat: add move_to_objective dispatcher to Xeryon 2-pos controller Co-Authored-By: Claude Sonnet 4.6 --- .../objective_changer_2_pos_controller.py | 30 +++++++++++ ...test_objective_changer_2_pos_controller.py | 53 +++++++++++++++++++ 2 files changed, 83 insertions(+) create mode 100644 software/tests/control/test_objective_changer_2_pos_controller.py diff --git a/software/control/objective_changer_2_pos_controller.py b/software/control/objective_changer_2_pos_controller.py index 5680b4de7..bf2709e07 100644 --- a/software/control/objective_changer_2_pos_controller.py +++ b/software/control/objective_changer_2_pos_controller.py @@ -60,6 +60,21 @@ def currentPosition(self) -> int: def setSpeed(self, value: float): self.axisX.setSpeed(value) + def move_to_objective(self, objective_name: str) -> None: + """Unified dispatcher to moveToPosition1 / moveToPosition2 based on the + per-machine XERYON_OBJECTIVE_SWITCHER_POS_1/POS_2 lists. Short-circuits + if already at the target position. Raises KeyError for unknown names.""" + from control._def import XERYON_OBJECTIVE_SWITCHER_POS_1, XERYON_OBJECTIVE_SWITCHER_POS_2 + + if objective_name in XERYON_OBJECTIVE_SWITCHER_POS_1: + if self.currentPosition() != 1: + self.moveToPosition1() + elif objective_name in XERYON_OBJECTIVE_SWITCHER_POS_2: + if self.currentPosition() != 2: + self.moveToPosition2() + else: + raise KeyError(f"Unknown objective '{objective_name}' for Xeryon 2-pos changer") + class ObjectiveChanger2PosController_Simulation: def __init__(self, sn: str, stage: Optional[squid.abc.AbstractStage] = None): @@ -97,3 +112,18 @@ def currentPosition(self) -> int: def setSpeed(self, value: float): pass + + def move_to_objective(self, objective_name: str) -> None: + """Unified dispatcher to moveToPosition1 / moveToPosition2 based on the + per-machine XERYON_OBJECTIVE_SWITCHER_POS_1/POS_2 lists. Short-circuits + if already at the target position. Raises KeyError for unknown names.""" + from control._def import XERYON_OBJECTIVE_SWITCHER_POS_1, XERYON_OBJECTIVE_SWITCHER_POS_2 + + if objective_name in XERYON_OBJECTIVE_SWITCHER_POS_1: + if self.currentPosition() != 1: + self.moveToPosition1() + elif objective_name in XERYON_OBJECTIVE_SWITCHER_POS_2: + if self.currentPosition() != 2: + self.moveToPosition2() + else: + raise KeyError(f"Unknown objective '{objective_name}' for Xeryon 2-pos changer") diff --git a/software/tests/control/test_objective_changer_2_pos_controller.py b/software/tests/control/test_objective_changer_2_pos_controller.py new file mode 100644 index 000000000..c3ee0661d --- /dev/null +++ b/software/tests/control/test_objective_changer_2_pos_controller.py @@ -0,0 +1,53 @@ +"""Tests for the move_to_objective dispatcher on the Xeryon 2-pos simulation.""" + +from __future__ import annotations + +import control._def +from control.objective_changer_2_pos_controller import ( + ObjectiveChanger2PosController_Simulation, +) + + +class FakeStage: + def __init__(self): + self.z_moves: list[float] = [] + + def move_z(self, delta_mm: float): + self.z_moves.append(delta_mm) + + +def test_move_to_objective_dispatches_pos1(monkeypatch): + monkeypatch.setattr(control._def, "XERYON_OBJECTIVE_SWITCHER_POS_1", ["4x", "10x"]) + monkeypatch.setattr(control._def, "XERYON_OBJECTIVE_SWITCHER_POS_2", ["20x", "40x"]) + sim = ObjectiveChanger2PosController_Simulation(sn="SIM", stage=FakeStage()) + sim.move_to_objective("4x") + assert sim.currentPosition() == 1 + + +def test_move_to_objective_dispatches_pos2(monkeypatch): + monkeypatch.setattr(control._def, "XERYON_OBJECTIVE_SWITCHER_POS_1", ["4x", "10x"]) + monkeypatch.setattr(control._def, "XERYON_OBJECTIVE_SWITCHER_POS_2", ["20x", "40x"]) + sim = ObjectiveChanger2PosController_Simulation(sn="SIM", stage=FakeStage()) + sim.move_to_objective("40x") + assert sim.currentPosition() == 2 + + +def test_move_to_objective_short_circuits_when_already_there(monkeypatch): + monkeypatch.setattr(control._def, "XERYON_OBJECTIVE_SWITCHER_POS_1", ["4x", "10x"]) + monkeypatch.setattr(control._def, "XERYON_OBJECTIVE_SWITCHER_POS_2", ["20x", "40x"]) + stage = FakeStage() + sim = ObjectiveChanger2PosController_Simulation(sn="SIM", stage=stage) + sim.move_to_objective("40x") # pos1 -> pos2: records Z move + z_moves_after_first = list(stage.z_moves) + sim.move_to_objective("40x") # already at pos2: no extra Z move + assert stage.z_moves == z_moves_after_first + + +def test_move_to_objective_unknown_raises(monkeypatch): + monkeypatch.setattr(control._def, "XERYON_OBJECTIVE_SWITCHER_POS_1", ["4x", "10x"]) + monkeypatch.setattr(control._def, "XERYON_OBJECTIVE_SWITCHER_POS_2", ["20x", "40x"]) + import pytest + + sim = ObjectiveChanger2PosController_Simulation(sn="SIM", stage=FakeStage()) + with pytest.raises(KeyError): + sim.move_to_objective("999x") From cc86c8251e031872a19355e716cb96c374b197b5 Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 23 Apr 2026 21:06:39 -0700 Subject: [PATCH 10/33] feat: build objective turret addon in MicroscopeAddons Wire ObjectiveTurret4PosController into MicroscopeAddons.build_from_global_config as an elif branch off the existing USE_XERYON block, using SIMULATE_OBJECTIVE_CHANGER for the simulation flag. Update the __init__ type hint to Optional[object] to accommodate both controller types. Co-Authored-By: Claude Sonnet 4.6 --- software/control/microscope.py | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/software/control/microscope.py b/software/control/microscope.py index 78b8eb782..430f012fa 100644 --- a/software/control/microscope.py +++ b/software/control/microscope.py @@ -38,6 +38,14 @@ else: ObjectiveChanger2PosController = None +if control._def.USE_OBJECTIVE_TURRET: + from control.objective_turret_controller import ( + ObjectiveTurret4PosController, + ObjectiveTurret4PosControllerSimulation, + ) +else: + ObjectiveTurret4PosController = None + if control._def.RUN_FLUIDICS: from control.fluidics import Fluidics else: @@ -129,6 +137,19 @@ def build_from_global_config( if not objective_changer_simulated else ObjectiveChanger2PosController_Simulation(sn=control._def.XERYON_SERIAL_NUMBER, stage=stage) ) + elif control._def.USE_OBJECTIVE_TURRET: + turret_kwargs = dict( + serial_number=control._def.OBJECTIVE_TURRET_SERIAL_NUMBER, + slave_id=control._def.OBJECTIVE_TURRET_SLAVE_ID, + baudrate=control._def.OBJECTIVE_TURRET_BAUDRATE, + positions=control._def.OBJECTIVE_TURRET_POSITIONS, + stage=stage, + ) + objective_changer = ( + ObjectiveTurret4PosController(**turret_kwargs) + if not objective_changer_simulated + else ObjectiveTurret4PosControllerSimulation(**turret_kwargs) + ) camera_focus = None if control._def.SUPPORT_LASER_AUTOFOCUS: @@ -184,7 +205,7 @@ def __init__( nl5: Optional[NL5] = None, cellx: Optional[serial_peripherals.CellX] = None, emission_filter_wheel: Optional[AbstractFilterWheelController] = None, - objective_changer: Optional[ObjectiveChanger2PosController] = None, + objective_changer: Optional[object] = None, camera_focus: Optional[AbstractCamera] = None, fluidics: Optional[Fluidics] = None, piezo_stage: Optional[PiezoStage] = None, From f28eacfd3ff0d046a57431432a34bd23d501c0ed Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 23 Apr 2026 21:08:46 -0700 Subject: [PATCH 11/33] refactor: unify objective-changer startup via move_to_objective Replace the Xeryon-specific pos1/pos2 dispatch in prepare_for_use with a single move_to_objective call that works for both Xeryon and turret controllers. The setSpeed call is kept behind USE_XERYON since the turret has no speed setter. Co-Authored-By: Claude Sonnet 4.6 --- software/control/microscope.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/software/control/microscope.py b/software/control/microscope.py index 430f012fa..94cc8802a 100644 --- a/software/control/microscope.py +++ b/software/control/microscope.py @@ -492,11 +492,9 @@ def _prepare_for_use(self, skip_init: bool = False): if self.addons.objective_changer: self.addons.objective_changer.home() - self.addons.objective_changer.setSpeed(control._def.XERYON_SPEED) - if control._def.DEFAULT_OBJECTIVE in control._def.XERYON_OBJECTIVE_SWITCHER_POS_1: - self.addons.objective_changer.moveToPosition1(move_z=False) - elif control._def.DEFAULT_OBJECTIVE in control._def.XERYON_OBJECTIVE_SWITCHER_POS_2: - self.addons.objective_changer.moveToPosition2(move_z=False) + if control._def.USE_XERYON: + self.addons.objective_changer.setSpeed(control._def.XERYON_SPEED) + self.addons.objective_changer.move_to_objective(control._def.DEFAULT_OBJECTIVE) def _sync_confocal_mode_from_hardware(self) -> bool: """Sync confocal mode state from spinning disk hardware. From a18ec65932d575adb0e8b245857db4fbc2d2c607 Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 23 Apr 2026 21:10:53 -0700 Subject: [PATCH 12/33] refactor: unify ObjectivesWidget dispatch via move_to_objective Co-Authored-By: Claude Sonnet 4.6 --- software/control/gui_hcs.py | 5 +---- software/control/widgets.py | 7 ++----- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/software/control/gui_hcs.py b/software/control/gui_hcs.py index 07414e56e..f52b272bf 100644 --- a/software/control/gui_hcs.py +++ b/software/control/gui_hcs.py @@ -878,10 +878,7 @@ def load_widgets(self): if self.piezo: self.piezoWidget = widgets.PiezoWidget(self.piezo) - if USE_XERYON: - self.objectivesWidget = widgets.ObjectivesWidget(self.objectiveStore, self.objective_changer) - else: - self.objectivesWidget = widgets.ObjectivesWidget(self.objectiveStore) + self.objectivesWidget = widgets.ObjectivesWidget(self.objectiveStore, self.objective_changer) if self.emission_filter_wheel: self.filterControllerWidget = widgets.FilterControllerWidget( diff --git a/software/control/widgets.py b/software/control/widgets.py index 52323c74e..0f2d0718a 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -3557,11 +3557,8 @@ def init_ui(self): def on_objective_changed(self, objective_name): self.objectiveStore.set_current_objective(objective_name) - if USE_XERYON: - if objective_name in XERYON_OBJECTIVE_SWITCHER_POS_1 and self.objective_changer.currentPosition() != 1: - self.objective_changer.moveToPosition1() - elif objective_name in XERYON_OBJECTIVE_SWITCHER_POS_2 and self.objective_changer.currentPosition() != 2: - self.objective_changer.moveToPosition2() + if self.objective_changer is not None: + self.objective_changer.move_to_objective(objective_name) self.signal_objective_changed.emit() From 103535ecfb6e55f833b9931ef4705407d47e8dea Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 23 Apr 2026 21:13:05 -0700 Subject: [PATCH 13/33] feat: dispatch objective-changer shutdown by controller type On full shutdown, retract Z then call moveToZero() for Xeryon or close() for the NiMotion turret. The outer restart-gate is unchanged so turret-only machines skip the block on restart (turret re-inits from any position without needing a home move). Co-Authored-By: Claude Sonnet 4.6 --- software/control/gui_hcs.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/software/control/gui_hcs.py b/software/control/gui_hcs.py index f52b272bf..d92b985b1 100644 --- a/software/control/gui_hcs.py +++ b/software/control/gui_hcs.py @@ -2718,9 +2718,12 @@ def _cleanup_common(self, for_restart: bool = False): else: raise - if USE_XERYON and self.objective_changer and z_retracted: + if self.objective_changer and z_retracted: try: - self.objective_changer.moveToZero() + if USE_XERYON: + self.objective_changer.moveToZero() + elif USE_OBJECTIVE_TURRET: + self.objective_changer.close() except Exception: if for_restart: self.log.exception(f"Error resetting objective changer during {context}") From 25caed525dc2725f8953847e47da047c1d58b203 Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 23 Apr 2026 21:14:42 -0700 Subject: [PATCH 14/33] feat: add resetObjectiveTurret handler on HighContentScreeningGui --- software/control/gui_hcs.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/software/control/gui_hcs.py b/software/control/gui_hcs.py index d92b985b1..474439afd 100644 --- a/software/control/gui_hcs.py +++ b/software/control/gui_hcs.py @@ -1925,6 +1925,25 @@ def openWorkflowRunner(self): self.workflowRunnerDialog.raise_() self.workflowRunnerDialog.activateWindow() + def resetObjectiveTurret(self): + """Clear faults, re-enable the motor, and rotate back to the current objective.""" + if self.objective_changer is None: + return + self.log.info("Resetting objective turret") + try: + self.objective_changer.clear_alarm() + self.objective_changer.enable() + self.objective_changer.move_to_objective(self.objectiveStore.current_objective) + except Exception as exc: + self.log.exception("Reset of objective turret failed") + from qtpy.QtWidgets import QMessageBox + + QMessageBox.warning( + self, + "Reset Objective Turret", + f"Failed to reset objective turret:\n{exc}", + ) + def _get_actual_acquisition_path(self) -> str: """Get the actual acquisition path (base_path + experiment_ID with timestamp).""" if hasattr(self, "multipointController") and self.multipointController: From 9311477db441aac31e09164b61828a1910e86778 Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 23 Apr 2026 21:16:54 -0700 Subject: [PATCH 15/33] refactor: drop redundant QMessageBox local import in resetObjectiveTurret --- software/control/gui_hcs.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/software/control/gui_hcs.py b/software/control/gui_hcs.py index 474439afd..acede11d3 100644 --- a/software/control/gui_hcs.py +++ b/software/control/gui_hcs.py @@ -1936,8 +1936,6 @@ def resetObjectiveTurret(self): self.objective_changer.move_to_objective(self.objectiveStore.current_objective) except Exception as exc: self.log.exception("Reset of objective turret failed") - from qtpy.QtWidgets import QMessageBox - QMessageBox.warning( self, "Reset Objective Turret", From 44bd390c8d2839a8247e91d16853b17e1e7e3be7 Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 23 Apr 2026 21:17:27 -0700 Subject: [PATCH 16/33] feat: add Reset Objective Turret entry to Utils menu --- software/main_hcs.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/software/main_hcs.py b/software/main_hcs.py index 54ac13522..8e2e1ba75 100644 --- a/software/main_hcs.py +++ b/software/main_hcs.py @@ -88,6 +88,11 @@ stage_utils_action.triggered.connect(win.stageUtils.show) microscope_utils_menu.addAction(stage_utils_action) + if control._def.USE_OBJECTIVE_TURRET: + reset_turret_action = QAction("Reset Objective Turret", win) + reset_turret_action.triggered.connect(win.resetObjectiveTurret) + microscope_utils_menu.addAction(reset_turret_action) + workflow_runner_action = QAction("Workflow Runner...", win) workflow_runner_action.triggered.connect(win.openWorkflowRunner) microscope_utils_menu.addAction(workflow_runner_action) From 8f91a4be7ccc430eafbe2635e3fdaf0229a6dbc9 Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 23 Apr 2026 21:32:20 -0700 Subject: [PATCH 17/33] fix: make Reset Objective Turret re-home before rotating Re-home the turret in resetObjectiveTurret so its position tracker is always re-synced to the physical slot. Without this, a fault mid-rotation could leave _current_objective matching the reset target, causing move_to_objective to short-circuit and skip the physical rotate. Also update three stale comments/docstrings in the shutdown/restart path that only mentioned Xeryon to reflect both controller types. Co-Authored-By: Claude Opus 4.7 (1M context) --- software/control/gui_hcs.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/software/control/gui_hcs.py b/software/control/gui_hcs.py index acede11d3..52df6a497 100644 --- a/software/control/gui_hcs.py +++ b/software/control/gui_hcs.py @@ -1926,13 +1926,16 @@ def openWorkflowRunner(self): self.workflowRunnerDialog.activateWindow() def resetObjectiveTurret(self): - """Clear faults, re-enable the motor, and rotate back to the current objective.""" + """Clear faults, re-enable the motor, re-home, and rotate back to the current objective.""" if self.objective_changer is None: return self.log.info("Resetting objective turret") try: self.objective_changer.clear_alarm() self.objective_changer.enable() + # Re-home so the position tracker matches the physical slot: avoids short-circuiting the + # rotate in move_to_objective if the tracker is stale from a fault mid-rotation. + self.objective_changer.home() self.objective_changer.move_to_objective(self.objectiveStore.current_objective) except Exception as exc: self.log.exception("Reset of objective turret failed") @@ -2616,8 +2619,9 @@ def _cleanup_common(self, for_restart: bool = False): Args: for_restart: If True, wrap operations in try-except to ensure cleanup completes. - Z retraction and objective reset still run when using Xeryon - (Xeryon must be zeroed before re-init), but are skipped otherwise. + Z retraction and objective-changer teardown still run when using Xeryon + (Xeryon must be zeroed before re-init). For a turret or a manual setup, + restart skips this block (turret re-inits cleanly from any position). """ context = "restart" if for_restart else "shutdown" @@ -2721,9 +2725,10 @@ def _cleanup_common(self, for_restart: bool = False): else: raise - # Retract Z and reset objective changer on full shutdown. - # On restart, only retract Z and reset if Xeryon objective changer is present - # (Xeryon must be zeroed before re-init; Z must retract first for safety). + # Retract Z and tear down the objective changer on full shutdown. + # On restart, this runs only for Xeryon (which must be zeroed before re-init); + # the turret is skipped because it re-inits cleanly from any position. + # Xeryon gets moveToZero(); turret gets close() (disables motor + disconnects serial). if not for_restart or USE_XERYON: z_retracted = False try: @@ -2792,7 +2797,8 @@ def _cleanup_common(self, for_restart: bool = False): raise def _cleanup_for_restart(self): - """Clean up hardware and resources for restart. Retracts Z and resets Xeryon if present.""" + """Clean up hardware and resources for restart. Retracts Z and zeros Xeryon if present + (skipped for turret or manual setups; the turret re-inits cleanly from any position).""" self._cleanup_common(for_restart=True) def closeEvent(self, event): From fde7fe4ac9fc382db642963e887c0daadee6a047 Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 23 Apr 2026 22:17:33 -0700 Subject: [PATCH 18/33] fix: address Copilot review on turret lifecycle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three surgical fixes from PR review: 1. resetObjectiveTurret: skip the final move_to_objective call when no objective has been selected yet (current_objective is None), instead of raising and popping an error dialog. 2. Shutdown/restart path: close the turret unconditionally — release the serial port so a restart can acquire it, and de-energize the motor on full shutdown regardless of whether Z-retract succeeded. Xeryon's zero-before-reinit path stays gated on z_retracted. 3. prepare_for_use: skip turret re-home on software restart. The motor stays powered across close()/re-init and retains its position register, so re-homing is wasted motion. Xeryon still re-homes (its findIndex is required and fast). Co-Authored-By: Claude Opus 4.7 (1M context) --- software/control/gui_hcs.py | 31 ++++++++++++++++++++----------- software/control/microscope.py | 6 +++++- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/software/control/gui_hcs.py b/software/control/gui_hcs.py index 52df6a497..2caa562ca 100644 --- a/software/control/gui_hcs.py +++ b/software/control/gui_hcs.py @@ -1936,7 +1936,9 @@ def resetObjectiveTurret(self): # Re-home so the position tracker matches the physical slot: avoids short-circuiting the # rotate in move_to_objective if the tracker is stale from a fault mid-rotation. self.objective_changer.home() - self.objective_changer.move_to_objective(self.objectiveStore.current_objective) + current = self.objectiveStore.current_objective + if current: + self.objective_changer.move_to_objective(current) except Exception as exc: self.log.exception("Reset of objective turret failed") QMessageBox.warning( @@ -2725,10 +2727,8 @@ def _cleanup_common(self, for_restart: bool = False): else: raise - # Retract Z and tear down the objective changer on full shutdown. - # On restart, this runs only for Xeryon (which must be zeroed before re-init); - # the turret is skipped because it re-inits cleanly from any position. - # Xeryon gets moveToZero(); turret gets close() (disables motor + disconnects serial). + # Z-retract + Xeryon zero: needed on full shutdown, and on Xeryon restart + # (Xeryon must be zeroed before re-init). if not for_restart or USE_XERYON: z_retracted = False try: @@ -2740,18 +2740,27 @@ def _cleanup_common(self, for_restart: bool = False): else: raise - if self.objective_changer and z_retracted: + if USE_XERYON and self.objective_changer and z_retracted: try: - if USE_XERYON: - self.objective_changer.moveToZero() - elif USE_OBJECTIVE_TURRET: - self.objective_changer.close() + self.objective_changer.moveToZero() except Exception: if for_restart: - self.log.exception(f"Error resetting objective changer during {context}") + self.log.exception(f"Error resetting Xeryon during {context}") else: raise + # Turret close: always release the serial port so the new process (on restart) + # can acquire it, and so the motor is de-energized on full shutdown. Independent + # of Z-retract success — the close path must run even if Z retract failed. + if USE_OBJECTIVE_TURRET and self.objective_changer: + try: + self.objective_changer.close() + except Exception: + if for_restart: + self.log.exception(f"Error closing turret during {context}") + else: + raise + if not for_restart: self.microcontroller.turn_off_all_pid() diff --git a/software/control/microscope.py b/software/control/microscope.py index 94cc8802a..1054d2133 100644 --- a/software/control/microscope.py +++ b/software/control/microscope.py @@ -491,7 +491,11 @@ def _prepare_for_use(self, skip_init: bool = False): raise if self.addons.objective_changer: - self.addons.objective_changer.home() + # Xeryon always re-homes (findIndex is fast and required). The turret skips + # homing on a software restart: the motor stays powered across close()/re-init + # and retains its position register, so a re-home would just be wasted motion. + if control._def.USE_XERYON or not skip_init: + self.addons.objective_changer.home() if control._def.USE_XERYON: self.addons.objective_changer.setSpeed(control._def.XERYON_SPEED) self.addons.objective_changer.move_to_objective(control._def.DEFAULT_OBJECTIVE) From fca3afdeac1802122e92823c504515b018b8ce30 Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 23 Apr 2026 22:49:35 -0700 Subject: [PATCH 19/33] fix: wait for RUNNING assertion in _wait_until_idle The old implementation had a fixed 50 ms grace sleep and then returned on the first poll that reported RUNNING=0. If the motor hadn't yet asserted RUNNING within 50 ms (homing-method-17 to a limit switch can take longer to spin up), _wait_until_idle would return early and the next Modbus write in move_to_objective would hit SLAVE_BUSY (FC=0x86, code=0x06) because the home was still executing internally. Mirror the seen_running pattern that _wait_for_position already uses: require RUNNING to go high at least once before accepting idle, with a bounded grace period so trivially-short or no-op commands still return. Co-Authored-By: Claude Opus 4.7 (1M context) --- software/control/objective_turret_controller.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/software/control/objective_turret_controller.py b/software/control/objective_turret_controller.py index cc2300eca..04bf1fd29 100644 --- a/software/control/objective_turret_controller.py +++ b/software/control/objective_turret_controller.py @@ -414,19 +414,27 @@ def _check_fault(status_word: int) -> None: raise RuntimeError(f"Motor reported fault (status word=0x{status_word:04X})") def _wait_until_idle(self, timeout_s: float) -> None: + # Require RUNNING to go high at least once before accepting not-running as "idle". + # Without this, a slow command spin-up (the motor hasn't asserted RUNNING yet when we + # first poll) causes an early return, and the next Modbus write hits SLAVE_BUSY because + # the motor is still processing the previous command. The grace period falls back to + # accepting not-running for trivially-short moves or no-op commands. deadline = time.monotonic() + timeout_s - time.sleep(POLL_INTERVAL_S) + grace_deadline = time.monotonic() + POLL_INTERVAL_S * 10 + seen_running = False while time.monotonic() < deadline: status = self._read_status_word() self._check_fault(status) - if not (status & STATUS_BIT_RUNNING): + running = bool(status & STATUS_BIT_RUNNING) + if running: + seen_running = True + elif seen_running or time.monotonic() >= grace_deadline: return time.sleep(POLL_INTERVAL_S) raise TimeoutError(f"Motion did not finish within {timeout_s:.1f}s") def _wait_for_position(self, target_pulses: int, timeout_s: float) -> None: # No leading sleep: seen_running prevents stall detection before the motor asserts RUNNING. - # (Contrast _wait_until_idle which has a leading sleep because it lacks a seen_running guard.) deadline = time.monotonic() + timeout_s seen_running = False last_pos: Optional[int] = None From c7a3f8b5768b6434a114151a6dd6bdaa2e15896a Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 23 Apr 2026 22:58:46 -0700 Subject: [PATCH 20/33] fix: retry on Modbus slave-busy responses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The NiMotion vendor FAQ confirms a known behavior: writing a parameter or switching run modes while the motor is still in the Enabled state returns a Modbus SLAVE_DEVICE_BUSY (exception code 0x06). The same applies to ACKNOWLEDGE (0x05). Our code already issues CW_DISABLE before the mode switch in _rotate_to, but there's a processing delay between the disable landing and the motor actually transitioning out of Enabled, and writes during that window get busy-rejected. Handle this at the Modbus transport layer: when we see an exception response with code 0x05 or 0x06, back off with exponential delay and retry. These are documented "try again later" responses per the Modbus spec; they don't indicate a failure and shouldn't consume the normal retry budget. Cap at 20 transient retries total to avoid an infinite loop if the motor is permanently stuck. Also revert the seen_running change in _wait_until_idle — retry-on-busy handles the same underlying timing race, and keeping two solutions for one problem just obscures which one is load-bearing. Co-Authored-By: Claude Opus 4.7 (1M context) --- software/control/modbus_rtu.py | 30 ++++++++++++++++++- .../control/objective_turret_controller.py | 13 ++------ 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/software/control/modbus_rtu.py b/software/control/modbus_rtu.py index 9aa610216..f9cfb09a9 100644 --- a/software/control/modbus_rtu.py +++ b/software/control/modbus_rtu.py @@ -272,6 +272,13 @@ FRAME_INTERVAL = 0.003 +# Modbus exception codes treated as transient — "slave is busy, ask again later". +# Per Modbus spec: 0x05 = ACKNOWLEDGE (slave accepted the request, still processing), +# 0x06 = SLAVE_DEVICE_BUSY (slave is engaged in a long-duration command). +TRANSIENT_EXCEPTION_CODES = {0x05, 0x06} +TRANSIENT_RETRIES = 20 +TRANSIENT_BASE_DELAY_S = 0.1 + def calculate_crc(data: bytes | bytearray) -> int: crc = 0xFFFF @@ -407,7 +414,9 @@ def write_register_32bit(self, slave_id: int, address: int, value: int, signed: def _send_receive(self, frame: bytes, expected_response_len: int) -> bytes: with self._lock: last_error: Optional[Exception] = None - for attempt in range(self._retries + 1): + transient_attempts = 0 + attempt = 0 + while attempt <= self._retries: try: self._serial.reset_input_buffer() self._serial.write(frame) @@ -418,11 +427,28 @@ def _send_receive(self, frame: bytes, expected_response_len: int) -> bytes: logger.warning(f"Modbus request failed (attempt {attempt + 1}/" f"{self._retries + 1}): {e}") if attempt < self._retries: time.sleep(FRAME_INTERVAL * 2) + attempt += 1 continue # Exception responses are 5 bytes — check before incomplete check if len(response) >= 5 and (response[1] & 0x80) and _verify_crc(response[:5]): exception_code = response[2] + # Transient "slave busy / acknowledge" responses: back off and retry per + # Modbus spec. Doesn't consume a normal retry slot — the slave is working, + # not failing. Capped at TRANSIENT_RETRIES total. + if exception_code in TRANSIENT_EXCEPTION_CODES and transient_attempts < TRANSIENT_RETRIES: + backoff = TRANSIENT_BASE_DELAY_S * (2 ** min(transient_attempts, 4)) + logger.debug( + "Modbus slave busy (FC=0x%02X, code=0x%02X); retry %d/%d after %.2fs", + response[1], + exception_code, + transient_attempts + 1, + TRANSIENT_RETRIES, + backoff, + ) + transient_attempts += 1 + time.sleep(backoff) + continue # redrive without incrementing `attempt` raise ModbusError( f"Modbus exception response: FC=0x{response[1]:02X}, " f"code=0x{exception_code:02X}", slave_id=response[0], @@ -438,6 +464,7 @@ def _send_receive(self, frame: bytes, expected_response_len: int) -> bytes: ) if attempt < self._retries: time.sleep(FRAME_INTERVAL * 2) + attempt += 1 continue if not _verify_crc(response): @@ -447,6 +474,7 @@ def _send_receive(self, frame: bytes, expected_response_len: int) -> bytes: ) if attempt < self._retries: time.sleep(FRAME_INTERVAL * 2) + attempt += 1 continue return response diff --git a/software/control/objective_turret_controller.py b/software/control/objective_turret_controller.py index 04bf1fd29..e306b09ea 100644 --- a/software/control/objective_turret_controller.py +++ b/software/control/objective_turret_controller.py @@ -414,21 +414,12 @@ def _check_fault(status_word: int) -> None: raise RuntimeError(f"Motor reported fault (status word=0x{status_word:04X})") def _wait_until_idle(self, timeout_s: float) -> None: - # Require RUNNING to go high at least once before accepting not-running as "idle". - # Without this, a slow command spin-up (the motor hasn't asserted RUNNING yet when we - # first poll) causes an early return, and the next Modbus write hits SLAVE_BUSY because - # the motor is still processing the previous command. The grace period falls back to - # accepting not-running for trivially-short moves or no-op commands. deadline = time.monotonic() + timeout_s - grace_deadline = time.monotonic() + POLL_INTERVAL_S * 10 - seen_running = False + time.sleep(POLL_INTERVAL_S) while time.monotonic() < deadline: status = self._read_status_word() self._check_fault(status) - running = bool(status & STATUS_BIT_RUNNING) - if running: - seen_running = True - elif seen_running or time.monotonic() >= grace_deadline: + if not (status & STATUS_BIT_RUNNING): return time.sleep(POLL_INTERVAL_S) raise TimeoutError(f"Motion did not finish within {timeout_s:.1f}s") From 02855ba038efcaed98d9cf19631460daa9a5430d Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 23 Apr 2026 23:12:01 -0700 Subject: [PATCH 21/33] fix: use FC 0x04 (read input) for NiMotion input registers Per the NiMotion register map (and confirmed against the SingleMotor reference driver), these registers live in the *input*-register address space, not the holding-register space: - 0x001A (microstep) - 0x001F (motion status word) - 0x0021 (current display position) Our Modbus client only supported FC 0x03 (read holding), so reads of these addresses were returning unrelated holding-register values at the same numeric address (e.g. holding 0x001F is "drive parameters", not the status word). This is why _wait_for_position saw impossible values like 786434 pulses and timed out waiting for a position that would never be reported. Add read_input_register and read_input_register_32bit (FC 0x04) to the Modbus client, and use them in the turret controller for the three input-register reads. Co-Authored-By: Claude Opus 4.7 (1M context) --- software/control/modbus_rtu.py | 31 +++++++++++++++++++ .../control/objective_turret_controller.py | 6 ++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/software/control/modbus_rtu.py b/software/control/modbus_rtu.py index f9cfb09a9..c19fcf802 100644 --- a/software/control/modbus_rtu.py +++ b/software/control/modbus_rtu.py @@ -305,6 +305,11 @@ def build_read_registers_frame(slave_id: int, address: int, count: int) -> bytes return _append_crc(frame) +def build_read_input_registers_frame(slave_id: int, address: int, count: int) -> bytes: + frame = struct.pack(">BBHH", slave_id, 0x04, address, count) + return _append_crc(frame) + + def build_write_register_frame(slave_id: int, address: int, value: int) -> bytes: frame = struct.pack(">BBHH", slave_id, 0x06, address, value) return _append_crc(frame) @@ -395,6 +400,32 @@ def read_register_32bit(self, slave_id: int, address: int, signed: bool = False) value -= 0x100000000 return value + def read_input_register(self, slave_id: int, address: int) -> int: + """Read a single 16-bit input register (FC 0x04). + + Input registers are a distinct address space from holding registers — the + same numeric address may refer to different data depending on FC. Some + devices (like the NiMotion stepper) place the status word and current + position in the input-register space, so FC 0x03 would return unrelated + holding-register data. + """ + self._require_connected() + frame = build_read_input_registers_frame(slave_id, address, 1) + response = self._send_receive(frame, expected_response_len=7) + return (response[3] << 8) | response[4] + + def read_input_register_32bit(self, slave_id: int, address: int, signed: bool = False) -> int: + """Read a 32-bit input register pair via FC 0x04 (see read_input_register).""" + self._require_connected() + frame = build_read_input_registers_frame(slave_id, address, 2) + response = self._send_receive(frame, expected_response_len=9) + high = (response[3] << 8) | response[4] + low = (response[5] << 8) | response[6] + value = (high << 16) | low + if signed and value >= 0x80000000: + value -= 0x100000000 + return value + def write_register(self, slave_id: int, address: int, value: int): self._require_connected() frame = build_write_register_frame(slave_id, address, value) diff --git a/software/control/objective_turret_controller.py b/software/control/objective_turret_controller.py index e306b09ea..9719869af 100644 --- a/software/control/objective_turret_controller.py +++ b/software/control/objective_turret_controller.py @@ -217,7 +217,7 @@ def __init__( try: self.clear_alarm() - microstep_raw = self._modbus.read_register(self._slave_id, REG_MICROSTEP) + microstep_raw = self._modbus.read_input_register(self._slave_id, REG_MICROSTEP) if not 0 <= microstep_raw <= 7: raise ValueError(f"Invalid microstep register value {microstep_raw} (expected 0..7)") self._microstep = 2**microstep_raw @@ -296,7 +296,7 @@ def pulses_per_position(self) -> int: @property def current_position_pulses(self) -> int: - return self._modbus.read_register_32bit(self._slave_id, REG_CURRENT_POSITION, signed=True) + return self._modbus.read_input_register_32bit(self._slave_id, REG_CURRENT_POSITION, signed=True) @property def current_objective(self) -> Optional[str]: @@ -406,7 +406,7 @@ def _write_holding(self, address: int, value: int) -> None: self._modbus.write_register(self._slave_id, address, value) def _read_status_word(self) -> int: - return self._modbus.read_register(self._slave_id, REG_STATUS_WORD) + return self._modbus.read_input_register(self._slave_id, REG_STATUS_WORD) @staticmethod def _check_fault(status_word: int) -> None: From 7b277d15d0044c6808bee8b43192e0f8a081e4bb Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 23 Apr 2026 23:16:27 -0700 Subject: [PATCH 22/33] fix: read microstep via FC 0x03 (it's a holding register) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The microstep register (0x001A) is in the holding-register space per the NiMotion register map (SingleMotor's register catalog confirms; there is no input register at 0x001A). My prior commit mistakenly routed this read through FC 0x04, so the device returned garbage (typically 0), which made self._microstep = 2**0 = 1 (full-step mode) and collapsed pulses_per_position to 137 — hence "move to 137 pulses, last position 449" with the motor off-slot. The status word (0x001F) and current position (0x0021) remain on FC 0x04 — those genuinely are input registers. Co-Authored-By: Claude Opus 4.7 (1M context) --- software/control/objective_turret_controller.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/software/control/objective_turret_controller.py b/software/control/objective_turret_controller.py index 9719869af..786076841 100644 --- a/software/control/objective_turret_controller.py +++ b/software/control/objective_turret_controller.py @@ -217,7 +217,7 @@ def __init__( try: self.clear_alarm() - microstep_raw = self._modbus.read_input_register(self._slave_id, REG_MICROSTEP) + microstep_raw = self._modbus.read_register(self._slave_id, REG_MICROSTEP) if not 0 <= microstep_raw <= 7: raise ValueError(f"Invalid microstep register value {microstep_raw} (expected 0..7)") self._microstep = 2**microstep_raw From 012e02a2d00adeaa09289e44d8bfd36736c500af Mon Sep 17 00:00:00 2001 From: You Yan Date: Mon, 18 May 2026 21:01:32 -0700 Subject: [PATCH 23/33] feat: calibrate turret homing search/zero speeds NiMotion defaults for find-switch (0x006C) and find-zero (0x006E) are 100 step/s, making homing ~10x slower than the SingleMotor reference setup. Calibrate both at connect to 1000 and 200 to match SingleMotor. Co-Authored-By: Claude Opus 4.7 (1M context) --- software/control/objective_turret_controller.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/software/control/objective_turret_controller.py b/software/control/objective_turret_controller.py index 786076841..c27ac4254 100644 --- a/software/control/objective_turret_controller.py +++ b/software/control/objective_turret_controller.py @@ -37,6 +37,8 @@ REG_DECEL = 0x0061 REG_HOMING_OFFSET = 0x0069 REG_HOMING_METHOD = 0x006B +REG_HOMING_SEARCH_SPEED = 0x006C +REG_HOMING_ZERO_SPEED = 0x006E REG_ZERO_RETURN = 0x0072 REG_CLEAR_ERROR_STORAGE = 0x0073 @@ -68,6 +70,8 @@ # Homing defaults (auto-calibrated on first connect) HOMING_METHOD = 17 HOMING_ORIGIN_OFFSET = 500 +HOMING_SEARCH_SPEED = 1000 +HOMING_ZERO_SPEED = 200 HOMING_ZERO_RETURN = 1 DI1_FUNCTION_NEG_LIMIT = 1 @@ -384,6 +388,18 @@ def _calibrate_homing_config(self) -> bool: is_32bit=True, signed=True, ), + self._calibrate_one( + REG_HOMING_SEARCH_SPEED, + HOMING_SEARCH_SPEED, + "homing_search_speed", + is_32bit=True, + ), + self._calibrate_one( + REG_HOMING_ZERO_SPEED, + HOMING_ZERO_SPEED, + "homing_zero_speed", + is_32bit=True, + ), self._calibrate_one(REG_ZERO_RETURN, HOMING_ZERO_RETURN, "zero_return"), self._calibrate_one( REG_DI_FUNCTION, From b4e13ce1b1f7c1328946d61ae6c68fd06c809fe0 Mon Sep 17 00:00:00 2001 From: You Yan Date: Mon, 18 May 2026 21:01:37 -0700 Subject: [PATCH 24/33] feat: add NiMotion EEPROM baseline recovery tool Standalone (pyserial only) field-recovery script for restoring a misbehaving NiMotion driver to the SingleMotor-verified baseline captured 2026-05-13. Copied from the SingleMotor repo. Use when a turret driver shows symptoms (alarms, broken homing). After running this, reconnect via ObjectiveTurret4PosController so its _calibrate_* routines re-apply turret-specific overrides on top of the stock baseline. Co-Authored-By: Claude Opus 4.7 (1M context) --- software/tools/write_baseline.py | 312 +++++++++++++++++++++++++++++++ 1 file changed, 312 insertions(+) create mode 100644 software/tools/write_baseline.py diff --git a/software/tools/write_baseline.py b/software/tools/write_baseline.py new file mode 100644 index 000000000..b458265cb --- /dev/null +++ b/software/tools/write_baseline.py @@ -0,0 +1,312 @@ +"""把正常驱动器的参数基线写入设备 EEPROM。 + +独立脚本，只依赖 pyserial（pip install pyserial）。 +不依赖项目内任何模块，可单独拷贝运行。 + +用法： + python3 scripts/write_baseline.py # 实际写入 + python3 scripts/write_baseline.py --dry-run # 只显示差异 + python3 scripts/write_baseline.py --port /dev/ttyUSB0 +""" + +import argparse +import sys +import time + +import serial +import serial.tools.list_ports + + +SLAVE_ID = 1 +BAUDRATE = 115200 + +# Modbus 功能码 +FC_READ_HOLDING = 0x03 +FC_WRITE_SINGLE = 0x06 +FC_WRITE_MULTIPLE = 0x10 + +# 基线参数：来自 2026-05-13 实测的正常驱动器 +# 格式：(地址, 期望值, 名称, 32位?) +# 跳过通讯参数(0x0000-0x0002)避免断连风险；跳过命令型寄存器和不保存的运行参数 +BASELINE = [ + (0x0003, 0, "Modbus 返回等待时间", False), + # 电机电气参数（基线为 0，表示未校准；属正常状态） + (0x000E, 0, "电阻", True), + (0x0010, 0, "电感", True), + (0x0012, 0, "反应电动势系数", True), + (0x0014, 0, "电压", False), + # 电流参数 + (0x0015, 0, "减速电流 (x100mA)", False), + (0x0016, 15, "怠机电流 (1.5A)", False), + (0x0017, 63, "加速电流 (6.3A)", False), + (0x0018, 41, "运行电流 (4.1A)", False), + (0x0019, 8, "过载电流 (0.8A)", False), + # 运动参数 + (0x001A, 4, "细分 (1:16)", False), + (0x001F, 1, "驱动参数（低速优化开）", False), + # I/O 配置 + (0x002C, 1, "DI1=负限位", True), + (0x002E, 0, "DI1 极性不取反", False), + (0x002F, 0, "输入上拉", False), + (0x0030, 69905, "输入触发方式", True), + (0x0034, 0, "I/O端口配置", False), + (0x0035, 0, "故障安全输出", False), + (0x0036, 0, "故障安全预定", False), + (0x0037, 0, "数字量输出", True), + # 运行模式 & 停机 + (0x0039, 1, "运行模式=位置", False), + (0x003A, 1, "操作启停=减速停机", False), + (0x003B, 0, "急停=无减速停机", False), + (0x003C, 0, "故障=无减速停机", False), + # 检测 + (0x0043, 0, "失速检测阈值", True), + # 位置/速度 + (0x0057, 0, "位置最小值", True), + (0x0059, 0, "位置最大值", True), + (0x005B, 60, "最大速度 60 Step/s", True), + (0x005D, 16, "最小速度 16 Step/s", True), + (0x005F, 600, "加速度 600 Step/s²", True), + (0x0061, 600, "减速度 600 Step/s²", True), + # 回零参数 + (0x0069, 0, "原点偏移 0", True), + (0x006B, 17, "回零方式 17", False), + (0x006C, 50, "寻找开关速度 50", True), + (0x006E, 20, "寻找零位速度 20", True), + (0x0072, 0, "零点回归 禁用", False), +] + + +# ── Modbus CRC16 ───────────────────────────────────────────── + + +def crc16(data: bytes) -> int: + """Modbus CRC16 (多项式 0xA001, 初始值 0xFFFF)""" + crc = 0xFFFF + for b in data: + crc ^= b + for _ in range(8): + if crc & 1: + crc = (crc >> 1) ^ 0xA001 + else: + crc >>= 1 + return crc + + +def append_crc(data: bytes) -> bytes: + c = crc16(data) + return data + bytes([c & 0xFF, (c >> 8) & 0xFF]) + + +def verify_crc(frame: bytes) -> bool: + if len(frame) < 4: + return False + return crc16(frame[:-2]) == (frame[-2] | (frame[-1] << 8)) + + +# ── Modbus 帧构建/解析 ─────────────────────────────────────── + + +def build_read(slave: int, addr: int, count: int) -> bytes: + return append_crc( + bytes( + [ + slave, + FC_READ_HOLDING, + (addr >> 8) & 0xFF, + addr & 0xFF, + (count >> 8) & 0xFF, + count & 0xFF, + ] + ) + ) + + +def build_write_single(slave: int, addr: int, value: int) -> bytes: + v = value & 0xFFFF + return append_crc( + bytes( + [ + slave, + FC_WRITE_SINGLE, + (addr >> 8) & 0xFF, + addr & 0xFF, + (v >> 8) & 0xFF, + v & 0xFF, + ] + ) + ) + + +def build_write_multiple(slave: int, addr: int, values: list[int]) -> bytes: + count = len(values) + body = bytes( + [ + slave, + FC_WRITE_MULTIPLE, + (addr >> 8) & 0xFF, + addr & 0xFF, + (count >> 8) & 0xFF, + count & 0xFF, + count * 2, + ] + ) + for v in values: + body += bytes([(v >> 8) & 0xFF, v & 0xFF]) + return append_crc(body) + + +def parse_read_response(raw: bytes) -> tuple[list[int] | None, str | None]: + """读保持寄存器响应解析。返回 (values, error_message)""" + if len(raw) < 5: + return None, "帧长不足" + if not verify_crc(raw): + return None, "CRC 错误" + if raw[1] & 0x80: + return None, f"Modbus 异常码 0x{raw[2]:02X}" + byte_count = raw[2] + if len(raw) < 3 + byte_count + 2: + return None, "数据段不完整" + values = [] + for i in range(0, byte_count, 2): + values.append((raw[3 + i] << 8) | raw[4 + i]) + return values, None + + +def parse_write_response(raw: bytes) -> str | None: + """写响应解析。返回 error_message 或 None""" + if len(raw) < 8: + return "帧长不足" + if not verify_crc(raw): + return "CRC 错误" + if raw[1] & 0x80: + return f"Modbus 异常码 0x{raw[2]:02X}" + return None + + +def combine_32bit(high: int, low: int, signed: bool = False) -> int: + v = (high << 16) | low + if signed and v >= 0x80000000: + v -= 0x100000000 + return v + + +def split_32bit(value: int) -> tuple[int, int]: + if value < 0: + value += 0x100000000 + return (value >> 16) & 0xFFFF, value & 0xFFFF + + +# ── 串口收发 ───────────────────────────────────────────────── + + +def transact(port: serial.Serial, frame: bytes, expected_len: int) -> bytes: + port.reset_input_buffer() + port.write(frame) + time.sleep(0.02) + return port.read(expected_len) + + +def read_holding(port, addr, count): + frame = build_read(SLAVE_ID, addr, count) + expected = 5 + count * 2 + raw = transact(port, frame, expected) + if not raw: + return None, "超时" + return parse_read_response(raw) + + +def write_single(port, addr, value): + frame = build_write_single(SLAVE_ID, addr, value) + raw = transact(port, frame, 8) + if not raw: + return "超时" + return parse_write_response(raw) + + +def write_32bit(port, addr, value): + high, low = split_32bit(value) + frame = build_write_multiple(SLAVE_ID, addr, [high, low]) + raw = transact(port, frame, 8) + if not raw: + return "超时" + return parse_write_response(raw) + + +# ── 主流程 ─────────────────────────────────────────────────── + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument("--port", default=None) + parser.add_argument("--dry-run", action="store_true") + args = parser.parse_args() + + available = [p.device for p in serial.tools.list_ports.comports()] + usb = [p for p in available if "USB" in p or "ACM" in p] + port_name = args.port or (usb[0] if usb else None) + if not port_name: + print(f"无 USB 串口。可见: {available}") + sys.exit(1) + + port = serial.Serial(port=port_name, baudrate=BAUDRATE, timeout=0.5) + print(f"端口: {port_name} 从站: {SLAVE_ID} {'(dry-run)' if args.dry_run else ''}") + print(f"基线参数项数: {len(BASELINE)}\n") + + print("[1] 停机/脱机（保证需脱机参数可写）...") + write_single(port, 0x0051, 0x0000) + time.sleep(0.1) + + print("\n[2] 检查并写入...") + diffs = [] + fails = [] + for addr, expected, name, is_32bit in BASELINE: + count = 2 if is_32bit else 1 + vals, err = read_holding(port, addr, count) + if err: + fails.append((addr, name, f"读取失败: {err}")) + print(f" ✗ 0x{addr:04X} {name:30s}: 读取失败 - {err}") + continue + current = combine_32bit(vals[0], vals[1], signed=True) if is_32bit else vals[0] + + if current == expected: + print(f" ✓ 0x{addr:04X} {name:30s}: {current} (一致)") + continue + + diffs.append((addr, current, expected, name)) + if args.dry_run: + print(f" ! 0x{addr:04X} {name:30s}: {current} → {expected}") + continue + + werr = write_32bit(port, addr, expected) if is_32bit else write_single(port, addr, expected) + time.sleep(0.05) + if werr: + fails.append((addr, name, f"写入失败: {werr}")) + print(f" ✗ 0x{addr:04X} {name:30s}: {current} → {expected} 失败 - {werr}") + else: + print(f" ✓ 0x{addr:04X} {name:30s}: {current} → {expected} 已写") + + if diffs and not args.dry_run: + print("\n[3] 保存到 EEPROM (0x0008 = 0x7376)...") + err = write_single(port, 0x0008, 0x7376) + print(f" ✓ 已保存" if not err else f" ✗ 保存失败: {err}") + time.sleep(0.5) + elif not diffs: + print("\n[3] 全部一致") + else: + print("\n[3] dry-run 模式，未保存") + + print("\n" + "=" * 60) + if not diffs and not fails: + print(f"✓ 全部 {len(BASELINE)} 项已匹配基线") + else: + print(f"修改 {len(diffs)} 项, 失败 {len(fails)} 项") + if fails: + print("\n失败项:") + for addr, name, msg in fails: + print(f" 0x{addr:04X} {name}: {msg}") + + port.close() + + +if __name__ == "__main__": + main() From 203e9cdca82f83dabfef13e05d9d1b31faf9034b Mon Sep 17 00:00:00 2001 From: You Yan Date: Mon, 18 May 2026 21:05:54 -0700 Subject: [PATCH 25/33] feat: short-circuit turret move on target-position equality When two objective names map to the same physical position (a realistic config, e.g. lens swap aliases), the previous early-out only checked name equality and would trigger a full Z-retract, no-op rotate, and Z-restore cycle on every alias switch. Compare current vs target position instead. The tracked objective name is still updated so the UI/state reflects the user's choice; the physical Z dance is skipped. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../control/objective_turret_controller.py | 12 +++++----- .../test_objective_turret_controller.py | 22 +++++++++++++++++++ 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/software/control/objective_turret_controller.py b/software/control/objective_turret_controller.py index c27ac4254..eed67efd5 100644 --- a/software/control/objective_turret_controller.py +++ b/software/control/objective_turret_controller.py @@ -138,8 +138,9 @@ def enable(self) -> None: def move_to_objective(self, objective_name: str, timeout_s: float = DEFAULT_MOVE_TIMEOUT_S) -> None: self._require_open() - _resolve_position(objective_name, self._positions) - if self._current_objective == objective_name: + target_position = _resolve_position(objective_name, self._positions) + if self._current_objective is not None and self._positions[self._current_objective] == target_position: + self._current_objective = objective_name return captured_z = self._retract_z_if_possible() @@ -149,7 +150,7 @@ def move_to_objective(self, objective_name: str, timeout_s: float = DEFAULT_MOVE logger.info( "Simulated turret moved to %s (position %d)", objective_name, - self._positions[objective_name], + target_position, ) def clear_alarm(self) -> None: @@ -264,8 +265,9 @@ def enable(self) -> None: def move_to_objective(self, objective_name: str, timeout_s: float = DEFAULT_MOVE_TIMEOUT_S) -> None: self._require_open() - _resolve_position(objective_name, self._positions) - if self._current_objective == objective_name: + target_position = _resolve_position(objective_name, self._positions) + if self._current_objective is not None and self._positions[self._current_objective] == target_position: + self._current_objective = objective_name return captured_z = self._retract_z_if_possible() diff --git a/software/tests/control/test_objective_turret_controller.py b/software/tests/control/test_objective_turret_controller.py index ce15ce4ea..a6647a2df 100644 --- a/software/tests/control/test_objective_turret_controller.py +++ b/software/tests/control/test_objective_turret_controller.py @@ -145,3 +145,25 @@ def test_move_to_objective_skips_z_retract_when_homing_z_disabled(monkeypatch): assert stage.z_moves == [] # retract is gated on HOMING_ENABLED_Z assert sim.current_objective == "10x" sim.close() + + +def test_move_between_aliased_objectives_skips_z_retract(monkeypatch): + monkeypatch.setattr(control._def, "HOMING_ENABLED_Z", True) + stage = FakeStage(z_mm=3.5) + sim = ObjectiveTurret4PosControllerSimulation( + serial_number="SIM-001", + positions={"4x_A": 1, "4x_B": 1, "10x": 2}, + stage=stage, + ) + + sim.move_to_objective("4x_A") + assert stage.z_moves == [OBJECTIVE_RETRACTED_POS_MM, 3.5] + stage.z_moves.clear() + + # Switching to a different name that maps to the same physical + # position updates the tracked objective but skips the Z dance. + sim.move_to_objective("4x_B") + assert stage.z_moves == [] + assert sim.current_objective == "4x_B" + + sim.close() From bc301b9d238699b8114ee849ddeb3e7a840a6c6e Mon Sep 17 00:00:00 2001 From: You Yan Date: Mon, 18 May 2026 21:14:11 -0700 Subject: [PATCH 26/33] refactor: extract _is_alias_for_current helper Two near-duplicate alias-detection conditionals (sim + real controller) became one named helper. Also fix write_baseline.py docstring usage path now that the script lives at software/tools/ instead of upstream's scripts/. Co-Authored-By: Claude Opus 4.7 (1M context) --- software/control/objective_turret_controller.py | 14 ++++++++++---- software/tools/write_baseline.py | 8 ++++---- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/software/control/objective_turret_controller.py b/software/control/objective_turret_controller.py index eed67efd5..f2f18f7e6 100644 --- a/software/control/objective_turret_controller.py +++ b/software/control/objective_turret_controller.py @@ -88,6 +88,13 @@ def _resolve_position(objective_name: str, positions: dict) -> int: raise KeyError(f"Unknown objective '{objective_name}'. Valid names: {sorted(positions)}") from None +def _is_alias_for_current(current: Optional[str], target_name: str, positions: dict) -> bool: + """True if `target_name` maps to the same physical slot as `current` under a different name.""" + if current is None: + return False + return _resolve_position(current, positions) == _resolve_position(target_name, positions) + + def _find_port(serial_number: str) -> str: matches = [p.device for p in list_ports.comports() if p.serial_number == serial_number] if not matches: @@ -138,10 +145,10 @@ def enable(self) -> None: def move_to_objective(self, objective_name: str, timeout_s: float = DEFAULT_MOVE_TIMEOUT_S) -> None: self._require_open() - target_position = _resolve_position(objective_name, self._positions) - if self._current_objective is not None and self._positions[self._current_objective] == target_position: + if _is_alias_for_current(self._current_objective, objective_name, self._positions): self._current_objective = objective_name return + target_position = _resolve_position(objective_name, self._positions) captured_z = self._retract_z_if_possible() self._current_objective = objective_name @@ -265,8 +272,7 @@ def enable(self) -> None: def move_to_objective(self, objective_name: str, timeout_s: float = DEFAULT_MOVE_TIMEOUT_S) -> None: self._require_open() - target_position = _resolve_position(objective_name, self._positions) - if self._current_objective is not None and self._positions[self._current_objective] == target_position: + if _is_alias_for_current(self._current_objective, objective_name, self._positions): self._current_objective = objective_name return diff --git a/software/tools/write_baseline.py b/software/tools/write_baseline.py index b458265cb..8af928f4e 100644 --- a/software/tools/write_baseline.py +++ b/software/tools/write_baseline.py @@ -3,10 +3,10 @@ 独立脚本，只依赖 pyserial（pip install pyserial）。 不依赖项目内任何模块，可单独拷贝运行。 -用法： - python3 scripts/write_baseline.py # 实际写入 - python3 scripts/write_baseline.py --dry-run # 只显示差异 - python3 scripts/write_baseline.py --port /dev/ttyUSB0 +用法（从 software/ 目录运行）： + python3 tools/write_baseline.py # 实际写入 + python3 tools/write_baseline.py --dry-run # 只显示差异 + python3 tools/write_baseline.py --port /dev/ttyUSB0 """ import argparse From 069bbe86f86ca7331724ce065b5f81f3a7864eac Mon Sep 17 00:00:00 2001 From: You Yan Date: Mon, 18 May 2026 21:28:38 -0700 Subject: [PATCH 27/33] fix: disable low-speed opt and force motor offline before calibration Two issues seen on real hardware: 1. Slave-busy on parameter writes. clear_alarm() only sends CW_CLEAR_FAULT, which is a no-op when the previous session crashed and left the motor in OPERATION_ENABLED. Writing CW_DISABLE explicitly (with a 0.1s settle, mirroring SingleMotor's setup scripts) ensures the device sits in SWITCH_ON_DISABLED before parameter writes that require it. 2. Positions 2/3/4 stalled short of target. Factory default for 0x001F (low-speed optimization) is 1, which splits position moves into slow/fast segments using min_speed (0x005D, default 16 step/s) near the endpoints. On longer travels the slow approach exceeds the 10s move timeout. SingleMotor ships a disable_low_speed_opt.py script for the same reason. Calibrate 0x001F to 0 alongside the other motion params. Co-Authored-By: Claude Opus 4.7 (1M context) --- software/control/objective_turret_controller.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/software/control/objective_turret_controller.py b/software/control/objective_turret_controller.py index f2f18f7e6..e2dcf01ba 100644 --- a/software/control/objective_turret_controller.py +++ b/software/control/objective_turret_controller.py @@ -27,6 +27,7 @@ REG_SAVE_PARAMS = 0x0008 REG_DI_FUNCTION = 0x002C REG_MICROSTEP = 0x001A +REG_LOW_SPEED_OPT = 0x001F # holding-register view; same address as status word on input side REG_STATUS_WORD = 0x001F REG_CURRENT_POSITION = 0x0021 REG_RUN_MODE = 0x0039 @@ -66,6 +67,10 @@ EXPECTED_ACCEL = 200 EXPECTED_DECEL = 200 EXPECTED_MAX_SPEED = 250 +# Disable low-speed optimization. Factory default is 1 (enabled), which splits +# position moves into slow/fast segments and causes missed steps on longer travels +# — visible as positions 2/3/4 landing off-target while position 1 (zero pulses) works. +LOW_SPEED_OPT_DISABLED = 0 # Homing defaults (auto-calibrated on first connect) HOMING_METHOD = 17 @@ -80,6 +85,9 @@ DEFAULT_MOVE_TIMEOUT_S = 10.0 DEFAULT_HOME_TIMEOUT_S = 30.0 +# Settle time after a control-word transition before the next write. +CONTROL_WORD_SETTLE_S = 0.1 + def _resolve_position(objective_name: str, positions: dict) -> int: try: @@ -228,6 +236,11 @@ def __init__( self._modbus.connect() try: self.clear_alarm() + # Some parameter registers (notably homing config) reject writes while the + # motor is in OPERATION_ENABLED — which can persist across crashed sessions + # where close() never ran. Force the device into SWITCH_ON_DISABLED first. + self._write_control(CW_DISABLE) + time.sleep(CONTROL_WORD_SETTLE_S) microstep_raw = self._modbus.read_register(self._slave_id, REG_MICROSTEP) if not 0 <= microstep_raw <= 7: @@ -382,6 +395,7 @@ def _calibrate_motion_params(self) -> bool: self._calibrate_one(REG_ACCEL, EXPECTED_ACCEL, "accel", is_32bit=True), self._calibrate_one(REG_DECEL, EXPECTED_DECEL, "decel", is_32bit=True), self._calibrate_one(REG_MAX_SPEED, EXPECTED_MAX_SPEED, "max_speed", is_32bit=True), + self._calibrate_one(REG_LOW_SPEED_OPT, LOW_SPEED_OPT_DISABLED, "low_speed_opt"), ] ) From f314ebfd02941e1a4df44ee1f332685dd232c21a Mon Sep 17 00:00:00 2001 From: You Yan Date: Mon, 18 May 2026 23:58:55 -0700 Subject: [PATCH 28/33] fix: turret motor params + homing reset + log routing Several fixes from real-hardware testing on the 4-position objective turret: - accel/decel kept at 200 (SingleMotor's working config). Sharper ramps (e.g. 600) cause per-move step loss of ~500 pulses; 200 is the only reliable value for this load. - Add min_speed=150 calibration. NiMotion ramps cruise -> min_speed at the end of position moves and continues at min_speed for the final approach. Default 16 step/s makes that segment audibly slow and many seconds long; 150 keeps it brief without losing decel range. - Add drive_param=0 calibration (0x001F). Manual value for "low-speed optimization disabled". Empirically a no-op on this firmware, kept for explicit known state. - HOMING_OFFSET=0 calibrated explicitly. The slot 1 == limit setup needs the post-homing counter at 0; the device's value can drift if any other tool has written to it. - set_zero (0x0047 = 0x535A) after homing motion. Without it the counter can sit at limit + decel_overshoot. - DEFAULT_MOVE_TIMEOUT_S bumped 10 -> 30. At accel=200 a slot 1 -> 4 move takes ~25s; SingleMotor's UI uses 30s for the same reason. - Pre-move and post-move (start/target/actual) plus post-home (pre/post set_zero) logging for diagnosis. Per-register read logging in _calibrate_one so every connect shows the on-device values. - Switch logger to squid.logging.get_logger so our INFO lines actually reach the squid stream + file handlers (logging.getLogger(__name__) was attaching outside the squid root). - Force motor into SWITCH_ON_DISABLED before calibration via explicit CW_DISABLE write + 0.1s settle. Avoids slave-busy responses when a previous session left the motor in OPERATION_ENABLED. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../control/objective_turret_controller.py | 66 +++++++++++++++---- 1 file changed, 54 insertions(+), 12 deletions(-) diff --git a/software/control/objective_turret_controller.py b/software/control/objective_turret_controller.py index e2dcf01ba..5a525a0a4 100644 --- a/software/control/objective_turret_controller.py +++ b/software/control/objective_turret_controller.py @@ -6,7 +6,6 @@ from __future__ import annotations -import logging import time from typing import Optional @@ -14,8 +13,9 @@ from control.modbus_rtu import ModbusRTUClient import squid.abc +import squid.logging -logger = logging.getLogger(__name__) +logger = squid.logging.get_logger("objective_turret_controller") # Turret mechanics GEAR_RATIO = 132 / 48 @@ -34,6 +34,7 @@ REG_CONTROL_WORD = 0x0051 REG_TARGET_POSITION = 0x0053 REG_MAX_SPEED = 0x005B +REG_MIN_SPEED = 0x005D REG_ACCEL = 0x005F REG_DECEL = 0x0061 REG_HOMING_OFFSET = 0x0069 @@ -42,6 +43,7 @@ REG_HOMING_ZERO_SPEED = 0x006E REG_ZERO_RETURN = 0x0072 REG_CLEAR_ERROR_STORAGE = 0x0073 +REG_SET_ZERO = 0x0047 # Control word values CW_DISABLE = 0x0000 @@ -54,6 +56,7 @@ # Magic values SAVE_PARAMS_MAGIC = 0x7376 CLEAR_ERROR_STORAGE_MAGIC = 0x6C64 +SET_ZERO_MAGIC = 0x535A # Run modes MODE_POSITION = 1 @@ -63,18 +66,32 @@ STATUS_BIT_FAULT = 1 << 3 STATUS_BIT_RUNNING = 1 << 12 -# Motion parameter defaults (auto-calibrated on first connect) +# Motion parameter defaults (auto-calibrated on first connect). +# Matches SingleMotor's check_init_params. Sharper accel/decel (e.g. baseline's +# 600/600) cause step loss; 200/200 is the known-good turret config. EXPECTED_ACCEL = 200 EXPECTED_DECEL = 200 EXPECTED_MAX_SPEED = 250 -# Disable low-speed optimization. Factory default is 1 (enabled), which splits -# position moves into slow/fast segments and causes missed steps on longer travels -# — visible as positions 2/3/4 landing off-target while position 1 (zero pulses) works. -LOW_SPEED_OPT_DISABLED = 0 +# NiMotion ramps cruise → min_speed at the end of position moves, then continues at +# min_speed for the final approach pulses. Default 16 step/s makes that segment +# very slow and audibly noisy. Higher values compress the slow tail; 150 ≈ 60% of +# max_speed leaves enough decel range to avoid step loss. +EXPECTED_MIN_SPEED = 150 +# 0x001F low-speed optimization. Manual: 0 = disabled, 1 = enabled. Empirically both +# values produce identical motion on this firmware (the slow tail is min_speed +# crawl, not LSO). Keeping the calibration to put the device in the manual's +# documented "disabled" state explicitly. +DRIVE_PARAM = 0 # Homing defaults (auto-calibrated on first connect) HOMING_METHOD = 17 -HOMING_ORIGIN_OFFSET = 500 +# Counter value at the switch position after homing. Slot 1 sits AT the limit on +# this turret, so we want counter=0 at the switch. SingleMotor's init_turret_params +# sets this to 500 for a turret where slot 1 is offset from the limit — that doesn't +# match our hardware, so we override to 0 explicitly. Forcing the value matters even +# though baseline default is 0, because anything that ran SingleMotor's setup scripts +# (or any other tool) may have left it at 500. +HOMING_ORIGIN_OFFSET = 0 HOMING_SEARCH_SPEED = 1000 HOMING_ZERO_SPEED = 200 HOMING_ZERO_RETURN = 1 @@ -82,7 +99,9 @@ # Polling POLL_INTERVAL_S = 0.05 -DEFAULT_MOVE_TIMEOUT_S = 10.0 +# At accel=200/max_speed=250, slot 1 → slot 4 (~6600 pulses) takes ~25s. 30s +# matches SingleMotor's UI timeout (_MOVING_TIMEOUT_MS in turret_panel.py). +DEFAULT_MOVE_TIMEOUT_S = 30.0 DEFAULT_HOME_TIMEOUT_S = 30.0 # Settle time after a control-word transition before the next write. @@ -275,7 +294,15 @@ def home(self, timeout_s: float = DEFAULT_HOME_TIMEOUT_S) -> None: self._write_control(CW_RUN_ABSOLUTE) self._write_control(CW_TRIGGER_ABSOLUTE) self._wait_until_idle(timeout_s) + # Mark the limit position as counter zero so absolute slot targets land at the + # right physical angle. Log before/after so we can confirm the write actually + # resets the counter on this firmware variant. + pre = self.current_position_pulses + self._write_holding(REG_SET_ZERO, SET_ZERO_MAGIC) + time.sleep(0.05) + post = self.current_position_pulses self._current_objective = None + logger.info("Homed: pre_set_zero=%d, post_set_zero=%d", pre, post) def enable(self) -> None: """Run the disable -> startup -> enable state-machine cycle.""" @@ -341,6 +368,13 @@ def _rotate_to(self, objective_name: str, timeout_s: float) -> None: position_index = _resolve_position(objective_name, self._positions) target_pulses = (position_index - 1) * self._pulses_per_position + logger.info( + "Rotating to %s: start=%d, target=%d", + objective_name, + self.current_position_pulses, + target_pulses, + ) + self._write_control(CW_DISABLE) self._write_holding(REG_RUN_MODE, MODE_POSITION) self._modbus.write_register_32bit(self._slave_id, REG_TARGET_POSITION, target_pulses, signed=True) @@ -349,6 +383,12 @@ def _rotate_to(self, objective_name: str, timeout_s: float) -> None: self._write_control(CW_RUN_ABSOLUTE) self._write_control(CW_TRIGGER_ABSOLUTE) self._wait_for_position(target_pulses, timeout_s) + logger.info( + "Rotated to %s: target=%d, actual=%d", + objective_name, + target_pulses, + self.current_position_pulses, + ) def _retract_z_if_possible(self) -> Optional[float]: from control._def import HOMING_ENABLED_Z, OBJECTIVE_RETRACTED_POS_MM @@ -379,14 +419,15 @@ def _calibrate_one( else: current = self._modbus.read_register(self._slave_id, addr) desired = (current & ~mask) | expected if mask is not None else expected + fmt = "0x%08X" if mask is not None else "%d" if current == desired: + logger.info(f"%s @ 0x%04X: device={fmt} matches desired (no write)", label, addr, current) return False if is_32bit: self._modbus.write_register_32bit(self._slave_id, addr, desired, signed=signed) else: self._modbus.write_register(self._slave_id, addr, desired) - fmt = "0x%08X" if mask is not None else "%d" - logger.info(f"Calibrated %s: {fmt} -> {fmt}", label, current, desired) + logger.info(f"%s @ 0x%04X: {fmt} -> {fmt} (wrote)", label, addr, current, desired) return True def _calibrate_motion_params(self) -> bool: @@ -395,7 +436,8 @@ def _calibrate_motion_params(self) -> bool: self._calibrate_one(REG_ACCEL, EXPECTED_ACCEL, "accel", is_32bit=True), self._calibrate_one(REG_DECEL, EXPECTED_DECEL, "decel", is_32bit=True), self._calibrate_one(REG_MAX_SPEED, EXPECTED_MAX_SPEED, "max_speed", is_32bit=True), - self._calibrate_one(REG_LOW_SPEED_OPT, LOW_SPEED_OPT_DISABLED, "low_speed_opt"), + self._calibrate_one(REG_MIN_SPEED, EXPECTED_MIN_SPEED, "min_speed", is_32bit=True), + self._calibrate_one(REG_LOW_SPEED_OPT, DRIVE_PARAM, "drive_param"), ] ) From 4026e22a8ec3d7b72c4c07ece3e0b083aed1c239 Mon Sep 17 00:00:00 2001 From: You Yan Date: Tue, 19 May 2026 00:08:29 -0700 Subject: [PATCH 29/33] revert: remove tools/write_baseline.py The baseline values it writes (max_speed=60, accel/decel=600, min_speed=16, zero_return=0, etc.) are for a generic stock-driver recovery, not the turret config. Running it on the turret hardware breaks the calibration we depend on and produced the slot-1-stops-at-515 bug we spent hours diagnosing. The risk of accidental execution outweighs the value of having a recovery shortcut in the tree. Co-Authored-By: Claude Opus 4.7 (1M context) --- software/tools/write_baseline.py | 312 ------------------------------- 1 file changed, 312 deletions(-) delete mode 100644 software/tools/write_baseline.py diff --git a/software/tools/write_baseline.py b/software/tools/write_baseline.py deleted file mode 100644 index 8af928f4e..000000000 --- a/software/tools/write_baseline.py +++ /dev/null @@ -1,312 +0,0 @@ -"""把正常驱动器的参数基线写入设备 EEPROM。 - -独立脚本，只依赖 pyserial（pip install pyserial）。 -不依赖项目内任何模块，可单独拷贝运行。 - -用法（从 software/ 目录运行）： - python3 tools/write_baseline.py # 实际写入 - python3 tools/write_baseline.py --dry-run # 只显示差异 - python3 tools/write_baseline.py --port /dev/ttyUSB0 -""" - -import argparse -import sys -import time - -import serial -import serial.tools.list_ports - - -SLAVE_ID = 1 -BAUDRATE = 115200 - -# Modbus 功能码 -FC_READ_HOLDING = 0x03 -FC_WRITE_SINGLE = 0x06 -FC_WRITE_MULTIPLE = 0x10 - -# 基线参数：来自 2026-05-13 实测的正常驱动器 -# 格式：(地址, 期望值, 名称, 32位?) -# 跳过通讯参数(0x0000-0x0002)避免断连风险；跳过命令型寄存器和不保存的运行参数 -BASELINE = [ - (0x0003, 0, "Modbus 返回等待时间", False), - # 电机电气参数（基线为 0，表示未校准；属正常状态） - (0x000E, 0, "电阻", True), - (0x0010, 0, "电感", True), - (0x0012, 0, "反应电动势系数", True), - (0x0014, 0, "电压", False), - # 电流参数 - (0x0015, 0, "减速电流 (x100mA)", False), - (0x0016, 15, "怠机电流 (1.5A)", False), - (0x0017, 63, "加速电流 (6.3A)", False), - (0x0018, 41, "运行电流 (4.1A)", False), - (0x0019, 8, "过载电流 (0.8A)", False), - # 运动参数 - (0x001A, 4, "细分 (1:16)", False), - (0x001F, 1, "驱动参数（低速优化开）", False), - # I/O 配置 - (0x002C, 1, "DI1=负限位", True), - (0x002E, 0, "DI1 极性不取反", False), - (0x002F, 0, "输入上拉", False), - (0x0030, 69905, "输入触发方式", True), - (0x0034, 0, "I/O端口配置", False), - (0x0035, 0, "故障安全输出", False), - (0x0036, 0, "故障安全预定", False), - (0x0037, 0, "数字量输出", True), - # 运行模式 & 停机 - (0x0039, 1, "运行模式=位置", False), - (0x003A, 1, "操作启停=减速停机", False), - (0x003B, 0, "急停=无减速停机", False), - (0x003C, 0, "故障=无减速停机", False), - # 检测 - (0x0043, 0, "失速检测阈值", True), - # 位置/速度 - (0x0057, 0, "位置最小值", True), - (0x0059, 0, "位置最大值", True), - (0x005B, 60, "最大速度 60 Step/s", True), - (0x005D, 16, "最小速度 16 Step/s", True), - (0x005F, 600, "加速度 600 Step/s²", True), - (0x0061, 600, "减速度 600 Step/s²", True), - # 回零参数 - (0x0069, 0, "原点偏移 0", True), - (0x006B, 17, "回零方式 17", False), - (0x006C, 50, "寻找开关速度 50", True), - (0x006E, 20, "寻找零位速度 20", True), - (0x0072, 0, "零点回归 禁用", False), -] - - -# ── Modbus CRC16 ───────────────────────────────────────────── - - -def crc16(data: bytes) -> int: - """Modbus CRC16 (多项式 0xA001, 初始值 0xFFFF)""" - crc = 0xFFFF - for b in data: - crc ^= b - for _ in range(8): - if crc & 1: - crc = (crc >> 1) ^ 0xA001 - else: - crc >>= 1 - return crc - - -def append_crc(data: bytes) -> bytes: - c = crc16(data) - return data + bytes([c & 0xFF, (c >> 8) & 0xFF]) - - -def verify_crc(frame: bytes) -> bool: - if len(frame) < 4: - return False - return crc16(frame[:-2]) == (frame[-2] | (frame[-1] << 8)) - - -# ── Modbus 帧构建/解析 ─────────────────────────────────────── - - -def build_read(slave: int, addr: int, count: int) -> bytes: - return append_crc( - bytes( - [ - slave, - FC_READ_HOLDING, - (addr >> 8) & 0xFF, - addr & 0xFF, - (count >> 8) & 0xFF, - count & 0xFF, - ] - ) - ) - - -def build_write_single(slave: int, addr: int, value: int) -> bytes: - v = value & 0xFFFF - return append_crc( - bytes( - [ - slave, - FC_WRITE_SINGLE, - (addr >> 8) & 0xFF, - addr & 0xFF, - (v >> 8) & 0xFF, - v & 0xFF, - ] - ) - ) - - -def build_write_multiple(slave: int, addr: int, values: list[int]) -> bytes: - count = len(values) - body = bytes( - [ - slave, - FC_WRITE_MULTIPLE, - (addr >> 8) & 0xFF, - addr & 0xFF, - (count >> 8) & 0xFF, - count & 0xFF, - count * 2, - ] - ) - for v in values: - body += bytes([(v >> 8) & 0xFF, v & 0xFF]) - return append_crc(body) - - -def parse_read_response(raw: bytes) -> tuple[list[int] | None, str | None]: - """读保持寄存器响应解析。返回 (values, error_message)""" - if len(raw) < 5: - return None, "帧长不足" - if not verify_crc(raw): - return None, "CRC 错误" - if raw[1] & 0x80: - return None, f"Modbus 异常码 0x{raw[2]:02X}" - byte_count = raw[2] - if len(raw) < 3 + byte_count + 2: - return None, "数据段不完整" - values = [] - for i in range(0, byte_count, 2): - values.append((raw[3 + i] << 8) | raw[4 + i]) - return values, None - - -def parse_write_response(raw: bytes) -> str | None: - """写响应解析。返回 error_message 或 None""" - if len(raw) < 8: - return "帧长不足" - if not verify_crc(raw): - return "CRC 错误" - if raw[1] & 0x80: - return f"Modbus 异常码 0x{raw[2]:02X}" - return None - - -def combine_32bit(high: int, low: int, signed: bool = False) -> int: - v = (high << 16) | low - if signed and v >= 0x80000000: - v -= 0x100000000 - return v - - -def split_32bit(value: int) -> tuple[int, int]: - if value < 0: - value += 0x100000000 - return (value >> 16) & 0xFFFF, value & 0xFFFF - - -# ── 串口收发 ───────────────────────────────────────────────── - - -def transact(port: serial.Serial, frame: bytes, expected_len: int) -> bytes: - port.reset_input_buffer() - port.write(frame) - time.sleep(0.02) - return port.read(expected_len) - - -def read_holding(port, addr, count): - frame = build_read(SLAVE_ID, addr, count) - expected = 5 + count * 2 - raw = transact(port, frame, expected) - if not raw: - return None, "超时" - return parse_read_response(raw) - - -def write_single(port, addr, value): - frame = build_write_single(SLAVE_ID, addr, value) - raw = transact(port, frame, 8) - if not raw: - return "超时" - return parse_write_response(raw) - - -def write_32bit(port, addr, value): - high, low = split_32bit(value) - frame = build_write_multiple(SLAVE_ID, addr, [high, low]) - raw = transact(port, frame, 8) - if not raw: - return "超时" - return parse_write_response(raw) - - -# ── 主流程 ─────────────────────────────────────────────────── - - -def main(): - parser = argparse.ArgumentParser() - parser.add_argument("--port", default=None) - parser.add_argument("--dry-run", action="store_true") - args = parser.parse_args() - - available = [p.device for p in serial.tools.list_ports.comports()] - usb = [p for p in available if "USB" in p or "ACM" in p] - port_name = args.port or (usb[0] if usb else None) - if not port_name: - print(f"无 USB 串口。可见: {available}") - sys.exit(1) - - port = serial.Serial(port=port_name, baudrate=BAUDRATE, timeout=0.5) - print(f"端口: {port_name} 从站: {SLAVE_ID} {'(dry-run)' if args.dry_run else ''}") - print(f"基线参数项数: {len(BASELINE)}\n") - - print("[1] 停机/脱机（保证需脱机参数可写）...") - write_single(port, 0x0051, 0x0000) - time.sleep(0.1) - - print("\n[2] 检查并写入...") - diffs = [] - fails = [] - for addr, expected, name, is_32bit in BASELINE: - count = 2 if is_32bit else 1 - vals, err = read_holding(port, addr, count) - if err: - fails.append((addr, name, f"读取失败: {err}")) - print(f" ✗ 0x{addr:04X} {name:30s}: 读取失败 - {err}") - continue - current = combine_32bit(vals[0], vals[1], signed=True) if is_32bit else vals[0] - - if current == expected: - print(f" ✓ 0x{addr:04X} {name:30s}: {current} (一致)") - continue - - diffs.append((addr, current, expected, name)) - if args.dry_run: - print(f" ! 0x{addr:04X} {name:30s}: {current} → {expected}") - continue - - werr = write_32bit(port, addr, expected) if is_32bit else write_single(port, addr, expected) - time.sleep(0.05) - if werr: - fails.append((addr, name, f"写入失败: {werr}")) - print(f" ✗ 0x{addr:04X} {name:30s}: {current} → {expected} 失败 - {werr}") - else: - print(f" ✓ 0x{addr:04X} {name:30s}: {current} → {expected} 已写") - - if diffs and not args.dry_run: - print("\n[3] 保存到 EEPROM (0x0008 = 0x7376)...") - err = write_single(port, 0x0008, 0x7376) - print(f" ✓ 已保存" if not err else f" ✗ 保存失败: {err}") - time.sleep(0.5) - elif not diffs: - print("\n[3] 全部一致") - else: - print("\n[3] dry-run 模式，未保存") - - print("\n" + "=" * 60) - if not diffs and not fails: - print(f"✓ 全部 {len(BASELINE)} 项已匹配基线") - else: - print(f"修改 {len(diffs)} 项, 失败 {len(fails)} 项") - if fails: - print("\n失败项:") - for addr, name, msg in fails: - print(f" 0x{addr:04X} {name}: {msg}") - - port.close() - - -if __name__ == "__main__": - main() From 2e3e472e53db705fcb46086220fd4212ec915368 Mon Sep 17 00:00:00 2001 From: You Yan Date: Tue, 19 May 2026 00:20:18 -0700 Subject: [PATCH 30/33] refactor: simplify turret calibration and tighten move_to_objective - Table-driven _MOTION_PARAMS / _HOMING_PARAMS lists at module scope so _calibrate_motion_params / _calibrate_homing_config each collapse to a one-liner over the table. ~30 lines net removed. - Logger uses get_logger(__name__) (matches the rest of control/*). - _calibrate_one log formatting unified (pre-format current/desired with fmt once, then plain %-args); no-write line demoted to DEBUG since it fires on every register on steady-state startup. - Trimmed historical reasoning from constant-block comments. - _calibrate_one mask path now applies (expected & mask) so high bits in expected can't stomp outside the masked field (Copilot review). - move_to_objective wraps _rotate_to in try/finally so Z is always restored even if rotate raises; _current_objective only updated on a successful rotate (Copilot review). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../control/objective_turret_controller.py | 109 +++++++----------- 1 file changed, 41 insertions(+), 68 deletions(-) diff --git a/software/control/objective_turret_controller.py b/software/control/objective_turret_controller.py index 5a525a0a4..d2610bb86 100644 --- a/software/control/objective_turret_controller.py +++ b/software/control/objective_turret_controller.py @@ -15,7 +15,7 @@ import squid.abc import squid.logging -logger = squid.logging.get_logger("objective_turret_controller") +logger = squid.logging.get_logger(__name__) # Turret mechanics GEAR_RATIO = 132 / 48 @@ -67,30 +67,23 @@ STATUS_BIT_RUNNING = 1 << 12 # Motion parameter defaults (auto-calibrated on first connect). -# Matches SingleMotor's check_init_params. Sharper accel/decel (e.g. baseline's -# 600/600) cause step loss; 200/200 is the known-good turret config. +# 200/200 accel/decel avoids step loss under the turret's inertial load (sharper +# ramps skip steps mid-move). EXPECTED_ACCEL = 200 EXPECTED_DECEL = 200 EXPECTED_MAX_SPEED = 250 -# NiMotion ramps cruise → min_speed at the end of position moves, then continues at -# min_speed for the final approach pulses. Default 16 step/s makes that segment -# very slow and audibly noisy. Higher values compress the slow tail; 150 ≈ 60% of -# max_speed leaves enough decel range to avoid step loss. +# NiMotion ramps cruise → min_speed at end of position moves, then continues at +# min_speed for final-approach pulses. Default 16 step/s makes that segment audibly +# slow; 150 keeps it brief while leaving decel range. EXPECTED_MIN_SPEED = 150 -# 0x001F low-speed optimization. Manual: 0 = disabled, 1 = enabled. Empirically both -# values produce identical motion on this firmware (the slow tail is min_speed -# crawl, not LSO). Keeping the calibration to put the device in the manual's -# documented "disabled" state explicitly. +# 0x001F low-speed optimization. Manual says 0 = disabled; empirically a no-op on +# this firmware (motion is identical at 0 or 1). Set to 0 for documented-disabled. DRIVE_PARAM = 0 -# Homing defaults (auto-calibrated on first connect) +# Homing defaults (auto-calibrated on first connect). Slot 1 sits at the limit +# on this turret, so counter=0 lands at the switch — HOMING_ORIGIN_OFFSET=0 +# enforces that against any tool that may have written a non-zero offset. HOMING_METHOD = 17 -# Counter value at the switch position after homing. Slot 1 sits AT the limit on -# this turret, so we want counter=0 at the switch. SingleMotor's init_turret_params -# sets this to 500 for a turret where slot 1 is offset from the limit — that doesn't -# match our hardware, so we override to 0 explicitly. Forcing the value matters even -# though baseline default is 0, because anything that ran SingleMotor's setup scripts -# (or any other tool) may have left it at 500. HOMING_ORIGIN_OFFSET = 0 HOMING_SEARCH_SPEED = 1000 HOMING_ZERO_SPEED = 200 @@ -107,6 +100,23 @@ # Settle time after a control-word transition before the next write. CONTROL_WORD_SETTLE_S = 0.1 +# Calibration tables: (register, expected value, label, kwargs-for-_calibrate_one). +_MOTION_PARAMS = [ + (REG_ACCEL, EXPECTED_ACCEL, "accel", {"is_32bit": True}), + (REG_DECEL, EXPECTED_DECEL, "decel", {"is_32bit": True}), + (REG_MAX_SPEED, EXPECTED_MAX_SPEED, "max_speed", {"is_32bit": True}), + (REG_MIN_SPEED, EXPECTED_MIN_SPEED, "min_speed", {"is_32bit": True}), + (REG_LOW_SPEED_OPT, DRIVE_PARAM, "drive_param", {}), +] +_HOMING_PARAMS = [ + (REG_HOMING_METHOD, HOMING_METHOD, "homing_method", {}), + (REG_HOMING_OFFSET, HOMING_ORIGIN_OFFSET, "homing_offset", {"is_32bit": True, "signed": True}), + (REG_HOMING_SEARCH_SPEED, HOMING_SEARCH_SPEED, "homing_search_speed", {"is_32bit": True}), + (REG_HOMING_ZERO_SPEED, HOMING_ZERO_SPEED, "homing_zero_speed", {"is_32bit": True}), + (REG_ZERO_RETURN, HOMING_ZERO_RETURN, "zero_return", {}), + (REG_DI_FUNCTION, DI1_FUNCTION_NEG_LIMIT, "DI1_function", {"is_32bit": True, "mask": 0xF}), +] + def _resolve_position(objective_name: str, positions: dict) -> int: try: @@ -294,9 +304,8 @@ def home(self, timeout_s: float = DEFAULT_HOME_TIMEOUT_S) -> None: self._write_control(CW_RUN_ABSOLUTE) self._write_control(CW_TRIGGER_ABSOLUTE) self._wait_until_idle(timeout_s) - # Mark the limit position as counter zero so absolute slot targets land at the - # right physical angle. Log before/after so we can confirm the write actually - # resets the counter on this firmware variant. + # Reset counter to 0 at the post-homing position so absolute slot targets + # land at the correct physical angle. pre = self.current_position_pulses self._write_holding(REG_SET_ZERO, SET_ZERO_MAGIC) time.sleep(0.05) @@ -317,9 +326,11 @@ def move_to_objective(self, objective_name: str, timeout_s: float = DEFAULT_MOVE return captured_z = self._retract_z_if_possible() - self._rotate_to(objective_name, timeout_s) - self._current_objective = objective_name - self._restore_z_if_captured(captured_z) + try: + self._rotate_to(objective_name, timeout_s) + self._current_objective = objective_name + finally: + self._restore_z_if_captured(captured_z) def clear_alarm(self) -> None: self._write_control(CW_CLEAR_FAULT) @@ -418,62 +429,24 @@ def _calibrate_one( current = self._modbus.read_register_32bit(self._slave_id, addr, signed=signed) else: current = self._modbus.read_register(self._slave_id, addr) - desired = (current & ~mask) | expected if mask is not None else expected + desired = (current & ~mask) | (expected & mask) if mask is not None else expected fmt = "0x%08X" if mask is not None else "%d" + current_str, desired_str = fmt % current, fmt % desired if current == desired: - logger.info(f"%s @ 0x%04X: device={fmt} matches desired (no write)", label, addr, current) + logger.debug("%s @ 0x%04X: device=%s matches desired (no write)", label, addr, current_str) return False if is_32bit: self._modbus.write_register_32bit(self._slave_id, addr, desired, signed=signed) else: self._modbus.write_register(self._slave_id, addr, desired) - logger.info(f"%s @ 0x%04X: {fmt} -> {fmt} (wrote)", label, addr, current, desired) + logger.info("%s @ 0x%04X: %s -> %s (wrote)", label, addr, current_str, desired_str) return True def _calibrate_motion_params(self) -> bool: - return any( - [ - self._calibrate_one(REG_ACCEL, EXPECTED_ACCEL, "accel", is_32bit=True), - self._calibrate_one(REG_DECEL, EXPECTED_DECEL, "decel", is_32bit=True), - self._calibrate_one(REG_MAX_SPEED, EXPECTED_MAX_SPEED, "max_speed", is_32bit=True), - self._calibrate_one(REG_MIN_SPEED, EXPECTED_MIN_SPEED, "min_speed", is_32bit=True), - self._calibrate_one(REG_LOW_SPEED_OPT, DRIVE_PARAM, "drive_param"), - ] - ) + return any([self._calibrate_one(*a, **kw) for *a, kw in _MOTION_PARAMS]) def _calibrate_homing_config(self) -> bool: - return any( - [ - self._calibrate_one(REG_HOMING_METHOD, HOMING_METHOD, "homing_method"), - self._calibrate_one( - REG_HOMING_OFFSET, - HOMING_ORIGIN_OFFSET, - "homing_offset", - is_32bit=True, - signed=True, - ), - self._calibrate_one( - REG_HOMING_SEARCH_SPEED, - HOMING_SEARCH_SPEED, - "homing_search_speed", - is_32bit=True, - ), - self._calibrate_one( - REG_HOMING_ZERO_SPEED, - HOMING_ZERO_SPEED, - "homing_zero_speed", - is_32bit=True, - ), - self._calibrate_one(REG_ZERO_RETURN, HOMING_ZERO_RETURN, "zero_return"), - self._calibrate_one( - REG_DI_FUNCTION, - DI1_FUNCTION_NEG_LIMIT, - "DI1_function", - is_32bit=True, - mask=0xF, - ), - ] - ) + return any([self._calibrate_one(*a, **kw) for *a, kw in _HOMING_PARAMS]) def _save_to_eeprom(self) -> None: self._write_holding(REG_SAVE_PARAMS, SAVE_PARAMS_MAGIC) From 05e6ec42f3a5181410589df9baed6932d238aff4 Mon Sep 17 00:00:00 2001 From: You Yan Date: Tue, 19 May 2026 00:22:56 -0700 Subject: [PATCH 31/33] fix: handle unknown objective names without crashing the GUI Addresses two Copilot review comments: - ObjectivesWidget.on_objective_changed wraps move_to_objective in a try/except for KeyError. Previously selecting an objective not in the changer's mapping (e.g. an objective listed in objectives.csv but not in the Xeryon/turret position lists) would raise from a Qt slot, destabilizing the event loop. Now shows a warning dialog and skips the signal emit. - Microscope._prepare_for_use wraps move_to_objective(DEFAULT_OBJECTIVE) in a try/except and re-raises as a clear RuntimeError. A config mismatch (DEFAULT_OBJECTIVE not listed for the active changer) now produces an actionable startup error instead of a bare KeyError. Co-Authored-By: Claude Opus 4.7 (1M context) --- software/control/microscope.py | 8 +++++++- software/control/widgets.py | 10 +++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/software/control/microscope.py b/software/control/microscope.py index 1054d2133..25f675f2e 100644 --- a/software/control/microscope.py +++ b/software/control/microscope.py @@ -498,7 +498,13 @@ def _prepare_for_use(self, skip_init: bool = False): self.addons.objective_changer.home() if control._def.USE_XERYON: self.addons.objective_changer.setSpeed(control._def.XERYON_SPEED) - self.addons.objective_changer.move_to_objective(control._def.DEFAULT_OBJECTIVE) + try: + self.addons.objective_changer.move_to_objective(control._def.DEFAULT_OBJECTIVE) + except KeyError as e: + raise RuntimeError( + f"DEFAULT_OBJECTIVE={control._def.DEFAULT_OBJECTIVE!r} is not configured " + f"for the active objective changer: {e}" + ) from e def _sync_confocal_mode_from_hardware(self) -> bool: """Sync confocal mode state from spinning disk hardware. diff --git a/software/control/widgets.py b/software/control/widgets.py index 0f2d0718a..a870adace 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -3558,7 +3558,15 @@ def init_ui(self): def on_objective_changed(self, objective_name): self.objectiveStore.set_current_objective(objective_name) if self.objective_changer is not None: - self.objective_changer.move_to_objective(objective_name) + try: + self.objective_changer.move_to_objective(objective_name) + except KeyError as e: + QMessageBox.warning( + self, + "Objective Not Available", + f"Objective '{objective_name}' is not configured for the objective changer:\n{e}", + ) + return self.signal_objective_changed.emit() From 29d84256d07ad3bdb78af163ac2af4afb052f804 Mon Sep 17 00:00:00 2001 From: You Yan Date: Tue, 19 May 2026 00:28:06 -0700 Subject: [PATCH 32/33] fix: remaining Copilot review items MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ModbusRTUClient: - _send_receive validates slave_id and function-code echo in the response before returning. Defends against cross-talk on a shared RS-485 bus where stray frames could otherwise pass the CRC check and be parsed as the expected reply. - connect() and disconnect() now hold the same lock as _send_receive so another thread can't close the serial port mid-transaction. tests/control/test_modbus_rtu.py (new): - 13 pure-Python tests covering calculate_crc (known vectors), _verify_crc round-trip + corruption + short-frame rejection, and the four frame builders (read/read-input/write-single/write-multiple) — including a parametrized round-trip for boundary values 0/1/0x7FFF/0x8000/0xFFFF. control/microscope.py: - objective_changer typed as Optional[ObjectiveChangerProtocol] instead of Optional[object]. The Protocol declares the methods both controller variants share (home, move_to_objective, close) so static checkers can catch misuse without forcing a hard Union. tests/control/test_objective_changer_2_pos_controller.py: - test_move_to_objective_short_circuits_when_already_there set up at pos1 first so the pos1 -> pos2 transition actually records a Z move; added an assertion that the first move recorded Z so the short-circuit case is genuinely exercised. Co-Authored-By: Claude Opus 4.7 (1M context) --- software/control/microscope.py | 17 ++- software/control/modbus_rtu.py | 59 ++++++---- software/tests/control/test_modbus_rtu.py | 108 ++++++++++++++++++ ...test_objective_changer_2_pos_controller.py | 5 + 4 files changed, 167 insertions(+), 22 deletions(-) create mode 100644 software/tests/control/test_modbus_rtu.py diff --git a/software/control/microscope.py b/software/control/microscope.py index 25f675f2e..2dff1fb01 100644 --- a/software/control/microscope.py +++ b/software/control/microscope.py @@ -1,6 +1,6 @@ import time from pathlib import Path -from typing import List, Optional +from typing import List, Optional, Protocol import imageio import numpy as np @@ -46,6 +46,19 @@ else: ObjectiveTurret4PosController = None + +class ObjectiveChangerProtocol(Protocol): + """Common interface implemented by both Xeryon 2-pos and NiMotion 4-pos turret controllers. + + Lets type checkers verify that consumers only call methods supported by every + objective-changer variant. Optional methods (e.g. `setSpeed` on Xeryon, + `clear_alarm` on the turret) are accessed via attribute lookup at runtime. + """ + + def home(self) -> None: ... + def move_to_objective(self, objective_name: str) -> None: ... + def close(self) -> None: ... + if control._def.RUN_FLUIDICS: from control.fluidics import Fluidics else: @@ -205,7 +218,7 @@ def __init__( nl5: Optional[NL5] = None, cellx: Optional[serial_peripherals.CellX] = None, emission_filter_wheel: Optional[AbstractFilterWheelController] = None, - objective_changer: Optional[object] = None, + objective_changer: Optional[ObjectiveChangerProtocol] = None, camera_focus: Optional[AbstractCamera] = None, fluidics: Optional[Fluidics] = None, piezo_stage: Optional[PiezoStage] = None, diff --git a/software/control/modbus_rtu.py b/software/control/modbus_rtu.py index c19fcf802..1d3fce243 100644 --- a/software/control/modbus_rtu.py +++ b/software/control/modbus_rtu.py @@ -354,28 +354,30 @@ def is_connected(self) -> bool: return self._serial is not None and self._serial.is_open def connect(self, port: Optional[str] = None, baudrate: Optional[int] = None): - if port is not None: - self._port = port - if baudrate is not None: - self._baudrate = baudrate - if self._port is None: - raise ModbusError("No serial port specified") - if self._serial is not None: - self._serial.close() - self._serial = None - try: - self._serial = serial.Serial(self._port, baudrate=self._baudrate, timeout=self._timeout) - except (serial.SerialException, OSError) as e: - raise ModbusError(str(e)) from e - logger.info(f"Modbus RTU connected: {self._port}") - - def disconnect(self): - if self._serial is not None: - try: + with self._lock: + if port is not None: + self._port = port + if baudrate is not None: + self._baudrate = baudrate + if self._port is None: + raise ModbusError("No serial port specified") + if self._serial is not None: self._serial.close() - finally: self._serial = None - logger.info("Modbus RTU disconnected") + try: + self._serial = serial.Serial(self._port, baudrate=self._baudrate, timeout=self._timeout) + except (serial.SerialException, OSError) as e: + raise ModbusError(str(e)) from e + logger.info(f"Modbus RTU connected: {self._port}") + + def disconnect(self): + with self._lock: + if self._serial is not None: + try: + self._serial.close() + finally: + self._serial = None + logger.info("Modbus RTU disconnected") def _require_connected(self): if not self.is_connected: @@ -508,6 +510,23 @@ def _send_receive(self, frame: bytes, expected_response_len: int) -> bytes: attempt += 1 continue + # Validate slave_id and function-code echo (defense on a shared + # RS-485 bus where stray frames or cross-talk could otherwise be + # silently parsed as a valid response). + if response[0] != frame[0] or response[1] != frame[1]: + last_error = ModbusError( + f"Response mismatch: expected slave=0x{frame[0]:02X} fc=0x{frame[1]:02X}, " + f"got slave=0x{response[0]:02X} fc=0x{response[1]:02X}", + slave_id=frame[0], + ) + logger.warning( + f"Modbus request failed (attempt {attempt + 1}/" f"{self._retries + 1}): {last_error}" + ) + if attempt < self._retries: + time.sleep(FRAME_INTERVAL * 2) + attempt += 1 + continue + return response raise last_error diff --git a/software/tests/control/test_modbus_rtu.py b/software/tests/control/test_modbus_rtu.py new file mode 100644 index 000000000..2a7f22bc3 --- /dev/null +++ b/software/tests/control/test_modbus_rtu.py @@ -0,0 +1,108 @@ +"""Pure-Python tests for Modbus RTU CRC, frame builders, and response parsing. + +No hardware required — exercises the codec in isolation. +""" + +from __future__ import annotations + +import pytest + +from control.modbus_rtu import ( + _verify_crc, + build_read_input_registers_frame, + build_read_registers_frame, + build_write_multiple_registers_frame, + build_write_register_frame, + calculate_crc, +) + + +# CRC-16 (Modbus) reference vectors. Values from the standard examples in +# common Modbus references. + + +def test_crc_known_vectors(): + # Slave 0x01, FC 0x03, addr 0x0000, count 0x0001 -> CRC 0x840A (LE) + assert calculate_crc(b"\x01\x03\x00\x00\x00\x01") == 0x0A84 + # Slave 0x11, FC 0x06, addr 0x0001, value 0x0003 -> CRC 0x9A9B (LE) + assert calculate_crc(b"\x11\x06\x00\x01\x00\x03") == 0x9B9A + + +def test_verify_crc_round_trip(): + body = b"\x01\x03\x02\x00\x06" + crc = calculate_crc(body) + frame = body + bytes([crc & 0xFF, (crc >> 8) & 0xFF]) + assert _verify_crc(frame) is True + + # Single-bit flip in the payload -> CRC fails. + corrupted = bytearray(frame) + corrupted[2] ^= 0x01 + assert _verify_crc(bytes(corrupted)) is False + + +def test_verify_crc_rejects_short_frames(): + assert _verify_crc(b"") is False + assert _verify_crc(b"\x01\x03\x00") is False # too short to carry CRC + + +# Frame builder shapes: length, opcode, parameters, and trailing CRC validity. + + +def test_build_read_holding_registers_frame(): + frame = build_read_registers_frame(slave_id=1, address=0x0021, count=2) + assert len(frame) == 8 + assert frame[0] == 0x01 # slave + assert frame[1] == 0x03 # FC=read holding + assert (frame[2] << 8) | frame[3] == 0x0021 + assert (frame[4] << 8) | frame[5] == 0x0002 + assert _verify_crc(frame) + + +def test_build_read_input_registers_frame_uses_fc4(): + frame = build_read_input_registers_frame(slave_id=1, address=0x001F, count=1) + assert frame[1] == 0x04 # FC=read input + assert _verify_crc(frame) + + +def test_build_write_single_register_frame(): + frame = build_write_register_frame(slave_id=2, address=0x0051, value=0x000F) + assert len(frame) == 8 + assert frame[0] == 0x02 + assert frame[1] == 0x06 # FC=write single + assert (frame[2] << 8) | frame[3] == 0x0051 + assert (frame[4] << 8) | frame[5] == 0x000F + assert _verify_crc(frame) + + +def test_build_write_multiple_registers_frame(): + frame = build_write_multiple_registers_frame(slave_id=1, address=0x0053, values=[0x0001, 0x9CB8]) + # slave(1) + fc(1) + addr(2) + qty(2) + byte_count(1) + 2*data(2) + crc(2) = 13 + assert len(frame) == 13 + assert frame[1] == 0x10 # FC=write multiple + assert (frame[2] << 8) | frame[3] == 0x0053 + assert (frame[4] << 8) | frame[5] == 2 # quantity + assert frame[6] == 4 # byte count + assert _verify_crc(frame) + + +def test_build_write_multiple_rejects_empty_values(): + # Spec: count >= 1; build with zero values shouldn't silently produce a + # malformed frame. Behavior is "frame with quantity=0 + byte_count=0"; + # confirm the builder still produces a CRC-valid frame so an exception + # propagates from the slave rather than from local CRC checks. + frame = build_write_multiple_registers_frame(slave_id=1, address=0x0000, values=[]) + assert _verify_crc(frame) + assert (frame[4] << 8) | frame[5] == 0 + assert frame[6] == 0 + + +@pytest.mark.parametrize( + "value", + [0x0000, 0x0001, 0x7FFF, 0x8000, 0xFFFF], +) +def test_write_register_round_trip(value): + """Build a write frame, then re-extract the value from the frame bytes.""" + frame = build_write_register_frame(slave_id=1, address=0x0010, value=value) + extracted = (frame[4] << 8) | frame[5] + assert extracted == value + assert _verify_crc(frame) diff --git a/software/tests/control/test_objective_changer_2_pos_controller.py b/software/tests/control/test_objective_changer_2_pos_controller.py index c3ee0661d..bc2e73106 100644 --- a/software/tests/control/test_objective_changer_2_pos_controller.py +++ b/software/tests/control/test_objective_changer_2_pos_controller.py @@ -37,7 +37,12 @@ def test_move_to_objective_short_circuits_when_already_there(monkeypatch): monkeypatch.setattr(control._def, "XERYON_OBJECTIVE_SWITCHER_POS_2", ["20x", "40x"]) stage = FakeStage() sim = ObjectiveChanger2PosController_Simulation(sn="SIM", stage=stage) + # The sim only retracts Z when moving from pos1 to pos2 (or vice versa), so + # initial state must be at pos1 for the pos1 -> pos2 move to record a Z move. + sim.move_to_objective("4x") + stage.z_moves.clear() sim.move_to_objective("40x") # pos1 -> pos2: records Z move + assert stage.z_moves, "expected pos1 -> pos2 to record a Z retract" z_moves_after_first = list(stage.z_moves) sim.move_to_objective("40x") # already at pos2: no extra Z move assert stage.z_moves == z_moves_after_first From 67648fdd462bdf6a996fac057da8bac88cf24c70 Mon Sep 17 00:00:00 2001 From: You Yan Date: Tue, 19 May 2026 00:35:22 -0700 Subject: [PATCH 33/33] refactor: post-review cleanup of Copilot patches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - widgets.on_objective_changed: move set_current_objective AFTER the successful move so the store doesn't end up ahead of the changer on KeyError. Revert the dropdown text in the except branch so the UI stays consistent with the actual state. - microscope._prepare_for_use: drop the redundant ': {e}' in the RuntimeError message; `from e` already chains the original exception. - microscope.ObjectiveChangerProtocol: trim the docstring (boilerplate removed; one-line summary of what's shared vs not). - modbus_rtu._send_receive: shorten the slave/FC validation comment. - tests/control/test_modbus_rtu: rename 'rejects_empty_values' → 'with_empty_values_still_produces_valid_frame' so the name matches the assertions; drop section-header / module-docstring narration. Co-Authored-By: Claude Opus 4.7 (1M context) --- software/control/microscope.py | 13 +++++-------- software/control/modbus_rtu.py | 5 ++--- software/control/widgets.py | 6 +++++- software/tests/control/test_modbus_rtu.py | 20 ++++---------------- 4 files changed, 16 insertions(+), 28 deletions(-) diff --git a/software/control/microscope.py b/software/control/microscope.py index 2dff1fb01..ec4fd458e 100644 --- a/software/control/microscope.py +++ b/software/control/microscope.py @@ -48,17 +48,14 @@ class ObjectiveChangerProtocol(Protocol): - """Common interface implemented by both Xeryon 2-pos and NiMotion 4-pos turret controllers. - - Lets type checkers verify that consumers only call methods supported by every - objective-changer variant. Optional methods (e.g. `setSpeed` on Xeryon, - `clear_alarm` on the turret) are accessed via attribute lookup at runtime. - """ + """Methods shared by both Xeryon and turret controllers. Controller-specific + methods (`setSpeed`, `clear_alarm`, …) are accessed via attribute lookup.""" def home(self) -> None: ... def move_to_objective(self, objective_name: str) -> None: ... def close(self) -> None: ... + if control._def.RUN_FLUIDICS: from control.fluidics import Fluidics else: @@ -515,8 +512,8 @@ def _prepare_for_use(self, skip_init: bool = False): self.addons.objective_changer.move_to_objective(control._def.DEFAULT_OBJECTIVE) except KeyError as e: raise RuntimeError( - f"DEFAULT_OBJECTIVE={control._def.DEFAULT_OBJECTIVE!r} is not configured " - f"for the active objective changer: {e}" + f"DEFAULT_OBJECTIVE={control._def.DEFAULT_OBJECTIVE!r} " + f"is not configured for the active objective changer" ) from e def _sync_confocal_mode_from_hardware(self) -> bool: diff --git a/software/control/modbus_rtu.py b/software/control/modbus_rtu.py index 1d3fce243..a3e90697c 100644 --- a/software/control/modbus_rtu.py +++ b/software/control/modbus_rtu.py @@ -510,9 +510,8 @@ def _send_receive(self, frame: bytes, expected_response_len: int) -> bytes: attempt += 1 continue - # Validate slave_id and function-code echo (defense on a shared - # RS-485 bus where stray frames or cross-talk could otherwise be - # silently parsed as a valid response). + # Reject frames from a different slave or function code — guards + # against cross-talk on a shared RS-485 bus. if response[0] != frame[0] or response[1] != frame[1]: last_error = ModbusError( f"Response mismatch: expected slave=0x{frame[0]:02X} fc=0x{frame[1]:02X}, " diff --git a/software/control/widgets.py b/software/control/widgets.py index a870adace..e604e36d4 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -3556,7 +3556,6 @@ def init_ui(self): self.setLayout(layout) def on_objective_changed(self, objective_name): - self.objectiveStore.set_current_objective(objective_name) if self.objective_changer is not None: try: self.objective_changer.move_to_objective(objective_name) @@ -3566,7 +3565,12 @@ def on_objective_changed(self, objective_name): "Objective Not Available", f"Objective '{objective_name}' is not configured for the objective changer:\n{e}", ) + # Revert the dropdown so it matches the store / actual changer state. + self.dropdown.blockSignals(True) + self.dropdown.setCurrentText(self.objectiveStore.current_objective) + self.dropdown.blockSignals(False) return + self.objectiveStore.set_current_objective(objective_name) self.signal_objective_changed.emit() diff --git a/software/tests/control/test_modbus_rtu.py b/software/tests/control/test_modbus_rtu.py index 2a7f22bc3..e4e64a7c1 100644 --- a/software/tests/control/test_modbus_rtu.py +++ b/software/tests/control/test_modbus_rtu.py @@ -1,7 +1,4 @@ -"""Pure-Python tests for Modbus RTU CRC, frame builders, and response parsing. - -No hardware required — exercises the codec in isolation. -""" +"""Pure-Python tests for Modbus RTU CRC, frame builders, and response parsing.""" from __future__ import annotations @@ -17,10 +14,6 @@ ) -# CRC-16 (Modbus) reference vectors. Values from the standard examples in -# common Modbus references. - - def test_crc_known_vectors(): # Slave 0x01, FC 0x03, addr 0x0000, count 0x0001 -> CRC 0x840A (LE) assert calculate_crc(b"\x01\x03\x00\x00\x00\x01") == 0x0A84 @@ -45,9 +38,6 @@ def test_verify_crc_rejects_short_frames(): assert _verify_crc(b"\x01\x03\x00") is False # too short to carry CRC -# Frame builder shapes: length, opcode, parameters, and trailing CRC validity. - - def test_build_read_holding_registers_frame(): frame = build_read_registers_frame(slave_id=1, address=0x0021, count=2) assert len(frame) == 8 @@ -85,11 +75,9 @@ def test_build_write_multiple_registers_frame(): assert _verify_crc(frame) -def test_build_write_multiple_rejects_empty_values(): - # Spec: count >= 1; build with zero values shouldn't silently produce a - # malformed frame. Behavior is "frame with quantity=0 + byte_count=0"; - # confirm the builder still produces a CRC-valid frame so an exception - # propagates from the slave rather than from local CRC checks. +def test_build_write_multiple_with_empty_values_still_produces_valid_frame(): + # Builder doesn't validate count >= 1; the slave is expected to reject + # quantity=0 with a Modbus exception (not a local CRC failure). frame = build_write_multiple_registers_frame(slave_id=1, address=0x0000, values=[]) assert _verify_crc(frame) assert (frame[4] << 8) | frame[5] == 0