[S30-134][feat] grating offset calibration small improvements - #3545
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe alignment routine now uses horizontal binning 1 during detector alignment, compensates exposure time, and restores detector values during cleanup. The simulator peak center calculation and alignment tests were updated. The SPARC2 alignment tab now limits grating calibration modes, disables conflicting controls during calibration, manages the lens-view crosshair, and restores the interface after completion. The grating calibration panel and button labels were renamed and updated. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR refines the SPARC2 grating offset auto-calibration workflow by improving the alignment tab UI labels/visibility, tightening GUI enable/disable behavior during automated procedures, and improving the simulated/algorithmic alignment behavior and tests.
Changes:
- Renames and rewords the grating calibration panel/button labels/tooltips in the XRC resources and generated
main_xrc.py. - Updates SPARC2 align tab controller to show/hide the grating panel per mode and to disable controls during acquisition/calibration.
- Improves grating offset alignment robustness (binning/exposure handling + restoration) and updates simulation centering + alignment tests.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/odemis/gui/xmlh/resources/panel_tab_sparc2_align.xrc |
UI text updates and panel renaming for grating calibration section. |
src/odemis/gui/main_xrc.py |
Regenerated XRC bindings to match renamed panel and updated UI strings. |
src/odemis/gui/cont/tabs/sparc2_align_tab.py |
Calibration UX/control-state improvements; grating panel visibility changes; acquisition-state handling. |
src/odemis/driver/simcam.py |
Adjusts simulated CCD “center” calculation to pixel-index center. |
src/odemis/acq/align/test/goffset_test.py |
Test adjustments for simulator behavior and misalignment setup. |
src/odemis/acq/align/goffset.py |
Adds detector binning normalization during alignment and restores prior detector state afterward. |
Suppressed comments (1)
src/odemis/gui/cont/tabs/sparc2_align_tab.py:1646
- If you defer forcing the crosshair until calibration actually starts, it should be enabled after the calibration future is created (so early returns don’t leave it stuck on).
# Bind progress & done callbacks
self._grating_calibration_future.add_done_callback(self._on_grating_calibration_done)
self.panel.btn_auto_grating_center.SetLabel("Cancel")
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
pieleric
changed the title
6cd96fa to
491bde5
Compare
The horizontal reoslution is important when looking for the closest position near the center. So the CCD should have binning = 1, horizontally, always.
* Rename "auto center" to "auto calib", to clarify what will happen. * Adjust the name of the button from pnl_calibrate to pnl_grating, to make it clearer. * Disable the rest of the tab while the calibration is running * Only allow to run the calibration procedure in LENS alignment mode. * Show the cross-hair while it's running.
Fix the computation of the center of the CCD in the simulator. It was shifted by 0.5 px, causing the "good" offset to actually not be so good. This triggered the failure of test_scale_not_misaligned. Also adjust the test_scale_not_misaligned to only concider the failure if offset is more than 1px off. It was using 1e-6, which is over the top in precision, especially as the simulator has noise. test_single_detector_updates_grating would fail if the previous tests were run, because the grating would be already calibrated, and so the test would say that nothing has happened. => mis-align the grating before running the calibration to make sure that something should happen.
491bde5 to
374c70b
Compare
Force the CCD binning to 1 px horizontally, to ensure the best precision.
Improve the SPARC GUI alignment tab:
it clearer.
Also fix some test cases due to issue with the simulator.