Skip to content

Alarm handling with AMSG - #52

Open
pheest wants to merge 7 commits into
epics-modules:masterfrom
pheest:Alarm_handling_with_AMSG
Open

Alarm handling with AMSG#52
pheest wants to merge 7 commits into
epics-modules:masterfrom
pheest:Alarm_handling_with_AMSG

Conversation

@pheest

@pheest pheest commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

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.

@tynanford

Copy link
Copy Markdown
Collaborator

Thanks @pheest , with #48 merged I think this PR can be updated to only include changes to devsupApp/src/devsup/ptable.py and devsupApp/src/devsup/test/test_db.py?

… request.

Accepted changes to dbrec.c made byhttps://github.com/epics-modules/pull/48
@pheest

pheest commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator Author

OK, done. I see test_db.py has continued conflicts that will need to be addressed.
I do not take issue with #48 on dbrec.c which has a slightly different implementation to mine.

@tynanford

Copy link
Copy Markdown
Collaborator

Thanks @pheest . I went ahead and merged the latest from master and resolved the conflicts. The main one being I renamed TestAlarm to TestAlarmScan (feel free to modify/change if you disagree).

Comment thread devsupApp/src/devsup/ptable.py Outdated
Comment thread devsupApp/src/devsup/ptable.py Outdated
Comment thread devsupApp/src/devsup/ptable.py Outdated
There does not appear to be a need for G._exec to be outside of the lock block.
Comment thread devsupApp/src/devsup/ptable.py Outdated
Comment thread devsupApp/src/devsup/ptable.py Outdated
Comment thread devsupApp/src/devsup/ptable.py Outdated
self.name = name
self.table, self.scan, self._value = table, scan, None
self.alarm, self.actions = 0, []
self.stat = UDF_ALARM

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@pheest pheest Aug 25, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

field(INP, "@devsup.ptable alarm set pini_yes")

Is 'set' intentional here, for an input record?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.stat

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are 2 things I'm curious about in ptable.py:

  1. What is the role played by value = property(_get_value, _set_value... - it seems to obfuscate.
  2. 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)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@tynanford tynanford mentioned this pull request Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants