Dbus connection contained in manager client or external (#188)

* create helper method to generate name for dbus connection

* reusing same connection within operations exe

* small refactor of dbus interface owning for slot

* broken build but wip

* exposing injectable dbus interface to config dbus client

* exposing injectable dbus interface in confman

* make it somewhat work

* fix incomplete code

* fix compile errors

* add name to dbus connection

* add missing if stmt in cmake

* Committing clang-format changes

* Exposing Type as json schema for ipc slots

* new state filter works strictly on unfiltered raw value from signal

* add getter for unfiltered value

exes/ethercat/tests/devices/beckhoff.cpp

@@ -22,7 +22,7 @@ using ut::expect;
 template <typename device_t>
 struct test_vars {
   asio::io_context ctx{};
-  ipc_manager_client_mock connect_interface{};
+  ipc_manager_client_mock connect_interface{ ctx };
   device_t device;
 };
 auto main(int argc, const char* argv[]) -> int {

exes/operations/inc/details/app_operation_mode_impl.hpp

@@ -16,10 +16,9 @@ template <template <typename, typename> typename signal_t, template <typename, t
 app_operation_mode<signal_t, slot_t>::app_operation_mode(boost::asio::io_context& ctx)
     : dbus_{ std::make_shared<sdbusplus::asio::connection>(ctx, tfc::dbus::sd_bus_open_system()) },
       dbus_object_server_{ std::make_unique<sdbusplus::asio::object_server>(dbus_) },
-      dbus_interface_{ dbus_object_server_->add_interface(
-          std::string{ operation::dbus::path.data(), operation::dbus::path.size() },
-          std::string{ operation::dbus::name.data(), operation::dbus::name.size() }) },
-      state_machine_{ std::make_unique<state_machine_owner_t>(ctx) }, logger_{ "app_operation_mode" } {
+      dbus_interface_{ dbus_object_server_->add_interface(std::string{ operation::dbus::path },
+                                                          std::string{ operation::dbus::name }) },
+      state_machine_{ std::make_unique<state_machine_owner_t>(ctx, dbus_) }, logger_{ "app_operation_mode" } {
   dbus_interface_->register_signal<operation::update_message>(
       std::string(operation::dbus::signal::update.data(), operation::dbus::signal::update.size()));
 
@@ -40,7 +39,12 @@ app_operation_mode<signal_t, slot_t>::app_operation_mode(boost::asio::io_context
     message.signal_send();
   });
 
-  dbus_->request_name(operation::dbus::name.data());
+  auto service_name{ tfc::dbus::make_dbus_process_name() };
+  if (service_name != tfc::operation::dbus::service_name) {
+    logger_.info("Service name '{}' is not default '{}'", service_name, tfc::operation::dbus::service_default);
+  }
+
+  dbus_->request_name(service_name.c_str());
 
   dbus_interface_->initialize();
 

exes/operations/inc/details/state_machine_owner_impl.hpp

@@ -10,9 +10,11 @@ namespace tfc::operation {
 // clang-format off
 template <template <typename, typename> typename signal_t, template <typename, typename> typename slot_t, template <typename, typename...> typename sml_t>
 // clang-format on
-state_machine_owner<signal_t, slot_t, sml_t>::state_machine_owner(asio::io_context& ctx)
-    : ctx_{ ctx }, states_{ std::make_shared<state_machine_t>(detail::state_machine<state_machine_owner>{ *this },
-                                                              tfc::logger::sml_logger{}) } {}
+state_machine_owner<signal_t, slot_t, sml_t>::state_machine_owner(asio::io_context& ctx,
+                                                                  std::shared_ptr<sdbusplus::asio::connection> conn)
+    : ctx_{ ctx }, dbus_{ std::move(conn) },
+      states_{ std::make_shared<state_machine_t>(detail::state_machine<state_machine_owner>{ *this },
+                                                 tfc::logger::sml_logger{}) } {}
 
 // clang-format off
 template <template <typename, typename> typename signal_t, template <typename, typename> typename slot_t, template <typename, typename...> typename sml_t>

exes/operations/inc/state_machine_owner.hpp

@@ -5,6 +5,7 @@
 #include <boost/sml.hpp>
 
 #include <tfc/confman.hpp>
+#include <tfc/dbus/sdbusplus_fwd.hpp>
 #include <tfc/ipc.hpp>
 #include <tfc/ipc/details/dbus_client_iface.hpp>
 #include <tfc/ipc/details/type_description.hpp>
@@ -46,7 +47,7 @@ template <template <typename description_t, typename manager_client_t = ipc_rule
           template <typename, typename...> typename sml_t = boost::sml::sm>
 class state_machine_owner {
 public:
-  explicit state_machine_owner(asio::io_context&);
+  explicit state_machine_owner(asio::io_context&, std::shared_ptr<sdbusplus::asio::connection>);
 
   auto set_mode(tfc::operation::mode_e new_mode) -> std::error_code;
 
@@ -94,12 +95,13 @@ class state_machine_owner {
 private:
   std::function<void(new_mode, old_mode)> on_new_state_{};
   asio::io_context& ctx_;
+  std::shared_ptr<sdbusplus::asio::connection> dbus_;
 
   using bool_signal_t = signal_t<ipc::details::type_bool>;
   using bool_slot_t = slot_t<ipc::details::type_bool>;
   using string_signal_t = signal_t<ipc::details::type_string>;
   using uint_signal_t = signal_t<ipc::details::type_uint>;
-  ipc_ruler::ipc_manager_client mclient_{ ctx_ };
+  ipc_ruler::ipc_manager_client mclient_{ dbus_ };
   bool_signal_t stopped_{ ctx_, mclient_, "stopped" };
   bool_signal_t starting_{ ctx_, mclient_, "starting" };
   bool_signal_t running_{ ctx_, mclient_, "running" };

exes/operations/tests/operation_mode_integration_test.cpp

@@ -21,7 +21,7 @@ struct operation_mode_test {
   asio::io_context ctx{};
   tfc::app_operation_mode<tfc::ipc::mock_signal, tfc::ipc::mock_slot> app{ ctx };
   tfc::operation::interface lib {
-    ctx
+    ctx, "operation", tfc::dbus::make_dbus_process_name()
   };
   ~operation_mode_test() {
     std::error_code code;

exes/operations/tests/operations_state_machine_owner_test.cpp

@@ -68,7 +68,8 @@ struct state_machine_mock {
 
 struct test_instance {
   asio::io_context ctx{};
-  tfc::operation::state_machine_owner<tfc::ipc::mock_signal, tfc::ipc::mock_slot, state_machine_mock> owner{ ctx };
+  std::shared_ptr<sdbusplus::asio::connection> dbus{ std::make_shared<sdbusplus::asio::connection>(ctx) };
+  tfc::operation::state_machine_owner<tfc::ipc::mock_signal, tfc::ipc::mock_slot, state_machine_mock> owner{ ctx, dbus };
 };
 
 auto main(int argc, char** argv) -> int {

exes/tfcctl/src/tfcctl.cpp

@@ -102,7 +102,9 @@ auto main(int argc, char** argv) -> int {
         [signal_name, &in_logger]<typename receiver_t>(receiver_t&& receiver) {
           if constexpr (!std::same_as<std::monostate, std::remove_cvref_t<receiver_t>>) {
             in_logger.trace("Connecting to signal {}", signal_name);
-            auto error = receiver->connect(signal_name);
+            std::string sig{ signal_name };
+            auto error =
+                receiver->connect(signal_name, [sig, &in_logger](auto const& val) { in_logger.info("{}: {}", sig, val); });
             if (error) {
               in_logger.warn("Failed to connect: {}", error.message());
             }
@@ -119,8 +121,7 @@ auto main(int argc, char** argv) -> int {
       if (type == ipc::details::type_e::unknown) {
         throw std::runtime_error{ fmt::format("Unknown typename in: {}\n", sig) };
       }
-      auto ipc{ ipc::details::make_any_slot_cb::make(type, ctx, slot_name,
-                                                     [sig, &logger](auto const& val) { logger.info("{}: {}", sig, val); }) };
+      auto ipc{ ipc::details::make_any_slot_cb::make(type, ctx, slot_name) };
       slot_connect(ipc, sig, logger);
       return ipc;
     }(signal_connect));

libs/confman/inc/public/tfc/confman.hpp

@@ -1,9 +1,14 @@
 #pragma once
 
+#include <filesystem>
 #include <functional>
+#include <memory>
+#include <string>
 #include <string_view>
+#include <system_error>
 #include <type_traits>
 
+#include <fmt/format.h>
 #include <glaze/glaze.hpp>
 
 #include <tfc/confman/detail/change.hpp>
@@ -29,18 +34,26 @@ class config {
   using type = config_storage_t;
   using storage_t = config_storage_t;
 
-  /// \brief construct config and deliver it to config manager
+  /// \brief construct config and expose it via dbus
   /// \param ctx context ref to which the config shall run in
   /// \param key identification of this config storage, requires to be unique
   config(asio::io_context& ctx, std::string_view key) : config{ ctx, key, config_storage_t{} } {}
 
-  /// \brief construct config and deliver it to config manager
+  /// \brief construct config and expose it via dbus
   /// \param conn valid dbus connection
   /// \param key identification of this config storage, requires to be unique
   config(std::shared_ptr<sdbusplus::asio::connection> conn, std::string_view key)
       : config{ conn, key, config_storage_t{} } {}
 
-  /// \brief construct config and deliver it to config manager
+  /// \brief construct config and expose it via dbus
+  /// \param interface valid dbus interface
+  /// \param key identification of this config storage, requires to be unique
+  /// \note the config will be accessible via dbus property on the `interface` using the given `key`
+  /// Take care that via this construction the frontend won't detect the config and show it automatically
+  config(std::shared_ptr<sdbusplus::asio::dbus_interface> interface, std::string_view key)
+      : config{ interface, key, config_storage_t{} } {}
+
+  /// \brief construct config and expose it via dbus
   /// \param ctx context ref to which the config shall run in
   /// \param key identification of this config storage, requires to be unique
   /// \param def default values of given storage type
@@ -49,16 +62,12 @@ class config {
   config(asio::io_context& ctx, std::string_view key, storage_type&& def)
       : client_{ ctx, key, std::bind_front(&config::string, this), std::bind_front(&config::schema, this),
                  std::bind_front(&config::from_string, this) },
-        storage_{ client_.io_context(), tfc::base::make_config_file_name(key, "json"), std::forward<storage_type>(def) },
+        storage_{ ctx, tfc::base::make_config_file_name(key, "json"), std::forward<storage_type>(def) },
         logger_(fmt::format("config.{}", key)) {
-    client_.initialize();
-    storage_.on_change([]() {
-      // todo this can lead too callback hell, set property calls dbus set prop and dbus set prop calls back
-      //      client_.set(detail::config_property{ .value = string(), .schema = schema() });
-    });
+    init();
   }
 
-  /// \brief construct config and deliver it to config manager
+  /// \brief construct config and expose it via dbus
   /// \param conn valid dbus connection
   /// \param key identification of this config storage, requires to be unique
   /// \param def default values of given storage type
@@ -69,11 +78,23 @@ class config {
                  std::bind_front(&config::from_string, this) },
         storage_{ client_.io_context(), tfc::base::make_config_file_name(key, "json"), std::forward<storage_type>(def) },
         logger_(fmt::format("config.{}", key)) {
-    client_.initialize();
-    storage_.on_change([]() {
-      // todo this can lead too callback hell, set property calls dbus set prop and dbus set prop calls back
-      //      client_.set(detail::config_property{ .value = string(), .schema = schema() });
-    });
+    init();
+  }
+
+  /// \brief construct config and expose it via dbus
+  /// \param interface valid dbus interface
+  /// \param key identification of this config storage, requires to be unique
+  /// \param def default values of given storage type
+  /// \note the config will be accessible via dbus property on the `interface` using the given `key`
+  /// Take care that via this construction the frontend won't detect the config and show it automatically
+  template <typename storage_type>
+    requires std::same_as<storage_t, std::remove_cvref_t<storage_type>>
+  config(std::shared_ptr<sdbusplus::asio::dbus_interface> interface, std::string_view key, storage_type&& def)
+      : client_{ interface, key, std::bind_front(&config::string, this), std::bind_front(&config::schema, this),
+                 std::bind_front(&config::from_string, this) },
+        storage_{ client_.get_io_context(), tfc::base::make_config_file_name(key, "json"), std::forward<storage_type>(def) },
+        logger_(fmt::format("config.{}", key)) {
+    init();
   }
 
   /// \brief Advanced constructor providing file storage interface and dbus client
@@ -136,6 +157,14 @@ class config {
   }
 
 protected:
+  void init() {
+    client_.initialize();
+    storage_.on_change([]() {
+      // todo this can lead too callback hell, set property calls dbus set prop and dbus set prop calls back
+      //      client_.set(detail::config_property{ .value = string(), .schema = schema() });
+    });
+  }
+
   friend struct detail::change<config>;
 
   // todo if this could be named `value` it would be neat

libs/confman/inc/public/tfc/confman/detail/config_dbus_client.hpp

@@ -2,9 +2,12 @@
 
 #include <filesystem>
 #include <functional>
+#include <memory>
 #include <string_view>
+#include <system_error>
 
 #include <tfc/dbus/sdbusplus_fwd.hpp>
+#include <tfc/dbus/string_maker.hpp>
 #include <tfc/stx/to_tuple.hpp>
 
 namespace boost::asio {
@@ -30,11 +33,14 @@ struct config_property {
 
 namespace dbus {
 static constexpr std::string_view property_name{ "config" };
-}
+static constexpr std::string_view config_path{ "Config" };
+static constexpr std::string_view path{ tfc::dbus::const_dbus_path<config_path> };
+}  // namespace dbus
 
 class config_dbus_client {
 public:
   using dbus_connection_t = std::shared_ptr<sdbusplus::asio::connection>;
+  using interface_t = std::shared_ptr<sdbusplus::asio::dbus_interface>;
 
   /// \brief Empty constructor
   /// \note Should only be used for testing !!!
@@ -43,24 +49,31 @@ class config_dbus_client {
   using value_call_t = std::function<std::string()>;
   using schema_call_t = std::function<std::string()>;
   using change_call_t = std::function<std::error_code(std::string_view)>;
+  /// \brief make dbus client using io_context
+  /// Create a new dbus connection with a property named `config` using the given `key` to make interface name
   config_dbus_client(asio::io_context& ctx, std::string_view key, value_call_t&&, schema_call_t&&, change_call_t&&);
+  /// \brief make dbus client using given dbus connection
+  /// Create a property named `config` using the given `key` to make interface name
   config_dbus_client(dbus_connection_t conn, std::string_view key, value_call_t&&, schema_call_t&&, change_call_t&&);
-
-  [[nodiscard]] auto io_context() const noexcept -> asio::io_context& { return ctx_; }
+  /// \brief make dbus client using given interface
+  /// Create a property with the given `key`
+  config_dbus_client(interface_t intf, std::string_view key, value_call_t&&, schema_call_t&&, change_call_t&&);
 
   void set(config_property&&) const;
 
   void initialize();
 
+  auto get_io_context() const noexcept -> asio::io_context&;
+
 private:
-  asio::io_context& ctx_;
   std::filesystem::path interface_path_{};
   std::string interface_name_{};
+  std::string property_name_{ dbus::property_name };
   value_call_t value_call_{};
   schema_call_t schema_call_{};
   change_call_t change_call_{};
   dbus_connection_t dbus_connection_{};
-  std::unique_ptr<sdbusplus::asio::dbus_interface, std::function<void(sdbusplus::asio::dbus_interface*)>> dbus_interface_{};
+  std::shared_ptr<sdbusplus::asio::dbus_interface> dbus_interface_{};
 };
 
 }  // namespace tfc::confman::detail