Allow arrays of strings to be used with Waveforms

.vscode/launch.json

@@ -9,7 +9,8 @@
             "type": "python",
             "request": "launch",
             "program": "${file}",
-            "console": "integratedTerminal"
+            "console": "integratedTerminal",
+            "justMyCode": false,
         },
         {
             "name": "Debug Unit Test",

CHANGELOG.rst

@@ -17,6 +17,7 @@ Added:
 
 Fixed:
 
+- `Allow arrays of strings to be used with Waveforms <../../pull/102>`_
 - `Fix DeprecationWarning from numpy when using "+" <../../pull/123>`_
 
 4.2.0_ - 2022-11-08

docs/reference/api.rst

@@ -342,6 +342,11 @@ All functions return a wrapped `ProcessDeviceSupportIn` or
     field type name.  Otherwise the field type is taken from the initial value
     if given, or defaults to ``'FLOAT'``.
 
+    .. note::
+        Storing arrays of strings differs from other values. String arrays will always
+        be assumed to be encoded as Unicode strings, and will be returned to the user
+        as a Python list rather than a Numpy array.
+
 
 The following functions generates specialised records.
 

softioc/builder.py

@@ -11,7 +11,7 @@
 
 from . import device, pythonSoftIoc  # noqa
 # Re-export this so users only have to import the builder
-from .device import SetBlocking # noqa
+from .device import SetBlocking, to_epics_str_array # noqa
 
 PythonDevice = pythonSoftIoc.PythonDevice()
 
@@ -148,6 +148,7 @@ def Action(name, **fields):
     'uint32':   'ULONG',
     'float32':  'FLOAT',
     'float64':  'DOUBLE',
+    'bytes320': 'STRING',  # Numpy term for 40-character byte str (40*8 bits)
 }
 
 # Coverts FTVL string to numpy type
@@ -160,6 +161,7 @@ def Action(name, **fields):
     'ULONG':    'uint32',
     'FLOAT':    'float32',
     'DOUBLE':   'float64',
+    'STRING':   'S40',
 }
 
 
@@ -211,11 +213,15 @@ def _waveform(value, fields):
 
         # Special case for [u]int64: if the initial value comes in as 64 bit
         # integers we cannot represent that, so recast it as [u]int32
+        # Special case for array of strings to mark each element as conforming
+        # to EPICS 40-character string limit
         if datatype is None:
             if initial_value.dtype == numpy.int64:
                 initial_value = numpy.require(initial_value, numpy.int32)
             elif initial_value.dtype == numpy.uint64:
                 initial_value = numpy.require(initial_value, numpy.uint32)
+            elif initial_value.dtype.char in ('S', 'U'):
+                initial_value = to_epics_str_array(initial_value)
     else:
         initial_value = numpy.array([], dtype = datatype)
         length = _get_length(fields)

softioc/device.py

@@ -351,16 +351,34 @@ class ao(ProcessDeviceSupportOut):
     _ctype_ = c_double
     _dbf_type_ = fields.DBF_DOUBLE
 
+def to_epics_str_array(value):
+    """Convert the given array of Python strings to an array of EPICS
+    nul-terminated strings"""
+    result = numpy.empty(len(value), 'S40')
+
+    for n, s in enumerate(value):
+        if isinstance(s, str):
+            val = EpicsString._ctype_()
+            val.value = s.encode() + b'\0'
+            result[n] = val.value
+        else:
+            result[n] = s
+    return result
+
 
 def _require_waveform(value, dtype):
-    if isinstance(value, bytes):
-        # Special case hack for byte arrays.  Surprisingly tricky:
-        value = numpy.frombuffer(value, dtype = numpy.uint8)
-    value = numpy.require(value, dtype = dtype)
-    if value.shape == ():
-        value.shape = (1,)
-    assert value.ndim == 1, 'Can\'t write multidimensional arrays'
-    return value
+    if dtype and dtype.char == 'S':
+        return to_epics_str_array(value)
+    else:
+        if isinstance(value, bytes):
+            # Special case hack for byte arrays.  Surprisingly tricky:
+            value = numpy.frombuffer(value, dtype = numpy.uint8)
+
+        value = numpy.require(value, dtype = dtype)
+        if value.shape == ():
+            value.shape = (1,)
+        assert value.ndim == 1, 'Can\'t write multidimensional arrays'
+        return value
 
 
 class WaveformBase(ProcessDeviceSupportCore):
@@ -412,7 +430,10 @@ def _value_to_epics(self, value):
         return numpy.copy(value)
 
     def _epics_to_value(self, value):
-        return value
+        if self._dtype.char == 'S':
+            return [_string_at(s, 40) for s in value]
+        else:
+            return value
 
     def _value_to_dbr(self, value):
         return self._dbf_type_, len(value), value.ctypes.data, value

tests/test_record_values.py

@@ -189,6 +189,76 @@ def record_values_names(fixture_value):
         ),
         numpy.ndarray,
     ),
+    (
+        "wIn_byte_string_array",
+        builder.WaveformIn,
+        [b"AB123", b"CD456", b"EF789"],
+        ["AB123", "CD456", "EF789"],
+        list,
+    ),
+    (
+        "wOut_byte_string_array",
+        builder.WaveformOut,
+        [b"12AB", b"34CD", b"56EF"],
+        ["12AB", "34CD", "56EF"],
+        list,
+    ),
+    (
+        "wIn_unicode_string_array",
+        builder.WaveformIn,
+        ["12€½", "34¾²", "56¹³"],
+        ["12€½", "34¾²", "56¹³"],
+        list,
+    ),
+    (
+        "wOut_unicode_string_array",
+        builder.WaveformOut,
+        ["12€½", "34¾²", "56¹³"],
+        ["12€½", "34¾²", "56¹³"],
+        list,
+    ),
+    (
+        "wIn_string_array",
+        builder.WaveformIn,
+        ["123abc", "456def", "7890ghi"],
+        ["123abc", "456def", "7890ghi"],
+        list,
+    ),
+    (
+        "wOut_string_array",
+        builder.WaveformOut,
+        ["123abc", "456def", "7890ghi"],
+        ["123abc", "456def", "7890ghi"],
+        list,
+    ),
+    (
+        "wIn_mixed_array_1",
+        builder.WaveformIn,
+        ["123abc", 456, "7890ghi"],
+        ["123abc", "456", "7890ghi"],
+        list,
+    ),
+    (
+        "wOut_mixed_array_1",
+        builder.WaveformOut,
+        ["123abc", 456, "7890ghi"],
+        ["123abc", "456", "7890ghi"],
+        list,
+    ),
+    (
+        "wIn_mixed_array_2",
+        builder.WaveformIn,
+        [123, 456, "7890ghi"],
+        ["123", "456", "7890ghi"],
+        list,
+    ),
+    (
+        "wOut_mixed_array_2",
+        builder.WaveformOut,
+        [123, 456, "7890ghi"],
+        ["123", "456", "7890ghi"],
+        list,
+    ),
     (
         "longStringIn_str",
         builder.longStringIn,
@@ -239,6 +309,7 @@ def record_values(request):
     """A list of parameters for record value setting/getting tests.
 
     Fields are:
+    - Record name
     - Record builder function
     - Input value passed to .set()/initial_value/caput
     - Expected output value after doing .get()/caget
@@ -376,8 +447,31 @@ def run_test_function(
     expected value. set_enum and get_enum determine when the record's value is
     set and how the value is retrieved, respectively."""
 
-    ctx = get_multiprocessing_context()
+    def is_valid(configuration):
+        """Remove some cases that cannot succeed.
+        Waveforms of Strings must have the value specified as initial value."""
+        (
+            record_name,
+            creation_func,
+            initial_value,
+            expected_value,
+            expected_type,
+        ) = configuration
+
+        if creation_func in (builder.WaveformIn, builder.WaveformOut):
+            if isinstance(initial_value, list) and \
+                    any(isinstance(val, (str, bytes)) for val in initial_value):
+                if set_enum is not SetValueEnum.INITIAL_VALUE:
+                    print(f"Removing {configuration}")
+                    return False
 
+        return True
+
+    record_configurations = [
+        x for x in record_configurations if is_valid(x)
+    ]
+
+    ctx = get_multiprocessing_context()
     parent_conn, child_conn = ctx.Pipe()
 
     ioc_process = ctx.Process(
@@ -462,6 +556,7 @@ def run_test_function(
 
                 if (
                     creation_func in [builder.WaveformOut, builder.WaveformIn]
+                    and hasattr(expected_value, 'dtype')
                     and expected_value.dtype in [numpy.float64, numpy.int32]
                 ):
                     log(
@@ -470,6 +565,11 @@ def run_test_function(
                         "scalar. Therefore we skip this check.")
                     continue
 
+                # caget on a waveform of strings will return a numpy array.
+                # Must extract it out to a list to match .get()
+                if isinstance(rec_val, numpy.ndarray) and len(rec_val) > 1 \
+                        and rec_val.dtype.char in ["S", "U"]:
+                    rec_val = [s for s in rec_val]
 
             record_value_asserts(
                 creation_func, rec_val, expected_value, expected_type
@@ -485,13 +585,6 @@ def run_test_function(
             pytest.fail("Process did not terminate")
 
 
-def skip_long_strings(record_values):
-    if (
-        record_values[0] in [builder.stringIn, builder.stringOut]
-        and len(record_values[1]) > 40
-    ):
-        pytest.skip("CAPut blocks strings > 40 characters.")
-
 
 class TestGetValue:
     """Tests that use .get() to check whether values applied with .set(),
@@ -509,6 +602,14 @@ def test_value_pre_init_set(self, record_values):
             expected_type,
         ) = record_values
 
+        if (
+            creation_func in [builder.WaveformIn, builder.WaveformOut] and
+            isinstance(initial_value, list) and
+            any(isinstance(s, (str, bytes)) for s in initial_value)
+        ):
+            pytest.skip("Cannot .set() a list of strings to a waveform, must"
+                        "initially specify using initial_value or FTVL")
+
         kwarg = {}
         if creation_func in [builder.WaveformIn, builder.WaveformOut]:
             kwarg = {"length": WAVEFORM_LENGTH}  # Must specify when no value
@@ -884,7 +985,48 @@ def test_waveform_rejects_overlong_values(self):
         w_in = builder.WaveformIn("W_IN", [1, 2, 3])
         w_out = builder.WaveformOut("W_OUT", [1, 2, 3])
 
+        w_in_str = builder.WaveformIn("W_IN_STR", ["ABC", "DEF"])
+        w_out_str = builder.WaveformOut("W_OUT_STR", ["ABC", "DEF"])
+
         with pytest.raises(AssertionError):
             w_in.set([1, 2, 3, 4])
         with pytest.raises(AssertionError):
             w_out.set([1, 2, 3, 4])
+        with pytest.raises(AssertionError):
+            w_in_str.set(["ABC", "DEF", "GHI"])
+        with pytest.raises(AssertionError):
+            w_out_str.set(["ABC", "DEF", "GHI"])
+
+    def test_waveform_rejects_late_strings(self):
+        """Test that a waveform won't allow a list of strings to be assigned
+        if no string list was given in initial waveform construction"""
+        w_in = builder.WaveformIn("W_IN", length=10)
+        w_out = builder.WaveformOut("W_OUT", length=10)
+
+        with pytest.raises(ValueError):
+            w_in.set(["ABC", "DEF"])
+        with pytest.raises(ValueError):
+            w_out.set(["ABC", "DEF"])
+
+    def test_waveform_rejects_long_array_of_strings(self):
+        """Test that a waveform of strings won't accept too long strings"""
+        w_in = builder.WaveformIn(
+            "W_IN",
+            initial_value=["123abc", "456def", "7890ghi"]
+        )
+        w_out = builder.WaveformIn(
+            "W_OUT",
+            initial_value=["123abc", "456def", "7890ghi"]
+        )
+
+        # Test putting too many elements
+        with pytest.raises(AssertionError):
+            w_in.set(["1", "2", "3", "4"])
+        with pytest.raises(AssertionError):
+            w_out.set(["1", "2", "3", "4"])
+
+        # Test putting too long a string
+        with pytest.raises(ValueError):
+            w_in.set([VERY_LONG_STRING])
+        with pytest.raises(ValueError):
+            w_out.set([VERY_LONG_STRING])