Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions config_utilities/include/config_utilities/factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 6 additions & 4 deletions config_utilities/include/config_utilities/virtual_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
24 changes: 14 additions & 10 deletions config_utilities/src/factory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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