From 9c1340d7d507e712501a4c056097280eab8ef200 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 1 Sep 2021 11:18:28 -0700 Subject: [PATCH 1/4] cleanup mbboDirect bit field handling B0 -> BF shall always follow the corresponding bit in VAL. Remove special handling for OMSL. --- .../database/src/std/rec/mbboDirectRecord.c | 134 ++++++++---------- .../src/std/rec/mbboDirectRecord.dbd.pod | 7 +- 2 files changed, 65 insertions(+), 76 deletions(-) diff --git a/modules/database/src/std/rec/mbboDirectRecord.c b/modules/database/src/std/rec/mbboDirectRecord.c index 7c5ba294cc..49ecf740e0 100644 --- a/modules/database/src/std/rec/mbboDirectRecord.c +++ b/modules/database/src/std/rec/mbboDirectRecord.c @@ -88,6 +88,14 @@ static long writeValue(mbboDirectRecord *); #define NUM_BITS 32 +static +void bitsFromVAL(mbboDirectRecord *prec) +{ + unsigned i; + for(i=0; ib0)[i] = !!(prec->val&(1u<udf && - prec->omsl == menuOmslsupervisory) { - /* Set initial B0 - B1F from VAL */ - epicsUInt32 val = prec->val; - epicsUInt8 *pBn = &prec->b0; - int i; - - for (i = 0; i < NUM_BITS; i++) { - *pBn++ = !! (val & 1); - val >>= 1; - } - } - + bitsFromVAL(prec); prec->mlst = prec->val; prec->oraw = prec->rval; prec->orbv = prec->rbv; @@ -174,24 +170,13 @@ static long process(struct dbCommon *pcommon) } prec->val = val; } - else if (prec->omsl == menuOmslsupervisory) { - epicsUInt8 *pBn = &prec->b0; - epicsUInt32 val = 0; - epicsUInt32 bit = 1; - int i; - - /* Construct VAL from B0 - B1F */ - for (i = 0; i < NUM_BITS; i++, bit <<= 1) - if (*pBn++) - val |= bit; - prec->val = val; - } else if (prec->udf) { - recGblSetSevr(prec, UDF_ALARM, prec->udfs); + recGblSetSevrMsg(prec, UDF_ALARM, prec->udfs, "UDFS"); goto CONTINUE; } prec->udf = FALSE; + bitsFromVAL(prec); /* Convert VAL to RVAL */ convert(prec); @@ -234,6 +219,9 @@ static long process(struct dbCommon *pcommon) recGblGetTimeStampSimm(prec, prec->simm, NULL); } + /* update bits to reflect any change made by dset */ + bitsFromVAL(prec); + monitor(prec); /* Wrap up */ @@ -255,60 +243,41 @@ static long special(DBADDR *paddr, int after) return 0; } - if (!after) - return 0; - - switch (paddr->special) { - case SPC_MOD: /* Bn field modified */ - if (prec->omsl == menuOmslsupervisory) { - /* Adjust VAL corresponding to the bit changed */ - epicsUInt8 *pBn = (epicsUInt8 *) paddr->pfield; - epicsUInt32 bit = 1 << (pBn - &prec->b0); - - if (*pBn) - prec->val |= bit; - else - prec->val &= ~bit; - - prec->udf = FALSE; - convert(prec); + if(after==0 && fieldIndex >= mbboDirectRecordB0 && fieldIndex <= mbboDirectRecordB1F) { + if(prec->omsl == menuOmslclosed_loop) { + /* To avoid confusion, reject changes to bit fields while in closed loop. + * Not a 100% solution as confusion can still arise if dset overwrites VAL. + */ + return S_db_noMod; } - break; - - case SPC_RESET: /* OMSL field modified */ - if (prec->omsl == menuOmslclosed_loop) { - /* Construct VAL from B0 - B1F */ - epicsUInt8 *pBn = &prec->b0; - epicsUInt32 val = 0, bit = 1; - int i; - for (i = 0; i < NUM_BITS; i++, bit <<= 1) - if (*pBn++) - val |= bit; - prec->val = val; + } else if(after==1 && fieldIndex >= mbboDirectRecordB0 && fieldIndex <= mbboDirectRecordB1F) { + /* Adjust VAL corresponding to the bit changed */ + epicsUInt8 *pBn = (epicsUInt8 *) paddr->pfield; + epicsUInt32 bit = 1 << (pBn - &prec->b0); + epicsUInt32 oobit = prec->obit; + + /* Because this is !(VAL and PP), dbPut() will always post a monitor on this B* field + * after we return. We must keep track of this change separately from MLST to handle + * situations where VAL and B* are changed prior to next monitor(). eg. by dset to + * reflect bits actually written. This is the role of OBIT. + */ + + if (*pBn) { + prec->val |= bit; + prec->obit |= bit; + } else { + prec->val &= ~bit; + prec->obit &= ~bit; } - else if (prec->omsl == menuOmslsupervisory) { - /* Set B0 - B1F from VAL and post monitors */ - epicsUInt32 val = prec->val; - epicsUInt8 *pBn = &prec->b0; - int i; - - for (i = 0; i < NUM_BITS; i++, pBn++, val >>= 1) { - epicsUInt8 oBn = *pBn; - - *pBn = !! (val & 1); - if (oBn != *pBn) - db_post_events(prec, pBn, DBE_VALUE | DBE_LOG); - } - } - break; + if(oobit!=prec->obit) + db_post_events(prec, &prec->obit, DBE_VALUE|DBE_LOG); + + prec->udf = FALSE; + convert(prec); - default: - recGblDbaddrError(S_db_badChoice, paddr, "mbboDirect: special"); - return S_db_badChoice; } - prec->udf = FALSE; return 0; } @@ -329,9 +298,24 @@ static void monitor(mbboDirectRecord *prec) if (prec->mlst != prec->val) { events |= DBE_VALUE | DBE_LOG; prec->mlst = prec->val; + db_post_events(prec, &prec->mlst, events); } - if (events) + if (events) { db_post_events(prec, &prec->val, events); + } + { + unsigned i; + epicsUInt32 bitsChanged = prec->obit ^ (epicsUInt32)prec->val; + + for(i=0; ib0)+i, events | DBE_VALUE | DBE_LOG); + } + } + prec->obit = prec->val; + db_post_events(prec, &prec->obit, events); + } events |= DBE_VALUE | DBE_LOG; if (prec->oraw != prec->rval) { diff --git a/modules/database/src/std/rec/mbboDirectRecord.dbd.pod b/modules/database/src/std/rec/mbboDirectRecord.dbd.pod index 649e752c89..7dedc8eeca 100644 --- a/modules/database/src/std/rec/mbboDirectRecord.dbd.pod +++ b/modules/database/src/std/rec/mbboDirectRecord.dbd.pod @@ -104,7 +104,6 @@ Parameters> for more on the record name (NAME) and description (DESC) fields. field(OMSL,DBF_MENU) { prompt("Output Mode Select") promptgroup("50 - Output") - special(SPC_RESET) pp(TRUE) interest(1) menu(menuOmsl) @@ -154,6 +153,11 @@ Parameters> for more on the record name (NAME) and description (DESC) fields. special(SPC_NOMOD) interest(3) } + field(OBIT,DBF_LONG) { + prompt("Last Bit mask Monitored") + special(SPC_NOMOD) + interest(3) + } field(SHFT,DBF_USHORT) { prompt("Shift") promptgroup("50 - Output") @@ -169,6 +173,7 @@ MASK is used by device support routine to read the hardware register. Record support sets low order NOBT bits. Device support can shift this value. MLST holds the value when the last monitor for value change was triggered. +OBIT has similar role wrt. the B* fields. =fields NOBT, ORAW, MASK, MLST From bf2cdc9f61f1bceb826bc55fc90a27fd025b470a Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Sat, 2 Oct 2021 19:52:11 -0500 Subject: [PATCH 2/4] mbboDirect: Remove debugging db_post_events() calls --- modules/database/src/std/rec/mbboDirectRecord.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/modules/database/src/std/rec/mbboDirectRecord.c b/modules/database/src/std/rec/mbboDirectRecord.c index 49ecf740e0..d3067f7c70 100644 --- a/modules/database/src/std/rec/mbboDirectRecord.c +++ b/modules/database/src/std/rec/mbboDirectRecord.c @@ -255,7 +255,6 @@ static long special(DBADDR *paddr, int after) /* Adjust VAL corresponding to the bit changed */ epicsUInt8 *pBn = (epicsUInt8 *) paddr->pfield; epicsUInt32 bit = 1 << (pBn - &prec->b0); - epicsUInt32 oobit = prec->obit; /* Because this is !(VAL and PP), dbPut() will always post a monitor on this B* field * after we return. We must keep track of this change separately from MLST to handle @@ -270,12 +269,9 @@ static long special(DBADDR *paddr, int after) prec->val &= ~bit; prec->obit &= ~bit; } - if(oobit!=prec->obit) - db_post_events(prec, &prec->obit, DBE_VALUE|DBE_LOG); prec->udf = FALSE; convert(prec); - } return 0; @@ -298,7 +294,6 @@ static void monitor(mbboDirectRecord *prec) if (prec->mlst != prec->val) { events |= DBE_VALUE | DBE_LOG; prec->mlst = prec->val; - db_post_events(prec, &prec->mlst, events); } if (events) { db_post_events(prec, &prec->val, events); @@ -314,7 +309,6 @@ static void monitor(mbboDirectRecord *prec) } } prec->obit = prec->val; - db_post_events(prec, &prec->obit, events); } events |= DBE_VALUE | DBE_LOG; From dabcf893f7170325a22fd830a6caeec7048ba923 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Sun, 3 Oct 2021 00:53:35 -0500 Subject: [PATCH 3/4] mbboDirect: Fix initialization from VAL vs. B* --- .../database/src/std/rec/mbboDirectRecord.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/modules/database/src/std/rec/mbboDirectRecord.c b/modules/database/src/std/rec/mbboDirectRecord.c index d3067f7c70..511f4b58b3 100644 --- a/modules/database/src/std/rec/mbboDirectRecord.c +++ b/modules/database/src/std/rec/mbboDirectRecord.c @@ -139,7 +139,24 @@ static long init_record(struct dbCommon *pcommon, int pass) status = 0; } - bitsFromVAL(prec); + if (!prec->udf) + bitsFromVAL(prec); + else { + /* Did user set any of the B0-B1F fields? */ + epicsUInt8 *pBn = &prec->b0; + epicsUInt32 val = 0, bit = 1; + int i; + + for (i = 0; i < NUM_BITS; i++, bit <<= 1) + if (*pBn++) + val |= bit; + + if (val) { /* Yes! */ + prec->val = val; + prec->udf = FALSE; + } + } + prec->mlst = prec->val; prec->oraw = prec->rval; prec->orbv = prec->rbv; From e867b0a095b1fddaeb337d4761248165cfbca3c5 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Mon, 4 Oct 2021 23:36:01 -0500 Subject: [PATCH 4/4] mbboDirect: Document the behavior changes Wording assumes this will go into the EPICS 7.0.6.1 release. --- .../src/std/rec/mbboDirectRecord.dbd.pod | 74 +++++++++++++------ 1 file changed, 52 insertions(+), 22 deletions(-) diff --git a/modules/database/src/std/rec/mbboDirectRecord.dbd.pod b/modules/database/src/std/rec/mbboDirectRecord.dbd.pod index 7dedc8eeca..55461de04d 100644 --- a/modules/database/src/std/rec/mbboDirectRecord.dbd.pod +++ b/modules/database/src/std/rec/mbboDirectRecord.dbd.pod @@ -9,11 +9,14 @@ =title Multi-Bit Binary Output Direct Record (mbboDirect) -The mbboDirect record performs the opposite function to that of the mbbiDirect -record. It accumulates bits (in the fields B0 - BF) as unsigned characters, and -converts them to a word which is then written out to hardware. If a bit field is -non-zero, it is interpreted as a binary 1. On the other hand, if it is zero, it -is interpreted as a binary 0. +The mbboDirect record performs roughly the opposite function to that of the +L. + +It can accept boolean values in its 32 bit fields (B0-B9, BA-BF, B10-B19 and +B1A-B1F), and converts them to a 32-bit signed integer in VAL which is provided +to the device support. A zero value in a bit field becomes a zero bit in VAL, a +non-zero value in a bit field becomes a one bit in VAL, with B0 being the least +signficant bit and B1F the MSB/sign bit. =recordtype mbboDirect @@ -33,24 +36,51 @@ These fields are listed in L. =head3 Desired Output Parameters -The mbboDirect record, like all output records, must specify where its output -originates. The output mode select field (OMSL) determines whether the output -originates from another record or from database access. When set to C<<< -closed_loop >>>, the desired output is retrieved from the link specified in the -desired output (DOL) field--which can specify either a database or channel -access link--and placed into the VAL field. When set to C<<< supervisory >>>, -the DOL field is ignored and the current value of VAL is used. The desired -output can be written into the VAL field via dpPuts at run-time when the record -is in C<<< supervisory >>> mode. DOL can also be a constant, in which case VAL -is initialized to the constant value. Note that OMSL cannot be C<<< closed_loop ->>> when DOL is a constant. - -VAL is then converted to RVAL in the routine described in the next section. -However, the C<<< Soft Channel >>> device support module for the mbboDirect -record writes the VAL field's value without any conversion. +Like all output records, the mbboDirect record must specify where its output +should originate when it gets processed. The Output Mode SeLect field (OMSL) +determines whether the output value should be read from another record or not. +When set to C<<< closed_loop >>>, a 32-bit integer value (the "desired output") +will be read from a link specified in the Desired Output Link (DOL) field and +placed into the VAL field. + +When OMSL is set to C<<< supervisory >>>, the DOL field is ignored during +processing and the contents of VAL are used. A value to be output may thus be +written direcly into the VAL field from elsewhere as long as the record is in +C<<< supervisory >>> mode. =fields OMSL, DOL, VAL +=head4 Bit Fields + +The fields B0 through BF and B10 through B1F provide an alternative way to set +the individual bits of the VAL field when the record is in C<<< supervisory >>> +mode. Writing to one of these fields will then modify the corresponding bit in +VAL, and writing to VAL will update these bit fields from that value. + +The VAL field is signed so it can be accessed through Channel Access as an +integer; if it were made unsigned (a C) its representation through +Channel Access would become a C, which could cause problems with some +client programs. + +Prior to the EPICS 7.0.6.1 release the individual bit fields were not updated +while the record was in C<<< closed_loop >>> mode with VAL being set from the +DOL link, and writing to the bit fields in that mode could cause the record to +process but the actual field values would not affect VAL at all. Changing the +OMSL field from C<<< closed_loop >>> to C<<< supervisory >>> would set the bit +fields from VAL at that time and trigger a monitor event for the bits that +changed at that time. At record initialization if VAL is defined and the OMSL +field is C<<< supervisory >>> the bit fields would be set from VAL. + +From EPICS 7.0.6.1 the bit fields get updated from VAL during record processing +and monitors are triggered on them in either mode. Attempts to write to the bit +fields while in C<<< closed_loop >>> mode will be rejected by the C +routine which may trigger an error from the client that wrote to them. During +initialization if the record is still undefined (UDF) after DOL has been read +and the device support initialized but at least one of the B0-B1F fields is +non-zero, the VAL field will be set from those fields and UDF will be cleared. + +=fields B0, B1, B2, B3, B4, B5, B6, B7, B8, B9, BA, BB, BC, BD, BE, BF, B10, B11, B12, B13, B14, B15, B16, B17, B18, B19, B1A, B1B, B1C, B1D, B1E, B1F + =head3 Convert and Write Parameters For records that are to write values to hardware devices, the OUT output link @@ -173,9 +203,9 @@ MASK is used by device support routine to read the hardware register. Record support sets low order NOBT bits. Device support can shift this value. MLST holds the value when the last monitor for value change was triggered. -OBIT has similar role wrt. the B* fields. +OBIT has a similar role for bits held in the B0-B1F fields. -=fields NOBT, ORAW, MASK, MLST +=fields NOBT, ORAW, MASK, MLST, OBIT =head3 Simulation Mode Parameters