Merge pull request #35 from ChimeraTK/exceptionHandling4

Exception handling4

include/DeviceModule.h

@@ -47,6 +47,7 @@
 #include "VirtualModule.h"
 #include <ChimeraTK/ForwardDeclarations.h>
 #include <ChimeraTK/RegisterPath.h>
+#include <ChimeraTK/Device.h>
 
 namespace ChimeraTK {
   class Application;
@@ -87,7 +88,6 @@ namespace ChimeraTK {
     /** Constructor: The device represented by this DeviceModule is identified by
      * either the device alias found in the DMAP file or directly an URI. */
     DeviceModule(Application* application, const std::string& deviceAliasOrURI);
-
     /** Default constructor: create dysfunctional device module */
     DeviceModule() {}
 
@@ -101,6 +101,7 @@ namespace ChimeraTK {
     DeviceModule& operator=(DeviceModule&& other) {
       assert(!moduleThread.joinable());
       Module::operator=(std::move(other));
+      device = std::move(other.device);
       deviceAliasOrURI = std::move(other.deviceAliasOrURI);
       registerNamePrefix = std::move(other.registerNamePrefix);
       deviceError = std::move(other.deviceError);
@@ -110,7 +111,6 @@ namespace ChimeraTK {
       owner->registerDeviceModule(this);
       return *this;
     }
-
     /** The subscript operator returns a VariableNetworkNode which can be used in
      * the Application::initialise()
      *  function to connect the register with another variable. */
@@ -138,6 +138,8 @@ namespace ChimeraTK {
      *  handling is done internally by ApplicationCore. */
     void reportException(std::string errMsg);
 
+    void prepare() override;
+
     void run() override;
 
     void terminate() override;
@@ -152,6 +154,8 @@ namespace ChimeraTK {
     /** This function connects DeviceError VariableGroup to ContolSystem*/
     void defineConnections() override;
 
+    mutable Device device;
+
    protected:
     // populate virtualisedModuleFromCatalog based on the information in the
     // device's catalogue
@@ -198,8 +202,14 @@ namespace ChimeraTK {
      *  The function is running an endless loop inside its own thread (moduleThread). */
     void handleException();
 
+    /** List of TransferElements to be written after the device has been opened. This is used to write constant feeders
+     *  to the device. */
+    std::list<boost::shared_ptr<TransferElement>> writeAfterOpen;
+
     Application* owner;
 
+    mutable bool deviceIsInitialized = false;
+
     friend class Application;
     friend class detail::DeviceModuleProxy;
   };

include/TestFacility.h

@@ -15,6 +15,7 @@
 #include <ChimeraTK/ScalarRegisterAccessor.h>
 
 #include "Application.h"
+#include "DeviceModule.h"
 #include "TestableModeAccessorDecorator.h"
 
 namespace ChimeraTK {
@@ -40,6 +41,16 @@ namespace ChimeraTK {
     void runApplication() const {
       Application::getInstance().run();
       Application::registerThread("TestThread");
+      Application::testableModeUnlock("waitDevicesToOpen");
+      while(true) {
+        boost::this_thread::yield();
+        bool allOpened = true;
+        for(auto dm : Application::getInstance().deviceModuleList) {
+          if(!dm->device.isOpened()) allOpened = false;
+        }
+        if(allOpened) break;
+      }
+      Application::testableModeLock("waitDevicesToOpen");
     }
 
     /** Perform a "step" of the application. This runs the application until all

src/Application.cc

@@ -152,17 +152,32 @@ void Application::run() {
   for(auto& module : getSubmoduleListRecursive()) {
     module->prepare();
   }
+  for(auto& deviceModule : deviceModuleList) {
+    deviceModule->prepare();
+  }
 
   // start the necessary threads for the FanOuts etc.
   for(auto& internalModule : internalModuleList) {
     internalModule->activate();
   }
 
-  // read all input variables once, to set the startup value e.g. coming from
-  // the config file (without triggering an action inside the application)
+  for(auto& deviceModule : deviceModuleList) {
+    deviceModule->run();
+  }
+
+  // Read all non-device variables once, to set the startup value from the persistency layer
+  // (without triggering an action inside the application)
+  // Note: this will read all application variables directly connected to either the control system or to another
+  // application module, e.g. the ConfigReader (which will provide initial values as well).
+  // Device variables are excluded for two reasons: Firstly, the device might be in an exception state when launching
+  // the application, so reading the variable would block until the device is properly opened. Secondly, some strange
+  // devices might cause side-effects when registers are read, and there would be no way to prevent these automatic
+  // reads from happening. If an application requires having an initial value from the device, it can simply issue
+  // a readLatest() at the beginning of its mainLoop() function.
   for(auto& module : getSubmoduleListRecursive()) {
     for(auto& variable : module->getAccessorList()) {
-      if(variable.getDirection().dir == VariableDirection::consuming) {
+      if(variable.getDirection().dir == VariableDirection::consuming &&
+          variable.getOwner().getFeedingNode().getType() != NodeType::Device) {
         variable.getAppAccessorNoType().readLatest();
       }
     }
@@ -172,9 +187,6 @@ void Application::run() {
   for(auto& module : getSubmoduleListRecursive()) {
     module->run();
   }
-  for(auto& deviceModule : deviceModuleList) {
-    deviceModule->run();
-  }
 }
 
 /*********************************************************************************************************************/
@@ -311,10 +323,9 @@ template<typename UserType>
 boost::shared_ptr<ChimeraTK::NDRegisterAccessor<UserType>> Application::createDeviceVariable(
     const std::string& deviceAlias, const std::string& registerName, VariableDirection direction, UpdateMode mode,
     size_t nElements) {
-  // open device if needed
+  // Device opens in DeviceModule
   if(deviceMap.count(deviceAlias) == 0) {
     deviceMap[deviceAlias] = ChimeraTK::BackendFactory::getInstance().createBackend(deviceAlias);
-    if(!deviceMap[deviceAlias]->isOpen()) deviceMap[deviceAlias]->open();
   }
 
   // use wait_for_new_data mode if push update mode was requested
@@ -949,7 +960,17 @@ void Application::typedMakeConnection(VariableNetwork& network) {
           auto impl = createDeviceVariable<UserType>(consumer.getDeviceAlias(), consumer.getRegisterName(),
               {VariableDirection::feeding, false}, consumer.getMode(), consumer.getNumberOfElements());
           impl->accessChannel(0) = feedingImpl->accessChannel(0);
-          impl->write();
+          // find the right DeviceModule for this alias name
+          DeviceModule* devmod = nullptr;
+          for(auto& dm : deviceModuleList) {
+            if(dm->deviceAliasOrURI == consumer.getDeviceAlias()) {
+              devmod = dm;
+              break;
+            }
+          }
+          assert(devmod != nullptr);
+          // register feeder to be written after the device has been opened
+          devmod->writeAfterOpen.push_back(impl);
         }
         else if(consumer.getType() == NodeType::TriggerReceiver) {
           throw ChimeraTK::logic_error("Using constants as triggers is not supported!");

src/DeviceModule.cc

@@ -5,7 +5,7 @@
  *      Author: Martin Hierholzer
  */
 
-#include <ChimeraTK/Device.h>
+//#include <ChimeraTK/Device.h>
 #include <ChimeraTK/DeviceBackend.h>
 
 #include "Application.h"
@@ -116,12 +116,12 @@ namespace ChimeraTK {
 
     virtualisedModuleFromCatalog = VirtualModule(deviceAliasOrURI, "Device module", ModuleType::Device);
 
+    if (!deviceIsInitialized){
+      device = Device(deviceAliasOrURI);
+      deviceIsInitialized = true;
+    }
     // obtain register catalogue
-    Device d;
-    d.open(deviceAliasOrURI); /// @todo: do not actually open the device (needs
-                              /// extension of DeviceAccess)!
-    auto catalog = d.getRegisterCatalogue();
-
+    auto catalog = device.getRegisterCatalogue();
     // iterate catalogue, create VariableNetworkNode for all registers starting
     // with the registerNamePrefix
     size_t prefixLength = registerNamePrefix.length();
@@ -238,11 +238,33 @@ namespace ChimeraTK {
 
   void DeviceModule::handleException() {
     Application::registerThread("DM_" + getName());
-    Device d;
     std::string error;
     owner->testableModeLock("Startup");
-
     try {
+      while(!device.isOpened()) {
+        try {
+          boost::this_thread::interruption_point();
+          usleep(500000);
+          device.open();
+          if(deviceError.status != 0) {
+            deviceError.status = 0;
+            deviceError.message = "";
+            deviceError.setCurrentVersionNumber({});
+            deviceError.writeAll();
+          }
+          for(auto& te : writeAfterOpen) {
+            te->write();
+          }
+        }
+        catch(ChimeraTK::runtime_error& e) {
+          if(deviceError.status != 1) {
+            deviceError.status = 1;
+            deviceError.message = error;
+            deviceError.setCurrentVersionNumber({});
+            deviceError.writeAll();
+          }
+        }
+      }
       while(true) {
         owner->testableModeUnlock("Wait for exception");
         errorQueue.pop_wait(error);
@@ -250,7 +272,6 @@ namespace ChimeraTK {
         owner->testableModeLock("Process exception");
         if(owner->isTestableModeEnabled()) --owner->testableMode_counter;
         std::lock_guard<std::mutex> lk(errorMutex);
-
         // report exception to the control system
         deviceError.status = 1;
         deviceError.message = error;
@@ -286,6 +307,17 @@ namespace ChimeraTK {
 
   /*********************************************************************************************************************/
 
+  void DeviceModule::prepare() {
+     if (!deviceIsInitialized){
+      device = Device(deviceAliasOrURI);
+      deviceIsInitialized = true;
+    }
+  }
+
+  /*********************************************************************************************************************/
+
+  /*********************************************************************************************************************/
+
   void DeviceModule::run() {
     // start the module thread
     assert(!moduleThread.joinable());

src/ExceptionHandlingDecorator.cc

@@ -7,6 +7,10 @@ namespace ChimeraTK {
   bool ExceptionHandlingDecorator<UserType>::doWriteTransfer(ChimeraTK::VersionNumber versionNumber) {
   retry:
     try {
+      if(!dm.device.isOpened()){
+        usleep(500000);
+        goto retry;
+      }
       return ChimeraTK::NDRegisterAccessorDecorator<UserType>::doWriteTransfer(versionNumber);
     }
     catch(ChimeraTK::runtime_error& e) {
@@ -19,6 +23,10 @@ namespace ChimeraTK {
   void ExceptionHandlingDecorator<UserType>::doReadTransfer() {
   retry:
     try {
+      if(!dm.device.isOpened()){
+        usleep(500000);
+        goto retry;
+      }
       return ChimeraTK::NDRegisterAccessorDecorator<UserType>::doReadTransfer();
     }
     catch(ChimeraTK::runtime_error& e) {
@@ -31,6 +39,10 @@ namespace ChimeraTK {
   bool ExceptionHandlingDecorator<UserType>::doReadTransferNonBlocking() {
   retry:
     try {
+      if(!dm.device.isOpened()){
+        usleep(500000);
+        goto retry;
+      }
       return ChimeraTK::NDRegisterAccessorDecorator<UserType>::doReadTransferNonBlocking();
     }
     catch(ChimeraTK::runtime_error& e) {
@@ -43,6 +55,10 @@ namespace ChimeraTK {
   bool ExceptionHandlingDecorator<UserType>::doReadTransferLatest() {
   retry:
     try {
+      if(!dm.device.isOpened()){
+        usleep(500000);
+        goto retry;
+      }
       return ChimeraTK::NDRegisterAccessorDecorator<UserType>::doReadTransferLatest();
     }
     catch(ChimeraTK::runtime_error& e) {
@@ -55,6 +71,10 @@ namespace ChimeraTK {
   TransferFuture ExceptionHandlingDecorator<UserType>::doReadTransferAsync() {
   retry:
     try {
+      if(!dm.device.isOpened()){
+        usleep(500000);
+        goto retry;
+      }
       return ChimeraTK::NDRegisterAccessorDecorator<UserType>::doReadTransferAsync();
     }
     catch(ChimeraTK::runtime_error& e) {
@@ -67,6 +87,10 @@ namespace ChimeraTK {
   void ExceptionHandlingDecorator<UserType>::doPreRead() {
   retry:
     try {
+      if(!dm.device.isOpened()){
+        usleep(500000);
+        goto retry;
+      }
       ChimeraTK::NDRegisterAccessorDecorator<UserType>::doPreRead();
     }
     catch(ChimeraTK::runtime_error& e) {
@@ -79,6 +103,10 @@ namespace ChimeraTK {
   void ExceptionHandlingDecorator<UserType>::doPostRead() {
   retry:
     try {
+      if(!dm.device.isOpened()){
+        usleep(500000);
+        goto retry;
+      }
       ChimeraTK::NDRegisterAccessorDecorator<UserType>::doPostRead();
     }
     catch(ChimeraTK::runtime_error& e) {
@@ -91,6 +119,10 @@ namespace ChimeraTK {
   void ExceptionHandlingDecorator<UserType>::doPreWrite() {
   retry:
     try {
+      if(!dm.device.isOpened()){
+        usleep(500000);
+        goto retry;
+      }
       ChimeraTK::NDRegisterAccessorDecorator<UserType>::doPreWrite();
     }
     catch(ChimeraTK::runtime_error& e) {
@@ -103,6 +135,10 @@ namespace ChimeraTK {
   void ExceptionHandlingDecorator<UserType>::doPostWrite() {
   retry:
     try {
+      if(!dm.device.isOpened()){
+        usleep(500000);
+        goto retry;
+      }
       ChimeraTK::NDRegisterAccessorDecorator<UserType>::doPostWrite();
     }
     catch(ChimeraTK::runtime_error& e) {

tests/executables_src/testDeviceAccessors.cc

@@ -90,8 +90,9 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(testFeedToDevice, T, test_types) {
   TestApplication<T> app;
 
   app.testModule.feedingToDevice >> app.dev["MyModule"]("actuator");
-  ctk::TestFacility test;
 
+  ctk::TestFacility test;
+  app.run();
   ChimeraTK::Device dev;
   dev.open("Dummy0");
   auto regacc = dev.getScalarRegisterAccessor<int>("/MyModule/actuator");
@@ -121,9 +122,10 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(testFeedToDeviceFanOut, T, test_types) {
 
   app.testModule.feedingToDevice >> app.dev["MyModule"]("actuator") >> app.dev["MyModule"]("readBack");
   ctk::TestFacility test;
-
+  app.run();
   ChimeraTK::Device dev;
   dev.open("Dummy0");
+
   auto regac = dev.getScalarRegisterAccessor<int>("/MyModule/actuator");
   auto regrb = dev.getScalarRegisterAccessor<int>("/MyModule/readBack");
 
@@ -159,7 +161,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(testConsumeFromDevice, T, test_types) {
 
   app.dev("/MyModule/actuator") >> app.testModule.consumingPoll;
   ctk::TestFacility test;
-
+  app.run();
   ChimeraTK::Device dev;
   dev.open("Dummy0");
   auto regacc = dev.getScalarRegisterAccessor<int>("/MyModule/actuator");
@@ -200,7 +202,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(testConsumingFanOut, T, test_types) {
   app.dev("/MyModule/actuator") >> app.testModule.consumingPoll >> app.testModule.consumingPush >>
       app.testModule.consumingPush2;
   ctk::TestFacility test;
-
+  app.run();
   ChimeraTK::Device dev;
   dev.open("Dummy0");
   auto regacc = dev.getScalarRegisterAccessor<int>("/MyModule/actuator");
@@ -346,7 +348,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(testConstantToDevice, T, test_types) {
 
   ctk::VariableNetworkNode::makeConstant<T>(true, 18) >> app.dev("/MyModule/actuator");
   ctk::TestFacility test;
-  app.run();
+  test.runApplication();
 
   ChimeraTK::Device dev;
   dev.open("Dummy0");
@@ -366,7 +368,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(testConstantToDeviceFanOut, T, test_types) {
 
   ctk::VariableNetworkNode::makeConstant<T>(true, 20) >> app.dev("/MyModule/actuator") >> app.dev("/MyModule/readBack");
   ctk::TestFacility test;
-  app.run();
+  test.runApplication();
 
   ChimeraTK::Device dev;
   dev.open("Dummy0");
@@ -387,7 +389,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(testDeviceModuleSubscriptOp, T, test_types) {
 
   app.testModule.feedingToDevice >> app.dev["MyModule"]("actuator");
   ctk::TestFacility test;
-
+  app.run();
   ChimeraTK::Device dev;
   dev.open("Dummy0");
   auto regacc = dev.getScalarRegisterAccessor<int>("/MyModule/actuator");

tests/executables_src/testExceptionHandling.cc

@@ -50,7 +50,6 @@ struct TestApplication : public ctk::Application {
 };
 
 /*********************************************************************************************************************/
-
 BOOST_AUTO_TEST_CASE(testExceptionHandlingRead) {
   std::cout << "testExceptionHandlingRead" << std::endl;
   TestApplication app;
@@ -231,3 +230,70 @@ BOOST_AUTO_TEST_CASE(testExceptionHandlingWrite) {
     BOOST_CHECK_EQUAL(status1, 0);
   }
 }
+/*********************************************************************************************************************/
+
+BOOST_AUTO_TEST_CASE(testExceptionHandlingOpen) {
+  std::cout << "testExceptionHandlingOpen" << std::endl;
+  TestApplication app;
+  boost::shared_ptr<ExceptionDummy> dummyBackend1 = boost::dynamic_pointer_cast<ExceptionDummy>(
+      ChimeraTK::BackendFactory::getInstance().createBackend(ExceptionDummyCDD1));
+  boost::shared_ptr<ExceptionDummy> dummyBackend2 = boost::dynamic_pointer_cast<ExceptionDummy>(
+      ChimeraTK::BackendFactory::getInstance().createBackend(ExceptionDummyCDD2));
+
+  ChimeraTK::DummyRegisterAccessor<int> readbackDummy1(dummyBackend1.get(), "MyModule", "readBack");
+  ChimeraTK::DummyRegisterAccessor<int> readbackDummy2(dummyBackend2.get(), "MyModule", "readBack");
+
+  // Connect the whole devices into the control system, and use the control system variable /trigger as trigger for
+  // both devices. The variable becomes a control system to application variable and writing to it through the test
+  // facility is generating the triggers.
+  app.dev1.connectTo(app.cs["Device1"], app.cs("trigger", typeid(int), 1));
+  app.dev2.connectTo(app.cs["Device2"], app.cs("trigger"));
+
+  // Do not enable testable mode. The testable mode would block in the wrong place, as the trigger for reading variables
+  // of a device in the error state is not being processed until the error state is cleared. We want to test that the
+  // second device still works while the first device is in error state, which would be impossible with testable mode
+  // enabled. As a consequence, our test logic has to work with timeouts (CHECK_TIMEOUT) etc. instead of the
+  // deterministic stepApplication().
+  ctk::TestFacility test(false);
+  dummyBackend1->throwExceptionOpen = true;
+  app.run(); // don't use TestFacility::runApplication() here as it blocks until all devices are open...
+
+  auto message1 = test.getScalar<std::string>(std::string("/Devices/") + ExceptionDummyCDD1 + "/message");
+  auto status1 = test.getScalar<int>(std::string("/Devices/") + ExceptionDummyCDD1 + "/status");
+  auto readback1 = test.getScalar<int>("/Device1/MyModule/readBack");
+  auto message2 = test.getScalar<std::string>(std::string("/Devices/") + ExceptionDummyCDD2 + "/message");
+  auto status2 = test.getScalar<int>(std::string("/Devices/") + ExceptionDummyCDD2 + "/status");
+  auto readback2 = test.getScalar<int>("/Device2/MyModule/readBack");
+
+  auto trigger = test.getScalar<int>("trigger");
+
+  readbackDummy1 = 100;
+  readbackDummy2 = 110;
+  trigger.write();
+  //device 1 is in Error state
+  CHECK_TIMEOUT(message1.readLatest(), 1000);
+  CHECK_TIMEOUT(status1.readLatest(), 1000);
+  BOOST_CHECK_EQUAL(status1, 1);
+  BOOST_CHECK(!readback1.readNonBlocking());
+  CHECK_TIMEOUT(readback2.readNonBlocking(), 1000);
+  BOOST_CHECK_EQUAL(readback2, 110);
+
+  // even with device 1 failing the second one must process the data, so send a new trigger
+  // before fixing dev1
+  readbackDummy2 = 120;
+  trigger.write();
+  CHECK_TIMEOUT(readback2.readNonBlocking(), 1000); // device 2 still works
+  BOOST_CHECK_EQUAL(readback2, 120);
+  //Device is not in error state.
+  CHECK_TIMEOUT(!message2.readLatest(), 1000);
+  CHECK_TIMEOUT(!status2.readLatest(), 1000);
+
+  //fix device 1
+  dummyBackend1->throwExceptionOpen = false;
+  //device 1 is fixed
+  CHECK_TIMEOUT(message1.readLatest(), 1000);
+  CHECK_TIMEOUT(status1.readLatest(), 1000);
+  BOOST_CHECK_EQUAL(status1, 0);
+  CHECK_TIMEOUT(readback1.readNonBlocking(), 1000);
+  BOOST_CHECK_EQUAL(readback1, 100);
+}