Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add soft signals with required metadata for Synchrotron and TetrAMM devices #591

Merged
merged 6 commits into from
Jun 17, 2024

Conversation

DiamondJoseph
Copy link
Contributor

Fixes issues with required fields for NeXus writing for i22.

Instructions to reviewer on how to test:

  1. Smoke test i22, p38 to ensure changes have not broken those beamlines
  2. Confirm that Hyperion is only using the SynchrotronMode signal from the Synchrotron and that still propagates as required

Checks for reviewer

  • Would the PR title make sense to a scientist on a set of release notes
  • If a new device has been added does it follow the standards
  • If changing the API for a pre-existing device, ensure that any beamlines using this device have updated their Bluesky plans accordingly
  • Have the connection tests for the relevant beamline(s) been run via dodal connect ${BEAMLINE}

@DiamondJoseph
Copy link
Contributor Author

Tagging @dperl-dls and @DominicOram: in Hyperion I could only find use of the synchrotron-mode signal, which remains propagated in the ConfigSignal: I'd like to take a better look at making a proper mode signal later on. I've left the top_up signals (renamed for consistency) for later use with a Suspender (I'm making an issue for one right now)

src/dodal/devices/tetramm.py Outdated Show resolved Hide resolved
src/dodal/beamlines/i22.py Outdated Show resolved Hide resolved
@DominicOram
Copy link
Contributor

I've left the top_up signals (renamed for consistency) for later use with a Suspender (I'm making an issue for one right now)

The topup signals are also used in the wait for topup plans (see failing tests). As such hyperion also mocks out topup_start_countdown for some of its tests. It could be argued that hyperion should be actually mocking the top_up plans though. Either way, I'm happy to make the required hyperion changes if you can fix the top up plans

@DiamondJoseph DiamondJoseph merged commit a37b9bc into main Jun 17, 2024
11 checks passed
@DiamondJoseph DiamondJoseph deleted the synchrotron_soft_signals branch June 17, 2024 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants