avoid log errors when type is missing but config is initialized (#35)

* avoid log errors when type is missing but config is initialized

* simplify logic and add comments

* drop commented code

* get required logic correct

Author: nathanhhughes

config_utilities/include/config_utilities/factory.h

@@ -61,11 +61,13 @@ std::vector<std::string> convertArguments() {
   };
 }
 
-//! @brief Helper function to read the type param from a node.
-bool getTypeImpl(const YAML::Node& data, std::string& type, const std::string& param_name);
-
-//! @brief Get type from YAML node directly
-bool getType(const YAML::Node& data, std::string& type);
+/** @brief Helper function to read the type param from a node.
+ * @param data YAML node to read type from
+ * @param type Type value to filll
+ * @param required Whether or not the type field is required
+ * @param param_name Field in YAML node to read (empty string defaults to Settings().factory_type_param_name)
+ */
+bool getType(const YAML::Node& data, std::string& type, bool required = true, const std::string& param_name = "");
 
 //! @brief Struct recording typenames for a module (i.e., the constructor signature). Can be used as a map key
 struct ModuleInfo {

config_utilities/include/config_utilities/virtual_config.h

@@ -205,14 +205,16 @@ template <typename BaseT>
 void declare_config(VirtualConfig<BaseT>& config) {
   auto data = internal::Visitor::visitVirtualConfig(config.isSet(), config.optional_, config.getType());
 
+  // underlying derived type is not required if the config is optional, or if the config has been
+  // initialized to a derived type already (i.e., config_ is already populated)
+  const bool type_required = !config.optional_ && !config.config_;
+
   // If setting values create the wrapped config using the string identifier.
   if (data) {
     std::string type;
-    const bool success = config.optional_ ? internal::getTypeImpl(*data, type, Settings().factory_type_param_name)
-                                          : internal::getType(*data, type);
-    if (success) {
+    if (internal::getType(*data, type, type_required)) {
       config.config_ = internal::ConfigFactory<BaseT>::create(type);
-    } else if (!config.optional_) {
+    } else if (type_required) {
       std::stringstream ss;
       ss << "Could not get type for '" << internal::ModuleInfo::fromTypes<BaseT>().typeInfo() << "'";
       internal::Logger::logError(ss.str());

config_utilities/src/factory.cpp

@@ -213,29 +213,33 @@ ModuleRegistry& ModuleRegistry::instance() {
 }
 
 // Helper function to read the type param from a node.
-bool getTypeImpl(const YAML::Node& data, std::string& type, const std::string& param_name) {
+bool getTypeImpl(const YAML::Node& data, std::string& type, const std::string& key) {
   if (!data.IsMap()) {
     return false;
   }
-  if (!data[param_name]) {
+
+  // Get the type or print an error.
+  if (!data[key]) {
     return false;
   }
+
   try {
-    type = data[param_name].as<std::string>();
+    type = data[key].as<std::string>();
   } catch (const YAML::Exception& e) {
     return false;
   }
+
   return true;
 }
 
-bool getType(const YAML::Node& data, std::string& type) {
-  // Get the type or print an error.
-  const std::string param_name = Settings::instance().factory_type_param_name;
-  if (!getTypeImpl(data, type, param_name)) {
-    Logger::logError("Could not read the param '" + param_name + "' to deduce the type of the module to create.");
-    return false;
+bool getType(const YAML::Node& data, std::string& type, bool required, const std::string& param_name) {
+  const std::string key = param_name.empty() ? Settings::instance().factory_type_param_name : param_name;
+  const auto success = getTypeImpl(data, type, key);
+  if (!success && required) {
+    Logger::logError("Could not read the param '" + key + "' to deduce the type of the module to create.");
   }
-  return true;
+
+  return success;
 }
 
 }  // namespace config::internal