Skip to content

Commit e127570

Browse files
dan-fernandescallumforresterDiamondRC
authored
Bimorph mirrors (#992)
* Create bimorph_mirrors dir with __init__.py * Create bimorph_mirror.py with basic class structure * Add VOUT_RBV channel creation to __init__ * Move bimorph_mirror.py out of bimorph_mirrors directory * Create BimorphMirrorChannel with vout_rbv SignalR * Change BimorphMirrorChannel child creation to procedural * Replace BimorphMirror child decleration to use BimorphMirrorChannel * Remove unused imports * Remove unused import * Remove uneccesary Format in add_children_as_readables * Add super().__init__ call * Add VTRGT_RBV, STATUS to BimorphMirrorChannel * Add BimorphMirror.set method * Add bimorph test file * Fix STATUS type * Make BimorphMirror.set await values in VOUT_RBV * Add assertion that channel exists to BimorphMirror.set * Replace .get with square brackets in BimorphMirror.set to avoid typing errors * Add BimorphMirrorChannel.shift * Add BimorphMirror.all_shift, .add_volt * Add BimorphMirror.channels, .status, .temps * Rename previous BimorphMirror.channels to BimorphMirror.channel_list to avoid member name clash * Add BimorphMirror.reset_err_proc, .err * Add BimorphOnOff Strict Enum * Add BimorphMirror.on_off * Add BimorphMirrorMode StrictEnum * Add BimorphMirror.op_mode * Change BimorphMirror.on_off datatype to BimorphMirrorOnOff rather than StrictEnum * Add docstrings * Remove uneccesary signals from BimorphMirror * Add missing attribute to docstring * Remove duplicate signal decleration * Remove uneccesary BimorphMirror signal * Change BimorphMirrorChannel.status to datatype BimorphMirrorOnOff * Add BimorphMirrorStatus, set as BimorphMirror.status datatype * Change BimorphMirror.channels datatype to str * Change BimorphMirrorChannel to declare all signals declaratively * Remove uneccesary argument * Move all PV delimiters from suffix into prefix * Fix incorrect use of PVSuffix * Make BimorphMirrorChannel inherit from EpicsDevice * Remove :CHANNELS signal from BimorphMirror * Rename BimorphMirror.channel_list to .channels * Remove unused import * Replace BimorphMirror.set wait on rbv with wait=True * Add bimorph test_set_channels * Organise imports * Make wait_for_value in BimorphMirror.set use DEFAULT_TIMEOUT instead of None * Rename test_set_channels, fix said test, and add test_set_channels_triggers_alltrgt_proc * Remove whitespace * Remove unused import * Add comments * Parametrize test_set_channels_wait_for_readback * Abstract setting vout mock values into fixture * Parametrize test_set_channels_wait_for_readback * Add whitespace * Renam set_vout_mock_values to set_vout_mock_and_return_value * Add test_set_channels_waits_for_vout_readback unimplemented * Add test_read, and BIMORPH_NAME global variable * Make mirror fixture take number_of_channels argument * Correct BimorphMirrorChannel Format signal types * Add valid_bimorph_values fixture * Change test parametrization from bimorph input to number of channels * Add format changes generated by tox pre-commit * Change explcit channel number parameter to reference to global list of channel numbers * Implement test_set_channels_waits_for_vout_readback * Add test_set_invalid_chanel_throws_error * Add Raises section to BimorphMirror.set docstring * Make BiorphMirror.__init__ number_of_channels have no default value * Replace all asserts inside for loops, to comprehension and comparison * Remove BIMORPH_NAME global variable * Remove prng from test input generation * Add BimorphMirror.__init__ number_of_channels validation * Add test_init_mirror_with_invalid_channels_throws_error * Add BimorphMirror.__init__ docstring * Replace assertion in BimorphMirror.set arg validation with raise ValueError * Make test_set_invalid_channel_throws_error expect correct error type * Replace real PV prefix with fake * Add mock_vtrgt_vout_propogation, remove set_vout_mock fixtures * Remove unused import * Fix import for ophyd async 0.9.0a1 * Update src/dodal/devices/bimorph_mirror.py Co-authored-by: Callum Forrester <callum.forrester@diamond.ac.uk> * Update src/dodal/devices/bimorph_mirror.py Co-authored-by: Callum Forrester <callum.forrester@diamond.ac.uk> * Update src/dodal/devices/bimorph_mirror.py Co-authored-by: Callum Forrester <callum.forrester@diamond.ac.uk> * Remove BimorphMirror.number_of_channels * Rename BimorphMirror.alltrgt_proc, .on_off for readability * Rename BimorphMirrorChannel.vtrgt, .vout for readbility * Change BimorphMirror.set type hinting * Replace BimorphMirror.set sequential input validation with collection Co-authored-by: Callum Forrester <callum.forrester@diamond.ac.uk> * Add test_init_mirror_with_zero_channels * Ensure bimorph has finished moving after set * wait for value * Update test_bimorph_mirror.py * Crop PV name for BUSY toggle * Crop PV name for BUSY toggle * await IDLE status * Update test to account for idling * Make BimorphMirrorChannel Movable, add set method * Remove unnecessary fixture from test_bimorph_mirror_channel_set * Make BimorphMirror.set set to target_voltage in serial, and wait check for mirror status. * Add wait for BimorphMirrorStatus.IDLE before trigger in BimorphMirror.set * Refactor mock_vtrgt_vout_propogation to use callback_on_mock_put * Add mock_bimorph_mirror_status_functionality * Aggregate mock_vtrgt_vout_propogation, mock_bimorph_mirror_status_functionality, into single bimorph_functionality fixture * Make test_set_channels_waits_for_vout_readback check subset of call_args rather than equality * Add bimorph_functionality fixture to test_set_channels_allows_tolerance * Remove tolerance in BimorphMirror.set * Remove unused argument in BimorphMirror.set * Add comment to BimorphMirror.set explaining serial set * Refactor test_bimorph_mirror to use mirror_with_mocked_put fixture * Add typing to mirror_with_mocked_put status function * Add type hinting to mirror_with_mocked_put vout_propogation_and_status function * Rename all ..vout.. references to ..output_voltage.. * Make BimorphMirror.set use set_and_wait_for_other_value rather than two seperate calls * Rewrite mirror_with_mocked_put to not use default arguments in helper functions, add docstring * Renamed functions in mirror_with_mocked_put fixture for readability * Set DEFAULT_TIMEOUT to 60 * Change BimorphMirror.set to write to output_voltage and to not trigger commit_target_voltages * Remove BimorphMirror.commit_target_voltages * Remove test_set_channels_triggers_alltrgt_proc * Change test_set_channels_waits_for_readback to read from output_voltage * Make test_set_one_channel read from output_voltage * Change BimorphMirror.set to accept array param rather than mapping (cherry picked from commit 38f3a904e323fe4f85bca38ae31c9283c082024e) * Change BimorphMirror.set to set in parallel * Add BimorphMirror.commit_target_voltages * Update tests to reflect BimorphMirror.set interface change (cherry picked from commit 602d07265d50429f74f30cf25d449a2fa78f802b) * Replace DeviceCollector with init_devices * Remove test_set_one_channel * Remove BimorphMirrorChannel.set * Remove merge conflict, fix BimorphMirror signature --------- Co-authored-by: Callum Forrester <callum.forrester@diamond.ac.uk> Co-authored-by: Richard Cunningham <richard.cunningham@diamond.ac.uk>
1 parent 3dc3b67 commit e127570

File tree

2 files changed

+116
-132
lines changed

2 files changed

+116
-132
lines changed

‎src/dodal/devices/bimorph_mirror.py‎

Lines changed: 29 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,16 @@
11
import asyncio
2-
from collections.abc import Mapping
32
from typing import Annotated as A
43

54
from bluesky.protocols import Movable
65
from ophyd_async.core import (
7-
DEFAULT_TIMEOUT,
86
AsyncStatus,
97
DeviceVector,
108
SignalR,
119
SignalRW,
1210
SignalW,
1311
StandardReadable,
1412
StrictEnum,
13+
set_and_wait_for_other_value,
1514
wait_for_value,
1615
)
1716
from ophyd_async.core import StandardReadableFormat as Format
@@ -23,6 +22,8 @@
2322
epics_signal_x,
2423
)
2524

25+
DEFAULT_TIMEOUT = 60
26+
2627

2728
class BimorphMirrorOnOff(StrictEnum):
2829
ON = "ON"
@@ -41,7 +42,7 @@ class BimorphMirrorStatus(StrictEnum):
4142
ERROR = "Error"
4243

4344

44-
class BimorphMirrorChannel(StandardReadable, Movable[float], EpicsDevice):
45+
class BimorphMirrorChannel(StandardReadable, EpicsDevice):
4546
"""Collection of PVs comprising a single bimorph channel.
4647
4748
Attributes:
@@ -56,23 +57,13 @@ class BimorphMirrorChannel(StandardReadable, Movable[float], EpicsDevice):
5657
status: A[SignalR[BimorphMirrorOnOff], PvSuffix("STATUS"), Format.CONFIG_SIGNAL]
5758
shift: A[SignalW[float], PvSuffix("SHIFT")]
5859

59-
@AsyncStatus.wrap
60-
async def set(self, value: float):
61-
"""Sets channel's VOUT to given value.
62-
63-
Args:
64-
value: float to set VOUT to
65-
"""
66-
await self.output_voltage.set(value)
67-
6860

69-
class BimorphMirror(StandardReadable, Movable[Mapping[int, float]]):
61+
class BimorphMirror(StandardReadable, Movable[list[float]]):
7062
"""Class to represent CAENels Bimorph Mirrors.
7163
7264
Attributes:
7365
channels: DeviceVector of BimorphMirrorChannel, indexed from 1, for each channel
7466
enabled: Writeable BimorphOnOff
75-
commit_target_voltages: Procable signal that writes values in each channel's VTRGT to VOUT
7667
status: Readable BimorphMirrorStatus Busy/Idle status
7768
err: Alarm status"""
7869

@@ -103,49 +94,51 @@ def __init__(self, prefix: str, number_of_channels: int, name=""):
10394
super().__init__(name=name)
10495

10596
@AsyncStatus.wrap
106-
async def set(self, value: Mapping[int, float], tolerance: float = 0.0001) -> None:
107-
"""Sets bimorph voltages in parrallel via target voltage and all proc.
97+
async def set(self, value: list[float]) -> None:
98+
"""Sets bimorph voltages in parallel via target voltage and all proc.
10899
109100
Args:
110-
value: Dict of channel numbers to target voltages
101+
value: List of float target voltages
111102
112103
Raises:
113104
ValueError: On set to non-existent channel"""
114105

115-
if any(key not in self.channels for key in value):
106+
if len(value) != len(self.channels):
116107
raise ValueError(
117-
f"Attempting to put to non-existent channels: {[key for key in value if (key not in self.channels)]}"
108+
f"Length of value input array does not match number of \
109+
channels: {len(value)} and {len(self.channels)}"
118110
)
119111

120-
# Write target voltages:
121-
await asyncio.gather(
122-
*[
123-
self.channels[i].target_voltage.set(target, wait=True)
124-
for i, target in value.items()
125-
]
126-
)
112+
# Write target voltages in serial
113+
# Voltages are written in serial as bimorph PSU cannot handle simultaneous sets
114+
for i, target in enumerate(value):
115+
await wait_for_value(
116+
self.status, BimorphMirrorStatus.IDLE, timeout=DEFAULT_TIMEOUT
117+
)
118+
await set_and_wait_for_other_value(
119+
self.channels[i + 1].target_voltage,
120+
target,
121+
self.status,
122+
BimorphMirrorStatus.BUSY,
123+
)
127124

128125
# Trigger set target voltages:
126+
await wait_for_value(
127+
self.status, BimorphMirrorStatus.IDLE, timeout=DEFAULT_TIMEOUT
128+
)
129129
await self.commit_target_voltages.trigger()
130130

131131
# Wait for values to propogate to voltage out rbv:
132132
await asyncio.gather(
133133
*[
134134
wait_for_value(
135-
self.channels[i].output_voltage,
136-
tolerance_func_builder(tolerance, target),
135+
self.channels[i + 1].output_voltage,
136+
target,
137137
timeout=DEFAULT_TIMEOUT,
138138
)
139-
for i, target in value.items()
139+
for i, target in enumerate(value)
140140
],
141141
wait_for_value(
142142
self.status, BimorphMirrorStatus.IDLE, timeout=DEFAULT_TIMEOUT
143143
),
144144
)
145-
146-
147-
def tolerance_func_builder(tolerance: float, target_value: float):
148-
def is_within_value(x):
149-
return abs(x - target_value) <= tolerance
150-
151-
return is_within_value

‎tests/devices/unit_tests/test_bimorph_mirror.py‎

Lines changed: 87 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,23 @@
1+
import asyncio
2+
from collections.abc import Callable
3+
from typing import Any
14
from unittest.mock import ANY, call, patch
25

36
import pytest
47
from bluesky.run_engine import RunEngine
5-
from ophyd_async.core import init_devices
6-
from ophyd_async.testing import get_mock_put
8+
from ophyd_async.core import init_devices, walk_rw_signals
9+
from ophyd_async.testing import callback_on_mock_put, get_mock_put, set_mock_value
710

8-
from dodal.devices.bimorph_mirror import BimorphMirror, BimorphMirrorStatus
11+
from dodal.devices.bimorph_mirror import (
12+
BimorphMirror,
13+
BimorphMirrorChannel,
14+
BimorphMirrorStatus,
15+
)
916

1017
VALID_BIMORPH_CHANNELS = [8, 12, 16, 24]
1118

1219

13-
@pytest.fixture
20+
@pytest.fixture(params=VALID_BIMORPH_CHANNELS)
1421
def mirror(request, RE: RunEngine) -> BimorphMirror:
1522
number_of_channels = request.param
1623

@@ -24,122 +31,117 @@ def mirror(request, RE: RunEngine) -> BimorphMirror:
2431

2532

2633
@pytest.fixture
27-
def valid_bimorph_values(mirror: BimorphMirror) -> dict[int, float]:
28-
return {i: float(i) for i in range(1, len(mirror.channels) + 1)}
34+
def valid_bimorph_values(mirror: BimorphMirror) -> list[float]:
35+
return [float(i) for i in range(1, len(mirror.channels) + 1)]
2936

3037

3138
@pytest.fixture
32-
def mock_vtrgt_vout_propogation(mirror: BimorphMirror):
33-
for channel in mirror.channels.values():
39+
def mirror_with_mocked_put(mirror: BimorphMirror):
40+
"""Returns BimorphMirror with some simulated behaviour.
41+
42+
BimorphMirror that simulates BimorphMirrorStatus BUSY/IDLE behaviour on all
43+
rw_signals, and propogation from target_voltage to output_voltage on each
44+
channel.
45+
46+
Args:
47+
mirror: BimorphMirror fixture
48+
"""
49+
50+
async def busy_idle():
51+
await asyncio.sleep(0)
52+
set_mock_value(mirror.status, BimorphMirrorStatus.BUSY)
53+
await asyncio.sleep(0)
54+
set_mock_value(mirror.status, BimorphMirrorStatus.IDLE)
55+
56+
async def start_busy_idle(*_: Any, **__: Any):
57+
asyncio.create_task(busy_idle())
58+
59+
for signal in walk_rw_signals(mirror).values():
60+
callback_on_mock_put(signal, start_busy_idle)
61+
62+
def callback_function(
63+
channel: BimorphMirrorChannel,
64+
) -> Callable[[float, bool], None]:
65+
def output_voltage_propogation_and_status(
66+
value: float,
67+
wait: bool = False,
68+
):
69+
channel.output_voltage.set(value, wait=wait)
70+
asyncio.create_task(busy_idle())
3471

35-
def effect(value: float, wait=False, signal=channel.output_voltage):
36-
signal.set(value, wait=wait)
72+
return output_voltage_propogation_and_status
3773

38-
get_mock_put(channel.target_voltage).side_effect = effect
74+
for channel in mirror.channels.values():
75+
callback_on_mock_put(channel.target_voltage, callback_function(channel))
76+
77+
return mirror
3978

4079

41-
@pytest.mark.parametrize("mirror", VALID_BIMORPH_CHANNELS, indirect=True)
4280
async def test_set_channels_waits_for_readback(
43-
mirror: BimorphMirror,
44-
valid_bimorph_values: dict[int, float],
45-
mock_vtrgt_vout_propogation,
81+
mirror_with_mocked_put: BimorphMirror,
82+
valid_bimorph_values: list[float],
4683
):
47-
await mirror.set(valid_bimorph_values)
84+
await mirror_with_mocked_put.set(valid_bimorph_values)
4885

49-
assert {
50-
key: await mirror.channels[key].target_voltage.get_value()
51-
for key in valid_bimorph_values
52-
} == valid_bimorph_values
86+
assert [
87+
await mirror_with_mocked_put.channels[i].target_voltage.get_value()
88+
for i in range(1, len(valid_bimorph_values) + 1)
89+
] == valid_bimorph_values
5390

5491

55-
@pytest.mark.parametrize("mirror", VALID_BIMORPH_CHANNELS, indirect=True)
5692
async def test_set_channels_triggers_alltrgt_proc(
57-
mirror: BimorphMirror,
58-
valid_bimorph_values: dict[int, float],
59-
mock_vtrgt_vout_propogation,
93+
mirror_with_mocked_put: BimorphMirror,
94+
valid_bimorph_values: list[float],
6095
):
61-
mock_alltrgt_proc = get_mock_put(mirror.commit_target_voltages)
96+
mock_alltrgt_proc = get_mock_put(mirror_with_mocked_put.commit_target_voltages)
6297

6398
mock_alltrgt_proc.assert_not_called()
6499

65-
await mirror.set(valid_bimorph_values)
100+
await mirror_with_mocked_put.set(valid_bimorph_values)
66101

67102
mock_alltrgt_proc.assert_called_once()
68103

69104

70-
@pytest.mark.parametrize("mirror", VALID_BIMORPH_CHANNELS, indirect=True)
71-
async def test_set_channels_waits_for_vout_readback(
72-
mirror: BimorphMirror,
73-
valid_bimorph_values: dict[int, float],
74-
mock_vtrgt_vout_propogation,
105+
async def test_set_channels_waits_for_output_voltage_readback(
106+
mirror_with_mocked_put: BimorphMirror,
107+
valid_bimorph_values: list[float],
75108
):
76109
with patch("dodal.devices.bimorph_mirror.wait_for_value") as mock_wait_for_value:
77110
mock_wait_for_value.assert_not_called()
78111

79-
await mirror.set(valid_bimorph_values)
112+
await mirror_with_mocked_put.set(valid_bimorph_values)
80113

81114
expected_call_arg_list = [
82-
call(mirror.channels[i].output_voltage, ANY, timeout=ANY)
83-
for i, val in valid_bimorph_values.items()
115+
call(
116+
mirror_with_mocked_put.channels[i + 1].output_voltage, ANY, timeout=ANY
117+
)
118+
for i, val in enumerate(valid_bimorph_values)
84119
]
85-
expected_call_arg_list.append(
86-
call(mirror.status, BimorphMirrorStatus.IDLE, timeout=ANY)
87-
)
88-
assert expected_call_arg_list == mock_wait_for_value.call_args_list
89-
90-
91-
@pytest.mark.parametrize("mirror", VALID_BIMORPH_CHANNELS, indirect=True)
92-
async def test_set_channels_allows_tolerance(
93-
mirror: BimorphMirror,
94-
valid_bimorph_values: dict[int, float],
95-
):
96-
for channel in mirror.channels.values():
97120

98-
def out_by_a_little(value: float, wait=False, signal=channel.output_voltage):
99-
signal.set(value + 0.00001, wait=wait)
100-
101-
get_mock_put(channel.target_voltage).side_effect = out_by_a_little
102-
103-
await mirror.set(valid_bimorph_values)
104-
105-
106-
@pytest.mark.parametrize("mirror", VALID_BIMORPH_CHANNELS, indirect=True)
107-
async def test_set_one_channel(mirror: BimorphMirror, mock_vtrgt_vout_propogation):
108-
values = {1: 1}
109-
110-
await mirror.set(values)
111-
112-
read = await mirror.read()
113-
114-
assert [
115-
await mirror.channels[key].target_voltage.get_value() for key in values
116-
] == list(values)
117-
118-
assert [
119-
read[f"{mirror.name}-channels-{key}-output_voltage"]["value"] for key in values
120-
] == list(values)
121+
assert all(
122+
c in mock_wait_for_value.call_args_list for c in expected_call_arg_list
123+
)
121124

122125

123-
@pytest.mark.parametrize("mirror", VALID_BIMORPH_CHANNELS, indirect=True)
124126
async def test_read(
125-
mirror: BimorphMirror,
126-
valid_bimorph_values: dict[int, float],
127-
mock_vtrgt_vout_propogation,
127+
mirror_with_mocked_put: BimorphMirror,
128+
valid_bimorph_values: list[float],
128129
):
129-
await mirror.set(valid_bimorph_values)
130+
await mirror_with_mocked_put.set(valid_bimorph_values)
130131

131-
read = await mirror.read()
132+
read = await mirror_with_mocked_put.read()
132133

133134
assert [
134-
read[f"{mirror.name}-channels-{i}-output_voltage"]["value"]
135-
for i in range(1, len(mirror.channels) + 1)
136-
] == list(valid_bimorph_values.values())
135+
read[f"{mirror_with_mocked_put.name}-channels-{i}-output_voltage"]["value"]
136+
for i in range(1, len(mirror_with_mocked_put.channels) + 1)
137+
] == list(valid_bimorph_values)
137138

138139

139-
@pytest.mark.parametrize("mirror", VALID_BIMORPH_CHANNELS, indirect=True)
140-
async def test_set_invalid_channel_throws_error(mirror: BimorphMirror):
140+
async def test_set_invalid_value_throws_error(mirror_with_mocked_put: BimorphMirror):
141141
with pytest.raises(ValueError):
142-
await mirror.set({len(mirror.channels) + 1: 0.0})
142+
await mirror_with_mocked_put.set(
143+
list(range(len(mirror_with_mocked_put.channels) + 1))
144+
)
143145

144146

145147
@pytest.mark.parametrize("number_of_channels", [-1])
@@ -150,18 +152,7 @@ async def test_init_mirror_with_invalid_channels_throws_error(number_of_channels
150152

151153
@pytest.mark.parametrize("number_of_channels", [0])
152154
async def test_init_mirror_with_zero_channels(number_of_channels):
153-
mirror = BimorphMirror(prefix="FAKE-PREFIX", number_of_channels=number_of_channels)
154-
assert len(mirror.channels) == 0
155-
156-
157-
@pytest.mark.parametrize("mirror", VALID_BIMORPH_CHANNELS, indirect=True)
158-
async def test_bimorph_mirror_channel_set(
159-
mirror: BimorphMirror,
160-
valid_bimorph_values: dict[int, float],
161-
):
162-
for value, channel in zip(
163-
valid_bimorph_values.values(), mirror.channels.values(), strict=True
164-
):
165-
assert await channel.output_voltage.get_value() != value
166-
await channel.set(value)
167-
assert await channel.output_voltage.get_value() == value
155+
mirror_with_mocked_put = BimorphMirror(
156+
prefix="FAKE-PREFIX", number_of_channels=number_of_channels
157+
)
158+
assert len(mirror_with_mocked_put.channels) == 0

0 commit comments

Comments
 (0)