Prevent module names and accessor names having slashes. This sometimes created bizarre behaviour.

Unfortunatly this currently breaks the ServerHistory module, as it relies on slashes in accessor names. The MicroDAQ module had the same issue but has already been fixed. This fix can be adopted to the ServerHistory module as well.

Modules/include/MicroDAQ.h

@@ -52,7 +52,7 @@ namespace ChimeraTK {
                 {"MicroDAQ.CONFIG"}};
 
       /** Add a Module as a source to this DAQ. */
-      void addSource(const Module &source, const std::string &namePrefix="");
+      void addSource(const Module &source, const RegisterPath &namePrefix="");
 
     protected:
 
@@ -61,6 +61,9 @@ namespace ChimeraTK {
       template<typename UserType>
       VariableNetworkNode getAccessor(const std::string &variableName);
 
+      /** Map of VariableGroups required to build the hierarchies. The key it the full path name. */
+      std::map<std::string, VariableGroup> groupMap;
+
       /** boost::fusion::map of UserTypes to std::lists containing the ArrayPollInput accessors. These accessors are
       *  dynamically created by the AccessorAttacher. */
       template<typename UserType>

Modules/src/MicroDAQ.cc

@@ -39,20 +39,40 @@ namespace ChimeraTK {
 
   /*********************************************************************************************************************/
 
-  void MicroDAQ::addSource(const Module &source, const std::string &namePrefix) {
+  void MicroDAQ::addSource(const Module &source, const RegisterPath &namePrefix) {
 
     // for simplification, first create a VirtualModule containing the correct hierarchy structure (obeying eliminate
     // hierarchy etc.)
     auto dynamicModel = source.findTag(".*");     /// @todo use virtualise() instead
 
+    // create variable group map for namePrefix if needed
+    if(groupMap.find(namePrefix) == groupMap.end()) {
+      // search for existing parent (if any)
+      auto parentPrefix = namePrefix;
+      while(groupMap.find(parentPrefix) == groupMap.end()) {
+        if(parentPrefix == "/") break;  // no existing parent found
+        parentPrefix = std::string(parentPrefix).substr(0,std::string(parentPrefix).find_last_of("/"));
+      }
+      // create all not-yet-existing parents
+      while(parentPrefix != namePrefix) {
+        EntityOwner *owner = this;
+        if(parentPrefix != "/") owner = &groupMap[parentPrefix];
+        auto stop = std::string(namePrefix).find_first_of("/", parentPrefix.length()+1);
+        if(stop == std::string::npos) stop = namePrefix.length();
+        RegisterPath name = std::string(namePrefix).substr(parentPrefix.length(),stop-parentPrefix.length());
+        parentPrefix /= name;
+        groupMap[parentPrefix] = VariableGroup(owner, std::string(name).substr(1), "");
+      }
+    }
+
     // add all accessors on this hierarchy level
     for(auto &acc : dynamicModel.getAccessorList()) {
-      boost::fusion::for_each(accessorListMap.table, detail::AccessorAttacher(acc, this, namePrefix+"/"+acc.getName()));
+      boost::fusion::for_each(accessorListMap.table, detail::AccessorAttacher(acc, this, namePrefix/acc.getName()));
     }
 
     // recurse into submodules
     for(auto mod : dynamicModel.getSubmoduleList()) {
-      addSource(*mod, namePrefix+"/"+mod->getName());
+      addSource(*mod, namePrefix/mod->getName());
     }
 
   }
@@ -74,7 +94,9 @@ namespace ChimeraTK {
     // add accessor and name to lists
     auto &accessorList = boost::fusion::at_key<UserType>(accessorListMap.table);
     auto &nameList = boost::fusion::at_key<UserType>(nameListMap.table);
-    accessorList.emplace_back(this, variableName, "", 0, "");
+    auto dirName = variableName.substr(0,variableName.find_last_of("/"));
+    auto baseName = variableName.substr(variableName.find_last_of("/")+1);
+    accessorList.emplace_back(&groupMap[dirName], baseName, "", 0, "");
     nameList.push_back(variableName);
 
     // return the accessor

include/ArrayAccessor.h

@@ -54,7 +54,7 @@ namespace ChimeraTK {
         return dataLoss;
       }
 
-  protected:
+    protected:
 
       friend class InversionOfControlAccessor<ArrayAccessor<UserType>>;
 

include/InversionOfControlAccessor.h

@@ -96,6 +96,10 @@ namespace ChimeraTK {
       {
         static_assert(std::is_base_of<InversionOfControlAccessor<Derived>, Derived>::value,
                       "InversionOfControlAccessor<> must be used in a curiously recurring template pattern!");
+        if(name.find_first_of("/") != std::string::npos) {
+          throw ChimeraTK::logic_error("Accessor names must not contain slashes: '"+name+"' in module '"
+                                       +owner->getQualifiedName()+"'.");
+        }
         owner->registerAccessor(node);
       }
 

include/VirtualModule.h

@@ -12,6 +12,7 @@
 
 #include <boost/thread.hpp>
 
+#include <ChimeraTK/RegisterPath.h>
 #include "Module.h"
 
 namespace ChimeraTK {
@@ -24,7 +25,11 @@ namespace ChimeraTK {
       /** Constructor */
       VirtualModule(const std::string &name, const std::string &description, ModuleType moduleType)
       : Module(nullptr, name, description), _moduleType(moduleType)
-      {}
+      {
+        if(name.find_first_of("/") != std::string::npos) {
+          throw ChimeraTK::logic_error("Module names must not contain slashes: '"+name+"'.");
+        }
+      }
 
       /** Copy constructor */
       VirtualModule(const VirtualModule &other);
@@ -45,11 +50,11 @@ namespace ChimeraTK {
       void addSubModule(VirtualModule module);
 
       /** Return the submodule with the given name. If it doesn't exist, create it first. */
-      VirtualModule& createAndGetSubmodule(const std::string& moduleName);
+      VirtualModule& createAndGetSubmodule(const RegisterPath& moduleName);
 
       /** Like createAndGetSubmodule(), but recursively create a hierarchy of submodules separated by "/" in the
        *  moduleName. */
-      VirtualModule& createAndGetSubmoduleRecursive(const std::string& moduleName);
+      VirtualModule& createAndGetSubmoduleRecursive(const RegisterPath& moduleName);
 
       ModuleType getModuleType() const override { return _moduleType; }
 

src/ApplicationModule.cc

@@ -18,6 +18,10 @@ namespace ChimeraTK {
     if(!dynamic_cast<ModuleGroup*>(owner) && !dynamic_cast<Application*>(owner)) {
       throw ChimeraTK::logic_error("ApplicationModules must be owned either by ModuleGroups or the Application!");
     }
+    if(name.find_first_of("/") != std::string::npos) {
+      throw ChimeraTK::logic_error("Module names must not contain slashes: '"+name+" owned by '"
+                                   +owner->getQualifiedName()+"'.");
+    }
   }
 
 /*********************************************************************************************************************/

src/ControlSystemModule.cc

@@ -11,7 +11,10 @@
 namespace ChimeraTK {
 
   ControlSystemModule::ControlSystemModule(const std::string& _variableNamePrefix)
-  : Module(&(Application::getInstance()), "ControlSystem:"+_variableNamePrefix, ""),
+    : Module(&(Application::getInstance()),
+             _variableNamePrefix.empty() ? "<ControlSystem>"
+               : _variableNamePrefix.substr(_variableNamePrefix.find_last_of("/")+1),
+             ""),
     variableNamePrefix(_variableNamePrefix)
   {}
 
@@ -19,6 +22,7 @@ namespace ChimeraTK {
 
   VariableNetworkNode ControlSystemModule::operator()(const std::string& variableName, const std::type_info &valueType,
                                                       size_t nElements) const {
+    assert(variableName.find_first_of("/") == std::string::npos);
     if(variables.count(variableName) == 0) {
       variables[variableName] = {variableNamePrefix/variableName, VariableDirection::invalid, valueType, nElements};
     }
@@ -28,6 +32,7 @@ namespace ChimeraTK {
   /*********************************************************************************************************************/
 
   Module& ControlSystemModule::operator[](const std::string& moduleName) const {
+    assert(moduleName.find_first_of("/") == std::string::npos);
     if(subModules.count(moduleName) == 0) {
       subModules[moduleName] = {variableNamePrefix/moduleName};
     }

src/DeviceModule.cc

@@ -14,7 +14,10 @@
 namespace ChimeraTK {
 
   DeviceModule::DeviceModule(const std::string& _deviceAliasOrURI, const std::string& _registerNamePrefix)
-  : Module(&(Application::getInstance()), "Device:"+_deviceAliasOrURI+":"+_registerNamePrefix, ""),
+  : Module(&(Application::getInstance()),
+           _registerNamePrefix.empty() ? "<Device:"+_deviceAliasOrURI+">"
+             : _registerNamePrefix.substr(_registerNamePrefix.find_last_of("/")+1),
+           ""),
     deviceAliasOrURI(_deviceAliasOrURI),
     registerNamePrefix(_registerNamePrefix)
   {}

src/VirtualModule.cc

@@ -109,24 +109,24 @@ namespace ChimeraTK {
 
 /*********************************************************************************************************************/
 
-  VirtualModule& VirtualModule::createAndGetSubmodule(const std::string& moduleName) {
+  VirtualModule& VirtualModule::createAndGetSubmodule(const RegisterPath& moduleName) {
     for(auto &sm : submodules) {
-      if(sm.getName() == moduleName) return sm;
+      if(moduleName == sm.getName()) return sm;
     }
-    addSubModule(VirtualModule(moduleName, getDescription(), getModuleType()));
+    addSubModule(VirtualModule(std::string(moduleName).substr(1), getDescription(), getModuleType()));
     return submodules.back();
   }
 
 /*********************************************************************************************************************/
 
-  VirtualModule& VirtualModule::createAndGetSubmoduleRecursive(const std::string& moduleName) {
-    auto slash = moduleName.find_first_of("/");
+  VirtualModule& VirtualModule::createAndGetSubmoduleRecursive(const RegisterPath& moduleName) {
+    auto slash = std::string(moduleName).find_first_of("/",1);
     if(slash == std::string::npos) {
       return createAndGetSubmodule(moduleName);
     }
     else {
-      auto firstSubmodule = moduleName.substr(0,slash);
-      auto remainingSubmodules = moduleName.substr(slash+1);
+      auto firstSubmodule = std::string(moduleName).substr(0,slash);
+      auto remainingSubmodules = std::string(moduleName).substr(slash+1);
       return createAndGetSubmodule(firstSubmodule).createAndGetSubmoduleRecursive(remainingSubmodules);
     }
   }