From 5dc58e7579f16e7f9fcf68a16b50745b9f382151 Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Tue, 7 Nov 2023 11:35:34 +0000 Subject: [PATCH 1/5] Add panda-specific save load plans --- src/ophyd_async/panda/__init__.py | 3 ++ src/ophyd_async/panda/panda_utils.py | 41 +++++++++++++++++++ tests/panda/test_panda_utils.py | 59 ++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+) create mode 100644 src/ophyd_async/panda/panda_utils.py create mode 100644 tests/panda/test_panda_utils.py diff --git a/src/ophyd_async/panda/__init__.py b/src/ophyd_async/panda/__init__.py index 85868e2e8d..ce5fa43c28 100644 --- a/src/ophyd_async/panda/__init__.py +++ b/src/ophyd_async/panda/__init__.py @@ -1,4 +1,5 @@ from .panda import PandA, PcapBlock, PulseBlock, PVIEntry, SeqBlock, SeqTable, pvi +from .panda_utils import load_panda, save_panda from .table import ( SeqTable, SeqTableRow, @@ -19,4 +20,6 @@ "SeqTableRow", "SeqTrigger", "pvi", + "load_panda", + "save_panda", ] diff --git a/src/ophyd_async/panda/panda_utils.py b/src/ophyd_async/panda/panda_utils.py new file mode 100644 index 0000000000..19fee3ec08 --- /dev/null +++ b/src/ophyd_async/panda/panda_utils.py @@ -0,0 +1,41 @@ +from typing import List, Optional + +from ophyd_async.core import ( + get_signal_values, + load_from_yaml, + save_to_yaml, + set_signal_values, + walk_rw_signals, +) +from ophyd_async.panda import PandA + + +def _get_panda_phases(panda: PandA, ignore: Optional[List[str]] = None): + # Panda has two load phases. If the signal name ends in the string "UNITS", it needs to be loaded first so put in first phase + signalRW_and_value = yield from get_signal_values(walk_rw_signals(panda), ignore) + phase_1 = {} + phase_2 = {} + for signal_name in signalRW_and_value.keys(): + if signal_name[-5:] == "units": + phase_1[signal_name] = signalRW_and_value[signal_name] + else: + phase_2[signal_name] = signalRW_and_value[signal_name] + + return [phase_1, phase_2] + + +def save_panda(panda: PandA, path: str, ignore: Optional[List[str]] = None): + """ + Saves all the panda PV's to a yaml file, ignoring any PV's in the `ignore` parameter + """ + phases = yield from _get_panda_phases(panda, ignore=ignore) + save_to_yaml(phases, path) + + +def load_panda(panda: PandA, path: str): + """ + Sets all PV's to a PandA using a previously saved configuration + """ + values = load_from_yaml(path) + signals_to_set = walk_rw_signals(panda) + yield from set_signal_values(signals_to_set, values) diff --git a/tests/panda/test_panda_utils.py b/tests/panda/test_panda_utils.py new file mode 100644 index 0000000000..c30a747136 --- /dev/null +++ b/tests/panda/test_panda_utils.py @@ -0,0 +1,59 @@ +from typing import TypeVar +from unittest.mock import patch + +import pytest +from bluesky import RunEngine +from ophyd_async.core import SignalRW, set_sim_value +from ophyd_async.core.device import DeviceCollector +from ophyd_async.epics.signal import epics_signal_rw +from ophyd_async.panda import PandA, load_panda, save_panda +from ophyd_async.panda.panda_utils import _get_panda_phases + +T = TypeVar("T") + + +@pytest.fixture +async def sim_panda(): + async with DeviceCollector(sim=True): + sim_panda = PandA("PANDAQSRV") + sim_panda.phase_1_signal_units: SignalRW = epics_signal_rw(int, "") + assert sim_panda.name == "sim_panda" + yield sim_panda + + +async def test_get_panda_phases(sim_panda): + def get_phases(panda): + phases = yield from _get_panda_phases(panda) + assert len(phases) == 2 + for key in phases[0].keys(): + assert key[-5:] == "units" + for key in phases[1].keys(): + assert not key[-5:] == "units" + + RE = RunEngine() + panda = PandA("PANDAQSRV") + await panda.connect(sim=True) + set_sim_value(sim_panda.phase_1_signal_units, 1) + RE(get_phases(sim_panda)) + + +@patch("ophyd_async.panda.panda_utils._get_panda_phases") +@patch("ophyd_async.panda.panda_utils.save_to_yaml") +async def test_save_panda(mock_save_to_yaml, mock_get_panda_phases, sim_panda): + RE = RunEngine() + RE(save_panda(sim_panda, "path")) + mock_get_panda_phases.assert_called_once() + mock_save_to_yaml.assert_called_once() + + +@patch("ophyd_async.panda.panda_utils.load_from_yaml") +@patch("ophyd_async.panda.panda_utils.walk_rw_signals") +@patch("ophyd_async.panda.panda_utils.set_signal_values") +async def test_load_panda( + mock_set_signal_values, mock_walk_rw_signals, mock_load_from_yaml, sim_panda +): + RE = RunEngine() + RE(load_panda(sim_panda, "path")) + mock_load_from_yaml.assert_called_once() + mock_walk_rw_signals.assert_called_once() + mock_set_signal_values.assert_called_once() From 241a45d6907ddeea65c8350a3aa89f80e2b97510 Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Tue, 7 Nov 2023 14:20:46 +0000 Subject: [PATCH 2/5] Make load generic, fix loading ignored PVs --- src/ophyd_async/core/__init__.py | 2 ++ src/ophyd_async/core/device_save_loader.py | 14 +++++++++ src/ophyd_async/panda/__init__.py | 2 +- src/ophyd_async/panda/panda_utils.py | 35 +++++----------------- tests/core/test_device_save_loader.py | 16 +++++++++- tests/panda/test_panda_utils.py | 21 +++---------- 6 files changed, 44 insertions(+), 46 deletions(-) diff --git a/src/ophyd_async/core/__init__.py b/src/ophyd_async/core/__init__.py index 711de316bb..efe329aca0 100644 --- a/src/ophyd_async/core/__init__.py +++ b/src/ophyd_async/core/__init__.py @@ -10,6 +10,7 @@ from .device import Device, DeviceCollector, DeviceVector from .device_save_loader import ( get_signal_values, + load_device, load_from_yaml, save_to_yaml, set_signal_values, @@ -97,4 +98,5 @@ "save_to_yaml", "set_signal_values", "walk_rw_signals", + "load_device", ] diff --git a/src/ophyd_async/core/device_save_loader.py b/src/ophyd_async/core/device_save_loader.py index 4ea7ebb517..3469b861bf 100644 --- a/src/ophyd_async/core/device_save_loader.py +++ b/src/ophyd_async/core/device_save_loader.py @@ -206,8 +206,22 @@ def set_signal_values( for phase_number, phase in enumerate(values): # Key is signal name for key, value in phase.items(): + # Skip ignored values + if value is None: + continue + if key in signals: yield from abs_set( signals[key], value, group=f"load-phase{phase_number}" ) + yield from wait(f"load-phase{phase_number}") + + +def load_device(device: Device, path: str): + """ + Sets all PV's to a device using a previously saved configuration + """ + values = load_from_yaml(path) + signals_to_set = walk_rw_signals(device) + yield from set_signal_values(signals_to_set, values) diff --git a/src/ophyd_async/panda/__init__.py b/src/ophyd_async/panda/__init__.py index ce5fa43c28..ee28ad0469 100644 --- a/src/ophyd_async/panda/__init__.py +++ b/src/ophyd_async/panda/__init__.py @@ -1,5 +1,5 @@ from .panda import PandA, PcapBlock, PulseBlock, PVIEntry, SeqBlock, SeqTable, pvi -from .panda_utils import load_panda, save_panda +from .panda_utils import save_panda from .table import ( SeqTable, SeqTableRow, diff --git a/src/ophyd_async/panda/panda_utils.py b/src/ophyd_async/panda/panda_utils.py index 19fee3ec08..d2e957afeb 100644 --- a/src/ophyd_async/panda/panda_utils.py +++ b/src/ophyd_async/panda/panda_utils.py @@ -1,41 +1,22 @@ from typing import List, Optional -from ophyd_async.core import ( - get_signal_values, - load_from_yaml, - save_to_yaml, - set_signal_values, - walk_rw_signals, -) +from ophyd_async.core import get_signal_values, save_to_yaml, walk_rw_signals from ophyd_async.panda import PandA def _get_panda_phases(panda: PandA, ignore: Optional[List[str]] = None): - # Panda has two load phases. If the signal name ends in the string "UNITS", it needs to be loaded first so put in first phase - signalRW_and_value = yield from get_signal_values(walk_rw_signals(panda), ignore) - phase_1 = {} - phase_2 = {} - for signal_name in signalRW_and_value.keys(): - if signal_name[-5:] == "units": - phase_1[signal_name] = signalRW_and_value[signal_name] - else: - phase_2[signal_name] = signalRW_and_value[signal_name] - + # Panda has two load phases. If the signal name ends in the string "UNITS", + # it needs to be loaded first so put in first phase + signals = walk_rw_signals(panda) + phase_2 = yield from get_signal_values(signals, ignore=ignore) + phase_1 = {n: phase_2.pop(n) for n in list(phase_2) if n.endswith("units")} return [phase_1, phase_2] def save_panda(panda: PandA, path: str, ignore: Optional[List[str]] = None): """ - Saves all the panda PV's to a yaml file, ignoring any PV's in the `ignore` parameter + Saves all the panda PV's to a yaml file, ignoring any PV's in the `ignore` + parameter """ phases = yield from _get_panda_phases(panda, ignore=ignore) save_to_yaml(phases, path) - - -def load_panda(panda: PandA, path: str): - """ - Sets all PV's to a PandA using a previously saved configuration - """ - values = load_from_yaml(path) - signals_to_set = walk_rw_signals(panda) - yield from set_signal_values(signals_to_set, values) diff --git a/tests/core/test_device_save_loader.py b/tests/core/test_device_save_loader.py index 27cfec244a..49905790c0 100644 --- a/tests/core/test_device_save_loader.py +++ b/tests/core/test_device_save_loader.py @@ -1,18 +1,19 @@ from enum import Enum from os import path from typing import Any, Dict, List +from unittest.mock import patch import numpy as np import numpy.typing as npt import pytest import yaml from bluesky.run_engine import RunEngine - from ophyd_async.core import ( Device, SignalR, SignalRW, get_signal_values, + load_device, load_from_yaml, save_to_yaml, set_signal_values, @@ -178,3 +179,16 @@ async def test_set_signal_values_restores_value(RE: RunEngine, device, tmp_path) array_value = await device.child2.sig1.get_value() assert string_value == "initial_string" assert np.array_equal(array_value, np.array([1, 1, 1, 1, 1])) + + +@patch("ophyd_async.core.device_save_loader.load_from_yaml") +@patch("ophyd_async.core.device_save_loader.walk_rw_signals") +@patch("ophyd_async.core.device_save_loader.set_signal_values") +async def test_load_device( + mock_set_signal_values, mock_walk_rw_signals, mock_load_from_yaml, device +): + RE = RunEngine() + RE(load_device(device, "path")) + mock_load_from_yaml.assert_called_once() + mock_walk_rw_signals.assert_called_once() + mock_set_signal_values.assert_called_once() diff --git a/tests/panda/test_panda_utils.py b/tests/panda/test_panda_utils.py index c30a747136..e0e6c67dc7 100644 --- a/tests/panda/test_panda_utils.py +++ b/tests/panda/test_panda_utils.py @@ -3,10 +3,11 @@ import pytest from bluesky import RunEngine + from ophyd_async.core import SignalRW, set_sim_value from ophyd_async.core.device import DeviceCollector from ophyd_async.epics.signal import epics_signal_rw -from ophyd_async.panda import PandA, load_panda, save_panda +from ophyd_async.panda import PandA, save_panda from ophyd_async.panda.panda_utils import _get_panda_phases T = TypeVar("T") @@ -15,7 +16,7 @@ @pytest.fixture async def sim_panda(): async with DeviceCollector(sim=True): - sim_panda = PandA("PANDAQSRV") + sim_panda = PandA("PANDA") sim_panda.phase_1_signal_units: SignalRW = epics_signal_rw(int, "") assert sim_panda.name == "sim_panda" yield sim_panda @@ -29,10 +30,9 @@ def get_phases(panda): assert key[-5:] == "units" for key in phases[1].keys(): assert not key[-5:] == "units" + return RE = RunEngine() - panda = PandA("PANDAQSRV") - await panda.connect(sim=True) set_sim_value(sim_panda.phase_1_signal_units, 1) RE(get_phases(sim_panda)) @@ -44,16 +44,3 @@ async def test_save_panda(mock_save_to_yaml, mock_get_panda_phases, sim_panda): RE(save_panda(sim_panda, "path")) mock_get_panda_phases.assert_called_once() mock_save_to_yaml.assert_called_once() - - -@patch("ophyd_async.panda.panda_utils.load_from_yaml") -@patch("ophyd_async.panda.panda_utils.walk_rw_signals") -@patch("ophyd_async.panda.panda_utils.set_signal_values") -async def test_load_panda( - mock_set_signal_values, mock_walk_rw_signals, mock_load_from_yaml, sim_panda -): - RE = RunEngine() - RE(load_panda(sim_panda, "path")) - mock_load_from_yaml.assert_called_once() - mock_walk_rw_signals.assert_called_once() - mock_set_signal_values.assert_called_once() From 09ca7f5acd9760f64dade8564e84109024b4c361 Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Tue, 7 Nov 2023 16:59:26 +0000 Subject: [PATCH 3/5] Wait for subprocesses to terminate in conftest include None type in load tests --- src/ophyd_async/panda/panda_utils.py | 2 +- tests/conftest.py | 5 ++++- tests/core/test_device_save_loader.py | 2 ++ tests/panda/test_panda_utils.py | 11 ++++------- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/ophyd_async/panda/panda_utils.py b/src/ophyd_async/panda/panda_utils.py index d2e957afeb..7b61a5674d 100644 --- a/src/ophyd_async/panda/panda_utils.py +++ b/src/ophyd_async/panda/panda_utils.py @@ -15,7 +15,7 @@ def _get_panda_phases(panda: PandA, ignore: Optional[List[str]] = None): def save_panda(panda: PandA, path: str, ignore: Optional[List[str]] = None): """ - Saves all the panda PV's to a yaml file, ignoring any PV's in the `ignore` + Saves all the panda PV's to a yaml file, ignoring any PV's in the ignore parameter """ phases = yield from _get_panda_phases(panda, ignore=ignore) diff --git a/tests/conftest.py b/tests/conftest.py index ec496795fd..575c47b122 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -68,7 +68,10 @@ def pva(): assert not p.poll(), p.stdout.read() yield processes - [p.terminate() for p in processes] + + for p in processes: + p.terminate() + p.communicate() @pytest.fixture diff --git a/tests/core/test_device_save_loader.py b/tests/core/test_device_save_loader.py index 49905790c0..a38e95c566 100644 --- a/tests/core/test_device_save_loader.py +++ b/tests/core/test_device_save_loader.py @@ -8,6 +8,7 @@ import pytest import yaml from bluesky.run_engine import RunEngine + from ophyd_async.core import ( Device, SignalR, @@ -149,6 +150,7 @@ async def test_load_from_yaml(RE: RunEngine, device, tmp_path): array = np.array([1, 1, 1, 1, 1]) await device.child1.sig1.set("initial_string") await device.child2.sig1.set(array) + await device.parent_sig1.set(None) RE(save_device(device, file_path)) values = load_from_yaml(file_path) diff --git a/tests/panda/test_panda_utils.py b/tests/panda/test_panda_utils.py index e0e6c67dc7..8cc4269513 100644 --- a/tests/panda/test_panda_utils.py +++ b/tests/panda/test_panda_utils.py @@ -1,4 +1,3 @@ -from typing import TypeVar from unittest.mock import patch import pytest @@ -10,8 +9,6 @@ from ophyd_async.panda import PandA, save_panda from ophyd_async.panda.panda_utils import _get_panda_phases -T = TypeVar("T") - @pytest.fixture async def sim_panda(): @@ -22,7 +19,7 @@ async def sim_panda(): yield sim_panda -async def test_get_panda_phases(sim_panda): +async def test_get_panda_phases(sim_panda, RE: RunEngine): def get_phases(panda): phases = yield from _get_panda_phases(panda) assert len(phases) == 2 @@ -32,15 +29,15 @@ def get_phases(panda): assert not key[-5:] == "units" return - RE = RunEngine() set_sim_value(sim_panda.phase_1_signal_units, 1) RE(get_phases(sim_panda)) @patch("ophyd_async.panda.panda_utils._get_panda_phases") @patch("ophyd_async.panda.panda_utils.save_to_yaml") -async def test_save_panda(mock_save_to_yaml, mock_get_panda_phases, sim_panda): - RE = RunEngine() +async def test_save_panda( + mock_save_to_yaml, mock_get_panda_phases, sim_panda, RE: RunEngine +): RE(save_panda(sim_panda, "path")) mock_get_panda_phases.assert_called_once() mock_save_to_yaml.assert_called_once() From 6686c353d1ab6c2b974871b854080e330bce1077 Mon Sep 17 00:00:00 2001 From: Rose Syrett <90774497+rosesyrett@users.noreply.github.com> Date: Thu, 14 Dec 2023 12:28:31 +0000 Subject: [PATCH 4/5] Add generic save_device function (#95) * Add generic save_device function Change panda_utils -> utils.py only, as it already resides in a panda submodule, and have this define the sorting function needed to save a generic device. * Update src/ophyd_async/panda/utils.py Co-authored-by: Tom C (DLS) <101418278+coretl@users.noreply.github.com> * Update src/ophyd_async/core/device_save_loader.py Co-authored-by: Tom C (DLS) <101418278+coretl@users.noreply.github.com> * lint * Modify docstrings and make docs build --------- Co-authored-by: Tom C (DLS) <101418278+coretl@users.noreply.github.com> --- src/ophyd_async/core/__init__.py | 2 + src/ophyd_async/core/device_save_loader.py | 109 ++++++++++++++++----- src/ophyd_async/panda/__init__.py | 5 +- src/ophyd_async/panda/panda_utils.py | 22 ----- src/ophyd_async/panda/utils.py | 15 +++ tests/panda/test_panda_utils.py | 42 ++++---- 6 files changed, 121 insertions(+), 74 deletions(-) delete mode 100644 src/ophyd_async/panda/panda_utils.py create mode 100644 src/ophyd_async/panda/utils.py diff --git a/src/ophyd_async/core/__init__.py b/src/ophyd_async/core/__init__.py index efe329aca0..1e1ff95649 100644 --- a/src/ophyd_async/core/__init__.py +++ b/src/ophyd_async/core/__init__.py @@ -12,6 +12,7 @@ get_signal_values, load_device, load_from_yaml, + save_device, save_to_yaml, set_signal_values, walk_rw_signals, @@ -99,4 +100,5 @@ "set_signal_values", "walk_rw_signals", "load_device", + "save_device", ] diff --git a/src/ophyd_async/core/device_save_loader.py b/src/ophyd_async/core/device_save_loader.py index 3469b861bf..88168b462d 100644 --- a/src/ophyd_async/core/device_save_loader.py +++ b/src/ophyd_async/core/device_save_loader.py @@ -45,30 +45,29 @@ def represent_data(self, data: Any) -> Any: def get_signal_values( signals: Dict[str, SignalRW[Any]], ignore: Optional[List[str]] = None ) -> Generator[Msg, Sequence[Location[Any]], Dict[str, Any]]: - """ - Read the values of SignalRW's, to be used alongside `walk_rw_signals`. Used as part - of saving a device. + """Get signal values in bulk. + + Used as part of saving the signals of a device to a yaml file. + Parameters ---------- - signals : Dict[str, SignalRW]: A dictionary matching the string attribute path - of a SignalRW to the signal itself + signals : Dict[str, SignalRW] + Dictionary with pv names and matching SignalRW values. Often the direct result + of :func:`walk_rw_signals`. - ignore : List of strings: . A list of string attribute paths to the SignalRW's - to be ignored. + ignore : Optional[List[str]] + Optional list of PVs that should be ignored. Returns ------- - Dict[str, Any]: A dictionary matching the string attribute path of a SignalRW - to the value of that signal. Ignored attributes are set to None. - - Yields: - Iterator[Dict[str, Any]]: The Location of a signal + Dict[str, Any] + A dictionary containing pv names and their associated values. Ignored pvs are + set to None. See Also -------- :func:`ophyd_async.core.walk_rw_signals` :func:`ophyd_async.core.save_to_yaml` - """ ignore = ignore or [] @@ -93,9 +92,11 @@ def get_signal_values( def walk_rw_signals( device: Device, path_prefix: Optional[str] = "" ) -> Dict[str, SignalRW[Any]]: - """ - Get all the SignalRWs from a device and store them with their dotted attribute - paths in a dictionary. Used as part of saving and loading a device + """Retrieve all SignalRWs from a device. + + Stores retrieved signals with their dotted attribute paths in a dictionary. Used as + part of saving and loading a device. + Parameters ---------- device : Device @@ -131,9 +132,7 @@ def walk_rw_signals( def save_to_yaml(phases: Sequence[Dict[str, Any]], save_path: str) -> None: - """ - Serialises and saves a phase or a set of phases of a device's SignalRW's to a - yaml file. + """Plan which serialises a phase or set of phases of SignalRWs to a yaml file. Parameters ---------- @@ -163,8 +162,7 @@ def save_to_yaml(phases: Sequence[Dict[str, Any]], save_path: str) -> None: def load_from_yaml(save_path: str) -> Sequence[Dict[str, Any]]: - """ - Returns a list of dicts with saved signal values from a yaml file + """Plan that returns a list of dicts with saved signal values from a yaml file. Parameters ---------- @@ -183,18 +181,22 @@ def load_from_yaml(save_path: str) -> Sequence[Dict[str, Any]]: def set_signal_values( signals: Dict[str, SignalRW[Any]], values: Sequence[Dict[str, Any]] ) -> Generator[Msg, None, None]: - """ - Loads a phase or a set of phases from a yaml file and puts value to an Ophyd device + """Maps signals from a yaml file into device signals. + + ``values`` contains signal values in phases, which are loaded in sequentially + into the provided signals, to ensure signals are set in the correct order. Parameters ---------- signals : Dict[str, SignalRW[Any]] Dictionary of named signals to be updated if value found in values argument. + Can be the output of :func:`walk_rw_signals()` for a device. values : Sequence[Dict[str, Any]] List of dictionaries of signal name and value pairs, if a signal matches the name of one in the signals argument, sets the signal to that value. The groups of signals are loaded in their list order. + Can be the output of :func:`load_from_yaml()` for a yaml file. See Also -------- @@ -219,9 +221,66 @@ def set_signal_values( def load_device(device: Device, path: str): - """ - Sets all PV's to a device using a previously saved configuration + """Plan which loads PVs from a yaml file into a device. + + Parameters + ---------- + device: Device + The device to load PVs into + path: str + Path of the yaml file to load + + See Also + -------- + :func:`ophyd_async.core.save_device` """ values = load_from_yaml(path) signals_to_set = walk_rw_signals(device) yield from set_signal_values(signals_to_set, values) + + +def all_at_once(values: Dict[str, Any]) -> Sequence[Dict[str, Any]]: + """Sort all the values into a single phase so they are set all at once""" + return [values] + + +def save_device( + device: Device, + path: str, + sorter: Callable[[Dict[str, Any]], Sequence[Dict[str, Any]]] = all_at_once, + ignore: Optional[List[str]] = None, +): + """Plan that saves the state of all PV's on a device using a sorter. + + The default sorter assumes all saved PVs can be loaded at once, and therefore + can be saved at one time, i.e. all PVs will appear on one list in the + resulting yaml file. + + This can be a problem, because when the yaml is ingested with + :func:`ophyd_async.core.load_device`, it will set all of those PVs at once. + However, some PV's need to be set before others - this is device specific. + + Therefore, users should consider the order of device loading and write their + own sorter algorithms accordingly. + + See :func:`ophyd_async.panda.phase_sorter` for a valid implementation of the + sorter. + + Parameters + ---------- + device : Device + The device whose PVs should be saved. + + path : str + The path where the resulting yaml should be saved to + + sorter : Callable[[Dict[str, Any]], Sequence[Dict[str, Any]]] + + ignore : Optional[List[str]] + + See Also + -------- + :func:`ophyd_async.core.load_device` + """ + values = yield from get_signal_values(walk_rw_signals(device), ignore=ignore) + save_to_yaml(sorter(values), path) diff --git a/src/ophyd_async/panda/__init__.py b/src/ophyd_async/panda/__init__.py index ee28ad0469..7fcdad714c 100644 --- a/src/ophyd_async/panda/__init__.py +++ b/src/ophyd_async/panda/__init__.py @@ -1,5 +1,4 @@ from .panda import PandA, PcapBlock, PulseBlock, PVIEntry, SeqBlock, SeqTable, pvi -from .panda_utils import save_panda from .table import ( SeqTable, SeqTableRow, @@ -7,6 +6,7 @@ seq_table_from_arrays, seq_table_from_rows, ) +from .utils import phase_sorter __all__ = [ "PandA", @@ -20,6 +20,5 @@ "SeqTableRow", "SeqTrigger", "pvi", - "load_panda", - "save_panda", + "phase_sorter", ] diff --git a/src/ophyd_async/panda/panda_utils.py b/src/ophyd_async/panda/panda_utils.py deleted file mode 100644 index 7b61a5674d..0000000000 --- a/src/ophyd_async/panda/panda_utils.py +++ /dev/null @@ -1,22 +0,0 @@ -from typing import List, Optional - -from ophyd_async.core import get_signal_values, save_to_yaml, walk_rw_signals -from ophyd_async.panda import PandA - - -def _get_panda_phases(panda: PandA, ignore: Optional[List[str]] = None): - # Panda has two load phases. If the signal name ends in the string "UNITS", - # it needs to be loaded first so put in first phase - signals = walk_rw_signals(panda) - phase_2 = yield from get_signal_values(signals, ignore=ignore) - phase_1 = {n: phase_2.pop(n) for n in list(phase_2) if n.endswith("units")} - return [phase_1, phase_2] - - -def save_panda(panda: PandA, path: str, ignore: Optional[List[str]] = None): - """ - Saves all the panda PV's to a yaml file, ignoring any PV's in the ignore - parameter - """ - phases = yield from _get_panda_phases(panda, ignore=ignore) - save_to_yaml(phases, path) diff --git a/src/ophyd_async/panda/utils.py b/src/ophyd_async/panda/utils.py new file mode 100644 index 0000000000..38bb015a9c --- /dev/null +++ b/src/ophyd_async/panda/utils.py @@ -0,0 +1,15 @@ +from typing import Any, Dict, Sequence + + +def phase_sorter(panda_signal_values: Dict[str, Any]) -> Sequence[Dict[str, Any]]: + # Panda has two load phases. If the signal name ends in the string "UNITS", + # it needs to be loaded first so put in first phase + phase_1, phase_2 = {}, {} + + for key, value in panda_signal_values.items(): + if key.endswith("units"): + phase_1[key] = value + else: + phase_2[key] = value + + return [phase_1, phase_2] diff --git a/tests/panda/test_panda_utils.py b/tests/panda/test_panda_utils.py index 8cc4269513..4b4548a3ee 100644 --- a/tests/panda/test_panda_utils.py +++ b/tests/panda/test_panda_utils.py @@ -3,11 +3,11 @@ import pytest from bluesky import RunEngine -from ophyd_async.core import SignalRW, set_sim_value +from ophyd_async.core import SignalRW, save_device from ophyd_async.core.device import DeviceCollector from ophyd_async.epics.signal import epics_signal_rw -from ophyd_async.panda import PandA, save_panda -from ophyd_async.panda.panda_utils import _get_panda_phases +from ophyd_async.panda import PandA +from ophyd_async.panda.utils import phase_sorter @pytest.fixture @@ -19,25 +19,19 @@ async def sim_panda(): yield sim_panda -async def test_get_panda_phases(sim_panda, RE: RunEngine): - def get_phases(panda): - phases = yield from _get_panda_phases(panda) - assert len(phases) == 2 - for key in phases[0].keys(): - assert key[-5:] == "units" - for key in phases[1].keys(): - assert not key[-5:] == "units" - return - - set_sim_value(sim_panda.phase_1_signal_units, 1) - RE(get_phases(sim_panda)) - - -@patch("ophyd_async.panda.panda_utils._get_panda_phases") -@patch("ophyd_async.panda.panda_utils.save_to_yaml") -async def test_save_panda( - mock_save_to_yaml, mock_get_panda_phases, sim_panda, RE: RunEngine -): - RE(save_panda(sim_panda, "path")) - mock_get_panda_phases.assert_called_once() +@patch("ophyd_async.core.device_save_loader.save_to_yaml") +async def test_save_panda(mock_save_to_yaml, sim_panda, RE: RunEngine): + RE(save_device(sim_panda, "path", sorter=phase_sorter)) mock_save_to_yaml.assert_called_once() + assert mock_save_to_yaml.call_args[0] == ( + [ + {"phase_1_signal_units": 0}, + { + "pulse.1.delay": 0.0, + "pulse.1.width": 0.0, + "seq.1.table": {}, + "seq.1.active": False, + }, + ], + "path", + ) From f1b3a15da78e0693db772f019208f2798587e1d9 Mon Sep 17 00:00:00 2001 From: Rose Syrett Date: Thu, 14 Dec 2023 14:40:22 +0000 Subject: [PATCH 5/5] Attempt to improve test coverage --- tests/core/test_device_save_loader.py | 36 +++++++++++++++++++-------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/tests/core/test_device_save_loader.py b/tests/core/test_device_save_loader.py index a38e95c566..1d50def471 100644 --- a/tests/core/test_device_save_loader.py +++ b/tests/core/test_device_save_loader.py @@ -16,10 +16,12 @@ get_signal_values, load_device, load_from_yaml, + save_device, save_to_yaml, set_signal_values, walk_rw_signals, ) +from ophyd_async.core.device_save_loader import all_at_once from ophyd_async.epics.signal import epics_signal_r, epics_signal_rw @@ -46,13 +48,6 @@ def __init__(self, name: str): self.position: npt.NDArray[np.int32] -def save_device(device, file_path): - signalRWs = walk_rw_signals(device) - values = yield from get_signal_values(signalRWs) - phases = sort_signal_by_phase(values) - save_to_yaml(phases, file_path) - - @pytest.fixture async def device() -> DummyDeviceGroup: device = DummyDeviceGroup("parent") @@ -132,7 +127,7 @@ async def test_yaml_formatting(RE: RunEngine, device, tmp_path): await device.child1.sig1.set("test_string") table_pv = {"VAL1": np.array([1, 2, 3, 4, 5]), "VAL2": np.array([6, 7, 8, 9, 10])} await device.child2.sig1.set(table_pv) - RE(save_device(device, file_path)) + RE(save_device(device, file_path, sorter=sort_signal_by_phase)) with open(file_path, "r") as file: expected = """\ @@ -151,7 +146,7 @@ async def test_load_from_yaml(RE: RunEngine, device, tmp_path): await device.child1.sig1.set("initial_string") await device.child2.sig1.set(array) await device.parent_sig1.set(None) - RE(save_device(device, file_path)) + RE(save_device(device, file_path, sorter=sort_signal_by_phase)) values = load_from_yaml(file_path) assert values[0]["child1.sig1"] == "initial_string" @@ -163,7 +158,7 @@ async def test_set_signal_values_restores_value(RE: RunEngine, device, tmp_path) await device.child1.sig1.set("initial_string") await device.child2.sig1.set(np.array([1, 1, 1, 1, 1])) - RE(save_device(device, file_path)) + RE(save_device(device, file_path, sorter=sort_signal_by_phase)) await device.child1.sig1.set("changed_string") await device.child2.sig1.set(np.array([2, 2, 2, 2, 2])) @@ -194,3 +189,24 @@ async def test_load_device( mock_load_from_yaml.assert_called_once() mock_walk_rw_signals.assert_called_once() mock_set_signal_values.assert_called_once() + + +async def test_set_signal_values_skips_ignored_values(device): + RE = RunEngine() + array = np.array([1, 1, 1, 1, 1]) + + await device.child1.sig1.set("initial_string") + await device.child2.sig1.set(array) + await device.parent_sig1.set(None) + + signals_of_device = walk_rw_signals(device) + values_to_set = [{"child1.sig1": None, "child2.sig1": np.array([2, 3, 4])}] + + RE(set_signal_values(signals_of_device, values_to_set)) + + assert np.all(await device.child2.sig1.get_value() == np.array([2, 3, 4])) + assert await device.child1.sig1.get_value() == "initial_string" + + +def test_all_at_once_sorter(): + assert all_at_once({"child1.sig1": 0}) == [{"child1.sig1": 0}]