Folks,
I have now tested changing asynPortDriver so that it returns strlen(value)+1 in readOctet and octetCallback.
First I tested the current version of asyn (R4-20) without the fix.
Here are the results of reading (caget) and monitoring (camonitor) a waveform record from areaDetector containing a file name. Initially the PV wave "test_12345", and I then shortened it to "test_1234" and "test_123".
Here is the output of caget on base 3.14.12.2:
corvette:~>/usr/local/epics/base-3.14.12.2/bin/linux-x86/caget -S 13SIM1:netCDF1:FileName_RBV
13SIM1:netCDF1:FileName_RBV test_12345
corvette:~>/usr/local/epics/base-3.14.12.2/bin/linux-x86/caget -S 13SIM1:netCDF1:FileName_RBV
13SIM1:netCDF1:FileName_RBV test_1234
corvette:~>/usr/local/epics/base-3.14.12.2/bin/linux-x86/caget -S 13SIM1:netCDF1:FileName_RBV
13SIM1:netCDF1:FileName_RBV test_123
So it is fine.
Here is the output of "camonitor -s" on 3.14.10, which did not support the -S option.
corvette:asyn/asyn/asynPortDriver>/usr/local/epics/base-3.14.10/bin/linux-x86/camonitor -s 13SIM1:netCDF1:FileName_RBV
13SIM1:netCDF1:FileName_RBV 2012-11-13 14:38:53.259329 256 116 't' 101 'e' 115 's' 116 't' 95 '_' 49 '1' 50 '2' 51 '3' 52 '4' 53 '5' 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
13SIM1:netCDF1:FileName_RBV 2012-11-13 14:39:18.875615 256 116 't' 101 'e' 115 's' 116 't' 95 '_' 49 '1' 50 '2' 51 '3' 52 '4' 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
13SIM1:netCDF1:FileName_RBV 2012-11-13 14:39:26.859753 256 116 't' 101 'e' 115 's' 116 't' 95 '_' 49 '1' 50 '2' 51 '3' 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
So it is correct, the strings are there, and the array is Nil terminated.
Here is the output of camonitor -S on base 3.14.12.2:
corvette:~>/usr/local/epics/base-3.14.12.2/bin/linux-x86/camonitor -S 13SIM1:netCDF1:FileName_RBV
13SIM1:netCDF1:FileName_RBV 2012-11-13 14:38:53.259329 test_12345
13SIM1:netCDF1:FileName_RBV 2012-11-13 14:39:18.875615 test_12345
13SIM1:netCDF1:FileName_RBV 2012-11-13 14:39:26.859753 test_12345
It is NOT correct. Because camonitor is only receiving NORD characters and is not adding a Nil if one is not present it prints the same string every time. You may recall that Ralph Lange posted a tech-talk message on Sept. 11, 2012 in which he fixed camonitor for the next release of EPICS.
His words: " Fix added. (Reluctantly, with a sour smile.)"
I then changed asynPortDriver so that it returns strlen(value)+1 in readOctet() and octetCallback. Here are the results from the same tests as above:
Here is the output of caget on base 3.14.12.2:
corvette:~>/usr/local/epics/base-3.14.11/bin/linux-x86/caget -S 13SIM1:netCDF1:FileName_RBV
13SIM1:netCDF1:FileName_RBV test_12345
corvette:~>/usr/local/epics/base-3.14.11/bin/linux-x86/caget -S 13SIM1:netCDF1:FileName_RBV
13SIM1:netCDF1:FileName_RBV test_1234
corvette:~>/usr/local/epics/base-3.14.11/bin/linux-x86/caget -S 13SIM1:netCDF1:FileName_RBV
13SIM1:netCDF1:FileName_RBV test_123
So it is still correct.
Here is the output of "camonitor -s" on 3.14.10, which did not support the -S option.
corvette:asyn/asyn/asynPortDriver>/usr/local/epics/base-3.14.10/bin/linux-x86/camonitor -s 13SIM1:netCDF1:FileName_RBV
13SIM1:netCDF1:FileName_RBV 2012-11-13 14:52:08.815884 256 116 't' 101 'e' 115 's' 116 't' 95 '_' 49 '1' 50 '2' 51 '3' 52 '4' 53 '5' 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
13SIM1:netCDF1:FileName_RBV 2012-11-13 14:52:54.912002 256 116 't' 101 'e' 115 's' 116 't' 95 '_' 49 '1' 50 '2' 51 '3' 52 '4' 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
13SIM1:netCDF1:FileName_RBV 2012-11-13 14:53:02.752834 256 116 't' 101 'e' 115 's' 116 't' 95 '_' 49 '1' 50 '2' 51 '3' 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
So it is still correct, the strings are there, and the array is Nil terminated.
Here is the output of camonitor -S on base 3.14.12.2:
corvette:~>/usr/local/epics/base-3.14.12.2/bin/linux-x86/camonitor -S 13SIM1:netCDF1:FileName_RBV
13SIM1:netCDF1:FileName_RBV 2012-11-13 14:52:08.815884 test_12345
13SIM1:netCDF1:FileName_RBV 2012-11-13 14:52:54.912002 test_1234
13SIM1:netCDF1:FileName_RBV 2012-11-13 14:53:02.752834 test_123
So it is NOW CORRECT. Because NORD now includes the trailing Nil the version of camonitor in base 3.14.12.2 works OK.
So do we agree with Andrew's recommendation that NORD should including the trailing Nil, or should NORD be the actual number of elements that were read, and clients must be fixed to handle this if they want to use long string support?
Mark
-----Original Message-----
From: Mark Rivers
Sent: Tuesday, November 13, 2012 1:58 PM
To: 'Eric Norum'; Andrew Johnson
Cc: [email protected] talk
Subject: RE: Long string support in CA clients and device support
Folks,
I've just done some investigation to see how asynPortDriver currently handles writing and reading strings in the parameter library.
The lowest level class is paramVal.cpp which implements setString and getString as follows:
void paramVal::setString(const char *value)
{
if (type != asynParamOctet)
throw ParamValWrongType("paramVal::setString can only handle asynParamOctet");
if (!isDefined() || (strcmp(data.sval, value)))
{
setDefined(true);
if (data.sval != NULL)
free(data.sval);
data.sval = epicsStrDup(value);
setValueChanged();
}
}
char* paramVal::getString()
{
if (type != asynParamOctet)
throw ParamValWrongType("paramVal::getString can only handle asynParamOctet");
if (!isDefined())
throw ParamValNotDefined("paramVal::geString value not defined");
return data.sval;
}
So setString just calls epicsStrDup (which adds a Nil terminator) and getString just returns a pointer to the string. No problems here.
The next class up is paramList.cpp, which implements setString and getString as follows:
asynStatus paramList::setString(int index, const char *value)
{
if (index < 0 || index >= this->nVals) return asynParamBadIndex;
try {
getParameter(index)->setString(value);
registerParameterChange(getParameter(index), index);
}
catch (ParamValWrongType&){
return asynParamWrongType;
}
catch (ParamListInvalidIndex&) {
return asynParamBadIndex;
}
return asynSuccess;
}
asynStatus paramList::getString(int index, int maxChars, char *value)
{
asynStatus status=asynSuccess;
try {
if (maxChars > 0) {
paramVal *pVal = getParameter(index);
status = pVal->getStatus();
strncpy(value, pVal->getString(), maxChars-1);
value[maxChars-1] = '\0';
}
}
catch (ParamValWrongType&){
return asynParamWrongType;
}
catch (ParamValNotDefined&){
return asynParamUndefined;
}
catch (ParamListInvalidIndex&) {
return asynParamBadIndex;
}
return status;
}
setString just calls paramVal->setString. getString calls strncpy, which will return a Nil terminated string if there is room. And then it sets the last element of the array to Nil, so the returned string is guaranteed to be Nil terminated, even if the buffer was not large enough.
The next class up is asynPortDriver. It implements setStringParam and getStringParam as follows:
asynStatus asynPortDriver::setStringParam(int list, int index, const char *value)
{
asynStatus status;
static const char *functionName = "setStringParam";
status = this->params[list]->setString(index, value);
if (status) reportSetParamErrors(status, index, list, functionName);
return(status);
}
asynStatus asynPortDriver::getStringParam(int list, int index, int maxChars, char *value)
{
asynStatus status;
static const char *functionName = "getStringParam";
status = this->params[list]->getString(index, maxChars, value);
if (status) reportGetParamErrors(status, index, list, functionName);
return(status);
}
So it just calls paramList::setString and paramList::getString, which were described above.
asynPortDriver also implements writeOctet and readOctet, which are the methods called by device support (via a thin C wrapper function)
asynStatus asynPortDriver::writeOctet(asynUser *pasynUser, const char *value,
size_t nChars, size_t *nActual)
{
int addr=0;
int function = pasynUser->reason;
asynStatus status = asynSuccess;
const char *functionName = "writeOctet";
status = getAddress(pasynUser, &addr); if (status != asynSuccess) return(status);
/* Set the parameter in the parameter library. */
status = (asynStatus)setStringParam(addr, function, (char *)value);
/* Do callbacks so higher layers see any changes */
status = (asynStatus)callParamCallbacks(addr, addr);
if (status)
epicsSnprintf(pasynUser->errorMessage, pasynUser->errorMessageSize,
"%s:%s: status=%d, function=%d, value=%s",
driverName, functionName, status, function, value);
else
asynPrint(pasynUser, ASYN_TRACEIO_DRIVER,
"%s:%s: function=%d, value=%s\n",
driverName, functionName, function, value);
*nActual = nChars;
return status;
}
asynStatus asynPortDriver::readOctet(asynUser *pasynUser,
char *value, size_t maxChars, size_t *nActual,
int *eomReason)
{
int function = pasynUser->reason;
int addr=0;
asynStatus status = asynSuccess;
const char *functionName = "readOctet";
status = getAddress(pasynUser, &addr); if (status != asynSuccess) return(status);
/* We just read the current value of the parameter from the parameter library.
* Those values are updated whenever anything could cause them to change */
status = (asynStatus)getStringParam(addr, function, maxChars, value);
if (status)
epicsSnprintf(pasynUser->errorMessage, pasynUser->errorMessageSize,
"%s:%s: status=%d, function=%d, value=%s",
driverName, functionName, status, function, value);
else
asynPrint(pasynUser, ASYN_TRACEIO_DRIVER,
"%s:%s: function=%d, value=%s\n",
driverName, functionName, function, value);
if (eomReason) *eomReason = ASYN_EOM_END;
*nActual = strlen(value);
return(status);
}
So writeOctet calls asynPortDriver::setStringParam. It returns nActual=nChars, i.e. the string length that was passed in. Note that writeOctet() is often overridden in derived classes if an action needs to be taken immediately when the string it written.
readOctet calls asynPortDriver::getStringParam. It returns nActual=strlen(value). readOctet is rarely overridden in derived classes, typically reading the last value from the parameter library is all that is required.
The final routine it paramList::octetCallback. This is the function that does callbacks to device support for strings with records that have SCAN=I/O Intr.
asynStatus paramList::octetCallback(int command, int addr)
{
ELLLIST *pclientList;
interruptNode *pnode;
asynStandardInterfaces *pInterfaces = this->pasynInterfaces;
int address;
char *value;
asynStatus status;
/* Pass octet interrupts */
value = getParameter(command)->getString();
getStatus(command, &status);
if (!pInterfaces->octetInterruptPvt) return(asynParamNotFound);
pasynManager->interruptStart(pInterfaces->octetInterruptPvt, &pclientList);
pnode = (interruptNode *)ellFirst(pclientList);
while (pnode) {
asynOctetInterrupt *pInterrupt = (asynOctetInterrupt *) pnode->drvPvt;
pasynManager->getAddr(pInterrupt->pasynUser, &address);
/* Set the status for the callback */
pInterrupt->pasynUser->auxStatus = status;
/* If this is not a multi-device then address is -1, change to 0 */
if (address == -1) address = 0;
if ((command == pInterrupt->pasynUser->reason) &&
(address == addr)) {
pInterrupt->callback(pInterrupt->userPvt,
pInterrupt->pasynUser,
value, strlen(value), ASYN_EOM_END);
}
pnode = (interruptNode *)ellNext(&pnode->node);
}
pasynManager->interruptEnd(pInterfaces->octetInterruptPvt);
return(asynSuccess);
}
It also passes the length as strlen(value).
Conclusions:
- The strings in the parameter library are guaranteed to be Nil terminated because they are only set with epicsStrDup.
- Strings passed back in readOctet and octetCallback are guaranteed to be Nil terminated.
- The lengths returned in readOctet and octetCallback are strlen(value), i.e. without the Nil.
- Should these be changed to strlen(value)+1? That will cause NORD to increase to be strlen(value)+1.
I'm willing to do this, but I tend to agree with Eric that it seems more logical for NORD to be the actual string length.
Mark
-----Original Message-----
From: Eric Norum [mailto:[email protected]]
Sent: Tuesday, November 13, 2012 1:20 PM
To: Andrew Johnson
Cc: Mark Rivers
Subject: Re: Long string support in CA clients and device support
The more that I think about this the more I'm unsure that increasing NORD to include a trailing '\0' is a good idea. NORD, as its name implies, is the number of characters read.
Existing message-based device support of which I'm aware (asyn IP, asyn serial to mention a couple) return just the number of characters actually read -- no extra '\0' added. So clients dealing with long string waveforms should already know that they have to deal with non-terminated strings. Having NORD incremented in some places and not in others just seems wrong to me.
--
Eric Norum
[email protected]
- Replies:
- Re: Long string support in CA clients and device support Eric Norum
- Re: Long string support in CA clients and device support Andrew Johnson
- References:
- Long string support in CA clients and device support Andrew Johnson
- RE: Long string support in CA clients and device support Mark Rivers
- Navigate by Date:
- Prev:
RE: Long string support in CA clients and device support Mark Rivers
- Next:
Re: Long string support in CA clients and device support Eric Norum
- Index:
1994
1995
1996
1997
1998
1999
2000
2001
2002
2003
2004
2005
2006
2007
2008
2009
2010
2011
<2012>
2013
2014
2015
2016
2017
2018
2019
2020
2021
2022
2023
2024
- Navigate by Thread:
- Prev:
RE: Long string support in CA clients and device support Mark Rivers
- Next:
Re: Long string support in CA clients and device support Eric Norum
- Index:
1994
1995
1996
1997
1998
1999
2000
2001
2002
2003
2004
2005
2006
2007
2008
2009
2010
2011
<2012>
2013
2014
2015
2016
2017
2018
2019
2020
2021
2022
2023
2024
|
·
Home
·
News
·
About
·
Base
·
Modules
·
Extensions
·
Distributions
·
Download
·