Repository navigation
Both plans now determine scan type based on ScanType enum. Tomography adds demo_tomography - #54
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Existing callers remain incompatible, the step-scan assertion expects incorrect dimensions, and velocity validation ignores custom trajectories.
3 open findings
What changed in this PR
Updates spectroscopy scans to use an explicit scan-type enum supplied by the frontend instead of automatic mode selection.
Changes:
- Adds
ScanTypeand requires it in the spectroscopy plan. - Consolidates demo grid parameters into spectroscopy and adds fly-scan velocity validation.
- Adds system tests for explicit step and fly scans.
| File | Description |
|---|---|
tests/system_tests/test_plans_system.py |
Adds explicit scan-mode system tests. |
src/test_rig_bluesky/spectroscopy_plans.py |
Consolidates plans and introduces enum-based scan selection. |
src/test_rig_bluesky/plans.py |
Defines the shared ScanType enum. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| *, | ||
| scan_type: ScanType, |
| xsteps = int(round(math.sqrt(total_number_of_scan_points))) | ||
| velocity = grid_size / (xsteps * exposure_time) |
There was a problem hiding this comment.
I see what copilot is saying, We have moved the guard from demo_spectroscopy onto spectroscopy, but technically this guard assumes a square grid to calculate max velocity which isn't necessarily true if you called a different spec. So by adding this guard here we are not allowing legitimate fly scans to happen here, so i think we may need to make a more intelligent guard. I am not sure how likely we are to be calling scans not from the UI currently?
| timeout=20, | ||
| ) | ||
|
|
||
| assert events["FINISHED"][0]["scanDimensions"] == [5] |
| "Performing a fly scan. Estimated velocity: %.2f mm/s.", | ||
| velocity, | ||
| ) | ||
| else: |
There was a problem hiding this comment.
I think i would prefer the explicit, and still exhaustive
if scan_type == ScanType.STEP:
to be clear
|
For now I’d suggest we drop the hard velocity guard from spectroscopy and only log a warning. The current check assumes the square grid the UI builds, so it can wrongly reject a valid fly scan when a different spec is passed in. ophyd-async already enforces each motor’s VMAX when it builds the pmac trajectory during prepare, so an unsafe scan will still be refused. We can add a true velocity guard after building the actual scan spec later. |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #54 +/- ##
==========================================
+ Coverage 48.35% 50.70% +2.34%
==========================================
Files 5 5
Lines 213 213
==========================================
+ Hits 103 108 +5
+ Misses 110 105 -5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|


updates plan to determine scan type based on enum choice passed from the UI frontend with accompanying tests.
The scope of this PR has simplified - changes: