Experimental Physics and
| |||||||||||||||
|
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 Attachment:
0002-teach-dbToMenuH-to-emit-an-array-of-item-strings.patch Attachment:
0003-Change-names-of-alarm-menu-defintions-to-follow-alar.patch Attachment:
0004-Rewrite-alarm.h-and-alarmString.h-to-use-generated-e.patch
| ||||||||||||||
ANJ, 02 Feb 2012 |
·
Home
·
News
·
About
·
Base
·
Modules
·
Extensions
·
Distributions
·
Download
·
· Search · EPICS V4 · IRMIS · Talk · Bugs · Documents · Links · Licensing · |