From 817aeb5882b334f5e387ee69d8510ea6be80bb1b Mon Sep 17 00:00:00 2001
From: Martin Hierholzer <martin.hierholzer@desy.de>
Date: Tue, 1 Sep 2020 16:08:50 +0200
Subject: [PATCH] properly use multiple TypeChangingDecorators with different
 types if necessary (with test)

---
 include/DoocsUpdater.h                        |  4 +++-
 src/DoocsUpdater.cc                           | 23 +++++++++++++++----
 ...stRetypeProperties-DoocsVariableConfig.xml | 15 ++++++++----
 tests/src/serverTestRetypeProperties.cpp      | 21 +++++++++++++++++
 4 files changed, 53 insertions(+), 10 deletions(-)

diff --git a/include/DoocsUpdater.h b/include/DoocsUpdater.h
index 90b1f67..00ab268 100644
--- a/include/DoocsUpdater.h
+++ b/include/DoocsUpdater.h
@@ -35,7 +35,7 @@ namespace ChimeraTK {
     // specified. The lock is held while the updaterFunction is called, so it must
     // neither obtained nor freed within the updaterFunction.
     void addVariable(
-        const ChimeraTK::TransferElementAbstractor& variable, EqFct* eq_fct, std::function<void()> updaterFunction);
+        ChimeraTK::TransferElementAbstractor variable, EqFct* eq_fct, std::function<void()> updaterFunction);
 
    protected:
     std::list<ChimeraTK::TransferElementAbstractor> _elementsToRead;
@@ -43,6 +43,8 @@ namespace ChimeraTK {
     // FIXME: make this an unordered map
     std::map<ChimeraTK::TransferElementID, std::vector<std::function<void()>>> _toDoocsUpdateMap;
     std::map<ChimeraTK::TransferElementID, std::vector<EqFct*>> _toDoocsEqFctMap;
+    std::map<ChimeraTK::TransferElementID, std::set<boost::shared_ptr<ChimeraTK::TransferElement>>>
+        _toDoocsAdditionalTransferElementsMap;
   };
 } // namespace ChimeraTK
 
diff --git a/src/DoocsUpdater.cc b/src/DoocsUpdater.cc
index 47242dc..029f18e 100644
--- a/src/DoocsUpdater.cc
+++ b/src/DoocsUpdater.cc
@@ -5,10 +5,8 @@
 
 namespace ChimeraTK {
 
-  void DoocsUpdater::addVariable(const TransferElementAbstractor& variable,
-      EqFct* eq_fct,
-      std::function<void()>
-          updaterFunction) {
+  void DoocsUpdater::addVariable(
+      TransferElementAbstractor variable, EqFct* eq_fct, std::function<void()> updaterFunction) {
     // Don't add the transfer element twice into the list of elements to read.
     // To check if there is such an element we use the map with the lookup table
     // which has a search function, instead of manually looking at the elements in
@@ -16,6 +14,9 @@ namespace ChimeraTK {
     if(_toDoocsUpdateMap.find(variable.getId()) == _toDoocsUpdateMap.end()) {
       _elementsToRead.push_back(variable);
     }
+    else {
+      _toDoocsAdditionalTransferElementsMap[variable.getId()].insert(variable.getHighLevelImplElement());
+    }
 
     _toDoocsUpdateMap[variable.getId()].push_back(updaterFunction);
     _toDoocsEqFctMap[variable.getId()].push_back(eq_fct);
@@ -42,6 +43,14 @@ namespace ChimeraTK {
 
     ReadAnyGroup group(_elementsToRead.begin(), _elementsToRead.end());
     while(true) {
+      // Call preRead for all TEs on _toDoocsAdditionalTransferElementsMap. waitAny() is doing this for all
+      // elements in the ReadAnyGroup. Unnecessary calls to preRead are ignored anyway.
+      for(auto& pair : _toDoocsAdditionalTransferElementsMap) {
+        for(auto& elem : pair.second) {
+          elem->preRead(ChimeraTK::TransferType::read);
+        }
+      }
+
       // Wait until any variable got an update
       auto notification = group.waitAny();
       auto updatedElement = notification.getId();
@@ -51,6 +60,12 @@ namespace ChimeraTK {
       }
       // Complete the read transfer of the process variable
       notification.accept();
+
+      // Call postRead for all TEs on _toDoocsAdditionalTransferElementsMap for the updated ID
+      for(auto& elem : _toDoocsAdditionalTransferElementsMap[updatedElement]) {
+        elem->postRead(ChimeraTK::TransferType::read, true);
+      }
+
       // Call all updater functions
       for(auto& updaterFunction : _toDoocsUpdateMap[updatedElement]) updaterFunction();
       // Unlock all involved locations
diff --git a/tests/serverTestRetypeProperties-DoocsVariableConfig.xml b/tests/serverTestRetypeProperties-DoocsVariableConfig.xml
index 0d71a24..ed6c680 100644
--- a/tests/serverTestRetypeProperties-DoocsVariableConfig.xml
+++ b/tests/serverTestRetypeProperties-DoocsVariableConfig.xml
@@ -2,20 +2,25 @@
 <device_server xmlns="https://github.com/ChimeraTK/ControlSystemAdapter-DoocsAdapter">
     <location name="TO_BYTE">
         <property source="/DOUBLE/DATA_TYPE_CONSTANT" name="DOUBLE.CONSTANT" type="byte" />
-   </location>
+        <property source="/INT/DATA_TYPE_CONSTANT" name="INT.CONSTANT" type="byte" />
+    </location>
     <location name="TO_SHORT">
         <property source="/FLOAT/DATA_TYPE_CONSTANT" name="FLOAT.CONSTANT" type="short" />
-   </location>
+        <property source="/INT/DATA_TYPE_CONSTANT" name="INT.CONSTANT" type="short" />
+    </location>
     <location name="TO_INT">
         <property source="/SHORT/DATA_TYPE_CONSTANT" name="SHORT.CONSTANT" type="int" />
-   </location>
+        <property source="/INT/DATA_TYPE_CONSTANT" name="INT.CONSTANT"/>
+    </location>
     <location name="TO_LONG">
         <property source="/UCHAR/DATA_TYPE_CONSTANT" name="UCHAR.CONSTANT" type="long" />
-   </location>
+        <property source="/INT/DATA_TYPE_CONSTANT" name="INT.CONSTANT" type="long" />
+    </location>
     <location name="TO_FLOAT">
         <property source="/INT/DATA_TYPE_CONSTANT" name="INT.CONSTANT" type="float" />
-   </location>
+    </location>
     <location name="TO_DOUBLE">
         <property source="/USHORT/DATA_TYPE_CONSTANT" name="USHORT.CONSTANT" type="double" />
+        <property source="/INT/DATA_TYPE_CONSTANT" name="INT.CONSTANT" type="double" />
    </location>
 </device_server>
diff --git a/tests/src/serverTestRetypeProperties.cpp b/tests/src/serverTestRetypeProperties.cpp
index 64e4a2d..e302ed4 100644
--- a/tests/src/serverTestRetypeProperties.cpp
+++ b/tests/src/serverTestRetypeProperties.cpp
@@ -23,4 +23,25 @@ BOOST_AUTO_TEST_CASE(testVariableExistence) {
   checkDoocsProperty<D_longarray>("//TO_LONG/UCHAR.CONSTANT", true, false);
   checkDoocsProperty<D_float>("//TO_FLOAT/INT.CONSTANT", true, false);
   checkDoocsProperty<D_double>("//TO_DOUBLE/USHORT.CONSTANT", true, false);
+
+  checkDoocsProperty<D_int>("//TO_BYTE/INT.CONSTANT", true, false);
+  checkDoocsProperty<D_int>("//TO_SHORT/INT.CONSTANT", true, false);
+  checkDoocsProperty<D_int>("//TO_INT/INT.CONSTANT", true, false);
+  checkDoocsProperty<D_longarray>("//TO_LONG/INT.CONSTANT", true, false);
+  checkDoocsProperty<D_double>("//TO_DOUBLE/INT.CONSTANT", true, false);
+}
+
+/// Check the values of the variables
+BOOST_AUTO_TEST_CASE(testVariableValues) {
+  BOOST_CHECK_EQUAL(DoocsServerTestHelper::doocsGet<int>("//TO_BYTE/INT.CONSTANT"), -((signed)sizeof(int)));
+  BOOST_CHECK_EQUAL(DoocsServerTestHelper::doocsGet<int>("//TO_SHORT/INT.CONSTANT"), -((signed)sizeof(int)));
+  BOOST_CHECK_EQUAL(DoocsServerTestHelper::doocsGet<int>("//TO_INT/INT.CONSTANT"), -((signed)sizeof(int)));
+  BOOST_CHECK_CLOSE(DoocsServerTestHelper::doocsGet<float>("//TO_FLOAT/INT.CONSTANT"), -((signed)sizeof(int)), 0.0001);
+
+  BOOST_CHECK_EQUAL(DoocsServerTestHelper::doocsGet<int>("//TO_BYTE/DOUBLE.CONSTANT"), 0); // rounded 1./sizeof(double)
+  BOOST_CHECK_EQUAL(DoocsServerTestHelper::doocsGet<int>("//TO_SHORT/FLOAT.CONSTANT"), 0); // rounded 1./sizeof(float)
+  BOOST_CHECK_EQUAL(DoocsServerTestHelper::doocsGet<int>("//TO_INT/SHORT.CONSTANT"), -((signed)sizeof(short)));
+  BOOST_CHECK_EQUAL(DoocsServerTestHelper::doocsGet<int>("//TO_LONG/UCHAR.CONSTANT"), sizeof(unsigned char));
+  BOOST_CHECK_CLOSE(
+      DoocsServerTestHelper::doocsGet<double>("//TO_DOUBLE/USHORT.CONSTANT"), sizeof(unsigned short), 0.0001);
 }
-- 
GitLab