Skip to content

Commit

Permalink
ioc: improve long string detection
Browse files Browse the repository at this point in the history
Partially replicate the '$' handling logic of dbChannelCreate()
in the case where no '$' was provided.
  • Loading branch information
mdavidsaver committed Jun 13, 2023
1 parent 0a10087 commit b7f680b
Show file tree
Hide file tree
Showing 11 changed files with 133 additions and 33 deletions.
47 changes: 43 additions & 4 deletions ioc/channel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,16 @@

#include <string>

#include <dbAccess.h>

#include "channel.h"
#include "dbentry.h"
#include "utilpvt.h"

#ifndef PVLINK_STRINGSZ
# define PVLINK_STRINGSZ 1024
#endif

namespace pvxs {
namespace ioc {
/**
Expand All @@ -20,17 +27,49 @@ namespace ioc {
* @param name the db channel name
*/
Channel::Channel(const char* name)
:std::shared_ptr<dbChannel>(std::shared_ptr<dbChannel>(dbChannelCreate(name),
:chan(std::shared_ptr<dbChannel>(std::shared_ptr<dbChannel>(dbChannelCreate(name),
[](dbChannel* ch) {
if (ch) {
dbChannelDelete(ch);
}
}))
})))
{
if(!*this)
throw std::runtime_error(SB()<<"Invalid PV: "<<name);
if (dbChannelOpen(get()))
throw std::invalid_argument(SB() << "Failed dbChannelOpen(\"" << dbChannelName(get()) <<"\")");
{
DBEntry ent(dbChannelRecord(chan));

form = ent.info("Q:form", "Default");

/* ~duplicate '$' handling logic in dbChannelCreate()
* when no '$' is present.
*
* situation circa Base 7.0.7
* At this point dbChannelCreate() has initialized chan->addr.
* SPC_DBADDR fields have been mangled by record support code.
* filter have been parsed, but not opened.
*/

auto field_type(dbChannelFieldType(chan));
if(field_type==DBF_STRING && dbChannelElements(chan)==1
&& dbChannelFieldSize(chan)>MAX_STRING_SIZE+1)
{
// scalar string field with extra capacity
chan->addr.no_elements = chan->addr.field_size;
chan->addr.field_size = 1;
chan->addr.field_type = DBF_CHAR;
chan->addr.dbr_field_type = DBR_CHAR;
form = "String";

} else if(field_type >= DBF_INLINK && field_type <= DBF_FWDLINK) {
chan->addr.no_elements = PVLINK_STRINGSZ;
chan->addr.field_size = 1;
chan->addr.dbr_field_type = DBR_CHAR;
form = "String";
}
}
if (dbChannelOpen(chan.get()))
throw std::invalid_argument(SB() << "Failed dbChannelOpen(\"" << dbChannelName(chan) <<"\")");
}

} // pvxs
Expand Down
24 changes: 22 additions & 2 deletions ioc/channel.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,21 @@ namespace ioc {
* from string and dbChannel to make its use simpler. It can be used wherever a dbChannel is used.
* As a bonus when constructed with parameters it provides an already open dbChannel.
*/
class Channel : public std::shared_ptr<dbChannel> {
class Channel {
std::shared_ptr<dbChannel> chan;
/* Whenever chan!=nullptr, 'form' points to a string which will out-live
* the associated dbChannel*. eg. either statically allocated, or an
* info() tag. Value should be one of:
* "Default",
* "String",
* "Binary",
* "Decimal",
* "Hex",
* "Exponential",
* "Engineering",
* (although it could be anything...)
*/
const char *form = nullptr;
public:
Channel() = default;
Channel(const Channel&) = default;
Expand All @@ -40,8 +54,14 @@ class Channel : public std::shared_ptr<dbChannel> {
Channel& operator=(Channel&&) = default;

operator dbChannel*() const {
return get();
return chan.get();
}
dbChannel* operator->() const { return chan.get(); }

dbChannel* get() const { return chan.get(); }
const char* format() const { return form; }

explicit operator bool() const { return chan.operator bool(); }
};

} // pvxs
Expand Down
8 changes: 4 additions & 4 deletions ioc/groupconfigprocessor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ void GroupConfigProcessor::addTemplatesForDefinedFields(std::vector<Member>& gro
const GroupDefinition& groupDefinition) {
for (auto& fieldDefinition: groupDefinition.fields) {
auto& field = group[fieldDefinition.name];
dbChannel* pDbChannel = field.value;
auto& pDbChannel(field.value);
switch(fieldDefinition.info.type) {
case MappingInfo::Scalar:
addMembersForScalarType(groupMembers, field, pDbChannel);
Expand Down Expand Up @@ -868,7 +868,7 @@ const char* GroupConfigProcessor::infoField(DBEntry& dbEntry, const char* key, c
* @param pDbChannel the db channel to get information on what scalar type to create
*/
void GroupConfigProcessor::addMembersForScalarType(std::vector<Member>& groupMembers, const Field& groupField,
const dbChannel* pDbChannel) {
const Channel& pDbChannel) {
using namespace pvxs::members;
assert(!groupField.fieldName.empty()); // Must not call with empty field name

Expand All @@ -887,7 +887,7 @@ void GroupConfigProcessor::addMembersForScalarType(std::vector<Member>& groupMem
* @param pDbChannel the channel used to get the type of the leaf member
*/
void GroupConfigProcessor::addMembersForPlainType(std::vector<Member>& groupMembers, const Field& groupField,
const dbChannel* pDbChannel) {
const Channel &pDbChannel) {
assert(!groupField.fieldName.empty()); // Must not call with empty field name

// Get the type for the leaf
Expand Down Expand Up @@ -960,7 +960,7 @@ void GroupConfigProcessor::addMembersForMetaData(std::vector<Member>& groupMembe
* @param pDbChannel the channel to define the type definition for
* @return the TypeDef for the channel
*/
TypeDef GroupConfigProcessor::getTypeDefForChannel(const dbChannel* pDbChannel) {
TypeDef GroupConfigProcessor::getTypeDefForChannel(const Channel& pDbChannel) {
// Get the type for the leaf
auto leafCode(IOCSource::getChannelValueType(pDbChannel, true));
TypeDef leaf;
Expand Down
6 changes: 3 additions & 3 deletions ioc/groupconfigprocessor.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ class GroupConfigProcessor {
static void addMembersForAnyType(std::vector<Member>& groupMembers, const Field& groupField);
static void addMembersForMetaData(std::vector<Member>& groupMembers, const Field& groupField);
static void addMembersForPlainType(std::vector<Member>& groupMembers, const Field& groupField,
const dbChannel* pDbChannel);
const Channel& pDbChannel);
static void addMembersForScalarType(std::vector<Member>& groupMembers, const Field& groupField,
const dbChannel* pDbChannel);
const Channel &pDbChannel);
static void addMembersForStructureType(std::vector<Member>& groupMembers, const Field& groupField);
static void defineGroupTriggers(FieldDefinition& fieldDefinition, const GroupDefinition& groupDefinition,
const TriggerNames& triggerNames, const std::string& groupName);
Expand All @@ -84,7 +84,7 @@ class GroupConfigProcessor {
static bool yajlParseHelper(std::istream& jsonGroupDefinitionStream, yajl_handle handle);
static void initialiseDbLocker(Group& group);
static void initialiseTriggers(Group& group, const GroupDefinition& groupDefinition);
static TypeDef getTypeDefForChannel(const dbChannel* pDbChannel);
static TypeDef getTypeDefForChannel(const Channel &pDbChannel);
};

} // ioc
Expand Down
20 changes: 8 additions & 12 deletions ioc/iocsource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ bool IOCSource::enabled()
return e==1;
}

void IOCSource::initialize(Value& value, const MappingInfo &info, dbChannel *chan)
void IOCSource::initialize(Value& value, const MappingInfo &info, const Channel& chan)
{
if(info.type==MappingInfo::Scalar) {
if(auto fld = value["display.form.choices"]) {
Expand All @@ -88,14 +88,11 @@ void IOCSource::initialize(Value& value, const MappingInfo &info, dbChannel *cha
fld = choices;

if(dbIsValueField(dbChannelFldDes(chan))) { // only apply Q:form to VAL
DBEntry ent(dbChannelRecord(chan));

if(auto tag = ent.info("Q:form")) {
for(auto i : range(choices.size())) {
if(choices[i] == tag) {
value["display.form.index"] = i;
break;
}
auto tag(chan.format());
for(auto i : range(choices.size())) {
if(choices[i] == tag) {
value["display.form.index"] = i;
break;
}
}
}
Expand Down Expand Up @@ -652,7 +649,7 @@ void IOCSource::put(dbChannel* pDbChannel, const Value& node, const MappingInfo
* @param errOnLinks determines whether to throw an error on finding links, default no
* @return the TypeCode that the channel is configured for
*/
TypeCode IOCSource::getChannelValueType(const dbChannel* chan, const bool errOnLinks) {
TypeCode IOCSource::getChannelValueType(const Channel& chan, const bool errOnLinks) {
/* for links, could check dbChannelFieldType().
* for long strings, dbChannelCreate() '$' handling overwrites dbAddr::field_type
* (for some reason...)
Expand All @@ -666,10 +663,9 @@ TypeCode IOCSource::getChannelValueType(const dbChannel* chan, const bool errOnL
throw std::runtime_error("Link fields not allowed in this context");

bool isArray = dbChannelFinalElements(chan)!=1;
bool stringlike = (field_type >= DBF_INLINK && field_type <= DBF_OUTLINK) || field_type == DBF_STRING;

// string-like field being treated as single char[]. aka. long string
if(stringlike && final_field_type==DBR_CHAR && isArray)
if(final_field_type==DBR_CHAR && isArray && strcmp(chan.format(), "String")==0)
return TypeCode::String;

TypeCode valueType(fromDbrType(final_field_type));
Expand Down
4 changes: 2 additions & 2 deletions ioc/iocsource.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class IOCSource {
public:
static bool enabled();

static void initialize(Value& value, const MappingInfo &info, dbChannel *chan);
static void initialize(Value& value, const MappingInfo &info, const Channel &chan);

static void get(Value& valuePrototype,
const MappingInfo& info, const Value &anyType,
Expand All @@ -62,7 +62,7 @@ class IOCSource {
// Common Utils
//////////////////////////////
// Utility function to get the TypeCode that the given database channel is configured for
static TypeCode getChannelValueType(const dbChannel* pDbChannel, bool errOnLinks = false);
static TypeCode getChannelValueType(const Channel &pDbChannel, bool errOnLinks = false);
static void
setForceProcessingFlag(const Value& pvRequest, const std::shared_ptr<SecurityControlObject>& securityControlObject);
};
Expand Down
4 changes: 2 additions & 2 deletions ioc/singlesource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ void onSubscribe(const std::shared_ptr<SingleSourceSubscriptionCtx>& subscriptio
* @return a value prototype for the given channel
*/
Value getValuePrototype(const std::shared_ptr<SingleInfo>& sinfo) {
dbChannel* chan(sinfo->chan);
auto& chan(sinfo->chan);
short dbrType(dbChannelFinalFieldType(chan));
auto valueType(IOCSource::getChannelValueType(chan));

Expand Down Expand Up @@ -237,7 +237,7 @@ void doneCallback(struct processNotify* notify) {
void singleGet(const SingleInfo& info,
std::unique_ptr<server::ExecOp>& getOperation,
const Value& valuePrototype) {
dbChannel* pDbChannel(info.chan);
auto& pDbChannel(info.chan);
try {
auto returnValue = valuePrototype.cloneEmpty();
// TODO: MappingInfo::nsecMask
Expand Down
1 change: 1 addition & 0 deletions test/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ testqsingle_SRCS += testioc_registerRecordDeviceDriver.cpp
testqsingle_LIBS = pvxsIoc pvxs $(EPICS_BASE_IOC_LIBS)
TESTFILES += ../testqsingle.db
TESTFILES += ../testqsingle64.db
TESTFILES += ../testqsinglelsi.db
TESTFILES += ../testioc.acf
TESTS += testqsingle

Expand Down
43 changes: 39 additions & 4 deletions test/testqsingle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@
#include "testioc.h"
#include "utilpvt.h"

#if EPICS_VERSION_INT >= VERSION_INT(3, 15, 0, 2)
# define HAVE_lsi
#endif

extern "C" {
extern int testioc_registerRecordDeviceDriver(struct dbBase*);
}
Expand Down Expand Up @@ -251,14 +255,14 @@ void testGetScalar()
" } display\n"
"}\n");

testFldEq<std::string>(ctxt.get("test:this:is:a:really:really:long:record:name.NAME").exec()->wait(5.0),
"value", "test:this:is:a:really:really:long:recor");
testFldEq<std::string>(ctxt.get("test:this:is:a:really:really:long:record:name.NAME").exec()->wait(5000.0),
"value", "test:this:is:a:really:really:long:record:name");

testFldEq<std::string>(ctxt.get("test:this:is:a:really:really:long:record:name.NAME$").exec()->wait(5.0),
"value", "test:this:is:a:really:really:long:record:name");

testFldEq<std::string>(ctxt.get("test:src.INP").exec()->wait(5.0),
"value", "test:this:is:a:really:really:long:recor");
"value", "test:this:is:a:really:really:long:record:name NPP NMS");

testFldEq<std::string>(ctxt.get("test:src.INP$").exec()->wait(5.0),
"value", "test:this:is:a:really:really:long:record:name NPP NMS");
Expand All @@ -284,6 +288,7 @@ void testGetScalar()
"display.limitHigh int32_t = 0\n"
"display.description string = \"\"\n"
"display.units string = \"\"\n"
"display.form.index int32_t = 0\n"
"display.form.choices string[] = {7}[\"Default\", \"String\", \"Binary\", \"Decimal\", \"Hex\", \"Exponential\", \"Engineering\"]\n"
"control.limitLow int32_t = 0\n"
"control.limitHigh int32_t = 0\n"
Expand All @@ -293,6 +298,30 @@ void testGetScalar()
"valueAlarm.highAlarmLimit int32_t = 0\n");
}

void testLongString()
{
testDiag("%s", __func__);
TestClient ctxt;

ctxt.put("test:long:str:wf")
.set("value", "test:this:is:a:really:really:long:string:value")
.exec()->wait(5.0);

auto val(ctxt.get("test:long:str:wf").exec()->wait(5.0));
testEq(val["value"].as<std::string>(), "test:this:is:a:really:really:long:string:value");

#ifdef HAVE_lsi
ctxt.put("test:long:str:lsi.VAL")
.set("value", "test:this:is:a:really:really:long:string:value")
.exec()->wait(5.0);

val = ctxt.get("test:long:str:lsi.VAL").exec()->wait(5.0);
testEq(val["value"].as<std::string>(), "test:this:is:a:really:really:long:string:value");

#else
testSkip(1, "No lsiRecord");
#endif
}

void testGetArray()
{
Expand Down Expand Up @@ -373,6 +402,7 @@ void testGetArray()
"display.limitHigh int32_t = 0\n"
"display.description string = \"\"\n"
"display.units string = \"\"\n"
"display.form.index int32_t = 0\n"
"display.form.choices string[] = {7}[\"Default\", \"String\", \"Binary\", \"Decimal\", \"Hex\", \"Exponential\", \"Engineering\"]\n"
"control.limitLow int32_t = 0\n"
"control.limitHigh int32_t = 0\n");
Expand Down Expand Up @@ -503,6 +533,7 @@ void testGetPut64()
"display.limitHigh uint64_t = 0\n"
"display.description string = \"\"\n"
"display.units string = \"\"\n"
"display.form.index int32_t = 0\n"
"display.form.choices string[] = {7}[\"Default\", \"String\", \"Binary\", \"Decimal\", \"Hex\", \"Exponential\", \"Engineering\"]\n"
"control.limitLow uint64_t = 0\n"
"control.limitHigh uint64_t = 0\n");
Expand Down Expand Up @@ -835,7 +866,7 @@ void testMonitorAIFilt(TestClient& ctxt)

MAIN(testqsingle)
{
testPlan(82);
testPlan(84);
testSetup();
pvxs::logger_config_env();
{
Expand All @@ -845,11 +876,15 @@ MAIN(testqsingle)
testdbReadDatabase("testioc.dbd", nullptr, nullptr);
testOk1(!testioc_registerRecordDeviceDriver(pdbbase));
testdbReadDatabase("testqsingle.db", nullptr, nullptr);
#ifdef HAVE_lsi
testdbReadDatabase("testqsinglelsi.db", nullptr, nullptr);
#endif
#ifdef DBR_UINT64
testdbReadDatabase("testqsingle64.db", nullptr, nullptr);
#endif
ioc.init();
testGetScalar();
testLongString();
testGetArray();
testPut();
testGetPut64();
Expand Down
5 changes: 5 additions & 0 deletions test/testqsingle.db
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ record(ai, "test:this:is:a:really:really:long:record:name") {}
record(ai, "test:src") {
field(INP , "test:this:is:a:really:really:long:record:name NPP")
}
record(waveform, "test:long:str:wf") {
field(FTVL, "CHAR")
field(NELM, "128")
info(Q:form, "String")
}
record(bo, "test:bo") {
field(ZNAM, "Zero")
field(ONAM, "One")
Expand Down
4 changes: 4 additions & 0 deletions test/testqsinglelsi.db
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
record(lsi, "test:long:str:lsi")
{
field(SIZV, 128)
}

0 comments on commit b7f680b

Please sign in to comment.