From 2358918f6dc4756a019cd5ff226c36819bf7c35b Mon Sep 17 00:00:00 2001
From: Martin Killenberg <martin.killenberg@desy.de>
Date: Tue, 7 Nov 2017 18:07:28 +0100
Subject: [PATCH] - changed update loop to use readAny - fixed last tests which
 were not using timeout based checks

---
 include/DoocsUpdater.h                |  7 +++----
 src/DoocsAdapter.cc                   |  4 +++-
 src/DoocsSpectrum.cc                  |  7 ++++---
 src/DoocsUpdater.cc                   | 29 ++++++++++++++++-----------
 src/eq_create.cc                      |  6 ++++++
 tests/src/serverTestReadWrite.cpp     |  6 +-----
 tests/src/serverTestRenameImport.cpp  | 16 +++++++--------
 tests/src/serverTestSpectrumArray.cpp | 19 +++++++-----------
 8 files changed, 49 insertions(+), 45 deletions(-)

diff --git a/include/DoocsUpdater.h b/include/DoocsUpdater.h
index b368140..971bf76 100644
--- a/include/DoocsUpdater.h
+++ b/include/DoocsUpdater.h
@@ -16,11 +16,10 @@ namespace ChimeraTK{
   class DoocsUpdater: public boost::noncopyable{
   public:
     ~DoocsUpdater();
-    void update(); // Update all variables once. This is the intermediate solution
-                   // before we have implemented the thread, and for testing
+    void update(); // Update all variables once. This is a convenience function for testing.
 
     void updateLoop(); // Endless loop with interruption point around the update function.
-                       // Intermediate solution until we have a working/testable version of readAny()
+
     void run();
     void stop();
 
@@ -29,7 +28,7 @@ namespace ChimeraTK{
     std::list< std::reference_wrapper< mtca4u::TransferElement > > _elementsToRead;
     boost::thread _syncThread;// we have to use boost thread to use interruption points
     //FIXME: make this an unordered map
-    std::map< mtca4u::TransferElement *, std::vector< std::function<void ()> > > _toDoocsUpdateMap;
+    std::map< mtca4u::TransferElement::ID, std::vector< std::function<void ()> > > _toDoocsUpdateMap;
 
   };
 }
diff --git a/src/DoocsAdapter.cc b/src/DoocsAdapter.cc
index cbe9a42..18a5f10 100644
--- a/src/DoocsAdapter.cc
+++ b/src/DoocsAdapter.cc
@@ -26,7 +26,9 @@ namespace ChimeraTK{
   void DoocsAdapter::waitUntilInitialised(){
     int i = 0;
     while(true){
-      if (isInitialised) {std::cout << "DoocsAdapter initialised afer " << i << std::endl; return;}
+      if (isInitialised){
+        return;
+      }
       // just sleep a bit. Use the "cheap" usleep, we don't care about precision here
       ++i;
       usleep(100);
diff --git a/src/DoocsSpectrum.cc b/src/DoocsSpectrum.cc
index e8d2130..5de0a22 100644
--- a/src/DoocsSpectrum.cc
+++ b/src/DoocsSpectrum.cc
@@ -55,6 +55,10 @@ namespace ChimeraTK {
   }
 
   void DoocsSpectrum::updateParameters(){
+    if (this->get_eqfct()){
+      this->get_eqfct()->lock();
+    }
+
     float start, increment;
     if (_startAccessor){
       start=_startAccessor->accessData(0);
@@ -67,9 +71,6 @@ namespace ChimeraTK {
       increment=this->spec_inc();
     }
     
-    if (this->get_eqfct()){
-      this->get_eqfct()->lock();
-    }
     spectrum_parameter( this->spec_time(), start, increment, this->spec_status() );
     if (this->get_eqfct()){
       this->get_eqfct()->unlock();
diff --git a/src/DoocsUpdater.cc b/src/DoocsUpdater.cc
index 78898fa..a816c66 100644
--- a/src/DoocsUpdater.cc
+++ b/src/DoocsUpdater.cc
@@ -3,18 +3,22 @@
 namespace ChimeraTK{
 
   void DoocsUpdater::addVariable( mtca4u::TransferElement & variable, std::function<void ()> updaterFunction){
-    _elementsToRead.push_back( std::reference_wrapper< mtca4u::TransferElement > (variable) );
-    _toDoocsUpdateMap[&variable].push_back(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 the list
+    // and compare the ID.
+    if ( _toDoocsUpdateMap.find(variable.getId()) == _toDoocsUpdateMap.end() ){
+      _elementsToRead.push_back( std::reference_wrapper< mtca4u::TransferElement > (variable) );
+    }
+    
+    _toDoocsUpdateMap[variable.getId()].push_back(updaterFunction);
   }
 
   void DoocsUpdater::update(){
-    for ( auto & mapElem : _toDoocsUpdateMap ){
-      ///@todo FIXME: This should be readNonBlocking(), or better readAny on the whole map
-      // Currently this is consistent behaviour in the location update, and needed
-      // for consistent testing.
-      if (mapElem.first->readLatest()){
-        for (auto & updaterFunc : mapElem.second){
-          updaterFunc();
+    for ( auto & transferElem : _elementsToRead ){
+      if (transferElem.get().readLatest()){
+        for (auto & updaterFunction : _toDoocsUpdateMap[transferElem.get().getId()]){
+          updaterFunction();
         }
       }
     }
@@ -22,9 +26,10 @@ namespace ChimeraTK{
 
   void DoocsUpdater::updateLoop(){
     while(true){
-      update();
-      // FIXME: This is brainstorming. Use testable sleep here
-      boost::this_thread::sleep_for(boost::chrono::milliseconds(10));
+      auto updatedElement = mtca4u::TransferElement::readAny(_elementsToRead);
+      for (auto & updaterFunction : _toDoocsUpdateMap[updatedElement]){
+        updaterFunction();
+      }
     }
   }
 
diff --git a/src/eq_create.cc b/src/eq_create.cc
index 16bde6b..598cf52 100644
--- a/src/eq_create.cc
+++ b/src/eq_create.cc
@@ -4,6 +4,7 @@
 #include "VariableMapper.h"
 #include "getAllVariableNames.h"
 #include <sys/stat.h>
+#include <mtca4u/ExperimentalFeatures.h>
 
 char const *object_name;
 static char const * XML_CONFIG_SUFFIX = "-DoocsVariableConfig.xml";
@@ -13,6 +14,11 @@ static ChimeraTK::DoocsAdapter doocsAdapter;
 /* eq_init_prolog is called before the locations are created, i.e. before the first call to eq_create.
  * We initialise the application, i.e. all process variables are created in this function. */
 void eq_init_prolog() {
+    //we are using experimental features. This is to remind me that experimental support
+    //should be turned off when readAny is not experiemtal any more.
+#warning Experimental features are enabled for using readAny! Turn off if not required any more.
+    ChimeraTK::ExperimentalFeatures::enable();
+  
     // set the DOOCS server name to the application name
     object_name = ChimeraTK::ApplicationBase::getInstance().getName().c_str();
     // Create static instances for all applications cores. They must not have overlapping
diff --git a/tests/src/serverTestReadWrite.cpp b/tests/src/serverTestReadWrite.cpp
index c881359..62266dd 100644
--- a/tests/src/serverTestReadWrite.cpp
+++ b/tests/src/serverTestReadWrite.cpp
@@ -32,10 +32,8 @@ void testReadWrite(){
   // halt the test application tread 
   referenceTestApplication.initialiseManualLoopControl();
   std::cout << "got the application main lock" << std::endl;
-
+  
   // just a few tests before we start
-  checkWithTimeout<int>( std::bind( &DoocsServerTestHelper::doocsGet<int>, "//INT/DATA_TYPE_CONSTANT"), -4);
-  CHECK_WITH_TIMEOUT( DoocsServerTestHelper::doocsGet<int>("//INT/DATA_TYPE_CONSTANT") == -4 );
   CHECK_WITH_TIMEOUT( DoocsServerTestHelper::doocsGet<int>("//INT/DATA_TYPE_CONSTANT") == -4 );
   CHECK_WITH_TIMEOUT( DoocsServerTestHelper::doocsGet<int>("//CHAR/DATA_TYPE_CONSTANT") == -1 );
   CHECK_WITH_TIMEOUT( DoocsServerTestHelper::doocsGet<int>("//INT/FROM_DEVICE_SCALAR") == 0 );
@@ -52,8 +50,6 @@ void testReadWrite(){
   referenceTestApplication.runMainLoopOnce();
   
   // now finally after the next update we should see the new data in doocs
-  checkWithTimeout<int>( std::bind( &DoocsServerTestHelper::doocsGet<int>, "//INT/FROM_DEVICE_SCALAR"), 42);
-  checkWithTimeout<int>( std::bind( &DoocsServerTestHelper::doocsGet<int>, "//CHAR/FROM_DEVICE_SCALAR"), 44);
   CHECK_WITH_TIMEOUT( DoocsServerTestHelper::doocsGet<int>("//INT/FROM_DEVICE_SCALAR") == 42 );
   CHECK_WITH_TIMEOUT( DoocsServerTestHelper::doocsGet<int>("//CHAR/FROM_DEVICE_SCALAR") == 44 );
 
diff --git a/tests/src/serverTestRenameImport.cpp b/tests/src/serverTestRenameImport.cpp
index 7b9409c..f62160b 100644
--- a/tests/src/serverTestRenameImport.cpp
+++ b/tests/src/serverTestRenameImport.cpp
@@ -33,7 +33,7 @@ void testVariableExistence(){
   checkDoocsProperty<D_intarray>("//MY_RENAMED_INTEGER_LOCATION/FROM_DEVICE_ARRAY" , true, false);
   checkDoocsProperty<D_intarray>("//MY_RENAMED_INTEGER_LOCATION/TO_DEVICE_ARRAY", true, true );
   checkDoocsProperty<D_int>("//MY_RENAMED_INTEGER_LOCATION/DATA_TYPE_CONSTANT", true, false);
-  BOOST_CHECK( DoocsServerTestHelper::doocsGet<int>("//MY_RENAMED_INTEGER_LOCATION/DATA_TYPE_CONSTANT") == -4);
+  CHECK_WITH_TIMEOUT( DoocsServerTestHelper::doocsGet<int>("//MY_RENAMED_INTEGER_LOCATION/DATA_TYPE_CONSTANT") == -4);
   checkDoocsProperty<D_int>("//MY_RENAMED_INTEGER_LOCATION/FROM_DEVICE_SCALAR", true, false);
   checkDoocsProperty<D_int>("//MY_RENAMED_INTEGER_LOCATION/TO_DEVICE_SCALAR", true ,true);
 
@@ -41,7 +41,7 @@ void testVariableExistence(){
   checkDoocsProperty<D_shortarray>("//SHORT/myStuff.FROM_DEVICE_ARRAY", true, false );
   checkDoocsProperty<D_shortarray>("//SHORT/myStuff.TO_DEVICE_ARRAY" );
   checkDoocsProperty<D_int>("//SHORT/myStuff.DATA_TYPE_CONSTANT", true, false);
-  BOOST_CHECK( DoocsServerTestHelper::doocsGet<int>("//SHORT/myStuff.DATA_TYPE_CONSTANT") == -2);
+  CHECK_WITH_TIMEOUT( DoocsServerTestHelper::doocsGet<int>("//SHORT/myStuff.DATA_TYPE_CONSTANT") == -2);
   checkDoocsProperty<D_int>("//SHORT/myStuff.FROM_DEVICE_SCALAR", true, false);
 
   checkDoocsProperty<D_int>("//CHERRY_PICKED/TO_DEVICE_SHORT");
@@ -50,7 +50,7 @@ void testVariableExistence(){
   checkDoocsProperty<D_doublearray>("//DOUBLE/FROM_DEVICE_ARRAY", true, false );
   checkDoocsProperty<D_doublearray>("//DOUBLE/TO_DEVICE_ARRAY" );
   checkDoocsProperty<D_double>("//DOUBLE/RENAMED_CONSTANT", false, false);
-  BOOST_CHECK( DoocsServerTestHelper::doocsGet<double>("//DOUBLE/RENAMED_CONSTANT") == 1./8.);
+  CHECK_WITH_TIMEOUT( DoocsServerTestHelper::doocsGet<double>("//DOUBLE/RENAMED_CONSTANT") == 1./8.);
   checkDoocsProperty<D_double>("//DOUBLE/FROM_DEVICE_SCALAR", true, false);
   checkDoocsProperty<D_double>("//DOUBLE/DOUBLE.TO_DEVICE_SCALAR", true, false);
   checkDoocsProperty<D_float>("//DOUBLE/I_AM_A_FLOAT_SCALAR");
@@ -60,14 +60,14 @@ void testVariableExistence(){
   checkDoocsProperty<D_floatarray>("//FLOAT/FROM_DEVICE_ARRAY", true, false );
   checkDoocsProperty<D_floatarray>("//FLOAT/TO_DEVICE_ARRAY" );
   checkDoocsProperty<D_float>("//FLOAT/DATA_TYPE_CONSTANT", true, false );
-  BOOST_CHECK( DoocsServerTestHelper::doocsGet<float>("//FLOAT/DATA_TYPE_CONSTANT") == 1./4.);
+  CHECK_WITH_TIMEOUT( DoocsServerTestHelper::doocsGet<float>("//FLOAT/DATA_TYPE_CONSTANT") == 1./4.);
   checkDoocsProperty<D_float>("//FLOAT/FROM_DEVICE_SCALAR", true, false);
 
   checkDoocsProperty<D_intarray>("//UINT/CONSTANT_ARRAY", true, false);
   checkDoocsProperty<D_intarray>("//UINT/FROM_DEVICE_ARRAY", true, false );
   checkDoocsProperty<D_intarray>("//UINT/TO_DEVICE_ARRAY" );
   checkDoocsProperty<D_int>("//UINT/DATA_TYPE_CONSTANT", true, false);
-  BOOST_CHECK( DoocsServerTestHelper::doocsGet<int>("//UINT/DATA_TYPE_CONSTANT") == 4);
+  CHECK_WITH_TIMEOUT( DoocsServerTestHelper::doocsGet<int>("//UINT/DATA_TYPE_CONSTANT") == 4);
   checkDoocsProperty<D_int>("//UINT/FROM_DEVICE_SCALAR", true, false);
   checkDoocsProperty<D_int>("//UINT/TO_DEVICE_SCALAR");
 
@@ -75,7 +75,7 @@ void testVariableExistence(){
   checkDoocsProperty<D_shortarray>("//USHORT/FROM_DEVICE_ARRAY", true, false);
   checkDoocsProperty<D_shortarray>("//USHORT/TO_DEVICE_ARRAY");
   checkDoocsProperty<D_int>("//USHORT/DATA_TYPE_CONSTANT", false, false );
-  BOOST_CHECK( DoocsServerTestHelper::doocsGet<int>("//USHORT/DATA_TYPE_CONSTANT") == 2);
+  CHECK_WITH_TIMEOUT( DoocsServerTestHelper::doocsGet<int>("//USHORT/DATA_TYPE_CONSTANT") == 2);
   checkDoocsProperty<D_int>("//USHORT/FROM_DEVICE_SCALAR", false, false);
   checkDoocsProperty<D_int>("//USHORT/TO_DEVICE_SCALAR", true, true);
 
@@ -83,7 +83,7 @@ void testVariableExistence(){
   checkDoocsProperty<D_bytearray>("//UCHAR/FROM_DEVICE_ARRAY", true, false );
   checkDoocsProperty<D_bytearray>("//UCHAR/TO_DEVICE_ARRAY" , true, false);
   checkDoocsProperty<D_int>("//UCHAR/DATA_TYPE_CONSTANT", true, false);
-  BOOST_CHECK( DoocsServerTestHelper::doocsGet<int>("//UCHAR/DATA_TYPE_CONSTANT") == 1);
+  CHECK_WITH_TIMEOUT( DoocsServerTestHelper::doocsGet<int>("//UCHAR/DATA_TYPE_CONSTANT") == 1);
   checkDoocsProperty<D_int>("//UCHAR/FROM_DEVICE_SCALAR", true, false);
   checkDoocsProperty<D_int>("//UCHAR/TO_DEVICE_SCALAR");
 
@@ -91,7 +91,7 @@ void testVariableExistence(){
   checkDoocsProperty<D_bytearray>("//CHAR/FROM_DEVICE_ARRAY", true, false );
   checkDoocsProperty<D_bytearray>("//CHAR/TO_DEVICE_ARRAY" );
   checkDoocsProperty<D_int>("//CHAR/DATA_TYPE_CONSTANT", true, false);
-  BOOST_CHECK( DoocsServerTestHelper::doocsGet<int>("//CHAR/DATA_TYPE_CONSTANT") == -1);
+  CHECK_WITH_TIMEOUT( DoocsServerTestHelper::doocsGet<int>("//CHAR/DATA_TYPE_CONSTANT") == -1);
   checkDoocsProperty<D_int>("//CHAR/FROM_DEVICE_SCALAR", true, false);
   checkDoocsProperty<D_int>("//CHAR/TO_DEVICE_SCALAR");
 }
diff --git a/tests/src/serverTestSpectrumArray.cpp b/tests/src/serverTestSpectrumArray.cpp
index 08d733d..dc7c517 100644
--- a/tests/src/serverTestSpectrumArray.cpp
+++ b/tests/src/serverTestSpectrumArray.cpp
@@ -29,14 +29,15 @@ using namespace ChimeraTK;
 
 // the array must have testStartValue+i at index i.
 template<class T>
-bool testArrayContent(std::string const & propertyName, T testStartValue){
+bool testArrayContent(std::string const & propertyName, T testStartValue, T delta){
   auto array = DoocsServerTestHelper::doocsGetArray<T>(propertyName);
   bool isOK = true;
   T currentTestValue=testStartValue;
   for (auto val : array){
-    if ( std::fabs(val-currentTestValue++) > 0.001 ){
+    if ( std::fabs(val-currentTestValue) > 0.001 ){
       isOK=false;
     }
+    currentTestValue+=delta;
   }
   return isOK;
 }
@@ -69,20 +70,14 @@ void testReadWrite(){
   // check the the control system side still sees 0 in all arrays. The application has not
   // reacted yet
   
-  auto notIntArray = DoocsServerTestHelper::doocsGetArray<float>("//INT/MY_RENAMED_INTARRAY");
-  for (auto val : notIntArray){
-    BOOST_CHECK( std::fabs(val) < 0.001 );
-  }
-  auto notFloatArray = DoocsServerTestHelper::doocsGetArray<float>("//DOUBLE/FROM_DEVICE_ARRAY");
-  for (auto val : notFloatArray){
-    BOOST_CHECK( std::fabs(val) < 0.001 );
-  }
+  CHECK_WITH_TIMEOUT( testArrayContent<float>("//INT/MY_RENAMED_INTARRAY", 0, 0) == true );
+  CHECK_WITH_TIMEOUT( testArrayContent<float>("//DOUBLE/FROM_DEVICE_ARRAY", 0, 0) == true );
   
   // run the application loop.
   referenceTestApplication.runMainLoopOnce();
 
-  CHECK_WITH_TIMEOUT( testArrayContent<float>("//INT/MY_RENAMED_INTARRAY", 140) == true );
-  CHECK_WITH_TIMEOUT( testArrayContent<float>("//DOUBLE/FROM_DEVICE_ARRAY", 240.3) == true );
+  CHECK_WITH_TIMEOUT( testArrayContent<float>("//INT/MY_RENAMED_INTARRAY", 140, 1) == true );
+  CHECK_WITH_TIMEOUT( testArrayContent<float>("//DOUBLE/FROM_DEVICE_ARRAY", 240.3, 1) == true );
   
 //  notIntArray = DoocsServerTestHelper::doocsGetArray<double>("//INT/MY_RENAMED_INTARRAY")
 //    ;
-- 
GitLab