1259 test oav plans#1292
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1292 +/- ##
==========================================
- Coverage 90.14% 90.02% -0.12%
==========================================
Files 130 132 +2
Lines 7834 7862 +28
==========================================
+ Hits 7062 7078 +16
- Misses 772 784 +12
🚀 New features to boost your workflow:
|
DominicOram
left a comment
DominicOram
left a comment
There was a problem hiding this comment.
Looks like a good start, thanks! If you merge main in it'll look a bit tidier. Mostly I think the asynchronous nature of bluesky/ophyd is what tripped you up here, put in a few more waits and it'll be all good.
| def feedback_check( | ||
| device: XBPMFeedback = inject("xbpm_feedback"), | ||
| ): | ||
| yield from bps.trigger(device) |
There was a problem hiding this comment.
Must: By default, trigger doesn't wait for the thing to finish. You want to do:
| yield from bps.trigger(device) | |
| yield from bps.trigger(device, wait=True) |
|
|
||
| # def oav_automation_test() | ||
| def feedback_check( | ||
| device: XBPMFeedback = inject("xbpm_feedback"), |
There was a problem hiding this comment.
Nit: device is generic. I would call it something more specific
| percentage: float, | ||
| attenuator: BinaryFilterAttenuator = inject("attenuator"), | ||
| ) -> MsgGenerator: | ||
| yield from bps.abs_set(attenuator, percentage / 100) |
There was a problem hiding this comment.
Should: As above, be careful with waiting for things to finish. You probably want this to only return when it's done so you should do:
| yield from bps.abs_set(attenuator, percentage / 100) | |
| yield from bps.abs_set(attenuator, percentage / 100, wait=True) |
or
| yield from bps.abs_set(attenuator, percentage / 100) | |
| yield from bps.mv(attenuator, percentage / 100) |
| yield from bps.abs_set(shutter.control_mode, ZebraShutterControl.MANUAL) | ||
| yield from bps.abs_set(shutter, shutter_state) | ||
|
|
||
| yield from bps.abs_set(shutter.control_mode, base_control_mode) |
There was a problem hiding this comment.
Should: As above, careful on waiting. I think you either want to add wait=True to all of these or change them to be mv instead
| group = "path setting" | ||
| yield from bps.abs_set(oav.grid_snapshot.filename, file_name, group=group) | ||
| yield from bps.abs_set(oav.grid_snapshot.directory, file_path, group=group) | ||
| yield from bps.wait(group) |
There was a problem hiding this comment.
Could: You can use mv to change multiple things at the same time if you want:
| group = "path setting" | |
| yield from bps.abs_set(oav.grid_snapshot.filename, file_name, group=group) | |
| yield from bps.abs_set(oav.grid_snapshot.directory, file_path, group=group) | |
| yield from bps.wait(group) | |
| yield from bps.mv(oav.grid_snapshot.filename, file_name, oav.grid_snapshot.directory, file_path) |
| yield from bps.abs_set(oav.grid_snapshot.filename, file_name, group=group) | ||
| yield from bps.abs_set(oav.grid_snapshot.directory, file_path, group=group) | ||
| yield from bps.wait(group) | ||
| yield from bps.trigger(oav.grid_snapshot) |
There was a problem hiding this comment.
Should: I think you want to use oav.snapshot both here and above when setting the filename. The grid_snapshot will try and draw a grid on top (something like https://ispyb.diamond.ac.uk/dc/visit/cm40607-4/dcg/16423891) which I don't think we need.
There was a problem hiding this comment.
Also, as above, need to add a wait=True
| def move_scintillator( | ||
| scintillator_state: InOut, scintillator: Scintillator = inject("scintillator") | ||
| ) -> MsgGenerator: | ||
| yield from bps.abs_set(scintillator.selected_pos, scintillator_state) |
There was a problem hiding this comment.
Should: As above, need a wait=True here or use mv
…x-bluesky into 1259_test_OAV_plans
|
This looks like it's covered by #1320 now, feel free to re-open if it isn't |
3 participants
Fixes #1259
Link to dodal PR (if required): #1563
(remember to update
pyproject.tomlwith the dodal commit tag if you need it for tests to pass!)Instructions to reviewer on how to test:
Checks for reviewer