Make pvi aware of DeviceVectors

src/ophyd_async/core/_device_filler.py

@@ -1,6 +1,5 @@
 from __future__ import annotations
 
-import re
 from abc import abstractmethod
 from collections.abc import Callable, Iterator, Sequence
 from typing import (
@@ -10,6 +9,7 @@
     NoReturn,
     Protocol,
     TypeVar,
+    cast,
     get_args,
     get_type_hints,
     runtime_checkable,
@@ -28,19 +28,6 @@
 LogicalName = NewType("LogicalName", str)
 
 
-def _strip_number_from_string(string: str) -> tuple[str, int | None]:
-    """Return ("pulse", 1) from "pulse1"."""
-    match = re.match(r"(.*?)(\d*)$", string)
-    assert match
-
-    name = match.group(1)
-    number = match.group(2) or None
-    if number is None:
-        return name, None
-    else:
-        return name, int(number)
-
-
 def _get_datatype(annotation: Any) -> type | None:
     """Return int from SignalRW[int]."""
     args = get_args(annotation)
@@ -187,14 +174,13 @@ def create_devices_from_annotations(
             dest[_logical(name)] = connector
 
     def create_device_vector_entries_to_mock(self, num: int):
-        for basename, cls in self._vector_device_type.items():
+        for name, cls in self._vector_device_type.items():
             assert cls, "Shouldn't happen"
-            for i in range(num):
-                name = f"{basename}{i + 1}"
+            for i in range(1, num + 1):
                 if issubclass(cls, Signal):
-                    self.fill_child_signal(name, cls)
+                    self.fill_child_signal(name, cls, i)
                 elif issubclass(cls, Device):
-                    self.fill_child_device(name, cls)
+                    self.fill_child_device(name, cls, i)
                 else:
                     self._raise(name, f"Can't make {cls}")
 
@@ -205,52 +191,44 @@ def check_filled(self, source: str):
                 f"{self._device.name}: cannot provision {unfilled} from {source}"
             )
 
-    def ensure_device_vectors(self, names: list[str]):
-        basenames: dict[LogicalName, set[int]] = {}
-        for name in names:
-            basename, number = _strip_number_from_string(name)
-            if number is not None:
-                basenames.setdefault(LogicalName(basename), set()).add(number)
-        for basename, numbers in basenames.items():
-            # If contiguous numbers starting at 1 then it's a device vector
-            length = len(numbers)
-            if (
-                length > 1
-                and numbers == set(range(1, length + 1))
-                and basename not in self._vector_device_type
-            ):
-                # We have no type hints, so use whatever we are told
-                self._vector_device_type[basename] = None
-                if hasattr(self._device, basename):
-                    self._raise(
-                        basename,
-                        "Cannot make child as it would shadow "
-                        f"{getattr(self._device, basename)}",
-                    )
-                setattr(self._device, basename, DeviceVector({}))
+    def _ensure_device_vector(self, name: LogicalName) -> DeviceVector:
+        if not hasattr(self._device, name):
+            # We have no type hints, so use whatever we are told
+            self._vector_device_type[name] = None
+            setattr(self._device, name, DeviceVector({}))
+        vector = getattr(self._device, name)
+        if not isinstance(vector, DeviceVector):
+            self._raise(name, f"Expected DeviceVector, got {vector}")
+        return vector
 
-    def fill_child_signal(self, name: str, signal_type: type[Signal]) -> SignalBackendT:
-        basename, number = _strip_number_from_string(name)
-        child = getattr(self._device, name, None)
+    def fill_child_signal(
+        self,
+        name: str,
+        signal_type: type[Signal],
+        vector_index: int | None = None,
+    ) -> SignalBackendT:
+        name = cast(LogicalName, name)
         if name in self._unfilled_backends:
             # We made it above
             backend, expected_signal_type = self._unfilled_backends.pop(name)
             self._filled_backends[name] = backend, expected_signal_type
         elif name in self._filled_backends:
             # We made it and filled it so return for validation
             backend, expected_signal_type = self._filled_backends[name]
-        elif basename in self._vector_device_type and isinstance(number, int):
-            # We need to add a new entry to an existing DeviceVector
-            backend = self._signal_backend_factory(self._signal_datatype[basename])
-            expected_signal_type = self._vector_device_type[basename] or signal_type
-            getattr(self._device, basename)[number] = signal_type(backend)
-        elif child is None:
+        elif vector_index:
+            # We need to add a new entry to a DeviceVector
+            vector = self._ensure_device_vector(name)
+            backend = self._signal_backend_factory(self._signal_datatype[name])
+            expected_signal_type = self._vector_device_type[name] or signal_type
+            vector[vector_index] = signal_type(backend)
+        elif child := getattr(self._device, name, None):
+            # There is an existing child, so raise
+            self._raise(name, f"Cannot make child as it would shadow {child}")
+        else:
             # We need to add a new child to the top level Device
-            backend = self._signal_backend_factory(datatype=None)
+            backend = self._signal_backend_factory(None)
             expected_signal_type = signal_type
             setattr(self._device, name, signal_type(backend))
-        else:
-            self._raise(name, f"Cannot make child as it would shadow {child}")
         if signal_type is not expected_signal_type:
             self._raise(
                 name,
@@ -259,31 +237,33 @@ def fill_child_signal(self, name: str, signal_type: type[Signal]) -> SignalBacke
         return backend
 
     def fill_child_device(
-        self, name: str, device_type: type[Device] = Device
+        self,
+        name: str,
+        device_type: type[Device] = Device,
+        vector_index: int | None = None,
     ) -> DeviceConnectorT:
-        basename, number = _strip_number_from_string(name)
-        child = getattr(self._device, name, None)
+        name = cast(LogicalName, name)
         if name in self._unfilled_connectors:
             # We made it above
             connector = self._unfilled_connectors.pop(name)
             self._filled_connectors[name] = connector
         elif name in self._filled_backends:
             # We made it and filled it so return for validation
             connector = self._filled_connectors[name]
-        elif basename in self._vector_device_type and isinstance(number, int):
-            # We need to add a new entry to an existing DeviceVector
-            vector_device_type = self._vector_device_type[basename] or device_type
+        elif vector_index:
+            # We need to add a new entry to a DeviceVector
+            vector = self._ensure_device_vector(name)
+            vector_device_type = self._vector_device_type[name] or device_type
             assert issubclass(
                 vector_device_type, Device
             ), f"{vector_device_type} is not a Device"
             connector = self._device_connector_factory()
-            getattr(self._device, basename)[number] = vector_device_type(
-                connector=connector
-            )
-        elif child is None:
+            vector[vector_index] = vector_device_type(connector=connector)
+        elif child := getattr(self._device, name, None):
+            # There is an existing child, so raise
+            self._raise(name, f"Cannot make child as it would shadow {child}")
+        else:
             # We need to add a new child to the top level Device
             connector = self._device_connector_factory()
             setattr(self._device, name, device_type(connector=connector))
-        else:
-            self._raise(name, f"Cannot make child as it would shadow {child}")
         return connector

src/ophyd_async/epics/core/_pvi_connector.py

@@ -13,8 +13,10 @@
 from ._epics_connector import fill_backend_with_prefix
 from ._signal import PvaSignalBackend, pvget_with_timeout
 
+Entry = dict[str, str]
 
-def _get_signal_details(entry: dict[str, str]) -> tuple[type[Signal], str, str]:
+
+def _get_signal_details(entry: Entry) -> tuple[type[Signal], str, str]:
     match entry:
         case {"r": read_pv}:
             return SignalR, read_pv, read_pv
@@ -50,6 +52,16 @@ def create_children_from_annotations(self, device: Device):
                 fill_backend_with_prefix(self.prefix, backend, annotations)
             self.filler.check_created()
 
+    def _fill_child(self, name: str, entry: Entry, vector_index: int | None = None):
+        if set(entry) == {"d"}:
+            connector = self.filler.fill_child_device(name, vector_index=vector_index)
+            connector.pvi_pv = entry["d"]
+        else:
+            signal_type, read_pv, write_pv = _get_signal_details(entry)
+            backend = self.filler.fill_child_signal(name, signal_type, vector_index)
+            backend.read_pv = read_pv
+            backend.write_pv = write_pv
+
     async def connect(
         self, device: Device, mock: bool, timeout: float, force_reconnect: bool
     ) -> None:
@@ -58,18 +70,19 @@ async def connect(
             self.filler.create_device_vector_entries_to_mock(2)
         else:
             pvi_structure = await pvget_with_timeout(self.pvi_pv, timeout)
-            entries: dict[str, dict[str, str]] = pvi_structure["value"].todict()
-            # Ensure we have device vectors for everything that should be there
-            self.filler.ensure_device_vectors(list(entries))
+            entries: dict[str, Entry | list[Entry | None]] = pvi_structure[
+                "value"
+            ].todict()
+            # Fill based on what PVI gives us
             for name, entry in entries.items():
-                if set(entry) == {"d"}:
-                    connector = self.filler.fill_child_device(name)
-                    connector.pvi_pv = entry["d"]
+                if isinstance(entry, dict):
+                    # This is a child
+                    self._fill_child(name, entry)
                 else:
-                    signal_type, read_pv, write_pv = _get_signal_details(entry)
-                    backend = self.filler.fill_child_signal(name, signal_type)
-                    backend.read_pv = read_pv
-                    backend.write_pv = write_pv
+                    # This is a DeviceVector of children
+                    for i, e in enumerate(entry):
+                        if e:
+                            self._fill_child(name, e, i)
             # Check that all the requested children have been filled
             self.filler.check_filled(f"{self.pvi_pv}: {entries}")
         # Set the name of the device to name all children

tests/fastcs/panda/db/panda.db

@@ -122,7 +122,7 @@ record(stringin, "$(IOC_NAME=PANDAQSRV):PULSE1:_PVI")
     field(VAL,  "$(IOC_NAME=PANDAQSRV):PULSE1:PVI")
     info(Q:group, {
         "$(IOC_NAME=PANDAQSRV):PVI": {
-            "value.pulse1.d": {
+            "value.pulse[1].d": {
                 "+channel": "VAL",
                 "+type": "plain"
             }
@@ -441,7 +441,7 @@ record(stringin, "$(IOC_NAME=PANDAQSRV):SEQ1:_PVI")
     field(VAL,  "$(IOC_NAME=PANDAQSRV):SEQ1:PVI")
     info(Q:group, {
         "$(IOC_NAME=PANDAQSRV):PVI": {
-            "value.seq1.d": {
+            "value.seq[1].d": {
                 "+channel": "VAL",
                 "+type": "plain",
                 "+putorder":18
@@ -578,7 +578,7 @@ $(INCLUDE_EXTRA_BLOCK=#){
 $(INCLUDE_EXTRA_BLOCK=#)    field(VAL,  "$(IOC_NAME=PANDAQSRV):EXTRA1:PVI")
 $(INCLUDE_EXTRA_BLOCK=#)    info(Q:group, {
 $(INCLUDE_EXTRA_BLOCK=#)        "$(IOC_NAME=PANDAQSRV):PVI": {
-$(INCLUDE_EXTRA_BLOCK=#)            "value.extra1.d": {
+$(INCLUDE_EXTRA_BLOCK=#)            "value.extra[1].d": {
 $(INCLUDE_EXTRA_BLOCK=#)                "+channel": "VAL",
 $(INCLUDE_EXTRA_BLOCK=#)                "+type": "plain"
 $(INCLUDE_EXTRA_BLOCK=#)            }
@@ -604,7 +604,7 @@ $(INCLUDE_EXTRA_BLOCK=#){
 $(INCLUDE_EXTRA_BLOCK=#)    field(VAL,  "$(IOC_NAME=PANDAQSRV):EXTRA2:PVI")
 $(INCLUDE_EXTRA_BLOCK=#)    info(Q:group, {
 $(INCLUDE_EXTRA_BLOCK=#)        "$(IOC_NAME=PANDAQSRV):PVI": {
-$(INCLUDE_EXTRA_BLOCK=#)            "value.extra2.d": {
+$(INCLUDE_EXTRA_BLOCK=#)            "value.extra[2].d": {
 $(INCLUDE_EXTRA_BLOCK=#)                "+channel": "VAL",
 $(INCLUDE_EXTRA_BLOCK=#)                "+type": "plain"
 $(INCLUDE_EXTRA_BLOCK=#)            }

tests/fastcs/panda/test_panda_connect.py

@@ -122,8 +122,9 @@ async def test_panda_with_missing_blocks(panda_pva, panda_t):
     with pytest.raises(
         RuntimeError,
         match=re.escape(
-            "mypanda: cannot provision ['pcap'] from PANDAQSRVI:PVI: {'pulse1': "
-            "{'d': 'PANDAQSRVI:PULSE1:PVI'}, 'seq1': {'d': 'PANDAQSRVI:SEQ1:PVI'}}"
+            "mypanda: cannot provision ['pcap'] from PANDAQSRVI:PVI: "
+            "{'pulse': [None, {'d': 'PANDAQSRVI:PULSE1:PVI'}],"
+            " 'seq': [None, {'d': 'PANDAQSRVI:SEQ1:PVI'}]}"
         ),
     ):
         await panda.connect()