From 2f61be85287266fc126b257c91ecf2e7b464d84d Mon Sep 17 00:00:00 2001 From: V-wsteman Date: Mon, 19 Oct 2026 14:33:35 +0200 Subject: [PATCH 1/4] feat(Galil): enhance motion control and parsing logic - Added time-based retries for parsing motion flags and position replies. - Improved connection handling with multiple address candidates. - Refactored wait methods for better clarity and functionality. --- .../instrument_drivers/Galil/dmc_41x3.py | 172 +++++++++++++++--- 1 file changed, 146 insertions(+), 26 deletions(-) diff --git a/src/qcodes/instrument_drivers/Galil/dmc_41x3.py b/src/qcodes/instrument_drivers/Galil/dmc_41x3.py index d211032ad5b..edd3fb208c6 100644 --- a/src/qcodes/instrument_drivers/Galil/dmc_41x3.py +++ b/src/qcodes/instrument_drivers/Galil/dmc_41x3.py @@ -4,6 +4,7 @@ Colloquially known as the "stepper motors". """ +import time from typing import TYPE_CHECKING import numpy as np @@ -29,6 +30,47 @@ "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: + 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 the three 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. + """ + parts = response.replace(":", " ").replace(",", " ").split() + if len(parts) < 3: + return None + try: + a, b, c = (int(float(part)) for part in parts[:3]) + except ValueError: + return None + return {"A": a, "B": b, "C": c} + class GalilMotionController(Instrument): """ @@ -61,8 +103,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]: """ @@ -441,19 +510,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 +633,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 +689,52 @@ 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". + + 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, 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 = { + axis: _parse_motion_flag(self.ask(f"MG _BG{axis}")) for axis in axes + } + # Done only when every axis reported a definite 0. + if all(flag == 0 for flag in flags.values()): + return + if time.monotonic() > deadline: + raise TimeoutError( + f"Axes {axes} did not finish moving within " + f"{timeout_s:g}s (motion flags: {flags})." + ) + time.sleep(_POLL_INTERVAL_S) 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: From f45f5b119b1a4111c4442b128398697474c048c3 Mon Sep 17 00:00:00 2001 From: V-wsteman Date: Wed, 21 Oct 2026 15:51:27 +0200 Subject: [PATCH 2/4] Add changelog entry for PR #8647 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/changes/newsfragments/8647.improved_driver | 1 + 1 file changed, 1 insertion(+) create mode 100644 docs/changes/newsfragments/8647.improved_driver diff --git a/docs/changes/newsfragments/8647.improved_driver b/docs/changes/newsfragments/8647.improved_driver new file mode 100644 index 00000000000..d042a4393e1 --- /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``. 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``. From a15af654492d17c335a0ff978a8c768b7102eee3 Mon Sep 17 00:00:00 2001 From: V-wsteman Date: Fri, 23 Oct 2026 13:07:05 +0200 Subject: [PATCH 3/4] fix(Galil): handle OverflowError in motion flag and position reply parsing --- src/qcodes/instrument_drivers/Galil/dmc_41x3.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/qcodes/instrument_drivers/Galil/dmc_41x3.py b/src/qcodes/instrument_drivers/Galil/dmc_41x3.py index edd3fb208c6..a801376356f 100644 --- a/src/qcodes/instrument_drivers/Galil/dmc_41x3.py +++ b/src/qcodes/instrument_drivers/Galil/dmc_41x3.py @@ -48,7 +48,7 @@ def _parse_motion_flag(response: str) -> int | None: return None try: return int(float(cleaned)) - except ValueError: + except (ValueError, OverflowError): return None @@ -57,17 +57,18 @@ 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 the three positions separated by commas and + 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. """ - parts = response.replace(":", " ").replace(",", " ").split() - if len(parts) < 3: + 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[:3]) - except ValueError: + a, b, c = (int(float(part)) for part in parts) + except (ValueError, OverflowError): return None return {"A": a, "B": b, "C": c} From 569a9c5db28ace64b1d9f49ff214a8d5c72a82d8 Mon Sep 17 00:00:00 2001 From: V-wsteman Date: Fri, 23 Oct 2026 13:41:23 +0200 Subject: [PATCH 4/4] fix(Galil): enhance motion completion handling with timeout and I/O error propagation --- .../newsfragments/8647.improved_driver | 2 +- .../instrument_drivers/Galil/dmc_41x3.py | 53 +++++++++++++++---- 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/docs/changes/newsfragments/8647.improved_driver b/docs/changes/newsfragments/8647.improved_driver index d042a4393e1..ecb57a9b4c3 100644 --- a/docs/changes/newsfragments/8647.improved_driver +++ b/docs/changes/newsfragments/8647.improved_driver @@ -1 +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``. 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``. +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 a801376356f..c84e8984f41 100644 --- a/src/qcodes/instrument_drivers/Galil/dmc_41x3.py +++ b/src/qcodes/instrument_drivers/Galil/dmc_41x3.py @@ -5,6 +5,7 @@ """ import time +from math import ceil from typing import TYPE_CHECKING import numpy as np @@ -165,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: @@ -697,29 +698,59 @@ def wait_till_axes_motion_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". + 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, after this many seconds. + keep returning unparsable replies or fail to respond, after + this many seconds. """ deadline = time.monotonic() + timeout_s try: while True: - flags = { - axis: _parse_motion_flag(self.ask(f"MG _BG{axis}")) for axis in axes - } - # Done only when every axis reported a definite 0. - if all(flag == 0 for flag in flags.values()): - return - if time.monotonic() > deadline: + 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})." ) - time.sleep(_POLL_INTERVAL_S) + # 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.write(f"MO {axes}")