feat: Add ZMotorConfig enum and improve piezo-only mode support#408
feat: Add ZMotorConfig enum and improve piezo-only mode support#408hongquanli merged 4 commits intomasterfrom
Conversation
Replace string-based Z_MOTOR_CONFIG with ZMotorConfig Enum to prevent typos and enable IDE autocomplete. Adds has_piezo() method for cleaner piezo detection. Config files with string values continue to work via convert_to_enum(). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Hide and auto-check "Piezo Z-Stack" checkbox in piezo-only mode - Hide Z controls from NavigationWidget (Stages) in piezo-only mode - Update Z plot to use piezo position (μm) in piezo-only mode - Fix Z plot units: convert stepper z_mm to μm for consistent display 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR enhances type safety and improves support for piezo-only Z-axis configurations by converting Z_MOTOR_CONFIG from a string to a type-safe enum with helper methods. It also adds UI adjustments that automatically configure piezo-only mode interfaces and fixes Z-coordinate display to correctly show positions in micrometers across different motor configurations.
Key changes:
- Introduce
ZMotorConfigenum withSTEPPER,STEPPER_PIEZO, andPIEZOvalues, plushas_piezo()andis_piezo_only()helper methods - Auto-hide and check "Piezo Z-Stack" checkboxes in acquisition widgets when in piezo-only mode
- Hide Z stepper controls in NavigationWidget when in piezo-only mode (Z controlled via piezo widget)
- Fix Z plot coordinates to display correct values in micrometers for all motor configurations
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| software/control/_def.py | Defines ZMotorConfig enum with conversion methods and helper functions; converts Z_MOTOR_CONFIG variable from string to enum and initializes IS_PIEZO_ONLY flag |
| software/control/widgets.py | Hides Z stepper controls in NavigationWidget for piezo-only mode; auto-checks and hides "Piezo Z-Stack" checkboxes in three acquisition widgets when IS_PIEZO_ONLY |
| software/control/gui_hcs.py | Updates Z coordinate calculation in _signal_new_image_fn to correctly display positions in micrometers: stepper-only uses stepper position, stepper+piezo combines both, piezo-only uses piezo position |
| software/tests/control/test_def.py | Adds comprehensive unit tests for ZMotorConfig enum covering value checks, string-to-enum conversion, enum passthrough, invalid value handling, and helper methods |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
software/control/gui_hcs.py
Outdated
| # Z for plot in μm: combine stepper + piezo when piezo is used, otherwise just stepper | ||
| stepper_z_um = info.position.z_mm * 1000 | ||
| if info.z_piezo_um is not None: | ||
| z_for_plot = info.z_piezo_um if IS_PIEZO_ONLY else stepper_z_um + info.z_piezo_um | ||
| else: | ||
| z_for_plot = stepper_z_um |
There was a problem hiding this comment.
In piezo-only mode, when info.z_piezo_um is None, falling back to stepper_z_um is incorrect since there is no stepper motor. This could lead to displaying zero or stale stepper positions. Consider either: (1) using 0 as the fallback for piezo-only mode when z_piezo_um is None, or (2) ensuring that z_piezo_um is never None in piezo-only acquisitions. The current logic assumes the stepper position is always valid, which contradicts the piezo-only configuration.
| # Z for plot in μm: combine stepper + piezo when piezo is used, otherwise just stepper | |
| stepper_z_um = info.position.z_mm * 1000 | |
| if info.z_piezo_um is not None: | |
| z_for_plot = info.z_piezo_um if IS_PIEZO_ONLY else stepper_z_um + info.z_piezo_um | |
| else: | |
| z_for_plot = stepper_z_um | |
| # Z for plot in μm: in mixed mode combine stepper + piezo when piezo is used, | |
| # otherwise just stepper. In piezo-only mode, ignore stepper and fall back to 0 | |
| # if no piezo position is available. | |
| stepper_z_um = info.position.z_mm * 1000 | |
| if IS_PIEZO_ONLY: | |
| if info.z_piezo_um is not None: | |
| z_for_plot = info.z_piezo_um | |
| else: | |
| z_for_plot = 0 | |
| else: | |
| if info.z_piezo_um is not None: | |
| z_for_plot = stepper_z_um + info.z_piezo_um | |
| else: | |
| z_for_plot = stepper_z_um |
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
f91a2e9 to
882989f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Z for plot in μm: piezo-only uses piezo position, mixed mode combines stepper + piezo | ||
| stepper_z_um = info.position.z_mm * 1000 | ||
| if IS_PIEZO_ONLY: | ||
| z_for_plot = info.z_piezo_um if info.z_piezo_um is not None else 0 | ||
| elif info.z_piezo_um is not None: | ||
| z_for_plot = stepper_z_um + info.z_piezo_um | ||
| else: | ||
| z_for_plot = stepper_z_um | ||
| self.signal_coordinates.emit(info.position.x_mm, info.position.y_mm, z_for_plot, info.region_id) |
There was a problem hiding this comment.
The comment on line 244 states "Z for plot in μm" which is accurate, but doesn't explain the complete logic flow. Consider expanding this comment to clarify that:
- In STEPPER mode: only stepper position is used (converted to μm)
- In STEPPER + PIEZO mode: stepper + piezo positions are combined
- In PIEZO-only mode: only piezo position is used
This would help future maintainers understand the different behavior across motor configurations.
| if IS_PIEZO_ONLY: | ||
| self.checkbox_usePiezo.setChecked(True) | ||
| self.checkbox_usePiezo.setVisible(False) |
There was a problem hiding this comment.
The piezo-only mode UI adjustment logic is duplicated across three different widget classes (lines 4057-4059, 5428-5430, and 7098-7100). Consider extracting this logic into a helper method or applying it in a shared base class to reduce code duplication and improve maintainability. This would ensure consistent behavior and make future changes easier.
Summary
Z_MOTOR_CONFIGfrom string toZMotorConfigEnum for type safety and IDE autocompleteTest plan
ZMotorConfigenum (6 tests passing)Z_MOTOR_CONFIG = "PIEZO"):Z_MOTOR_CONFIG = "STEPPER"): UI unchanged, Z plot in μmZ_MOTOR_CONFIG = "STEPPER + PIEZO"): checkbox visible, Z plot shows combined Z🤖 Generated with Claude Code