Experimental Physics and
Industrial Control System

"Davidsaver, Michael" <[email protected]> · Mon, 6 Jul 2009 17:35:03 -0400

Hello Ben,

My purpose in suggesting this change is really only that the way things
are now strikes me as strange.  I do not think it is generally good
practice to have non-static variable definitions in a header file.
Also, that there are so many redundant declarations of the alarm state
and severity (ie MINOR_ALARM, epicsSevMinor, and menuAlarmSevrMINOR).

> I think it is questionable whether it is really an improvement. Alarm
> stati
> and severities are a central concept used all over the place in the
> core of
> the EPICS database, moving this stuff to CA doesn't sound right to me.
> What
> is the gain?

It would be good to put the string array definitions somewhere other
than a header.
It doesn't really matter which library, perhaps libdb is a better
choice.

That said, I think the correct thing would be to have a function (maybe
const char* epicsGetSev(enum ...)) instead of allowing direct access to
the array of strings.  This would allow codes to be added and strings
changed more easily, even at runtime (think i18n).

> > We've never really got the alarm enums right; there are equivalent
> > definitions in src/db/menuAlarmStat.dbd which must match alarm.h,
and
> we
> > currently have the header alarmString.h which currently instantiates
> a
> > global alarm string array into whatever file it's included by.  This
> is
> > used both internally (by catools and cap5) and externally (by MEDM,
> ALH,
> > etc).

As for the second item.  I have attached a couple of patches which teach
the dbToMenuH utility to emit some extra code when generating headers.
This allows most of the redundancy in alarm.h and alarmString.h to be
removed.  As an example menuAlarmSevr.h would look like this:

#ifndef INCmenuAlarmSevrH
#define INCmenuAlarmSevrH
typedef enum {
	epicsSevNone,
	epicsSevMinor,
	epicsSevMajor,
	epicsSevInvalid
} menuAlarmSevr;
epicsShareExtern const char* menuAlarmSevrStrings[4];
#endif

#ifdef menuAlarmSevrStringsDef
const char* menuAlarmSevrStrings[4] = {
	"NO_ALARM",
	"MINOR",
	"MAJOR",
	"INVALID"
};
#endif

Patches (against 3.14 branch 20090621)

These are independent of my previous patches.

1) Move alarm.h and alarmString.h from src/dbStatic/ to src/db/

Also change 'INC +=' in relevant Makefiles.  I am not attaching this one
since it is long but uninteresting.

2) Teach dbToMenuH to emit an array of strings

The generated headers include an unconditional extern declaration of an
appropriately sized array of strings.  A separate macro controls
inclusion of the definition.

3) Change alarm menu choice enum names.

Of the three redundant classes of names (MINOR_ALARM, epicsSevMinor, and
menuAlarmSevrMINOR), it seems easiest to remove menuAlarmSevrMINOR, but
epicsSevMinor could also go, or both.  In Base only the MINOR_ALARM
(typedef) names are used.

4) Rewrite alarm.h and alarmString.h to use generated enums and strings

These two headers become essentially wrappers for menuAlarmSevr.h and
menuAlarmStat.h.

I have tried this with ALH and EDM (successfully).  The loss of the
menuAlarmSevrMINOR names is the only API change.

Michael

Experimental Physics and Industrial Control System

Experimental Physics and
Industrial Control System