From 0962fe3b981d5413621e491406345e3663023753 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Tue, 10 Dec 2024 14:11:28 +0100 Subject: [PATCH 01/39] Add performance checks to clang-tidy config Signed-off-by: Kai-Uwe Hermann --- .clang-tidy | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.clang-tidy b/.clang-tidy index 8f2e4abb..1d046d0d 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,6 +1,8 @@ Checks: > bugprone*, misc-const-correctness, + performance*, + -performance-avoid-endl, -llvmlibc*, -fuchsia-overloaded-operator, -fuchsia-statically-constructed-objects, From 3f76a885bd6abac1bfa671c83c71104daf5403b6 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Tue, 10 Dec 2024 14:12:23 +0100 Subject: [PATCH 02/39] Log module startup time for JavaScript and Python modules as well Signed-off-by: Kai-Uwe Hermann --- everestjs/everestjs.cpp | 6 ++++++ everestpy/src/everest/module.cpp | 2 +- everestpy/src/everest/module.hpp | 7 +++++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/everestjs/everestjs.cpp b/everestjs/everestjs.cpp index 71cdb1c7..c55f3142 100644 --- a/everestjs/everestjs.cpp +++ b/everestjs/everestjs.cpp @@ -556,6 +556,8 @@ static Napi::Value is_condition_satisfied_req(const Requirement& req, const Napi static Napi::Value boot_module(const Napi::CallbackInfo& info) { BOOST_LOG_FUNCTION(); + const auto start_time = std::chrono::system_clock::now(); + auto env = info.Env(); auto available_handlers_prop = Napi::Object::New(env); @@ -946,6 +948,10 @@ static Napi::Value boot_module(const Napi::CallbackInfo& info) { ctx->js_cb = std::make_unique(env, callback_wrapper); ctx->everest->register_on_ready_handler(framework_ready_handler); + const auto end_time = std::chrono::system_clock::now(); + EVLOG_info << "Module " << fmt::format(Everest::TERMINAL_STYLE_BLUE, "{}", module_id) << " initialized [" + << std::chrono::duration_cast(end_time - start_time).count() << "ms]"; + ctx->everest->spawn_main_loop_thread(); } catch (std::exception& e) { diff --git a/everestpy/src/everest/module.cpp b/everestpy/src/everest/module.cpp index 82760241..40fdcadc 100644 --- a/everestpy/src/everest/module.cpp +++ b/everestpy/src/everest/module.cpp @@ -21,7 +21,7 @@ Module::Module(const RuntimeSession& session) : Module(get_variable_from_env("EV } Module::Module(const std::string& module_id_, const RuntimeSession& session_) : - module_id(module_id_), session(session_) { + module_id(module_id_), session(session_), start_time(std::chrono::system_clock::now()) { this->mqtt_abstraction = std::make_shared(session.get_mqtt_settings()); this->mqtt_abstraction->connect(); diff --git a/everestpy/src/everest/module.hpp b/everestpy/src/everest/module.hpp index ffd801a7..8d84ca04 100644 --- a/everestpy/src/everest/module.hpp +++ b/everestpy/src/everest/module.hpp @@ -3,6 +3,7 @@ #ifndef EVERESTPY_MODULE_HPP #define EVERESTPY_MODULE_HPP +#include #include #include #include @@ -28,6 +29,11 @@ class Module { handle->register_on_ready_handler(std::move(on_ready_handler)); } + const auto end_time = std::chrono::system_clock::now(); + EVLOG_info << "Module " << fmt::format(Everest::TERMINAL_STYLE_BLUE, "{}", this->module_id) << " initialized [" + << std::chrono::duration_cast(end_time - this->start_time).count() + << "ms]"; + handle->signal_ready(); } @@ -78,6 +84,7 @@ class Module { private: const std::string module_id; const RuntimeSession& session; + const std::chrono::time_point start_time; std::unique_ptr rs; std::shared_ptr mqtt_abstraction; std::unique_ptr config_; From 2b17dfb3f9990912ad4386228cd6df04d2e8ffb2 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Tue, 10 Dec 2024 14:18:50 +0100 Subject: [PATCH 03/39] Use const ref for a few more variables and parameters Signed-off-by: Kai-Uwe Hermann --- everestpy/src/everest/everestpy.cpp | 2 +- everestpy/src/everest/module.cpp | 2 +- everestpy/src/everest/module.hpp | 6 +++--- include/framework/everest.hpp | 6 +++--- include/framework/runtime.hpp | 2 +- include/utils/message_queue.hpp | 4 ++-- include/utils/module_config.hpp | 2 +- include/utils/mqtt_abstraction.hpp | 4 ++-- include/utils/mqtt_abstraction_impl.hpp | 2 +- lib/config.cpp | 14 +++++++------- lib/everest.cpp | 21 +++++++++++---------- lib/message_queue.cpp | 6 +++--- lib/module_config.cpp | 2 +- lib/mqtt_abstraction.cpp | 3 ++- lib/mqtt_abstraction_impl.cpp | 3 ++- lib/runtime.cpp | 2 +- src/controller/command_api.cpp | 6 +++--- src/manager.cpp | 10 +++++----- 18 files changed, 50 insertions(+), 47 deletions(-) diff --git a/everestpy/src/everest/everestpy.cpp b/everestpy/src/everest/everestpy.cpp index 5ed43c01..fe38ed85 100644 --- a/everestpy/src/everest/everestpy.cpp +++ b/everestpy/src/everest/everestpy.cpp @@ -130,7 +130,7 @@ PYBIND11_MODULE(everestpy, m) { .def(py::init()) .def("say_hello", &Module::say_hello) .def("init_done", py::overload_cast<>(&Module::init_done)) - .def("init_done", py::overload_cast>(&Module::init_done)) + .def("init_done", py::overload_cast&>(&Module::init_done)) .def("call_command", &Module::call_command) .def("publish_variable", &Module::publish_variable) .def("implement_command", &Module::implement_command) diff --git a/everestpy/src/everest/module.cpp b/everestpy/src/everest/module.cpp index 40fdcadc..21ccc373 100644 --- a/everestpy/src/everest/module.cpp +++ b/everestpy/src/everest/module.cpp @@ -12,7 +12,7 @@ std::unique_ptr Module::create_everest_instance(const std::string& module_id, const Everest::Config& config, const Everest::RuntimeSettings& rs, - std::shared_ptr mqtt_abstraction) { + const std::shared_ptr& mqtt_abstraction) { return std::make_unique(module_id, config, rs.validate_schema, mqtt_abstraction, rs.telemetry_prefix, rs.telemetry_enabled); } diff --git a/everestpy/src/everest/module.hpp b/everestpy/src/everest/module.hpp index 8d84ca04..75152474 100644 --- a/everestpy/src/everest/module.hpp +++ b/everestpy/src/everest/module.hpp @@ -22,11 +22,11 @@ class Module { ModuleSetup say_hello(); - void init_done(std::function on_ready_handler) { + void init_done(const std::function& on_ready_handler) { this->handle->check_code(); if (on_ready_handler) { - handle->register_on_ready_handler(std::move(on_ready_handler)); + handle->register_on_ready_handler(on_ready_handler); } const auto end_time = std::chrono::system_clock::now(); @@ -101,7 +101,7 @@ class Module { static std::unique_ptr create_everest_instance(const std::string& module_id, const Everest::Config& config, const Everest::RuntimeSettings& rs, - std::shared_ptr mqtt_abstraction); + const std::shared_ptr& mqtt_abstraction); ModuleInfo module_info{}; std::map requirements; diff --git a/include/framework/everest.hpp b/include/framework/everest.hpp index e093125e..38feb466 100644 --- a/include/framework/everest.hpp +++ b/include/framework/everest.hpp @@ -49,7 +49,7 @@ struct ErrorFactory; class Everest { public: Everest(std::string module_id, const Config& config, bool validate_data_with_schema, - std::shared_ptr mqtt_abstraction, const std::string& telemetry_prefix, + const std::shared_ptr& mqtt_abstraction, const std::string& telemetry_prefix, bool telemetry_enabled); // forbid copy assignment and copy construction @@ -66,7 +66,7 @@ class Everest { /// /// \brief Allows a module to indicate that it provides the given command \p cmd /// - void provide_cmd(const std::string impl_id, const std::string cmd_name, const JsonCommand handler); + void provide_cmd(const std::string& impl_id, const std::string cmd_name, const JsonCommand& handler); void provide_cmd(const cmd& cmd); /// @@ -217,7 +217,7 @@ class Everest { bool telemetry_enabled; std::optional module_tier_mappings; - void handle_ready(nlohmann::json data); + void handle_ready(const nlohmann::json& data); void heartbeat(); diff --git a/include/framework/runtime.hpp b/include/framework/runtime.hpp index 3b7c4df0..4d48fb4a 100644 --- a/include/framework/runtime.hpp +++ b/include/framework/runtime.hpp @@ -194,7 +194,7 @@ class ModuleLoader { explicit ModuleLoader(int argc, char* argv[], ModuleCallbacks callbacks) : ModuleLoader(argc, argv, callbacks, {"undefined project", "undefined version", "undefined git version"}){}; explicit ModuleLoader(int argc, char* argv[], ModuleCallbacks callbacks, - const VersionInformation version_information); + const VersionInformation& version_information); int initialize(); }; diff --git a/include/utils/message_queue.hpp b/include/utils/message_queue.hpp index 58168764..44153d28 100644 --- a/include/utils/message_queue.hpp +++ b/include/utils/message_queue.hpp @@ -79,10 +79,10 @@ class MessageHandler { void stop(); /// \brief Adds a \p handler that will receive messages from the queue. - void add_handler(std::shared_ptr handler); + void add_handler(const std::shared_ptr& handler); /// \brief Removes a specific \p handler - void remove_handler(std::shared_ptr handler); + void remove_handler(const std::shared_ptr& handler); /// \brief \returns the number of registered handlers std::size_t count_handlers(); diff --git a/include/utils/module_config.hpp b/include/utils/module_config.hpp index 6808d2bd..a9c7ffa1 100644 --- a/include/utils/module_config.hpp +++ b/include/utils/module_config.hpp @@ -9,7 +9,7 @@ namespace Everest { /// \brief get config from manager via mqtt -nlohmann::json get_module_config(std::shared_ptr mqtt, const std::string& module_id); +nlohmann::json get_module_config(const std::shared_ptr& mqtt, const std::string& module_id); } // namespace Everest #endif // UTILS_MODULE_CONFIG_HPP diff --git a/include/utils/mqtt_abstraction.hpp b/include/utils/mqtt_abstraction.hpp index 5bff00d8..c7977b16 100644 --- a/include/utils/mqtt_abstraction.hpp +++ b/include/utils/mqtt_abstraction.hpp @@ -85,8 +85,8 @@ class MQTTAbstraction { std::shared_future get_main_loop_future(); /// - /// \copydoc MQTTAbstractionImpl::register_handler(const std::string&, std::shared_ptr, QOS) - void register_handler(const std::string& topic, std::shared_ptr handler, QOS qos); + /// \copydoc MQTTAbstractionImpl::register_handler(const std::string&, const std::shared_ptr&, QOS) + void register_handler(const std::string& topic, const std::shared_ptr& handler, QOS qos); /// /// \copydoc MQTTAbstractionImpl::unregister_handler(const std::string&, const Token&) diff --git a/include/utils/mqtt_abstraction_impl.hpp b/include/utils/mqtt_abstraction_impl.hpp index b1e47321..55efb09d 100644 --- a/include/utils/mqtt_abstraction_impl.hpp +++ b/include/utils/mqtt_abstraction_impl.hpp @@ -96,7 +96,7 @@ class MQTTAbstractionImpl { /// /// \brief subscribes to the given \p topic and registers a callback \p handler that is called when a message /// arrives on the topic. With \p qos a MQTT Quality of Service level can be set. - void register_handler(const std::string& topic, std::shared_ptr handler, QOS qos); + void register_handler(const std::string& topic, const std::shared_ptr& handler, QOS qos); /// /// \brief unsubscribes a handler identified by its \p token from the given \p topic diff --git a/lib/config.cpp b/lib/config.cpp index 7a097dcf..3077b034 100644 --- a/lib/config.cpp +++ b/lib/config.cpp @@ -78,8 +78,8 @@ static ParsedConfigMap parse_config_map(const json& config_map_schema, const jso // validate each config entry for (const auto& config_entry_el : config_map_schema.items()) { - const std::string config_entry_name = config_entry_el.key(); - const json config_entry = config_entry_el.value(); + const std::string& config_entry_name = config_entry_el.key(); + const json& config_entry = config_entry_el.value(); // only convenience exception, would be catched by schema validation below if not thrown here if (!config_entry.contains("default") and !config_map.contains(config_entry_name)) { @@ -125,7 +125,7 @@ static auto get_provides_for_probe_module(const std::string& probe_module_id, co const auto& connections = module_config.value("connections", json::object()); for (const auto& connection : connections.items()) { - const std::string req_id = connection.key(); + const std::string& req_id = connection.key(); const std::string module_name = module_config.at("module"); const auto& module_manifest = manifests.at(module_name); @@ -161,7 +161,7 @@ static auto get_provides_for_probe_module(const std::string& probe_module_id, co static auto get_requirements_for_probe_module(const std::string& probe_module_id, const json& config, const json& manifests) { - const auto probe_module_config = config.at(probe_module_id); + const auto& probe_module_config = config.at(probe_module_id); auto requirements = json::object(); @@ -579,7 +579,7 @@ void ManagerConfig::load_and_validate_manifest(const std::string& module_id, con // validate config for !module { - const json config_map = module_config.at("config_module"); + const json& config_map = module_config.at("config_module"); const json config_map_schema = this->manifests[module_config.at("module").get()]["config"]; try { @@ -1167,7 +1167,7 @@ ModuleConfigs Config::get_module_configs(const std::string& module_id) const { for (const auto& entry : conf_map.value().items()) { const json entry_type = config_schema.at(entry.key()).at("type"); ConfigEntry value; - const json data = entry.value(); + const json& data = entry.value(); if (data.is_string()) { value = data.get(); @@ -1239,7 +1239,7 @@ void Config::ref_loader(const json_uri& uri, json& schema) { schema = nlohmann::json_schema::draft7_schema_builtin; return; } else { - const auto path = uri.path(); + const auto& path = uri.path(); if (this->types.contains(path)) { schema = this->types[path]; EVLOG_verbose << fmt::format("ref path \"{}\" schema has been found.", path); diff --git a/lib/everest.cpp b/lib/everest.cpp index 09aa0940..cea88580 100644 --- a/lib/everest.cpp +++ b/lib/everest.cpp @@ -39,7 +39,7 @@ const auto remote_cmd_res_timeout_seconds = 300; const std::array TELEMETRY_RESERVED_KEYS = {{"connector_id"}}; Everest::Everest(std::string module_id_, const Config& config_, bool validate_data_with_schema, - std::shared_ptr mqtt_abstraction, const std::string& telemetry_prefix, + const std::shared_ptr& mqtt_abstraction, const std::string& telemetry_prefix, bool telemetry_enabled) : mqtt_abstraction(mqtt_abstraction), config(config_), @@ -71,7 +71,8 @@ Everest::Everest(std::string module_id_, const Config& config_, bool validate_da // setup error_manager_req_global if enabled + error_database + error_state_monitor if (this->module_manifest.contains("enable_global_errors") && this->module_manifest.at("enable_global_errors").get()) { - std::shared_ptr global_error_database = std::make_shared(); + const std::shared_ptr& global_error_database = + std::make_shared(); const error::ErrorManagerReqGlobal::SubscribeGlobalAllErrorsFunc subscribe_global_all_errors_func = [this](const error::ErrorCallback& callback, const error::ErrorCallback& clear_callback) { this->subscribe_global_all_errors(callback, clear_callback); @@ -90,7 +91,7 @@ Everest::Everest(std::string module_id_, const Config& config_, bool validate_da // setup error_managers, error_state_monitors, error_factories and error_databases for all implementations for (const std::string& impl : Config::keys(this->module_manifest.at("provides"))) { // setup shared database - std::shared_ptr error_database = std::make_shared(); + const std::shared_ptr error_database = std::make_shared(); // setup error manager const std::string interface_name = this->module_manifest.at("provides").at(impl).at("interface"); @@ -188,7 +189,7 @@ Everest::Everest(std::string module_id_, const Config& config_, bool validate_da } // register handler for global ready signal - const auto handle_ready_wrapper = [this](const std::string&, json data) { this->handle_ready(data); }; + const auto handle_ready_wrapper = [this](const std::string&, const json& data) { this->handle_ready(data); }; const auto everest_ready = std::make_shared(HandlerType::ExternalMQTT, std::make_shared(handle_ready_wrapper)); this->mqtt_abstraction->register_handler(fmt::format("{}ready", mqtt_everest_prefix), everest_ready, QOS::QOS2); @@ -265,7 +266,7 @@ void Everest::check_code() { this->config.get_manifests()[this->config.get_main_config()[this->module_id]["module"].get()]; for (const auto& element : module_manifest.at("provides").items()) { const auto& impl_id = element.key(); - const auto impl_manifest = element.value(); + const auto& impl_manifest = element.value(); const auto interface_definition = this->config.get_interface_definition(impl_manifest.at("interface")); std::set cmds_not_registered; @@ -666,13 +667,13 @@ void Everest::subscribe_global_all_errors(const error::ErrorCallback& callback, for (const auto& [module_id, module_name] : this->config.get_module_names()) { const json provides = this->config.get_manifests().at(module_name).at("provides"); for (const auto& impl : provides.items()) { - const std::string impl_id = impl.key(); + const std::string& impl_id = impl.key(); const std::string interface = impl.value().at("interface"); const json errors = this->config.get_interface_definition(interface).at("errors"); for (const auto& error_namespace_it : errors.items()) { - const std::string error_type_namespace = error_namespace_it.key(); + const std::string& error_type_namespace = error_namespace_it.key(); for (const auto& error_name_it : error_namespace_it.value().items()) { - const std::string error_type_name = error_name_it.key(); + const std::string& error_type_name = error_name_it.key(); const std::string raise_topic = fmt::format("{}/error/{}/{}", this->config.mqtt_prefix(module_id, impl_id), error_type_namespace, error_type_name); @@ -784,7 +785,7 @@ void Everest::signal_ready() { /// \brief Ready handler for global readyness (e.g. all modules are ready now). /// This will called when receiving the global ready signal from manager. /// -void Everest::handle_ready(json data) { +void Everest::handle_ready(const json& data) { BOOST_LOG_FUNCTION(); EVLOG_debug << fmt::format("handle_ready: {}", data.dump()); @@ -818,7 +819,7 @@ void Everest::handle_ready(json data) { // this->heartbeat_thread = std::thread(&Everest::heartbeat, this); } -void Everest::provide_cmd(const std::string impl_id, const std::string cmd_name, const JsonCommand handler) { +void Everest::provide_cmd(const std::string& impl_id, const std::string cmd_name, const JsonCommand& handler) { BOOST_LOG_FUNCTION(); // extract manifest definition of this command diff --git a/lib/message_queue.cpp b/lib/message_queue.cpp index f92ea7ea..d9903722 100644 --- a/lib/message_queue.cpp +++ b/lib/message_queue.cpp @@ -70,7 +70,7 @@ MessageHandler::MessageHandler() : running(true) { std::vector> local_handlers; { const std::lock_guard handlers_lock(handler_list_mutex); - for (auto handler : this->handlers) { + for (const auto& handler : this->handlers) { local_handlers.push_back(handler); } } @@ -129,14 +129,14 @@ void MessageHandler::stop() { this->cv.notify_all(); } -void MessageHandler::add_handler(std::shared_ptr handler) { +void MessageHandler::add_handler(const std::shared_ptr& handler) { { const std::lock_guard lock(this->handler_list_mutex); this->handlers.insert(handler); } } -void MessageHandler::remove_handler(std::shared_ptr handler) { +void MessageHandler::remove_handler(const std::shared_ptr& handler) { { const std::lock_guard lock(this->handler_list_mutex); auto it = std::find(this->handlers.begin(), this->handlers.end(), handler); diff --git a/lib/module_config.cpp b/lib/module_config.cpp index f92e80fe..644a2f6c 100644 --- a/lib/module_config.cpp +++ b/lib/module_config.cpp @@ -16,7 +16,7 @@ using json = nlohmann::json; inline constexpr int mqtt_get_config_timeout_ms = 5000; -json get_module_config(std::shared_ptr mqtt, const std::string& module_id) { +json get_module_config(const std::shared_ptr& mqtt, const std::string& module_id) { const auto& everest_prefix = mqtt->get_everest_prefix(); const auto get_config_topic = fmt::format("{}modules/{}/get_config", everest_prefix, module_id); diff --git a/lib/mqtt_abstraction.cpp b/lib/mqtt_abstraction.cpp index 111e00db..fd342a4f 100644 --- a/lib/mqtt_abstraction.cpp +++ b/lib/mqtt_abstraction.cpp @@ -96,7 +96,8 @@ std::shared_future MQTTAbstraction::get_main_loop_future() { return mqtt_abstraction->get_main_loop_future(); } -void MQTTAbstraction::register_handler(const std::string& topic, std::shared_ptr handler, QOS qos) { +void MQTTAbstraction::register_handler(const std::string& topic, const std::shared_ptr& handler, + QOS qos) { BOOST_LOG_FUNCTION(); mqtt_abstraction->register_handler(topic, handler, qos); } diff --git a/lib/mqtt_abstraction_impl.cpp b/lib/mqtt_abstraction_impl.cpp index a3b50553..3c2f2dab 100644 --- a/lib/mqtt_abstraction_impl.cpp +++ b/lib/mqtt_abstraction_impl.cpp @@ -409,7 +409,8 @@ void MQTTAbstractionImpl::on_mqtt_disconnect() { EVLOG_AND_THROW(EverestInternalError("Lost connection to MQTT broker")); } -void MQTTAbstractionImpl::register_handler(const std::string& topic, std::shared_ptr handler, QOS qos) { +void MQTTAbstractionImpl::register_handler(const std::string& topic, const std::shared_ptr& handler, + QOS qos) { BOOST_LOG_FUNCTION(); switch (handler->type) { diff --git a/lib/runtime.cpp b/lib/runtime.cpp index eaf53ae2..cdc6bca5 100644 --- a/lib/runtime.cpp +++ b/lib/runtime.cpp @@ -397,7 +397,7 @@ ModuleCallbacks::ModuleCallbacks( } ModuleLoader::ModuleLoader(int argc, char* argv[], ModuleCallbacks callbacks, - const VersionInformation version_information) : + const VersionInformation& version_information) : runtime_settings(nullptr), callbacks(callbacks), version_information(version_information) { if (!this->parse_command_line(argc, argv)) { this->should_exit = true; diff --git a/src/controller/command_api.cpp b/src/controller/command_api.cpp index 4d4e58d4..88a9aed8 100644 --- a/src/controller/command_api.cpp +++ b/src/controller/command_api.cpp @@ -28,7 +28,7 @@ nlohmann::json CommandApi::handle(const std::string& cmd, const json& params) { if (cmd == "get_modules") { auto modules_list = json::object(); - for (const auto item : fs::directory_iterator(this->config.module_dir)) { + for (const auto& item : fs::directory_iterator(this->config.module_dir)) { if (!fs::is_directory(item)) { continue; } @@ -48,7 +48,7 @@ nlohmann::json CommandApi::handle(const std::string& cmd, const json& params) { } else if (cmd == "get_configs") { auto config_list = json::object(); - for (const auto item : fs::directory_iterator(this->config.configs_dir)) { + for (const auto& item : fs::directory_iterator(this->config.configs_dir)) { if (!fs::is_regular_file(item)) { continue; } @@ -65,7 +65,7 @@ nlohmann::json CommandApi::handle(const std::string& cmd, const json& params) { } else if (cmd == "get_interfaces") { auto interface_list = json::object(); - for (const auto item : fs::directory_iterator(this->config.interface_dir)) { + for (const auto& item : fs::directory_iterator(this->config.interface_dir)) { if (!fs::is_regular_file(item)) { continue; diff --git a/src/manager.cpp b/src/manager.cpp index 908a352f..0baa513c 100644 --- a/src/manager.cpp +++ b/src/manager.cpp @@ -229,7 +229,7 @@ static std::map spawn_modules(const std::vector started_modules; - const auto rs = ms.get_runtime_settings(); + const auto& rs = ms.get_runtime_settings(); for (const auto& module : modules) { @@ -267,7 +267,7 @@ std::mutex modules_ready_mutex; void cleanup_retained_topics(ManagerConfig& config, MQTTAbstraction& mqtt_abstraction, const std::string& mqtt_everest_prefix) { - const auto interface_definitions = config.get_interface_definitions(); + const auto& interface_definitions = config.get_interface_definitions(); mqtt_abstraction.publish(fmt::format("{}interfaces", mqtt_everest_prefix), std::string(), QOS::QOS2, true); @@ -405,7 +405,7 @@ static std::map start_modules(ManagerConfig& config, MQTTAbs const Handler module_ready_handler = [module_name, &mqtt_abstraction, &config, standalone_modules, mqtt_everest_prefix = ms.mqtt_settings.everest_prefix, - &status_fifo](const std::string&, nlohmann::json json) { + &status_fifo](const std::string&, const nlohmann::json& json) { EVLOG_debug << fmt::format("received module ready signal for module: {}({})", module_name, json.dump()); const std::unique_lock lock(modules_ready_mutex); // FIXME (aw): here are race conditions, if the ready handler gets called while modules are shut down! @@ -449,7 +449,7 @@ static std::map start_modules(ManagerConfig& config, MQTTAbs 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&, nlohmann::json json) { + &mqtt_abstraction](const std::string&, const nlohmann::json& json) { mqtt_abstraction.publish(config_topic, serialized_mod_config.dump()); }; @@ -720,7 +720,7 @@ int boot(const po::variables_map& vm) { const auto& main_config = config->get_main_config(); for (const auto& module : main_config.items()) { - const std::string module_id = module.key(); + const std::string& module_id = module.key(); // check if standalone parameter is set const auto& module_config = main_config.at(module_id); if (module_config.value("standalone", false)) { From f5d43332b8d84c7f2bd0ecc2cc9f83bfbde9d3a8 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Tue, 10 Dec 2024 14:29:04 +0100 Subject: [PATCH 04/39] 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 --- 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 3077b034..2efed0d7 100644 --- a/lib/config.cpp +++ b/lib/config.cpp @@ -1087,10 +1087,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 0baa513c..1889e842 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 9d4da5697db4be4359a3bb198abba03493735f19 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Tue, 10 Dec 2024 17:22:59 +0100 Subject: [PATCH 05/39] Refactor schema loading and validation Now when a schema is loaded a validator for it is instantiated as well Then validators can then be re-used instead of instantiating new ones for the same schema multiple times Signed-off-by: Kai-Uwe Hermann --- include/utils/config.hpp | 56 +++++++++------ lib/config.cpp | 146 +++++++++++++++++++++------------------ lib/everest.cpp | 11 ++- 3 files changed, 118 insertions(+), 95 deletions(-) diff --git a/include/utils/config.hpp b/include/utils/config.hpp index 2230c22d..40f38859 100644 --- a/include/utils/config.hpp +++ b/include/utils/config.hpp @@ -30,7 +30,7 @@ struct RuntimeSettings; /// /// \brief A structure that contains all available schemas /// -struct schemas { +struct Schemas { nlohmann::json config; ///< The config schema nlohmann::json manifest; ///< The manifest scheme nlohmann::json interface; ///< The interface schema @@ -38,6 +38,19 @@ struct schemas { nlohmann::json error_declaration_list; ///< The error-declaration-list schema }; +struct Validators { + std::unique_ptr config; + std::unique_ptr manifest; + std::unique_ptr type; + std::unique_ptr interface; + std::unique_ptr error_declaration_list; +}; + +struct SchemaValidation { + Schemas schemas; + Validators validators; +}; + /// /// \brief Allowed format of a type URI, which are of a format like this /type_file_name#/TypeName /// @@ -50,6 +63,17 @@ struct ImplementationInfo { std::string impl_intf; }; +/// +/// \brief A simple json schema loader that uses the builtin draft7 schema of +/// the json schema validator when it encounters it, throws an exception +/// otherwise +void loader(const nlohmann::json_uri& uri, nlohmann::json& schema); + +/// +/// \brief An extension to the default format checker of the json schema +/// validator supporting uris +void format_checker(const std::string& format, const std::string& value); + /// /// \brief Base class for configs /// @@ -62,7 +86,7 @@ class ConfigBase { nlohmann::json interfaces; nlohmann::json interface_definitions; nlohmann::json types; - schemas _schemas; + Schemas schemas; std::unordered_map tier_mappings; // experimental caches @@ -190,6 +214,8 @@ class ManagerConfig : public ConfigBase { private: const ManagerSettings& ms; std::unordered_map> telemetry_configs; + Validators validators; + std::unique_ptr draft7_validator; /// /// \brief loads and validates the manifest of the module \p module_id using the provided \p module config @@ -328,14 +354,15 @@ class Config : public ConfigBase { /// \brief loads the config.json and manifest.json in the schemes subfolder of /// the provided \p schemas_dir /// - /// \returns the config and manifest schemas - static schemas load_schemas(const fs::path& schemas_dir); + /// \returns the loaded configs and related validators + static SchemaValidation load_schemas(const fs::path& schemas_dir); /// /// \brief loads and validates a json schema at the provided \p path /// - /// \returns the loaded json schema as a json object - static nlohmann::json load_schema(const fs::path& path); + /// \returns the loaded json schema as a json object as well as a related schema validator + static std::tuple> + load_schema(const fs::path& path); /// /// \brief loads all module manifests relative to the \p main_dir @@ -348,25 +375,14 @@ class Config : public ConfigBase { /// /// \returns a set of object keys static std::set keys(const nlohmann::json& object); - - /// - /// \brief A simple json schema loader that uses the builtin draft7 schema of - /// the json schema validator when it encounters it, throws an exception - /// otherwise - static void loader(const nlohmann::json_uri& uri, nlohmann::json& schema); - - /// - /// \brief An extension to the default format checker of the json schema - /// validator supporting uris - static void format_checker(const std::string& format, const std::string& value); }; } // namespace Everest NLOHMANN_JSON_NAMESPACE_BEGIN -template <> struct adl_serializer { - static void to_json(nlohmann::json& j, const Everest::schemas& s); +template <> struct adl_serializer { + static void to_json(nlohmann::json& j, const Everest::Schemas& s); - static void from_json(const nlohmann::json& j, Everest::schemas& s); + static void from_json(const nlohmann::json& j, Everest::Schemas& s); }; NLOHMANN_JSON_NAMESPACE_END diff --git a/lib/config.cpp b/lib/config.cpp index 2efed0d7..059c11e0 100644 --- a/lib/config.cpp +++ b/lib/config.cpp @@ -48,9 +48,37 @@ struct ParsedConfigMap { std::set unknown_config_entries; }; +void loader(const json_uri& uri, json& schema) { + BOOST_LOG_FUNCTION(); + + if (uri.location() == "http://json-schema.org/draft-07/schema") { + schema = nlohmann::json_schema::draft7_schema_builtin; + return; + } + + // TODO(kai): think about supporting more urls here + EVTHROW(EverestInternalError(fmt::format("{} is not supported for schema loading at the moment\n", uri.url()))); +} + +void format_checker(const std::string& format, const std::string& value) { + BOOST_LOG_FUNCTION(); + + if (format == "uri") { + if (value.find("://") == std::string::npos) { + EVTHROW(std::invalid_argument("URI does not contain :// - invalid")); + } + } else if (format == "uri-reference") { + if (!std::regex_match(value, type_uri_regex)) { + EVTHROW(std::invalid_argument("Type URI is malformed.")); + } + } else { + nlohmann::json_schema::default_string_format_check(format, value); + } +} + static void validate_config_schema(const json& config_map_schema) { // iterate over every config entry - json_validator validator(Config::loader, Config::format_checker); + json_validator validator(loader, format_checker); for (const auto& config_item : config_map_schema.items()) { if (!config_item.value().contains("default")) { continue; @@ -92,7 +120,7 @@ static ParsedConfigMap parse_config_map(const json& config_map_schema, const jso } else if (config_entry.contains("default")) { config_entry_value = config_entry.at("default"); // use default value defined in manifest } - json_validator validator(Config::loader, Config::format_checker); + json_validator validator(loader, format_checker); validator.set_root_schema(config_entry); try { auto patch = validator.validate(config_entry_value); @@ -339,7 +367,7 @@ const json& ConfigBase::get_settings() const { const json ConfigBase::get_schemas() const { BOOST_LOG_FUNCTION(); - return this->_schemas; + return this->schemas; } json ConfigBase::get_error_types() { @@ -483,9 +511,7 @@ void ManagerConfig::load_and_validate_manifest(const std::string& module_id, con this->manifests[module_name] = load_yaml(manifest_path); } - json_validator validator(Config::loader, Config::format_checker); - validator.set_root_schema(this->_schemas.manifest); - const auto patch = validator.validate(this->manifests[module_name]); + const auto patch = this->validators.manifest->validate(this->manifests[module_name]); if (!patch.is_null()) { // extend manifest with default values this->manifests[module_name] = this->manifests[module_name].patch(patch); @@ -613,7 +639,7 @@ std::tuple ManagerConfig::load_and_validate_with_schema(const fs: int64_t validation_ms = 0; const auto start_time_validate = std::chrono::system_clock::now(); - json_validator validator(Config::loader, Config::format_checker); + json_validator validator(loader, format_checker); validator.set_root_schema(schema); validator.validate(json_to_validate); const auto end_time_validate = std::chrono::system_clock::now(); @@ -647,9 +673,7 @@ json ManagerConfig::load_interface_file(const std::string& intf_name) { // this subschema can not use allOf with the draft-07 schema because that will cause our validator to // add all draft-07 default values which never validate (the {"not": true} default contradicts everything) // --> validating against draft-07 will be done in an extra step below - json_validator validator(Config::loader, Config::format_checker); - validator.set_root_schema(this->_schemas.interface); - auto patch = validator.validate(interface_json); + auto patch = this->validators.interface->validate(interface_json); if (!patch.is_null()) { // extend config entry with default values interface_json = interface_json.patch(patch); @@ -662,7 +686,6 @@ json ManagerConfig::load_interface_file(const std::string& intf_name) { } // validate every cmd arg/result and var definition against draft-07 schema - validator.set_root_schema(draft07); for (auto& var_entry : interface_json["vars"].items()) { auto& var_value = var_entry.value(); // erase "description" @@ -683,7 +706,7 @@ json ManagerConfig::load_interface_file(const std::string& intf_name) { } } } - validator.validate(var_value); + this->draft7_validator->validate(var_value); } for (auto& cmd_entry : interface_json["cmds"].items()) { auto& cmd = interface_json["cmds"][cmd_entry.key()]; @@ -697,14 +720,14 @@ json ManagerConfig::load_interface_file(const std::string& intf_name) { if (arg_entry.contains("description")) { arg_entry.erase("description"); } - validator.validate(arg_entry); + this->draft7_validator->validate(arg_entry); } auto& result = interface_json["cmds"][cmd_entry.key()]["result"]; // erase "description" if (result.contains("description")) { result.erase("description"); } - validator.validate(result); + this->draft7_validator->validate(result); } return interface_json; @@ -903,7 +926,7 @@ void ManagerConfig::parse(json config) { EVLOG_verbose << fmt::format("Loading type file at: {}", fs::canonical(type_file_path).c_str()); const auto [type_json, validate_ms] = - load_and_validate_with_schema(type_file_path, this->_schemas.type); + load_and_validate_with_schema(type_file_path, this->schemas.type); total_time_validation_ms += validate_ms; this->types[type_path] = type_json.at("types"); @@ -934,7 +957,7 @@ void ManagerConfig::parse(json config) { EVLOG_verbose << fmt::format("Loading error file at: {}", fs::canonical(error_file_path).c_str()); const auto [error_json, validate_ms] = - load_and_validate_with_schema(error_file_path, this->_schemas.error_declaration_list); + load_and_validate_with_schema(error_file_path, this->schemas.error_declaration_list); total_time_validation_ms += validate_ms; } catch (const std::exception& e) { @@ -1048,8 +1071,12 @@ ManagerConfig::ManagerConfig(const ManagerSettings& ms) : ConfigBase(ms.mqtt_set this->interfaces = json({}); this->interface_definitions = json({}); this->types = json({}); - this->_schemas = Config::load_schemas(this->ms.schemas_dir); + auto schema_validation = Config::load_schemas(this->ms.schemas_dir); + this->schemas = schema_validation.schemas; + this->validators = std::move(schema_validation.validators); this->error_map = error::ErrorTypeMap(this->ms.errors_dir); + this->draft7_validator = std::make_unique(loader, format_checker); + this->draft7_validator->set_root_schema(draft07); // load and process config file const fs::path config_path = this->ms.config_file; @@ -1071,9 +1098,7 @@ ManagerConfig::ManagerConfig(const ManagerSettings& ms) : ConfigBase(ms.mqtt_set EVLOG_verbose << "No user-config provided."; } - json_validator validator(Config::loader, Config::format_checker); - validator.set_root_schema(this->_schemas.config); - const auto patch = validator.validate(complete_config); + const auto patch = this->validators.config->validate(complete_config); if (!patch.is_null()) { // extend config with default values complete_config = complete_config.patch(patch); @@ -1114,7 +1139,7 @@ Config::Config(const MQTTSettings& mqtt_settings, json serialized_config) : Conf this->telemetry_config = serialized_config.at("telemetry_config"); } - this->_schemas = serialized_config.at("schemas"); + this->schemas = serialized_config.at("schemas"); this->error_map = error::ErrorTypeMap(); this->error_map.load_error_types_map(serialized_config.at("error_map")); } @@ -1249,21 +1274,33 @@ void Config::ref_loader(const json_uri& uri, json& schema) { EVTHROW(EverestInternalError(fmt::format("{} is not supported for schema loading at the moment\n", uri.url()))); } -schemas Config::load_schemas(const fs::path& schemas_dir) { +SchemaValidation Config::load_schemas(const fs::path& schemas_dir) { BOOST_LOG_FUNCTION(); - schemas schemas; + SchemaValidation schema_validation; EVLOG_debug << fmt::format("Loading base schema files for config and manifests... from: {}", schemas_dir.string()); - schemas.config = Config::load_schema(schemas_dir / "config.yaml"); - schemas.manifest = Config::load_schema(schemas_dir / "manifest.yaml"); - schemas.interface = Config::load_schema(schemas_dir / "interface.yaml"); - schemas.type = Config::load_schema(schemas_dir / "type.yaml"); - schemas.error_declaration_list = Config::load_schema(schemas_dir / "error-declaration-list.yaml"); - - return schemas; + auto [config_schema, config_val] = Config::load_schema(schemas_dir / "config.yaml"); + schema_validation.schemas.config = config_schema; + schema_validation.validators.config = std::move(config_val); + auto [manifest_schema, manifest_val] = Config::load_schema(schemas_dir / "manifest.yaml"); + schema_validation.schemas.manifest = manifest_schema; + schema_validation.validators.manifest = std::move(manifest_val); + auto [interface_schema, interface_val] = Config::load_schema(schemas_dir / "interface.yaml"); + schema_validation.schemas.interface = interface_schema; + schema_validation.validators.interface = std::move(interface_val); + auto [type_schema, type_val] = Config::load_schema(schemas_dir / "type.yaml"); + schema_validation.schemas.type = type_schema; + schema_validation.validators.type = std::move(type_val); + auto [error_declaration_list_schema, error_declaration_list_val] = + Config::load_schema(schemas_dir / "error-declaration-list.yaml"); + schema_validation.schemas.error_declaration_list = error_declaration_list_schema; + schema_validation.validators.error_declaration_list = std::move(error_declaration_list_val); + + return schema_validation; } -json Config::load_schema(const fs::path& path) { +std::tuple> +Config::load_schema(const fs::path& path) { BOOST_LOG_FUNCTION(); if (!fs::exists(path)) { @@ -1273,18 +1310,19 @@ json Config::load_schema(const fs::path& path) { EVLOG_debug << fmt::format("Loading schema file at: {}", fs::canonical(path).string()); - const json schema = load_yaml(path); + json schema = load_yaml(path); - json_validator validator(Config::loader, Config::format_checker); + auto validator = std::make_unique(loader, format_checker); try { - validator.set_root_schema(schema); + validator->set_root_schema(schema); } catch (const std::exception& e) { EVLOG_AND_THROW(EverestInternalError( fmt::format("Validation of schema '{}' failed, here is why: {}", path.string(), e.what()))); } - return schema; + return std::make_tuple>( + std::move(schema), std::move(validator)); } json Config::load_all_manifests(const std::string& modules_dir, const std::string& schemas_dir) { @@ -1292,7 +1330,7 @@ json Config::load_all_manifests(const std::string& modules_dir, const std::strin json manifests = json({}); - const schemas schemas = Config::load_schemas(schemas_dir); + auto schema_validation = Config::load_schemas(schemas_dir); const fs::path modules_path = fs::path(modules_dir); @@ -1308,9 +1346,7 @@ json Config::load_all_manifests(const std::string& modules_dir, const std::strin try { manifests[module_name] = load_yaml(manifest_path); - json_validator validator(Config::loader, Config::format_checker); - validator.set_root_schema(schemas.manifest); - validator.validate(manifests.at(module_name)); + schema_validation.validators.manifest->validate(manifests.at(module_name)); } catch (const std::exception& e) { EVLOG_AND_THROW(EverestConfigError( fmt::format("Failed to load and parse module manifest file of module {}: {}", module_name, e.what()))); @@ -1340,38 +1376,10 @@ std::set Config::keys(const json& object) { return keys; } -void Config::loader(const json_uri& uri, json& schema) { - BOOST_LOG_FUNCTION(); - - if (uri.location() == "http://json-schema.org/draft-07/schema") { - schema = nlohmann::json_schema::draft7_schema_builtin; - return; - } - - // TODO(kai): think about supporting more urls here - EVTHROW(EverestInternalError(fmt::format("{} is not supported for schema loading at the moment\n", uri.url()))); -} - -void Config::format_checker(const std::string& format, const std::string& value) { - BOOST_LOG_FUNCTION(); - - if (format == "uri") { - if (value.find("://") == std::string::npos) { - EVTHROW(std::invalid_argument("URI does not contain :// - invalid")); - } - } else if (format == "uri-reference") { - if (!std::regex_match(value, type_uri_regex)) { - EVTHROW(std::invalid_argument("Type URI is malformed.")); - } - } else { - nlohmann::json_schema::default_string_format_check(format, value); - } -} - } // namespace Everest NLOHMANN_JSON_NAMESPACE_BEGIN -void adl_serializer::to_json(nlohmann::json& j, const Everest::schemas& s) { +void adl_serializer::to_json(nlohmann::json& j, const Everest::Schemas& s) { j = {{"config", s.config}, {"manifest", s.manifest}, {"interface", s.interface}, @@ -1379,7 +1387,7 @@ void adl_serializer::to_json(nlohmann::json& j, const Everest: {"error_declaration_list", s.error_declaration_list}}; } -void adl_serializer::from_json(const nlohmann::json& j, Everest::schemas& s) { +void adl_serializer::from_json(const nlohmann::json& j, Everest::Schemas& s) { s.config = j.at("config"); s.manifest = j.at("manifest"); s.interface = j.at("interface"); diff --git a/lib/everest.cpp b/lib/everest.cpp index cea88580..5622a7bd 100644 --- a/lib/everest.cpp +++ b/lib/everest.cpp @@ -348,7 +348,7 @@ json Everest::call_cmd(const Requirement& req, const std::string& cmd_name, json try { json_validator validator( [this](const json_uri& uri, json& schema) { this->config.ref_loader(uri, schema); }, - Config::format_checker); + format_checker); validator.set_root_schema(cmd_definition.at("arguments").at(arg_name)); validator.validate(json_args.at(arg_name)); } catch (const std::exception& e) { @@ -438,8 +438,7 @@ void Everest::publish_var(const std::string& impl_id, const std::string& var_nam const auto var_definition = impl_intf.at("vars").at(var_name); try { json_validator validator( - [this](const json_uri& uri, json& schema) { this->config.ref_loader(uri, schema); }, - Config::format_checker); + [this](const json_uri& uri, json& schema) { this->config.ref_loader(uri, schema); }, format_checker); validator.set_root_schema(var_definition); validator.validate(value); } catch (const std::exception& e) { @@ -493,7 +492,7 @@ void Everest::subscribe_var(const Requirement& req, const std::string& var_name, try { json_validator validator( [this](const json_uri& uri, json& schema) { this->config.ref_loader(uri, schema); }, - Config::format_checker); + format_checker); validator.set_root_schema(requirement_manifest_vardef); validator.validate(data); } catch (const std::exception& e) { @@ -858,7 +857,7 @@ void Everest::provide_cmd(const std::string& impl_id, const std::string cmd_name } json_validator validator( [this](const json_uri& uri, json& schema) { this->config.ref_loader(uri, schema); }, - Config::format_checker); + format_checker); validator.set_root_schema(cmd_definition.at("arguments").at(arg_name)); validator.validate(data.at("args").at(arg_name)); } @@ -884,7 +883,7 @@ void Everest::provide_cmd(const std::string& impl_id, const std::string cmd_name (!cmd_definition.contains("result") || cmd_definition.at("result").is_null()))) { json_validator validator( [this](const json_uri& uri, json& schema) { this->config.ref_loader(uri, schema); }, - Config::format_checker); + format_checker); validator.set_root_schema(cmd_definition.at("result")); validator.validate(res_data.at("retval")); } From 9d5496ccac8a12068a3af538891d288aa9017767 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Tue, 10 Dec 2024 17:24:15 +0100 Subject: [PATCH 06/39] Ad some moves that clang-tidy suggested Signed-off-by: Kai-Uwe Hermann --- everestpy/src/everest/misc.cpp | 2 +- include/framework/runtime.hpp | 3 ++- lib/config.cpp | 15 ++++++++------- lib/everest.cpp | 2 +- lib/runtime.cpp | 6 +++--- lib/types.cpp | 6 +++--- 6 files changed, 18 insertions(+), 16 deletions(-) diff --git a/everestpy/src/everest/misc.cpp b/everestpy/src/everest/misc.cpp index e2e948b8..d3a7d5a5 100644 --- a/everestpy/src/everest/misc.cpp +++ b/everestpy/src/everest/misc.cpp @@ -107,7 +107,7 @@ ModuleSetup create_setup_from_config(const std::string& module_id, Everest::Conf const auto& req_route = req_route_list[i]; const auto fulfillment = Fulfillment{req_route["module_id"], req_route["implementation_id"], {requirement_id, i}}; - fulfillment_list.emplace_back(std::move(fulfillment)); + fulfillment_list.emplace_back(fulfillment); } } diff --git a/include/framework/runtime.hpp b/include/framework/runtime.hpp index 4d48fb4a..5488328b 100644 --- a/include/framework/runtime.hpp +++ b/include/framework/runtime.hpp @@ -192,7 +192,8 @@ class ModuleLoader { public: explicit ModuleLoader(int argc, char* argv[], ModuleCallbacks callbacks) : - ModuleLoader(argc, argv, callbacks, {"undefined project", "undefined version", "undefined git version"}){}; + ModuleLoader(argc, argv, std::move(callbacks), + {"undefined project", "undefined version", "undefined git version"}){}; explicit ModuleLoader(int argc, char* argv[], ModuleCallbacks callbacks, const VersionInformation& version_information); diff --git a/lib/config.cpp b/lib/config.cpp index 059c11e0..3e27a93f 100644 --- a/lib/config.cpp +++ b/lib/config.cpp @@ -209,7 +209,7 @@ static auto get_requirements_for_probe_module(const std::string& probe_module_id if (module_config_it == config.end()) { EVLOG_AND_THROW( - EverestConfigError("ProbeModule refers to a non-existent module id '" + module_id + "'")); + EverestConfigError(fmt::format("ProbeModule refers to a non-existent module id '{}'", module_id))); } const auto& module_manifest = manifests.at(module_config_it->at("module")); @@ -217,14 +217,15 @@ static auto get_requirements_for_probe_module(const std::string& probe_module_id const auto& module_provides_it = module_manifest.find("provides"); if (module_provides_it == module_manifest.end()) { - EVLOG_AND_THROW(EverestConfigError("ProbeModule requires something from module id' " + module_id + - "', but it does not provide anything")); + EVLOG_AND_THROW(EverestConfigError(fmt::format( + "ProbeModule requires something from module id '{}' but it does not provide anything", module_id))); } const auto& provide_it = module_provides_it->find(impl_id); if (provide_it == module_provides_it->end()) { - EVLOG_AND_THROW(EverestConfigError("ProbeModule requires something from module id '" + module_id + - "', but it does not provide '" + impl_id + "'")); + EVLOG_AND_THROW(EverestConfigError( + fmt::format("ProbeModule requires something from module id '{}', but it does not provide '{}'", + module_id, impl_id))); } const std::string interface = provide_it->at("interface"); @@ -297,7 +298,7 @@ std::string create_printable_identifier(const ImplementationInfo& info, const st BOOST_LOG_FUNCTION(); // no implementation id yet so only return this kind of string: - const auto module_string = fmt::format("{}:{}", info.module_id, info.module_name); + auto module_string = fmt::format("{}:{}", info.module_id, info.module_name); if (impl_id.empty()) { return module_string; } @@ -910,7 +911,7 @@ void ManagerConfig::resolve_all_requirements() { } void ManagerConfig::parse(json config) { - this->main = config; + this->main = std::move(config); // load type files if (this->ms.runtime_settings->validate_schema) { int64_t total_time_validation_ms = 0, total_time_parsing_ms = 0; diff --git a/lib/everest.cpp b/lib/everest.cpp index 5622a7bd..09b06936 100644 --- a/lib/everest.cpp +++ b/lib/everest.cpp @@ -975,7 +975,7 @@ void Everest::provide_cmd(const cmd& cmd) { // call cmd handlers (handle async or normal handlers being both: // methods or functions) // FIXME (aw): this behaviour needs to be checked, i.e. how to distinguish in json between no value and null? - return handler(data).value_or(nullptr); + return handler(std::move(data)).value_or(nullptr); }); } diff --git a/lib/runtime.cpp b/lib/runtime.cpp index cdc6bca5..80797ac2 100644 --- a/lib/runtime.cpp +++ b/lib/runtime.cpp @@ -398,7 +398,7 @@ ModuleCallbacks::ModuleCallbacks( ModuleLoader::ModuleLoader(int argc, char* argv[], ModuleCallbacks callbacks, const VersionInformation& version_information) : - runtime_settings(nullptr), callbacks(callbacks), version_information(version_information) { + runtime_settings(nullptr), callbacks(std::move(callbacks)), version_information(version_information) { if (!this->parse_command_line(argc, argv)) { this->should_exit = true; return; @@ -478,11 +478,11 @@ int ModuleLoader::initialize() { ModuleAdapter module_adapter; module_adapter.call = [&everest](const Requirement& req, const std::string& cmd_name, Parameters args) { - return everest.call_cmd(req, cmd_name, args); + return everest.call_cmd(req, cmd_name, std::move(args)); }; module_adapter.publish = [&everest](const std::string& param1, const std::string& param2, Value param3) { - return everest.publish_var(param1, param2, param3); + return everest.publish_var(param1, param2, std::move(param3)); }; module_adapter.subscribe = [&everest](const Requirement& req, const std::string& var_name, diff --git a/lib/types.cpp b/lib/types.cpp index 19b0e30f..ba58c570 100644 --- a/lib/types.cpp +++ b/lib/types.cpp @@ -7,15 +7,15 @@ TypedHandler::TypedHandler(const std::string& name_, const std::string& id_, HandlerType type_, std::shared_ptr handler_) : - name(name_), id(id_), type(type_), handler(handler_) { + name(name_), id(id_), type(type_), handler(std::move(handler_)) { } TypedHandler::TypedHandler(const std::string& name_, HandlerType type_, std::shared_ptr handler_) : - TypedHandler(name_, "", type_, handler_) { + TypedHandler(name_, "", type_, std::move(handler_)) { } TypedHandler::TypedHandler(HandlerType type_, std::shared_ptr handler_) : - TypedHandler("", "", type_, handler_) { + TypedHandler("", "", type_, std::move(handler_)) { } bool operator<(const Requirement& lhs, const Requirement& rhs) { From a93bdfabba82c2990057b9c3f3f2c56a38eb42f9 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Tue, 10 Dec 2024 17:24:50 +0100 Subject: [PATCH 07/39] 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 3e27a93f..13693656 100644 --- a/lib/config.cpp +++ b/lib/config.cpp @@ -1105,9 +1105,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 c74055ba88f4d1e4d3e919b7d79ca4c507b860c7 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Tue, 10 Dec 2024 17:26:52 +0100 Subject: [PATCH 08/39] Improve yaml file loading performance by reserving the appropriate amount of space in the string the file content gets read into Signed-off-by: Kai-Uwe Hermann --- lib/yaml_loader.cpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/lib/yaml_loader.cpp b/lib/yaml_loader.cpp index fb4a576f..1b4ac079 100644 --- a/lib/yaml_loader.cpp +++ b/lib/yaml_loader.cpp @@ -78,6 +78,16 @@ static nlohmann::ordered_json ryml_to_nlohmann_json(const c4::yml::NodeRef& ryml } } +static std::string load_file_content(const std::filesystem::path& path) { + std::ifstream ifs(path.string()); + ifs.seekg(0, std::ios::end); + std::string content; + content.reserve(ifs.tellg()); + ifs.seekg(0); + content.assign(std::istreambuf_iterator(ifs), std::istreambuf_iterator()); + return content; +} + static std::string load_yaml_content(std::filesystem::path path) { namespace fs = std::filesystem; @@ -92,16 +102,14 @@ static std::string load_yaml_content(std::filesystem::path path) { // first check for yaml, if not found try fall back to json and evlog debug deprecated if (fs::exists(path)) { - std::ifstream ifs(path.string()); - return std::string(std::istreambuf_iterator(ifs), std::istreambuf_iterator()); + return load_file_content(path); } path.replace_extension(".json"); if (fs::exists(path)) { EVLOG_info << "Deprecated: loaded file in json format"; - std::ifstream ifs(path.string()); - return std::string(std::istreambuf_iterator(ifs), std::istreambuf_iterator()); + return load_file_content(path); } // failed to find yaml and json From b8385de724f363291eaaa4b51292193a3cfaf9fc Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Tue, 10 Dec 2024 17:27:50 +0100 Subject: [PATCH 09/39] Log the number of modules started Signed-off-by: Kai-Uwe Hermann --- src/manager.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/manager.cpp b/src/manager.cpp index 1889e842..e8913923 100644 --- a/src/manager.cpp +++ b/src/manager.cpp @@ -302,6 +302,8 @@ static std::map start_modules(ManagerConfig& config, MQTTAbs const auto& main_config = config.get_main_config(); const auto& module_names = config.get_module_names(); + const auto number_of_modules = main_config.size(); + EVLOG_info << "Starting " << number_of_modules << " modules"; modules_to_spawn.reserve(main_config.size()); const auto interface_definitions = config.get_interface_definitions(); From 0bc601b74c7d499a2eb1dd33e08178f13f83fede Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Tue, 10 Dec 2024 22:44:05 +0100 Subject: [PATCH 10/39] Fix narrowing conversion warning in yaml error handler by claming maximum length Signed-off-by: Kai-Uwe Hermann --- lib/yaml_loader.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/yaml_loader.cpp b/lib/yaml_loader.cpp index 1b4ac079..5ef781cf 100644 --- a/lib/yaml_loader.cpp +++ b/lib/yaml_loader.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include @@ -11,13 +12,18 @@ #include +static std::streamsize clamp(std::size_t len) { + return (len <= std::numeric_limits::max()) ? static_cast(len) + : std::numeric_limits::max(); +} + static void yaml_error_handler(const char* msg, std::size_t len, ryml::Location loc, void*) { std::stringstream error_msg; error_msg << "YAML parsing error: "; if (loc) { if (not loc.name.empty()) { - error_msg.write(loc.name.str, loc.name.len); + error_msg.write(loc.name.str, clamp(loc.name.len)); error_msg << ":"; } error_msg << loc.line << ":"; @@ -28,7 +34,7 @@ static void yaml_error_handler(const char* msg, std::size_t len, ryml::Location error_msg << " (" << loc.offset << "B):"; } } - error_msg.write(msg, len); + error_msg.write(msg, clamp(len)); throw std::runtime_error(error_msg.str()); } From bb1c2de95d39ea236cf9afdcbbbcc8f294804019 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Tue, 10 Dec 2024 22:44:44 +0100 Subject: [PATCH 11/39] Fix widening conversion warning by multiplying two std::size_t already Signed-off-by: Kai-Uwe Hermann --- include/utils/mqtt_abstraction_impl.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/utils/mqtt_abstraction_impl.hpp b/include/utils/mqtt_abstraction_impl.hpp index 55efb09d..85d2cb07 100644 --- a/include/utils/mqtt_abstraction_impl.hpp +++ b/include/utils/mqtt_abstraction_impl.hpp @@ -20,7 +20,7 @@ #include -constexpr std::size_t MQTT_BUF_SIZE = 500 * 1024; +constexpr std::size_t MQTT_BUF_SIZE = std::size_t{500} * std::size_t{1024}; namespace Everest { /// \brief Contains a payload and the topic it was received on with additional QOS From 0acb24fce9963f95269c864be78461b4f3851037 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Tue, 10 Dec 2024 22:45:49 +0100 Subject: [PATCH 12/39] Bump version to 0.19.2 Signed-off-by: Kai-Uwe Hermann --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e3d0acae..e3989639 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,7 +1,7 @@ cmake_minimum_required(VERSION 3.14) project(everest-framework - VERSION 0.19.1 + VERSION 0.19.2 DESCRIPTION "The open operating system for e-mobility charging stations" LANGUAGES CXX C ) From 9ddb9cee1a1d0922c2aaee4bf616a0ff5edda62a Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Wed, 11 Dec 2024 12:19:37 +0100 Subject: [PATCH 13/39] 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 644a2f6c..be844476 100644 --- a/lib/module_config.cpp +++ b/lib/module_config.cpp @@ -104,6 +104,10 @@ json get_module_config(const std::shared_ptr& mqtt, const std:: 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 e8913923..d5a9f1e7 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, @@ -354,10 +356,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 d40f7c365d08e3ab423cc85de40185f92e931cdb Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Thu, 12 Dec 2024 00:05:15 +0100 Subject: [PATCH 14/39] 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 40f38859..da80cf23 100644 --- a/include/utils/config.hpp +++ b/include/utils/config.hpp @@ -313,7 +313,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 @@ -327,7 +327,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 13693656..b01ba09a 100644 --- a/lib/config.cpp +++ b/lib/config.cpp @@ -132,7 +132,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}; @@ -1153,7 +1153,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); } @@ -1181,14 +1181,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(); @@ -1218,9 +1216,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 be844476..ec93a273 100644 --- a/lib/module_config.cpp +++ b/lib/module_config.cpp @@ -92,8 +92,18 @@ json get_module_config(const std::shared_ptr& mqtt, const std:: 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(const std::shared_ptr& mqtt, const std:: 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 d5a9f1e7..da15cabc 100644 --- a/src/manager.cpp +++ b/src/manager.cpp @@ -346,7 +346,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 dbd4dc2143ce0d5ae915ba6cd46807d6bb7beee9 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Thu, 12 Dec 2024 16:06:30 +0100 Subject: [PATCH 15/39] async get() function for MQTT used in get_module_config refactor interfaces, manifests and types get_async Signed-off-by: Kai-Uwe Hermann --- include/utils/mqtt_abstraction.hpp | 4 + include/utils/mqtt_abstraction_impl.hpp | 4 + include/utils/types.hpp | 6 + lib/module_config.cpp | 159 ++++++++++++++---------- lib/mqtt_abstraction.cpp | 5 + lib/mqtt_abstraction_impl.cpp | 15 +++ 6 files changed, 127 insertions(+), 66 deletions(-) diff --git a/include/utils/mqtt_abstraction.hpp b/include/utils/mqtt_abstraction.hpp index c7977b16..64967840 100644 --- a/include/utils/mqtt_abstraction.hpp +++ b/include/utils/mqtt_abstraction.hpp @@ -64,6 +64,10 @@ class MQTTAbstraction { /// \copydoc MQTTAbstractionImpl::unsubscribe(const std::string&) void unsubscribe(const std::string& topic); + /// + /// \copydoc MQTTAbstractionImpl::get_async(const std::string&, QOS) + AsyncReturn get_async(const std::string& topic, QOS qos); + /// /// \copydoc MQTTAbstractionImpl::get(const std::string&, QOS) nlohmann::json get(const std::string& topic, QOS qos); diff --git a/include/utils/mqtt_abstraction_impl.hpp b/include/utils/mqtt_abstraction_impl.hpp index 85d2cb07..706491b3 100644 --- a/include/utils/mqtt_abstraction_impl.hpp +++ b/include/utils/mqtt_abstraction_impl.hpp @@ -80,6 +80,10 @@ class MQTTAbstractionImpl { /// \brief unsubscribes from the given \p topic void unsubscribe(const std::string& topic); + /// + /// \brief subscribe topic to asynchronously get value on the subscribed topic + AsyncReturn get_async(const std::string& topic, QOS qos); + /// /// \brief subscribe and wait for value on the subscribed topic nlohmann::json get(const std::string& topic, QOS qos); diff --git a/include/utils/types.hpp b/include/utils/types.hpp index 80115c53..4e3c660a 100644 --- a/include/utils/types.hpp +++ b/include/utils/types.hpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -60,6 +61,11 @@ struct TypedHandler { using Token = std::shared_ptr; +struct AsyncReturn { + std::future future; + Token token; +}; + /// \brief MQTT Quality of service enum class QOS { QOS0, ///< At most once delivery diff --git a/lib/module_config.cpp b/lib/module_config.cpp index ec93a273..233378c6 100644 --- a/lib/module_config.cpp +++ b/lib/module_config.cpp @@ -15,105 +15,132 @@ namespace Everest { using json = nlohmann::json; inline constexpr int mqtt_get_config_timeout_ms = 5000; +using FutureCallback = std::tuple>; + +void populate_future_cbs(std::vector& future_cbs, const std::shared_ptr& mqtt, + const std::string& everest_prefix, const std::string& topic, json& out) { + future_cbs.push_back(std::make_tuple>( + mqtt->get_async(topic, QOS::QOS2), [&topic, &everest_prefix, &out](json result) { + out = result; + + return topic; + })); +} + +void populate_future_cbs_arr(std::vector& future_cbs, const std::shared_ptr& mqtt, + const std::string& everest_prefix, const std::string& topic, + const std::string& inner_topic_part, json& array_out, json& out) { + future_cbs.push_back(std::make_tuple>( + mqtt->get_async(topic, QOS::QOS2), + [topic, everest_prefix, &mqtt, &inner_topic_part, &array_out, &out](json result_array) { + array_out = result_array; + std::vector array_future_cbs; + for (const auto& key : result_array) { + auto key_topic = fmt::format("{}{}{}", everest_prefix, inner_topic_part, key.get()); + array_future_cbs.push_back(std::make_tuple>( + mqtt->get_async(key_topic, QOS::QOS2), [key, key_topic, &out](json key_response) { + out[key] = key_response; + return key_topic; + })); + } + for (auto&& [retval, callback] : array_future_cbs) { + auto inner_topic = callback(retval.future.get()); + mqtt->unregister_handler(inner_topic, retval.token); + } + return topic; + })); +} json get_module_config(const std::shared_ptr& mqtt, const std::string& module_id) { + const auto start_time = std::chrono::system_clock::now(); const auto& everest_prefix = mqtt->get_everest_prefix(); const auto get_config_topic = fmt::format("{}modules/{}/get_config", everest_prefix, module_id); - const auto config_topic = fmt::format("{}modules/{}/config", everest_prefix, module_id); - std::promise res_promise; - std::future res_future = res_promise.get_future(); - const auto res_handler = [module_id, &res_promise](const std::string& topic, json data) { - EVLOG_verbose << fmt::format("Incoming config for {}", module_id); + json result; - res_promise.set_value(std::move(data)); - }; + std::vector future_cbs; - const std::shared_ptr res_token = - std::make_shared(HandlerType::GetConfig, std::make_shared(res_handler)); - mqtt->register_handler(config_topic, res_token, QOS::QOS2); + // config + auto config = json::object(); + const auto config_topic = fmt::format("{}modules/{}/config", everest_prefix, module_id); + populate_future_cbs(future_cbs, mqtt, everest_prefix, config_topic, config); const json config_publish_data = json::object({{"type", "full"}}); - mqtt->publish(get_config_topic, config_publish_data, QOS::QOS2); - // wait for result future - const std::chrono::time_point res_wait = - std::chrono::steady_clock::now() + std::chrono::milliseconds(mqtt_get_config_timeout_ms); - std::future_status res_future_status; - do { - res_future_status = res_future.wait_until(res_wait); - } while (res_future_status == std::future_status::deferred); - - json result; - if (res_future_status == std::future_status::timeout) { - mqtt->unregister_handler(config_topic, res_token); - EVLOG_AND_THROW( - EverestTimeoutError(fmt::format("Timeout while waiting for result of get_config of {}", module_id))); - } - if (res_future_status == std::future_status::ready) { - result = res_future.get(); - } - mqtt->unregister_handler(config_topic, res_token); - + // interfaces const auto interface_names_topic = fmt::format("{}interfaces", everest_prefix); - const auto interface_names = mqtt->get(interface_names_topic, QOS::QOS2); + auto interface_names = json::object(); auto interface_definitions = json::object(); - for (const auto& interface : interface_names) { - auto interface_topic = fmt::format("{}interface_definitions/{}", everest_prefix, interface.get()); - auto interface_definition = mqtt->get(interface_topic, QOS::QOS2); - interface_definitions[interface] = interface_definition; - } + const std::string interface_inner_topic_part = "interface_definitions/"; + populate_future_cbs_arr(future_cbs, mqtt, everest_prefix, interface_names_topic, interface_inner_topic_part, + interface_names, interface_definitions); - result["interface_definitions"] = interface_definitions; + // manifests + const auto module_names_topic = fmt::format("{}module_names", everest_prefix); + auto module_names_out = json::object(); + auto manifests = json::object(); + const std::string manifests_inner_topic_part = "manifests/"; + populate_future_cbs_arr(future_cbs, mqtt, everest_prefix, module_names_topic, manifests_inner_topic_part, + module_names_out, manifests); + // types const auto type_names_topic = fmt::format("{}types", everest_prefix); - const auto type_names = mqtt->get(type_names_topic, QOS::QOS2); + auto type_names = json::object(); auto type_definitions = json::object(); - for (const auto& type_name : type_names) { - // type_definition keys already start with a / so omit it in the topic name - auto type_topic = fmt::format("{}type_definitions{}", everest_prefix, type_name.get()); - auto type_definition = mqtt->get(type_topic, QOS::QOS2); - type_definitions[type_name] = type_definition; - } - result["types"] = type_definitions; + // type_definition keys already start with a / so omit it in the topic name + const std::string type_definitions_inner_topic_part = "type_definitions"; + populate_future_cbs_arr(future_cbs, mqtt, everest_prefix, type_names_topic, type_definitions_inner_topic_part, + type_names, type_definitions); + // module provides + auto module_provides = json::object(); const auto module_provides_topic = fmt::format("{}module_provides", everest_prefix); - const auto module_provides = mqtt->get(module_provides_topic, QOS::QOS2); - result["module_provides"] = module_provides; + populate_future_cbs(future_cbs, mqtt, everest_prefix, module_provides_topic, module_provides); + // settings + auto settings = json::object(); const auto settings_topic = fmt::format("{}settings", everest_prefix); - const auto settings = mqtt->get(settings_topic, QOS::QOS2); - result["settings"] = settings; + populate_future_cbs(future_cbs, mqtt, everest_prefix, settings_topic, settings); + // schemas + auto schemas = json::object(); const auto schemas_topic = fmt::format("{}schemas", everest_prefix); - const auto schemas = mqtt->get(schemas_topic, QOS::QOS2); - result["schemas"] = schemas; + populate_future_cbs(future_cbs, mqtt, everest_prefix, schemas_topic, 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; + // error_types_map + auto error_types_map = json::object(); + const auto error_types_map_topic = fmt::format("{}error_types_map", everest_prefix); + populate_future_cbs(future_cbs, mqtt, everest_prefix, error_types_map_topic, error_types_map); - const auto manifests_topic = fmt::format("{}manifests", everest_prefix); - 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; + // module_config_cache + auto module_config_cache = json::object(); + const auto module_config_cache_topic = fmt::format("{}module_config_cache", everest_prefix); + populate_future_cbs(future_cbs, mqtt, everest_prefix, module_config_cache_topic, module_config_cache); + + for (auto&& [retval, callback] : future_cbs) { + auto topic = callback(retval.future.get()); + mqtt->unregister_handler(topic, retval.token); } + result["mappings"] = config.at("mappings"); + result["module_config"] = config.at("module_config"); + result["module_names"] = module_names_out; + result["interface_definitions"] = interface_definitions; result["manifests"] = manifests; - - const auto error_types_map_topic = fmt::format("{}error_types_map", everest_prefix); - const auto error_types_map = mqtt->get(error_types_map_topic, QOS::QOS2); + result["types"] = type_definitions; + result["module_provides"] = module_provides; + result["settings"] = settings; + result["schemas"] = schemas; result["error_map"] = error_types_map; - - const auto module_config_cache_topic = fmt::format("{}module_config_cache", everest_prefix); - const auto module_config_cache = mqtt->get(module_config_cache_topic, QOS::QOS2); result["module_config_cache"] = module_config_cache; + const auto end_time = std::chrono::system_clock::now(); + + EVLOG_info << "get_module_config(): " + << std::chrono::duration_cast(end_time - start_time).count() << "ms"; return result; } diff --git a/lib/mqtt_abstraction.cpp b/lib/mqtt_abstraction.cpp index fd342a4f..3edcd1d4 100644 --- a/lib/mqtt_abstraction.cpp +++ b/lib/mqtt_abstraction.cpp @@ -71,6 +71,11 @@ void MQTTAbstraction::unsubscribe(const std::string& topic) { mqtt_abstraction->unsubscribe(topic); } +AsyncReturn MQTTAbstraction::get_async(const std::string& topic, QOS qos) { + BOOST_LOG_FUNCTION(); + return mqtt_abstraction->get_async(topic, qos); +} + json MQTTAbstraction::get(const std::string& topic, QOS qos) { BOOST_LOG_FUNCTION(); return mqtt_abstraction->get(topic, qos); diff --git a/lib/mqtt_abstraction_impl.cpp b/lib/mqtt_abstraction_impl.cpp index 3c2f2dab..1923ed1b 100644 --- a/lib/mqtt_abstraction_impl.cpp +++ b/lib/mqtt_abstraction_impl.cpp @@ -207,6 +207,21 @@ void MQTTAbstractionImpl::unsubscribe(const std::string& topic) { notify_write_data(); } +AsyncReturn MQTTAbstractionImpl::get_async(const std::string& topic, QOS qos) { + auto res_promise = std::make_shared>(); + std::future res_future = res_promise->get_future(); + + auto res_handler = [this, res_promise](const std::string& topic, json data) mutable { + res_promise->set_value(std::move(data)); + }; + + const auto res_token = + std::make_shared(HandlerType::GetConfig, std::make_shared(res_handler)); + this->register_handler(topic, res_token, QOS::QOS2); + + return {std::move(res_future), res_token}; +} + json MQTTAbstractionImpl::get(const std::string& topic, QOS qos) { BOOST_LOG_FUNCTION(); std::promise res_promise; From 1077b00d35306c56ba4ec7c1d53bb3259814470e Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Mon, 16 Dec 2024 11:27:01 +0100 Subject: [PATCH 16/39] 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 c55f3142..80041e64 100644 --- a/everestjs/everestjs.cpp +++ b/everestjs/everestjs.cpp @@ -887,14 +887,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 250ee4a2b4412036c6fb88ec4443b6fc830784d5 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Mon, 16 Dec 2024 11:28:10 +0100 Subject: [PATCH 17/39] Capturing more vars as refs Signed-off-by: Kai-Uwe Hermann --- lib/module_config.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/module_config.cpp b/lib/module_config.cpp index 233378c6..ae4c284a 100644 --- a/lib/module_config.cpp +++ b/lib/module_config.cpp @@ -32,19 +32,19 @@ void populate_future_cbs_arr(std::vector& future_cbs, const std: const std::string& inner_topic_part, json& array_out, json& out) { future_cbs.push_back(std::make_tuple>( mqtt->get_async(topic, QOS::QOS2), - [topic, everest_prefix, &mqtt, &inner_topic_part, &array_out, &out](json result_array) { + [&topic, &everest_prefix, &mqtt, &inner_topic_part, &array_out, &out](json result_array) { array_out = result_array; std::vector array_future_cbs; for (const auto& key : result_array) { - auto key_topic = fmt::format("{}{}{}", everest_prefix, inner_topic_part, key.get()); + const auto key_topic = fmt::format("{}{}{}", everest_prefix, inner_topic_part, key.get()); array_future_cbs.push_back(std::make_tuple>( - mqtt->get_async(key_topic, QOS::QOS2), [key, key_topic, &out](json key_response) { + mqtt->get_async(key_topic, QOS::QOS2), [&key, &key_topic, &out](json key_response) { out[key] = key_response; return key_topic; })); } for (auto&& [retval, callback] : array_future_cbs) { - auto inner_topic = callback(retval.future.get()); + const auto inner_topic = callback(retval.future.get()); mqtt->unregister_handler(inner_topic, retval.token); } return topic; From 5551fed0a0d4917221391533f739d823e337f3fe Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Mon, 16 Dec 2024 11:28:34 +0100 Subject: [PATCH 18/39] Log which topic cause a timeout exception in get() Signed-off-by: Kai-Uwe Hermann --- lib/mqtt_abstraction_impl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mqtt_abstraction_impl.cpp b/lib/mqtt_abstraction_impl.cpp index 1923ed1b..f547c0f1 100644 --- a/lib/mqtt_abstraction_impl.cpp +++ b/lib/mqtt_abstraction_impl.cpp @@ -246,7 +246,7 @@ json MQTTAbstractionImpl::get(const std::string& topic, QOS qos) { json result; if (res_future_status == std::future_status::timeout) { this->unregister_handler(topic, res_token); - EVLOG_AND_THROW(EverestTimeoutError(fmt::format("Timeout while waiting for result of get()"))); + EVLOG_AND_THROW(EverestTimeoutError(fmt::format("Timeout while waiting for result of get({})", topic))); } if (res_future_status == std::future_status::ready) { result = res_future.get(); From c2f2f601f746d92560f61ca8ab0da43f6071491f Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Wed, 18 Dec 2024 18:12:08 +0100 Subject: [PATCH 19/39] Use get_with_timeout in get_module_config Signed-off-by: Kai-Uwe Hermann --- lib/module_config.cpp | 66 ++++++++++++++++++++++++++++++++----------- 1 file changed, 49 insertions(+), 17 deletions(-) diff --git a/lib/module_config.cpp b/lib/module_config.cpp index ae4c284a..a53f956f 100644 --- a/lib/module_config.cpp +++ b/lib/module_config.cpp @@ -2,6 +2,7 @@ // Copyright Pionix GmbH and Contributors to EVerest #include +#include #include @@ -15,40 +16,71 @@ namespace Everest { using json = nlohmann::json; inline constexpr int mqtt_get_config_timeout_ms = 5000; -using FutureCallback = std::tuple>; +using FutureCallback = std::tuple, std::string>; void populate_future_cbs(std::vector& future_cbs, const std::shared_ptr& mqtt, const std::string& everest_prefix, const std::string& topic, json& out) { future_cbs.push_back(std::make_tuple>( - mqtt->get_async(topic, QOS::QOS2), [&topic, &everest_prefix, &out](json result) { + mqtt->get_async(topic, QOS::QOS2), + [topic, &everest_prefix, &out](json result) { out = result; return topic; - })); + }, + topic)); +} + +json get_with_timeout(std::future future, const std::shared_ptr& mqtt, const std::string& topic, + const Token& token) { + // wait for result future + const std::chrono::time_point wait = + std::chrono::steady_clock::now() + std::chrono::milliseconds(mqtt_get_config_timeout_ms); + std::future_status future_status; + do { + future_status = future.wait_until(wait); + } while (future_status == std::future_status::deferred); + + json result; + if (future_status == std::future_status::timeout) { + mqtt->unregister_handler(topic, token); + EVLOG_AND_THROW(EverestTimeoutError(fmt::format("Timeout while waiting for result of get({})", topic))); + } + if (future_status == std::future_status::ready) { + return future.get(); + } + + return result; } void populate_future_cbs_arr(std::vector& future_cbs, const std::shared_ptr& mqtt, const std::string& everest_prefix, const std::string& topic, const std::string& inner_topic_part, json& array_out, json& out) { - future_cbs.push_back(std::make_tuple>( + future_cbs.push_back(std::make_tuple, std::string>( mqtt->get_async(topic, QOS::QOS2), - [&topic, &everest_prefix, &mqtt, &inner_topic_part, &array_out, &out](json result_array) { + [topic, &everest_prefix, &mqtt, &inner_topic_part, &array_out, &out](json result_array) { array_out = result_array; std::vector array_future_cbs; - for (const auto& key : result_array) { - const auto key_topic = fmt::format("{}{}{}", everest_prefix, inner_topic_part, key.get()); - array_future_cbs.push_back(std::make_tuple>( - mqtt->get_async(key_topic, QOS::QOS2), [&key, &key_topic, &out](json key_response) { + std::set keys; + for (const auto& element : result_array) { + keys.insert(element.get()); + } + for (const auto& key : keys) { + const auto key_topic = fmt::format("{}{}{}", everest_prefix, inner_topic_part, key); + array_future_cbs.push_back(std::make_tuple, std::string>( + mqtt->get_async(key_topic, QOS::QOS2), + [&key, key_topic, &out](json key_response) { out[key] = key_response; return key_topic; - })); + }, + fmt::format("{}{}{}", everest_prefix, inner_topic_part, key))); } - for (auto&& [retval, callback] : array_future_cbs) { - const auto inner_topic = callback(retval.future.get()); + for (auto&& [retval, callback, tpc] : array_future_cbs) { + const auto inner_topic = callback(get_with_timeout(std::move(retval.future), mqtt, tpc, retval.token)); mqtt->unregister_handler(inner_topic, retval.token); } return topic; - })); + }, + fmt::format("{}", topic))); } json get_module_config(const std::shared_ptr& mqtt, const std::string& module_id) { @@ -120,8 +152,8 @@ json get_module_config(const std::shared_ptr& mqtt, const std:: const auto module_config_cache_topic = fmt::format("{}module_config_cache", everest_prefix); populate_future_cbs(future_cbs, mqtt, everest_prefix, module_config_cache_topic, module_config_cache); - for (auto&& [retval, callback] : future_cbs) { - auto topic = callback(retval.future.get()); + for (auto&& [retval, callback, tpc] : future_cbs) { + auto topic = callback(get_with_timeout(std::move(retval.future), mqtt, tpc, retval.token)); mqtt->unregister_handler(topic, retval.token); } @@ -139,8 +171,8 @@ json get_module_config(const std::shared_ptr& mqtt, const std:: const auto end_time = std::chrono::system_clock::now(); - EVLOG_info << "get_module_config(): " - << std::chrono::duration_cast(end_time - start_time).count() << "ms"; + EVLOG_info << "get_module_config(" << module_id + << "): " << std::chrono::duration_cast(end_time - start_time).count() << "ms"; return result; } From b96933ba736fd1721f8c5d750dcf91766237a8b9 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Wed, 18 Dec 2024 18:13:03 +0100 Subject: [PATCH 20/39] Fix usage of MQTTSettings uses socket in manager Signed-off-by: Kai-Uwe Hermann --- src/manager.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/manager.cpp b/src/manager.cpp index da15cabc..eedc73b8 100644 --- a/src/manager.cpp +++ b/src/manager.cpp @@ -638,7 +638,7 @@ int boot(const po::variables_map& vm) { EVLOG_info << ms.version_information; EVLOG_info << ""; - if (ms.mqtt_settings.uses_socket()) { + if (not ms.mqtt_settings.uses_socket()) { EVLOG_info << "Using MQTT broker " << ms.mqtt_settings.broker_host << ":" << ms.mqtt_settings.broker_port; } else { EVLOG_info << "Using MQTT broker unix domain sockets:" << ms.mqtt_settings.broker_socket_path; @@ -756,7 +756,7 @@ int boot(const po::variables_map& vm) { auto mqtt_abstraction = MQTTAbstraction(ms.mqtt_settings); if (!mqtt_abstraction.connect()) { - if (ms.mqtt_settings.broker_socket_path.empty()) { + if (not ms.mqtt_settings.uses_socket()) { EVLOG_error << fmt::format("Cannot connect to MQTT broker at {}:{}", ms.mqtt_settings.broker_host, ms.mqtt_settings.broker_port); } else { From e6bf11cf46d489ddc6189f13e61a722628ff4916 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Wed, 18 Dec 2024 18:45:04 +0100 Subject: [PATCH 21/39] 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 09b06936..d7bee6ad 100644 --- a/lib/everest.cpp +++ b/lib/everest.cpp @@ -230,11 +230,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 5d63f809ca2f688c3cd78617ef62ef311819b1b6 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Wed, 18 Dec 2024 19:28:51 +0100 Subject: [PATCH 22/39] More const ref usage Signed-off-by: Kai-Uwe Hermann --- everestjs/everestjs.cpp | 16 ++++++++-------- everestpy/src/everest/misc.cpp | 4 ++-- lib/everest.cpp | 6 +++--- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/everestjs/everestjs.cpp b/everestjs/everestjs.cpp index 80041e64..727a2ac1 100644 --- a/everestjs/everestjs.cpp +++ b/everestjs/everestjs.cpp @@ -614,10 +614,10 @@ static Napi::Value boot_module(const Napi::CallbackInfo& info) { "Module with identifier '" << module_id << "' not found in config!")); } - const std::string& module_name = config->get_main_config()[module_id]["module"].get(); - auto module_manifest = config->get_manifests()[module_name]; + const std::string& module_name = config->get_module_name(module_id); + const auto& module_manifest = config->get_manifests().at(module_name); // FIXME (aw): get_classes should be called get_units and should contain the type of class for each unit - auto module_impls = config->get_interfaces()[module_name]; + const auto& module_impls = config->get_interfaces().at(module_name); // initialize everest framework const auto& module_identifier = config->printable_identifier(module_id); @@ -735,9 +735,9 @@ static Napi::Value boot_module(const Napi::CallbackInfo& info) { auto uses_list_reqs_prop = Napi::Object::New(env); auto uses_cmds_prop = Napi::Object::New(env); auto uses_list_cmds_prop = Napi::Object::New(env); - for (auto& requirement : module_manifest["requires"].items()) { + for (const auto& requirement : module_manifest.at("requires").items()) { auto req_prop = Napi::Object::New(env); - auto const& requirement_id = requirement.key(); + const auto& requirement_id = requirement.key(); json req_route_list = config->resolve_requirement(module_id, requirement_id); // if this was a requirement with min_connections == 1 and max_connections == 1, // this will be simply a single connection, but an array of connections otherwise @@ -749,13 +749,13 @@ static Napi::Value boot_module(const Napi::CallbackInfo& info) { auto req_array_prop = Napi::Array::New(env); auto req_mod_cmds_array = Napi::Array::New(env); for (std::size_t i = 0; i < req_route_list.size(); i++) { - auto req_route = req_route_list[i]; + const auto& req_route = req_route_list[i]; const std::string& requirement_module_id = req_route["module_id"]; const std::string& requirement_impl_id = req_route["implementation_id"]; // FIXME (aw): why is const auto& not possible for the following line? // we only want cmds/vars from the required interface to be usable, not from it's child interfaces - std::string interface_name = req_route["required_interface"].get(); - auto requirement_impl_intf = config->get_interface_definition(interface_name); + const std::string& interface_name = req_route["required_interface"].get(); + const auto& requirement_impl_intf = config->get_interface_definition(interface_name); auto requirement_vars = Everest::Config::keys(requirement_impl_intf["vars"]); auto requirement_cmds = Everest::Config::keys(requirement_impl_intf["cmds"]); diff --git a/everestpy/src/everest/misc.cpp b/everestpy/src/everest/misc.cpp index d3a7d5a5..f52f683b 100644 --- a/everestpy/src/everest/misc.cpp +++ b/everestpy/src/everest/misc.cpp @@ -82,8 +82,8 @@ RuntimeSession::RuntimeSession() { ModuleSetup create_setup_from_config(const std::string& module_id, Everest::Config& config) { ModuleSetup setup; - const std::string& module_name = config.get_main_config().at(module_id).at("module"); - const auto module_manifest = config.get_manifests().at(module_name); + const std::string& module_name = config.get_module_name(module_id); + const auto& module_manifest = config.get_manifests().at(module_name); // setup connections for (const auto& requirement : module_manifest.at("requires").items()) { diff --git a/lib/everest.cpp b/lib/everest.cpp index d7bee6ad..c13f0429 100644 --- a/lib/everest.cpp +++ b/lib/everest.cpp @@ -667,7 +667,7 @@ void Everest::subscribe_global_all_errors(const error::ErrorCallback& callback, const json provides = this->config.get_manifests().at(module_name).at("provides"); for (const auto& impl : provides.items()) { const std::string& impl_id = impl.key(); - const std::string interface = impl.value().at("interface"); + const std::string& interface = impl.value().at("interface"); const json errors = this->config.get_interface_definition(interface).at("errors"); for (const auto& error_namespace_it : errors.items()) { const std::string& error_type_namespace = error_namespace_it.key(); @@ -983,8 +983,8 @@ json Everest::get_cmd_definition(const std::string& module_id, const std::string bool is_call) { BOOST_LOG_FUNCTION(); - const std::string module_name = this->config.get_module_name(module_id); - const auto cmds = this->config.get_module_cmds(module_name, impl_id); + const auto& module_name = this->config.get_module_name(module_id); + const auto& cmds = this->config.get_module_cmds(module_name, impl_id); if (!this->config.module_provides(module_name, impl_id)) { if (!is_call) { From 04ec269ba73e2fcfa70b34629286318140359237 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Sat, 21 Dec 2024 20:03:38 +0100 Subject: [PATCH 23/39] 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 eedc73b8..f5f7ab18 100644 --- a/src/manager.cpp +++ b/src/manager.cpp @@ -400,6 +400,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()) { @@ -458,17 +469,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 4b6d16ea14c6ccc8eae483adebf820371313b1ed Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Sat, 21 Dec 2024 20:37:52 +0100 Subject: [PATCH 24/39] everestpy: initialize logging in module ctor not RuntimeSession Signed-off-by: Kai-Uwe Hermann --- everestpy/src/everest/misc.cpp | 8 ++------ everestpy/src/everest/misc.hpp | 5 +++++ everestpy/src/everest/module.cpp | 2 ++ 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/everestpy/src/everest/misc.cpp b/everestpy/src/everest/misc.cpp index f52f683b..6ccc52f9 100644 --- a/everestpy/src/everest/misc.cpp +++ b/everestpy/src/everest/misc.cpp @@ -63,18 +63,14 @@ RuntimeSession::RuntimeSession(const std::string& prefix, const std::string& con // We extract the settings from the config file so everest-testing doesn't break const auto ms = Everest::ManagerSettings(prefix, config_file); - Everest::Logging::init(ms.runtime_settings->logging_config_file.string()); + this->logging_config_file = ms.runtime_settings->logging_config_file; this->mqtt_settings = ms.mqtt_settings; } RuntimeSession::RuntimeSession() { - const auto module_id = get_variable_from_env("EV_MODULE"); - - namespace fs = std::filesystem; - const fs::path logging_config_file = + this->logging_config_file = Everest::assert_file(get_variable_from_env("EV_LOG_CONF_FILE"), "Default logging config"); - Everest::Logging::init(logging_config_file.string(), module_id); this->mqtt_settings = get_mqtt_settings_from_env(); } diff --git a/everestpy/src/everest/misc.hpp b/everestpy/src/everest/misc.hpp index dd817b2c..7f08769b 100644 --- a/everestpy/src/everest/misc.hpp +++ b/everestpy/src/everest/misc.hpp @@ -22,8 +22,13 @@ class RuntimeSession { return mqtt_settings; } + const std::filesystem::path& get_logging_config_file() const { + return logging_config_file; + } + private: Everest::MQTTSettings mqtt_settings; + std::filesystem::path logging_config_file; }; struct Interface { diff --git a/everestpy/src/everest/module.cpp b/everestpy/src/everest/module.cpp index 21ccc373..3e0f3582 100644 --- a/everestpy/src/everest/module.cpp +++ b/everestpy/src/everest/module.cpp @@ -23,6 +23,8 @@ Module::Module(const RuntimeSession& session) : Module(get_variable_from_env("EV Module::Module(const std::string& module_id_, const RuntimeSession& session_) : module_id(module_id_), session(session_), start_time(std::chrono::system_clock::now()) { + Everest::Logging::init(session.get_logging_config_file().string(), module_id); + this->mqtt_abstraction = std::make_shared(session.get_mqtt_settings()); this->mqtt_abstraction->connect(); this->mqtt_abstraction->spawn_main_loop_thread(); From ed5152ec851c9f3713011c873d76b49384a9a5d1 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Sat, 21 Dec 2024 20:39:59 +0100 Subject: [PATCH 25/39] everestpy: add new RuntimeSession ctor that accepts MQTTSettings and logging configuration directly Signed-off-by: Kai-Uwe Hermann --- everestpy/src/everest/everestpy.cpp | 12 +++++++++++- everestpy/src/everest/misc.cpp | 11 +++++++++++ everestpy/src/everest/misc.hpp | 2 ++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/everestpy/src/everest/everestpy.cpp b/everestpy/src/everest/everestpy.cpp index fe38ed85..2637a103 100644 --- a/everestpy/src/everest/everestpy.cpp +++ b/everestpy/src/everest/everestpy.cpp @@ -22,10 +22,20 @@ namespace py = pybind11; PYBIND11_MODULE(everestpy, m) { + py::class_(m, "MQTTSettings") + .def(py::init<>()) + .def_readwrite("broker_socket_path", &Everest::MQTTSettings::broker_socket_path) + .def_readwrite("broker_host", &Everest::MQTTSettings::broker_host) + .def_readwrite("broker_port", &Everest::MQTTSettings::broker_port) + .def_readwrite("everest_prefix", &Everest::MQTTSettings::everest_prefix) + .def_readwrite("external_prefix", &Everest::MQTTSettings::external_prefix) + .def("uses_socket", &Everest::MQTTSettings::uses_socket); + // FIXME (aw): add m.doc? py::class_(m, "RuntimeSession") .def(py::init<>()) - .def(py::init()); + .def(py::init()) + .def(py::init()); py::class_(m, "ModuleInfoPaths") .def_readonly("etc", &ModuleInfo::Paths::etc) diff --git a/everestpy/src/everest/misc.cpp b/everestpy/src/everest/misc.cpp index 6ccc52f9..d9d06a5b 100644 --- a/everestpy/src/everest/misc.cpp +++ b/everestpy/src/everest/misc.cpp @@ -53,6 +53,17 @@ static Everest::MQTTSettings get_mqtt_settings_from_env() { } } +RuntimeSession::RuntimeSession(const Everest::MQTTSettings& mqtt_settings, const std::string& logging_config) { + this->mqtt_settings = mqtt_settings; + if (logging_config.empty()) { + this->logging_config_file = Everest::assert_dir(Everest::defaults::PREFIX, "Default prefix") / + std::filesystem::path(Everest::defaults::SYSCONF_DIR) / + Everest::defaults::NAMESPACE / Everest::defaults::LOGGING_CONFIG_NAME; + } else { + this->logging_config_file = Everest::assert_file(logging_config, "Default logging config"); + } +} + /// This is just kept for compatibility RuntimeSession::RuntimeSession(const std::string& prefix, const std::string& config_file) { EVLOG_warning diff --git a/everestpy/src/everest/misc.hpp b/everestpy/src/everest/misc.hpp index 7f08769b..17829db7 100644 --- a/everestpy/src/everest/misc.hpp +++ b/everestpy/src/everest/misc.hpp @@ -14,6 +14,8 @@ const std::string get_variable_from_env(const std::string& variable, const std:: class RuntimeSession { public: + RuntimeSession(const Everest::MQTTSettings& mqtt_settings, const std::string& logging_config); + RuntimeSession(const std::string& prefix, const std::string& config_file); RuntimeSession(); From 30cf7e2b7af5e446d82395e96841c7c9575bc18a Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Sat, 21 Dec 2024 20:41:08 +0100 Subject: [PATCH 26/39] everestpy: add short documentation comments and deprecated RuntimeSession ctor that accepts config file Signed-off-by: Kai-Uwe Hermann --- everestpy/src/everest/misc.hpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/everestpy/src/everest/misc.hpp b/everestpy/src/everest/misc.hpp index 17829db7..f2b98dd9 100644 --- a/everestpy/src/everest/misc.hpp +++ b/everestpy/src/everest/misc.hpp @@ -14,10 +14,14 @@ const std::string get_variable_from_env(const std::string& variable, const std:: class RuntimeSession { public: + /// \brief Allows python modules to directly pass \p mqtt_settings as well as a \p logging_config RuntimeSession(const Everest::MQTTSettings& mqtt_settings, const std::string& logging_config); + [[deprecated("Consider switching to the newer RuntimeSession() or RuntimeSession(mqtt_settings, logging_config) " + "ctors that receive module configuration via MQTT")]] RuntimeSession(const std::string& prefix, const std::string& config_file); + /// \brief Get settings and configuration via MQTT based on certain environment variables RuntimeSession(); const Everest::MQTTSettings& get_mqtt_settings() const { From 9a90570722df59b00f59ba364746e2eb336d4b72 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Sat, 21 Dec 2024 20:41:32 +0100 Subject: [PATCH 27/39] More constref and move usage Signed-off-by: Kai-Uwe Hermann --- lib/config.cpp | 2 +- lib/module_config.cpp | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/config.cpp b/lib/config.cpp index b01ba09a..dca7d746 100644 --- a/lib/config.cpp +++ b/lib/config.cpp @@ -1184,7 +1184,7 @@ ModuleConfigs Config::get_module_configs(const std::string& module_id) const { ConfigMap processed_conf_map; for (const auto& entry : conf_map.value().items()) { const auto& entry_value = entry.value(); - const json entry_type = entry_value.at("type"); + const json& entry_type = entry_value.at("type"); ConfigEntry value; const json& data = entry_value.at("value"); diff --git a/lib/module_config.cpp b/lib/module_config.cpp index a53f956f..fdb9f2ec 100644 --- a/lib/module_config.cpp +++ b/lib/module_config.cpp @@ -3,6 +3,7 @@ #include #include +#include #include @@ -23,7 +24,7 @@ void populate_future_cbs(std::vector& future_cbs, const std::sha future_cbs.push_back(std::make_tuple>( mqtt->get_async(topic, QOS::QOS2), [topic, &everest_prefix, &out](json result) { - out = result; + out = std::move(result); return topic; }, @@ -57,7 +58,7 @@ void populate_future_cbs_arr(std::vector& future_cbs, const std: const std::string& inner_topic_part, json& array_out, json& out) { future_cbs.push_back(std::make_tuple, std::string>( mqtt->get_async(topic, QOS::QOS2), - [topic, &everest_prefix, &mqtt, &inner_topic_part, &array_out, &out](json result_array) { + [topic, &everest_prefix, &mqtt, &inner_topic_part, &array_out, &out](const json& result_array) { array_out = result_array; std::vector array_future_cbs; std::set keys; @@ -65,10 +66,10 @@ void populate_future_cbs_arr(std::vector& future_cbs, const std: keys.insert(element.get()); } for (const auto& key : keys) { - const auto key_topic = fmt::format("{}{}{}", everest_prefix, inner_topic_part, key); + auto key_topic = fmt::format("{}{}{}", everest_prefix, inner_topic_part, key); array_future_cbs.push_back(std::make_tuple, std::string>( mqtt->get_async(key_topic, QOS::QOS2), - [&key, key_topic, &out](json key_response) { + [&key, key_topic, &out](const json& key_response) { out[key] = key_response; return key_topic; }, From c4737e358a400c1040ac7a35d5c1c885ab1574c5 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Sat, 21 Dec 2024 21:33:41 +0100 Subject: [PATCH 28/39] Add --retain-topics flag to manager to keep retained topics after startup Remove them by default now but allow them to be explicitly kept for inspection Signed-off-by: Kai-Uwe Hermann --- include/utils/mqtt_abstraction.hpp | 4 ++++ include/utils/mqtt_abstraction_impl.hpp | 6 ++++++ lib/mqtt_abstraction.cpp | 5 +++++ lib/mqtt_abstraction_impl.cpp | 19 +++++++++++++++++++ src/manager.cpp | 23 +++++++++++++++++------ 5 files changed, 51 insertions(+), 6 deletions(-) diff --git a/include/utils/mqtt_abstraction.hpp b/include/utils/mqtt_abstraction.hpp index 64967840..1ff3754e 100644 --- a/include/utils/mqtt_abstraction.hpp +++ b/include/utils/mqtt_abstraction.hpp @@ -64,6 +64,10 @@ class MQTTAbstraction { /// \copydoc MQTTAbstractionImpl::unsubscribe(const std::string&) void unsubscribe(const std::string& topic); + /// + /// \copydoc MQTTAbstractionImpl::clear_retained_topics() + void clear_retained_topics(); + /// /// \copydoc MQTTAbstractionImpl::get_async(const std::string&, QOS) AsyncReturn get_async(const std::string& topic, QOS qos); diff --git a/include/utils/mqtt_abstraction_impl.hpp b/include/utils/mqtt_abstraction_impl.hpp index 706491b3..70c34ddc 100644 --- a/include/utils/mqtt_abstraction_impl.hpp +++ b/include/utils/mqtt_abstraction_impl.hpp @@ -80,6 +80,10 @@ class MQTTAbstractionImpl { /// \brief unsubscribes from the given \p topic void unsubscribe(const std::string& topic); + /// + /// \brief clears any previously published topics that had the retain flag set + void clear_retained_topics(); + /// /// \brief subscribe topic to asynchronously get value on the subscribed topic AsyncReturn get_async(const std::string& topic, QOS qos); @@ -125,6 +129,8 @@ class MQTTAbstractionImpl { MessageQueue message_queue; std::vector> messages_before_connected; std::mutex messages_before_connected_mutex; + std::mutex retained_topics_mutex; + std::vector retained_topics; Thread mqtt_mainloop_thread; std::shared_future main_loop_future; diff --git a/lib/mqtt_abstraction.cpp b/lib/mqtt_abstraction.cpp index 3edcd1d4..e1a78eb5 100644 --- a/lib/mqtt_abstraction.cpp +++ b/lib/mqtt_abstraction.cpp @@ -71,6 +71,11 @@ void MQTTAbstraction::unsubscribe(const std::string& topic) { mqtt_abstraction->unsubscribe(topic); } +void MQTTAbstraction::clear_retained_topics() { + BOOST_LOG_FUNCTION(); + mqtt_abstraction->clear_retained_topics(); +} + AsyncReturn MQTTAbstraction::get_async(const std::string& topic, QOS qos) { BOOST_LOG_FUNCTION(); return mqtt_abstraction->get_async(topic, qos); diff --git a/lib/mqtt_abstraction_impl.cpp b/lib/mqtt_abstraction_impl.cpp index f547c0f1..8b9ed653 100644 --- a/lib/mqtt_abstraction_impl.cpp +++ b/lib/mqtt_abstraction_impl.cpp @@ -155,6 +155,13 @@ void MQTTAbstractionImpl::publish(const std::string& topic, const std::string& d if (retain) { publish_flags |= MQTT_PUBLISH_RETAIN; + if (not(data.empty() and qos == QOS::QOS0)) { + // topic should be retained, so save the topic in retained_topics + // do not save the topic when the payload is empty and QOS is set to 0 which means a retained topic is to be + // cleared + const std::lock_guard lock(retained_topics_mutex); + this->retained_topics.push_back(topic); + } } if (!this->mqtt_is_connected) { @@ -207,6 +214,18 @@ void MQTTAbstractionImpl::unsubscribe(const std::string& topic) { notify_write_data(); } +void MQTTAbstractionImpl::clear_retained_topics() { + BOOST_LOG_FUNCTION(); + const std::lock_guard lock(retained_topics_mutex); + + for (const auto& retained_topic : retained_topics) { + this->publish(retained_topic, std::string(), QOS::QOS0, true); + EVLOG_verbose << "Cleared retained topic: " << retained_topic; + } + + retained_topics.clear(); +} + AsyncReturn MQTTAbstractionImpl::get_async(const std::string& topic, QOS qos) { auto res_promise = std::make_shared>(); std::future res_future = res_promise->get_future(); diff --git a/src/manager.cpp b/src/manager.cpp index f5f7ab18..ed24d128 100644 --- a/src/manager.cpp +++ b/src/manager.cpp @@ -297,7 +297,8 @@ void cleanup_retained_topics(ManagerConfig& config, MQTTAbstraction& mqtt_abstra static std::map start_modules(ManagerConfig& config, MQTTAbstraction& mqtt_abstraction, const std::vector& ignored_modules, const std::vector& standalone_modules, - const ManagerSettings& ms, StatusFifo& status_fifo) { + const ManagerSettings& ms, StatusFifo& status_fifo, + bool retain_topics) { BOOST_LOG_FUNCTION(); std::vector modules_to_spawn; @@ -426,8 +427,8 @@ static std::map start_modules(ManagerConfig& config, MQTTAbs } const Handler module_ready_handler = [module_name, &mqtt_abstraction, &config, standalone_modules, - mqtt_everest_prefix = ms.mqtt_settings.everest_prefix, - &status_fifo](const std::string&, const nlohmann::json& json) { + mqtt_everest_prefix = ms.mqtt_settings.everest_prefix, &status_fifo, + retain_topics](const std::string&, const nlohmann::json& json) { EVLOG_debug << fmt::format("received module ready signal for module: {}({})", module_name, json.dump()); const std::unique_lock lock(modules_ready_mutex); // FIXME (aw): here are race conditions, if the ready handler gets called while modules are shut down! @@ -449,6 +450,12 @@ static std::map start_modules(ManagerConfig& config, MQTTAbs [](const auto& element) { return element.second.ready; })) { const auto complete_end_time = std::chrono::system_clock::now(); status_fifo.update(StatusFifo::ALL_MODULES_STARTED); + if (not retain_topics) { + EVLOG_info << "Clearing retained topics published by manager during startup"; + mqtt_abstraction.clear_retained_topics(); + } else { + EVLOG_info << "Keeping retained topics published by manager during startup for inspection"; + } EVLOG_info << fmt::format( TERMINAL_STYLE_OK, "🚙🚙🚙 All modules are initialized. EVerest up and running [{}ms] 🚙🚙🚙", std::chrono::duration_cast(complete_end_time - complete_start_time) @@ -676,6 +683,8 @@ int boot(const po::variables_map& vm) { return EXIT_SUCCESS; } + const bool retain_topics = (vm.count("retain-topics") != 0); + const auto start_time = std::chrono::system_clock::now(); std::unique_ptr config; try { @@ -769,7 +778,7 @@ int boot(const po::variables_map& vm) { mqtt_abstraction.spawn_main_loop_thread(); auto module_handles = - start_modules(*config, mqtt_abstraction, ignored_modules, standalone_modules, ms, status_fifo); + start_modules(*config, mqtt_abstraction, ignored_modules, standalone_modules, ms, status_fifo, retain_topics); bool modules_started = true; bool restart_modules = false; @@ -834,8 +843,8 @@ int boot(const po::variables_map& vm) { #ifdef ENABLE_ADMIN_PANEL if (module_handles.size() == 0 && restart_modules) { - module_handles = - start_modules(*config, mqtt_abstraction, ignored_modules, standalone_modules, ms, status_fifo); + module_handles = start_modules(*config, mqtt_abstraction, ignored_modules, standalone_modules, ms, + status_fifo, retain_topics); restart_modules = false; modules_started = true; } @@ -899,6 +908,8 @@ int main(int argc, char* argv[]) { "looked up in the default config directory"); desc.add_options()("status-fifo", po::value()->default_value(""), "Path to a named pipe, that shall be used for status updates from the manager"); + desc.add_options()("retain-topics", "Retain configuration MQTT topics setup by manager for inspection, by default " + "these will be cleared after startup"); po::variables_map vm; From 61603c900562e1d34c82135d3807d329157cfc0d Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Sat, 21 Dec 2024 21:34:53 +0100 Subject: [PATCH 29/39] Remove old cleanup_retained_topics functions since this is now provided by the MQTT abstraction Signed-off-by: Kai-Uwe Hermann --- src/manager.cpp | 30 ------------------------------ 1 file changed, 30 deletions(-) diff --git a/src/manager.cpp b/src/manager.cpp index ed24d128..958d7179 100644 --- a/src/manager.cpp +++ b/src/manager.cpp @@ -265,35 +265,6 @@ struct ModuleReadyInfo { std::map modules_ready; std::mutex modules_ready_mutex; -void cleanup_retained_topics(ManagerConfig& config, MQTTAbstraction& mqtt_abstraction, - const std::string& mqtt_everest_prefix) { - const auto& interface_definitions = config.get_interface_definitions(); - - mqtt_abstraction.publish(fmt::format("{}interfaces", mqtt_everest_prefix), std::string(), QOS::QOS2, true); - - for (const auto& interface_definition : interface_definitions.items()) { - mqtt_abstraction.publish( - fmt::format("{}interface_definitions/{}", mqtt_everest_prefix, interface_definition.key()), std::string(), - QOS::QOS2, true); - } - - mqtt_abstraction.publish(fmt::format("{}types", mqtt_everest_prefix), std::string(), QOS::QOS2, true); - - mqtt_abstraction.publish(fmt::format("{}module_provides", mqtt_everest_prefix), std::string(), QOS::QOS2, true); - - mqtt_abstraction.publish(fmt::format("{}settings", mqtt_everest_prefix), std::string(), QOS::QOS2, true); - - mqtt_abstraction.publish(fmt::format("{}schemas", mqtt_everest_prefix), std::string(), QOS::QOS2, true); - - mqtt_abstraction.publish(fmt::format("{}manifests", mqtt_everest_prefix), std::string(), QOS::QOS2, true); - - 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, const std::vector& ignored_modules, const std::vector& standalone_modules, @@ -460,7 +431,6 @@ static std::map start_modules(ManagerConfig& config, MQTTAbs TERMINAL_STYLE_OK, "🚙🚙🚙 All modules are initialized. EVerest up and running [{}ms] 🚙🚙🚙", std::chrono::duration_cast(complete_end_time - complete_start_time) .count()); - // cleanup_retained_topics(config, mqtt_abstraction, mqtt_everest_prefix); mqtt_abstraction.publish(fmt::format("{}ready", mqtt_everest_prefix), nlohmann::json(true)); } else if (!standalone_modules.empty()) { if (modules_spawned == modules_ready.size() - standalone_modules.size()) { From 6c19fb42597addf633f0074b7f2bd3a12885f94d Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Sat, 21 Dec 2024 21:52:52 +0100 Subject: [PATCH 30/39] clang-format Signed-off-by: Kai-Uwe Hermann --- everestpy/src/everest/misc.hpp | 4 ++-- lib/config.cpp | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/everestpy/src/everest/misc.hpp b/everestpy/src/everest/misc.hpp index f2b98dd9..a49073d7 100644 --- a/everestpy/src/everest/misc.hpp +++ b/everestpy/src/everest/misc.hpp @@ -18,8 +18,8 @@ class RuntimeSession { RuntimeSession(const Everest::MQTTSettings& mqtt_settings, const std::string& logging_config); [[deprecated("Consider switching to the newer RuntimeSession() or RuntimeSession(mqtt_settings, logging_config) " - "ctors that receive module configuration via MQTT")]] - RuntimeSession(const std::string& prefix, const std::string& config_file); + "ctors that receive module configuration via MQTT")]] RuntimeSession(const std::string& prefix, + const std::string& config_file); /// \brief Get settings and configuration via MQTT based on certain environment variables RuntimeSession(); diff --git a/lib/config.cpp b/lib/config.cpp index dca7d746..41368740 100644 --- a/lib/config.cpp +++ b/lib/config.cpp @@ -132,7 +132,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}; From 6405e8eabcb9b66f5e80d57ebdbb2618438dd434 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Thu, 2 Jan 2025 14:56:55 +0100 Subject: [PATCH 31/39] Erase MessageHandler for topics without any registered handlers This drastically reduces the amount of lingering threads just hanging around and doing absolutely nothing Signed-off-by: Kai-Uwe Hermann --- lib/mqtt_abstraction_impl.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/mqtt_abstraction_impl.cpp b/lib/mqtt_abstraction_impl.cpp index 8b9ed653..5ff29b05 100644 --- a/lib/mqtt_abstraction_impl.cpp +++ b/lib/mqtt_abstraction_impl.cpp @@ -398,9 +398,11 @@ void MQTTAbstractionImpl::on_mqtt_message(const Message& message) { } lock.unlock(); + // TODO(kai): this should maybe not even be a warning since it can happen that we unsubscribe from a topic and + // have removed the message handler but the MQTT unsubscribe didn't complete yet and we still receive messages + // on this topic that we can just ignore if (!found) { - EVLOG_AND_THROW( - EverestInternalError(fmt::format("Internal error: topic '{}' should have a matching handler!", topic))); + EVLOG_warning << fmt::format("Internal error: topic '{}' should have a matching handler!", topic); } } catch (boost::exception& e) { EVLOG_critical << fmt::format("Caught MQTT on_message boost::exception:\n{}", @@ -519,6 +521,7 @@ void MQTTAbstractionImpl::unregister_handler(const std::string& topic, const Tok EVLOG_verbose << fmt::format("Unsubscribing from {}", topic); this->unsubscribe(topic); } + this->message_handlers.erase(topic); } const std::string handler_count = (number_of_handlers == 0) ? "None" : std::to_string(number_of_handlers); From 7c539ac88a83e982caf3dec44276ddfe74dfe2bd Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Thu, 2 Jan 2025 14:57:53 +0100 Subject: [PATCH 32/39] Use .at() instead of [] for map access Signed-off-by: Kai-Uwe Hermann --- lib/mqtt_abstraction_impl.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/mqtt_abstraction_impl.cpp b/lib/mqtt_abstraction_impl.cpp index 5ff29b05..96561d38 100644 --- a/lib/mqtt_abstraction_impl.cpp +++ b/lib/mqtt_abstraction_impl.cpp @@ -488,15 +488,15 @@ void MQTTAbstractionImpl::register_handler(const std::string& topic, const std:: if (this->message_handlers.count(topic) == 0) { this->message_handlers.emplace(std::piecewise_construct, std::forward_as_tuple(topic), std::forward_as_tuple()); } - this->message_handlers[topic].add_handler(handler); + this->message_handlers.at(topic).add_handler(handler); // only subscribe for this topic if we aren't already and the mqtt client is connected // if we are not connected the on_mqtt_connect() callback will subscribe to the topic - if (this->mqtt_is_connected && this->message_handlers[topic].count_handlers() == 1) { + if (this->mqtt_is_connected && this->message_handlers.at(topic).count_handlers() == 1) { EVLOG_verbose << fmt::format("Subscribing to {}", topic); this->subscribe(topic, qos); } - EVLOG_verbose << fmt::format("#handler[{}] = {}", topic, this->message_handlers[topic].count_handlers()); + EVLOG_verbose << fmt::format("#handler[{}] = {}", topic, this->message_handlers.at(topic).count_handlers()); } void MQTTAbstractionImpl::unregister_handler(const std::string& topic, const Token& token) { From 1e4f79e42f79f886c421ffe1424b9aabb1b93bcd Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Fri, 3 Jan 2025 13:10:11 +0100 Subject: [PATCH 33/39] Exit early with EXIT_FAILURE if there are no modules to start Signed-off-by: Kai-Uwe Hermann --- src/manager.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/manager.cpp b/src/manager.cpp index 958d7179..8b58f17a 100644 --- a/src/manager.cpp +++ b/src/manager.cpp @@ -277,6 +277,10 @@ static std::map start_modules(ManagerConfig& config, MQTTAbs const auto& main_config = config.get_main_config(); const auto& module_names = config.get_module_names(); const auto number_of_modules = main_config.size(); + if (number_of_modules == 0) { + EVLOG_info << "No modules to start"; + return {}; + } EVLOG_info << "Starting " << number_of_modules << " modules"; modules_to_spawn.reserve(main_config.size()); @@ -749,6 +753,9 @@ int boot(const po::variables_map& vm) { auto module_handles = start_modules(*config, mqtt_abstraction, ignored_modules, standalone_modules, ms, status_fifo, retain_topics); + if (module_handles.empty()) { + return EXIT_FAILURE; + } bool modules_started = true; bool restart_modules = false; From 9b68f6d1f6eb4f1acd94585bbf2e903e23d7452f Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Mon, 6 Jan 2025 10:08:24 +0100 Subject: [PATCH 34/39] Make logged duration of get_module_config less verbose Signed-off-by: Kai-Uwe Hermann --- lib/module_config.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/module_config.cpp b/lib/module_config.cpp index fdb9f2ec..c2de3283 100644 --- a/lib/module_config.cpp +++ b/lib/module_config.cpp @@ -172,8 +172,9 @@ json get_module_config(const std::shared_ptr& mqtt, const std:: const auto end_time = std::chrono::system_clock::now(); - EVLOG_info << "get_module_config(" << module_id - << "): " << std::chrono::duration_cast(end_time - start_time).count() << "ms"; + EVLOG_debug << "get_module_config(" << module_id + << "): " << std::chrono::duration_cast(end_time - start_time).count() + << "ms"; return result; } From abad2b8efa51b9061d3fbf5b4aed887d667a91e1 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Tue, 7 Jan 2025 14:08:23 +0100 Subject: [PATCH 35/39] Remove redundant unique_ptrs in Validators struct Signed-off-by: Kai-Uwe Hermann --- include/utils/config.hpp | 13 ++++++------- lib/config.cpp | 16 ++++++++-------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/include/utils/config.hpp b/include/utils/config.hpp index da80cf23..39f9ef0f 100644 --- a/include/utils/config.hpp +++ b/include/utils/config.hpp @@ -39,11 +39,11 @@ struct Schemas { }; struct Validators { - std::unique_ptr config; - std::unique_ptr manifest; - std::unique_ptr type; - std::unique_ptr interface; - std::unique_ptr error_declaration_list; + nlohmann::json_schema::json_validator config; + nlohmann::json_schema::json_validator manifest; + nlohmann::json_schema::json_validator type; + nlohmann::json_schema::json_validator interface; + nlohmann::json_schema::json_validator error_declaration_list; }; struct SchemaValidation { @@ -361,8 +361,7 @@ class Config : public ConfigBase { /// \brief loads and validates a json schema at the provided \p path /// /// \returns the loaded json schema as a json object as well as a related schema validator - static std::tuple> - load_schema(const fs::path& path); + static std::tuple load_schema(const fs::path& path); /// /// \brief loads all module manifests relative to the \p main_dir diff --git a/lib/config.cpp b/lib/config.cpp index 41368740..b99a20a8 100644 --- a/lib/config.cpp +++ b/lib/config.cpp @@ -513,7 +513,7 @@ void ManagerConfig::load_and_validate_manifest(const std::string& module_id, con this->manifests[module_name] = load_yaml(manifest_path); } - const auto patch = this->validators.manifest->validate(this->manifests[module_name]); + const auto patch = this->validators.manifest.validate(this->manifests[module_name]); if (!patch.is_null()) { // extend manifest with default values this->manifests[module_name] = this->manifests[module_name].patch(patch); @@ -675,7 +675,7 @@ json ManagerConfig::load_interface_file(const std::string& intf_name) { // this subschema can not use allOf with the draft-07 schema because that will cause our validator to // add all draft-07 default values which never validate (the {"not": true} default contradicts everything) // --> validating against draft-07 will be done in an extra step below - auto patch = this->validators.interface->validate(interface_json); + auto patch = this->validators.interface.validate(interface_json); if (!patch.is_null()) { // extend config entry with default values interface_json = interface_json.patch(patch); @@ -1100,7 +1100,7 @@ ManagerConfig::ManagerConfig(const ManagerSettings& ms) : ConfigBase(ms.mqtt_set EVLOG_verbose << "No user-config provided."; } - const auto patch = this->validators.config->validate(complete_config); + const auto patch = this->validators.config.validate(complete_config); if (!patch.is_null()) { // extend config with default values complete_config = complete_config.patch(patch); @@ -1298,7 +1298,7 @@ SchemaValidation Config::load_schemas(const fs::path& schemas_dir) { return schema_validation; } -std::tuple> +std::tuple Config::load_schema(const fs::path& path) { BOOST_LOG_FUNCTION(); @@ -1311,16 +1311,16 @@ Config::load_schema(const fs::path& path) { json schema = load_yaml(path); - auto validator = std::make_unique(loader, format_checker); + auto validator = nlohmann::json_schema::json_validator(loader, format_checker); try { - validator->set_root_schema(schema); + validator.set_root_schema(schema); } catch (const std::exception& e) { EVLOG_AND_THROW(EverestInternalError( fmt::format("Validation of schema '{}' failed, here is why: {}", path.string(), e.what()))); } - return std::make_tuple>( + return std::make_tuple( std::move(schema), std::move(validator)); } @@ -1345,7 +1345,7 @@ json Config::load_all_manifests(const std::string& modules_dir, const std::strin try { manifests[module_name] = load_yaml(manifest_path); - schema_validation.validators.manifest->validate(manifests.at(module_name)); + schema_validation.validators.manifest.validate(manifests.at(module_name)); } catch (const std::exception& e) { EVLOG_AND_THROW(EverestConfigError( fmt::format("Failed to load and parse module manifest file of module {}: {}", module_name, e.what()))); From 3814cddf57617b22be53ef86455b7ea3127ba4a9 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Tue, 7 Jan 2025 14:12:49 +0100 Subject: [PATCH 36/39] Turn load_schema and load_schemas into free functions Signed-off-by: Kai-Uwe Hermann --- include/utils/config.hpp | 26 +++++----- lib/config.cpp | 106 +++++++++++++++++++-------------------- 2 files changed, 66 insertions(+), 66 deletions(-) diff --git a/include/utils/config.hpp b/include/utils/config.hpp index 39f9ef0f..2f3de54d 100644 --- a/include/utils/config.hpp +++ b/include/utils/config.hpp @@ -74,6 +74,19 @@ void loader(const nlohmann::json_uri& uri, nlohmann::json& schema); /// validator supporting uris void format_checker(const std::string& format, const std::string& value); +/// +/// \brief loads and validates a json schema at the provided \p path +/// +/// \returns the loaded json schema as a json object as well as a related schema validator +std::tuple load_schema(const fs::path& path); + +/// +/// \brief loads the config.json and manifest.json in the schemes subfolder of +/// the provided \p schemas_dir +/// +/// \returns the loaded configs and related validators +SchemaValidation load_schemas(const fs::path& schemas_dir); + /// /// \brief Base class for configs /// @@ -350,19 +363,6 @@ class Config : public ConfigBase { /// otherwise void ref_loader(const nlohmann::json_uri& uri, nlohmann::json& schema); - /// - /// \brief loads the config.json and manifest.json in the schemes subfolder of - /// the provided \p schemas_dir - /// - /// \returns the loaded configs and related validators - static SchemaValidation load_schemas(const fs::path& schemas_dir); - - /// - /// \brief loads and validates a json schema at the provided \p path - /// - /// \returns the loaded json schema as a json object as well as a related schema validator - static std::tuple load_schema(const fs::path& path); - /// /// \brief loads all module manifests relative to the \p main_dir /// diff --git a/lib/config.cpp b/lib/config.cpp index b99a20a8..871836bf 100644 --- a/lib/config.cpp +++ b/lib/config.cpp @@ -76,6 +76,57 @@ void format_checker(const std::string& format, const std::string& value) { } } +std::tuple +load_schema(const fs::path& path) { + BOOST_LOG_FUNCTION(); + + if (!fs::exists(path)) { + EVLOG_AND_THROW( + EverestInternalError(fmt::format("Schema file does not exist at: {}", fs::absolute(path).string()))); + } + + EVLOG_debug << fmt::format("Loading schema file at: {}", fs::canonical(path).string()); + + json schema = load_yaml(path); + + auto validator = nlohmann::json_schema::json_validator(loader, format_checker); + + try { + validator.set_root_schema(schema); + } catch (const std::exception& e) { + EVLOG_AND_THROW(EverestInternalError( + fmt::format("Validation of schema '{}' failed, here is why: {}", path.string(), e.what()))); + } + + return std::make_tuple( + std::move(schema), std::move(validator)); +} + +SchemaValidation load_schemas(const fs::path& schemas_dir) { + BOOST_LOG_FUNCTION(); + SchemaValidation schema_validation; + + EVLOG_debug << fmt::format("Loading base schema files for config and manifests... from: {}", schemas_dir.string()); + auto [config_schema, config_val] = load_schema(schemas_dir / "config.yaml"); + schema_validation.schemas.config = config_schema; + schema_validation.validators.config = std::move(config_val); + auto [manifest_schema, manifest_val] = load_schema(schemas_dir / "manifest.yaml"); + schema_validation.schemas.manifest = manifest_schema; + schema_validation.validators.manifest = std::move(manifest_val); + auto [interface_schema, interface_val] = load_schema(schemas_dir / "interface.yaml"); + schema_validation.schemas.interface = interface_schema; + schema_validation.validators.interface = std::move(interface_val); + auto [type_schema, type_val] = load_schema(schemas_dir / "type.yaml"); + schema_validation.schemas.type = type_schema; + schema_validation.validators.type = std::move(type_val); + auto [error_declaration_list_schema, error_declaration_list_val] = + load_schema(schemas_dir / "error-declaration-list.yaml"); + schema_validation.schemas.error_declaration_list = error_declaration_list_schema; + schema_validation.validators.error_declaration_list = std::move(error_declaration_list_val); + + return schema_validation; +} + static void validate_config_schema(const json& config_map_schema) { // iterate over every config entry json_validator validator(loader, format_checker); @@ -1073,7 +1124,7 @@ ManagerConfig::ManagerConfig(const ManagerSettings& ms) : ConfigBase(ms.mqtt_set this->interfaces = json({}); this->interface_definitions = json({}); this->types = json({}); - auto schema_validation = Config::load_schemas(this->ms.schemas_dir); + auto schema_validation = load_schemas(this->ms.schemas_dir); this->schemas = schema_validation.schemas; this->validators = std::move(schema_validation.validators); this->error_map = error::ErrorTypeMap(this->ms.errors_dir); @@ -1273,63 +1324,12 @@ void Config::ref_loader(const json_uri& uri, json& schema) { EVTHROW(EverestInternalError(fmt::format("{} is not supported for schema loading at the moment\n", uri.url()))); } -SchemaValidation Config::load_schemas(const fs::path& schemas_dir) { - BOOST_LOG_FUNCTION(); - SchemaValidation schema_validation; - - EVLOG_debug << fmt::format("Loading base schema files for config and manifests... from: {}", schemas_dir.string()); - auto [config_schema, config_val] = Config::load_schema(schemas_dir / "config.yaml"); - schema_validation.schemas.config = config_schema; - schema_validation.validators.config = std::move(config_val); - auto [manifest_schema, manifest_val] = Config::load_schema(schemas_dir / "manifest.yaml"); - schema_validation.schemas.manifest = manifest_schema; - schema_validation.validators.manifest = std::move(manifest_val); - auto [interface_schema, interface_val] = Config::load_schema(schemas_dir / "interface.yaml"); - schema_validation.schemas.interface = interface_schema; - schema_validation.validators.interface = std::move(interface_val); - auto [type_schema, type_val] = Config::load_schema(schemas_dir / "type.yaml"); - schema_validation.schemas.type = type_schema; - schema_validation.validators.type = std::move(type_val); - auto [error_declaration_list_schema, error_declaration_list_val] = - Config::load_schema(schemas_dir / "error-declaration-list.yaml"); - schema_validation.schemas.error_declaration_list = error_declaration_list_schema; - schema_validation.validators.error_declaration_list = std::move(error_declaration_list_val); - - return schema_validation; -} - -std::tuple -Config::load_schema(const fs::path& path) { - BOOST_LOG_FUNCTION(); - - if (!fs::exists(path)) { - EVLOG_AND_THROW( - EverestInternalError(fmt::format("Schema file does not exist at: {}", fs::absolute(path).string()))); - } - - EVLOG_debug << fmt::format("Loading schema file at: {}", fs::canonical(path).string()); - - json schema = load_yaml(path); - - auto validator = nlohmann::json_schema::json_validator(loader, format_checker); - - try { - validator.set_root_schema(schema); - } catch (const std::exception& e) { - EVLOG_AND_THROW(EverestInternalError( - fmt::format("Validation of schema '{}' failed, here is why: {}", path.string(), e.what()))); - } - - return std::make_tuple( - std::move(schema), std::move(validator)); -} - json Config::load_all_manifests(const std::string& modules_dir, const std::string& schemas_dir) { BOOST_LOG_FUNCTION(); json manifests = json({}); - auto schema_validation = Config::load_schemas(schemas_dir); + auto schema_validation = load_schemas(schemas_dir); const fs::path modules_path = fs::path(modules_dir); From ad1ecdaf818ade6a4c4bc519a4a219f21be91bb1 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Tue, 7 Jan 2025 14:26:50 +0100 Subject: [PATCH 37/39] Move get and get_async from MQTTAbstraction to module config since it is only used there Signed-off-by: Kai-Uwe Hermann --- include/utils/mqtt_abstraction.hpp | 8 ---- include/utils/mqtt_abstraction_impl.hpp | 8 ---- include/utils/types.hpp | 5 --- lib/module_config.cpp | 60 +++++++++++++++++++++++-- lib/mqtt_abstraction.cpp | 10 ----- lib/mqtt_abstraction_impl.cpp | 49 -------------------- 6 files changed, 57 insertions(+), 83 deletions(-) diff --git a/include/utils/mqtt_abstraction.hpp b/include/utils/mqtt_abstraction.hpp index 1ff3754e..403e2dd8 100644 --- a/include/utils/mqtt_abstraction.hpp +++ b/include/utils/mqtt_abstraction.hpp @@ -68,14 +68,6 @@ class MQTTAbstraction { /// \copydoc MQTTAbstractionImpl::clear_retained_topics() void clear_retained_topics(); - /// - /// \copydoc MQTTAbstractionImpl::get_async(const std::string&, QOS) - AsyncReturn get_async(const std::string& topic, QOS qos); - - /// - /// \copydoc MQTTAbstractionImpl::get(const std::string&, QOS) - nlohmann::json get(const std::string& topic, QOS qos); - /// /// \brief Get MQTT topic prefix for the "everest" topic const std::string& get_everest_prefix() const; diff --git a/include/utils/mqtt_abstraction_impl.hpp b/include/utils/mqtt_abstraction_impl.hpp index 70c34ddc..756926af 100644 --- a/include/utils/mqtt_abstraction_impl.hpp +++ b/include/utils/mqtt_abstraction_impl.hpp @@ -84,14 +84,6 @@ class MQTTAbstractionImpl { /// \brief clears any previously published topics that had the retain flag set void clear_retained_topics(); - /// - /// \brief subscribe topic to asynchronously get value on the subscribed topic - AsyncReturn get_async(const std::string& topic, QOS qos); - - /// - /// \brief subscribe and wait for value on the subscribed topic - nlohmann::json get(const std::string& topic, QOS qos); - /// /// \brief Spawn a thread running the mqtt main loop /// \returns a future, which will be fulfilled on thread termination diff --git a/include/utils/types.hpp b/include/utils/types.hpp index 4e3c660a..5e35eb5c 100644 --- a/include/utils/types.hpp +++ b/include/utils/types.hpp @@ -61,11 +61,6 @@ struct TypedHandler { using Token = std::shared_ptr; -struct AsyncReturn { - std::future future; - Token token; -}; - /// \brief MQTT Quality of service enum class QOS { QOS0, ///< At most once delivery diff --git a/lib/module_config.cpp b/lib/module_config.cpp index c2de3283..878394f5 100644 --- a/lib/module_config.cpp +++ b/lib/module_config.cpp @@ -14,15 +14,69 @@ #include namespace Everest { +struct AsyncReturn { + std::future future; + Token token; +}; + using json = nlohmann::json; inline constexpr int mqtt_get_config_timeout_ms = 5000; using FutureCallback = std::tuple, std::string>; +AsyncReturn get_async(const std::shared_ptr& mqtt, const std::string& topic, QOS qos) { + auto res_promise = std::make_shared>(); + std::future res_future = res_promise->get_future(); + + auto res_handler = [res_promise](const std::string& topic, json data) mutable { + res_promise->set_value(std::move(data)); + }; + + const auto res_token = + std::make_shared(HandlerType::GetConfig, std::make_shared(res_handler)); + mqtt->register_handler(topic, res_token, QOS::QOS2); + + return {std::move(res_future), res_token}; +} + +json get(const std::shared_ptr& mqtt, const std::string& topic, QOS qos) { + BOOST_LOG_FUNCTION(); + std::promise res_promise; + std::future res_future = res_promise.get_future(); + + const auto res_handler = [&res_promise](const std::string& topic, json data) { + res_promise.set_value(std::move(data)); + }; + + const std::shared_ptr res_token = + std::make_shared(HandlerType::GetConfig, std::make_shared(res_handler)); + mqtt->register_handler(topic, res_token, QOS::QOS2); + + // wait for result future + const std::chrono::time_point res_wait = + std::chrono::steady_clock::now() + std::chrono::milliseconds(mqtt_get_config_timeout_ms); + std::future_status res_future_status; + do { + res_future_status = res_future.wait_until(res_wait); + } while (res_future_status == std::future_status::deferred); + + json result; + if (res_future_status == std::future_status::timeout) { + mqtt->unregister_handler(topic, res_token); + EVLOG_AND_THROW(EverestTimeoutError(fmt::format("Timeout while waiting for result of get({})", topic))); + } + if (res_future_status == std::future_status::ready) { + result = res_future.get(); + } + mqtt->unregister_handler(topic, res_token); + + return result; +} + void populate_future_cbs(std::vector& future_cbs, const std::shared_ptr& mqtt, const std::string& everest_prefix, const std::string& topic, json& out) { future_cbs.push_back(std::make_tuple>( - mqtt->get_async(topic, QOS::QOS2), + get_async(mqtt, topic, QOS::QOS2), [topic, &everest_prefix, &out](json result) { out = std::move(result); @@ -57,7 +111,7 @@ void populate_future_cbs_arr(std::vector& future_cbs, const std: const std::string& everest_prefix, const std::string& topic, const std::string& inner_topic_part, json& array_out, json& out) { future_cbs.push_back(std::make_tuple, std::string>( - mqtt->get_async(topic, QOS::QOS2), + get_async(mqtt, topic, QOS::QOS2), [topic, &everest_prefix, &mqtt, &inner_topic_part, &array_out, &out](const json& result_array) { array_out = result_array; std::vector array_future_cbs; @@ -68,7 +122,7 @@ void populate_future_cbs_arr(std::vector& future_cbs, const std: for (const auto& key : keys) { auto key_topic = fmt::format("{}{}{}", everest_prefix, inner_topic_part, key); array_future_cbs.push_back(std::make_tuple, std::string>( - mqtt->get_async(key_topic, QOS::QOS2), + get_async(mqtt, key_topic, QOS::QOS2), [&key, key_topic, &out](const json& key_response) { out[key] = key_response; return key_topic; diff --git a/lib/mqtt_abstraction.cpp b/lib/mqtt_abstraction.cpp index e1a78eb5..09351390 100644 --- a/lib/mqtt_abstraction.cpp +++ b/lib/mqtt_abstraction.cpp @@ -76,16 +76,6 @@ void MQTTAbstraction::clear_retained_topics() { mqtt_abstraction->clear_retained_topics(); } -AsyncReturn MQTTAbstraction::get_async(const std::string& topic, QOS qos) { - BOOST_LOG_FUNCTION(); - return mqtt_abstraction->get_async(topic, qos); -} - -json MQTTAbstraction::get(const std::string& topic, QOS qos) { - BOOST_LOG_FUNCTION(); - return mqtt_abstraction->get(topic, qos); -} - const std::string& MQTTAbstraction::get_everest_prefix() const { BOOST_LOG_FUNCTION(); return everest_prefix; diff --git a/lib/mqtt_abstraction_impl.cpp b/lib/mqtt_abstraction_impl.cpp index 96561d38..6c4dc9d7 100644 --- a/lib/mqtt_abstraction_impl.cpp +++ b/lib/mqtt_abstraction_impl.cpp @@ -226,55 +226,6 @@ void MQTTAbstractionImpl::clear_retained_topics() { retained_topics.clear(); } -AsyncReturn MQTTAbstractionImpl::get_async(const std::string& topic, QOS qos) { - auto res_promise = std::make_shared>(); - std::future res_future = res_promise->get_future(); - - auto res_handler = [this, res_promise](const std::string& topic, json data) mutable { - res_promise->set_value(std::move(data)); - }; - - const auto res_token = - std::make_shared(HandlerType::GetConfig, std::make_shared(res_handler)); - this->register_handler(topic, res_token, QOS::QOS2); - - return {std::move(res_future), res_token}; -} - -json MQTTAbstractionImpl::get(const std::string& topic, QOS qos) { - BOOST_LOG_FUNCTION(); - std::promise res_promise; - std::future res_future = res_promise.get_future(); - - const auto res_handler = [this, &res_promise](const std::string& topic, json data) { - res_promise.set_value(std::move(data)); - }; - - const std::shared_ptr res_token = - std::make_shared(HandlerType::GetConfig, std::make_shared(res_handler)); - this->register_handler(topic, res_token, QOS::QOS2); - - // wait for result future - const std::chrono::time_point res_wait = - std::chrono::steady_clock::now() + std::chrono::milliseconds(mqtt_get_timeout_ms); - std::future_status res_future_status; - do { - res_future_status = res_future.wait_until(res_wait); - } while (res_future_status == std::future_status::deferred); - - json result; - if (res_future_status == std::future_status::timeout) { - this->unregister_handler(topic, res_token); - EVLOG_AND_THROW(EverestTimeoutError(fmt::format("Timeout while waiting for result of get({})", topic))); - } - if (res_future_status == std::future_status::ready) { - result = res_future.get(); - } - this->unregister_handler(topic, res_token); - - return result; -} - void MQTTAbstractionImpl::notify_write_data() { // FIXME (aw): error handling eventfd_write(this->event_fd, 1); From 96835c046fa743967f7b16e1b14b1231221111d5 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Tue, 7 Jan 2025 14:30:13 +0100 Subject: [PATCH 38/39] load_schema and load_schemas: clang-format Signed-off-by: Kai-Uwe Hermann --- lib/config.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/config.cpp b/lib/config.cpp index 871836bf..f4383a00 100644 --- a/lib/config.cpp +++ b/lib/config.cpp @@ -76,8 +76,7 @@ void format_checker(const std::string& format, const std::string& value) { } } -std::tuple -load_schema(const fs::path& path) { +std::tuple load_schema(const fs::path& path) { BOOST_LOG_FUNCTION(); if (!fs::exists(path)) { @@ -98,8 +97,8 @@ load_schema(const fs::path& path) { fmt::format("Validation of schema '{}' failed, here is why: {}", path.string(), e.what()))); } - return std::make_tuple( - std::move(schema), std::move(validator)); + return std::make_tuple(std::move(schema), + std::move(validator)); } SchemaValidation load_schemas(const fs::path& schemas_dir) { From b42680a882dc4b512cf22772332523d001b64a87 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Tue, 7 Jan 2025 15:16:44 +0100 Subject: [PATCH 39/39] Revert to old file loading code for redability reasons Signed-off-by: Kai-Uwe Hermann --- lib/yaml_loader.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/lib/yaml_loader.cpp b/lib/yaml_loader.cpp index 5ef781cf..a69cbae8 100644 --- a/lib/yaml_loader.cpp +++ b/lib/yaml_loader.cpp @@ -86,12 +86,7 @@ static nlohmann::ordered_json ryml_to_nlohmann_json(const c4::yml::NodeRef& ryml static std::string load_file_content(const std::filesystem::path& path) { std::ifstream ifs(path.string()); - ifs.seekg(0, std::ios::end); - std::string content; - content.reserve(ifs.tellg()); - ifs.seekg(0); - content.assign(std::istreambuf_iterator(ifs), std::istreambuf_iterator()); - return content; + return std::string(std::istreambuf_iterator(ifs), std::istreambuf_iterator()); } static std::string load_yaml_content(std::filesystem::path path) {