Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ioc: improve long string detection #47

Merged
merged 2 commits into from
Jun 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 48 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,54 @@ 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()) <<"\")");
{
if( dbIsValueField(dbChannelFldDes(chan))) {
// info() hint only applies to VAL
DBEntry ent(dbChannelRecord(chan));
form = ent.info("Q:form", "Default");

} else {
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
4 changes: 2 additions & 2 deletions ioc/dbentry.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ class DBEntry {
return &ent;
}

const char* info(const char *key) {
const char *ret = nullptr;
const char* info(const char *key, const char* defval=nullptr) {
const char *ret = defval;
if(!dbFindInfo(&ent, key)) {
ret = ent.pinfonode->string;
}
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
Loading