Alarm handling with AMSG - #52
Conversation
… request. Accepted changes to dbrec.c made byhttps://github.com/epics-modules/pull/48
|
OK, done. I see test_db.py has continued conflicts that will need to be addressed. |
|
Thanks @pheest . I went ahead and merged the latest from master and resolved the conflicts. The main one being I renamed |
There does not appear to be a need for G._exec to be outside of the lock block.
| self.name = name | ||
| self.table, self.scan, self._value = table, scan, None | ||
| self.alarm, self.actions = 0, [] | ||
| self.stat = UDF_ALARM |
There was a problem hiding this comment.
I believe we should default this to COMM_ALARM? I understand the default of UDF_ALARM but I think this code already covers that case when the value hasn't been set at all:
if value is not None:
......
else:
# undefined value
rec.setSevr(INVALID_ALARM, UDF_ALARM)
So then the only time self.stat default value comes into play is when the user sets .alarm but does not set .stat. In that case defaulting to COMM_ALARM would maintain backwards compatibility with dbrec.c default and also seems better fit than UDF_ALARM? Otherwise you could have a MAJOR alarm with a default UDF_ALARM STAT field?
There was a problem hiding this comment.
Hi @tynanford, I believe we discussed this in #48 on July 22, and thought that we had agreement on the issue.
My intent was that the value of the value should default to the expected value after IOC start-up.
Which is UDF_ALARM if PINI is not set and NO_ALARM if it is.
My tests do explicitly check the value (without explicitly setting the stat field).
I'm aware that the setSevr function defaults to COMM_ALARM if the second parameter is not specified.
All cases in ptable.py do provide the first two parameters.
A change I could make, if you agree, is to default self.stat to None, and then set it explicitly in _ParamSupBase,init to the actual value read back from rec.STAT.
This could differ from the expected value if, for example the record is malformed or PINI processing causes an error .
I have tested this change locally.
There was a problem hiding this comment.
Hi @pheest yes sorry, after looking into this a bit I found that defaulting to UDF_ALARM means that a user who doesn't set .stat would then get a default of UDF for all alarms. Here is a test IOC to show what I mean: https://github.com/tynanford/alarmDemo
With the current master branch of pyDevSup:
$ camonitor PINI:YES PINI:NO
PINI:YES 2026-08-26 11:04:01.302082 0 COMM MAJOR
PINI:NO <undefined> 55 UDF NO_ALARM
PINI:YES 2026-08-26 11:04:06.302560 1
PINI:YES 2026-08-26 11:04:11.302999 2 COMM MAJOR
PINI:YES 2026-08-26 11:04:16.303536 3
PINI:YES 2026-08-26 11:04:21.303729 4 COMM MAJOR
With your changes now PINI:YES shows UDF alarm:
$ camonitor PINI:YES PINI:NO
PINI:YES 2026-08-26 11:05:04.059871 0 UDF MAJOR
PINI:NO <undefined> 55 UDF NO_ALARM
PINI:YES 2026-08-26 11:05:09.060527 1
PINI:YES 2026-08-26 11:05:14.061057 2 UDF MAJOR
PINI:YES 2026-08-26 11:05:19.061630 3
PINI:YES 2026-08-26 11:05:24.062074 4 UDF MAJOR
Also it seems like the UDF alarm handling already is covered by the if value is not None: line in ptable.py. Does this follow or am I thinking about this wrong?
There was a problem hiding this comment.
I'm not saying COMM_ALARM is the best default but it seems to fit better than UDF which should be for only before a record is initialized?
A change I could make, if you agree, is to default self.stat to None, and then set it explicitly in _ParamSupBase,init to the actual value read back from rec.STAT.
This sounds interesting.. then we don't decide in pyDevSup what the default is which seems good?
There was a problem hiding this comment.
field(INP, "@devsup.ptable alarm set pini_yes")
Is 'set' intentional here, for an input record?
There was a problem hiding this comment.
I agree completely. I am someone who is doing exactly that.
I've been working to enhance your test app - including a reflection of the above.
One thing I've noticed is that records that don't specify the VAL field are always in UDF state even as their values are changed. This is irrespective of the PINI value.
Can you confirm this? Do you have any understanding of why this is? I do not.
There was a problem hiding this comment.
Yes I see the same behavior as you do. Claude had found what looked to be a bug a few weeks ago but I held off looking into it more - https://github.com/epics-modules/pyDevSup/blob/master/devsupApp/src/devsup/ptable.py#L219 .
Claude suggested self.raw should check for == RVAL not != RVAL which seemed to make sense to me. When self.raw is true then the EPICS base UDF handling is skipped and flipping this logic fixed 2 of your test PVs (with VAL in db file removed)
But this comment https://github.com/epics-modules/pyDevSup/blob/master/devsupApp/src/devsup/interfaces.py#L6 shows self.raw is supposed to be true for VAL.
Does this mean we need to clear UDF ourselves in ptable.py since modifying VAL in device support is considered "raw device support"?
diff --git a/devsupApp/src/devsup/ptable.py b/devsupApp/src/devsup/ptable.py
index 3c7665e..6b5d176 100644
--- a/devsupApp/src/devsup/ptable.py
+++ b/devsupApp/src/devsup/ptable.py
@@ -248,6 +248,7 @@ class _ParamSupGet(_ParamSupBase):
value = value[:len(self.vdata)]
self.vdata[:len(value)] = value
self.vfld.putarraylen(len(value))
+ rec.UDF = 0
if alarm:
rec.setSevr(alarm, stat, amsg)
else:
@@ -271,6 +272,8 @@ class _ParamSupSet(_ParamSupGet):
# A copy is made which can be used without locking the record
value = self.vdata[:self.vfld.getarraylen()].copy()
+ rec.UDF = 0
with self.inst.table.lock:
oval, self.inst.value = self.inst.value, value
stat = self.inst.statThere was a problem hiding this comment.
There are 2 things I'm curious about in ptable.py:
- What is the role played by value = property(_get_value, _set_value... - it seems to obfuscate.
- What is the role played by: it seems to be saying an alarm once activated cannot be deactivated.
if alarm:
rec.setSevr(alarm, stat, amsg)
There was a problem hiding this comment.
In the process database, the UDF field defaults to 1. There are special case in dbPutString() and dbPut() where assigning VAL implicitly clears UDF, this effects assignment from .db files and remote respectively. Assignment of VAL by device support is more varied, and depends on record type. eg. Generally speaking input record types will clear UDF if devices support read/process function returns zero. Output record types will clear it after reading from DOL into VAL, but otherwise depend on one of the special cases.
The severity signaled with UDF_ALARM is configurable by the UDFS field, which defaults to INVALID_ALARM (I have never had cause to change UDFS).
There was a problem hiding this comment.
Hi @tynanford, I think the point that Claude is getting at is that if the device has an RVAL field, that should be used and self.raw should be True. Otherwise the VAL field should be used and self.raw should be False.
I think it is not possible to obtain this information by calling either:
rec.info('pyfield','VAL') or rec.info('pyfield','RVAL').
At least, both these calls just return 'VAL' or 'RVAL', respectively.
I am using this logic, and can confirm that it results in correct behaviour of the alarm fields:
self.raw = False
try:
self.vfld = rec.field("RVAL")
self.raw = True
except KeyError:
self.vfld = rec.field("VAL")
Thank you, @mdavidsaver. However, I'm not sure I follow you.
My questions refer to the required behaviour in ptable.py, what you are saying seems lower-level than that.
This PR incorporates and expands on #48
In dbrec.c, I have added a third parameter 'amsg' to the call, defaulting to None.
Use of the 'z' argument means that either a string or None is accepted.
If it is None, NULL is the resulting C value.
If EPICS_VERSION is < 7.0.6, the parameter is ignored.
In ptable.py, the stat field is initialised to UDF_ALARM.
This will be overridden if the PINI field is "YES".
This ensures the value of the field matches the actual output value after initial processing.
The alarm, stat and amsg values are applied to both input and output records. This requires the record be scanned in order to update the status.
(My project need is to alert the user when an incorrect value is set that does not meet complex validation rules.)
In test_db.py, I have added tests to verify correct operation of both input and output record alarms.