changed DoocsFactory to not create unnecessary decorators of the wrong type...

changed DoocsFactory to not create unnecessary decorators of the wrong type when auto-creating variables. Test for spectra still failing

include/DoocsPVFactory.h

@@ -45,6 +45,9 @@ namespace ChimeraTK {
 
     boost::shared_ptr<D_fct> autoCreate( std::shared_ptr<PropertyDescription> const & propertyDescription);
  
+    template <class IMPL_T, class DOOCS_SCALAR_T, class DOOCS_PRIMARY_T, class DOOCS_ARRAY_T, class DOOCS_ARRAY_PRIMITIVE_T>
+    boost::shared_ptr<D_fct> typedCreateScalarOrArray( ProcessVariable & processVariable, AutoPropertyDescription const & propertyDescription, DecoratorType decoratorType, ArrayDescription::DataType arrayDataType);
+    
   };
 
 

src/DoocsPVFactory.cc

@@ -113,77 +113,68 @@ namespace ChimeraTK {
     return doocsPV;
   }
 
+  // fixme: some of the variables needed here are redundant and can be sovled with mpl and/or fusion maps
+  template <class IMPL_T, class DOOCS_SCALAR_T, class DOOCS_PRIMITIVE_T, class DOOCS_ARRAY_T, class DOOCS_ARRAY_PRIMITIVE_T>
+  boost::shared_ptr<D_fct> DoocsPVFactory::typedCreateScalarOrArray( ProcessVariable & processVariable, AutoPropertyDescription const & autoPropertyDescription, DecoratorType decoratorType, ArrayDescription::DataType arrayDataType){
+    // We have to convert to the original NDRegisterAccessor to determine the number of samples.
+    // We cannot use a decorator because scalar and array DOOCS_PRIMITIVE_T can be different,
+    // and once a decorator is created you cannot get the other type any more.
+
+    auto & ndAccessor = dynamic_cast<mtca4u::NDRegisterAccessor<IMPL_T> &>(processVariable);
+    auto nSamples = ndAccessor.getNumberOfSamples();
+
+    if (nSamples == 1){
+      return createDoocsScalar<DOOCS_PRIMITIVE_T, DOOCS_SCALAR_T>(autoPropertyDescription, decoratorType);
+    }else{
+      return typedCreateDoocsArray<DOOCS_ARRAY_PRIMITIVE_T, DOOCS_ARRAY_T>(ArrayDescription(autoPropertyDescription, arrayDataType));
+    }
+  }
+
   boost::shared_ptr<D_fct> DoocsPVFactory::autoCreate( std::shared_ptr<PropertyDescription> const & propertyDescription ){
     // do auto creation
     auto autoPropertyDescription = std::static_pointer_cast<AutoPropertyDescription>(propertyDescription);
-
+    
     auto pvName = autoPropertyDescription->source;
     auto processVariable = _controlSystemPVManager->getProcessVariable(pvName);
 
     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 (nSamples == 1){
-      if (valueType == typeid(int8_t)) {
-        return createDoocsScalar<int32_t, D_int>(*autoPropertyDescription, DecoratorType::C_style_conversion);
-      } else if (valueType == typeid(uint8_t)) {
-        return createDoocsScalar<int32_t, D_int>(*autoPropertyDescription, DecoratorType::C_style_conversion);
-      } else if (valueType == typeid(int16_t)) {
-        return createDoocsScalar<int32_t, D_int>(*autoPropertyDescription, DecoratorType::C_style_conversion);
-      } else if (valueType == typeid(uint16_t)) {
-        return createDoocsScalar<int32_t, D_int>(*autoPropertyDescription, DecoratorType::C_style_conversion);
-      } else if (valueType == typeid(int32_t)) {
-        return createDoocsScalar<int32_t, D_int>(*autoPropertyDescription, DecoratorType::C_style_conversion);
-      } else if (valueType == typeid(uint32_t)) {
-        return createDoocsScalar<int32_t, D_int>(*autoPropertyDescription, DecoratorType::C_style_conversion);
-      } else if (valueType == typeid(float)) {
-        return createDoocsScalar<float, D_float>(*autoPropertyDescription, DecoratorType::C_style_conversion);
-      } else if (valueType == typeid(double)) {
-        return createDoocsScalar<double, D_double>(*autoPropertyDescription, DecoratorType::range_checking);
-      } else if (valueType == typeid(std::string)) {
-        return createDoocsScalar<std::string, D_string>(*autoPropertyDescription, DecoratorType::range_checking);
-      } else {
-        throw std::invalid_argument("unsupported value type");
-      }
-    }else{ //nSamples > 1
-      if (valueType == typeid(int8_t)) {
-        return  typedCreateDoocsArray<int8_t, D_bytearray>(ArrayDescription(*autoPropertyDescription, ArrayDescription::DataType::Byte));
-      } else if (valueType == typeid(uint8_t)) {
-        return  typedCreateDoocsArray<int8_t, D_bytearray>(ArrayDescription(*autoPropertyDescription, ArrayDescription::DataType::Byte));
-      } else if (valueType == typeid(int16_t)) {
-        return  typedCreateDoocsArray<int16_t, D_shortarray>(ArrayDescription(*autoPropertyDescription, ArrayDescription::DataType::Byte));
-      } else if (valueType == typeid(uint16_t)) {
-        return  typedCreateDoocsArray<int16_t, D_shortarray>(ArrayDescription(*autoPropertyDescription, ArrayDescription::DataType::Byte));
-      } else if (valueType == typeid(int32_t)) {
-        return  typedCreateDoocsArray<int32_t, D_intarray>(ArrayDescription(*autoPropertyDescription, ArrayDescription::DataType::Byte));
-      } else if (valueType == typeid(uint32_t)) {
-        return  typedCreateDoocsArray<int32_t, D_intarray>(ArrayDescription(*autoPropertyDescription, ArrayDescription::DataType::Byte));
-      } else if (valueType == typeid(int64_t)) {
-        return  typedCreateDoocsArray<long long int, D_longarray>(ArrayDescription(*autoPropertyDescription, ArrayDescription::DataType::Byte));
-      } else if (valueType == typeid(uint64_t)) {
-        return  typedCreateDoocsArray<long long int, D_longarray>(ArrayDescription(*autoPropertyDescription, ArrayDescription::DataType::Byte));
-      } else if (valueType == typeid(float)) {
-        return  typedCreateDoocsArray<float, D_floatarray>(ArrayDescription(*autoPropertyDescription, ArrayDescription::DataType::Byte));
-      } else if (valueType == typeid(double)) {
-        return  typedCreateDoocsArray<double, D_doublearray>(ArrayDescription(*autoPropertyDescription, ArrayDescription::DataType::Byte));
-      } else {
-        throw std::invalid_argument("unsupported value type");
-      }
+
+    // fixme: make this a boost::foreach in the data types provided by DeviceAccess.
+    // This will detect uncovered new data types at compile time, not only at run time
+    if (valueType == typeid(int8_t)) {
+      return typedCreateScalarOrArray<int8_t, D_int, int32_t, D_bytearray, int8_t>(*processVariable, *autoPropertyDescription, DecoratorType::C_style_conversion, ArrayDescription::DataType::Byte);
+    }else if (valueType == typeid(uint8_t)) {
+      return typedCreateScalarOrArray<uint8_t, D_int, int32_t, D_bytearray, int8_t>(*processVariable, *autoPropertyDescription, DecoratorType::C_style_conversion, ArrayDescription::DataType::Byte);
+
+    } else if (valueType == typeid(int16_t)) {
+      return typedCreateScalarOrArray<int16_t, D_int, int32_t, D_shortarray, int16_t>(*processVariable, *autoPropertyDescription, DecoratorType::C_style_conversion, ArrayDescription::DataType::Short);
+    } else if (valueType == typeid(uint16_t)) {
+      return typedCreateScalarOrArray<uint16_t, D_int, int32_t, D_shortarray, int16_t>(*processVariable, *autoPropertyDescription, DecoratorType::C_style_conversion, ArrayDescription::DataType::Short);
+    } else if (valueType == typeid(int32_t)) {
+      return typedCreateScalarOrArray<int32_t, D_int, int32_t, D_intarray, int32_t>(*processVariable, *autoPropertyDescription, DecoratorType::C_style_conversion, ArrayDescription::DataType::Int);
+    } else if (valueType == typeid(uint32_t)) {
+      return typedCreateScalarOrArray<uint32_t, D_int, int32_t, D_intarray, int32_t>(*processVariable, *autoPropertyDescription, DecoratorType::C_style_conversion, ArrayDescription::DataType::Int);
+    } else if (valueType == typeid(int64_t)) {
+      // there is no scalar int64 representation in doocs, so we always create an array, also for length = 1
+      return typedCreateDoocsArray<long long int, D_longarray>(ArrayDescription(*autoPropertyDescription, ArrayDescription::DataType::Long));
+    } else if (valueType == typeid(uint64_t)) {
+      return typedCreateDoocsArray<long long int, D_longarray>(ArrayDescription(*autoPropertyDescription, ArrayDescription::DataType::Long));
+    } else if (valueType == typeid(float)) {
+      return typedCreateScalarOrArray<float, D_float, float, D_floatarray, float>(*processVariable, *autoPropertyDescription, DecoratorType::range_checking, ArrayDescription::DataType::Float);
+    } else if (valueType == typeid(double)) {
+      return typedCreateScalarOrArray<double, D_double, double, D_doublearray, double>(*processVariable, *autoPropertyDescription, DecoratorType::range_checking, ArrayDescription::DataType::Double);
+    } else if (valueType == typeid(std::string)) {
+      // FIXME returning scalar also for arrays. This should result in an error
+      return createDoocsScalar<std::string, D_string>(*autoPropertyDescription, DecoratorType::range_checking);
+    } else {
+      throw std::invalid_argument("unsupported value type");
     }
+
  }
 
   template<class DOOCS_PRIMITIVE_T, class DOOCS_T>

src/DoocsUpdater.cc

@@ -4,8 +4,6 @@ namespace ChimeraTK{
 
   void DoocsUpdater::addVariable( mtca4u::TransferElement & variable, std::function<void ()> updaterFunction){
     _elementsToRead.push_back( std::reference_wrapper< mtca4u::TransferElement > (variable) );
-    std::cout << "adding callback function to " << variable.getName() << " at "
-              << &variable << std::endl;
     _toDoocsUpdateMap[&variable].push_back(updaterFunction);
   }
 
@@ -15,8 +13,6 @@ namespace ChimeraTK{
       // Currently this is consistent behaviour in the location update, and needed
       // for consistent testing.
       if (mapElem.first->readLatest()){
-        std::cout << "callbacks for " << mapElem.first->getName() << std::endl;
-        std::cout << "vector has a size of " << mapElem.second.size() << std::endl;
         for (auto & updaterFunc : mapElem.second){
           updaterFunc();
         }

tests/src/testDoocsPVFactory.cpp

@@ -25,7 +25,8 @@ using boost::shared_ptr;
 
 // use boost meta-programming to use test case templates
 // The list of types is an mpl type
-typedef boost::mpl::list<int32_t, uint32_t,
+typedef boost::mpl::list<int64_t, uint64_t,
+                         int32_t, uint32_t,
 			 int16_t, uint16_t,
 			 int8_t, uint8_t,
 			 float, double> simple_test_types;
@@ -179,7 +180,13 @@ BOOST_AUTO_TEST_CASE_TEMPLATE( testCreateArray, T, simple_test_types ){
   shared_ptr<DevicePVManager> devManager = pvManagers.second;
 
   static const size_t arraySize = 10;
+  // PVs can only get one decorator, so we have to put one array for each decorator/doocs type we want to test
   devManager->createProcessArray<T>(deviceToControlSystem,"A/fromDeviceArray1",arraySize);
+  devManager->createProcessArray<T>(deviceToControlSystem,"A/fromDeviceArray2",arraySize);
+  devManager->createProcessArray<T>(deviceToControlSystem,"A/fromDeviceArray3",arraySize);
+  devManager->createProcessArray<T>(deviceToControlSystem,"A/fromDeviceArray4",arraySize);
+  devManager->createProcessArray<T>(deviceToControlSystem,"A/fromDeviceArray5",arraySize);
+  devManager->createProcessArray<T>(deviceToControlSystem,"A/fromDeviceArray6",arraySize);
 
   // we need this later anyway, do we make a temporary variable
   auto pvNames = ChimeraTK::getAllVariableNames( csManager );
@@ -188,12 +195,12 @@ BOOST_AUTO_TEST_CASE_TEMPLATE( testCreateArray, T, simple_test_types ){
   
   DoocsPVFactory factory(&myEqFct, updater, csManager);
 
-  testArrayIsCorrectType<D_bytearray>(factory, ArrayDescription::DataType::Byte);
-  testArrayIsCorrectType<D_shortarray>(factory, ArrayDescription::DataType::Short);
-  testArrayIsCorrectType<D_intarray>(factory, ArrayDescription::DataType::Int);
-  testArrayIsCorrectType<D_longarray>(factory, ArrayDescription::DataType::Long);
-  testArrayIsCorrectType<D_floatarray>(factory, ArrayDescription::DataType::Float);
-  testArrayIsCorrectType<D_doublearray>(factory, ArrayDescription::DataType::Double);
+  testArrayIsCorrectType<D_bytearray>(factory, ArrayDescription::DataType::Byte, "fromDeviceArray1");
+  testArrayIsCorrectType<D_shortarray>(factory, ArrayDescription::DataType::Short, "fromDeviceArray2");
+  testArrayIsCorrectType<D_intarray>(factory, ArrayDescription::DataType::Int, "fromDeviceArray3");
+  testArrayIsCorrectType<D_longarray>(factory, ArrayDescription::DataType::Long, "fromDeviceArray4");
+  testArrayIsCorrectType<D_floatarray>(factory, ArrayDescription::DataType::Float, "fromDeviceArray5");
+  testArrayIsCorrectType<D_doublearray>(factory, ArrayDescription::DataType::Double, "fromDeviceArray6");
 }
 
 BOOST_AUTO_TEST_CASE( testAutoCreateArray ){
@@ -233,13 +240,12 @@ BOOST_AUTO_TEST_CASE( testAutoCreateArray ){
   testArrayIsCorrectType<D_doublearray>(factory, ArrayDescription::DataType::Auto, "toDeviceDoubleArray");
 }
 
-BOOST_AUTO_TEST_CASE( testErrorHandling ){
-    std::pair< shared_ptr<ControlSystemPVManager>,
+BOOST_AUTO_TEST_CASE( testInt64Scalar ){
+  std::pair< shared_ptr<ControlSystemPVManager>,
 	     shared_ptr<DevicePVManager> > pvManagers = createPVManager();
   shared_ptr<ControlSystemPVManager> csManager = pvManagers.first;
   shared_ptr<DevicePVManager> devManager = pvManagers.second;
 
-  // int64 is not supported yet
   devManager->createProcessArray<int64_t>(controlSystemToDevice,"I/toDeviceInt",1);
 
   DoocsUpdater updater;
@@ -248,17 +254,11 @@ BOOST_AUTO_TEST_CASE( testErrorHandling ){
 
   ProcessVariable::SharedPtr processScalar = 
     csManager->getProcessArray<int64_t>("I/toDeviceInt");
-  // Intentionally put the int64 scalar to the int32 create function.
-  // Unfortunately BOOST_CHECK cannot deal with multiple template parameters,
-  // so we have to trick it
+
   auto description = std::make_shared<AutoPropertyDescription>("I/toDeviceInt", "I", "toDeviceInt");
-  try{    factory.create( description );
-    // In a working unit test this line should not be hit, so er exclude it
-    // from the coverage report.
-    BOOST_ERROR( "createDoocsScalar did not throw as expected");//LCOV_EXCL_LINE
-  }catch(std::invalid_argument &e){
-    BOOST_CHECK( std::string("unsupported value type") == e.what() );
-  }
+  auto variable64 = factory.create( description );
+  // 64 bit integer scalars become D_longarrays because there is no 64 bit scalar representation in Doocs
+  BOOST_CHECK( boost::dynamic_pointer_cast<D_longarray>(variable64) );
 }
 
 // After you finished all test you have to end the test suite.