Skip to content
This repository has been archived by the owner on Sep 2, 2024. It is now read-only.

349 Create OAV snapshot plan #1437

Merged
merged 4 commits into from
Jun 13, 2024
Merged

349 Create OAV snapshot plan #1437

merged 4 commits into from
Jun 13, 2024

Conversation

rtuck99
Copy link
Contributor

@rtuck99 rtuck99 commented Jun 11, 2024

Part 1 of fix for DiamondLightSource/mx-bluesky#388

This creates an experiment plan for taking oav snapshots and also removes some unused parameters from the ispyb_extras parameters. ispyb_extras is now no longer needed for grid scans. OAV snapshots for rotations are still taken by GDA and snapshot paths supplied in parameters; this new plan is not yet invoked by the rotation plan, that work will be in a subsequent PR.

Link to dodal PR (if required): #N/A
(remember to update setup.cfg with the dodal commit tag if you need it for tests to pass!)

To test:

  1. Tests pass

@rtuck99 rtuck99 changed the base branch from main to change_ispyb_dev_to_ispyb_restore June 11, 2024 10:03
composite: OavSnapshotComposite,
parameters: WithSnapshot,
oav_parameters: OAVParameters,
wait: bool = True,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wait parameter is here as requested in original issue, but currently unused since we have to wait for the snapshot trigger completion before firing the event. Am open to suggestions to remove it / alternate structuring to make it useful.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it is we can remove this, I agree. If we need to do something more complex async we'll need to do it at device level anyway.

@rtuck99 rtuck99 requested review from d-perl and DominicOram and removed request for d-perl June 11, 2024 11:05
@rtuck99 rtuck99 marked this pull request as ready for review June 11, 2024 11:05
@rtuck99 rtuck99 force-pushed the 349_create_oav_snapshot_plan branch from 486d71e to eed8d74 Compare June 12, 2024 14:43
@rtuck99 rtuck99 changed the base branch from change_ispyb_dev_to_ispyb_restore to main June 12, 2024 14:45
Copy link
Contributor

@d-perl d-perl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks! I agree about removing the unused param, I'll leave it to you to merge after

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

composite: OavSnapshotComposite,
parameters: WithSnapshot,
oav_parameters: OAVParameters,
wait: bool = True,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it is we can remove this, I agree. If we need to do something more complex async we'll need to do it at device level anyway.

@rtuck99 rtuck99 merged commit 1006193 into main Jun 13, 2024
6 checks passed
@rtuck99 rtuck99 deleted the 349_create_oav_snapshot_plan branch June 13, 2024 11:59
olliesilvester pushed a commit to olliesilvester/mx-bluesky that referenced this pull request Aug 23, 2024
* (DiamondLightSource/hyperion#349) Basic experiment plan for taking snapshots
* Remove xtal_snapshots and xtal_snapshots_omega_end from ispyb_extras
* (DiamondLightSource/hyperion#349) Tidy parameter model diagram and commit image
* (DiamondLightSource/hyperion#349) tidyups
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants