RE: Inconsistency in devAsynOctet for stringout and waveform records

[Mark Rivers <rivers@cars.uchicago.edu>]

Hi Torsten,

> However, it seems as if the ASYN_EOM_CNT "get lost" and the  asynPortDriver does not check for ASYN_EOM_CNT,
> but blindly applies strlen() to the read buffer. Is the the case ?

No, that is not the case.  You are mixing up 2 things.  asynPortDriver never talks directly to drvAsynIPPort or drvAsynSerialPort or another other asynOctet driver.  It only does that through a derived class driver, such as a motor driver, or an areaDetector driver, etc.    asynPortDriver has a parameter library.  For the asynOctet interface that parameter library has setStringParam() and getStringParam() methods.  So the parameter library is explicitly only used for strings, and applying strlen() is appropriate.  But asynPortDriver could be used with the asynOctet interface for other types of data if a driver needed to do so, it just could not use the parameter library.  In 99% of the cases the asynOctet interface from device support will be used for strings, so I wanted to make that convenient in asynPortDriver by supporting that type in the parameter library.  But it is not limited to strings.

> The second question:
> It seems as if the whole asyn framework is designed to handle octets as NUL-terminated strings, and NUL-terminated strings as octets,
> Because strlen() is used in the parameter library, there is now way to handle binary data anyway.
> (Otherwise I would suspect that we don't need strlen() at all... ) As this seems to be the case, it looks for me as if the
> asyn framework only handles NUL terminated strings today,

No, that is not correct.  You are mixing up "the whole asyn framework" with the "string support in the asynPortDriver parameter library".  Those are very different things.  The asyn framework is very commonly used to send binary data over the asynOctet interface.  For example:

- Using streamDevice to send and receive binary data using the asynOctet interface to the drvAsynIPPort and drvAsynSerial port drivers.

- My asyn Modbus driver.  It also uses the asynOctet interface to send and receive binary data using the asynOctet interface to the drvAsynIPPort and drvAsynSerial port drivers.

The asyn framework also supports other interfaces like asynInt8Array which can be used to send binary data to drivers, and the standard asyn device support works fine for these interfaces as well.

The discussion here is confined to a much more specific subset of the asyn framework, namely the standard asyn device support for stringout and waveform records. Most people do not use this device support to send data directly to the drvAsynIPPort and drvAsynSerialPort drivers, they normally use specialized device support like StreamDevice or motor record device support for this.

In fact I just did a search of all of the databases in synApps.  If a database is using the devAsynOctet.c for stringout or waveform records in the standard asyn device support it will have its output link DTYP set to "asynOctetWrite".  So I searched for that string in all of the synApps databases.

find . -name '*.template' -exec grep -H asynOctetWrite {} \;
find . -name '*.db' -exec grep -H asynOctetWrite {} \;

These are the drivers that use that device support:

drvDG645:  Uses asynOctet->write only for strings, and ignores the numchars argument, uses strlen().

tds:  Uses asynOctet->write only for strings, and ignores the numchars argument or uses asynPortDriver base class which uses strings.

areaDetector, motor, dxp, modbus, quadEM:  These all use asynPortDriver and only use the asynOctet device support to send strings to the driver.


> And the write function can safely treat the maxChars parameter, like this, re-using my_strnlen() from
> asyn/devEpics/devAsynOctet.c
...
> What do I miss ?

The problem is that with the existing version of asyn if you write an asyn port driver that uses the standard asyn device support for strings then the maxChars value your driver receives will be different depending on whether the string that was sent to your driver came from a stringout record or a waveform record.  If it came from a stringout record it will not include the trailing nil while if it came from a waveform record it will include the trailing nil.  But people assume that a waveform record with FTVL=CHAR can be swapped transparently for a stringout record, to support string lengths greater than 40 characters. This inconsistency imposes a complexity on writing an asyn port driver that I would like to avoid if possible.

Mark



________________________________________
From: Torsten Bögershausen [torsten.bogershausen@esss.se]
Sent: Saturday, July 23, 2016 9:22 PM
To: Mark Rivers; Eric Norum
Cc: tech-talk@aps.anl.gov list
Subject: Re: Inconsistency in devAsynOctet for stringout and waveform records

2 Comments here:
- The readIt() implementation add a trailing NUL,
   if there is space in the buffer:
   asyn/drvAsynSerial/drvAsynIPPort.c, line 778:
     /* If there is room add a null byte */
     if (thisRead < (int) maxchars)
         data[thisRead] = 0;
     else
         reason |= ASYN_EOM_CNT;
     if (gotEom) *gotEom = reason;

However, it seems as if the ASYN_EOM_CNT "get lost"
and the  asynPortDriver does not check for ASYN_EOM_CNT,
but blindly applies strlen() to the read buffer.
Is the the case ?

The second question:
It seems as if the whole asyn framework is designed to handle
octets as NUL-terminated strings, and NUL-terminated strings
as octets,
Because strlen() is used in the parameter library, there
is now way to handle binary data anyway.
(Otherwise I would suspect that we don't need strlen() at all... )
As this seems to be the case, it looks for me as if the
asyn framework only handles NUL terminated strings today,

And the write function can safely treat the maxChars parameter,
like this, re-using my_strnlen() from
asyn/devEpics/devAsynOctet.c

extern "C" {static asynStatus writeOctet(void *drvPvt, asynUser *pasynUser,
                               const char *value, size_t maxChars,
size_t *nActual)
{
     asynPortDriver *pPvt = (asynPortDriver *)drvPvt;
     asynStatus status;
     size_t len = my_strnlen(value, maxChars);

     if (len > maxChars) len = maxChars;

     pPvt->lock();
     status = pPvt->writeOctet(pasynUser, value, len, nActual);
     pPvt->unlock();
     return(status);
}}

What do I miss ?






On 07/24/2016 02:42 AM, Mark Rivers wrote:
>> This is where I think that caput -S is now doing things incorrectly.
>
> But it is not just caput -S.  It is medm, and dbpf in the IOC shell, etc.  I think that ship has sailed: strings in waveform records include the nil in NORD.  In fact that is what Andrew recommended in this thread for the inverse problem, i.e. reads from a driver:
>
> http://www.aps.anl.gov/epics/tech-talk/2012/msg02251.php<http://www.aps.anl.gov/epics/tech-talk/2012/msg02251.php>
>
> asynPortDriver is now doing it that way on read operations.
>
> Mark
>
>
>
> ________________________________
> From: Eric Norum [wenorum@lbl.gov]
> Sent: Saturday, July 23, 2016 6:43 PM
> To: Mark Rivers
> Cc: tech-talk@aps.anl.gov list
> Subject: Re: Inconsistency in devAsynOctet for stringout and waveform records
>
> But it seems to me that what you are requesting is what creates the need for drviers to require two paths and to somehow intuit which one to take.
> I think that we’re in agreement that sending null characters to a device is a bad idea, right?  If so, then if the stringout record included the trailing null in the request count how could a driver know that in this case it should strip the trailing null when in the case of being called from a waveform record it should send the full request?
>
> I think the existing behaviour is right.  Drivers send the actual request count (which in the stringout case does not include the terminator).  If a waveform record is being used to handle long strings writers should not include the trailing terminator in the request count.  This is where I think that caput -S is now doing things incorrectly.
>
> On Jul 23, 2016, at 4:29 PM, Mark Rivers <rivers@cars.uchicago.edu<redir.aspx?REF=fis2kRoQaO-WzVqEI0E1NtRHOLgUx0SkeOsSi2yRXWggtf_BWrPTCAFtYWlsdG86cml2ZXJzQGNhcnMudWNoaWNhZ28uZWR1>> wrote:
>
> The problem is that then all asyn port drivers that are expecting to receive strings need to handle two different cases, one where the length being passed includes the nil, and one where it does not.  When the user changes from a stringout to a waveform record because their string just exceeded 40 characters, the driver receives something fundamentally different.
>
> Is this what we want?
>
> I am not proposing that we change the ability of the waveform record to handle arbitrary data, because I am not changing the behavior for the waveform record.  I am proposing changing the behavior of the stringout record to match the waveform record.
>
>