diff --git a/src/ophyd_async/epics/_backend/_p4p.py b/src/ophyd_async/epics/_backend/_p4p.py index 4859e9d10e..f4acf0c1c1 100644 --- a/src/ophyd_async/epics/_backend/_p4p.py +++ b/src/ophyd_async/epics/_backend/_p4p.py @@ -1,6 +1,7 @@ import asyncio import atexit import logging +import time from dataclasses import dataclass from enum import Enum from typing import Any, Dict, List, Optional, Sequence, Type, Union @@ -119,9 +120,7 @@ def value(self, value): def descriptor(self, source: str, value) -> Descriptor: choices = [e.value for e in self.enum_class] - return dict( - source=source, dtype="string", shape=[], choices=choices - ) # type: ignore + return dict(source=source, dtype="string", shape=[], choices=choices) class PvaEnumBoolConverter(PvaConverter): @@ -141,6 +140,20 @@ def descriptor(self, source: str, value) -> Descriptor: return dict(source=source, dtype="object", shape=[]) # type: ignore +class PvaDictConverter(PvaConverter): + def reading(self, value): + ts = time.time() + value = value.todict() + # Alarm severity is vacuously 0 for a table + return dict(value=value, timestamp=ts, alarm_severity=0) + + def value(self, value: Value): + return value.todict() + + def descriptor(self, source: str, value) -> Descriptor: + raise NotImplementedError("Describing Dict signals not currently supported") + + class DisconnectedPvaConverter(PvaConverter): def __getattribute__(self, __name: str) -> Any: raise NotImplementedError("No PV has been set as connect() has not been called") @@ -149,7 +162,9 @@ def __getattribute__(self, __name: str) -> Any: def make_converter(datatype: Optional[Type], values: Dict[str, Any]) -> PvaConverter: pv = list(values)[0] typeid = get_unique({k: v.getID() for k, v in values.items()}, "typeids") - typ = get_unique({k: type(v["value"]) for k, v in values.items()}, "value types") + typ = get_unique( + {k: type(v.get("value")) for k, v in values.items()}, "value types" + ) if "NTScalarArray" in typeid and typ == list: # Waveform of strings, check we wanted this if datatype and datatype != Sequence[str]: @@ -203,6 +218,8 @@ def make_converter(datatype: Optional[Type], values: Dict[str, Any]) -> PvaConve return PvaConverter() elif "NTTable" in typeid: return PvaTableConverter() + elif "structure" in typeid: + return PvaDictConverter() else: raise TypeError(f"{pv}: Unsupported typeid {typeid}") diff --git a/src/ophyd_async/epics/pvi.py b/src/ophyd_async/epics/pvi.py new file mode 100644 index 0000000000..a71880ca1f --- /dev/null +++ b/src/ophyd_async/epics/pvi.py @@ -0,0 +1,70 @@ +from typing import Callable, Dict, FrozenSet, Optional, Type, TypedDict, TypeVar + +from ophyd_async.core.signal import Signal +from ophyd_async.core.signal_backend import SignalBackend +from ophyd_async.core.utils import DEFAULT_TIMEOUT +from ophyd_async.epics._backend._p4p import PvaSignalBackend +from ophyd_async.epics.signal.signal import ( + epics_signal_r, + epics_signal_rw, + epics_signal_w, + epics_signal_x, +) + +T = TypeVar("T") + + +_pvi_mapping: Dict[FrozenSet[str], Callable[..., Signal]] = { + frozenset({"r", "w"}): lambda dtype, read_pv, write_pv: epics_signal_rw( + dtype, read_pv, write_pv + ), + frozenset({"rw"}): lambda dtype, read_pv, write_pv: epics_signal_rw( + dtype, read_pv, write_pv + ), + frozenset({"r"}): lambda dtype, read_pv, _: epics_signal_r(dtype, read_pv), + frozenset({"w"}): lambda dtype, _, write_pv: epics_signal_w(dtype, write_pv), + frozenset({"x"}): lambda _, __, write_pv: epics_signal_x(write_pv), +} + + +class PVIEntry(TypedDict, total=False): + d: str + r: str + rw: str + w: str + x: str + + +async def pvi_get( + read_pv: str, timeout: float = DEFAULT_TIMEOUT +) -> Dict[str, PVIEntry]: + """Makes a PvaSignalBackend purely to connect to PVI information. + + This backend is simply thrown away at the end of this method. This is useful + because the backend handles a CancelledError exception that gets thrown on + timeout, and therefore can be used for error reporting.""" + backend: SignalBackend = PvaSignalBackend(None, read_pv, read_pv) + await backend.connect(timeout=timeout) + d: Dict[str, Dict[str, Dict[str, str]]] = await backend.get_value() + pv_info = d.get("pvi") or {} + result = {} + + for attr_name, attr_info in pv_info.items(): + result[attr_name] = PVIEntry(**attr_info) # type: ignore + + return result + + +def make_signal(signal_pvi: PVIEntry, dtype: Optional[Type[T]] = None) -> Signal[T]: + """Make a signal. + + This assumes datatype is None so it can be used to create dynamic signals. + """ + operations = frozenset(signal_pvi.keys()) + pvs = [signal_pvi[i] for i in operations] # type: ignore + signal_factory = _pvi_mapping[operations] + + write_pv = "pva://" + pvs[0] + read_pv = write_pv if len(pvs) < 2 else "pva://" + pvs[1] + + return signal_factory(dtype, read_pv, write_pv) diff --git a/src/ophyd_async/epics/signal/__init__.py b/src/ophyd_async/epics/signal/__init__.py index ca9cebd3fd..2bbcff867a 100644 --- a/src/ophyd_async/epics/signal/__init__.py +++ b/src/ophyd_async/epics/signal/__init__.py @@ -1,8 +1,6 @@ -from .pvi_get import pvi_get from .signal import epics_signal_r, epics_signal_rw, epics_signal_w, epics_signal_x __all__ = [ - "pvi_get", "epics_signal_r", "epics_signal_rw", "epics_signal_w", diff --git a/src/ophyd_async/epics/signal/pvi_get.py b/src/ophyd_async/epics/signal/pvi_get.py deleted file mode 100644 index f31899448c..0000000000 --- a/src/ophyd_async/epics/signal/pvi_get.py +++ /dev/null @@ -1,22 +0,0 @@ -from typing import Dict, TypedDict - -from p4p.client.asyncio import Context - - -class PVIEntry(TypedDict, total=False): - d: str - r: str - rw: str - w: str - x: str - - -async def pvi_get(pv: str, ctxt: Context, timeout: float = 5.0) -> Dict[str, PVIEntry]: - pv_info = ctxt.get(pv, timeout=timeout).get("pvi").todict() - - result = {} - - for attr_name, attr_info in pv_info.items(): - result[attr_name] = PVIEntry(**attr_info) # type: ignore - - return result diff --git a/src/ophyd_async/panda/__init__.py b/src/ophyd_async/panda/__init__.py index 7fcdad714c..3f36f1f239 100644 --- a/src/ophyd_async/panda/__init__.py +++ b/src/ophyd_async/panda/__init__.py @@ -1,4 +1,4 @@ -from .panda import PandA, PcapBlock, PulseBlock, PVIEntry, SeqBlock, SeqTable, pvi +from .panda import PandA, PcapBlock, PulseBlock, PVIEntry, SeqBlock, SeqTable from .table import ( SeqTable, SeqTableRow, @@ -19,6 +19,5 @@ "SeqTable", "SeqTableRow", "SeqTrigger", - "pvi", "phase_sorter", ] diff --git a/src/ophyd_async/panda/panda.py b/src/ophyd_async/panda/panda.py index db8313a517..37e514a442 100644 --- a/src/ophyd_async/panda/panda.py +++ b/src/ophyd_async/panda/panda.py @@ -1,28 +1,12 @@ from __future__ import annotations -import atexit import re -from typing import ( - Callable, - Dict, - FrozenSet, - Optional, - Tuple, - Type, - TypedDict, - cast, - get_args, - get_origin, - get_type_hints, -) - -from p4p.client.thread import Context +from typing import Dict, Optional, Tuple, cast, get_args, get_origin, get_type_hints from ophyd_async.core import ( DEFAULT_TIMEOUT, Device, DeviceVector, - NotConnected, Signal, SignalBackend, SignalR, @@ -30,13 +14,7 @@ SignalX, SimSignalBackend, ) -from ophyd_async.epics.signal import ( - epics_signal_r, - epics_signal_rw, - epics_signal_w, - epics_signal_x, - pvi_get, -) +from ophyd_async.epics.pvi import PVIEntry, make_signal, pvi_get from ophyd_async.panda.table import SeqTable @@ -54,14 +32,6 @@ class PcapBlock(Device): active: SignalR[bool] -class PVIEntry(TypedDict, total=False): - d: str - r: str - rw: str - w: str - x: str - - def _block_name_number(block_name: str) -> Tuple[str, Optional[int]]: """Maps a panda block name to a block and number. @@ -80,7 +50,7 @@ def _block_name_number(block_name: str) -> Tuple[str, Optional[int]]: return block_name, None -def _remove_inconsistent_blocks(pvi_info: Dict[str, PVIEntry]) -> None: +def _remove_inconsistent_blocks(pvi_info: Optional[Dict[str, PVIEntry]]) -> None: """Remove blocks from pvi information. This is needed because some pandas have 'pcap' and 'pcap1' blocks, which are @@ -88,6 +58,8 @@ def _remove_inconsistent_blocks(pvi_info: Dict[str, PVIEntry]) -> None: for example. """ + if pvi_info is None: + return pvi_keys = set(pvi_info.keys()) for k in pvi_keys: kn = re.sub(r"\d*$", "", k) @@ -95,51 +67,14 @@ def _remove_inconsistent_blocks(pvi_info: Dict[str, PVIEntry]) -> None: del pvi_info[k] -async def pvi( - pv: str, ctxt: Context, timeout: float = DEFAULT_TIMEOUT -) -> Dict[str, PVIEntry]: - try: - result = await pvi_get(pv, ctxt, timeout=timeout) - except TimeoutError as exc: - raise NotConnected(pv) from exc - - _remove_inconsistent_blocks(result) - return result - - class PandA(Device): - _ctxt: Optional[Context] = None - pulse: DeviceVector[PulseBlock] seq: DeviceVector[SeqBlock] pcap: PcapBlock def __init__(self, prefix: str, name: str = "") -> None: super().__init__(name) - self._init_prefix = prefix - self.pvi_mapping: Dict[FrozenSet[str], Callable[..., Signal]] = { - frozenset({"r", "w"}): lambda dtype, rpv, wpv: epics_signal_rw( - dtype, rpv, wpv - ), - frozenset({"rw"}): lambda dtype, rpv, wpv: epics_signal_rw(dtype, rpv, wpv), - frozenset({"r"}): lambda dtype, rpv, wpv: epics_signal_r(dtype, rpv), - frozenset({"w"}): lambda dtype, rpv, wpv: epics_signal_w(dtype, wpv), - frozenset({"x"}): lambda dtype, rpv, wpv: epics_signal_x(wpv), - } - - @property - def ctxt(self) -> Context: - if PandA._ctxt is None: - PandA._ctxt = Context("pva", nt=False) - - @atexit.register - def _del_ctxt(): - # If we don't do this we get messages like this on close: - # Error in sys.excepthook: - # Original exception was: - PandA._ctxt = None - - return PandA._ctxt + self._prefix = prefix def verify_block(self, name: str, num: Optional[int]): """Given a block name and number, return information about a block.""" @@ -172,7 +107,7 @@ async def _make_block( block = self.verify_block(name, num) field_annos = get_type_hints(block, globalns=globals()) - block_pvi = await pvi(block_pv, self.ctxt, timeout=timeout) if not sim else None + block_pvi = await pvi_get(block_pv, timeout=timeout) if not sim else None # finds which fields this class actually has, e.g. delay, width... for sig_name, sig_type in field_annos.items(): @@ -181,7 +116,6 @@ async def _make_block( # if not in sim mode, if block_pvi: - block_pvi = cast(Dict[str, PVIEntry], block_pvi) # try to get this block in the pvi. entry: Optional[PVIEntry] = block_pvi.get(sig_name) if entry is None: @@ -190,7 +124,7 @@ async def _make_block( + f"an {sig_name} signal which has not been retrieved by PVI." ) - signal = self._make_signal(entry, args[0] if len(args) > 0 else None) + signal: Signal = make_signal(entry, args[0] if len(args) > 0 else None) else: backend: SignalBackend = SimSignalBackend( @@ -205,8 +139,7 @@ async def _make_block( for attr, attr_pvi in block_pvi.items(): if not hasattr(block, attr): # makes any extra signals - signal = self._make_signal(attr_pvi) - setattr(block, attr, signal) + setattr(block, attr, make_signal(attr_pvi)) return block @@ -219,28 +152,13 @@ async def _make_untyped_block( included dynamically anyway. """ block = Device() - block_pvi: Dict[str, PVIEntry] = await pvi(block_pv, self.ctxt, timeout=timeout) + block_pvi: Dict[str, PVIEntry] = await pvi_get(block_pv, timeout=timeout) for signal_name, signal_pvi in block_pvi.items(): - signal = self._make_signal(signal_pvi) - setattr(block, signal_name, signal) + setattr(block, signal_name, make_signal(signal_pvi)) return block - def _make_signal(self, signal_pvi: PVIEntry, dtype: Optional[Type] = None): - """Make a signal. - - This assumes datatype is None so it can be used to create dynamic signals. - """ - operations = frozenset(signal_pvi.keys()) - pvs = [signal_pvi[i] for i in operations] # type: ignore - signal_factory = self.pvi_mapping[operations] - - write_pv = pvs[0] - read_pv = write_pv if len(pvs) == 1 else pvs[1] - - return signal_factory(dtype, "pva://" + read_pv, "pva://" + write_pv) - # TODO redo to set_panda_block? confusing name def set_attribute(self, name: str, num: Optional[int], block: Device): """Set a block on the panda. @@ -269,10 +187,9 @@ async def connect( makes all required blocks. """ pvi_info = ( - await pvi(self._init_prefix + ":PVI", self.ctxt, timeout=timeout) - if not sim - else None + await pvi_get(self._prefix + "PVI", timeout=timeout) if not sim else None ) + _remove_inconsistent_blocks(pvi_info) hints = { attr_name: attr_type diff --git a/tests/epics/test_records.db b/tests/epics/test_records.db index 9fecbdb490..389fdc4c08 100644 --- a/tests/epics/test_records.db +++ b/tests/epics/test_records.db @@ -281,3 +281,27 @@ record(waveform, "$(P)ntndarray:data") } }) } + +record(ao, "$(P)width") +{ + info(Q:group, { + "$(P)pvi": { + "pvi.width.rw": { + "+channel": "NAME", + "+type": "plain" + } + } + }) +} + +record(ao, "$(P)height") +{ + info(Q:group, { + "$(P)pvi": { + "pvi.height.rw": { + "+channel": "NAME", + "+type": "plain" + } + } + }) +} diff --git a/tests/epics/test_signals.py b/tests/epics/test_signals.py index 2a5207acd8..e0a69f490e 100644 --- a/tests/epics/test_signals.py +++ b/tests/epics/test_signals.py @@ -1,5 +1,4 @@ import asyncio -import os import random import re import string @@ -10,7 +9,18 @@ from dataclasses import dataclass from enum import Enum from pathlib import Path -from typing import Any, Callable, Literal, Optional, Sequence, Tuple, Type, TypedDict +from typing import ( + Any, + Callable, + Dict, + Literal, + Optional, + Sequence, + Tuple, + Type, + TypedDict, +) +from unittest.mock import ANY import numpy as np import numpy.typing as npt @@ -27,12 +37,6 @@ PV_PREFIX = "".join(random.choice(string.ascii_lowercase) for _ in range(12)) -@pytest.fixture -def _ensure_removed(): - yield - os.remove("test.yaml") - - @dataclass class IOC: process: subprocess.Popen @@ -97,9 +101,12 @@ async def assert_updates(self, expected_value): "timestamp": pytest.approx(time.time(), rel=0.1), "alarm_severity": 0, } - reading, value = await self.updates.get() - assert value == expected_value == await self.backend.get_value() - assert reading == expected_reading == await self.backend.get_reading() + backend_reading = await asyncio.wait_for(self.backend.get_reading(), timeout=5) + reading, value = await asyncio.wait_for(self.updates.get(), timeout=5) + backend_value = await asyncio.wait_for(self.backend.get_value(), timeout=5) + + assert value == expected_value == backend_value + assert reading == expected_reading == backend_reading def close(self): self.backend.set_callback(None) @@ -191,13 +198,13 @@ def waveform_d(value): ], ) async def test_backend_get_put_monitor( - _ensure_removed: None, ioc: IOC, datatype: Type[T], suffix: str, initial_value: T, put_value: T, descriptor: Callable[[Any], dict], + tmp_path, ): # ca can't support all the types dtype = get_dtype(datatype) @@ -219,8 +226,9 @@ async def test_backend_get_put_monitor( ioc, suffix, descriptor(put_value), put_value, initial_value, datatype=None ) - save_to_yaml([{"test": put_value}], "test.yaml") - loaded = load_from_yaml("test.yaml") + yaml_path = tmp_path / "test.yaml" + save_to_yaml([{"test": put_value}], yaml_path) + loaded = load_from_yaml(yaml_path) assert np.all(loaded[0]["test"] == put_value) @@ -332,6 +340,39 @@ async def test_pva_table(ioc: IOC) -> None: q.close() +async def test_pvi_structure(ioc: IOC) -> None: + if ioc.protocol == "ca": + # CA can't do structure + return + # Make and connect the backend + backend = await ioc.make_backend(Dict[str, Any], "pvi") + + # Make a monitor queue that will monitor for updates + q = MonitorQueue(backend) + + expected = { + "pvi": { + "width": { + "rw": f"{PV_PREFIX}:{ioc.protocol}:width", + }, + "height": { + "rw": f"{PV_PREFIX}:{ioc.protocol}:height", + }, + }, + "record": ANY, + } + + try: + # Check descriptor + with pytest.raises(NotImplementedError): + await backend.get_descriptor() + # Check initial value + await q.assert_updates(expected) + + finally: + q.close() + + async def test_pva_ntdarray(ioc: IOC): if ioc.protocol == "ca": # CA can't do ndarray diff --git a/tests/panda/test_panda.py b/tests/panda/test_panda.py index 11fd288755..8fcf6836bd 100644 --- a/tests/panda/test_panda.py +++ b/tests/panda/test_panda.py @@ -1,7 +1,6 @@ """Test file specifying how we want to eventually interact with the panda...""" import copy -import traceback from typing import Dict import numpy as np @@ -9,7 +8,8 @@ from ophyd_async.core import DeviceCollector from ophyd_async.core.utils import NotConnected -from ophyd_async.panda import PandA, PVIEntry, SeqTable, SeqTrigger, pvi +from ophyd_async.panda import PandA, PVIEntry, SeqTable, SeqTrigger +from ophyd_async.panda.panda import _remove_inconsistent_blocks class DummyDict: @@ -39,7 +39,7 @@ def get(self, pv: str, timeout: float = 0.0): @pytest.fixture async def sim_panda(): async with DeviceCollector(sim=True): - sim_panda = PandA("PANDAQSRV") + sim_panda = PandA("PANDAQSRV:", "sim_panda") assert sim_panda.name == "sim_panda" yield sim_panda @@ -55,7 +55,7 @@ def test_panda_name_set(): assert panda.name == "panda" -async def test_pvi_get_for_inconsistent_blocks(): +async def test_inconsistent_blocks(): dummy_pvi = { "pcap": {}, "pcap1": {}, @@ -65,9 +65,9 @@ async def test_pvi_get_for_inconsistent_blocks(): "sfp3_sync_out": {}, } - resulting_pvi = await pvi("", MockCtxt(dummy_pvi)) - assert "sfp3_sync_out1" not in resulting_pvi - assert "pcap1" not in resulting_pvi + _remove_inconsistent_blocks(dummy_pvi) + assert "sfp3_sync_out1" not in dummy_pvi + assert "pcap1" not in dummy_pvi async def test_panda_children_connected(sim_panda: PandA): @@ -107,13 +107,13 @@ async def test_panda_children_connected(sim_panda: PandA): async def test_panda_with_missing_blocks(pva): - panda = PandA("PANDAQSRVI") + panda = PandA("PANDAQSRVI:") with pytest.raises(AssertionError): await panda.connect() async def test_panda_with_extra_blocks_and_signals(pva): - panda = PandA("PANDAQSRV") + panda = PandA("PANDAQSRV:") await panda.connect() assert panda.extra # type: ignore @@ -123,7 +123,7 @@ async def test_panda_with_extra_blocks_and_signals(pva): async def test_panda_block_missing_signals(pva): - panda = PandA("PANDAQSRVIB") + panda = PandA("PANDAQSRVIB:") with pytest.raises(Exception) as exc: await panda.connect() @@ -135,26 +135,9 @@ async def test_panda_block_missing_signals(pva): async def test_panda_unable_to_connect_to_pvi(): - panda = PandA("pva://NON-EXISTENT") + panda = PandA("NON-EXISTENT:") with pytest.raises(NotConnected) as exc: await panda.connect(timeout=0.01) assert exc.value._errors == "pva://NON-EXISTENT:PVI" - - files = [ - __file__, - "ophyd_async/panda/panda.py", - "ophyd_async/panda/panda.py", - ] - funcs = ["test_panda_unable_to_connect_to_pvi", "connect", "pvi"] - lines = [ - "await panda.connect(timeout=0.01)", - 'await pvi(self._init_prefix + ":PVI", self.ctxt, timeout=timeout)', - "raise NotConnected(pv) from exc", - ] - - for idx, each_frame in enumerate(traceback.extract_tb(exc.tb)): - assert each_frame.filename.endswith(files[idx]) - assert each_frame.name == funcs[idx] - assert each_frame.line == lines[idx]