Skip to content

Conversation

@AdvancedImagingUTSW
Copy link
Collaborator

Add support for Thorlabs Kinesis (serial) stages: new pylablib-backed API (pykinesis_controller), a KINESISStage implementation in the Thorlabs stage module, and configuration DB entry. Add quadratic and centered_cubic galvo waveforms and wire them into GalvoBase. Make MS2000 serial buffer sizing platform-safe (only attempt set_buffer_size on Windows). Update docs to list new waveform options and KINESIS stage config, and add/adjust unit tests for MS2000, waveforms, galvo base, and device startup flow (including a test ensuring the KINESIS start path uses the factory).

Add support for Thorlabs Kinesis (serial) stages: new pylablib-backed API (pykinesis_controller), a KINESISStage implementation in the Thorlabs stage module, and configuration DB entry. Add quadratic and centered_cubic galvo waveforms and wire them into GalvoBase. Make MS2000 serial buffer sizing platform-safe (only attempt set_buffer_size on Windows). Update docs to list new waveform options and KINESIS stage config, and add/adjust unit tests for MS2000, waveforms, galvo base, and device startup flow (including a test ensuring the KINESIS start path uses the factory).
@codecov
Copy link

codecov bot commented Feb 9, 2026

Codecov Report

❌ Patch coverage is 38.72832% with 106 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.94%. Comparing base (6c5bc85) to head (501572c).

Files with missing lines Patch % Lines
...odel/devices/APIs/thorlabs/pykinesis_controller.py 0.00% 58 Missing ⚠️
src/navigate/model/devices/stage/thorlabs.py 43.58% 44 Missing ⚠️
...te/model/devices/APIs/asi/asi_MS2000_controller.py 66.66% 2 Missing ⚠️
src/navigate/model/waveforms.py 92.30% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1176      +/-   ##
===========================================
+ Coverage    52.86%   52.94%   +0.07%     
===========================================
  Files          197      198       +1     
  Lines        24032    24196     +164     
===========================================
+ Hits         12704    12810     +106     
- Misses       11328    11386      +58     
Flag Coverage Δ
unittests 52.94% <38.72%> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 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.

@AdvancedImagingUTSW
Copy link
Collaborator Author

@SJShep - I apologize for letting your PR languish. There were a few things that prevented us from merging it immediately. This PR draft integrates many of the major changes you made, and leaves some key points out (e.g., scan direction, tiling wizard). Happy to discuss those individually. Any chance you could test this out on your system?

@AdvancedImagingUTSW AdvancedImagingUTSW marked this pull request as ready for review February 11, 2026 18:30
Copilot AI review requested due to automatic review settings February 11, 2026 18:30
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds Thorlabs Kinesis (serial) stage support and expands galvo waveform generation options, alongside documentation updates and unit tests to validate the new behaviors.

Changes:

  • Added KINESISStage (pylablib-backed) stage implementation and configuration DB entry.
  • Added quadratic and centered_cubic waveforms and integrated them into GalvoBase.
  • Made ASI MS2000 serial buffer sizing Windows-only and added targeted tests.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/navigate/model/waveforms.py Adds quadratic and centered_cubic waveform generators.
src/navigate/model/devices/galvo/base.py Wires new waveform options into galvo waveform selection logic.
src/navigate/model/devices/stage/thorlabs.py Adds KINESISStage and config helpers for stage hardware entries/unit scaling.
src/navigate/model/devices/APIs/thorlabs/pykinesis_controller.py Introduces pylablib-backed serial Kinesis wrapper used by KINESISStage.
src/navigate/model/devices/APIs/asi/asi_MS2000_controller.py Makes set_buffer_size conditional on Windows for compatibility.
src/navigate/config/configuration_database.py Registers “KINESIS” as a supported stage device type.
docs/source/02_user_guide/01_supported_hardware/stage.rst Documents MS2000 buffer behavior and adds KINESIS config section.
docs/source/02_user_guide/01_supported_hardware/galvo.rst Documents additional supported galvo waveform options.
test/model/test_waveforms.py Adds tests covering the new waveform functions.
test/model/devices/galvo/test_galvo_base.py Extends galvo base tests to include the new waveform options.
test/model/test_device_startup_functions.py Adds test ensuring KINESIS stage startup path uses start_device flow.
test/model/devices/APIs/asi/test_asi_ms2000_controller.py Adds Windows/Linux-specific tests for MS2000 buffer sizing behavior.
test/model/devices/APIs/asi/__init__.py Makes the ASI test directory a package for imports.

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

Comment on lines +99 to +100
if SLEEP_AFTER_WAIT:
sleep(SLEEP_AFTER_WAIT)
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

move_to_position() always sleeps for SLEEP_AFTER_WAIT even when wait_till_done is False. This adds an unconditional 100ms delay to every non-blocking move, which can significantly slow scans or interactive motion. Consider only sleeping when wait_till_done is True (or rename the constant/flag if the delay is required unconditionally).

Suggested change
if SLEEP_AFTER_WAIT:
sleep(SLEEP_AFTER_WAIT)
if SLEEP_AFTER_WAIT:
sleep(SLEEP_AFTER_WAIT)

Copilot uses AI. Check for mistakes.
Comment on lines +575 to +580
try:
self.stop()
self.kinesis_controller.close()
except Exception:
pass

Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

KINESISStage.__del__ swallows all exceptions silently. If stop()/close() fails (or if initialization was partial and kinesis_controller is missing), this will hide cleanup issues and make device shutdown problems hard to debug. Suggest logging at least at debug level and guarding against self.kinesis_controller being None.

Suggested change
try:
self.stop()
self.kinesis_controller.close()
except Exception:
pass
"""Best-effort cleanup of the KINESIS stage controller.
Exceptions are logged but not propagated to avoid errors during
garbage collection, especially if initialization was partial.
"""
kinesis = getattr(self, "kinesis_controller", None)
if kinesis is None:
return
try:
self.stop()
except Exception as exc:
logger.debug(
"Error while stopping KINESISStage during __del__: %s",
exc,
exc_info=True,
)
try:
kinesis.close()
except Exception as exc:
logger.debug(
"Error while closing KINESISStage during __del__: %s",
exc,
exc_info=True,
)

Copilot uses AI. Check for mistakes.
Comment on lines +592 to +598
def report_position(self) -> dict[str, float]:
try:
pos = self.kinesis_controller.get_current_position(self.steps_per_um)
setattr(self, f"{self.axes[0]}_pos", pos)
except Exception:
pass
return self.get_position_dict()
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

report_position() catches a blanket Exception and ignores it, which can mask real communication/configuration problems and leave stale positions without any indication. Consider catching the expected exception types from the pylablib backend (or logging unexpected exceptions at debug/warning) so failures are diagnosable.

Copilot uses AI. Check for mistakes.
Comment on lines +85 to +86
except Exception:
pass
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

'except' clause does nothing but pass and there is no explanatory comment.

Suggested change
except Exception:
pass
except Exception as exc:
# Ignore stop errors during shutdown, but log for diagnostics.
logger.warning("Failed to stop KINESIS stage cleanly: %s", exc)

Copilot uses AI. Check for mistakes.
@SJShep
Copy link
Contributor

SJShep commented Feb 11, 2026

@SJShep - I apologize for letting your PR languish. There were a few things that prevented us from merging it immediately. This PR draft integrates many of the major changes you made, and leaves some key points out (e.g., scan direction, tiling wizard). Happy to discuss those individually. Any chance you could test this out on your system?

@AdvancedImagingUTSW I would be happy to test this out on our system. Let me update you early next week. Thanks for helping out!

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