[S30-134][feat] grating offset calibration small improvements - #3545
[S30-134][feat] grating offset calibration small improvements#3545pieleric wants to merge 3 commits into
Conversation
The horizontal reoslution is important when looking for the closest position near the center. So the CCD should have binning = 1, horizontally, always.
|
Warning Review limit reached
Next review available in: 34 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
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.
| restore_attrs = {} | ||
| if not model.hasVA(detector, "binning") or detector.binning.value[0] == 1: | ||
| return restore_attrs |
| # Show the cross-hair, at the middle, to confirm the alignment is correct. | ||
| self.panel.vp_align_lens.canvas.fit_view_to_content() | ||
| self._grating_calib_prev_crosshair = self.panel.vp_align_lens.view.show_crosshair.value | ||
| self.panel.vp_align_lens.view.show_crosshair.value = True |
pieleric
changed the title
* 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.
6cd96fa to
491bde5
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.