Merge ~dirk.zimoch/epics-base:named-events-backward-compatibility into ~epics-core/epics-base/+git/epics-base:3.15

[Dirk Zimoch]

Make named soft events backward compatible to calculated numeric events (e.g by calc records).
Every event name that represents a number and that rounds down to an integer 1 ... 255 is a numeric events. If it rounds to 0 or is not finite (nan, +/-inf) it is no event.

b2d6b67... by Dirk Zimoch on 2018-02-16

removed obsolete code in scanpel

[Andrew Johnson (anj)]

Hi Dirk,

You can commit to a branch after you have submitted a Merge Request for it and the new commits will automatically appear in the Merge Request (check this one out, it has your b2d6b67 commit in it). Submitting a *new* MR implies there's something major that is different than before, maybe you rebased the branch. Please don't do resubmits for minor additional changes like this, they are unnecessary and send confusing signals to the reviewers.

I will take a look at this when I get a chance.

Thanks,

- Andrew

[mdavidsaver (mdavidsaver)]

I haven't had a chance to do any run tests, but from reading the code it looks reasonable.

The only (minor) issue I see is that the test code for eventRecord needs to be moved under src/std/rec/test/

[Dirk Zimoch (dirk.zimoch)]

Oh, I didn‘t know that.
Dirk

> Am 16.02.2018 um 17:17 schrieb Andrew Johnson <email address hidden>:
>
> Hi Dirk,
>
> You can commit to a branch after you have submitted a Merge Request for it and the new commits will automatically appear in the Merge Request (check this one out, it has your b2d6b67 commit in it). Submitting a *new* MR implies there's something major that is different than before, maybe you rebased the branch. Please don't do resubmits for minor additional changes like this, they are unnecessary and send confusing signals to the reviewers.
>
> I will take a look at this when I get a chance.
>
> Thanks,
>
> - Andrew
> --
> https://code.launchpad.net/~dirk.zimoch/epics-base/+git/epics-base/+merge/337644
> You are the owner of ~dirk.zimoch/epics-base:named-events-backward-compatibility.

[Dirk Zimoch (dirk.zimoch)]

I had put it in db/test because my intent was to test the db event code,
not to test the records. The records are just the tool to test that the
db code works as intended. But if there is a problem having records in
db/test I see no problem moving it.

Dirk

On 16.02.2018 17:38, mdavidsaver wrote:
> I haven't had a chance to do any run tests, but from reading the code it looks reasonable.
>
> The only (minor) issue I see is that the test code for eventRecord needs to be moved under src/std/rec/test/
>
> Diff comments:
>
>> diff --git a/src/ioc/db/test/Makefile b/src/ioc/db/test/Makefile
>> index 215545f..77e485a 100644
>> --- a/src/ioc/db/test/Makefile
>> +++ b/src/ioc/db/test/Makefile
>> @@ -138,6 +138,21 @@ testHarness_SRCS += dbPutGetTest.db
>> TESTFILES += ../dbPutGetTest.db
>> TESTS += testPutGetTest
>>
>> +TARGETS += $(COMMON_DIR)/scanEventTest.dbd
>> +DBDDEPENDS_FILES += scanEventTest.dbd$(DEP)
>> +scanEventTest_DBD += menuGlobal.dbd
>> +scanEventTest_DBD += menuConvert.dbd
>> +scanEventTest_DBD += menuScan.dbd
>> +scanEventTest_DBD += eventRecord.dbd
>> +scanEventTest_DBD += devEventSoft.dbd
>> +scanEventTest_DBD += calcRecord.dbd
>> +scanEventTest_DBD += dfanoutRecord.dbd
>> +TESTPROD_HOST += scanEventTest
>> +scanEventTest_SRCS += scanEventTest.c
>> +scanEventTest_SRCS += scanEventTest_registerRecordDeviceDriver.cpp
>> +scanEventTest_LIBS = dbRecStd
>> +TESTS += scanEventTest
>
> Tests using libdbRecStd and base.dbd need to be under src/std/rec/test/ (cf. compressTest.c). So the test code needs to move.
>
>> +
>> # This runs all the test programs in a known working order:
>> testHarness_SRCS += epicsRunDbTests.c
>>
>
>

Unmerged commits

b2d6b67... by Dirk Zimoch on 2018-02-16

removed obsolete code in scanpel

1e9826d... by Dirk Zimoch on 2018-02-13

add tests for calculated numeric soft events

0691fc5... by Dirk Zimoch on 2018-02-13

fix scanEvent test

8a3080c... by Dirk Zimoch on 2018-02-13

added test for named soft events

d19afc7... by Dirk Zimoch on 2018-02-12

Updated RELEASE_NOTES.html with soft event fix.

adf5375... by Dirk Zimoch on 2018-02-12

fix scanpel glob matching

7d836d9... by Dirk Zimoch on 2018-02-09

some cleanup and scanpel improvement

51e492f... by Dirk Zimoch on 2018-02-09

Fix numeric events: any number from 0 up to less than 256 is theated as an integer event. Event 0 is no event.

diff --git a/documentation/RELEASE_NOTES.html b/documentation/RELEASE_NOTES.html
index 33f12c4..d8dd5d5 100644
--- a/documentation/RELEASE_NOTES.html
+++ b/documentation/RELEASE_NOTES.html
@@ -29,11 +29,28 @@ be put in a locally created base/configure/CONFIG_SITE.local file instead
 of having go modify or replace the original. A new .gitignore pattern
 tells git to ignore all configure/*.local files.</p>
 
+<h3>Fix problem with numeric soft events</h3>
+<p>Changing from numeric to named soft events introduced an incompatibility
+when a numeric event is converted from a DOUBLE, e.g. from a calc record.
+To make named events backward compatible to numeric events, the logic has
+changed as follows:</p>
+<ul><li>Leading and trailing spaces are strippd from the event name
+    <li>Empty event names are no events
+    <li>If the event name can be converted to double:
+    <ul><li>If the double is not finite (inf or nan) it is no event.
+        <li>If the double truncated to integer is between 1 and 255
+            it is a numeric event. Its name is changed to the decimal
+            integer representation.
+        <li>If the double truncated to integer is 0 it is no event.
+    </ul>
+    <li>All other cases are named events
+</ul>
+<p>Also <code>scanpel</code> has been modified to accept a glob pattern for
+filtering and to show events with no connected records as well.</p>
 
 <h2 align="center">Changes from the 3.14 branch since 3.15.5</h2>
 
 <!-- Insert inherited items immediately below here ... -->
-
 <h3>Extend maximum Posix epicsEventWaitWithTimeout() delay</h3>
 
 <p>The Posix implementation of epicsEventWaitWithTimeout() was limiting the
diff --git a/src/ioc/db/dbScan.c b/src/ioc/db/dbScan.c
index e0c48d4..cabd736 100644
--- a/src/ioc/db/dbScan.c
+++ b/src/ioc/db/dbScan.c
@@ -27,6 +27,7 @@
 #include "epicsEvent.h"
 #include "epicsExit.h"
 #include "epicsInterrupt.h"
+#include "epicsMath.h"
 #include "epicsMutex.h"
 #include "epicsPrint.h"
 #include "epicsRingPointer.h"
@@ -111,8 +112,8 @@ static char *priorityName[NUM_CALLBACK_PRIORITIES] = {
 typedef struct event_list {
     CALLBACK            callback[NUM_CALLBACK_PRIORITIES];
     scan_list           scan_list[NUM_CALLBACK_PRIORITIES];
-    struct event_list *next;
-    char                event_name[MAX_STRING_SIZE];
+    struct event_list   *next;
+    char                eventname[1]; /* actually arbitrary size */
 } event_list;
 static event_list * volatile pevent_list[256];
 static epicsMutexId event_lock;
@@ -249,11 +250,6 @@ void scanAdd(struct dbCommon *precord)
         event_list *pel;
 
         eventname = precord->evnt;
-        if (strlen(eventname) >= MAX_STRING_SIZE) {
-            recGblRecordError(S_db_badField, (void *)precord,
-                "scanAdd: too long EVNT value");
-            return;
-        }
         prio = precord->prio;
         if (prio < 0 || prio >= NUM_CALLBACK_PRIORITIES) {
             recGblRecordError(-1, (void *)precord,
@@ -317,24 +313,17 @@ void scanDelete(struct dbCommon *precord)
         recGblRecordError(-1, (void *)precord,
             "scanDelete detected illegal SCAN value");
     } else if (scan == menuScanEvent) {
-        char* eventname;
         int prio;
         event_list *pel;
         scan_list *psl = 0;
 
-        eventname = precord->evnt;
         prio = precord->prio;
         if (prio < 0 || prio >= NUM_CALLBACK_PRIORITIES) {
             recGblRecordError(-1, (void *)precord,
                 "scanDelete detected illegal PRIO field");
             return;
         }
-        do /* multithreading: make sure pel is consistent */
-            pel = pevent_list[0];
-        while (pel != pevent_list[0]);
-        for (; pel; pel=pel->next) {
-            if (strcmp(pel->event_name, eventname) == 0) break;
-        }
+        pel = eventNameToHandle(precord->evnt);
         if (pel && (psl = &pel->scan_list[prio]))
             deleteFromList(precord, psl);
     } else if (scan == menuScanI_O_Intr) {
@@ -422,14 +411,12 @@ int scanpel(const char* eventname)   /* print event list */
     int prio;
     event_list *pel;
 
-    do /* multithreading: make sure pel is consistent */
-        pel = pevent_list[0];
-    while (pel != pevent_list[0]);
-    for (; pel; pel = pel->next) {
-        if (!eventname || strcmp(pel->event_name, eventname) == 0) {
+    for (pel = pevent_list[0]; pel; pel = pel->next) {
+        if (!eventname || epicsStrGlobMatch(pel->eventname, eventname)) {
+            printf("Event \"%s\"\n", pel->eventname);
             for (prio = 0; prio < NUM_CALLBACK_PRIORITIES; prio++) {
                 if (ellCount(&pel->scan_list[prio].list) == 0) continue;
-                sprintf(message, "Event \"%s\" Priority %s", pel->event_name, priorityName[prio]);
+                sprintf(message, " Priority %s", priorityName[prio]);
                 printList(&pel->scan_list[prio], message);
             }
         }
@@ -480,18 +467,53 @@ event_list *eventNameToHandle(const char *eventname)
     int prio;
     event_list *pel;
     static epicsThreadOnceId onceId = EPICS_THREAD_ONCE_INIT;
-
-    if (!eventname || eventname[0] == 0)
-        return NULL;
+    double eventnumber;
+    size_t namelength;
+
+    if (!eventname) return NULL;
+    while (isspace((unsigned char)eventname[0])) eventname++;
+    if (!eventname[0]) return NULL;
+    namelength = strlen(eventname);
+    while (isspace((unsigned char)eventname[namelength-1])) namelength--;
+
+    /* Backward compatibility with numeric events:
+       Treat any string that represents a double with an
+       integer part between 0 and 255 the same as the integer
+       because it is most probably a conversion from double
+       like from a calc record.
+    */
+    if (epicsParseDouble(eventname, &eventnumber, NULL) == 0)
+    {
+        if (!finite(eventnumber))
+            return NULL; /* Inf and NaN are no events */
+        if (eventnumber >= 0 && eventnumber < 256)
+        {
+            if (eventnumber < 1)
+                return NULL; /* 0 is no event */
+            if ((pel = pevent_list[(int)eventnumber]) != NULL)
+                return pel;
+        }
+    }
+    else 
+        eventnumber = 0; /* not a numeric event */
 
     epicsThreadOnce(&onceId, eventOnce, NULL);
     epicsMutexMustLock(event_lock);
     for (pel = pevent_list[0]; pel; pel=pel->next) {
-        if (strcmp(pel->event_name, eventname) == 0) break;
+        if (strncmp(pel->eventname, eventname, namelength) == 0
+            && pel->eventname[namelength] == 0)
+            break;
     }
     if (pel == NULL) {
-        pel = dbCalloc(1, sizeof(event_list));
-        strcpy(pel->event_name, eventname);
+        pel = dbCalloc(1, sizeof(event_list) + namelength);
+        if (eventnumber > 0)
+        {
+            /* backward compatibility: make all numeric events look like integers */
+            sprintf(pel->eventname, "%i", (int)eventnumber);
+            pevent_list[(int)eventnumber] = pel;
+        }
+        else
+            strncpy(pel->eventname, eventname, namelength);
         for (prio = 0; prio < NUM_CALLBACK_PRIORITIES; prio++) {
             callbackSetUser(&pel->scan_list[prio], &pel->callback[prio]);
             callbackSetPriority(prio, &pel->callback[prio]);
@@ -501,12 +523,6 @@ event_list *eventNameToHandle(const char *eventname)
         }
         pel->next=pevent_list[0];
         pevent_list[0]=pel;
-        { /* backward compatibility */
-            char* p;
-            long e = strtol(eventname, &p, 0);
-            if (*p == 0 && e > 0 && e <= 255)
-                pevent_list[e] = pel;
-        }
     }
     epicsMutexUnlock(event_lock);
     return pel;
@@ -527,13 +543,8 @@ void postEvent(event_list *pel)
 /* backward compatibility */
 void post_event(int event)
 {
-    event_list* pel;
-
     if (event <= 0 || event > 255) return;
-    do { /* multithreading: make sure pel is consistent */
-        pel = pevent_list[event];
-    } while (pel != pevent_list[event]);
-    postEvent(pel);
+    postEvent(pevent_list[event]);
 }
 

 static void ioscanOnce(void *arg)
diff --git a/src/ioc/db/dbScan.h b/src/ioc/db/dbScan.h
index 28d2e13..d310271 100644
--- a/src/ioc/db/dbScan.h
+++ b/src/ioc/db/dbScan.h
@@ -50,7 +50,7 @@ epicsShareFunc void scanCleanup(void);
 
 epicsShareFunc EVENTPVT eventNameToHandle(const char* event);
 epicsShareFunc void postEvent(EVENTPVT epvt);
-epicsShareFunc void post_event(int event) EPICS_DEPRECATED;
+epicsShareFunc void post_event(int event);
 epicsShareFunc void scanAdd(struct dbCommon *);
 epicsShareFunc void scanDelete(struct dbCommon *);
 epicsShareFunc double scanPeriod(int scan);
diff --git a/src/ioc/db/test/Makefile b/src/ioc/db/test/Makefile
index 215545f..77e485a 100644
--- a/src/ioc/db/test/Makefile
+++ b/src/ioc/db/test/Makefile
@@ -138,6 +138,21 @@ testHarness_SRCS += dbPutGetTest.db
 TESTFILES += ../dbPutGetTest.db
 TESTS += testPutGetTest
 
+TARGETS += $(COMMON_DIR)/scanEventTest.dbd
+DBDDEPENDS_FILES += scanEventTest.dbd$(DEP)
+scanEventTest_DBD += menuGlobal.dbd
+scanEventTest_DBD += menuConvert.dbd
+scanEventTest_DBD += menuScan.dbd
+scanEventTest_DBD += eventRecord.dbd
+scanEventTest_DBD += devEventSoft.dbd
+scanEventTest_DBD += calcRecord.dbd
+scanEventTest_DBD += dfanoutRecord.dbd
+TESTPROD_HOST += scanEventTest
+scanEventTest_SRCS += scanEventTest.c
+scanEventTest_SRCS += scanEventTest_registerRecordDeviceDriver.cpp
+scanEventTest_LIBS = dbRecStd
+TESTS += scanEventTest
+
 # This runs all the test programs in a known working order:
 testHarness_SRCS += epicsRunDbTests.c
 
diff --git a/src/ioc/db/test/devEventSoft.dbd b/src/ioc/db/test/devEventSoft.dbd
new file mode 100644
index 0000000..ec3bc2d
--- /dev/null
+++ b/src/ioc/db/test/devEventSoft.dbd
@@ -0,0 +1 @@
+device(event,CONSTANT,devEventSoft,"Soft Channel")
diff --git a/src/ioc/db/test/epicsRunDbTests.c b/src/ioc/db/test/epicsRunDbTests.c
index 05fedeb..ce41815 100644
--- a/src/ioc/db/test/epicsRunDbTests.c
+++ b/src/ioc/db/test/epicsRunDbTests.c
@@ -29,6 +29,7 @@ int testDbChannel(void);
 int chfPluginTest(void);
 int arrShorthandTest(void);
 int recGblCheckDeadbandTest(void);
+int scanEventTest(void);
 
 void epicsRunDbTests(void)
 {
@@ -47,6 +48,7 @@ void epicsRunDbTests(void)
     runTest(arrShorthandTest);
     runTest(recGblCheckDeadbandTest);
     runTest(chfPluginTest);
+    runTest(scanEventTest);
 
     dbmfFreeChunks();
 
diff --git a/src/ioc/db/test/scanEventTest.c b/src/ioc/db/test/scanEventTest.c
new file mode 100644
index 0000000..9835b7b
--- /dev/null
+++ b/src/ioc/db/test/scanEventTest.c
@@ -0,0 +1,137 @@
+/*************************************************************************\
+* Copyright (c) 2018 Paul Scherrer Institut
+* EPICS BASE is distributed subject to a Software License Agreement found
+* in file LICENSE that is included with this distribution.
+ \*************************************************************************/
+
+/*
+ *  Author: Dirk Zimoch <[e-mail]>
+ */
+
+#include <string.h>
+#include "dbStaticLib.h"
+#include "dbAccessDefs.h"
+#include "dbUnitTest.h"
+#include "testMain.h"
+#include "osiFileName.h"
+#include "dbScan.h"
+
+void scanEventTest_registerRecordDeviceDriver(struct dbBase *);
+
+/* test name to event number:
+    num = 0 is no event,
+    num > 0 is numeric event
+    num < 0 is string event (use same unique number for aliases)
+*/
+const struct {char* name; int num;} events[] = {
+    {"",              0},
+    {"       ",       0},
+    {"0",             0},
+    {"0.000000",      0},
+    {"-0.00000",      0},
+    {"0.9",           0},
+    {"nan",           0},
+    {"NaN",           0},
+    {"-NAN",          0},
+    {"-inf",          0},
+    {"inf",           0},
+    {"2",             2},
+    {"2.000000",      2},
+    {"2.5",           2},
+    {" 2.5  ",        2},
+    {"+0x02",         2},
+    {"3",             3},
+    {"info 1",       -1},
+    {"   info 1  ",  -1},
+    {"-0.9",         -2},
+    {"-2",           -3},
+    {"-2.000000",    -4},
+    {"-2.5",         -5},
+    {" -2.5  ",      -5},
+};
+
+MAIN(scanEventTest)
+{
+    int i, e;
+    int aliases[512] ;
+    int expected_count[512];
+    #define INDX(i) 256-events[i].num
+    #define MAXEV 5
+    
+    testPlan(NELEMENTS(events)*2+(MAXEV+1)*5);
+    
+    memset(aliases, 0, sizeof(aliases));
+    memset(expected_count, 0, sizeof(expected_count));
+
+    if (dbReadDatabase(&pdbbase, "scanEventTest.dbd",
+            "." OSI_PATH_LIST_SEPARATOR ".." OSI_PATH_LIST_SEPARATOR
+            "../O.Common" OSI_PATH_LIST_SEPARATOR "O.Common", NULL))
+        testAbort("Database description 'scanEventTest.dbd' not found");
+
+    scanEventTest_registerRecordDeviceDriver(pdbbase);
+    for (i = 0; i < NELEMENTS(events); i++) {
+        char substitutions[256];
+        sprintf(substitutions, "N=%d,EVENT=%s", i, events[i].name);
+        if (dbReadDatabase(&pdbbase, "scanEventTest.db",
+                "." OSI_PATH_LIST_SEPARATOR "..", substitutions))
+            testAbort("Error reading test database 'scanEventTest.db'");
+    }
+    testIocInitOk();
+    testDiag("Test if eventNameToHandle() strips spaces and handles numeric events");
+    for (i = 0; i < NELEMENTS(events); i++) {
+        EVENTPVT pev = eventNameToHandle(events[i].name);
+        /* test that some names are not events (num=0) */
+        if (events[i].num == 0)
+            testOk(pev == NULL, "\"%s\" -> no event", events[i].name);
+        else
+        {
+            expected_count[INDX(i)]++; /* +1 for postEvent */
+            if (events[i].num > 0)
+            {
+                testOk(pev != NULL, "\"%s\" -> numeric event %d", events[i].name, events[i].num);
+                expected_count[INDX(i)]++; /* +1 for post_event */
+            }
+            else
+            {
+                 /* test that some strings resolve the same event (num!=0) */
+                 if (!aliases[INDX(i)])
+                 {
+                     aliases[INDX(i)] = i;
+                     testOk(pev != NULL, "\"%s\" -> new named event", events[i].name);
+                 }
+                 else
+                 {
+                     testOk(pev == eventNameToHandle(events[aliases[INDX(i)]].name),
+                         "\"%s\" alias for \"%s\"", events[i].name, events[aliases[INDX(i)]].name);
+                 }
+            }
+        }
+        post_event(events[i].num); /* triggers numeric events only */
+        postEvent(pev);
+    }
+    
+    testDiag("Check calculated numeric events (backward compatibility)");
+    for (e = 0; e <= MAXEV; e++) {
+        testdbPutFieldOk("eventnum", DBR_LONG, e);
+        testdbGetFieldEqual("e1", DBR_LONG, e);
+        testdbGetFieldEqual("e2", DBR_LONG, e);
+        testdbPutFieldOk("e3", DBR_LONG, e);
+        testdbPutFieldOk("e3.PROC", DBR_LONG, 1);
+        if (e != 0)
+            for (i = 0; i < NELEMENTS(events); i++)
+                if (events[i].num == e) {
+                    expected_count[INDX(i)]+=3; /* +1 for eventnum->e1, +1 for e2<-eventnum, +1 for e3 */
+                    break;
+                }
+    }
+    
+    testDiag("Check if events have been processed the expected number of times");
+    for (i = 0; i < NELEMENTS(events); i++) {
+        char pvname[100];
+        sprintf(pvname, "c%d", i);
+        testDiag("Event \"%s\" expected %d times", events[i].name, expected_count[INDX(i)]);
+        testdbGetFieldEqual(pvname, DBR_LONG, expected_count[INDX(i)]);
+    }
+
+    return testDone();
+}
diff --git a/src/ioc/db/test/scanEventTest.db b/src/ioc/db/test/scanEventTest.db
new file mode 100644
index 0000000..9118823
--- /dev/null
+++ b/src/ioc/db/test/scanEventTest.db
@@ -0,0 +1,16 @@
+record(calc, "c$(N)") {
+  field(SCAN, "Event")
+  field(EVNT, "$(EVENT)")
+  field(CALC, "VAL+1")
+}
+record(dfanout, "eventnum") {
+  field(OUTA, "e1 PP")
+  field(FLNK, "e2")
+}
+record(event, "e1") {
+}
+record(event, "e2") {
+  field(INP,  "eventnum")
+}
+record(event, "e3") {
+}