diff --git a/.vscode/launch.json b/.vscode/launch.json index 21c0b227..39175105 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -9,7 +9,8 @@ "type": "python", "request": "launch", "program": "${file}", - "console": "integratedTerminal" + "console": "integratedTerminal", + "justMyCode": false, }, { "name": "Debug Unit Test", diff --git a/CHANGELOG.rst b/CHANGELOG.rst index db3d872b..9c8ba245 100644 --- a/CHANGELOG.rst +++ b/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 diff --git a/docs/reference/api.rst b/docs/reference/api.rst index 22094b25..8b7a23bc 100644 --- a/docs/reference/api.rst +++ b/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. diff --git a/softioc/builder.py b/softioc/builder.py index 6db5fa08..70ae8a1c 100644 --- a/softioc/builder.py +++ b/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) diff --git a/softioc/device.py b/softioc/device.py index 7a577c29..4fcb5add 100644 --- a/softioc/device.py +++ b/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 diff --git a/tests/test_record_values.py b/tests/test_record_values.py index bf36ce12..b3c882ed 100644 --- a/tests/test_record_values.py +++ b/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])