Optionally use fastcs eiger for XRC #1436
Conversation
jacob720
added
i03
DominicOram
left a comment
DominicOram
left a comment
There was a problem hiding this comment.
Thanks, some comments in code. When you test it I suspect you might get issues with not calling complete and instead calling unstage (at
| @pydantic.dataclasses.dataclass(config={"arbitrary_types_allowed": True}) | ||
| class FlyScanEssentialDevices: | ||
| eiger: EigerDetector | ||
| eiger: EigerDetector | FastCSEiger |
There was a problem hiding this comment.
Must: Annoyingly I don't think this is going to work. The code in
will populate the composite based on the parameter name being the same as the device name and the type being the same. I think you will instead need to have a new device in this composite calledfastcs_eiger: FastEiger
There was a problem hiding this comment.
Ah ok, I wanted to avoid that for now as it would involve creating a fastcs eiger in the i04.py device factory, as the I04 XRC composite class inherits from FlyScanEssentialDevices. But maybe that isn't a problem, I've just created an I04 fastcs Eiger.
There was a problem hiding this comment.
Yh, we will want to test on i04 at some point anyway
| eiger.set_detector_parameters(parameters.detector_params) | ||
| eiger: EigerDetector | FastCSEiger = composite.eiger | ||
| if isinstance(eiger, FastCSEiger): | ||
| yield from configure_and_arm_detector( |
There was a problem hiding this comment.
Should: is there a dodal PR needed? I can't see this in main
There was a problem hiding this comment.
Yeah, will update the PR
DiamondLightSource/dodal#1706
| trigger=DetectorTrigger.EDGE_TRIGGER, | ||
| deadtime=0.0001, | ||
| ), | ||
| # group=data_collection_group |
There was a problem hiding this comment.
You want CONST.WAIT.GRID_READY_FOR_DC
jacob720
force-pushed
the
use_fastcs_eiger_for_xrc
branch
from
9efc7f7 to
045aa72
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1436 +/- ##
==========================================
- Coverage 92.26% 92.24% -0.03%
==========================================
Files 142 142
Lines 8031 8042 +11
==========================================
+ Hits 7410 7418 +8
- Misses 621 624 +3
🚀 New features to boost your workflow:
|
olliesilvester
left a comment
olliesilvester
left a comment
There was a problem hiding this comment.
Noticed a couple of things after briefly skimming
| gridscan: FastGridScanCommon, | ||
| detector: EigerDetector, # Once Eiger inherits from StandardDetector, use that type instead | ||
| detector: EigerDetector | ||
| | FastCSEiger, # use StandardDetector once old eiger not in use |
There was a problem hiding this comment.
I don't think we can actually use StandardDetector for the type here since we use dev_shm, which is specific to Odin. Maybe we can use StandardDetector[DetectorController, OdinWriter]
There was a problem hiding this comment.
That's good to know, could we just use FastCSEiger? How much more general is StandardDetector[DetectorController, OdinWriter] than FastCSEiger
There was a problem hiding this comment.
Well theoretically any standard detector which writes using odin could do the XRC plan, but I doubt we'll be doing this anytime soon. So yeah, can just use FastCSEiger for now
| ) | ||
|
|
||
|
|
||
| def kickoff_and_complete_gridscan( |
There was a problem hiding this comment.
The old eiger device waits for frames when we unstage. The fast-cs eiger will just immediately disarm on unstage.
For the fast-cs eiger we need to do yield from bps.complete instead - this will wait for frames with a default timeout, or the timeout specified in TriggerInfo
|
Waiting on testing time |
jacob720
force-pushed
the
use_fastcs_eiger_for_xrc
branch
from
5ae5d0d to
ee0f35a
Compare
jacob720
force-pushed
the
use_fastcs_eiger_for_xrc
branch
from
ac58072 to
080a87c
Compare
Fixes #1076
Requires bluesky/ophyd-async#1127 and DiamondLightSource/dodal#1706
Instructions to reviewer on how to test:
Checks for reviewer