Skip to content

Add a Valve class #245

Draft
Jennefer Maldonado (jennmald) wants to merge 4 commits intoNSLS2:mainfrom
jennmald:valve
Draft

Add a Valve class #245
Jennefer Maldonado (jennmald) wants to merge 4 commits intoNSLS2:mainfrom
jennmald:valve

Conversation

@jennmald
Copy link
Copy Markdown
Collaborator

Not sure if this is too much of a naive approach to the fix we did at SMI with Eliot.
Adds stop function to help with valve functionality.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a new Valve device type in nslsii/devices.py, intended to support “valve functionality” by adding a stop() hook similar to the shutter behavior.

Changes:

  • Adds a new Valve class inheriting from TwoButtonShutter.
  • Introduces a Valve.stop() method placeholder.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread nslsii/devices.py Outdated
@tacaswell
Copy link
Copy Markdown
Contributor

By my count we have a version of this at HEX, ISS, SRX, QAS, CHX, SIX, and LIX

Comment thread nslsii/devices.py Outdated
self._set_st = None
self.read_attrs = ['status']

class Valve(TwoButtonShutter):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

At least at SRX we are (were?) using this for the a photon shutter, I think we need a better name.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

so something that captures, valves + photon shutters?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The interesting property is that "does not close on stop", putting more in a normal comment.

@tacaswell
Copy link
Copy Markdown
Contributor

TwoButtonShutter is a bad name that I'm pretty sure I came up with live on a beamline with someone looking over my shoulder as getting this to work was the blocking issue to getting an experiment working. We think we know all of the users of this code (as far as we know it is only used on our floor) so we can be a bit aggressive with changing things.

I would propose the following names

class TwoButtonActuator:  # no stop
class TwoButtonActuatorAutoClose(TwoButtonActuator):  # the current class

class TwoButtonShutter(TwoButtonActuatorAutoClose):  # empty now, switch to warning next fall, remove next spring

Comment thread nslsii/devices.py


class TwoButtonActuatorAutoClose(TwoButtonActuator):
def stop(self, *, success=False):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This now does not close despite the name.

The default stop method is a no-op so we should move the stop from the other class to this one.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I am not sure if I just made it worse.

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