Skip to content

fix: LaserAFConfig defaults from _def.py & status bar height stability#517

Merged
Alpaca233 merged 3 commits into
Cephla-Lab:masterfrom
Alpaca233:bugfix/laser-af-config-defaults
Mar 20, 2026
Merged

fix: LaserAFConfig defaults from _def.py & status bar height stability#517
Alpaca233 merged 3 commits into
Cephla-Lab:masterfrom
Alpaca233:bugfix/laser-af-config-defaults

Conversation

@Alpaca233
Copy link
Copy Markdown
Collaborator

Summary

  • LaserAFConfig defaults now reference _def.py constants instead of hardcoded values. Several defaults had diverged (e.g. spot_detection_mode was DUAL_RIGHT instead of DUAL_LEFT, correlation_threshold was 0.9 vs 0.7, focus_camera_exposure_time_ms was 0.2 vs 2.0).
  • Status bar no longer grows taller when error messages contain newlines. Newlines are collapsed to spaces in the display text, and the message label has a fixed height.

Test plan

  • All 182 config-related tests pass
  • All 32 WarningErrorWidget tests pass
  • Verify status bar stays single-line with multi-line error messages in the running app

🤖 Generated with Claude Code

…tus bar height changes

1) LaserAFConfig had hardcoded defaults that diverged from _def.py:
   - spot_detection_mode: DUAL_RIGHT vs DUAL_LEFT
   - min_peak_prominence: 0.25 vs 0.20
   - correlation_threshold: 0.9 vs 0.7
   - focus_camera_exposure_time_ms: 0.2 vs 2.0
   Now uses default_factory referencing _def.py constants so they stay in sync.

2) Status bar WarningErrorWidget could grow taller when error messages
   contained newlines. Fixed by collapsing whitespace in display text
   and setting a fixed height on the message label.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
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

This PR aligns LaserAFConfig model defaults with the canonical laser autofocus constants in control/_def.py, and hardens the status bar warning/error widget so multi-line messages don’t increase the status bar height.

Changes:

  • Update LaserAFConfig defaults to reference _def.py constants and extend tests to assert the defaults match _def.py.
  • Prevent status bar height growth by collapsing newlines in displayed messages and constraining the message label height.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
software/control/models/laser_af_config.py Swaps hardcoded Laser AF defaults for _def.py-driven defaults via default_factory.
software/tests/control/test_acquisition_config_models.py Updates the LaserAFConfig default test to validate against _def.py constants.
software/control/widgets.py Makes status bar message display single-line and height-stable when incoming log messages contain newlines.

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

default_factory=lambda: float(_def.LASER_AF_SPOT_SPACING), description="Expected spot spacing"
)
filter_sigma: Optional[float] = Field(
default_factory=lambda: _def.LASER_AF_FILTER_SIGMA, description="Gaussian filter sigma (-1 to disable)"
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

The field description says "Gaussian filter sigma (-1 to disable)", but _def.LASER_AF_FILTER_SIGMA is None and utils.find_spot_location() only applies filtering when filter_sigma is not None and filter_sigma > 0. Update the description (or the sentinel) so the docs match the actual disable behavior (e.g., "None or <= 0 to disable").

Suggested change
default_factory=lambda: _def.LASER_AF_FILTER_SIGMA, description="Gaussian filter sigma (-1 to disable)"
default_factory=lambda: _def.LASER_AF_FILTER_SIGMA,
description="Gaussian filter sigma (None or <= 0 to disable)",

Copilot uses AI. Check for mistakes.
Comment thread software/control/widgets.py Outdated
# Message text
# Message text – fixed height so newlines / long text can't grow the status bar
self.label_text = QLabel()
self.label_text.setFixedHeight(20)
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

Hard-coding setFixedHeight(20) can clip the message text when users run with larger fonts / DPI scaling (accessibility), since the widget height is now fixed regardless of font metrics. Consider computing the fixed height from QFontMetrics(self.label_text.font()).height() (plus margins) or tying it to self.label_icon.height() so the status bar stays single-line without truncating glyphs.

Suggested change
self.label_text.setFixedHeight(20)
# Compute height from font metrics and icon size to avoid clipping with larger fonts/DPI
text_height = self.label_text.fontMetrics().height()
icon_height = self.label_icon.sizeHint().height()
self.label_text.setFixedHeight(max(text_height, icon_height))

Copilot uses AI. Check for mistakes.
Alpaca233 and others added 2 commits March 19, 2026 23:48
…FConfig

- spot_detection_mode: remove incomplete enum list from description
- filter_sigma: fix description to say "None to disable" (not -1)
- Add 8 missing default_factory field assertions to test

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…mber

Addresses Copilot review comment: hardcoded setFixedHeight(20) could
clip text with larger fonts/DPI. Now computed from font metrics.
Also hide unimplemented MULTI_SECOND_RIGHT spot detection mode from UI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Alpaca233 Alpaca233 merged commit 36dc144 into Cephla-Lab:master Mar 20, 2026
2 checks passed
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