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

Make PSS Shutter devices more flexible for different beamlines #838

Closed
canismarko opened this issue Apr 5, 2023 · 5 comments · Fixed by #857
Closed

Make PSS Shutter devices more flexible for different beamlines #838

canismarko opened this issue Apr 5, 2023 · 5 comments · Fixed by #857
Labels
Milestone

Comments

@canismarko
Copy link
Collaborator

The ApsPssShutter does not match our beamline configuration (25-ID).

The open signal is created as {PREFIX}_Open but should be {PREFIX}_OPEN_EPICS to work on our hardware. The close signal is the same (but with "Close", etc.).

Here is the subclass of ApsPssShutterWithStatus that matches our hardware: https://github.com/spc-group/haven/blob/main/haven/instrument/shutter.py

@canismarko canismarko added the bug label Apr 5, 2023
@prjemian prjemian added this to the 1.6.16 milestone Apr 5, 2023
@prjemian
Copy link
Contributor

Also, there are no unit tests on the shutter classes.

@prjemian
Copy link
Contributor

This code should become more flexible:

open_signal = Component(EpicsSignal, "Open")
close_signal = Component(EpicsSignal, "Close")

@prjemian
Copy link
Contributor

@canismarko - Looking at the haven code, this looks like an unusual API signature. I'm expecting prefix to be the first argument, such as:

def __init__(self, prefix, state_pv, *args, **kwargs):

I plan to make the API here similar, allowing for close_pv and open_pv as kwargs.

@prjemian
Copy link
Contributor

As in:

class ApsPssShutter(ShutterBase):

    # ...

    open_signal = FormattedComponent(EpicsSignal, "{self.open_pv}")
    close_signal = FormattedComponent(EpicsSignal, "{self.close_pv}")

    def __init__(self, prefix, *args, close_pv=None, open_pv=None, **kwargs):
        self.open_pv = open_pv or f"{prefix}Open"
        self.close_pv = close_pv or f"{prefix}Close"
        # TODO" ?? what about prefix if open_pv and/or close_pv are not None ??
        super().__init__(prefix, *args, **kwargs)

    # ...

The ApsPssShutterWithStatus() class does not need changes since it passes close_pv and open_pv as unnamed kwargs to the super() class.

prjemian added a commit that referenced this issue May 17, 2023
prjemian added a commit that referenced this issue May 17, 2023
prjemian added a commit that referenced this issue May 17, 2023
prjemian added a commit that referenced this issue May 17, 2023
prjemian added a commit that referenced this issue May 17, 2023
@prjemian
Copy link
Contributor

Shutter might be in unknown state. CI failed for that:

/home/runner/work/apstools/apstools/apstools/devices/tests/test_shutters.py:96: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

shutter = EpicsMotorShutter(prefix='gp:m16', name='shutter', read_attrs=['busy', 'signal', 'signal.user_readback', 'signal.user_...signal', 'signal.user_offset', 'signal.user_offset_dir', 'signal.velocity', 'signal.acceleration', 'signal.motor_egu'])

    def operate_shutter(shutter):
        shutter.open()
        timed_pause()
>       assert shutter.state == "open"
E       AssertionError: assert 'unknown' == 'open'
E         - open
E         + unknown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants