From d8713629de4f6c583766dbb8e77b3c10a714be56 Mon Sep 17 00:00:00 2001 From: AlexWells Date: Thu, 16 Jul 2026 09:40:39 +0100 Subject: [PATCH 1/3] Correctly set alarms during blocking processing This was an oversight in PR #202 --- softioc/device.py | 1 + tests/test_records.py | 71 ++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 67 insertions(+), 5 deletions(-) diff --git a/softioc/device.py b/softioc/device.py index 82130963..dead0459 100644 --- a/softioc/device.py +++ b/softioc/device.py @@ -291,6 +291,7 @@ def set(self, value, process=True, # However if we do not process, we must do this here to keep the # Python and EPICS values in line if not process: + self.process_severity(_record, severity, alarm) self._value = (value, severity, alarm) db_put_field_process(_record.NAME, dbf_code, data, length, process) diff --git a/tests/test_records.py b/tests/test_records.py index ad0af407..6089e043 100644 --- a/tests/test_records.py +++ b/tests/test_records.py @@ -684,7 +684,7 @@ def test_on_update_true_false(self, out_records): self.on_update_runner(out_records, True, False) def on_update_recursive_set_test_func( - self, device_name, conn + self, device_name, conn, severity, alarm ): log("CHILD: Child started") @@ -692,7 +692,7 @@ def on_update_recursive_set_test_func( async def on_update_func(new_val): log("CHILD: on_update_func started") - record.set(0, process=False) + record.set(0, process=False, severity=severity, alarm=alarm) conn.send("C") # "Callback" log("CHILD: on_update_func ended") @@ -718,8 +718,8 @@ async def on_update_func(new_val): log("CHILD: Received exit command, child exiting") - async def test_on_update_recursive_set(self): - """Test that on_update functions correctly when the on_update + async def test_on_update_recursive_set_value(self): + """Test that on_update sets values correctly when the on_update callback sets the value of the record again (with process=False). See issue #201""" @@ -730,7 +730,7 @@ async def test_on_update_recursive_set(self): process = ctx.Process( target=self.on_update_recursive_set_test_func, - args=(device_name, child_conn), + args=(device_name, child_conn, alarm.NO_ALARM, alarm.NO_ALARM), ) process.start() @@ -773,6 +773,67 @@ async def test_on_update_recursive_set(self): pytest.fail("Process did not terminate") + async def test_on_update_recursive_set_alarm(self): + """Test that on_update sets alarms correctly when the on_update + callback sets the value of the record again (with process=False). + See issue #201""" + + ctx = get_multiprocessing_context() + parent_conn, child_conn = ctx.Pipe() + + device_name = create_random_prefix() + + expected_sevr = alarm.MAJOR_ALARM + expected_alarm = alarm.HIGH_ALARM # arbitrary choice + + process = ctx.Process( + target=self.on_update_recursive_set_test_func, + args=(device_name, child_conn, expected_sevr, expected_alarm), + ) + + process.start() + + log("PARENT: Child started, waiting for R command") + + from aioca import caget, caput + + try: + # Wait for message that IOC has started + select_and_recv(parent_conn, "R") + + log("PARENT: received R command") + + record = f"{device_name}:ACTION" + + val = await caget(record) + + assert val == 1, "ACTION record did not start with value 1" + + await caput(record, 1, wait=True) + + + sevr = await caget(record + ".SEVR") + assert sevr == expected_sevr + + sevr = await caget(record + ".STAT") + assert sevr == expected_alarm + + # Expect one "C" + select_and_recv(parent_conn, "C") + + # ...But if we receive another we know there's a problem + if parent_conn.poll(5): # Shorter timeout to make this quicker + pytest.fail("Received unexpected second message") + + finally: + log("PARENT:Sending Done command to child") + parent_conn.send("D") # "Done" + process.join(timeout=TIMEOUT) + log(f"PARENT: Join completed with exitcode {process.exitcode}") + if process.exitcode is None: + pytest.fail("Process did not terminate") + + class TestBlocking: """Tests related to the Blocking functionality""" From bc1a67dc0406015639bc5b3d5e7baa51d8cc4e9d Mon Sep 17 00:00:00 2001 From: AlexWells Date: Thu, 16 Jul 2026 10:28:35 +0100 Subject: [PATCH 2/3] Add CHANGELOG message --- CHANGELOG.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index ff70667c..d3eb7752 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -12,6 +12,7 @@ Unreleased_ Fixed: +- `Correctly set alarms during blocking processing <../../pull/209>`_ - `Fix infinite loop when setting record's own value from on_update callback <../../pull/202>`_ 4.7.0_ - 2026-01-14 From 6b991a4712becbf33295151992295410566c2a88 Mon Sep 17 00:00:00 2001 From: AlexWells Date: Thu, 16 Jul 2026 15:23:03 +0100 Subject: [PATCH 3/3] Handle resetting severity and alarm I can't see a reason why we should only set these values if they are non-zero, it's perfectly valid to do this to reset errors --- softioc/device.py | 8 ++++- softioc/device_core.py | 3 ++ tests/test_records.py | 78 +++++++++++++++++++++++++++++++++++++----- 3 files changed, 79 insertions(+), 10 deletions(-) diff --git a/softioc/device.py b/softioc/device.py index dead0459..17ed150e 100644 --- a/softioc/device.py +++ b/softioc/device.py @@ -291,7 +291,13 @@ def set(self, value, process=True, # However if we do not process, we must do this here to keep the # Python and EPICS values in line if not process: - self.process_severity(_record, severity, alarm) + # Normally a call to dbProcess would handle setting/re-setting + # alarm levels for us, however if we are not processing we + # obviously can't rely on that. Hence we have to do it + # ourselves. We don't use `process_severity` as we want to + # unconditionally set these, regardless of previous severity. + _record.NSEV = severity + _record.NSTA = alarm self._value = (value, severity, alarm) db_put_field_process(_record.NAME, dbf_code, data, length, process) diff --git a/softioc/device_core.py b/softioc/device_core.py index cfc5e8c3..8179c0d0 100644 --- a/softioc/device_core.py +++ b/softioc/device_core.py @@ -177,6 +177,9 @@ def _get_ioinit_info(self, cmd, record, ppvt): def process_severity(self, record, severity, alarm): '''Support routine to implement alarm processing.''' + # This matches the logic in the EPICS function recGblSetSevr, which + # always maximises severity. This is called as part of EPICS record + # processing if severity > record.NSEV: record.NSEV = severity record.NSTA = alarm diff --git a/tests/test_records.py b/tests/test_records.py index 6089e043..b666a386 100644 --- a/tests/test_records.py +++ b/tests/test_records.py @@ -683,8 +683,8 @@ def test_on_update_true_false(self, out_records): always_update is True and the put'ed value is always different""" self.on_update_runner(out_records, True, False) - def on_update_recursive_set_test_func( - self, device_name, conn, severity, alarm + def on_update_recursive_set_value_test_func( + self, device_name, conn, ): log("CHILD: Child started") @@ -692,7 +692,7 @@ def on_update_recursive_set_test_func( async def on_update_func(new_val): log("CHILD: on_update_func started") - record.set(0, process=False, severity=severity, alarm=alarm) + record.set(0, process=False) conn.send("C") # "Callback" log("CHILD: on_update_func ended") @@ -729,8 +729,8 @@ async def test_on_update_recursive_set_value(self): device_name = create_random_prefix() process = ctx.Process( - target=self.on_update_recursive_set_test_func, - args=(device_name, child_conn, alarm.NO_ALARM, alarm.NO_ALARM), + target=self.on_update_recursive_set_value_test_func, + args=(device_name, child_conn), ) process.start() @@ -773,10 +773,58 @@ async def test_on_update_recursive_set_value(self): pytest.fail("Process did not terminate") + def on_update_recursive_set_alarm_test_func( + self, device_name, conn, severity, alrm + ): + log("CHILD: Child started") + + builder.SetDeviceName(device_name) + + async def on_update_func(new_val): + log(f"CHILD: on_update_func started value {new_val}") + if new_val == 2: + record.set( + new_val, + process=False, + severity=severity, + alarm=alrm + ) + elif new_val == 3: + record.set( + new_val, + process=False, + severity=alarm.NO_ALARM, + alarm=alarm.NO_ALARM + ) + conn.send("C") # "Callback" + log("CHILD: on_update_func ended") + + record = builder.longOut( + "ACTION", + on_update=on_update_func, + blocking=True, + initial_value=1 # A non-zero value, to check it changes + ) + + dispatcher = asyncio_dispatcher.AsyncioDispatcher() + builder.LoadDatabase() + softioc.iocInit(dispatcher) + + conn.send("R") # "Ready" + + log("CHILD: Sent R over Connection to Parent") + + # Keep process alive while main thread runs CAGET + if conn.poll(TIMEOUT): + val = conn.recv() + assert val == "D", "Did not receive expected Done character" + + log("CHILD: Received exit command, child exiting") + async def test_on_update_recursive_set_alarm(self): """Test that on_update sets alarms correctly when the on_update callback sets the value of the record again (with process=False). - See issue #201""" + See PR #209""" ctx = get_multiprocessing_context() parent_conn, child_conn = ctx.Pipe() @@ -787,7 +835,7 @@ async def test_on_update_recursive_set_alarm(self): expected_alarm = alarm.HIGH_ALARM # arbitrary choice process = ctx.Process( - target=self.on_update_recursive_set_test_func, + target=self.on_update_recursive_set_alarm_test_func, args=(device_name, child_conn, expected_sevr, expected_alarm), ) @@ -809,8 +857,8 @@ async def test_on_update_recursive_set_alarm(self): assert val == 1, "ACTION record did not start with value 1" - await caput(record, 1, wait=True) - + # Trigger callback to set alarm states + await caput(record, 2, wait=True) sevr = await caget(record + ".SEVR") assert sevr == expected_sevr @@ -821,6 +869,18 @@ async def test_on_update_recursive_set_alarm(self): # Expect one "C" select_and_recv(parent_conn, "C") + # Trigger another callback to clear alarm states + await caput(record, 3, wait=True) + + sevr = await caget(record + ".SEVR") + assert sevr == alarm.NO_ALARM + + sevr = await caget(record + ".STAT") + assert sevr == alarm.NO_ALARM + + # Expect second "C" + select_and_recv(parent_conn, "C") + # ...But if we receive another we know there's a problem if parent_conn.poll(5): # Shorter timeout to make this quicker pytest.fail("Received unexpected second message")