g+
g+ Communities
Argonne National Laboratory

Experimental Physics and
Industrial Control System

2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  <20132014  Index 2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  <20132014 
<== Date ==> <== Thread ==>

Subject: Re: mbboDirect changes
From: Andrew Johnson <anj@aps.anl.gov>
To: Mark Rivers <rivers@cars.uchicago.edu>
Cc: EPICS core-talk <core-talk@aps.anl.gov>, Eric Norum <wenorum@lbl.gov>, Rod Nussbaumer <bomr@triumf.ca>
Date: Fri, 15 Feb 2013 12:09:27 -0600
Hi Mark,

Sorry for the delay in responding.

On 2013-02-13 Mark Rivers wrote:
> The asyn device support init_record for mbboDirect currently does the
>  following:
> 
>     /* Read the current value from the device */
>     status = pasynUInt32DigitalSyncIO->read(pPvt->pasynUserSync,
>                       &value, pPvt->mask, pPvt->pasynUser->timeout);
>     if (status == asynSuccess) {
>         pr->rval = value & pr->mask;
>         return 0;
>     }
>     return 2;
> 
> So if it successfully reads a value from the driver it sets rval (not val)
> and returns 0.  How do I modify your code below to handle this case?  Do I
> simply replace prec->val with prec->rval?
> 
>         epicsUInt16 val = prec->rval;

When you return 0, the record does this:

		rval = prec->rval;
		if (prec->shft > 0) rval >>= prec->shft;
		prec->val =  (unsigned short) rval;
		prec->udf = FALSE;

The user can set SHFT in their .db file, which you should honor (ignoring 
negative shifts as they result in Undefined Behavior according to the C 
standard).  You might want to incorporate the above lines straight into your 
init_record() routine and always return 2 to save duplicating work.

Since you already have the value in what I assume is a 32-bit integer, I'd go 
with something like this for your mbboDirect device support (untested):

    /* Read the current value from the device */
    status = pasynUInt32DigitalSyncIO->read(pPvt->pasynUserSync,
                      &value, pPvt->mask, pPvt->pasynUser->timeout);
    if (status == asynSuccess) {
        epicsUInt8 *pBn = &pr->b0;
        int i;

        value &= pr->mask;
        if (pr->shft > 0) value >>= pr->shft;
        pr->val =  (unsigned short) value;
        pr->udf = FALSE;

        for (i = 0; i < 16; i++) {
            *pBn++ = !! (value & 1);
            value >>= 1;
        }
    }
    return 2;

HTH,

- Andrew

> -----Original Message-----
> From: Andrew Johnson [mailto:anj@aps.anl.gov]
> Sent: Thursday, December 06, 2012 12:22 PM
> To: Mark Rivers
> Cc: Eric Norum; Rod Nussbaumer; EPICS core-talk
> Subject: mbboDirect changes
> 
> Hi Mark,
> 
> On 2012-11-26 Mark Rivers wrote:
> > What was the conclusion of the mbboDirect record initialization thread?
> 
> The mbbxDirect records are a bit of a mess.  It looks like they were
>  written by hacking away at the mbbx records until they did what the author
>  wanted, but they don't seem to implement a coherent model of an object. 
>  The wiki page for mbboDirect still talks about the SDEF field, which was
>  one of the fields that were removed from the mbbo record, and doesn't
>  discuss the Bn fields in the record description text at all.
> 
> For 3.14.x IOCs a device support's init_record() routine should initialize
>  the Bn fields itself if it sets RVAL/VAL and returns 0/2.  Here's the code
>  for doing that:
> 
>         epicsUInt16 val = prec->val;
>         epicsUInt8 *pBn = &prec->b0;
>         int i;
> 
>         for (i = 0; i < 16; i++) {
>             *pBn++ = !! (val & 1);
>             val >>= 1;
>         }
> 
> I have uncommitted changes for 3.15 where the record type initializes the
>  Bn fields from RVAL/VAL after the init_record() call as long as UDF is
>  clear and OMSL is supervisory.  UDF gets cleared for you if you set RVAL
>  and return 0, but you will have to clear UDF yourself if you set VAL and
>  return 2 (this matches the expectations of the other output record types).
> 
> I've made setting the Bn fields conditional on OMSL to match the process()
>  and special() routines, which already use OMSL to determine whether the Bn
>  fields should be read from or not.  Closed-loop means process() reads from
>  DOL into VAL and ignores the Bn field values; supervisory means the Bn
>  fields are read to set VAL, so it only makes sense to initialize the Bn
>  fields in supervisory mode.
> 
> While trying to internalize how the record initialization works after
>  making that modification, I came up with the following descriptions, which
>  show that the interaction between the various parameters make the record
>  hard to comprehend:
> 
> * For device support whose init_record() routine reads RVAL from the
>   hardware and returns 0, the VAL field will always contain the shifted
>   RVAL; the Bn fields will be set from the VAL field if OMSL=supervisory.
> 
> * For device support whose init_record() routine returns 2, unless DOL
>   contains an integer literal the VAL and Bn fields will not be modified
>   by the record initialization (if the device support doesn't set them,
>   they will get whatever values were loaded from the .db files).  If DOL
>   is set to an integer literal, that value will be used to initialize the
>   VAL field.  The Bn fields are set from the VAL field if OMSL=supervisory
>   in this case.
> 
> Note that the OMSL and Bn fields are special; setting OMSL to supervisory
> results in VAL being calculated from Bn, and subsequent changes to the Bn
> fields also immediately update VAL.  However the Bn fields are never set
>  from VAL anywhere in the existing record code, nor does the record post
>  monitors on them (since it never changes them itself).  This caused me to
>  pause before committing the above change.
> 
> I might have expected the Bn fields to be updated from VAL either
>  immediately after reading DOL in closed-loop mode, or at least whenever
>  OMSL gets changed from closed-loop to supervisory.  The existing code
>  treats the Bn fields as pure inputs for manually setting the bits when in
>  supervisory mode.  I will probably also make special() update the Bn
>  fields from VAL and post monitors on those that change when OMSL gets set
>  back to supervisory; I think that's rather more friendly.
> 
> Comments welcome.
> 
> - Andrew
-- 
There is no such thing as a free lunch.  When invited for lunch,
it is best to check if you are there to eat, or to be eaten.
-- Clive Robinson

References:
RE: mbboDirect changes Mark Rivers

Navigate by Date:
Prev: RE: mbboDirect changes Mark Rivers
Next: [Merge] lp:~epics-core/epics-base/array-opt into lp:epics-base mdavidsaver
Index: 2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  <20132014 
Navigate by Thread:
Prev: RE: mbboDirect changes Mark Rivers
Next: [Merge] lp:~epics-core/epics-base/array-opt into lp:epics-base mdavidsaver
Index: 2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  <20132014 
ANJ, 18 Nov 2013 Valid HTML 4.01! · Home · News · About · Base · Modules · Extensions · Distributions · Download ·
· EPICSv4 · IRMIS · Talk · Bugs · Documents · Links · Licensing ·