Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Dbus connection contained in manager client or external #188

Merged
merged 33 commits into from
Oct 10, 2023
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
4253f58
create helper method to generate name for dbus connection
jbbjarnason Oct 8, 2023
ff891ed
reusing same connection within operations exe
jbbjarnason Oct 8, 2023
5e5f5e1
small refactor of dbus interface owning for slot
jbbjarnason Oct 8, 2023
6f36774
broken build but wip
jbbjarnason Oct 8, 2023
ddb751d
exposing injectable dbus interface to config dbus client
jbbjarnason Oct 9, 2023
ee29e42
exposing injectable dbus interface in confman
jbbjarnason Oct 9, 2023
8944130
wip moving new state detection to filter
jbbjarnason Oct 9, 2023
e106cb3
wip moving new state detection to filter
jbbjarnason Oct 9, 2023
8cee350
wip going out of scope
jbbjarnason Oct 9, 2023
8a07d6b
make it somewhat work
jbbjarnason Oct 9, 2023
5967676
fix incomplete code
jbbjarnason Oct 9, 2023
3956003
fix compile errors
jbbjarnason Oct 9, 2023
7a562bd
add name to dbus connection
jbbjarnason Oct 9, 2023
80f6aab
add missing if stmt in cmake
jbbjarnason Oct 9, 2023
4c4cde4
remove files
jbbjarnason Oct 9, 2023
9939946
Committing clang-format changes
jbbjarnason Oct 9, 2023
ca05f49
sonar being stupid
jbbjarnason Oct 9, 2023
f86e4d1
me being stupid ?
jbbjarnason Oct 9, 2023
2103908
nope sonar being stupid
jbbjarnason Oct 9, 2023
d0a2863
format
jbbjarnason Oct 9, 2023
18f47cd
Exposing Type as json schema for ipc slots
jbbjarnason Oct 10, 2023
ef5e4ac
new state filter works strictly on unfiltered raw value from signal
jbbjarnason Oct 10, 2023
bb30e1c
add getter for unfiltered value
jbbjarnason Oct 10, 2023
b92f098
fix gcc warning
jbbjarnason Oct 10, 2023
3fc29f1
disable sonar
jbbjarnason Oct 10, 2023
3e444f0
move disable sonar
jbbjarnason Oct 10, 2023
71dce6c
fix tests
jbbjarnason Oct 10, 2023
bf5cc59
naming
jbbjarnason Oct 10, 2023
7a67cac
fix operation mode after service name changes
jbbjarnason Oct 10, 2023
d69ef4f
fix segfault in test
jbbjarnason Oct 10, 2023
689ef6b
fix confman tests
jbbjarnason Oct 10, 2023
e2cef61
damn boy fix ipc
jbbjarnason Oct 10, 2023
f0f76eb
Committing clang-format changes
jbbjarnason Oct 10, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 0 additions & 48 deletions .github/workflows/vcpkg-install-test.yml
omarhogni marked this conversation as resolved.
Show resolved Hide resolved

This file was deleted.

2 changes: 1 addition & 1 deletion exes/ethercat/tests/devices/beckhoff.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
14 changes: 9 additions & 5 deletions exes/operations/inc/details/app_operation_mode_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()));

Expand All @@ -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();

Expand Down
8 changes: 5 additions & 3 deletions exes/operations/inc/details/state_machine_owner_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
6 changes: 4 additions & 2 deletions exes/operations/inc/state_machine_owner.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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" };
Expand Down
2 changes: 1 addition & 1 deletion exes/operations/tests/operation_mode_integration_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{};
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 {
Expand Down
7 changes: 4 additions & 3 deletions exes/tfcctl/src/tfcctl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand All @@ -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));
Expand Down
59 changes: 44 additions & 15 deletions libs/confman/inc/public/tfc/confman.hpp
Original file line number Diff line number Diff line change
@@ -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>
Expand All @@ -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`
omarhogni marked this conversation as resolved.
Show resolved Hide resolved
/// 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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
21 changes: 16 additions & 5 deletions libs/confman/inc/public/tfc/confman/detail/config_dbus_client.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

#include <filesystem>
#include <functional>
#include <memory>
#include <string_view>
#include <system_error>

#include <tfc/dbus/sdbusplus_fwd.hpp>
#include <tfc/stx/to_tuple.hpp>
Expand Down Expand Up @@ -30,11 +32,13 @@ struct config_property {

namespace dbus {
static constexpr std::string_view property_name{ "config" };
}
static constexpr std::string_view config_path{ "Config" };
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed path of config, should we change the property_name as well to capital first letter? @SindriTh

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me, what does the DBUS naming standard say?

} // 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 !!!
Expand All @@ -43,24 +47,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
Loading
Loading