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

Create a common base for attenuator #912

Merged
merged 8 commits into from
Dec 2, 2024
Merged
17 changes: 17 additions & 0 deletions src/dodal/beamlines/i24.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from dodal.common.beamlines.beamline_utils import BL, device_instantiation
from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline
from dodal.devices.attenuator_base import AttenuatorBase
from dodal.devices.detector import DetectorParams
from dodal.devices.eiger import EigerDetector
from dodal.devices.hutch_shutter import HutchShutter
Expand Down Expand Up @@ -29,6 +30,22 @@
set_utils_beamline(BL)


def attenuator(
wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False
) -> AttenuatorBase:
"""Get a basic (temporarily without filter wheels) attenuator device for i24,
instantiate it if it hasn't already been. If this is called when already \
instantiated in i24, it will return the existing object."""
# For filter wheels see https://github.com/DiamondLightSource/dodal/issues/927
return device_instantiation(
AttenuatorBase,
"attenuator",
"-OP-ATTN-01:",
wait_for_connection,
fake_with_ophyd_sim,
)


def aperture(
wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False
) -> Aperture:
Expand Down
13 changes: 4 additions & 9 deletions src/dodal/devices/attenuator.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@
AsyncStatus,
DeviceVector,
SignalR,
StandardReadable,
wait_for_value,
)
from ophyd_async.epics.core import epics_signal_r, epics_signal_rw, epics_signal_x
from ophyd_async.epics.core import epics_signal_r, epics_signal_x

from dodal.devices.attenuator_base import AttenuatorBase
from dodal.log import LOGGER


class Attenuator(StandardReadable, Movable):
class Attenuator(AttenuatorBase, Movable):
"""The attenuator will insert filters into the beam to reduce its transmission.

This device should be set with:
Expand All @@ -38,14 +38,9 @@ def __init__(self, prefix: str, name: str = ""):
}
)

self._desired_transmission = epics_signal_rw(float, prefix + "T2A:SETVAL1")
self._use_current_energy = epics_signal_x(prefix + "E2WL:USECURRENTENERGY.PROC")
self._change = epics_signal_x(prefix + "FANOUT")

with self.add_children_as_readables():
self.actual_transmission = epics_signal_r(float, prefix + "MATCH")

super().__init__(name)
super().__init__(prefix, name)

@AsyncStatus.wrap
async def set(self, value: float):
Expand Down
22 changes: 22 additions & 0 deletions src/dodal/devices/attenuator_base.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
from ophyd_async.core import StandardReadable
from ophyd_async.epics.core import epics_signal_r, epics_signal_rw, epics_signal_x

# This will need a set
# See https://github.com/DiamondLightSource/dodal/issues/928


class AttenuatorBase(StandardReadable):
"""An attenuator base class with a minimum set of common PVs to phase1 beamlines.

The desired transmission value is fractional (i.e 0-1) instead of a percentage, \
and when read from the device the actual_transmission will also return a fraction.
"""

def __init__(self, prefix: str, name: str = "") -> None:
self._desired_transmission = epics_signal_rw(float, prefix + "T2A:SETVAL1")
self._use_current_energy = epics_signal_x(prefix + "E2WL:USECURRENTENERGY.PROC")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should: Either this attenuator is only used for reading and we remove these PVs or we want to add the setting too, in which case AttenuatorBase should have a set.

If we're going to do setting as well I actually think you might be able to get away with exactly the same set between the two. In the i24 case:

  • _filters_in_position contains only BL24I-OP-ATTN-01:MP1:INPOS and BL24I-OP-ATTN-01:MP2:INPOS
  • _calculated_filter_states are soft signals that are all high
  • _change is a soft signal that's a no-op

If we don't do this in this issue can you put this in a new issue please?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Made an issue here #928


with self.add_children_as_readables():
self.actual_transmission = epics_signal_r(float, prefix + "MATCH")

super().__init__(name)
Loading