-
Notifications
You must be signed in to change notification settings - Fork 5
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks mostly good, but the main comments:
-
I don't think we need a separate pulse block. I also think we can combine the 'wait for position and send one pulse' and 'send all other pulses' steps.
-
I know this was me originally, but I think we should try making the deadtime equal to what it says in the eiger manual, 100 ns
-
I haven't looked at the yaml file in massive detail, but we should make sure the position part of this sequencer table is ignored now, like in
/dls_sw/i03/software/daq_configuration/panda_configs/flyscan_pcap_ignore_seq.yaml
. This was previously causing a race condition in the panda ioc, which has apparently been fixed now, but still better to be safe.
3. Send out the remaining x_steps - 1 triggers every time_between_steps_ms | ||
4. Wait for physical trigger from motion script to mark change of direction | ||
5. Wait for POSA (X2) to be less than X_START + X_STEP_SIZE * x_steps + exposure distance, then | ||
send 1 trigger | ||
6. Send the remaining x_steps - 1 triggers every time_between_steps_ms | ||
7. Go back to step one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3. Send out the remaining x_steps - 1 triggers every time_between_steps_ms | |
4. Wait for physical trigger from motion script to mark change of direction | |
5. Wait for POSA (X2) to be less than X_START + X_STEP_SIZE * x_steps + exposure distance, then | |
send 1 trigger | |
6. Send the remaining x_steps - 1 triggers every time_between_steps_ms | |
7. Go back to step one. | |
3. Send out the remaining x_steps - 1 triggers every time_between_steps_ms, then stop sending triggers | |
4. Wait for physical trigger from motion script to mark change of direction | |
5. Wait for POSA (X2) to be less than X_START + X_STEP_SIZE * x_steps + exposure distance, then | |
send 1 trigger | |
6. Send the remaining x_steps - 1 triggers every time_between_steps_ms | |
7. Go back to step one. |
…internally rather than using clock generator. * Add saved new panda layout inc. screehshot. * Update unit tests
bd58549
to
41a6bec
Compare
Have simplified the sequence table but not removed the pulse generator or reduced the deadtime as discussed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is in a testable state for tomorrow! I have a few more comments, but they're mostly nits
7:Go back to step one. | ||
|
||
1. Wait for physical trigger from motion script to mark start of scan / change of direction | ||
2. Wait for POSA (X2) to be greater than X_START and send x_steps triggers every time_between_steps_ms |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
2. Wait for POSA (X2) to be greater than X_START and send x_steps triggers every time_between_steps_ms | |
2. Wait for POSA (X2) to be greater than X_START and send triggers every time_between_steps_ms, until x_steps triggers have been sent |
@@ -127,7 +135,6 @@ def setup_panda_for_flyscan( | |||
|
|||
Args: | |||
panda (HDFPanda): The PandA Ophyd device | |||
config_yaml_path (str): Path to the yaml file containing the desired PandA PVs | |||
parameters (PandAGridScanParams): Grid parameters | |||
initial_x (float): Motor positions at time of PandA setup | |||
exposure_time_s (float): Detector exposure time per trigger |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing sample_velocity_mm_per_s
here
@@ -139,7 +146,14 @@ def setup_panda_for_flyscan( | |||
Yields: | |||
Iterator[MsgGenerator] | |||
""" | |||
yield from load_device(panda, config_yaml_path) | |||
assert parameters.x_steps > 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assert parameters.x_steps > 0 | |
assert parameters.x_steps > 0, "panda grid scan must have x_steps > 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to merge now I think, after that missing docstring is added
* (DiamondLightSource/hyperion#1474) Configure panda sequencer table to generate trigger sequence internally rather than using clock generator. * Add saved new panda layout inc. screehshot. * Update unit tests * (DiamondLightSource/hyperion#1474) move save-panda to dodal * (DiamondLightSource/hyperion#1474) Tidy up documentation * Make pyright happy * Remove redundant comment from tests * (DiamondLightSource/hyperion#1474) load the .yaml device layout as a module resource * Fix CI unit test failure due to bluesky event loop not running * Simplify the sequencer table as per PR comment * Update docstring
Fixes #1474
Corresponding save-panda cli utility:
Link to dodal PR (if required): DiamondLightSource/dodal#702
To test: