Skip to content

Commit

Permalink
Add exception on overlong string setting
Browse files Browse the repository at this point in the history
Also add more tests
  • Loading branch information
AlexanderWells-diamond committed Mar 29, 2023
1 parent 3bb01fa commit 3e3d20e
Show file tree
Hide file tree
Showing 7 changed files with 109 additions and 26 deletions.
3 changes: 2 additions & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
"type": "python",
"request": "launch",
"program": "${file}",
"console": "integratedTerminal"
"console": "integratedTerminal",
"justMyCode": false,
},
{
"name": "Debug Unit Test",
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
-------------------
Expand Down
5 changes: 5 additions & 0 deletions docs/reference/api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
8 changes: 4 additions & 4 deletions softioc/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
26 changes: 15 additions & 11 deletions softioc/device.py
Original file line number Diff line number Diff line change
Expand Up @@ -351,20 +351,28 @@ 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)

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 == ():
Expand Down Expand Up @@ -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):
Expand Down
1 change: 0 additions & 1 deletion softioc/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -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']
91 changes: 82 additions & 9 deletions tests/test_record_values.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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,
),
Expand All @@ -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,
),
Expand All @@ -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,
),
Expand All @@ -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,
),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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])

0 comments on commit 3e3d20e

Please sign in to comment.