From 1d2475f1d49f64922cd2019caf141c7682c19893 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Wed, 8 Jan 2025 11:53:12 +0100 Subject: [PATCH 1/8] Remove serialize function from config The same functionality can be achieved without first including the main config in the serialized config and then deleting it... Signed-off-by: Kai-Uwe Hermann # Conflicts: # src/manager.cpp Signed-off-by: Kai-Uwe Hermann --- include/utils/config.hpp | 4 ---- lib/config.cpp | 4 ---- src/manager.cpp | 16 +++++++--------- tests/test_config.cpp | 12 ------------ 4 files changed, 7 insertions(+), 29 deletions(-) diff --git a/include/utils/config.hpp b/include/utils/config.hpp index 0b9cf611..2230c22d 100644 --- a/include/utils/config.hpp +++ b/include/utils/config.hpp @@ -259,10 +259,6 @@ class ManagerConfig : public ConfigBase { /// \brief Create a ManagerConfig from the provided ManagerSettings \p ms explicit ManagerConfig(const ManagerSettings& ms); - /// - /// \brief Serialize the config to json - nlohmann::json serialize(); - /// /// \returns a TelemetryConfig if this has been configured for the given \p module_id std::optional get_telemetry_config(const std::string& module_id); diff --git a/lib/config.cpp b/lib/config.cpp index a7fb872f..0529382a 100644 --- a/lib/config.cpp +++ b/lib/config.cpp @@ -1088,10 +1088,6 @@ ManagerConfig::ManagerConfig(const ManagerSettings& ms) : ConfigBase(ms.mqtt_set } } -json ManagerConfig::serialize() { - return json::object({{"main", this->main}, {"module_names", this->module_names}}); -} - std::optional ManagerConfig::get_telemetry_config(const std::string& module_id) { BOOST_LOG_FUNCTION(); diff --git a/src/manager.cpp b/src/manager.cpp index beeb78fc..f2a9267e 100644 --- a/src/manager.cpp +++ b/src/manager.cpp @@ -300,10 +300,10 @@ static std::map start_modules(ManagerConfig& config, MQTTAbs std::vector modules_to_spawn; - const auto main_config = config.get_main_config(); + const auto& main_config = config.get_main_config(); + const auto& module_names = config.get_module_names(); modules_to_spawn.reserve(main_config.size()); - const auto serialized_config = config.serialize(); const auto interface_definitions = config.get_interface_definitions(); std::vector interface_names; for (auto& interface_definition : interface_definitions.items()) { @@ -352,12 +352,13 @@ static std::map start_modules(ManagerConfig& config, MQTTAbs mqtt_abstraction.publish(fmt::format("{}module_config_cache", ms.mqtt_settings.everest_prefix), module_config_cache, QOS::QOS2, true); - for (const auto& module : serialized_config.at("module_names").items()) { - const std::string& module_name = module.key(); - json serialized_mod_config = serialized_config; + for (const auto& module_name_entry : module_names) { + const auto& module_name = module_name_entry.first; + const auto& module_type = module_name_entry.second; + json serialized_mod_config = json::object({{"module_names", module_names}}); serialized_mod_config["module_config"] = json::object(); + serialized_mod_config["module_config"][module_name] = main_config.at(module_name); // add mappings of fulfillments - serialized_mod_config["module_config"][module_name] = serialized_config.at("main").at(module_name); const auto fulfillments = config.get_fulfillments(module_name); serialized_mod_config["mappings"] = json::object(); for (const auto& fulfillment_list : fulfillments) { @@ -373,7 +374,6 @@ static std::map start_modules(ManagerConfig& config, MQTTAbs if (mappings.has_value()) { serialized_mod_config["mappings"][module_name] = mappings.value(); } - serialized_mod_config.erase("main"); // FIXME: do not put this "main" config in there in the first place const auto telemetry_config = config.get_telemetry_config(module_name); if (telemetry_config.has_value()) { serialized_mod_config["telemetry_config"] = telemetry_config.value(); @@ -384,8 +384,6 @@ static std::map start_modules(ManagerConfig& config, MQTTAbs continue; } - // FIXME (aw): shall create a ref to main_confit.at(module_name)! - const std::string module_type = main_config.at(module_name).at("module"); // FIXME (aw): implicitely adding ModuleReadyInfo and setting its ready member auto module_it = modules_ready.emplace(module_name, ModuleReadyInfo{false, nullptr, nullptr}).first; diff --git a/tests/test_config.cpp b/tests/test_config.cpp index dddd1e79..516ce9c3 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -171,18 +171,6 @@ SCENARIO("Check Config Constructor", "[!throws]") { }()); } } - GIVEN("A valid config with a valid module serialized") { - auto ms = - Everest::ManagerSettings(bin_dir + "valid_module_config/", bin_dir + "valid_module_config/config.yaml"); - THEN("It should not throw at all") { - CHECK_NOTHROW([&]() { - auto mc = Everest::ManagerConfig(ms); - auto serialized = mc.serialize(); - CHECK(serialized.at("module_names").size() == 1); - CHECK(serialized.at("module_names").at("valid_module") == "TESTValidManifest"); - }()); - } - } GIVEN("A valid config in legacy json format with a valid module") { auto ms = Everest::ManagerSettings(bin_dir + "valid_module_config_json/", bin_dir + "valid_module_config_json/config.json"); From f100f1d13a22add25a5ab8ba7a7ce3b8d3af1791 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Tue, 10 Dec 2024 17:24:50 +0100 Subject: [PATCH 2/8] Directly pass the active_modules part of a config to the parse function Skip storing it in a temporary variable Signed-off-by: Kai-Uwe Hermann --- lib/config.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/config.cpp b/lib/config.cpp index 0529382a..00e9491d 100644 --- a/lib/config.cpp +++ b/lib/config.cpp @@ -1080,9 +1080,8 @@ ManagerConfig::ManagerConfig(const ManagerSettings& ms) : ConfigBase(ms.mqtt_set complete_config = complete_config.patch(patch); } - const auto config = complete_config.at("active_modules"); this->settings = this->ms.get_runtime_settings(); - this->parse(config); + this->parse(complete_config.at("active_modules")); } catch (const std::exception& e) { EVLOG_AND_THROW(EverestConfigError(fmt::format("Failed to load and parse config file: {}", e.what()))); } From f82241afd516fec2e8a5210e7889e7c459b75d3b Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Wed, 11 Dec 2024 12:19:37 +0100 Subject: [PATCH 3/8] Only publish module_names (a mapping of module type to id) once and retain Previously this was sent to every module individually which isn't necessary Signed-off-by: Kai-Uwe Hermann --- lib/module_config.cpp | 4 ++++ src/manager.cpp | 7 ++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/module_config.cpp b/lib/module_config.cpp index f92e80fe..84d22526 100644 --- a/lib/module_config.cpp +++ b/lib/module_config.cpp @@ -104,6 +104,10 @@ json get_module_config(std::shared_ptr mqtt, const std::string& const auto module_config_cache = mqtt->get(module_config_cache_topic, QOS::QOS2); result["module_config_cache"] = module_config_cache; + const auto module_names_topic = fmt::format("{}module_names", everest_prefix); + const auto module_names = mqtt->get(module_names_topic, QOS::QOS2); + result["module_names"] = module_names; + return result; } diff --git a/src/manager.cpp b/src/manager.cpp index f2a9267e..39d27daf 100644 --- a/src/manager.cpp +++ b/src/manager.cpp @@ -290,6 +290,8 @@ void cleanup_retained_topics(ManagerConfig& config, MQTTAbstraction& mqtt_abstra mqtt_abstraction.publish(fmt::format("{}error_types_map", mqtt_everest_prefix), std::string(), QOS::QOS2, true); mqtt_abstraction.publish(fmt::format("{}module_config_cache", mqtt_everest_prefix), std::string(), QOS::QOS2, true); + + mqtt_abstraction.publish(fmt::format("{}module_names", mqtt_everest_prefix), std::string(), QOS::QOS2, true); } static std::map start_modules(ManagerConfig& config, MQTTAbstraction& mqtt_abstraction, @@ -352,10 +354,13 @@ static std::map start_modules(ManagerConfig& config, MQTTAbs mqtt_abstraction.publish(fmt::format("{}module_config_cache", ms.mqtt_settings.everest_prefix), module_config_cache, QOS::QOS2, true); + mqtt_abstraction.publish(fmt::format("{}module_names", ms.mqtt_settings.everest_prefix), module_names, QOS::QOS2, + true); + for (const auto& module_name_entry : module_names) { const auto& module_name = module_name_entry.first; const auto& module_type = module_name_entry.second; - json serialized_mod_config = json::object({{"module_names", module_names}}); + json serialized_mod_config = json::object(); serialized_mod_config["module_config"] = json::object(); serialized_mod_config["module_config"][module_name] = main_config.at(module_name); // add mappings of fulfillments From 1d28865120bf0b3395d840d6a6b567ce8bd85d0e Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Thu, 12 Dec 2024 00:05:15 +0100 Subject: [PATCH 4/8] Publish manifest individually Add type to parsed_config_map and remove config entry from manifest sent via MQTT Signed-off-by: Kai-Uwe Hermann --- include/utils/config.hpp | 4 ++-- lib/config.cpp | 16 +++++++--------- lib/module_config.cpp | 15 +++++++++++---- src/manager.cpp | 8 +++++++- 4 files changed, 27 insertions(+), 16 deletions(-) diff --git a/include/utils/config.hpp b/include/utils/config.hpp index 2230c22d..d5a204f0 100644 --- a/include/utils/config.hpp +++ b/include/utils/config.hpp @@ -287,7 +287,7 @@ class Config : public ConfigBase { /// /// \returns the commands that the modules \p module_name implements from the given implementation \p impl_id - nlohmann::json get_module_cmds(const std::string& module_name, const std::string& impl_id); + const nlohmann::json& get_module_cmds(const std::string& module_name, const std::string& impl_id); /// /// \brief A RequirementInitialization contains everything needed to initialize a requirement in user code. This @@ -301,7 +301,7 @@ class Config : public ConfigBase { /// /// \returns a json object that contains the module config options - nlohmann::json get_module_json_config(const std::string& module_id); + const nlohmann::json& get_module_json_config(const std::string& module_id); /// /// \brief assemble basic information about the module (id, name, diff --git a/lib/config.cpp b/lib/config.cpp index 00e9491d..166108c3 100644 --- a/lib/config.cpp +++ b/lib/config.cpp @@ -104,7 +104,7 @@ static ParsedConfigMap parse_config_map(const json& config_map_schema, const jso throw ConfigParseException(ConfigParseException::SCHEMA, config_entry_name, err.what()); } - parsed_config_map[config_entry_name] = config_entry_value; + parsed_config_map[config_entry_name] = json::object({{"value", config_entry_value}, {"type", config_entry.at("type")}}); } return {parsed_config_map, unknown_config_entries}; @@ -1128,7 +1128,7 @@ bool Config::module_provides(const std::string& module_name, const std::string& return (provides.find(impl_id) != provides.end()); } -json Config::get_module_cmds(const std::string& module_name, const std::string& impl_id) { +const json& Config::get_module_cmds(const std::string& module_name, const std::string& impl_id) { return this->module_config_cache.at(module_name).cmds.at(impl_id); } @@ -1156,14 +1156,12 @@ ModuleConfigs Config::get_module_configs(const std::string& module_id) const { const json manifest = this->manifests.at(module_type); for (const auto& conf_map : config_maps.items()) { - const json config_schema = (conf_map.key() == "!module") - ? manifest.at("config") - : manifest.at("provides").at(conf_map.key()).at("config"); ConfigMap processed_conf_map; for (const auto& entry : conf_map.value().items()) { - const json entry_type = config_schema.at(entry.key()).at("type"); + const auto& entry_value = entry.value(); + const json entry_type = entry_value.at("type"); ConfigEntry value; - const json& data = entry.value(); + const json& data = entry_value.at("value"); if (data.is_string()) { value = data.get(); @@ -1193,9 +1191,9 @@ ModuleConfigs Config::get_module_configs(const std::string& module_id) const { } // FIXME (aw): check if module_id does not exist -json Config::get_module_json_config(const std::string& module_id) { +const json& Config::get_module_json_config(const std::string& module_id) { BOOST_LOG_FUNCTION(); - return this->main[module_id]["config_maps"]; + return this->main.at(module_id).at("config_maps"); } ModuleInfo Config::get_module_info(const std::string& module_id) const { diff --git a/lib/module_config.cpp b/lib/module_config.cpp index 84d22526..aad5c476 100644 --- a/lib/module_config.cpp +++ b/lib/module_config.cpp @@ -92,8 +92,18 @@ json get_module_config(std::shared_ptr mqtt, const std::string& const auto schemas = mqtt->get(schemas_topic, QOS::QOS2); result["schemas"] = schemas; + const auto module_names_topic = fmt::format("{}module_names", everest_prefix); + const auto module_names = mqtt->get(module_names_topic, QOS::QOS2); + result["module_names"] = module_names; + const auto manifests_topic = fmt::format("{}manifests", everest_prefix); - const auto manifests = mqtt->get(manifests_topic, QOS::QOS2); + auto manifests = json::object(); + for (const auto& module_name : module_names) { + auto manifest_topic = fmt::format("{}manifests/{}", everest_prefix, module_name.get()); + auto manifest = mqtt->get(manifest_topic, QOS::QOS2); + manifests[module_name] = manifest; + } + result["manifests"] = manifests; const auto error_types_map_topic = fmt::format("{}error_types_map", everest_prefix); @@ -104,9 +114,6 @@ json get_module_config(std::shared_ptr mqtt, const std::string& const auto module_config_cache = mqtt->get(module_config_cache_topic, QOS::QOS2); result["module_config_cache"] = module_config_cache; - const auto module_names_topic = fmt::format("{}module_names", everest_prefix); - const auto module_names = mqtt->get(module_names_topic, QOS::QOS2); - result["module_names"] = module_names; return result; } diff --git a/src/manager.cpp b/src/manager.cpp index 39d27daf..d12d174f 100644 --- a/src/manager.cpp +++ b/src/manager.cpp @@ -344,7 +344,13 @@ static std::map start_modules(ManagerConfig& config, MQTTAbs mqtt_abstraction.publish(fmt::format("{}schemas", ms.mqtt_settings.everest_prefix), schemas, QOS::QOS2, true); const auto manifests = config.get_manifests(); - mqtt_abstraction.publish(fmt::format("{}manifests", ms.mqtt_settings.everest_prefix), manifests, QOS::QOS2, true); + + for (const auto& manifest : manifests.items()) { + auto manifest_copy = manifest.value(); + manifest_copy.erase("config"); + mqtt_abstraction.publish(fmt::format("{}manifests/{}", ms.mqtt_settings.everest_prefix, manifest.key()), + manifest_copy, QOS::QOS2, true); + } const auto error_types_map = config.get_error_types(); mqtt_abstraction.publish(fmt::format("{}error_types_map", ms.mqtt_settings.everest_prefix), error_types_map, From 5a7585c7be95d8d20fbdd3e5dcfdecdad18d2cc3 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Mon, 16 Dec 2024 11:27:01 +0100 Subject: [PATCH 5/8] Fix everestjs config parsing with new config entry format Signed-off-by: Kai-Uwe Hermann --- everestjs/conversions.cpp | 8 ++++++++ everestjs/conversions.hpp | 1 + everestjs/everestjs.cpp | 7 ++++--- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/everestjs/conversions.cpp b/everestjs/conversions.cpp index 2ed49d76..e3038b4e 100644 --- a/everestjs/conversions.cpp +++ b/everestjs/conversions.cpp @@ -54,6 +54,14 @@ Everest::json convertToJson(const Napi::Value& value) { napi_valuetype_strings[value.Type()])); } +Everest::json convertToConfigMap(const Everest::json& json_config) { + json config_map; + for (auto& entry : json_config.items()) { + config_map[entry.key()] = entry.value().at("value"); + } + return config_map; +} + Everest::TelemetryMap convertToTelemetryMap(const Napi::Object& obj) { BOOST_LOG_FUNCTION(); Everest::TelemetryMap telemetry; diff --git a/everestjs/conversions.hpp b/everestjs/conversions.hpp index edbcdb7d..25798d47 100644 --- a/everestjs/conversions.hpp +++ b/everestjs/conversions.hpp @@ -29,6 +29,7 @@ static const char* const napi_valuetype_strings[] = { }; Everest::json convertToJson(const Napi::Value& value); +Everest::json convertToConfigMap(const Everest::json& json_config); Everest::TelemetryMap convertToTelemetryMap(const Napi::Object& obj); Napi::Value convertToNapiValue(const Napi::Env& env, const Everest::json& value); diff --git a/everestjs/everestjs.cpp b/everestjs/everestjs.cpp index 924372f4..1e40460f 100644 --- a/everestjs/everestjs.cpp +++ b/everestjs/everestjs.cpp @@ -885,14 +885,15 @@ static Napi::Value boot_module(const Napi::CallbackInfo& info) { auto module_config_prop = Napi::Object::New(env); auto module_config_impl_prop = Napi::Object::New(env); - for (auto& config_map : module_config.items()) { + for (const auto& config_map : module_config.items()) { + const auto& json_config_map = convertToConfigMap(config_map.value()); if (config_map.key() == "!module") { module_config_prop.DefineProperty(Napi::PropertyDescriptor::Value( - "module", convertToNapiValue(env, config_map.value()), napi_enumerable)); + "module", convertToNapiValue(env, json_config_map), napi_enumerable)); continue; } module_config_impl_prop.DefineProperty(Napi::PropertyDescriptor::Value( - config_map.key(), convertToNapiValue(env, config_map.value()), napi_enumerable)); + config_map.key(), convertToNapiValue(env, json_config_map), napi_enumerable)); } module_config_prop.DefineProperty( Napi::PropertyDescriptor::Value("impl", module_config_impl_prop, napi_enumerable)); From 95f1999df9ffac79ad78fa829c81200e0dba0d37 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Wed, 18 Dec 2024 18:45:04 +0100 Subject: [PATCH 6/8] Use get_module_name instead of get_module_info if only the module name is required Signed-off-by: Kai-Uwe Hermann --- lib/everest.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/everest.cpp b/lib/everest.cpp index d570da2d..c195cd2b 100644 --- a/lib/everest.cpp +++ b/lib/everest.cpp @@ -229,11 +229,11 @@ void Everest::heartbeat() { void Everest::publish_metadata() { BOOST_LOG_FUNCTION(); - const auto module_info = this->config.get_module_info(this->module_id); - const auto manifest = this->config.get_manifests().at(module_info.name); + const auto& module_name = this->config.get_module_name(this->module_id); + const auto& manifest = this->config.get_manifests().at(module_name); json metadata = json({}); - metadata["module"] = module_info.name; + metadata["module"] = module_name; if (manifest.contains("provides")) { metadata["provides"] = json({}); From 44e76e145b6580f902e1a7d72a803cf8cba1d63c Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Sat, 21 Dec 2024 20:03:38 +0100 Subject: [PATCH 7/8] Re-order config handler in manager Signed-off-by: Kai-Uwe Hermann --- src/manager.cpp | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/manager.cpp b/src/manager.cpp index d12d174f..ee54295b 100644 --- a/src/manager.cpp +++ b/src/manager.cpp @@ -398,6 +398,17 @@ static std::map start_modules(ManagerConfig& config, MQTTAbs // FIXME (aw): implicitely adding ModuleReadyInfo and setting its ready member auto module_it = modules_ready.emplace(module_name, ModuleReadyInfo{false, nullptr, nullptr}).first; + const std::string config_topic = fmt::format("{}/config", config.mqtt_module_prefix(module_name)); + const Handler module_get_config_handler = [module_name, config_topic, serialized_mod_config, + &mqtt_abstraction](const std::string&, const nlohmann::json& json) { + mqtt_abstraction.publish(config_topic, serialized_mod_config.dump(), QOS::QOS2); + }; + + const std::string get_config_topic = fmt::format("{}/get_config", config.mqtt_module_prefix(module_name)); + module_it->second.get_config_token = std::make_shared( + HandlerType::ExternalMQTT, std::make_shared(module_get_config_handler)); + mqtt_abstraction.register_handler(get_config_topic, module_it->second.get_config_token, QOS::QOS2); + const auto capabilities = [&module_config = main_config.at(module_name)]() { const auto cap_it = module_config.find("capabilities"); if (cap_it == module_config.end()) { @@ -456,17 +467,6 @@ static std::map start_modules(ManagerConfig& config, MQTTAbs std::make_shared(HandlerType::ExternalMQTT, std::make_shared(module_ready_handler)); mqtt_abstraction.register_handler(ready_topic, module_it->second.ready_token, QOS::QOS2); - const std::string config_topic = fmt::format("{}/config", config.mqtt_module_prefix(module_name)); - const Handler module_get_config_handler = [module_name, config_topic, serialized_mod_config, - &mqtt_abstraction](const std::string&, const nlohmann::json& json) { - mqtt_abstraction.publish(config_topic, serialized_mod_config.dump()); - }; - - const std::string get_config_topic = fmt::format("{}/get_config", config.mqtt_module_prefix(module_name)); - module_it->second.get_config_token = std::make_shared( - HandlerType::ExternalMQTT, std::make_shared(module_get_config_handler)); - mqtt_abstraction.register_handler(get_config_topic, module_it->second.get_config_token, QOS::QOS2); - if (std::any_of(standalone_modules.begin(), standalone_modules.end(), [module_name](const auto& element) { return element == module_name; })) { EVLOG_info << "Not starting standalone module: " << fmt::format(TERMINAL_STYLE_BLUE, "{}", module_name); From 09708045cfc8c6467c008c2b69627fb08e2dcf7e Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Wed, 8 Jan 2025 12:46:56 +0100 Subject: [PATCH 8/8] clang-format Signed-off-by: Kai-Uwe Hermann --- lib/config.cpp | 3 ++- lib/module_config.cpp | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/config.cpp b/lib/config.cpp index 166108c3..a4c0dd13 100644 --- a/lib/config.cpp +++ b/lib/config.cpp @@ -104,7 +104,8 @@ static ParsedConfigMap parse_config_map(const json& config_map_schema, const jso throw ConfigParseException(ConfigParseException::SCHEMA, config_entry_name, err.what()); } - parsed_config_map[config_entry_name] = json::object({{"value", config_entry_value}, {"type", config_entry.at("type")}}); + parsed_config_map[config_entry_name] = + json::object({{"value", config_entry_value}, {"type", config_entry.at("type")}}); } return {parsed_config_map, unknown_config_entries}; diff --git a/lib/module_config.cpp b/lib/module_config.cpp index aad5c476..ccca8915 100644 --- a/lib/module_config.cpp +++ b/lib/module_config.cpp @@ -114,7 +114,6 @@ json get_module_config(std::shared_ptr mqtt, const std::string& const auto module_config_cache = mqtt->get(module_config_cache_topic, QOS::QOS2); result["module_config_cache"] = module_config_cache; - return result; }