Skip to content
Open
Changes from 11 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
7a4946f
feat(shutters): add `shutters.py`
joao-biondo Mar 17, 2026
4e5c8a3
feat(shutters): add firs idea for shutters devices.
joao-biondo Mar 17, 2026
867f197
feat(shutters): add docstrings for the shutter devices.
joao-biondo Mar 17, 2026
9cf44ce
fix(shutters): set correct `ophyd.Kind` for components.
joao-biondo Mar 17, 2026
7de535a
chore(shutters): set parameter `type` for the `__init__` method of sh…
joao-biondo Mar 17, 2026
b0135b4
feat(shutters): overwrite `read_configuration` method for dealing wit…
joao-biondo Mar 17, 2026
e49b4a5
fix(shutters): swap names from the two shutter classes.
joao-biondo Mar 23, 2026
88df15c
chore(shutters): make docstring example of usage simpler.
joao-biondo Mar 23, 2026
f9aa1c1
fix(shutters): fix typos in shutters class.
joao-biondo Mar 30, 2026
b338fe3
refactor(shutters): change `ophyd.Kind` of permission signal to "omit…
joao-biondo Mar 30, 2026
28726b1
refactor(shutters): fix the actuation PVs shuffixes `OPENCLOSE`, `OPE…
joao-biondo Apr 22, 2026
5c75b25
fix(shutters): fix typo in docstring.
joao-biondo Apr 22, 2026
e52a7d8
refactor(shutters): instantiate `permission` as attribute if `permis…
joao-biondo Apr 23, 2026
3ac7d59
refactor(shutters): add `permission_flag` inside `set` method and `co…
joao-biondo Apr 23, 2026
f769d15
feat(shutters): add `_is_closed` method to check the status of photon…
joao-biondo Apr 24, 2026
b227be8
chore: fix typo in docstring.
joao-biondo Apr 24, 2026
9f659d8
refactor(shutters): make the permission check for the `set` method si…
joao-biondo Apr 27, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
189 changes: 189 additions & 0 deletions src/sophys/common/devices/shutters.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,189 @@
from ophyd import EpicsSignal, EpicsSignalRO, FormattedComponent, Device
from ophyd.pv_positioner import PVPositionerComparator
from ..utils.status import PremadeStatus
from ophyd.status import AndStatus, SubscriptionStatus


Comment thread
joao-biondo marked this conversation as resolved.
class ShutterToggle(PVPositionerComparator):
"""
Comment thread
joao-biondo marked this conversation as resolved.
Abstraction layer for shutters with one actuation PV (OPENCLOSE) and one readback PV (PG_STATUS). There's an optional parameter for a permission PV.

Parameters
----------
prefix: str
Prefix for the shutter's PVs.

setpoint_suffix: str
Suffix for the actuation PV. NOTE: This should be place/location of the shutter, e.g. OEA/FOE.
The PV will be formatted as "{prefix}{setpoint_suffix}OPENCLOSE".

readback_suffix: str
Suffix for the readback PV, e.g. PG_STATUS

permission_suffix: str, optional
Permssion PV string, if it exists.

NOTE
----
This implemantation considers that the value of the `readback` signal is 0 for an open shutter and 1 for a closed shutter.
This is not so intuitive, so the `set` method considers that 1 is for opennig and 0 for closing the shutter.

Usage Example
-------------
>>> shutter = ShutterOpenClose(prefix="prefix", setpoint_suffix="setpoint_suffix", readback="readback_suffix", name="shutter")
>>> shutter.set(0).wait() # for closing
>>> shutter.set(1).wait() # for opening
"""

real_setpoint = None
setpoint = FormattedComponent(
EpicsSignal, "{prefix}{setpoint_suffix}OPENCLOSE", kind="config"
)
readback = FormattedComponent(
EpicsSignalRO, "{prefix}{readback_suffix}", kind="hinted"
)
permission = FormattedComponent(
Comment thread
joao-biondo marked this conversation as resolved.
Outdated
EpicsSignalRO, "{permission_pv}", string=True, kind="omitted"
)

def __init__(
self,
*args,
setpoint_suffix: str,
readback_suffix: str,
permission_pv: str = None,
**kwargs,
):
self.setpoint_suffix = setpoint_suffix
self.readback_suffix = readback_suffix
self.permission_pv = permission_pv
super().__init__(*args, **kwargs)

def set(self, value, *args, **kwargs):
if self.permission.connected:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't we do a self.permission.wait_for_connection(timeout=<something small>) here? Otherwise we're not guaranteed the connection was even attempted before this point.

Besides, there should be a flag set in the ctor for when permission_pv is not defined, so that we don't even check anything when it isn't.

To avoid a needless connection attempt and useless network traffic, I think the permission signal shouldn't even be a component, it should instead be a signal attribute of the object, that only gets created when a permission_pv is specified. The fact that it's omitted means we don't lose much with that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nice! I was struggling to find a good way to deal with this permission_pv. Thanks so much for the suggestions!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I read the documentation of the get method, and there's this arg connection_timeout that if passed, self.wait_for_connection() will be called before getting the PV value, so I thought it would solve the problem of connection.
Maybe, this could not even be necessary using this strategy of a flag in the ctor, which makes way more sense than the previous implementation.

if not self.permission.get():
raise PremadeStatus(
success=False,
exception=PermissionError(
f"Shutter open permission is denied: {self.permission.pvname} {self.permission.get()}"
),
)

if (
value == self.readback.get()
): # Since we're swapping the readback values (o for closing and 1 for opennig), we actuate when value == readback
Comment thread
joao-biondo marked this conversation as resolved.
Outdated
self.real_setpoint = 1 if value == 0 else 0
return super().set(1, *args, **kwargs)
else:
return PremadeStatus(success=True)

def done_comparator(self, readback, setpoint):
return self.real_setpoint == readback

Comment thread
joao-biondo marked this conversation as resolved.

class ShutterOpenClose(Device):
"""
Abstraction layer for shutters with two actuation PV (OPEN and CLOSE) and two readback PV (PS_STATUS and GS_STATUS). There's an optional parameter for a permission PV.

Parameters
----------
prefix: str
Prefix for the shutter's PVs.

shutter_suffix: str
Suffix for the OPEN and CLOSE PVs. NOTE: This should be place/location of the shutter, e.g. OEA/FOE.
The PVs will be formatted as "{prefix}{shutter_suffix}OPEN" and "{prefix}{shutter_suffix}CLOSE".

ps_suffix: str
Suffix for one readback PVs, e.g. PS_STATUS

gs_suffix: str
Suffix for the second readback PV, e.g. GS_STATUS

permission_pv: str, optional
Permssion PV string, if it exists.

NOTES
-----
This implemantation considers that the value of the `readback` signal is 0 for an open shutter and 1 for a closed shutter.
This is not so intuitive, so the `set` method considers that 1 is for opennig and 0 for closing the shutter.
Comment thread
joao-biondo marked this conversation as resolved.
Outdated

The `return` of the `set` method is an `AndStatus` with both `readback` signals.

There's a `done_comparator` method that returns the state of the shutter, based in the two `readback` PVs. This method is
used as the `callback` for both `readback` signals.

Usage Example
-------------
>>> shutter = ShutterToggle(prefix="prefix", open_suffix="open_suffix", close_suffix="close_suffix", ps_suffix="ps_suffix", gs_suffix="gs_suffix", name="shutter")
>>> shutter.set(0).wait() # for closing
>>> shutter.set(1).wait() # for opening
"""

setpoint = None
photon_status = FormattedComponent(
EpicsSignalRO, "{prefix}{ps_suffix}", kind="hinted"
)
gamma_status = FormattedComponent(
EpicsSignalRO, "{prefix}{gs_suffix}", kind="hinted"
)
open = FormattedComponent(
EpicsSignal, "{prefix}{shutter_suffix}OPEN", kind="config"
)
close = FormattedComponent(
EpicsSignal, "{prefix}{shutter_suffix}CLOSE", kind="config"
)
permission = FormattedComponent(
EpicsSignalRO, "{permission_pv}", string=True, kind="omitted"
)

def __init__(
self,
*args,
shutter_suffix: str,
ps_suffix: str,
gs_suffix: str,
permission_pv: str = None,
**kwargs,
):
self.shutter_suffix = shutter_suffix
self.ps_suffix = ps_suffix
self.gs_suffix = gs_suffix
self.permission_pv = permission_pv
super().__init__(*args, **kwargs)

def set(self, value, *args, **kwargs):
if self.permission.connected:
if not self.permission.get():
raise PremadeStatus(
success=False,
exception=PermissionError(
f"Shutter open permission is denied: {self.permission.pvname} {self.permission.get()}"
),
)
Comment on lines +170 to +171

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Felipe, from IPE, suggested adding a check to wheter the shutter is already on the desired state, before calling the set method to 'openorclose`. Do you guys think this is valid ?

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.

I think this is only necessary on the Toggle shutters and you're already doing that.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It depends on the low-level implementation. If the shutters are already closed and you try to close it anyways, will it just ignore the command, or will it move the shutter to resettle? If it's the latter, the suggestion Felipe made is a potential optimization, in this case saving at least 3 seconds of settle time.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a great a point. I'd need to check with someone more knowledgeable on the low-level implementation of these devices. I'll check on that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@flowln, I found useful to include the check, as it can, at the very least, optimize the device. I'll check with @RafaelLyra8 , who is more experienced with this shutters, if we need a settle_time of 3 seconds.


if value == 0:
self.close.set(1, *args, **kwargs).wait()

elif value == 1:
self.open.set(1, *args, **kwargs).wait()

else:
raise PremadeStatus(
success=False,
exception=Exception(f"The value {value} is not a valid option!"),
)

self.setpoint = value

return AndStatus(
SubscriptionStatus(self.photon_status, self.done_comparator, settle_time=3),
SubscriptionStatus(self.gamma_status, self.done_comparator, settle_time=3),
timeout=15,
)

def done_comparator(self, value, **kwargs):
is_closed = (
self.photon_status.get() == 1 and self.gamma_status.get() == 1
) # NOTE: if one of the status is equal to zero, the shutter can be partially open
return is_closed if self.setpoint == 0 else not is_closed
Loading