wip #116: Change API of postRead and doPostRead

- These functions receive bool hasNewData as second argument - In readNonBlocking and readLatest, postRead is now called unconditionally
ChimeraTK · Apr 14, 2020 · 5ce12c2 · 5ce12c2
1 parent 66507b5
commit 5ce12c2
Show file tree

Hide file tree

Showing 21 changed files with 48 additions and 44 deletions.
diff --git a/device/include/ReadAnyGroup.h b/device/include/ReadAnyGroup.h
@@ -290,7 +290,8 @@ namespace ChimeraTK {
     catch(detail::DiscardValueException&) {
       return false;
     }
-    this->transferElement.getHighLevelImplElement()->postRead(TransferType::readAsync);
+    // FIXME: Added true just get compilation working. What the appropriate value here?
+    this->transferElement.getHighLevelImplElement()->postRead(TransferType::readAsync, true);
     return true;
   }
 

diff --git a/device/src/TransferFuture.cc b/device/src/TransferFuture.cc
@@ -15,7 +15,8 @@ namespace ChimeraTK {
     catch(detail::DiscardValueException&) {
       goto retry;
     }
-    _transferElement->postRead(TransferType::readAsync);
+
+    _transferElement->postRead(TransferType::readAsync, !_transferElement->hasSeenException);
   }
 
   bool TransferFuture::hasNewData() {

diff --git a/device/src/TransferGroup.cc b/device/src/TransferGroup.cc
@@ -26,10 +26,10 @@ namespace ChimeraTK {
       elem->readTransfer();
     }
     for(auto& elem : copyDecorators) {
-      elem->postRead(TransferType::read);
+      elem->postRead(TransferType::read, !elem->hasSeenException);
     }
     for(auto& elem : highLevelElements) {
-      elem->postRead(TransferType::read);
+      elem->postRead(TransferType::read, !elem->hasSeenException);
     }
   }
 

diff --git a/device_backends/LogicalNameMapping/include/LNMBackendBitAccessor.h b/device_backends/LogicalNameMapping/include/LNMBackendBitAccessor.h
@@ -103,9 +103,9 @@ namespace ChimeraTK {
       _accessor->preRead(type);
     }
 
-    void doPostRead(TransferType type) override {
+    void doPostRead(TransferType type, bool hasNewData) override {
       std::lock_guard<std::mutex> lock(*_mutex);
-      _accessor->postRead(type);
+      _accessor->postRead(type, hasNewData);
       if(_accessor->accessData(0) & _bitMask) {
         NDRegisterAccessor<UserType>::buffer_2D[0][0] = numericToUserType<UserType>(true);
       }

diff --git a/device_backends/LogicalNameMapping/include/LNMBackendChannelAccessor.h b/device_backends/LogicalNameMapping/include/LNMBackendChannelAccessor.h
@@ -95,8 +95,8 @@ namespace ChimeraTK {
       _accessor->preRead(type);
     }
 
-    void doPostRead(TransferType type) override {
-      _accessor->postRead(type);
+    void doPostRead(TransferType type, bool hasNewData) override {
+      _accessor->postRead(type, hasNewData);
       _accessor->accessChannel(_info->channel).swap(NDRegisterAccessor<UserType>::buffer_2D[0]);
     }
 

diff --git a/device_backends/LogicalNameMapping/include/LNMBackendVariableAccessor.h b/device_backends/LogicalNameMapping/include/LNMBackendVariableAccessor.h
@@ -68,7 +68,8 @@ namespace ChimeraTK {
         NDRegisterAccessor<UserType>::buffer_2D[0].resize(numberOfWords);
         // FIXME: Read seems to be the closest of what we do here;
         // revise TraferType if incorrect
-        doPostRead(TransferType::read);
+        // FIXME Check hasNewData argument
+        doPostRead(TransferType::read, true);
       }
       catch(...) {
         this->shutdown();
@@ -110,7 +111,7 @@ namespace ChimeraTK {
 
     bool isWriteable() const override { return _info->targetType != LNMBackendRegisterInfo::TargetType::CONSTANT; }
 
-    void doPostRead(TransferType) override {
+    void doPostRead(TransferType, bool /*hasNewData*/) override {
       std::lock_guard<std::mutex> lock(_info->valueTable_mutex);
       for(size_t i = 0; i < NDRegisterAccessor<UserType>::buffer_2D[0].size(); ++i) {
         callForType(_info->valueType, [&, this](auto arg) {

diff --git a/device_backends/LogicalNameMapping/src/LNMMathPlugin.cc b/device_backends/LogicalNameMapping/src/LNMMathPlugin.cc
@@ -42,7 +42,7 @@ namespace ChimeraTK { namespace LNMBackend {
 
     void doPreRead(TransferType type) override { _target->preRead(type); }
 
-    void doPostRead(TransferType type) override;
+    void doPostRead(TransferType type, bool hasNewData) override;
 
     void doPreWrite(TransferType type) override;
 
@@ -114,8 +114,8 @@ namespace ChimeraTK { namespace LNMBackend {
   /********************************************************************************************************************/
 
   template<typename UserType>
-  void MathPluginDecorator<UserType>::doPostRead(TransferType type) {
-    _target->postRead(type);
+  void MathPluginDecorator<UserType>::doPostRead(TransferType type, bool hasNewData) {
+    _target->postRead(type, hasNewData);
 
     // update data pointer
     valueView->rebase(_target->accessChannel(0).data());

diff --git a/device_backends/LogicalNameMapping/src/LNMMonostableTriggerPlugin.cc b/device_backends/LogicalNameMapping/src/LNMMonostableTriggerPlugin.cc
@@ -64,7 +64,7 @@ namespace ChimeraTK { namespace LNMBackend {
       throw ChimeraTK::logic_error("LogicalNameMappingBackend MonostableTriggerPluginPlugin: Reading is not allowed.");
     }
 
-    void doPostRead(TransferType) override {
+    void doPostRead(TransferType, bool hasNewData) override {
       throw ChimeraTK::logic_error("LogicalNameMappingBackend MonostableTriggerPluginPlugin: Reading is not allowed.");
     }
 

diff --git a/device_backends/LogicalNameMapping/src/LNMMultiplierPlugin.cc b/device_backends/LogicalNameMapping/src/LNMMultiplierPlugin.cc
@@ -42,7 +42,7 @@ namespace ChimeraTK { namespace LNMBackend {
 
     void doPreRead(TransferType type) override { _target->preRead(type); }
 
-    void doPostRead(TransferType type) override;
+    void doPostRead(TransferType type, bool hasNewData) override;
 
     void doPreWrite(TransferType type) override;
 
@@ -58,8 +58,8 @@ namespace ChimeraTK { namespace LNMBackend {
   };
 
   template<typename UserType>
-  void MultiplierPluginDecorator<UserType>::doPostRead(TransferType type) {
-    _target->postRead(type);
+  void MultiplierPluginDecorator<UserType>::doPostRead(TransferType type, bool hasNewData) {
+    _target->postRead(type, hasNewData);
     for(size_t i = 0; i < _target->getNumberOfChannels(); ++i) {
       for(size_t k = 0; k < _target->getNumberOfSamples(); ++k) {
         buffer_2D[i][k] = numericToUserType<UserType>(_target->accessData(i, k) * _factor);

diff --git a/device_backends/Subdevice/include/SubdeviceRegisterAccessor.h b/device_backends/Subdevice/include/SubdeviceRegisterAccessor.h
@@ -37,7 +37,7 @@ namespace ChimeraTK {
 
     void doPreRead(TransferType type) override;
 
-    void doPostRead(TransferType type) override;
+    void doPostRead(TransferType type, bool hasNewData) override;
 
     void doPreWrite(TransferType type) override;
 

diff --git a/device_backends/Subdevice/src/SubdeviceBackend.cc b/device_backends/Subdevice/src/SubdeviceBackend.cc
@@ -183,8 +183,8 @@ namespace ChimeraTK {
 
     void doPreRead(TransferType type) override { _target->preRead(type); }
 
-    void doPostRead(TransferType type) override {
-      _target->postRead(type);
+    void doPostRead(TransferType type, bool hasNewData) override {
+      _target->postRead(type, hasNewData);
       for(size_t i = 0; i < this->buffer_2D.size(); ++i) {
         _fixedPointConverter.template vectorToCooked<UserType>(
             _target->accessChannel(i).begin(), _target->accessChannel(i).end(), buffer_2D[i].begin());

diff --git a/device_backends/Subdevice/src/SubdeviceRegisterAccessor.cc b/device_backends/Subdevice/src/SubdeviceRegisterAccessor.cc
@@ -70,7 +70,7 @@ namespace ChimeraTK {
 
   /*********************************************************************************************************************/
 
-  void SubdeviceRegisterAccessor::doPostRead(TransferType) {
+  void SubdeviceRegisterAccessor::doPostRead(TransferType, bool /*hasNewData*/) {
     assert(NDRegisterAccessor<int32_t>::buffer_2D[0].size() == _buffer.size());
     NDRegisterAccessor<int32_t>::buffer_2D[0].swap(_buffer);
   }

diff --git a/device_backends/include/CopyRegisterDecorator.h b/device_backends/include/CopyRegisterDecorator.h
@@ -40,8 +40,8 @@ namespace ChimeraTK {
       throw ChimeraTK::logic_error("ChimeraTK::CopyRegisterDecorator: Accessor is not writeable.");
     }
 
-    void doPostRead(TransferType type) override {
-      _target->postRead(type);
+    void doPostRead(TransferType type, bool hasNewData) override {
+      _target->postRead(type, hasNewData);
       for(size_t i = 0; i < _target->getNumberOfChannels(); ++i) buffer_2D[i] = _target->accessChannel(i);
     }
 

diff --git a/device_backends/include/NDRegisterAccessorDecorator.h b/device_backends/include/NDRegisterAccessorDecorator.h
@@ -32,7 +32,7 @@ namespace ChimeraTK {
 
       void doPreRead(TransferType type) override = 0;
 
-      void doPostRead(TransferType type) override = 0;
+      void doPostRead(TransferType type, bool hasNewData) override = 0;
 
       void doPreWrite(TransferType type) override = 0;
 
@@ -60,8 +60,8 @@ namespace ChimeraTK {
 
       void doPreRead(TransferType type) override { _target->preRead(type); }
 
-      void doPostRead(TransferType type) override {
-        _target->postRead(type);
+      void doPostRead(TransferType type, bool hasNewData) override {
+        _target->postRead(type, hasNewData);
         for(size_t i = 0; i < _target->getNumberOfChannels(); ++i) buffer_2D[i].swap(_target->accessChannel(i));
       }
 

diff --git a/device_backends/include/NumericAddressedBackendMuxedRegisterAccessor.h b/device_backends/include/NumericAddressedBackendMuxedRegisterAccessor.h
@@ -70,7 +70,7 @@ namespace ChimeraTK {
       return true;
     }
 
-    void doPostRead(TransferType type) override;
+    void doPostRead(TransferType type, bool hasNewData) override;
 
     bool doWriteTransfer(ChimeraTK::VersionNumber versionNumber = {}) override;
 
@@ -282,12 +282,12 @@ namespace ChimeraTK {
   /********************************************************************************************************************/
 
   template<class UserType>
-  void NumericAddressedBackendMuxedRegisterAccessor<UserType>::doPostRead(TransferType type) {
+  void NumericAddressedBackendMuxedRegisterAccessor<UserType>::doPostRead(TransferType type, bool hasNewData) {
     for(size_t i = 0; i < _converters.size(); ++i) {
       _converters[i].template vectorToCooked<UserType>(_startIterators[i], _endIterators[i], buffer_2D[i].begin());
     }
     currentVersion = {};
-    SyncNDRegisterAccessor<UserType>::doPostRead(type);
+    SyncNDRegisterAccessor<UserType>::doPostRead(type, hasNewData);
   }
 
   /********************************************************************************************************************/

diff --git a/device_backends/include/NumericAddressedBackendRegisterAccessor.h b/device_backends/include/NumericAddressedBackendRegisterAccessor.h
@@ -235,9 +235,9 @@ namespace ChimeraTK {
       return false;
     }
 
-    void doPostRead(TransferType type) override {
+    void doPostRead(TransferType type, bool hasNewData) override {
       _prePostActionsImplementor.doPostRead();
-      SyncNDRegisterAccessor<UserType>::doPostRead(type);
+      SyncNDRegisterAccessor<UserType>::doPostRead(type, hasNewData);
     }
 
     void doPreWrite(TransferType) override { _prePostActionsImplementor.doPreWrite(); }

diff --git a/device_backends/include/NumericAddressedLowLevelTransferElement.h b/device_backends/include/NumericAddressedLowLevelTransferElement.h
@@ -59,7 +59,7 @@ namespace ChimeraTK {
       return true;
     }
 
-    void doPostRead(TransferType) override { currentVersion = {}; }
+    void doPostRead(TransferType, bool hasNewData) override { currentVersion = {}; }
 
     TransferFuture doReadTransferAsync() override { // LCOV_EXCL_LINE
       // This function is not needed and will never be called. If readAsync() is

diff --git a/device_backends/include/TransferElement.h b/device_backends/include/TransferElement.h
@@ -120,7 +120,8 @@ namespace ChimeraTK {
       this->readTransactionInProgress = false;
       preRead(TransferType::read);
       readTransfer();
-      postRead(TransferType::read);
+
+      postRead(TransferType::read, !hasSeenException);
     }
 
     /** 
@@ -146,9 +147,9 @@ namespace ChimeraTK {
       }
       this->readTransactionInProgress = false;
       preRead(TransferType::readNonBlocking);
-      bool ret = readTransferNonBlocking();
-      if(ret) postRead(TransferType::readNonBlocking);
-      return ret;
+      bool hasNewData = readTransferNonBlocking();
+      postRead(TransferType::readNonBlocking, hasNewData);
+      return hasNewData;
     }
 
     /** Read the latest value, discarding any other update since the last read if
@@ -169,7 +170,7 @@ namespace ChimeraTK {
       this->readTransactionInProgress = false;
       preRead(TransferType::readLatest);
       bool ret2 = readTransferLatest();
-      if(ret2) postRead(TransferType::readLatest);
+      postRead(TransferType::readLatest, ret2);
       return ret || ret2;
     }
 
@@ -432,11 +433,11 @@ namespace ChimeraTK {
      *  a read was executed directly on the underlying accessor. This function must
      *  be implemented to extract the read data from the underlying accessor and
      *  expose it to the user. */
-    void postRead(TransferType type) {
+    void postRead(TransferType type, bool hasNewData) {
       if(!readTransactionInProgress) return;
       readTransactionInProgress = false;
       hasActiveFuture = false;
-      doPostRead(type);
+      doPostRead(type, hasNewData);
       // Note: doPostRead can throw an exception, but in that case hasSeenException must be false (we can only have one
       // exception at a time). In case other code is added here later which needs to be executed after doPostRead()
       // always, a try-catch block may be necessary.
@@ -453,7 +454,7 @@ namespace ChimeraTK {
      *  must be acceptable to call this function while the device is closed or not functional (see isFunctional()) and
      *  no exception is thrown. */
    protected:
-    virtual void doPostRead(TransferType) {}
+    virtual void doPostRead(TransferType, bool) {}
 
    public:
     /** Function called by the TransferFuture before entering a potentially

diff --git a/tests/executables_src/testAsyncRead.cpp b/tests/executables_src/testAsyncRead.cpp
@@ -80,7 +80,7 @@ class AsyncTestDummy : public DeviceBackendImpl {
 
     void doPreRead(TransferType) override {}
 
-    void doPostRead(TransferType) override {
+    void doPostRead(TransferType, bool /*hasNewData*/) override {
       buffer_2D[0][0] = _backend->registers.at(getName());
       currentVersion = {};
     }

diff --git a/tests/executables_src/testDataConsistencyGroup.cpp b/tests/executables_src/testDataConsistencyGroup.cpp
@@ -37,7 +37,7 @@ class Accessor : public NDRegisterAccessor<UserType> {
 
   void doPreRead(TransferType) override {}
 
-  void doPostRead(TransferType) override {}
+  void doPostRead(TransferType, bool /*hasNewData*/) override {}
 
   AccessModeFlags getAccessModeFlags() const override { return {AccessMode::wait_for_new_data}; }
   bool isReadOnly() const override { return false; }

diff --git a/tests/executables_src/testTransferGroup.cpp b/tests/executables_src/testTransferGroup.cpp
@@ -497,9 +497,9 @@ struct CountingDecorator : NDRegisterAccessorDecorator<T> {
     NDRegisterAccessorDecorator<T>::doPreRead(type);
   }
 
-  void doPostRead(TransferType type) override {
+  void doPostRead(TransferType type, bool hasNewData) override {
     nPostRead++;
-    NDRegisterAccessorDecorator<T>::doPostRead(type);
+    NDRegisterAccessorDecorator<T>::doPostRead(type, hasNewData);
   }
 
   void doPreWrite(TransferType type) override {