From e0d5925af34fff3bf13d1355513eadd38ac623df Mon Sep 17 00:00:00 2001 From: Sinisa Veseli Date: Tue, 30 Apr 2024 13:04:08 -0500 Subject: [PATCH 1/6] add interface to check if master field was requested --- src/copy/pvCopy.cpp | 4 ++++ src/pv/pvStructureCopy.h | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/src/copy/pvCopy.cpp b/src/copy/pvCopy.cpp index 7694ecf..0916878 100644 --- a/src/copy/pvCopy.cpp +++ b/src/copy/pvCopy.cpp @@ -432,6 +432,7 @@ bool PVCopy::init(epics::pvData::PVStructurePtr const &pvRequest) PVStructurePtr pvMasterStructure = pvMaster; size_t len = pvRequest->getPVFields().size(); bool entireMaster = false; + requestHasMasterField = false; PVStructurePtr pvOptions; if(len==0) { entireMaster = true; @@ -441,6 +442,9 @@ bool PVCopy::init(epics::pvData::PVStructurePtr const &pvRequest) // then assume the top level PV structure is requested PVStructurePtr masterFieldPtr = pvMaster->getSubField("_"); PVStructurePtr requestFieldPtr = pvRequest->getSubField("_"); + if (requestFieldPtr) { + requestHasMasterField = true; + } if (!masterFieldPtr && requestFieldPtr) { entireMaster = true; pvOptions = requestFieldPtr->getSubField("_options"); diff --git a/src/pv/pvStructureCopy.h b/src/pv/pvStructureCopy.h index 3ae37a3..8bce170 100644 --- a/src/pv/pvStructureCopy.h +++ b/src/pv/pvStructureCopy.h @@ -167,6 +167,10 @@ class epicsShareClass PVCopy : * name is the subField name and value is the subField value. */ epics::pvData::PVStructurePtr getOptions(std::size_t fieldOffset); + /** + * Is master field requested? + */ + bool isMasterFieldRequested() const {return requestHasMasterField;} /** * For debugging. */ @@ -183,6 +187,7 @@ class epicsShareClass PVCopy : CopyNodePtr headNode; epics::pvData::PVStructurePtr cacheInitStructure; epics::pvData::BitSetPtr ignorechangeBitSet; + bool requestHasMasterField; void traverseMaster( CopyNodePtr const &node, From 9d94e95521db4c86f6a4e5ed6e32e7b41602cc2a Mon Sep 17 00:00:00 2001 From: Sinisa Veseli Date: Tue, 30 Apr 2024 13:08:06 -0500 Subject: [PATCH 2/6] make sure only one record field has pointer to the master field; fix code indents to 4 spaces --- src/database/pvRecord.cpp | 39 +++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/src/database/pvRecord.cpp b/src/database/pvRecord.cpp index 5357fc8..e3b7225 100644 --- a/src/database/pvRecord.cpp +++ b/src/database/pvRecord.cpp @@ -481,27 +481,30 @@ void PVRecordStructure::init() for(size_t i=0; igetField()->getType()==structure) { - PVStructurePtr xxx = static_pointer_cast(pvField); - PVRecordStructurePtr pvRecordStructure( + PVStructurePtr xxx = static_pointer_cast(pvField); + PVRecordStructurePtr pvRecordStructure( new PVRecordStructure(xxx,self,pvRecord)); - pvRecordFields->push_back(pvRecordStructure); - pvRecordStructure->init(); + pvRecordFields->push_back(pvRecordStructure); + pvRecordStructure->init(); } else { - PVRecordFieldPtr pvRecordField( + PVRecordFieldPtr pvRecordField( new PVRecordField(pvField,self,pvRecord)); - pvRecordFields->push_back(pvRecordField); - pvRecordField->init(); - // Master field listeners will be called before - // calling listeners for the first subfield - if (!masterFieldCallbackSet) { - masterFieldCallbackSet = true; - // Find master field - PVRecordStructurePtr p = pvRecordField->parent.lock(); - while (p) { - pvRecordField->master = p; - p = p->parent.lock(); - } - } + pvRecordFields->push_back(pvRecordField); + pvRecordField->init(); + // Master field listeners will be called before + // calling listeners for the first subfield + if (!masterFieldCallbackSet) { + masterFieldCallbackSet = true; + // Find master field + PVRecordStructurePtr p = pvRecordField->parent.lock(); + while (p) { + PVRecordStructurePtr p2 = p->parent.lock(); + if (!p2) { + pvRecordField->master = p; + } + p = p2; + } + } } } } From 94b48e489377d766e50ff1045c0822e5a2b40422 Mon Sep 17 00:00:00 2001 From: Sinisa Veseli Date: Tue, 30 Apr 2024 13:09:33 -0500 Subject: [PATCH 3/6] do not proceed with pvcopy in master field callback unless master field was requested --- src/pvAccess/monitorFactory.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/pvAccess/monitorFactory.cpp b/src/pvAccess/monitorFactory.cpp index c467c50..2e9e7e5 100644 --- a/src/pvAccess/monitorFactory.cpp +++ b/src/pvAccess/monitorFactory.cpp @@ -292,6 +292,12 @@ void MonitorLocal::dataPut(PVRecordFieldPtr const & pvRecordField) { cout << "MonitorLocal::dataPut(pvRecordField)" << endl; } + // If this record field is the master field, and the master field was not + // requested, we do not proceed with copy + bool isMasterField = pvRecordField->getPVRecord()->getPVStructure()->getFieldOffset()==0; + if (isMasterField && !pvCopy->isMasterFieldRequested()) { + return; + } if(state!=active) return; { Lock xx(mutex); From cd7d8735af69665f31b45671f2af0b2f6970d987 Mon Sep 17 00:00:00 2001 From: Sinisa Veseli Date: Tue, 30 Apr 2024 13:09:59 -0500 Subject: [PATCH 4/6] update master field tests --- test/src/testPVCopy.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/src/testPVCopy.cpp b/test/src/testPVCopy.cpp index a86493f..9719eb1 100644 --- a/test/src/testPVCopy.cpp +++ b/test/src/testPVCopy.cpp @@ -289,6 +289,7 @@ static void testMasterField(PVRecordPtr const& pvRecord) cout << "Master PV structure from copy" << endl << *pvMasterField << endl; cout << "Master PV structure from copy offset " << pvMasterField->getFieldOffset() << endl; } + testOk1(pvCopy->isMasterFieldRequested()); testOk1(pvMasterField->getNumberFields() == pvStructureRecord->getNumberFields()); testOk1(pvMasterField->getFieldOffset() == 0); PVStructurePtr pvStructureCopy = pvCopy->createPVStructure(); @@ -431,7 +432,7 @@ static void masterFieldTest() MAIN(testPVCopy) { - testPlan(70); + testPlan(71); scalarTest(); arrayTest(); powerSupplyTest(); From 7c78b15a5c797ffeea1544bc1f714af5339f4206 Mon Sep 17 00:00:00 2001 From: Sinisa Veseli Date: Mon, 6 May 2024 10:30:45 -0500 Subject: [PATCH 5/6] added tests for changed set --- test/src/Makefile | 5 + test/src/testChannelMonitor.cpp | 312 ++++++++++++++++++++++++++++++++ 2 files changed, 317 insertions(+) create mode 100644 test/src/testChannelMonitor.cpp diff --git a/test/src/Makefile b/test/src/Makefile index a7eb5e3..57ee58e 100644 --- a/test/src/Makefile +++ b/test/src/Makefile @@ -33,3 +33,8 @@ TESTPROD_HOST += testPVAServer testPVAServer_SRCS += testPVAServer.cpp testHarness_SRCS += testPVAServer.cpp TESTS += testPVAServer + +TESTPROD_HOST += testChannelMonitor +testChannelMonitor_SRCS += testChannelMonitor.cpp +testHarness_SRCS += testChannelMonitor.cpp +TESTS += testChannelMonitor diff --git a/test/src/testChannelMonitor.cpp b/test/src/testChannelMonitor.cpp new file mode 100644 index 0000000..3d63278 --- /dev/null +++ b/test/src/testChannelMonitor.cpp @@ -0,0 +1,312 @@ +/* testChannelMonitor.cpp */ +/** + * Copyright - See the COPYRIGHT that is included with this distribution. + * EPICS pvData is distributed subject to a Software License Agreement found + * in file LICENSE that is included with this distribution. + */ + +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace std; +using std::tr1::static_pointer_cast; +using namespace epics::pvData; +using namespace epics::pvAccess; +using namespace epics::pvDatabase; +namespace TR1 = std::tr1; + +static bool debug = true; + +PVStructurePtr createTestPvStructure() +{ + FieldCreatePtr fieldCreate = getFieldCreate(); + StandardFieldPtr standardField = getStandardField() +; + PVDataCreatePtr pvDataCreate = getPVDataCreate(); + + return pvDataCreate->createPVStructure( + fieldCreate->createFieldBuilder()-> + add("id",pvInt) -> + add("x",pvInt) -> + add("y",pvInt) -> + add("z",pvInt) -> + add("alarm",standardField->alarm()) -> + add("timeStamp",standardField->timeStamp()) -> + createStructure()); +} + +class ChannelMonitorRequesterImpl : public MonitorRequester +{ +public: + + ChannelMonitorRequesterImpl(const std::string& channelName_) + : channelName(channelName_) + , lastReceivedPvStructure(createTestPvStructure()) + , lastReceivedBitSet() + { + } + + virtual string getRequesterName() + { + return "ChannelMonitorRequesterImpl"; + } + + virtual void message(const std::string& message, MessageType messageType) + { + cout << "[" << getRequesterName() << "] message(" << message << ", " << getMessageTypeName(messageType) << ")" << endl; + } + + virtual void monitorConnect(const epics::pvData::Status& status, const Monitor::shared_pointer& /*monitor*/, const Structure::const_shared_pointer& /*structure*/) + { + if (status.isSuccess()) { + // show warning + if (!status.isOK()) { + cout << "[" << channelName << "] channel monitor create: " << status << endl; + } + connectionEvent.signal(); + } + else { + cout << "[" << channelName << "] failed to create channel monitor: " << status << endl; + } + } + + virtual void monitorEvent(const Monitor::shared_pointer& monitor) + { + MonitorElement::shared_pointer element; + while ((element = monitor->poll())) { + cout << "changed/overrun " << *element->changedBitSet << '/' << *element->overrunBitSet << endl; + if (!lastReceivedBitSet) { + lastReceivedBitSet = BitSet::create(element->changedBitSet->size()); + } + lastReceivedBitSet->clear(); + *lastReceivedBitSet |= *element->changedBitSet; + lastReceivedPvStructure->copyUnchecked(*element->pvStructurePtr); + monitor->release(element); + } + } + + virtual void unlisten(const Monitor::shared_pointer& /*monitor*/) + { + } + + bool waitUntilConnected(double timeOut) + { + return connectionEvent.wait(timeOut); + } + + PVStructurePtr getLastReceivedPvStructure() + { + return lastReceivedPvStructure; + } + + BitSetPtr getLastReceivedBitSet() + { + return lastReceivedBitSet; + } + +private: + Event event; + Event connectionEvent; + string channelName; + PVStructurePtr lastReceivedPvStructure; + BitSetPtr lastReceivedBitSet; +}; + +class ChannelRequesterImpl : public ChannelRequester +{ +private: + Event event; + +public: + + virtual string getRequesterName() + { + return "ChannelRequesterImpl"; + }; + + virtual void message(const std::string& message, MessageType messageType) + { + cout << "[" << getRequesterName() << "] message(" << message << ", " << getMessageTypeName(messageType) << ")" << endl; + } + + virtual void channelCreated(const Status& status, const Channel::shared_pointer& channel) + { + if (status.isSuccess()) { + // show warning + if (!status.isOK()) { + cout << "[" << channel->getChannelName() << "] channel create: " << status << endl; + } + } + else { + cout << "[" << channel->getChannelName() << "] failed to create a channel: " << status << endl; + } + } + + virtual void channelStateChange(const Channel::shared_pointer& /*channel*/, Channel::ConnectionState connectionState) + { + if (connectionState == Channel::CONNECTED) { + event.signal(); + } + else { + cout << Channel::ConnectionStateNames[connectionState] << endl; + exit(3); + } + } + + bool waitUntilConnected(double timeOut) + { + return event.wait(timeOut); + } +}; + + +static void test() +{ + PVDatabasePtr master = PVDatabase::getMaster(); + ChannelProviderLocalPtr channelProvider = getChannelProviderLocal(); + string recordName = "positions"; + PVStructurePtr pvStructure = createTestPvStructure(); + PVRecordPtr pvRecord = PVRecord::create(recordName,pvStructure); + master->addRecord(pvRecord); + pvRecord = master->findRecord(recordName); + { + pvRecord->lock(); + pvRecord->process(); + pvRecord->unlock(); + } + if(debug) {cout << "processed positions" << endl; } + ServerContext::shared_pointer ctx = startPVAServer("local",0,true,true); + testOk1(ctx.get() != 0); + + ClientFactory::start(); + ChannelProvider::shared_pointer provider = ChannelProviderRegistry::clients()->getProvider("pva"); + + cout << "creating channel: " << recordName << endl; + TR1::shared_ptr channelRequesterImpl(new ChannelRequesterImpl()); + + Channel::shared_pointer channel = provider->createChannel(recordName, channelRequesterImpl); + bool channelConnected = channelRequesterImpl->waitUntilConnected(1.0); + testOk1(channelConnected); + if (channelConnected) { + string remoteAddress = channel->getRemoteAddress(); + cout << "remote address: " << remoteAddress << endl; + } + + string request = ""; + PVStructure::shared_pointer pvRequest = CreateRequest::create()->createRequest(request); + TR1::shared_ptr cmRequesterImpl(new ChannelMonitorRequesterImpl(channel->getChannelName())); + Monitor::shared_pointer monitor = channel->createMonitor(cmRequesterImpl, pvRequest); + bool monitorConnected = cmRequesterImpl->waitUntilConnected(1.0); + testOk1(monitorConnected); + Status status = monitor->start(); + testOk1(status.isOK()); + epicsThreadSleep(1); + + // Set id, x + { + pvRecord->beginGroupPut(); + PVIntPtr id = pvStructure->getSubField("id"); + id->put(1); + PVIntPtr x = pvStructure->getSubField("x"); + x->put(1); + pvRecord->endGroupPut(); + } + epicsThreadSleep(1); + + // Changed set for (id,x): 0 unset, 1 set, 2 set, 3 unset, 4 unset + BitSetPtr changedSet = cmRequesterImpl->getLastReceivedBitSet(); + testOk1(!changedSet->get(0) && changedSet->get(1) && changedSet->get(2) && !changedSet->get(3) && !changedSet->get(4)); + testOk1(*pvStructure == *cmRequesterImpl->getLastReceivedPvStructure()); + + // Set id, y + { + pvRecord->beginGroupPut(); + PVIntPtr id = pvStructure->getSubField("id"); + id->put(2); + PVIntPtr y = pvStructure->getSubField("y"); + y->put(2); + pvRecord->endGroupPut(); + } + epicsThreadSleep(1); + + // Changed set for (id,y): 0 unset, 1 set, 2 unset, 3 set, 4 unset + changedSet = cmRequesterImpl->getLastReceivedBitSet(); + testOk1(!changedSet->get(0) && changedSet->get(1) && !changedSet->get(2) && changedSet->get(3) && !changedSet->get(4)); + testOk1(*pvStructure == *cmRequesterImpl->getLastReceivedPvStructure()); + + // Set id, z + { + pvRecord->beginGroupPut(); + PVIntPtr id = pvStructure->getSubField("id"); + id->put(3); + PVIntPtr z = pvStructure->getSubField("z"); + z->put(3); + pvRecord->endGroupPut(); + } + epicsThreadSleep(1); + + // Changed set for (id,z): 0 unset, 1 set, 2 unset, 3 unset, 4 set + changedSet = cmRequesterImpl->getLastReceivedBitSet(); + testOk1(!changedSet->get(0) && changedSet->get(1) && !changedSet->get(2) && !changedSet->get(3) && changedSet->get(4)); + testOk1(*pvStructure == *cmRequesterImpl->getLastReceivedPvStructure()); + + status = monitor->stop(); + testOk1(status.isOK()); + + // Test master field + request = "field(_)"; + pvRequest = CreateRequest::create()->createRequest(request); + cmRequesterImpl = TR1::shared_ptr(new ChannelMonitorRequesterImpl(channel->getChannelName())); + monitor = channel->createMonitor(cmRequesterImpl, pvRequest); + monitorConnected = cmRequesterImpl->waitUntilConnected(1.0); + testOk1(monitorConnected); + status = monitor->start(); + testOk1(status.isOK()); + epicsThreadSleep(1); + { + pvRecord->beginGroupPut(); + PVIntPtr id = pvStructure->getSubField("id"); + id->put(4); + PVIntPtr x = pvStructure->getSubField("x"); + x->put(4); + pvRecord->endGroupPut(); + } + epicsThreadSleep(1); + // Changed set with master field requested: 0 set + changedSet = cmRequesterImpl->getLastReceivedBitSet(); + testOk1(changedSet->get(0)); + testOk1(*pvStructure == *cmRequesterImpl->getLastReceivedPvStructure()); + + status = monitor->stop(); + testOk1(status.isOK()); + +} + +MAIN(testChannelMonitor) +{ + testPlan(16); + test(); + return 0; +} From b31d5079bf5721ee9eeb834a6509eb65878c1654 Mon Sep 17 00:00:00 2001 From: Sinisa Veseli Date: Wed, 3 Jul 2024 21:55:48 -0500 Subject: [PATCH 6/6] updated release notes --- documentation/RELEASE_NOTES.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/documentation/RELEASE_NOTES.md b/documentation/RELEASE_NOTES.md index e3cea46..eb9f5ae 100644 --- a/documentation/RELEASE_NOTES.md +++ b/documentation/RELEASE_NOTES.md @@ -2,6 +2,10 @@ This document summarizes the changes to the module between releases. +## Release 4.7.2 + +* Resolved issue with changed field set in the case where the top level (master) field ("_") is not requested by the client, but the master field callback causes all fields to be marked as updated, rather than only those fields that have actually been modified. + ## Release 4.7.1 (EPICS 7.0.8, Dec 2023) * Added data distributor plugin which can be used for distributing data between