Skip to content

Improvements to Galil driver - #8677

Draft
Jens Hedegaard Nielsen (jenshnielsen) wants to merge 6 commits into
microsoft:mainfrom
jenshnielsen:galil_fixes
Draft

Jens Hedegaard Nielsen (jenshnielsen) wants to merge 6 commits into
microsoft:mainfrom
jenshnielsen:galil_fixes

Conversation

@jenshnielsen

Copy link
Copy Markdown
Collaborator

No description provided.

@codecov

codecov Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.55172% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 75.70%. Comparing base (572851c) to head (026dcdb).

Files with missing lines Patch % Lines
src/qcodes/instrument_drivers/Galil/dmc_41x3.py 96.55% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8677      +/-   ##
==========================================
+ Coverage   75.02%   75.70%   +0.68%     
==========================================
  Files         307      307              
  Lines       32506    32498       -8     
==========================================
+ Hits        24386    24604     +218     
+ Misses       8120     7894     -226     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Setting it only updated the cached value without moving the motors.
Also add tests for the driver with a stand-in for gclib.
on_off_status and off_when_error_occurs only looked at the first
character of the reply, so the empty or bare ':' replies that gclib
can return under rapid polling raised or gave the wrong state. Retry
them like is_in_motion does.
close() only closed the gclib connection, so the controller stayed
registered and could not be created again with the same name.
Move the retry of empty or bare ':' replies into ask_raw instead of
separate loops in a few methods, so all getters get it. The motion
wait loop polls once per query and keeps its own deadline handling.
Malformed non-empty replies now raise instead of being retried.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

Static review identified no blocking issues, and focused regression tests and release notes cover the behavioral changes.

No open issues 🎉

What changed in this PR

Improves the Galil DMC-41x3 driver’s reply handling and cleanup, with regression tests and release notes.

Changes:

  • Centralizes retries for empty replies and parses complete numeric status responses.
  • Makes absolute position read-only and ensures closing a controller releases its QCoDeS registration.
  • Adds fake-gclib tests covering reply handling, motion polling, and controller lifecycle.
File Description
tests/​drivers/​test_galil_dmc_41x3.py Adds regression tests without requiring vendor gclib.
src/​qcodes/​instrument_drivers/​Galil/​dmc_41x3.py Updates retries, parsing, cleanup, and parameter behavior.
docs/​changes/​newsfragments/​8677.improved_driver Documents reply-handling and cleanup improvements.
docs/​changes/​newsfragments/​8677.breaking Documents the read-only absolute-position change.

Review effort: Balanced

"""
val = self.ask(f"MG _MO{self._axis}")
if val[0] == "1":
if int(float(self.ask(f"MG _MO{self._axis}"))) == 1:

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

why is it no longer required to take the first element

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants