You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Currently not all beamlines have adopted device_instantiation. It would be neat if all devices were initialized in the same way,
As I understand it the requirements are
Invoke a function that makes a specific device (for a specific beamline)
Can be configured to either perform or skip the step of ensuring all PVs are connected
Can be configured to make alternative devices based on profiles (e.g. dummy objects, S03)
Store the device as a singleton in a backing store somewhere
Should be possible to be lazy (make devices as-needed) or eager (make all devices at once for a particular beamline)
Should be introspectable so that something like blueapi can ask "what devices exist on beamline x?"
All above should be configurable globally and then individually
Should be maintainable, changes should not break all previous devices
An example of the current solution used on I03:
@skip_device(lambda: BL=="s03")defeiger(
wait_for_connection: bool=True,
fake_with_ophyd_sim: bool=False,
params: Optional[DetectorParams] =None,
) ->EigerDetector:
"""Get the i03 Eiger device, instantiate it if it hasn't already been. If this is called when already instantiated in i03, it will return the existing object. If called with params, will update those params to the Eiger object. """defset_params(eiger: EigerDetector):
ifparamsisnotNone:
eiger.set_detector_parameters(params)
returndevice_instantiation(
device_factory=EigerDetector,
name="eiger",
prefix="-EA-EIGER-01:",
wait=wait_for_connection,
fake=fake_with_ophyd_sim,
post_create=set_params,
)
Which can then be invoked like so:
fromdodal.beamlines.i03importeigerdefmy_plan():
eiger=eiger() # Only makes a new Eiger if none exists, otherwise uses cached versionyieldfromdo_stuff_with(eiger)
I think this is moving in the right direction and I'm somewhat happy to move the test rigs to this model, however I have a couple of concerns that would be good to address sooner rather than later. Firstly I'm worried about maintaining the boilerplate: The set of parameters to device_instantiation cannot really change now as it will be called by every single device factory. Second I feel this is a bit obfuscating, in the above example I would quite like to see just the logic of how to make an eiger, and all the logic of making a fake eiger, storing it in a backing store etc. being abstracted somewhere else (with an override in case any unusual logic is required). I'm wondering if a decorator would be a better fit, something like
@device(wait_for_connection=True)defeiger(profile: str, params: Optional[DetectorParams] =None) ->EigerDetector:
pv_prefix=i03_prefix(profile) # either i03 or s03eiger=EigerDetector(name="eiger", prefix=f"{pv_prefix}-EA-EIGER-01:")
ifparamsisnotNone:
eiger.set_detector_parameters(params)
The decorator handles all of the boilerplate seen earlier. It's signature is still not that flexible but is at least no longer tied to the factory signature. There would be default routines for making a fake version and storing the singleton somewhere that could be optionally overridden.
Now is probably also a good time to make sure the design works well with both Ophyd v1 and v2. For example, they both handle device names a bit differently. Will make a separate issue for that.
All thoughts on the above welcome. Paging @DominicOram and @dperl-dls. If there's appetite for the decorator concept I'm happy to have a go at it for the test rigs and see if it looks like something we can use.
The text was updated successfully, but these errors were encountered:
Option discussed with @coretl in #154 is to make a new device context manager that works with both v1 and v2 devices, example usage would be something like:
Decision here is to migrate all devices to ophyd-async and replace device_instantiation with decorators. Also add a "profile" mechanism to cover multiple groups of devices being instantiated. There should still be a make_all_devices function of some sort either in dodal or blueapi.
Seems like most of the relevant discussion for this is actually in #415 (comment) now so I'm closing this but feel free to reopen if you think the separate issues are still useful
Currently not all beamlines have adopted
device_instantiation
. It would be neat if all devices were initialized in the same way,As I understand it the requirements are
An example of the current solution used on I03:
Which can then be invoked like so:
I think this is moving in the right direction and I'm somewhat happy to move the test rigs to this model, however I have a couple of concerns that would be good to address sooner rather than later. Firstly I'm worried about maintaining the boilerplate: The set of parameters to
device_instantiation
cannot really change now as it will be called by every single device factory. Second I feel this is a bit obfuscating, in the above example I would quite like to see just the logic of how to make an eiger, and all the logic of making a fake eiger, storing it in a backing store etc. being abstracted somewhere else (with an override in case any unusual logic is required). I'm wondering if a decorator would be a better fit, something likeThe decorator handles all of the boilerplate seen earlier. It's signature is still not that flexible but is at least no longer tied to the factory signature. There would be default routines for making a fake version and storing the singleton somewhere that could be optionally overridden.
Now is probably also a good time to make sure the design works well with both Ophyd v1 and v2. For example, they both handle device names a bit differently. Will make a separate issue for that.
All thoughts on the above welcome. Paging @DominicOram and @dperl-dls. If there's appetite for the decorator concept I'm happy to have a go at it for the test rigs and see if it looks like something we can use.
The text was updated successfully, but these errors were encountered: