Add Shutter devices to sophys-common `devices` by joao-biondo · Pull Request #88 · cnpem/sophys-common

joao-biondo · 2026-03-17T14:03:22Z

Two types of shutters were developed, ShutterOpenClose (with one actuation and one readback PV) and ShutterToggle (with two actuation and readback PVs).
The devices consider that the readback value of 0 represent an open shutter whereas a value of 1 represent a closed shutter. In the set method logic, this values are swapped, e.g. a set call with 0 closes the shutter and a call with 1 opens it.

…utter devices.

…h disconnected `permission` signal.

RafaelLyra8 · 2026-03-20T13:34:18Z

+        Prefix for the shutter's PVs.
+
+    setpoint_suffix: str
+        Suffix for the actuation PV, e.g. OPENCLOSE


Are there cases where the suffix is different than OPENCLOSE?

I'll check on that @RafaelLyra8!

@RafaelLyra8 and @vhemalheiro , first of all I'm so sorry for the delay, I've spent the last weeks testing some devices/plans at PNR, and during this period I couldn't test this shutter devices there because the hutches were kept open.

Now to the progress I made:

I've searched in Archiver for every beamline possible PVs for shutters. I've searched for PVs in the following signature: XXX:A:EPS/PPS and XXX:F:EPS/PPS.

I found out that some beamlines (CED, MGN, SPE and QUA), have the FOE shutter as a toggle case, but the suffix remains the same OPENCLOSE. At SPE and QUA there are actually two FOE shutters: FOEPGOPENCLOSE and FOEPSOPENCLOSE , so I think the two status PS_STATUS, PG_STATUS would be related to each of the shutters.

I also found out that in the EPS PVs, almost all beamlines have a SHUTTERopen(closed) PV that store the status of the shutters (they'd also be EpicsSignalRO). I'm not sure how they're different from the GS, PS and PG status PVs, it seems to me that they have the same information.

I found permission PVs at SAB as well, and they also have a different prefix, just like at IPE. In general, the permission PVs are found in EPS and the actuation at PPS.

I couldn't search for shutter that are controlled via another devices/controllers, like TATU or CRIO, since this would require a deeper knowledge of the beamlines.

From what I found, I think we could fix the suffixes OPENCOLSE, OPEN and CLOSE in the devices, allowing the user to insert only the hutch, e.g. FOE or OEA.
About the other status PVs (SHUTTERopen/closed) I'm not really sure if we should or even how we could use them. At QUA this PV is instantiated as an EpicsSignalRO and I could not find any other mention to a shutter, so I think they control it only at the IHM, or via some other device.

What do you guys think ?

RafaelLyra8 · 2026-03-20T13:36:41Z

+    Usage Example
+    -------------
+    >>> shutter = ShutterOpenClose(prefix="prefix", setpoint_suffix="setpoint_suffix", readback="readback_suffix", name="shutter")
+    >>> from bluesky.plans_stubs import mv


I don't think this import is necessary for the example. You could simplify in order to just use the set(0).wait(), instead of mv, as this is only related to ophyd.

RafaelLyra8 · 2026-03-20T13:41:35Z

+        return self.real_setpoint == readback
+
+    def read_configuration(self, *args, **kwargs):
+        if self.permission.connected:


Do you need permission to read the device, wouldn't it be only necessary when setting it?

What happened is that when the permission is not given, the read_configuration method raises an error, because the Signal is not connected. That's why I made a new read_configuration to deal with this case. I think this happens because I declared it as "config". Thinking about it now, maybe I can declare it as "omitted", and that might be resolved.

I declared the Signal as "omitted" and this solved the problem. Thanks for pointing it out.

RafaelLyra8 · 2026-03-20T13:44:02Z

+            }
+
+
+class ShutterToggle(Device):


Maybe exchange the name between the classes, as toggling is more related to the OPENCLOSE case, as you only send 1 to it and it toggles its value.

Sorry @RafaelLyra8! @vhemalheiro pointed it out for me that I swapped the names kkkkk

…ted" and remove `read_configuration` method.

joao-biondo · 2026-04-16T12:20:41Z

About testing, again, I'm very sorry that I couldn't test the devices yet. At PNR, they'll be keeping the hutches opened on Mondays, so I don't think I'll be able to test there so soon. Do you guys know any other beamline that would give us the time to test this devices ? I'm not acquainted with the other beamline's staff.

joao-biondo · 2026-04-16T12:21:41Z

Also, one last comment, do you guys think we add a FastShutter device ? From the orchestration meetings, it seemed like another case of common shutters for some beamlines.

joao-biondo · 2026-04-16T12:14:03Z

+        Prefix for the shutter's PVs.
+
+    setpoint_suffix: str
+        Suffix for the actuation PV, e.g. OPENCLOSE


@RafaelLyra8 and @vhemalheiro , first of all I'm so sorry for the delay, I've spent the last weeks testing some devices/plans at PNR, and during this period I couldn't test this shutter devices there because the hutches were kept open.

Now to the progress I made:

I've searched in Archiver for every beamline possible PVs for shutters. I've searched for PVs in the following signature: XXX:A:EPS/PPS and XXX:F:EPS/PPS.

I found out that some beamlines (CED, MGN, SPE and QUA), have the FOE shutter as a toggle case, but the suffix remains the same OPENCLOSE. At SPE and QUA there are actually two FOE shutters: FOEPGOPENCLOSE and FOEPSOPENCLOSE , so I think the two status PS_STATUS, PG_STATUS would be related to each of the shutters.

I also found out that in the EPS PVs, almost all beamlines have a SHUTTERopen(closed) PV that store the status of the shutters (they'd also be EpicsSignalRO). I'm not sure how they're different from the GS, PS and PG status PVs, it seems to me that they have the same information.

I found permission PVs at SAB as well, and they also have a different prefix, just like at IPE. In general, the permission PVs are found in EPS and the actuation at PPS.

I couldn't search for shutter that are controlled via another devices/controllers, like TATU or CRIO, since this would require a deeper knowledge of the beamlines.

From what I found, I think we could fix the suffixes OPENCOLSE, OPEN and CLOSE in the devices, allowing the user to insert only the hutch, e.g. FOE or OEA.
About the other status PVs (SHUTTERopen/closed) I'm not really sure if we should or even how we could use them. At QUA this PV is instantiated as an EpicsSignalRO and I could not find any other mention to a shutter, so I think they control it only at the IHM, or via some other device.

What do you guys think ?

joao-biondo · 2026-04-16T12:15:35Z

+                    ),
+                )


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 ?

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

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.

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.

@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.

joao-biondo · 2026-04-16T12:17:31Z

+        return self.real_setpoint == readback
+
+    def read_configuration(self, *args, **kwargs):
+        if self.permission.connected:


I declared the Signal as "omitted" and this solved the problem. Thanks for pointing it out.

…N` and `CLOSE`. These suffixes were fixed as, after research, they were found to be repeated in the beamline's PVs.

joao-biondo · 2026-04-22T18:39:24Z

@RafaelLyra8 sorry, I had not made the changes of fixing the PV suffixes before the comment, now they are fixed. Thanks for the approval! I'll just wait for some beamline time for tests before merging it.

flowln · 2026-04-22T19:32:18Z

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


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.

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

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.

flowln · 2026-04-22T19:42:37Z

+                    ),
+                )


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.

…ion_pv` is defined.

…nnection_timeout` param to ensure connection attempt.

… and gamma shutters. This method was added so that a check to the current status can be made before the attempt of open/close.

flowln · 2026-04-24T16:59:59Z

+        if self.permission_pv is not None:
+            self.permission = EpicsSignalRO(f"{self.permission_pv}", name="permission")
+            self.permission_flag = True
+        else:
+            self.permission_flag = False


Suggested change

if self.permission_pv is not None:

self.permission = EpicsSignalRO(f"{self.permission_pv}", name="permission")

self.permission_flag = True

else:

self.permission_flag = False

if self.pemission_pv is not None:

self.permission = EpicsSignalRO(f"{self.permission_pv}", name="permission")

then to check permission_flag:

if hasattr(self, "permission"):

It would also make sense to me to change self.permission_pv to self._permission_pv_name and self.permission to self.permission_signal.

flowln · 2026-04-24T17:04:11Z

+            try:
+                if not self.permission.get(connection_timeout=2, **kwargs):
+                    raise PremadeStatus(
+                        success=False,
+                        exception=PermissionError(
+                            f"Shutter open permission is denied: {self.permission.pvname} {self.permission.get()}"
+                        ),
+                    )
+            except TimeoutError:
+                raise


Suggested change

try:

if not self.permission.get(connection_timeout=2, **kwargs):

raise PremadeStatus(

success=False,

exception=PermissionError(

f"Shutter open permission is denied: {self.permission.pvname} {self.permission.get()}"

),

)

except TimeoutError:

raise

permission_status = self.permission.get(connection_timeout=2, **kwargs)

if not permission_status:

raise PremadeStatus(

success=False,

exception=PermissionError(

f"Shutter open permission is denied: {self.permission.pvname} {permission_status}"

),

)

No need for the try ... except wrap if all you're doing is re-raising the exception. This would also avoid getting the value twice, the second time with DEFAULT_CONNECTION_TIMEOUT, which is normally like 10s.

…mpler. The names of the attributes related to the permission PV are changed to make more sense with their functions. The check is made simpler by verifying the existence of the attribute `permission_signal` and the `try ... except` wrap was removed since it was unnecessary.

joao-biondo · 2026-04-27T19:16:21Z

@flowln thank you so much for the suggestions, I think the devices are more complete with these changes! I'm learning a lot for the reviews all of you guys give me. I talked with @RafaelLyra8 and I'll test the ShutterOpenClose with a smaller settle_time inside the SubscriptionStatus

joao-biondo added 6 commits March 17, 2026 08:41

feat(shutters): add shutters.py

7a4946f

feat(shutters): add firs idea for shutters devices.

4e5c8a3

feat(shutters): add docstrings for the shutter devices.

867f197

fix(shutters): set correct ophyd.Kind for components.

9cf44ce

chore(shutters): set parameter type for the __init__ method of sh…

7de535a

…utter devices.

feat(shutters): overwrite read_configuration method for dealing wit…

b0135b4

…h disconnected `permission` signal.

joao-biondo requested review from RafaelLyra8 and vhemalheiro March 17, 2026 14:03

joao-biondo commented Mar 17, 2026

View reviewed changes

Comment thread src/sophys/common/devices/shutters.py

joao-biondo commented Mar 17, 2026

View reviewed changes

Comment thread src/sophys/common/devices/shutters.py

joao-biondo commented Mar 17, 2026

View reviewed changes

Comment thread src/sophys/common/devices/shutters.py

joao-biondo self-assigned this Mar 17, 2026

RafaelLyra8 reviewed Mar 20, 2026

View reviewed changes

RafaelLyra8 requested changes Mar 20, 2026

View reviewed changes

joao-biondo added 4 commits March 23, 2026 17:28

fix(shutters): swap names from the two shutter classes.

e49b4a5

chore(shutters): make docstring example of usage simpler.

88df15c

fix(shutters): fix typos in shutters class.

f9aa1c1

refactor(shutters): change ophyd.Kind of permission signal to "omit…

b338fe3

…ted" and remove `read_configuration` method.

joao-biondo requested a review from RafaelLyra8 April 16, 2026 12:21

joao-biondo commented Apr 17, 2026

View reviewed changes

RafaelLyra8 approved these changes Apr 17, 2026

View reviewed changes

refactor(shutters): fix the actuation PVs shuffixes OPENCLOSE, `OPE…

28726b1

…N` and `CLOSE`. These suffixes were fixed as, after research, they were found to be repeated in the beamline's PVs.

flowln reviewed Apr 22, 2026

View reviewed changes

joao-biondo added 4 commits April 22, 2026 17:05

fix(shutters): fix typo in docstring.

5c75b25

refactor(shutters): instantiate permission as attribute if `permiss…

e52a7d8

…ion_pv` is defined.

refactor(shutters): add permission_flag inside set method and `co…

3ac7d59

…nnection_timeout` param to ensure connection attempt.

feat(shutters): add _is_closed method to check the status of photon…

f769d15

… and gamma shutters. This method was added so that a check to the current status can be made before the attempt of open/close.

chore: fix typo in docstring.

b227be8

joao-biondo requested review from RafaelLyra8 and flowln April 24, 2026 15:53

flowln reviewed Apr 24, 2026

View reviewed changes

		}


		class ShutterToggle(Device):

Uh oh!

Conversation

joao-biondo commented Mar 17, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joao-biondo commented Apr 16, 2026

Uh oh!

joao-biondo commented Apr 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joao-biondo commented Apr 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joao-biondo commented Apr 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants