bluesky / ophyd

hardware abstraction in Python with an emphasis on EPICS
https://blueskyproject.io/ophyd
BSD 3-Clause "New" or "Revised" License
51 stars 79 forks source link

Potential issue with PVpositioner class not stopping/pausing the RE with failure of mechanical positioner #402

Open ambarb opened 7 years ago

ambarb commented 7 years ago

Defined the following classes. Two classes were chosen for the actuator in and out because there exists no single PV for global control of the positioner.

class LinearActOut(PVPositioner):
    readback = Cpt(EpicsSignalRO, 'Pos-Sts')
    setpoint = Cpt(EpicsSignal, 'Cmd:Out-Cmd')
    done = Cpt(EpicsSignalRO, 'Sw:OutLim-Sts')
    done_val = 0

class LinearActIn(PVPositioner):
    readback = Cpt(EpicsSignalRO, 'Pos-Sts')
    setpoint = Cpt(EpicsSignal, 'Cmd:In-Cmd')
    done = Cpt(EpicsSignalRO, 'Sw:InLim-Sts')
    done_val = 0

The reasoning for picking the limit switch as the done signal was so that the plan would only continue once the positioner is in place.

The device was defined as:

# FCCD slow shutter
ssh_in = LinearActIn('XF:23IDA-EPS{DP:1-Sh:1}', name='ssh_in')
ssh_out = LinearActOut('XF:23IDA-EPS{DP:1-Sh:1}', name='ssh_out')

And plans were created that did not include a while loop for checking the limits because it was thought bluesky and ophyd would check this for me:

def ssh_close():
    yield from bp.mv(ssh_in.setpoint,1)
    #while shchk():
    #    #raise Exception('Inconsistent shutter status!')

def ssh_open():
    yield from bp.mv(ssh_out.setpoint,1)
    #while not shchk():
        #raise Exception('Inconsistent shutter status!')

However, I wanted to test that I defined things correctly. Everything works when I do not create a failure condition. That is, with the actuator powered,'ssh_close() will put the actuator in. If howerver, I leave the limit switches connected but remove the power source from the actuator only, it is impoosible to put the actuator in.

Below is a plan output with timestamps and a CSS plot of the pertinent PVs. The normal condition was tested first (2x) and then the failure operating condition was tested (3x). During the failure condition, the RE completed without incident. Please advise.

Normal operating condition

See that the green PV (limit associated with the out position) does change to 0 as expected, as well as the orange PV (0 when actuator is in)..

In [3]: RE(pchain(ssh_open(),ct(num=2),ssh_close(),ct(num=2),ssh_open()))
Transient Scan ID: 79389 @ 2017/06/21 17:43:32
Persistent Unique Scan ID: '3f64dbcd-341d-4dff-bc17-966591bb4a9f'
+-----------+------------+
|   seq_num |       time |
+-----------+------------+
|         1 | 17:43:36.3 |
|         2 | 17:43:36.7 |
+-----------+------------+
generator ct ['3f64db'] (scan num: 79389)
Transient Scan ID: 79390 @ 2017/06/21 17:43:37
Persistent Unique Scan ID: '7f7bbe89-a86c-4962-ae6a-703e70a81ff7'
+-----------+------------+
|   seq_num |       time |
+-----------+------------+
|         1 | 17:43:39.6 |
|         2 | 17:43:39.9 |
+-----------+------------+
generator ct ['7f7bbe'] (scan num: 79390)
Out[3]: 
['3f64dbcd-341d-4dff-bc17-966591bb4a9f',
 '7f7bbe89-a86c-4962-ae6a-703e70a81ff7']

In [4]: RE(pchain(ssh_open(),ct(num=2),ssh_close(),ct(num=2),ssh_open()))
Transient Scan ID: 79391 @ 2017/06/21 17:43:47
Persistent Unique Scan ID: '31aa4cb6-97fd-4237-ab3c-3d5f736f39f1'
+-----------+------------+
|   seq_num |       time |
+-----------+------------+
|         1 | 17:43:49.9 |
|         2 | 17:43:50.4 |
+-----------+------------+
generator ct ['31aa4c'] (scan num: 79391)
Transient Scan ID: 79392 @ 2017/06/21 17:43:50
Persistent Unique Scan ID: 'fb10bc70-74c7-4f58-bdf5-9086fbadf6d6'
+-----------+------------+
|   seq_num |       time |
+-----------+------------+
|         1 | 17:43:53.8 |
|         2 | 17:43:54.4 |
+-----------+------------+
generator ct ['fb10bc'] (scan num: 79392)
Out[4]: 
['31aa4cb6-97fd-4237-ab3c-3d5f736f39f1',
 'fb10bc70-74c7-4f58-bdf5-9086fbadf6d6']

Failure condition

See that the green PV (limit associated with the out position) does not change to 0 as expected.

In [5]: RE(pchain(ssh_open(),ct(num=2),ssh_close(),ct(num=2),ssh_open()))
Transient Scan ID: 79393 @ 2017/06/21 17:44:58
Persistent Unique Scan ID: 'dece1d4b-8971-44fa-b242-9d87605124d5'
+-----------+------------+
|   seq_num |       time |
+-----------+------------+
|         1 | 17:45:00.7 |
|         2 | 17:45:01.2 |
+-----------+------------+
generator ct ['dece1d'] (scan num: 79393)
Transient Scan ID: 79394 @ 2017/06/21 17:45:01
Persistent Unique Scan ID: '456eaf59-e7d8-4f20-9076-e5358c04814a'
+-----------+------------+
|   seq_num |       time |
+-----------+------------+
|         1 | 17:45:04.6 |
|         2 | 17:45:05.1 |
+-----------+------------+
generator ct ['456eaf'] (scan num: 79394)
Out[5]: 
['dece1d4b-8971-44fa-b242-9d87605124d5',
 '456eaf59-e7d8-4f20-9076-e5358c04814a']

In [6]: RE(pchain(ssh_open(),ct(num=2),ssh_close(),ct(num=2),ssh_open()))
Transient Scan ID: 79395 @ 2017/06/21 17:45:12
Persistent Unique Scan ID: '9570673f-1311-4340-ae43-31a87bca9508'
+-----------+------------+
|   seq_num |       time |
+-----------+------------+
|         1 | 17:45:14.5 |
|         2 | 17:45:14.8 |
+-----------+------------+
generator ct ['957067'] (scan num: 79395)
Transient Scan ID: 79396 @ 2017/06/21 17:45:15
Persistent Unique Scan ID: '0bc604a2-b92e-4f43-bb9a-4f1ae58933f8'
+-----------+------------+
|   seq_num |       time |
+-----------+------------+
|         1 | 17:45:18.3 |
|         2 | 17:45:18.9 |
+-----------+------------+
generator ct ['0bc604'] (scan num: 79396)
Out[6]: 
['9570673f-1311-4340-ae43-31a87bca9508',
 '0bc604a2-b92e-4f43-bb9a-4f1ae58933f8']

In [7]: RE(pchain(ssh_open(),ct(num=2),ssh_close(),ct(num=2),ssh_open()))
Transient Scan ID: 79397 @ 2017/06/21 17:45:41
Persistent Unique Scan ID: '31990af7-cc00-48d7-b347-e61e36880088'
+-----------+------------+
|   seq_num |       time |
+-----------+------------+
|         1 | 17:45:44.4 |
|         2 | 17:45:44.6 |
+-----------+------------+
generator ct ['31990a'] (scan num: 79397) 
Transient Scan ID: 79398 @ 2017/06/21 17:45:45
Persistent Unique Scan ID: '04e4a993-0e4e-438f-88e1-231fc3c6f599'
+-----------+------------+
|   seq_num |       time |
+-----------+------------+
|         1 | 17:45:48.1 |
|         2 | 17:45:48.6 |
+-----------+------------+
generator ct ['04e4a9'] (scan num: 79398)
Out[7]: 
['31990af7-cc00-48d7-b347-e61e36880088',
 '04e4a993-0e4e-438f-88e1-231fc3c6f599']

pvpositioner_issue

ambarb commented 7 years ago

Some more useful information while the actuator is out (ssh_open())??


In [8]: ssh_in.done_val
Out[8]: 0

In [9]: ssh_in.done_value
Out[9]: 1

In [10]: ssh_in.done
Out[10]: EpicsSignalRO(read_pv='XF:23IDA-EPS{DP:1-Sh:1}Sw:InLim-Sts', name='ssh_in_done', parent='ssh_in', value=0, timestamp=1498081435.707882, pv_kw={}, auto_monitor=False, string=False)

In [11]: ssh_out.done_val
Out[11]: 0

In [12]: ssh_out.done_value
Out[12]: 1

In [13]: ssh_out.done
Out[13]: EpicsSignalRO(read_pv='XF:23IDA-EPS{DP:1-Sh:1}Sw:OutLim-Sts', name='ssh_out_done', parent='ssh_out', value=1, timestamp=1498081435.707885, pv_kw={}, auto_monitor=False, string=False)

Acturator not powered and left in the out position (open), with msg_hook.

In [16]: RE(ssh_open())
set: (EpicsSignal(read_pv='XF:23IDA-EPS{DP:1-Sh:1}Cmd:Out-Cmd', name='ssh_out_setpoint', parent='ssh_out', value=0, timestamp=1498081549.8369, pv_kw={}, auto_monitor=False, string=False, write_pv='XF:23IDA-EPS{DP:1-Sh:1}Cmd:Out-Cmd', limits=False, put_complete=False)), (1,), {'group': 'a0da8727-f529-4fc8-a1ac-59fa8ff37995'}
wait: (None), (), {'group': 'a0da8727-f529-4fc8-a1ac-59fa8ff37995'}
Out[16]: []

In [17]: RE(ssh_close())
set: (EpicsSignal(read_pv='XF:23IDA-EPS{DP:1-Sh:1}Cmd:In-Cmd', name='ssh_in_setpoint', parent='ssh_in', value=0, timestamp=1498081546.241438, pv_kw={}, auto_monitor=False, string=False, write_pv='XF:23IDA-EPS{DP:1-Sh:1}Cmd:In-Cmd', limits=False, put_complete=False)), (1,), {'group': '460e6172-e20b-425d-87fc-eab5830bf365'}
wait: (None), (), {'group': '460e6172-e20b-425d-87fc-eab5830bf365'}
Out[17]: []

In [18]: RE(ssh_open())
set: (EpicsSignal(read_pv='XF:23IDA-EPS{DP:1-Sh:1}Cmd:Out-Cmd', name='ssh_out_setpoint', parent='ssh_out', value=0, timestamp=1498082420.577823, pv_kw={}, auto_monitor=False, string=False, write_pv='XF:23IDA-EPS{DP:1-Sh:1}Cmd:Out-Cmd', limits=False, put_complete=False)), (1,), {'group': 'e6a0ebb7-7e2f-4b77-a47f-f595585366ea'}
wait: (None), (), {'group': 'e6a0ebb7-7e2f-4b77-a47f-f595585366ea'}
Out[18]: []
ambarb commented 7 years ago

Here is the work around using the same PVs, but not making more ophyd devices/signals.

def ssh_close():
    yield from bp.mv(ssh_in.setpoint,1)
    ##all of the below should  not be needed if ophyd functioned as it should.
    n=1
    while n is 1:
        print('Waiting for the slow shutter to close.')
        n=caget('XF:23IDA-EPS{DP:1-Sh:1}Sw:OutLim-Sts')
        yield from bp.sleep(0.3)

def ssh_open():
    yield from bp.mv(ssh_out.setpoint,1)
    n=1
    while n is 1:
        print('Waiting for the slow shutter to open.')
        n=caget('XF:23IDA-EPS{DP:1-Sh:1}Sw:InLim-Sts')
        yield from bp.sleep(0.3)

In [29]: RE(ssh_close())
set: (EpicsSignal(read_pv='XF:23IDA-EPS{DP:1-Sh:1}Cmd:In-Cmd', name='ssh_in_setpoint', parent='ssh_in', value=0, timestamp=1498083024.5683, pv_kw={}, auto_monitor=False, string=False, write_pv='XF:23IDA-EPS{DP:1-Sh:1}Cmd:In-Cmd', limits=False, put_complete=False)), (1,), {'group': '1f24a88d-3446-4fc7-827d-c9f3e2e55124'}
wait: (None), (), {'group': '1f24a88d-3446-4fc7-827d-c9f3e2e55124'}
sleep: (None), (0.3,), {}
Waiting for the slow shutter to close.
sleep: (None), (0.3,), {}
Waiting for the slow shutter to close.
sleep: (None), (0.3,), {}
Waiting for the slow shutter to close.
sleep: (None), (0.3,), {}
Waiting for the slow shutter to close.
sleep: (None), (0.3,), {}
Out[29]: []

In [30]: RE(ssh_open())
set: (EpicsSignal(read_pv='XF:23IDA-EPS{DP:1-Sh:1}Cmd:Out-Cmd', name='ssh_out_setpoint', parent='ssh_out', value=0, timestamp=1498083018.988245, pv_kw={}, auto_monitor=False, string=False, write_pv='XF:23IDA-EPS{DP:1-Sh:1}Cmd:Out-Cmd', limits=False, put_complete=False)), (1,), {'group': '22fac985-a583-4bda-b4b9-dd62ed784e1e'}
wait: (None), (), {'group': '22fac985-a583-4bda-b4b9-dd62ed784e1e'}
Waiting for the slow shutter to open.
sleep: (None), (0.3,), {}
Waiting for the slow shutter to open.
sleep: (None), (0.3,), {}
Waiting for the slow shutter to open.
sleep: (None), (0.3,), {}
Waiting for the slow shutter to open.
sleep: (None), (0.3,), {}
Waiting for the slow shutter to open.
sleep: (None), (0.3,), {}
Out[30]: []

In [31]: 
ambarb commented 7 years ago

Found that the following works better, which is probably what is intended. However, the run engine does not return if the you ask for the actuator to go in and it is already in. Please advise on how to fix this part.

RE(mv(ssh_in,1)) RE(mv(ssh_out,1))

tacaswell commented 7 years ago

Right, with the PVPositioner, you want to be moving the whole positioner, if you move just the setpoint then the higher logic is not triggered and you just change that PV value and reports when it is done changing).

The reason it does not work twice in a row is that there in never a message output by epics (because the limit switch value does not change values) so we wait forever for a message that will never come. Something like will short-circuit set logic in the case when we know it will be redundant.


class LinearActOut(PVPositioner):
    readback = Cpt(EpicsSignalRO, 'Pos-Sts')
    setpoint = Cpt(EpicsSignal, 'Cmd:Out-Cmd')
    done = Cpt(EpicsSignalRO, 'Sw:OutLim-Sts')
    done_val = 0

    def set(self, val):
        if self.done.get() == self.done_val:
            return DeviceStatus(self, done=True, success=True)
        return super().set(val) 

It is probably better to use a class like https://github.com/NSLS-II-XFP/profile_collection/blob/master/startup/10-motors.py#L8 which lets you do

yield from bp.mv(ssh, 'Open')
yield from bp.mv(sh, 'Close')

ps

This is a very clever class :+1:

ambarb commented 7 years ago

That is a neat trick with the twobuttonshutter class. I would have never found that on my own. I was going to get some help to make a universal open/close PV, but now I don’t have to!

We use the same structured PV for putting diodes and YAGs in the beam (not shutters), so I image multiple beam lines would benefit but might not find this because they aren’t looking for shutters but have something driven by the EPS.

Thanks a bunch!

From: Thomas A Caswell notifications@github.com<mailto:notifications@github.com> Reply-To: NSLS-II/ophyd reply@reply.github.com<mailto:reply@reply.github.com> Date: Wednesday, June 21, 2017 at 10:59 PM To: NSLS-II/ophyd ophyd@noreply.github.com<mailto:ophyd@noreply.github.com> Cc: Andi Barbour abarbour@bnl.gov<mailto:abarbour@bnl.gov>, Author author@noreply.github.com<mailto:author@noreply.github.com> Subject: Re: [NSLS-II/ophyd] Potential issue with PVpositioner class not stopping/pausing the RE with failure of mechanical positioner (#402)

Right, with the PVPositioner, you want to be moving the whole positioner, if you move just the setpoint then the higher logic is not triggered and you just change that PV value and reports when it is done changing).

The reason it does not work twice in a row is that there in never a message output by epics (because the limit switch value does not change values) so we wait forever for a message that will never come. Something like will short-circuit set logic in the case when we know it will be redundant.

class LinearActOut(PVPositioner): readback = Cpt(EpicsSignalRO, 'Pos-Sts') setpoint = Cpt(EpicsSignal, 'Cmd:Out-Cmd') done = Cpt(EpicsSignalRO, 'Sw:OutLim-Sts') done_val = 0

def set(self, val):
    if self.done.get() == self.done_val:
        return DeviceStatus(self, done=True, success=True)
    return super().set(val)

It is probably better to use a class like https://github.com/NSLS-II-XFP/profile_collection/blob/master/startup/10-motors.py#L8 which lets you do

yield from bp.mv(ssh, 'Open') yield from bp.mv(sh, 'Close')

ps

This is a very clever class 👍

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/NSLS-II/ophyd/issues/402#issuecomment-310262028, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AM_7GaPGd07mhTENNARyUJFMeEsxLh_jks5sGdivgaJpZM4OBkSH.

danielballan commented 7 years ago

As far as we know CSX is getting actual motor records so that EpicsMotor can be used instead of PVPositioners, as of tonight. If so this problem should go away. Is that right @ambarb?

ambarb commented 7 years ago

@danielballan

These things are not epics motor records and never will be. They are essentially a positioner that is in and out. The PV logic lives inside the EPS ioc so an EpicsMotor record is serious overkill. The way the EPS group programs these sorts of devices is to have a seperate PVs to insert or remove. For instance, it is not possible to use caput XF23{SomeEPSactuator:1}Cmd-In 0 to move the position to the out state. It looks like the same ioc model was exactly used for the shutter in the example @tacaswell shared from XFP. It could be modeled directly after the photons hutter. It looks like a PPS photon shutter, but I am not sure since I am not at XFP.

CSX-1 has 3 of these devices that were made by various people. I did ask Ruslan to change it so that all the suffixes (the bit after {}) are all the same so I could make a CSS template. I do not know if the EPS group has a standard in place now or not. Harman is probably the best person to ask.

You are right in that we have some other things that are PVpositioner (sy and sz) that need to be made into full EpicsMotor records.