Made panda use pva signal backend instead of pvi_get

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}")
 

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)

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",

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",
 ]

src/ophyd_async/panda/panda.py

@@ -1,42 +1,20 @@
 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,
     SignalRW,
     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,66 +50,31 @@ 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
     inconsistent with the assumption that pandas should only have a 'pcap' block,
     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)
         if kn and k != kn and kn in pvi_keys:
             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