From 4edf6bda54c5870fc7646d990026dcb1655af4dd Mon Sep 17 00:00:00 2001 From: AlexWells Date: Thu, 13 Oct 2022 09:54:29 +0100 Subject: [PATCH 1/3] Allow "status" and "severity" on In record init Add new tests and refactor existing ones Add docs --- CHANGELOG.rst | 2 ++ docs/reference/api.rst | 15 +++++++++-- softioc/builder.py | 13 +++++++++ tests/test_records.py | 60 ++++++++++++++++++++++++++---------------- 4 files changed, 66 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 7f34159a..2422aaf6 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -10,6 +10,8 @@ Versioning `_. Unreleased_ ----------- +- `Allow "status" and "severity" on In record init <../../pull/111>`_ + 4.1.0_ - 2022-08-05 ------------------- diff --git a/docs/reference/api.rst b/docs/reference/api.rst index 2967c21d..1fe05be5 100644 --- a/docs/reference/api.rst +++ b/docs/reference/api.rst @@ -165,6 +165,19 @@ Test Facilities`_ documentation for more details of each function. This is used to specify an initial value for each record. + .. _status and severity: + + `status and severity` + ~~~~~~~~~~~~~~~~~~~~~ + + Only available on IN records. These can be used with the alarm value enums + from `softioc.alarm` to set the initial alarm state of a record. + + .. note:: + + By default the builder module sets ``PINI`` to ``YES``, which means the + record will process at initialization and the alarm status will be reset. + .. _on_update: `on_update` @@ -243,8 +256,6 @@ Test Facilities`_ documentation for more details of each function. .. seealso:: `SetBlocking` for configuring a global default blocking value - - For all of these functions any EPICS database field can be assigned a value by passing it as a keyword argument for the corresponding field name (in upper case) or by assigning to the corresponding field of the returned record object. diff --git a/softioc/builder.py b/softioc/builder.py index 497e8b5e..062c58cb 100644 --- a/softioc/builder.py +++ b/softioc/builder.py @@ -1,5 +1,6 @@ import os import numpy + from .softioc import dbLoadDatabase from epicsdbbuilder import * @@ -24,10 +25,17 @@ def _set_in_defaults(fields): fields.setdefault('SCAN', 'I/O Intr') fields.setdefault('PINI', 'YES') fields.setdefault('DISP', 1) + _set_alarm(fields) def _set_out_defaults(fields): fields.setdefault('OMSL', 'supervisory') +def _set_alarm(fields): + if "status" in fields: + fields['STAT'] = _statStrings[fields.pop('status')] + if "severity" in fields: + fields['SEVR'] = _severityStrings[fields.pop('severity')] + # For longout and ao we want DRV{L,H} to match {L,H}OPR by default def _set_scalar_out_defaults(fields, DRVL, DRVH): fields['DRVL'] = DRVL @@ -76,6 +84,11 @@ def longOut(name, DRVL=None, DRVH=None, EGU=None, **fields): # All the severity strings supported by SV _severityStrings = ['NO_ALARM', 'MINOR', 'MAJOR', 'INVALID'] +_statStrings = [ + 'NO_ALARM', 'READ', 'WRITE', 'HIHI', 'HIGH', 'LOLO', 'LOW', 'STATE', 'COS', + 'COMM', 'TIMEOUT', 'HWLIMIT', 'CALC', 'SCAN', 'LINK', 'SOFT', 'BAD_SUB', + 'UDF', 'DISABLE', 'SIMM', 'READ_ACCESS', 'WRITE_ACCESS'] + # Converts a list of (option [,severity]) values or tuples into field settings # suitable for mbbi and mbbo records. def _process_mbb_values(options, fields): diff --git a/tests/test_records.py b/tests/test_records.py index 35d97d3b..26eeae54 100644 --- a/tests/test_records.py +++ b/tests/test_records.py @@ -17,6 +17,7 @@ ) from softioc import asyncio_dispatcher, builder, softioc +from softioc import alarm from softioc.device import SetBlocking # Test file for miscellaneous tests related to records @@ -24,6 +25,15 @@ # Test parameters DEVICE_NAME = "RECORD-TESTS" +in_records = [ + builder.aIn, + builder.boolIn, + builder.longIn, + builder.mbbIn, + builder.stringIn, + builder.WaveformIn, +] + def test_records(tmp_path): # Ensure we definitely unload all records that may be hanging over from # previous tests, then create exactly one instance of expected records. @@ -60,32 +70,18 @@ def test_enum_length_restriction(): "seventeen", ) - -def test_DISP_defaults_on(): +@pytest.mark.parametrize("creation_func", in_records) +def test_DISP_defaults_on(creation_func): """Test that all IN record types have DISP=1 set by default""" - in_records = [ - builder.aIn, - builder.boolIn, - builder.longIn, - builder.mbbIn, - builder.stringIn, - builder.WaveformIn, - ] - - record_counter = 0 + kwargs = {} - for creation_func in in_records: - kwargs = {} - record_counter += 1 - record_name = "DISP" + str(record_counter) + if creation_func == builder.WaveformIn: + kwargs = {"length": 1} - if creation_func == builder.WaveformIn: - kwargs = {"length": 1} + record = creation_func("RECORD", **kwargs) - record = creation_func(record_name, **kwargs) - - # Note: DISP attribute won't exist if field not specified - assert record.DISP.Value() == 1 + # Note: DISP attribute won't exist if field not specified + assert record.DISP.Value() == 1 def test_DISP_can_be_overridden(): @@ -183,6 +179,26 @@ def test_pini_always_on(): mbbi = builder.mbbIn("BBB", initial_value=5) assert mbbi.PINI.Value() == "YES" +@pytest.mark.parametrize("creation_func", in_records) +def test_setting_alarm_in_records(creation_func): + """Test that In records can have a custom alarm value set using the "status" + and "severity" keywords""" + kwargs = {} + if creation_func == builder.WaveformIn: + kwargs["length"] = 1 + + record = creation_func( + "NEW_RECORD", + severity=alarm.MINOR_ALARM, + status=alarm.LOLO_ALARM, + **kwargs + ) + + assert record.STAT.Value() == "LOLO" + assert record.SEVR.Value() == "MINOR" + + + def validate_fixture_names(params): """Provide nice names for the out_records fixture in TestValidate class""" return params[0].__name__ From 421068ef79216ccbab80045915578018230fc00b Mon Sep 17 00:00:00 2001 From: AlexWells Date: Fri, 14 Oct 2022 13:37:44 +0100 Subject: [PATCH 2/3] Disallow specifying both our and EPICS keywords This stops us from accidentally overriding a "STAT" value if there's also "status" defined, for example --- softioc/builder.py | 6 ++++-- tests/test_records.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/softioc/builder.py b/softioc/builder.py index 062c58cb..cc53b913 100644 --- a/softioc/builder.py +++ b/softioc/builder.py @@ -31,9 +31,11 @@ def _set_out_defaults(fields): fields.setdefault('OMSL', 'supervisory') def _set_alarm(fields): - if "status" in fields: + if 'status' in fields: + assert 'STAT' not in fields, 'Can\'t specify both status and STAT' fields['STAT'] = _statStrings[fields.pop('status')] - if "severity" in fields: + if 'severity' in fields: + assert 'SEVR' not in fields, 'Can\'t specify both severity and SEVR' fields['SEVR'] = _severityStrings[fields.pop('severity')] # For longout and ao we want DRV{L,H} to match {L,H}OPR by default diff --git a/tests/test_records.py b/tests/test_records.py index 26eeae54..5f2a5ecc 100644 --- a/tests/test_records.py +++ b/tests/test_records.py @@ -197,6 +197,36 @@ def test_setting_alarm_in_records(creation_func): assert record.STAT.Value() == "LOLO" assert record.SEVR.Value() == "MINOR" +@pytest.mark.parametrize("creation_func", in_records) +def test_setting_alarm_invalid_keywords(creation_func): + """Test that In records correctly block specifying both STAT and status, + and SEVR and severity""" + + kwargs = {} + if creation_func == builder.WaveformIn: + kwargs["length"] = 1 + + with pytest.raises(AssertionError) as e: + creation_func( + "NEW_RECORD", + severity=alarm.MINOR_ALARM, + SEVR="MINOR", + status=alarm.LOLO_ALARM, + **kwargs + ) + + assert e.value.args[0] == 'Can\'t specify both severity and SEVR' + + with pytest.raises(AssertionError) as e: + creation_func( + "NEW_RECORD", + severity=alarm.MINOR_ALARM, + status=alarm.LOLO_ALARM, + STAT="LOLO", + **kwargs + ) + + assert e.value.args[0] == 'Can\'t specify both status and STAT' def validate_fixture_names(params): From dc4a63f829ecf51e30138a0f1be92707f777fca6 Mon Sep 17 00:00:00 2001 From: AlexWells Date: Fri, 14 Oct 2022 13:46:40 +0100 Subject: [PATCH 3/3] Update documentation after code review --- docs/reference/api.rst | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/docs/reference/api.rst b/docs/reference/api.rst index 1fe05be5..062200f3 100644 --- a/docs/reference/api.rst +++ b/docs/reference/api.rst @@ -165,18 +165,16 @@ Test Facilities`_ documentation for more details of each function. This is used to specify an initial value for each record. - .. _status and severity: + .. _status, severity: - `status and severity` + `status, severity` ~~~~~~~~~~~~~~~~~~~~~ Only available on IN records. These can be used with the alarm value enums - from `softioc.alarm` to set the initial alarm state of a record. + from `softioc.alarm` to set the initial alarm state and severity of a record. - .. note:: - - By default the builder module sets ``PINI`` to ``YES``, which means the - record will process at initialization and the alarm status will be reset. + These are not useful unless ``PINI`` is set to ``NO``, as otherwise the record + will be processed on initialization and the alarm status will be reset. .. _on_update: