diff --git a/docs/changes/newsfragments/8647.improved_driver b/docs/changes/newsfragments/8647.improved_driver new file mode 100644 index 00000000000..ecb57a9b4c3 --- /dev/null +++ b/docs/changes/newsfragments/8647.improved_driver @@ -0,0 +1 @@ +The Galil DMC-41x3 driver now retries empty or malformed controller replies when polling motion status and axis positions, and waiting for motion to complete is bounded by a timeout (300 s by default) that raises ``TimeoutError``. Each motion-status query is limited by the remaining motion timeout, with millisecond resolution, and restores the original I/O timeout afterward. Connecting now falls back through the ``--direct -s ALL`` and ``-s ALL`` options before using the bare address. Pressing Ctrl-C while waiting for motion now calls ``abort()`` and then turns the axes off with ``MO``. diff --git a/src/qcodes/instrument_drivers/Galil/dmc_41x3.py b/src/qcodes/instrument_drivers/Galil/dmc_41x3.py index d211032ad5b..c84e8984f41 100644 --- a/src/qcodes/instrument_drivers/Galil/dmc_41x3.py +++ b/src/qcodes/instrument_drivers/Galil/dmc_41x3.py @@ -4,6 +4,8 @@ Colloquially known as the "stepper motors". """ +import time +from math import ceil from typing import TYPE_CHECKING import numpy as np @@ -29,6 +31,48 @@ "environment." ) from e +_POLL_INTERVAL_S = 0.01 +_DEFAULT_MOTION_TIMEOUT_S = 300.0 +_REPLY_RETRIES = 10 + + +def _parse_motion_flag(response: str) -> int | None: + """ + Parse the reply to ``MG _BG``. + + Under rapid polling gclib can return the bare ``:`` prompt or an empty + string instead of a number. Return ``None`` for such transient or + otherwise unparsable replies so that the caller can retry. + """ + cleaned = response.strip().strip(":").strip() + if not cleaned: + return None + try: + return int(float(cleaned)) + except (ValueError, OverflowError): + return None + + +def _parse_position_reply(response: str) -> dict[str, int] | None: + """ + Parse the reply to ``PA ?,?,?`` into a dictionary with keys ``"A"``, + ``"B"`` and ``"C"``. + + The controller returns exactly three numeric positions separated by commas and + sometimes terminated by the ``:`` prompt, e.g. ``"100, 200, 300"``. + Return ``None`` for a bare ``:`` prompt, an empty reply or a malformed + reply so that the caller can retry. + """ + cleaned = response.strip().removesuffix(":").strip() + parts = cleaned.split(",") + if len(parts) != 3: + return None + try: + a, b, c = (int(float(part)) for part in parts) + except (ValueError, OverflowError): + return None + return {"A": a, "B": b, "C": c} + class GalilMotionController(Instrument): """ @@ -61,8 +105,35 @@ def open(self) -> None: This applies that Motion controller no more requests for an IP address and a connection to the Motion controller can be done by the IP address burned in. + + If the address already contains gclib switches (e.g. + ``"
--direct -s ALL"``) it is used as is. Otherwise the + address forms ``"
--direct -s ALL"``, ``"
-s ALL"`` + and ``"
"`` are tried in turn. + + Raises: + RuntimeError: if the connection fails for all address forms. + """ - self.g.GOpen(self._address + " --direct -s ALL") + address = self._address + if "--" in address or " -" in address: + candidates = [address] + else: + candidates = [f"{address} --direct -s ALL", f"{address} -s ALL", address] + + errors: list[str] = [] + for candidate in candidates: + try: + self.g.GOpen(candidate) + return + except gclib.GclibError as e: + errors.append(f"{candidate!r}: {e}") + + raise RuntimeError( + f"Failed to connect to Galil controller. Tried address strings: " + f"{candidates}. Confirm controller address/subnet reachability and " + f"power/link status. Errors: {' | '.join(errors)}" + ) def get_idn(self) -> dict[str, str | None]: """ @@ -95,7 +166,7 @@ def timeout(self, val: int) -> None: Sets timeout for the instrument Args: - val: time in milliseconds. -1 disables the timeout + val: time in milliseconds. -1 restores the initial connection timeout. """ if val < -1: @@ -441,19 +512,33 @@ def is_in_motion(self) -> int: """ Checks if the motor is in motion or not. return 1, if motor is in motion otherwise 0 - """ - return int(float(self.ask(f"MG _BG{self._axis}"))) - def wait_till_motor_motion_complete(self) -> None: + Transient empty replies of the controller are retried. + """ + response = "" + for _ in range(_REPLY_RETRIES): + response = self.ask(f"MG _BG{self._axis}") + flag = _parse_motion_flag(response) + if flag is not None: + return flag + time.sleep(_POLL_INTERVAL_S) + raise RuntimeError( + f"Could not parse motion flag of axis {self._axis} after retries: " + f"{response!r}" + ) + + def wait_till_motor_motion_complete( + self, timeout_s: float = _DEFAULT_MOTION_TIMEOUT_S + ) -> None: """ Wait for motion on the motor to complete + + Args: + timeout_s: raise ``TimeoutError`` if the motor is still moving, or + keeps returning unparsable replies, after this many seconds. + """ - try: - while self.is_in_motion(): - pass - except KeyboardInterrupt: - self.parent.abort() - self.off() + self.parent.wait_till_axes_motion_complete(self._axis, timeout_s) def error_magnitude(self) -> float: """ @@ -550,14 +635,19 @@ def _set_default_update_time(self) -> None: def _get_absolute_position(self) -> dict[str, int]: """ Gets absolution position of the motors from the defined origin - """ - result = {} - data = self.ask("PA ?,?,?").split(" ") - result["A"] = int(data[0][:-1]) - result["B"] = int(data[1][:-1]) - result["C"] = int(data[2]) - return result + Transient empty replies of the controller are retried. + """ + response = "" + for _ in range(_REPLY_RETRIES): + response = self.ask("PA ?,?,?") + parsed = _parse_position_reply(response) + if parsed is not None: + return parsed + time.sleep(_POLL_INTERVAL_S) + raise RuntimeError( + f"Could not parse absolute position reply after retries: {response!r}" + ) def end_program(self) -> None: """ @@ -601,20 +691,82 @@ def begin_motors(self) -> None: """ self.write("BG") - def wait_till_motion_complete(self) -> None: + def wait_till_axes_motion_complete( + self, axes: str = "ABC", timeout_s: float = _DEFAULT_MOTION_TIMEOUT_S + ) -> None: """ - This method waits for the motion on all motors to complete + Wait until none of the given axes is moving + + The controller is polled at a fixed interval. Transient empty replies + are treated as "not yet finished". Each query's I/O timeout is capped + by the remaining motion timeout (rounded up to milliseconds), and the + original I/O timeout is restored afterward. I/O errors before the + motion deadline are propagated. If the I/O timeout is -1, the initial + connection timeout is not exposed by gclib, so only the remaining + motion timeout and the 32767 ms API limit bound the query. + + Args: + axes: axis letters to wait for, e.g. ``"AB"``. + timeout_s: raise ``TimeoutError`` if the axes are still moving, or + keep returning unparsable replies or fail to respond, after + this many seconds. + """ + deadline = time.monotonic() + timeout_s try: - while ( - int(float(self.ask("MG _BGA"))) - or int(float(self.ask("MG _BGB"))) - or int(float(self.ask("MG _BGC"))) - ): - pass + while True: + flags: dict[str, int | None] = {} + for axis in axes: + remaining = deadline - time.monotonic() + if remaining <= 0: + raise TimeoutError( + f"Axes {axes} did not finish moving within " + f"{timeout_s:g}s (motion flags: {flags})." + ) + original_timeout = self.g.timeout + # GTimeout takes a signed short in the C API. + query_timeout_ms = min(ceil(remaining * 1000), 32767) + if original_timeout >= 0: + query_timeout_ms = min(query_timeout_ms, original_timeout) + try: + self.timeout(query_timeout_ms) + response = self.ask(f"MG _BG{axis}") + except gclib.GclibError as e: + if time.monotonic() >= deadline: + raise TimeoutError( + f"Axes {axes} did not finish moving within " + f"{timeout_s:g}s (querying axis {axis})." + ) from e + raise + finally: + self.timeout(original_timeout) + flags[axis] = _parse_motion_flag(response) + remaining = deadline - time.monotonic() + if remaining <= 0: + raise TimeoutError( + f"Axes {axes} did not finish moving within " + f"{timeout_s:g}s (motion flags: {flags})." + ) + # Done only when every axis reported a definite 0. + if all(flag == 0 for flag in flags.values()): + return + time.sleep(min(_POLL_INTERVAL_S, remaining)) except KeyboardInterrupt: self.abort() - self.motors_off() + self.write(f"MO {axes}") + + def wait_till_motion_complete( + self, timeout_s: float = _DEFAULT_MOTION_TIMEOUT_S + ) -> None: + """ + This method waits for the motion on all motors to complete + + Args: + timeout_s: raise ``TimeoutError`` if the motors are still moving + after this many seconds. + + """ + self.wait_till_axes_motion_complete("ABC", timeout_s) class GalilDMC4133Arm: