Aithre lasershaping #910
Aithre lasershaping #910
Conversation
Change stages to goniometer
…s at different speeds
There was a problem hiding this comment.
So the different files used in configuring the OAV are confusing and horrible and a bit of a hold over from GDA, I have written #911 to document them.
I think the long and short of it though is that you only need them if your beam centre/microns per pixel changes with zoom so in your case (I assume) you don't. With the changes I have suggested in the code I can do:
from bluesky import plan_stubs as bps
from bluesky.run_engine import RunEngine
from ophyd_async.core import DeviceCollector
RE = RunEngine()
with DeviceCollector():
my_oav = OAV("BL03I-DI-OAV-01:", "my_oav")
def take_an_image():
yield from bps.mv(my_oav.snapshot.directory, "/scratch/")
yield from bps.mv(my_oav.snapshot.filename, "test_image")
yield from bps.trigger(my_oav.snapshot, wait=True)
RE(take_an_image())and I get the snapshot I expect (from i03's OAV).
| "beam_centre_y": 1028, | ||
| } | ||
|
|
||
| class OAV(StandardReadable) |
There was a problem hiding this comment.
There's a missing colon here, you can tell at first glance in github because the __init__ below it has no highlighting unlike other functions
| class OAV(StandardReadable) | |
| class OAV(StandardReadable): |
| def __init__(self, prefix: str, config: OAVConfig, name: str = ""): | ||
| self.oav_config = config |
There was a problem hiding this comment.
OAVConfig is mainly there to handle how beam centre/microns per pixel changes with zoom. If we don't have a zoom controller here then I'm guessing these are fixed, in which case we can just remove it.
| def __init__(self, prefix: str, config: OAVConfig, name: str = ""): | |
| self.oav_config = config | |
| def __init__(self, prefix: str, name: str = ""): |
| self.microns_per_pixel_x = laserOAVconfig["microns_per_pixel_x"] | ||
| self.microns_per_pixel_y = laserOAVconfig["microns_per_pixel_y"] | ||
| self.beam_centre_x = create_hardware_backed_soft_signal(int, lambda: self._get_beam_position("X")) | ||
| self.beam_centre_y = create_hardware_backed_soft_signal(int, lambda: self._get_beam_position("Y")) |
There was a problem hiding this comment.
In the other OAV we have these as signals as they may change. Signals let you easily monitor things. In the case of a fixed microns/pixel and beam centre they don't have to be signals but I think it's nicer for consistency. Making fixed signals is simpler, you don't need the create_hardware_backed_soft_signal stuff and so can remove _get_beam_position below too:
from ophyd_async.core import soft_signal_r_and_setter
self.microns_per_pixel_x, _ = soft_signal_r_and_setter(float, laserOAVconfig["microns_per_pixel_x"])
self.microns_per_pixel_y, _ = soft_signal_r_and_setter(float, laserOAVconfig["microns_per_pixel_x"])
self.beam_centre_x, _ = soft_signal_r_and_setter(float, laserOAVconfig["beam_centre_x"])
self.beam_centre_y, _ = soft_signal_r_and_setter(float, laserOAVconfig["beam_centre_y"])|
|
||
|
|
||
| # LA18L-DI-OAV-01: is prefix for lab 18 OAV laser shaping | ||
| # not sure if I need to use this or just create a new json and use the standard |
There was a problem hiding this comment.
I think removing the zoom reduces the complexity so much that it makes sense to rewrite it like this. We could probably do some more code re-use where this and the other OAV have a shared parent class or something but we can leave that to another day once it's working.
- rename oav parameters to include file extension - typos etc.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #910 +/- ##
==========================================
- Coverage 95.70% 93.21% -2.49%
==========================================
Files 132 140 +8
Lines 5334 5810 +476
==========================================
+ Hits 5105 5416 +311
- Misses 229 394 +165 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
PR for laser shaping work, makes it easier for people to comment on the code
Checks for reviewer
dodal connect ${BEAMLINE}