From 3e3d20ede977f0c857b9111deeeda60d6b874573 Mon Sep 17 00:00:00 2001 From: AlexWells Date: Fri, 24 Mar 2023 15:47:42 +0000 Subject: [PATCH] Add exception on overlong string setting Also add more tests --- .vscode/launch.json | 3 +- CHANGELOG.rst | 1 + docs/reference/api.rst | 5 ++ softioc/builder.py | 8 ++-- softioc/device.py | 26 ++++++----- softioc/fields.py | 1 - tests/test_record_values.py | 91 +++++++++++++++++++++++++++++++++---- 7 files changed, 109 insertions(+), 26 deletions(-) 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 bff680ac..e978a2d3 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -26,6 +26,7 @@ Added: - `Allow "status" and "severity" on In record init <../../pull/111>`_ - `Allow users to reset the list of created records <../../pull/114>`_ +- `Allow arrays of strings to be used with Waveforms <../../pull/102>`_ 4.1.0_ - 2022-08-05 ------------------- diff --git a/docs/reference/api.rst b/docs/reference/api.rst index 22094b25..ae85f200 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:: + When storing arrays of strings, it is possible to store Unicode characters. + However, as EPICS has no Unicode support the resultant values will be stored + as byte strings. Care must be taken when encoding/decoding the values. + The following functions generates specialised records. diff --git a/softioc/builder.py b/softioc/builder.py index 8fa42ba6..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,7 +148,7 @@ def Action(name, **fields): 'uint32': 'ULONG', 'float32': 'FLOAT', 'float64': 'DOUBLE', - 'bytes320': 'STRING', # Numpy's term for a 40-character string (40*8 bits) + 'bytes320': 'STRING', # Numpy term for 40-character byte str (40*8 bits) } # Coverts FTVL string to numpy type @@ -220,8 +220,8 @@ def _waveform(value, fields): 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 = numpy.require(initial_value, numpy.dtype("S40")) + 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 f5dbcfdc..ad5b8c00 100644 --- a/softioc/device.py +++ b/softioc/device.py @@ -351,6 +351,20 @@ 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): @@ -358,13 +372,7 @@ def _require_waveform(value, dtype): value = numpy.frombuffer(value, dtype = numpy.uint8) if dtype and dtype.char == 'S': - result = numpy.empty(len(value), 'S40') - for n, s in enumerate(value): - if isinstance(s, str): - result[n] = s.encode('UTF-8') - else: - result[n] = s - return result + return to_epics_str_array(value) value = numpy.require(value, dtype = dtype) if value.shape == (): @@ -419,10 +427,6 @@ def _value_to_epics(self, value): # common class of bug, at the cost of duplicated code and data, here we # ensure a copy is taken of the value. assert len(value) <= self._nelm, 'Value too long for waveform' - # TODO: line below triggers "DeprecationWarning: Applying '+' to a - # non-numerical array is ill-defined. Returning a copy, but in the - # future this will error." - # Probably should change to numpy.copy? return +value def _epics_to_value(self, value): diff --git a/softioc/fields.py b/softioc/fields.py index 648c61a5..7725e081 100644 --- a/softioc/fields.py +++ b/softioc/fields.py @@ -146,5 +146,4 @@ def __set_time(self, address, new_time): address[0].secs -= EPICS_epoch -# TODO: DbfCodeToNumpy doesn't exist! __all__ = ['RecordFactory', 'DbfCodeToNumpy', 'ca_timestamp'] diff --git a/tests/test_record_values.py b/tests/test_record_values.py index 84465d8d..9d040f8a 100644 --- a/tests/test_record_values.py +++ b/tests/test_record_values.py @@ -32,7 +32,7 @@ "string and so lets test it and prove that shall we?" # The numpy dtype of all arrays of strings -DTYPE_STRING = "S40" +NUMPY_DTYPE_STRING = "S40" def record_func_names(fixture_value): @@ -194,15 +194,12 @@ def record_values_names(fixture_value): ), numpy.ndarray, ), - - # TODO: Unicode versions of below tests? - ( "wIn_byte_string_array", builder.WaveformIn, [b"AB123", b"CD456", b"EF789"], numpy.array( - ["AB123", "CD456", "EF789"], dtype=DTYPE_STRING + ["AB123", "CD456", "EF789"], dtype=NUMPY_DTYPE_STRING ), numpy.ndarray, ), @@ -211,7 +208,35 @@ def record_values_names(fixture_value): builder.WaveformOut, [b"12AB", b"34CD", b"56EF"], numpy.array( - ["12AB", "34CD", "56EF"], dtype=DTYPE_STRING + ["12AB", "34CD", "56EF"], dtype=NUMPY_DTYPE_STRING + ), + numpy.ndarray, + ), + ( + "wIn_unicode_string_array", + builder.WaveformIn, + ["12€½", "34¾²", "56¹³"], + numpy.array( + [ + b'12\xe2\x82\xac\xc2\xbd', + b'34\xc2\xbe\xc2\xb2', + b'56\xc2\xb9\xc2\xb3' + ], + dtype=NUMPY_DTYPE_STRING + ), + numpy.ndarray, + ), + ( + "wOut_unicode_string_array", + builder.WaveformOut, + ["12€½", "34¾²", "56¹³"], + numpy.array( + [ + b'12\xe2\x82\xac\xc2\xbd', + b'34\xc2\xbe\xc2\xb2', + b'56\xc2\xb9\xc2\xb3' + ], + dtype=NUMPY_DTYPE_STRING ), numpy.ndarray, ), @@ -220,7 +245,7 @@ def record_values_names(fixture_value): builder.WaveformIn, ["123abc", "456def", "7890ghi"], numpy.array( - ["123abc", "456def", "7890ghi"], dtype=DTYPE_STRING + ["123abc", "456def", "7890ghi"], dtype=NUMPY_DTYPE_STRING ), numpy.ndarray, ), @@ -229,7 +254,7 @@ def record_values_names(fixture_value): builder.WaveformOut, ["123abc", "456def", "7890ghi"], numpy.array( - ["123abc", "456def", "7890ghi"], dtype=DTYPE_STRING + ["123abc", "456def", "7890ghi"], dtype=NUMPY_DTYPE_STRING ), numpy.ndarray, ), @@ -539,10 +564,19 @@ def is_valid(configuration): "scalar. Therefore we skip this check.") continue + # caget on a waveform of strings will return unicode. Have to + # convert it manually to binary. if isinstance(rec_val, numpy.ndarray) and len(rec_val) > 1 \ and rec_val.dtype.char in ["S", "U"]: + result = numpy.empty(len(rec_val), NUMPY_DTYPE_STRING) + for n, s in enumerate(rec_val): + if isinstance(s, str): + result[n] = s.encode('UTF-8', errors= 'ignore') + else: + result[n] = s + rec_val = result # caget won't retrieve the full length 40 buffer - rec_val = rec_val.astype(DTYPE_STRING) + rec_val = rec_val.astype(NUMPY_DTYPE_STRING) record_value_asserts( creation_func, rec_val, expected_value, expected_type @@ -958,7 +992,46 @@ 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 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"] + ) + + 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(ValueError): + w_in.set([VERY_LONG_STRING]) + with pytest.raises(ValueError): + w_out.set([VERY_LONG_STRING])