From 77619b2332e503add379c9fabed69ce446a6b325 Mon Sep 17 00:00:00 2001
From: Martin Killenberg <martin.killenberg@desy.de>
Date: Thu, 14 Sep 2017 09:41:08 +0200
Subject: [PATCH] split createDoocsProperty into createDoocsScalar and
 createDoocsSpectrum for further refactoring

---
 include/DoocsPVFactory.h         |  10 +-
 src/DoocsPVFactory.cc            | 152 ++++++++++++++++++++++---------
 tests/src/testDoocsPVFactory.cpp |   8 +-
 3 files changed, 121 insertions(+), 49 deletions(-)

diff --git a/include/DoocsPVFactory.h b/include/DoocsPVFactory.h
index 1b42422..4dea456 100644
--- a/include/DoocsPVFactory.h
+++ b/include/DoocsPVFactory.h
@@ -29,18 +29,22 @@ namespace ChimeraTK {
   protected:
     EqFct * _eqFct; //< The EqFct which is holding the factory. Needed in the constructor of the doocs properties.
     boost::shared_ptr<ControlSystemSynchronizationUtility> _syncUtility; //< The syncUtility is needed to register listeners
-    boost::shared_ptr<ControlSystemPVManager> _controlSystemPVManager; //< The pv manager, needed to get the instances @todo Do we need it in a proper design?
+    boost::shared_ptr<ControlSystemPVManager> _controlSystemPVManager; //< The pv manager, needed to get the instances
 
     // create the DOOCS property. Note: DOOCS_T is only used for scalar properties, not for arrays!
     template<class T, class DOOCS_T>
-    typename boost::shared_ptr<D_fct> createDoocsProperty(typename ProcessVariable::SharedPtr & processVariable);
+    typename boost::shared_ptr<D_fct> createDoocsScalar(typename ProcessVariable::SharedPtr & processVariable);
+    template<class T>
+    typename boost::shared_ptr<D_fct> createDoocsSpectrum(typename ProcessVariable::SharedPtr & processVariable);
+
+    boost::shared_ptr<D_fct> autoCreate( std::shared_ptr<PropertyDescription> const & propertyDescription);
  
   };
 
 
   // specialisation for strings
   template<>
-  typename boost::shared_ptr<D_fct> DoocsPVFactory::createDoocsProperty<std::string, D_string>(typename ProcessVariable::SharedPtr & processVariable);
+  typename boost::shared_ptr<D_fct> DoocsPVFactory::createDoocsScalar<std::string, D_string>(typename ProcessVariable::SharedPtr & processVariable);
   
   
 }//namespace ChimeraTK
diff --git a/src/DoocsPVFactory.cc b/src/DoocsPVFactory.cc
index 146ed2c..3e97426 100644
--- a/src/DoocsPVFactory.cc
+++ b/src/DoocsPVFactory.cc
@@ -8,6 +8,7 @@
 #include "DoocsPVFactory.h"
 #include "splitStringAtFirstSlash.h"
 #include "VariableMapper.h"
+#include <ChimeraTK/ControlSystemAdapter/TypeChangingDecorator.h>
 
 namespace ChimeraTK {
 
@@ -18,7 +19,7 @@ namespace ChimeraTK {
   }
 
   template<class T, class DOOCS_T>
-  typename boost::shared_ptr<D_fct> DoocsPVFactory::createDoocsProperty(typename ProcessVariable::SharedPtr & processVariable) {
+  typename boost::shared_ptr<D_fct> DoocsPVFactory::createDoocsScalar(typename ProcessVariable::SharedPtr & processVariable) {
     // the DoocsProcessArray needs the real ProcessScalar type, not just ProcessVariable
     typename boost::shared_ptr< mtca4u::NDRegisterAccessor<T> > processArray
       = boost::dynamic_pointer_cast< mtca4u::NDRegisterAccessor<T> >(processVariable);
@@ -33,42 +34,37 @@ namespace ChimeraTK {
     
     assert(processArray->getNumberOfChannels() == 1);
     boost::shared_ptr<D_fct> doocsPV;
-    if(processArray->getNumberOfSamples() > 1 ) {
-      doocsPV.reset( new DoocsSpectrum<T>(_eqFct, propertyDescription->name, processArray, *_syncUtility) );
+    // Histories seem to be supported by DOOCS only for property names shorter than 64 characters, so disable history for longer names.
+    // The DOOCS property name is the variable name without the location name and the separating slash between location and property name.
+    if(propertyDescription->name.length() > 64) {
+      std::cerr << "WARNING: Disabling history for " << processArray->getName() << ". Name is too long." << std::endl;
+      doocsPV.reset( new DoocsProcessScalar<T, DOOCS_T>(propertyDescription->name.c_str(), _eqFct, processArray, *_syncUtility) );
     }
-    else { // scalar
-      // Histories seem to be supported by DOOCS only for property names shorter than 64 characters, so disable history for longer names.
-      // The DOOCS property name is the variable name without the location name and the separating slash between location and property name.
-      if(propertyDescription->name.length() > 64) {
-        std::cerr << "WARNING: Disabling history for " << processArray->getName() << ". Name is too long." << std::endl;
+    else{
+      if (autoPropertyDescription && autoPropertyDescription->hasHistory){
+        // version with history: EqFtc first
+        doocsPV.reset( new DoocsProcessScalar<T, DOOCS_T>(_eqFct, propertyDescription->name.c_str(), processArray, *_syncUtility) );
+      }else{
+        // version without history: name first
         doocsPV.reset( new DoocsProcessScalar<T, DOOCS_T>(propertyDescription->name.c_str(), _eqFct, processArray, *_syncUtility) );
       }
-      else{
-        if (autoPropertyDescription && autoPropertyDescription->hasHistory){
-          // version with history: EqFtc first
-          doocsPV.reset( new DoocsProcessScalar<T, DOOCS_T>(_eqFct, propertyDescription->name.c_str(), processArray, *_syncUtility) );
-        }else{
-          // version without history: name first
-          doocsPV.reset( new DoocsProcessScalar<T, DOOCS_T>(propertyDescription->name.c_str(), _eqFct, processArray, *_syncUtility) );
-        }
-      }// if name too long
-    }// if scalar
+    }// if name too long
 
     // FIXME: Make it scalar and put it into one if query
     if (autoPropertyDescription && !(autoPropertyDescription->isWriteable)){
       doocsPV->set_ro_access();
     }
-
+    
     // set read only mode if configures in the xml file or for output variables
     if (!processArray->isWriteable()){// || !propertyDescription.isWriteable){
       doocsPV->set_ro_access();
     }
-
+    
     return doocsPV;
   }
 
   template<>
-  boost::shared_ptr<D_fct> DoocsPVFactory::createDoocsProperty<std::string, D_string>(
+  boost::shared_ptr<D_fct> DoocsPVFactory::createDoocsScalar<std::string, D_string>(
             boost::shared_ptr<ProcessVariable> & processVariable) {
     // the DoocsProcessArray needs the real ProcessScalar type, not just ProcessVariable
     boost::shared_ptr< mtca4u::NDRegisterAccessor<std::string> > processArray
@@ -85,32 +81,104 @@ namespace ChimeraTK {
     return boost::shared_ptr<D_fct>( new DoocsProcessScalar<std::string, D_string>(_eqFct, propertyDescription->name.c_str(), processArray, *_syncUtility) );
   }
 
- boost::shared_ptr<D_fct> DoocsPVFactory::create( ProcessVariable::SharedPtr & processVariable ){
+  template<class T>
+  typename boost::shared_ptr<D_fct> DoocsPVFactory::createDoocsSpectrum(typename ProcessVariable::SharedPtr & processVariable) {
+    // the DoocsProcessArray needs the real type, not just ProcessVariable
+    typename boost::shared_ptr< mtca4u::NDRegisterAccessor<T> > processArray
+      = boost::dynamic_pointer_cast< mtca4u::NDRegisterAccessor<T> >(processVariable);
+    if (!processArray){
+      throw std::invalid_argument(std::string("DoocsPVFactory::createDoocsArray : processArray is of the wrong type ")
+				  + processVariable->getValueType().name());
+    }
+
+    auto propertyDescription = VariableMapper::getInstance().getAllProperties().at(processVariable->getName());
+    // FIXME: This has to go for scalars
+    auto autoPropertyDescription = std::dynamic_pointer_cast<AutoPropertyDescription>(propertyDescription);
+    
+    assert(processArray->getNumberOfChannels() == 1);
+    boost::shared_ptr<D_fct> doocsPV( new DoocsSpectrum<T>(_eqFct, propertyDescription->name, processArray, *_syncUtility) );
+
+    // FIXME: Make it scalar and put it into one if query
+    if (autoPropertyDescription && !(autoPropertyDescription->isWriteable)){
+      doocsPV->set_ro_access();
+    }
+
+    // set read only mode if configures in the xml file or for output variables
+    if (!processArray->isWriteable()){// || !propertyDescription.isWriteable){
+      doocsPV->set_ro_access();
+    }
+
+    return doocsPV;
+  }
+
+  boost::shared_ptr<D_fct> DoocsPVFactory::autoCreate( std::shared_ptr<PropertyDescription> const & propertyDescription ){
+    // do auto creation
+    auto pvName = std::static_pointer_cast<AutoPropertyDescription>(propertyDescription)->source;
+    auto pv = _controlSystemPVManager->getProcessVariable(pvName);
+    // fixme: merge the create stuff in here and remove the create function.
+    return create( pv );
+  }
+  
+  boost::shared_ptr<D_fct> DoocsPVFactory::create( ProcessVariable::SharedPtr & processVariable ){
     std::type_info const & valueType = processVariable->getValueType();
+    // We use an int accessor decorator just to have some type.
+    // All we need to do here is to determine the number of samples. No need to write an if/then/else
+    // on the type, or a boot::fustion::for_each, which is in the decorator factory.
+    // Unfortynately we need different impl_type -> doocs_type decision for scalars and arrays,
+    // so we have to know this here.
+    auto intDecoratedPV = ChimeraTK::getDecorator<int>( *processVariable );
+    auto nSamples = intDecoratedPV->getNumberOfSamples();
 
+    /*  TODO: 
+        - create functions "createDoocsArray" and "createDoocsSpectrum"
+        - first use spectrum here for 1D, then switch to array (tests need to be adapted)
+        - create spectrum, array and d_int/float/double upon request from 1d (scalar for D_array and 1D)
+    */
     // Unfortunately we need a big if/else block to hard-code the template
     // parameter. The value type in only known at run time,
     // but the template parameter has to be known at compile time.
-    if (valueType == typeid(int8_t)) {
-      return createDoocsProperty<int8_t, D_int>(processVariable);
-    } else if (valueType == typeid(uint8_t)) {
-      return createDoocsProperty<uint8_t, D_int>(processVariable);
-    } else if (valueType == typeid(int16_t)) {
-      return createDoocsProperty<int16_t, D_int>(processVariable);
-    } else if (valueType == typeid(uint16_t)) {
-      return createDoocsProperty<uint16_t, D_int>(processVariable);
-    } else if (valueType == typeid(int32_t)) {
-      return createDoocsProperty<int32_t, D_int>(processVariable);
-    } else if (valueType == typeid(uint32_t)) {
-      return createDoocsProperty<uint32_t, D_int>(processVariable);
-    } else if (valueType == typeid(float)) {
-      return createDoocsProperty<float, D_float>(processVariable);
-    } else if (valueType == typeid(double)) {
-      return createDoocsProperty<double, D_double>(processVariable);
-    } else if (valueType == typeid(std::string)) {
-      return createDoocsProperty<std::string, D_string>(processVariable);
-    } else {
-      throw std::invalid_argument("unsupported value type");
+    if (nSamples == 1){
+      if (valueType == typeid(int8_t)) {
+        return createDoocsScalar<int8_t, D_int>(processVariable);
+      } else if (valueType == typeid(uint8_t)) {
+        return createDoocsScalar<uint8_t, D_int>(processVariable);
+      } else if (valueType == typeid(int16_t)) {
+        return createDoocsScalar<int16_t, D_int>(processVariable);
+      } else if (valueType == typeid(uint16_t)) {
+        return createDoocsScalar<uint16_t, D_int>(processVariable);
+      } else if (valueType == typeid(int32_t)) {
+        return createDoocsScalar<int32_t, D_int>(processVariable);
+      } else if (valueType == typeid(uint32_t)) {
+        return createDoocsScalar<uint32_t, D_int>(processVariable);
+      } else if (valueType == typeid(float)) {
+        return createDoocsScalar<float, D_float>(processVariable);
+      } else if (valueType == typeid(double)) {
+        return createDoocsScalar<double, D_double>(processVariable);
+      } else if (valueType == typeid(std::string)) {
+        return createDoocsScalar<std::string, D_string>(processVariable);
+      } else {
+        throw std::invalid_argument("unsupported value type");
+      }
+    }else{ //nSamples > 1
+      if (valueType == typeid(int8_t)) {
+        return createDoocsSpectrum<int8_t>(processVariable);
+      } else if (valueType == typeid(uint8_t)) {
+        return createDoocsSpectrum<uint8_t>(processVariable);
+      } else if (valueType == typeid(int16_t)) {
+        return createDoocsSpectrum<int16_t>(processVariable);
+      } else if (valueType == typeid(uint16_t)) {
+        return createDoocsSpectrum<uint16_t>(processVariable);
+      } else if (valueType == typeid(int32_t)) {
+        return createDoocsSpectrum<int32_t>(processVariable);
+      } else if (valueType == typeid(uint32_t)) {
+        return createDoocsSpectrum<uint32_t>(processVariable);
+      } else if (valueType == typeid(float)) {
+        return createDoocsSpectrum<float>(processVariable);
+      } else if (valueType == typeid(double)) {
+        return createDoocsSpectrum<double>(processVariable);
+      } else {
+        throw std::invalid_argument("unsupported value type");
+      }
     }
  }
 
diff --git a/tests/src/testDoocsPVFactory.cpp b/tests/src/testDoocsPVFactory.cpp
index f1cce35..13ec209 100644
--- a/tests/src/testDoocsPVFactory.cpp
+++ b/tests/src/testDoocsPVFactory.cpp
@@ -42,8 +42,8 @@ public:
   }
 
   template<class T, class DOOCS_T>
-  typename boost::shared_ptr<D_fct> createDoocsProperty(typename ProcessVariable::SharedPtr & processVariable){
-    return DoocsPVFactory::createDoocsProperty<T, DOOCS_T>(processVariable);
+  typename boost::shared_ptr<D_fct> createDoocsScalar(typename ProcessVariable::SharedPtr & processVariable){
+    return DoocsPVFactory::createDoocsScalar<T, DOOCS_T>(processVariable);
   }
 };
 
@@ -218,7 +218,7 @@ BOOST_AUTO_TEST_CASE( testErrorHandling ){
   // Unfortunately BOOST_CHECK cannot deal with multiple template parameters,
   // so we have to trick it
   try{
-    testableFactory.createDoocsProperty<int32_t, D_int>( processScalar );
+    testableFactory.createDoocsScalar<int32_t, D_int>( processScalar );
     // In a working unit test this line should not be hit, so er exclude it
     // from the coverage report.
     BOOST_FAIL( "createDoocsScalar did not throw as expected");//LCOV_EXCL_LINE
@@ -227,7 +227,7 @@ BOOST_AUTO_TEST_CASE( testErrorHandling ){
 
   // now the same with arrays
   ProcessVariable::SharedPtr processArray = csManager->getProcessArray<int64_t>("A/toDeviceArray");
-  BOOST_CHECK_THROW( ( testableFactory.createDoocsProperty<int32_t, D_int>(processArray) ),
+  BOOST_CHECK_THROW( ( testableFactory.createDoocsScalar<int32_t, D_int>(processArray) ),
 		      std::invalid_argument );
 
    // finally we check that the create method catches the not-supported type.
-- 
GitLab