Skip to content

Add shutter device for i19#1023

Merged
noemifrisina merged 30 commits intomainfrom
1020_hutch-shutter-i19
Feb 6, 2025
Merged

Add shutter device for i19#1023
noemifrisina merged 30 commits intomainfrom
1020_hutch-shutter-i19

Conversation

@noemifrisina
Copy link
Collaborator

Fixes #1020

Instructions to reviewer on how to test:

  1. Check that dodal connect i19-1 and i19-2 still works
  2. Run tests

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}

@codecov
Copy link

codecov bot commented Jan 27, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.67%. Comparing base (3f6af7a) to head (d42a6fa).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1023      +/-   ##
==========================================
+ Coverage   97.66%   97.67%   +0.01%     
==========================================
  Files         159      160       +1     
  Lines        6560     6592      +32     
==========================================
+ Hits         6407     6439      +32     
  Misses        153      153              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@noemifrisina noemifrisina marked this pull request as ready for review January 28, 2025 09:54
@noemifrisina noemifrisina requested a review from a team as a code owner January 28, 2025 09:54
Copy link
Contributor

@DominicOram DominicOram left a comment

Choose a reason for hiding this comment

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

Thanks! Some comments but no showstoppers

Comment on lines +50 to +53
# NOTE Warn but don't fail
LOGGER.warning(
f"{self.hutch_request} is not the hutch in use. Shutter will not be operated."
)
Copy link
Contributor

Choose a reason for hiding this comment

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

So this is a warning not an error so that you can test plans on the beamline whilst the other shutter is in use? I think this is probably another use case for #754?

Could: Maybe we should have a flag called TEST_MODE where it will fail here loudly if not in that mode? Like the undulator does

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, definitely a use case. Although I would add that the general HutchShutter device should also have a TEST_MODE flag to be able to still use code in order to be able to test on machine days - run into this problem testing last week on i24, and it would be applicable to I19 as well, independently of the hutch in use...

def __init__(self, prefix: str, hutch: HutchState, name: str = "") -> None:
self.shutter = HutchShutter(prefix=prefix, name=name)
bl_prefix = prefix.split("-")[0]
self.hutch_state = epics_signal_r(str, f"{bl_prefix}-OP-STAT-01:EHStatus.VALA")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should: This should be of type HutchState

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tried that in the first instance, but unfortunately the actual PV is not an Enum and it wouldn't connect...

@AsyncStatus.wrap
async def set(self, value: ShutterDemand):
hutch_in_use = await self.hutch_state.get_value()
LOGGER.info(f"Current hutch in use: {hutch_in_use}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Could: Do we know what invalid means? Is there a way to get out of the invalid state? If we know what action needs to be taken sometimes it's useful to put it in e.g. "Contact X group"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So far I have only seen "INVALID" when the beampipe between the two hutches is not correctly placed, although I am sure there may be more. I can add a message for that for now and add to it as I find the use cases

@noemifrisina noemifrisina merged commit 5672e55 into main Feb 6, 2025
19 checks passed
@noemifrisina noemifrisina deleted the 1020_hutch-shutter-i19 branch February 6, 2025 16:48
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.

Hutch-dependant shutter device for I19

2 participants