From 1c74e347457365bc0a5c4f564ade8b1faf82f9af Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Thu, 1 Jan 2026 11:52:02 +0100 Subject: [PATCH 01/12] Fix clang warnings Fix modernize warnings in find_if calls. Fix unused include warning. Fix passing const values by value. Fix marking nodiscard warning Signed-off-by: Eduardo Gonzalez --- examples/connmanctl.cpp | 29 +++++++++++------------ include/amarula/dbus/connman/gmanager.hpp | 4 +++- src/dbus/gconnman_manager.cpp | 11 ++++----- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/examples/connmanctl.cpp b/examples/connmanctl.cpp index c943fa6..27c2c5d 100644 --- a/examples/connmanctl.cpp +++ b/examples/connmanctl.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include @@ -19,7 +20,7 @@ auto main() -> int { bool connecting = false; Connman connman; const auto manager = connman.manager(); - manager->onRequestInputPassphrase([&](auto service) -> auto { + manager->onRequestInputPassphrase([&](const auto& service) -> auto { const auto name = service->properties().getName(); std::string passphrase; @@ -46,14 +47,13 @@ auto main() -> int { } } // Trim whitespace - line.erase(line.begin(), - std::find_if(line.begin(), line.end(), [](int character) { + line.erase(line.begin(), std::ranges::find_if(line, [](int character) { return std::isspace(character) == 0; })); - line.erase(std::find_if(line.rbegin(), line.rend(), - [](int character) { - return std::isspace(character) == 0; - }) + line.erase(std::ranges::find_if(std::ranges::reverse_view(line), + [](int character) { + return std::isspace(character) == 0; + }) .base(), line.end()); @@ -118,9 +118,8 @@ auto main() -> int { << service->objPath() << "\n"; } } else { - auto iterator = std::find_if( - services.begin(), services.end(), - [&arg](const auto& service) { + auto iterator = std::ranges::find_if( + services, [&arg](const auto& service) { return service->objPath() == arg || service->properties().getName() == arg; }); @@ -225,8 +224,8 @@ auto main() -> int { } const bool connect = (cmd == "connect"); const auto services = manager->services(); - auto iterator = std::find_if( - services.begin(), services.end(), [&arg](const auto& service) { + auto iterator = + std::ranges::find_if(services, [&arg](const auto& service) { return service->objPath() == arg || service->properties().getName() == arg; }); @@ -279,8 +278,8 @@ auto main() -> int { continue; } const auto services = manager->services(); - auto iterator = std::find_if( - services.begin(), services.end(), [&arg](const auto& service) { + auto iterator = + std::ranges::find_if(services, [&arg](const auto& service) { return service->objPath() == arg || service->properties().getName() == arg; }); @@ -311,4 +310,4 @@ auto main() -> int { } std::cout << "Exiting.\n"; return 0; -} \ No newline at end of file +} diff --git a/include/amarula/dbus/connman/gmanager.hpp b/include/amarula/dbus/connman/gmanager.hpp index e7d6393..8db7c4f 100644 --- a/include/amarula/dbus/connman/gmanager.hpp +++ b/include/amarula/dbus/connman/gmanager.hpp @@ -123,7 +123,9 @@ class Manager : public DBusProxy { void onTechnologiesChanged(OnTechListChangedCallback callback); void onServicesChanged(OnServListChangedCallback callback); - auto internalAgentPath() const -> std::string { return agent_->path_; }; + [[nodiscard]] auto internalAgentPath() const -> std::string { + return agent_->path_; + }; private: enum class InputType : std::uint8_t { diff --git a/src/dbus/gconnman_manager.cpp b/src/dbus/gconnman_manager.cpp index f6ad993..46921b3 100644 --- a/src/dbus/gconnman_manager.cpp +++ b/src/dbus/gconnman_manager.cpp @@ -12,9 +12,9 @@ #include #include #include -#include #include #include +#include #include #include #include @@ -95,11 +95,10 @@ void Manager::setup_agent() { std::lock_guard const lock(mtx_); GVariantBuilder builder; g_variant_builder_init(&builder, G_VARIANT_TYPE("a{sv}")); - auto service_it = - std::find_if(services_.begin(), services_.end(), - [&service_path](const auto service) { - return service->objPath() == service_path; - }); + auto service_it = std::ranges::find_if( + services_, [&service_path](const auto& service) { + return service->objPath() == service_path; + }); if (service_it != services_.end()) { auto* parsed_fields = parse_fields(fields); From ccf105e3ee40b88a57de27d4e3ef769a3dfedfa1 Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Wed, 1 Apr 2026 16:18:00 +0200 Subject: [PATCH 02/12] CMakeLists: Remove unused includes Remove unused CPackIFW and CTest include statements, this solves #18. Signed-off-by: Eduardo Gonzalez --- CMakeLists.txt | 2 -- 1 file changed, 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 70ff9eb..9c6a281 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -88,7 +88,6 @@ if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME) add_subdirectory(tests) endif(BUILD_TESTS) if(BUILD_EXAMPLES) - include(CTest) add_subdirectory(examples) endif(BUILD_EXAMPLES) if(BUILD_DOCS) @@ -106,7 +105,6 @@ if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME) ) set(CPACK_PACKAGE_FILE_NAME ${cpack_file_name}) include(CPack) - include(CPackIFW) cpack_add_component(${PROJECT_NAME}) endif(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME) From 6718a47a65d1e2fcc5af7d4512df590b5c841b6b Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Wed, 1 Apr 2026 11:15:51 +0200 Subject: [PATCH 03/12] gconnman_serv_test: Wired can not be removed Calling this method(remove) on Ethernet devices, hidden WiFi services or provisioned services will cause an error message. It is not possible to remove these kind of services. https://git.kernel.org/pub/scm/network/connman/connman.git/tree/doc/ service-api.txt#n98 Signed-off-by: Eduardo Gonzalez --- tests/gconnman_serv_test.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/gconnman_serv_test.cpp b/tests/gconnman_serv_test.cpp index ff54298..5260e80 100644 --- a/tests/gconnman_serv_test.cpp +++ b/tests/gconnman_serv_test.cpp @@ -12,6 +12,7 @@ using Amarula::DBus::G::Connman::Connman; using Error = Amarula::DBus::G::Connman::ServProperties::Error; using State = Amarula::DBus::G::Connman::ServProperties::State; using Type = Amarula::DBus::G::Connman::TechProperties::Type; +using ServType = Amarula::DBus::G::Connman::ServProperties::Type; TEST(Connman, getServs) { bool called = false; @@ -96,7 +97,8 @@ TEST(Connman, ForgetAndDisconnectService) { const auto name = props.getName(); const auto state = props.getState(); - if (props.getError() != Error::None || props.isFavorite()) { + if ((props.getError() != Error::None || props.isFavorite()) && + props.getType() != ServType::Ethernet) { std::cout << "Removing service: " << name << '\n'; serv->remove([serv, name](bool success) { EXPECT_TRUE(success); From 0cc47de85326302fd31dc0cbb951497e94a08ff7 Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Wed, 18 Mar 2026 16:47:27 +0100 Subject: [PATCH 04/12] .gitignore: Ignore CMakeLists.txt.user Signed-off-by: Eduardo Gonzalez --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 195a574..9fed214 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ build* doc tags CMakeUserPresets.json +CMakeLists.txt.user From 0cc9899609697b6df7380b00ca8f05029e71f635 Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Wed, 7 Jan 2026 16:27:10 +0100 Subject: [PATCH 05/12] Proxy: remove passing proxy member in methods Signed-off-by: Eduardo Gonzalez --- include/amarula/dbus/gproxy.hpp | 18 +++++++++--------- src/dbus/gconnman_clock.cpp | 12 ++++++------ src/dbus/gconnman_manager.cpp | 10 +++++----- src/dbus/gconnman_service.cpp | 14 +++++++------- src/dbus/gconnman_technology.cpp | 6 +++--- 5 files changed, 30 insertions(+), 30 deletions(-) diff --git a/include/amarula/dbus/gproxy.hpp b/include/amarula/dbus/gproxy.hpp index d7a7f77..ca2a311 100644 --- a/include/amarula/dbus/gproxy.hpp +++ b/include/amarula/dbus/gproxy.hpp @@ -135,7 +135,7 @@ class DBusProxy : public std::enable_shared_from_this> { void getProperties(PropertiesCallback callback = nullptr) { auto data = prepareCallback(std::move(callback)); - callMethod(proxy_, nullptr, "GetProperties", nullptr, + callMethod(nullptr, "GetProperties", nullptr, &DBusProxy::get_property_cb, data.release()); } @@ -226,25 +226,25 @@ class DBusProxy : public std::enable_shared_from_this> { self->template executeCallBack(counter, success); } - static void setProperty(GDBusProxy* proxy, const gchar* arg_name, - GVariant* arg_value, GCancellable* cancellable, - GAsyncReadyCallback callback, gpointer user_data + void setProperty(const gchar* arg_name, GVariant* arg_value, + GCancellable* cancellable, GAsyncReadyCallback callback, + gpointer user_data ) { std::array tuple_elements{ g_variant_new_string(arg_name), g_variant_new_variant(arg_value)}; GVariant* parameters = g_variant_new_tuple(tuple_elements.data(), 2); - g_dbus_proxy_call(proxy, "SetProperty", parameters, + g_dbus_proxy_call(proxy_, "SetProperty", parameters, G_DBUS_CALL_FLAGS_NONE, -1, cancellable, callback, user_data); } - static void callMethod(GDBusProxy* proxy, GCancellable* cancellable, - const gchar* arg_name, GVariant* parameters, - GAsyncReadyCallback callback, gpointer user_data) { + void callMethod(GCancellable* cancellable, const gchar* arg_name, + GVariant* parameters, GAsyncReadyCallback callback, + gpointer user_data) { g_dbus_proxy_call( - proxy, arg_name, + proxy_, arg_name, (parameters != nullptr) ? parameters : g_variant_new_tuple(nullptr, 0), G_DBUS_CALL_FLAGS_NONE, -1, cancellable, callback, user_data); diff --git a/src/dbus/gconnman_clock.cpp b/src/dbus/gconnman_clock.cpp index 1765413..2cc45a9 100644 --- a/src/dbus/gconnman_clock.cpp +++ b/src/dbus/gconnman_clock.cpp @@ -52,22 +52,22 @@ Clock::Clock(DBus* dbus) void Clock::setTime(uint64_t time, PropertiesSetCallback callback) { auto data = prepareCallback(std::move(callback)); - setProperty(proxy(), TIME_STR, g_variant_new_uint64(time), nullptr, + setProperty(TIME_STR, g_variant_new_uint64(time), nullptr, &Clock::finishAsyncCall, data.release()); } void Clock::setTimeZone(const std::string& timezone, PropertiesSetCallback callback) { auto data = prepareCallback(std::move(callback)); - setProperty(proxy(), TIMEZONE_STR, g_variant_new_string(timezone.c_str()), - nullptr, &Clock::finishAsyncCall, data.release()); + setProperty(TIMEZONE_STR, g_variant_new_string(timezone.c_str()), nullptr, + &Clock::finishAsyncCall, data.release()); } void Clock::setTimeUpdates(const Properties::TimeUpdate time_updates, PropertiesSetCallback callback) { auto data = prepareCallback(std::move(callback)); setProperty( - proxy(), TIMEUPDATES_STR, + TIMEUPDATES_STR, g_variant_new_string((TIME_UPDATE_MAP.toString(time_updates)).data()), nullptr, &Clock::finishAsyncCall, data.release()); } @@ -76,7 +76,7 @@ void Clock::setTimeZoneUpdates( const Properties::TimeZoneUpdate time_zone_updates, PropertiesSetCallback callback) { auto data = prepareCallback(std::move(callback)); - setProperty(proxy(), TIMEZONEUPDATES_STR, + setProperty(TIMEZONEUPDATES_STR, g_variant_new_string( (TIME_ZONE_UPDATE_MAP.toString(time_zone_updates)).data()), nullptr, &Clock::finishAsyncCall, data.release()); @@ -92,7 +92,7 @@ void Clock::setTimeServers(const std::vector& servers, g_variant_builder_add_value(&builder, str_variant); } GVariant* servers_variant = g_variant_builder_end(&builder); - setProperty(proxy(), TIMESERVERS_STR, servers_variant, nullptr, + setProperty(TIMESERVERS_STR, servers_variant, nullptr, &Clock::finishAsyncCall, data.release()); g_variant_builder_clear(&builder); } diff --git a/src/dbus/gconnman_manager.cpp b/src/dbus/gconnman_manager.cpp index 46921b3..5f11a21 100644 --- a/src/dbus/gconnman_manager.cpp +++ b/src/dbus/gconnman_manager.cpp @@ -199,7 +199,7 @@ void Manager::setup_agent() { void Manager::setOfflineMode(bool offline_mode, PropertiesSetCallback callback) { auto data = prepareCallback(std::move(callback)); - setProperty(proxy(), OFFLINEMODE_STR, + setProperty(OFFLINEMODE_STR, g_variant_new_boolean(static_cast(offline_mode)), nullptr, &Manager::finishAsyncCall, data.release()); } @@ -276,12 +276,12 @@ void Manager::get_proxies_cb(GObject* proxy, GAsyncResult* res, } void Manager::get_technologies() { - callMethod(proxy(), nullptr, GETTECHNOLOGIES_STR, nullptr, + callMethod(nullptr, GETTECHNOLOGIES_STR, nullptr, &Manager::get_proxies_cb, this); } void Manager::get_services() { - callMethod(proxy(), nullptr, GETSERVICES_STR, nullptr, + callMethod(nullptr, GETSERVICES_STR, nullptr, &Manager::get_proxies_cb, this); } @@ -290,7 +290,7 @@ void Manager::registerAgent(const std::string& object_path, auto data = prepareCallback(std::move(callback)); GVariant* child = g_variant_new_object_path(object_path.c_str()); GVariant* parameters = g_variant_new_tuple(&child, 1); - callMethod(proxy(), nullptr, REGISTERAGENT_STR, parameters, + callMethod(nullptr, REGISTERAGENT_STR, parameters, &Manager::finishAsyncCall, data.release()); } @@ -299,7 +299,7 @@ void Manager::unregisterAgent(const std::string& object_path, auto data = prepareCallback(std::move(callback)); GVariant* child = g_variant_new_object_path(object_path.c_str()); GVariant* parameters = g_variant_new_tuple(&child, 1); - callMethod(proxy(), nullptr, UNREGISTERAGENT_STR, parameters, + callMethod(nullptr, UNREGISTERAGENT_STR, parameters, &Manager::finishAsyncCall, data.release()); } diff --git a/src/dbus/gconnman_service.cpp b/src/dbus/gconnman_service.cpp index 6e5fdf3..f5fbcd3 100644 --- a/src/dbus/gconnman_service.cpp +++ b/src/dbus/gconnman_service.cpp @@ -89,26 +89,26 @@ Service::Service(DBus* dbus, const gchar* obj_path) void Service::connect(PropertiesSetCallback callback) { auto data = prepareCallback(std::move(callback)); - callMethod(proxy(), nullptr, CONNECT_STR, nullptr, - &Service::finishAsyncCall, data.release()); + callMethod(nullptr, CONNECT_STR, nullptr, &Service::finishAsyncCall, + data.release()); } void Service::disconnect(PropertiesSetCallback callback) { auto data = prepareCallback(std::move(callback)); - callMethod(proxy(), nullptr, DISCONNECT_STR, nullptr, - &Service::finishAsyncCall, data.release()); + callMethod(nullptr, DISCONNECT_STR, nullptr, &Service::finishAsyncCall, + data.release()); } void Service::remove(PropertiesSetCallback callback) { auto data = prepareCallback(std::move(callback)); - callMethod(proxy(), nullptr, REMOVE_STR, nullptr, &Service::finishAsyncCall, + callMethod(nullptr, REMOVE_STR, nullptr, &Service::finishAsyncCall, data.release()); } void Service::setAutoconnect(const bool autoconnect, PropertiesSetCallback callback) { auto data = prepareCallback(std::move(callback)); - setProperty(proxy(), AUTOCONNECT_STR, + setProperty(AUTOCONNECT_STR, g_variant_new_boolean(static_cast(autoconnect)), nullptr, &Service::finishAsyncCall, data.release()); } @@ -117,7 +117,7 @@ void Service::setNameServers(const std::vector& name_servers, PropertiesSetCallback callback) { auto data = prepareCallback(std::move(callback)); auto variant = vector_to_as(name_servers); - setProperty(proxy(), NAMESERVERS_CONFIGURATION_STR, variant.get(), nullptr, + setProperty(NAMESERVERS_CONFIGURATION_STR, variant.get(), nullptr, &Service::finishAsyncCall, data.release()); } diff --git a/src/dbus/gconnman_technology.cpp b/src/dbus/gconnman_technology.cpp index ea1fbd8..d94a80b 100644 --- a/src/dbus/gconnman_technology.cpp +++ b/src/dbus/gconnman_technology.cpp @@ -30,15 +30,15 @@ Technology::Technology(DBus* dbus, const gchar* obj_path) void Technology::setPowered(bool powered, PropertiesSetCallback callback) { auto data = prepareCallback(std::move(callback)); - setProperty(proxy(), POWERED_STR, + setProperty(POWERED_STR, g_variant_new_boolean(static_cast(powered)), nullptr, &Technology::finishAsyncCall, data.release()); } void Technology::scan(PropertiesSetCallback callback) { auto data = prepareCallback(std::move(callback)); - callMethod(proxy(), nullptr, SCAN_STR, nullptr, - &Technology::finishAsyncCall, data.release()); + callMethod(nullptr, SCAN_STR, nullptr, &Technology::finishAsyncCall, + data.release()); } void TechProperties::update(const gchar* key, GVariant* value) { From 09acff0aec7a3eb20740670680da2bcc33be5f62 Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Fri, 20 Mar 2026 17:55:06 +0100 Subject: [PATCH 06/12] gproxy.hpp: Reuse callMethod Signed-off-by: Eduardo Gonzalez --- include/amarula/dbus/gproxy.hpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/include/amarula/dbus/gproxy.hpp b/include/amarula/dbus/gproxy.hpp index ca2a311..eabd425 100644 --- a/include/amarula/dbus/gproxy.hpp +++ b/include/amarula/dbus/gproxy.hpp @@ -235,9 +235,7 @@ class DBusProxy : public std::enable_shared_from_this> { g_variant_new_string(arg_name), g_variant_new_variant(arg_value)}; GVariant* parameters = g_variant_new_tuple(tuple_elements.data(), 2); - g_dbus_proxy_call(proxy_, "SetProperty", parameters, - G_DBUS_CALL_FLAGS_NONE, -1, cancellable, callback, - user_data); + callMethod(cancellable, "SetProperty", parameters, callback, user_data); } void callMethod(GCancellable* cancellable, const gchar* arg_name, From b5f2caf58231db9029a20aa1247ec6351d563722 Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Fri, 20 Mar 2026 16:55:02 +0100 Subject: [PATCH 07/12] tests: Add check thread id of callbacks Iterate all the test files in the CMake configuration in order to remove duplicated code. Define a class that iterates the global default context simulating what Qt libraries does. The latter class saves the thread id where the object is created and the thread id where the global default context it is iterated. Use the class on the tests to check that callbacks are not run on the main thread nor in the thread iterating the global default context. This helps to test the issue presented in #21. Signed-off-by: Eduardo Gonzalez --- tests/CMakeLists.txt | 22 ++--- tests/gconnman_clock_test.cpp | 45 +++++++-- tests/gconnman_serv_test.cpp | 166 ++++++++++++++++++++++++---------- tests/gconnman_tech_test.cpp | 96 +++++++++++++------- tests/qt_thread_bundle.hpp | 50 ++++++++++ 5 files changed, 281 insertions(+), 98 deletions(-) create mode 100644 tests/qt_thread_bundle.hpp diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index fe5356e..432499a 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -20,21 +20,21 @@ install( COMPONENT ${PROJECT_NAME}-dev) if(BUILD_CONNMAN) - add_executable(gconnman_clock_test gconnman_clock_test.cpp) - target_link_libraries(gconnman_clock_test PRIVATE GConnmanDbus gtest_main) + foreach(connman_test gconnman_clock_test gconnman_tech_test + gconnman_serv_test) + add_executable(${connman_test} ${connman_test}.cpp qt_thread_bundle.hpp) + target_link_libraries(${connman_test} PRIVATE GConnmanDbus gtest_main) - add_executable(gconnman_tech_test gconnman_tech_test.cpp) - target_link_libraries(gconnman_tech_test PRIVATE GConnmanDbus gtest_main) + target_include_directories(${connman_test} + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) - add_executable(gconnman_serv_test gconnman_serv_test.cpp) - target_link_libraries(gconnman_serv_test PRIVATE GConnmanDbus gtest_main) + install( + TARGETS ${connman_test} + EXPORT ${PROJECT_NAME}-config + COMPONENT ${PROJECT_NAME}-dev) + endforeach() - install( - TARGETS gconnman_clock_test gconnman_tech_test gconnman_serv_test - EXPORT ${PROJECT_NAME}-config - COMPONENT ${PROJECT_NAME}-dev) endif(BUILD_CONNMAN) - add_executable(log_test log_test.cpp) target_link_libraries(log_test PRIVATE GDbusProxy gtest_main) diff --git a/tests/gconnman_clock_test.cpp b/tests/gconnman_clock_test.cpp index b402906..095ee7c 100644 --- a/tests/gconnman_clock_test.cpp +++ b/tests/gconnman_clock_test.cpp @@ -7,6 +7,8 @@ #include #include +#include "qt_thread_bundle.hpp" + using Amarula::DBus::G::Connman::Connman; using TimeUpdate = Amarula::DBus::G::Connman::ClockProperties::TimeUpdate; using TimeZoneUpdate = TimeUpdate; @@ -16,16 +18,29 @@ constexpr auto TEST_TIME_ZONE = "America/Vancouver"; constexpr int SLEEP_DURATION_SECONDS = 3; TEST(Connman, ClockSetTimeUpdates) { + const QtThreadBundle qt_thread_bundle; const Connman connman; - connman.clock()->onPropertyChanged([](auto& props) { - std::cout << "onPropertyChanged:\n"; - props.print(); - }); + connman.clock()->onPropertyChanged( + [main_tid = qt_thread_bundle.main_tid, + loop_tid = qt_thread_bundle.loop_tid](auto& props) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + std::cout << "onPropertyChanged:\n"; + props.print(); + }); connman.clock()->setTimeUpdates( - TimeUpdate::Auto, [&connman](auto success) { EXPECT_TRUE(success); }); + TimeUpdate::Auto, [&connman, main_tid = qt_thread_bundle.main_tid, + loop_tid = qt_thread_bundle.loop_tid](auto success) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + EXPECT_TRUE(success); + }); } TEST(Connman, ClockSetTime) { + const QtThreadBundle qt_thread_bundle; const Connman connman; const guint time = TEST_TIME; connman.clock()->onPropertyChanged([](auto& props) { @@ -46,6 +61,7 @@ TEST(Connman, ClockSetTime) { } TEST(Connman, ClockSetTimeServers1) { + const QtThreadBundle qt_thread_bundle; const Connman connman; connman.clock()->onPropertyChanged([](auto& props) { std::cout << "onPropertyChanged:\n"; @@ -58,6 +74,7 @@ TEST(Connman, ClockSetTimeServers1) { } TEST(Connman, ClockSetTimeServers2) { + const QtThreadBundle qt_thread_bundle; const Connman connman; connman.clock()->onPropertyChanged([](auto& props) { std::cout << "onPropertyChanged:\n"; @@ -75,6 +92,7 @@ TEST(Connman, ClockSetTimeServers2) { } TEST(Connman, ClockSetTimeZoneUpdates) { + const QtThreadBundle qt_thread_bundle; const Connman connman; connman.clock()->onPropertyChanged([](auto& props) { std::cout << "onPropertyChanged:\n"; @@ -86,6 +104,7 @@ TEST(Connman, ClockSetTimeZoneUpdates) { } TEST(Connman, ClockSetTimeZone) { + const QtThreadBundle qt_thread_bundle; const Connman connman; connman.clock()->onPropertyChanged([](auto& props) { std::cout << "onPropertyChanged:\n"; @@ -106,14 +125,26 @@ TEST(Connman, ClockSetTimeZone) { } TEST(Connman, ClockProxyInitialization) { - EXPECT_NO_THROW({ const Connman connman; }); + EXPECT_NO_THROW({ + const QtThreadBundle qt_thread_bundle; + const Connman connman; + }); } TEST(Connman, clockGetPropertiesNull) { + const QtThreadBundle qt_thread_bundle; const Connman connman; connman.clock()->getProperties(); } TEST(Connman, ClockGetProperties) { + const QtThreadBundle qt_thread_bundle; const Connman connman; - connman.clock()->getProperties([](auto& props) { props.print(); }); + connman.clock()->getProperties( + [main_tid = qt_thread_bundle.main_tid, + loop_tid = qt_thread_bundle.loop_tid](auto& props) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + props.print(); + }); } diff --git a/tests/gconnman_serv_test.cpp b/tests/gconnman_serv_test.cpp index 5260e80..49b1ff4 100644 --- a/tests/gconnman_serv_test.cpp +++ b/tests/gconnman_serv_test.cpp @@ -7,6 +7,8 @@ #include #include +#include "qt_thread_bundle.hpp" + using Amarula::DBus::G::Connman::Connman; using Error = Amarula::DBus::G::Connman::ServProperties::Error; @@ -17,14 +19,23 @@ using ServType = Amarula::DBus::G::Connman::ServProperties::Type; TEST(Connman, getServs) { bool called = false; { + const QtThreadBundle qt_thread_bundle; const Connman connman; const auto manager = connman.manager(); - manager->onTechnologiesChanged([&](const auto& technologies) { + manager->onTechnologiesChanged([main_tid = qt_thread_bundle.main_tid, + loop_tid = qt_thread_bundle.loop_tid]( + const auto& technologies) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); ASSERT_FALSE(technologies.empty()) << "No technologies returned"; // Power on all technologies for (const auto& tech : technologies) { - tech->onPropertyChanged([&](const auto& prop) { + tech->onPropertyChanged([main_tid, loop_tid](const auto& prop) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); EXPECT_TRUE(prop.isPowered()) << "Technology " << prop.getName() << " was not powered ON"; @@ -34,7 +45,11 @@ TEST(Connman, getServs) { const auto prop = tech->properties(); const auto name = prop.getName(); if (!prop.isPowered()) { - tech->setPowered(true, [&, name](auto success) { + tech->setPowered(true, [main_tid, loop_tid, + name](auto success) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); EXPECT_TRUE(success) << "Set power for " << name << " did not succeed"; }); @@ -42,14 +57,19 @@ TEST(Connman, getServs) { } }); - manager->onServicesChanged([&](const auto& services) { - called = true; - ASSERT_FALSE(services.empty()); - for (const auto& serv : services) { - const auto props = serv->properties(); - props.print(); - } - }); + manager->onServicesChanged( + [&called, main_tid = qt_thread_bundle.main_tid, + loop_tid = qt_thread_bundle.loop_tid](const auto& services) { + called = true; + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + ASSERT_FALSE(services.empty()); + for (const auto& serv : services) { + const auto props = serv->properties(); + props.print(); + } + }); } ASSERT_TRUE(called) << "TechnologiesChanged callback was never called"; } @@ -57,27 +77,38 @@ TEST(Connman, getServs) { TEST(Connman, setNameServers) { bool called = false; { + const QtThreadBundle qt_thread_bundle; const Connman connman; const auto manager = connman.manager(); - manager->onServicesChanged([&](const auto& services) { - called = true; - ASSERT_FALSE(services.empty()); - for (const auto& serv : services) { - const auto props = serv->properties(); - const auto name = props.getName(); - props.print(); - serv->onPropertyChanged([](const auto& properties) { - std::cout << "onPropertyChange:\n"; - properties.print(); - }); - serv->setNameServers( - {"8.8.8.8", "4.4.4.4"}, [&, name](auto success) { - EXPECT_TRUE(success) << "Set setNameServers for " - << name << " did not succeed"; + manager->onServicesChanged( + [&called, main_tid = qt_thread_bundle.main_tid, + loop_tid = qt_thread_bundle.loop_tid](const auto& services) { + called = true; + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + ASSERT_FALSE(services.empty()); + for (const auto& serv : services) { + const auto props = serv->properties(); + const auto name = props.getName(); + props.print(); + serv->onPropertyChanged([](const auto& properties) { + std::cout << "onPropertyChange:\n"; + properties.print(); }); - } - }); + serv->setNameServers( + {"8.8.8.8", "4.4.4.4"}, + [name, main_tid, loop_tid](auto success) { + const auto callback_tid = + std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + EXPECT_TRUE(success) << "Set setNameServers for " + << name << " did not succeed"; + }); + } + }); } ASSERT_TRUE(called) << "TechnologiesChanged callback was never called"; } @@ -86,11 +117,18 @@ TEST(Connman, ForgetAndDisconnectService) { bool called = false; { + const QtThreadBundle qt_thread_bundle; const Connman connman; const auto manager = connman.manager(); - manager->onServicesChanged([&](const auto& services) { + manager->onServicesChanged([&called, + main_tid = qt_thread_bundle.main_tid, + loop_tid = qt_thread_bundle.loop_tid]( + const auto& services) { called = true; + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); ASSERT_FALSE(services.empty()) << "No services returned"; for (const auto& serv : services) { const auto props = serv->properties(); @@ -100,14 +138,21 @@ TEST(Connman, ForgetAndDisconnectService) { if ((props.getError() != Error::None || props.isFavorite()) && props.getType() != ServType::Ethernet) { std::cout << "Removing service: " << name << '\n'; - serv->remove([serv, name](bool success) { + serv->remove([serv, name, main_tid, + loop_tid](bool success) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); EXPECT_TRUE(success); std::cout << "Service removed: " << name << '\n'; serv->properties().print(); }); } else if (state == State::Ready || state == State::Online) { std::cout << "Disconnecting service: " << name << '\n'; - serv->disconnect([serv](bool success) { + serv->disconnect([serv, main_tid, loop_tid](bool success) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); EXPECT_TRUE(success); std::cout << "Service disconnected: " << serv->objPath() << '\n'; @@ -125,19 +170,31 @@ TEST(Connman, ConnectWifi) { bool called = false; bool called_request_input = false; { + const QtThreadBundle qt_thread_bundle; const Connman connman; const auto manager = connman.manager(); - manager->onRequestInputPassphrase([&](auto service) -> auto { - called_request_input = true; - std::cout << "Requesting input passphrase for service: " - << service->properties().getName() << '\n'; - const auto name = service->properties().getName(); - EXPECT_EQ(name, "connmantest"); - return std::pair{true, "amaruladev"}; - }); + manager->onRequestInputPassphrase( + [&called_request_input, main_tid = qt_thread_bundle.main_tid, + loop_tid = qt_thread_bundle.loop_tid](auto service) -> auto { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + called_request_input = true; + std::cout << "Requesting input passphrase for service: " + << service->properties().getName() << '\n'; + const auto name = service->properties().getName(); + EXPECT_EQ(name, "connmantest"); + return std::pair{true, "amaruladev"}; + }); - manager->onServicesChanged([&](const auto& services) { + manager->onServicesChanged([&called, manager, + main_tid = qt_thread_bundle.main_tid, + loop_tid = qt_thread_bundle.loop_tid]( + const auto& services) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); called = true; ASSERT_FALSE(services.empty()) << "No services returned"; @@ -153,15 +210,30 @@ TEST(Connman, ConnectWifi) { if (state == State::Idle) { std::cout << "Connecting to service: " << name << '\n'; props.print(); - serv->onPropertyChanged([](const auto& properties) { - std::cout << "onPropertyChange:\n"; - properties.print(); - }); + serv->onPropertyChanged( + [main_tid, loop_tid](const auto& properties) { + const auto callback_tid = + std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + std::cout << "onPropertyChange:\n"; + properties.print(); + }); manager->registerAgent( manager->internalAgentPath(), - [serv, manager](const auto success) { + [serv, manager, main_tid, + loop_tid](const auto success) { + const auto callback_tid = + std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); EXPECT_TRUE(success); - serv->connect([serv, manager](bool success) { + serv->connect([serv, manager, main_tid, + loop_tid](bool success) { + const auto callback_tid = + std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); EXPECT_TRUE(success); std::cout << "Service connected successfully: " @@ -192,4 +264,4 @@ TEST(Connman, ConnectWifi) { ASSERT_TRUE(called) << "ServicesChanged callback was never called"; ASSERT_TRUE(called_request_input) << "Did not requested user input"; -} \ No newline at end of file +} diff --git a/tests/gconnman_tech_test.cpp b/tests/gconnman_tech_test.cpp index 47c84c4..4be4010 100644 --- a/tests/gconnman_tech_test.cpp +++ b/tests/gconnman_tech_test.cpp @@ -3,28 +3,36 @@ #include #include +#include "qt_thread_bundle.hpp" + using Amarula::DBus::G::Connman::Connman; using Type = Amarula::DBus::G::Connman::TechProperties::Type; TEST(Connman, getTechs) { bool called = false; { + const QtThreadBundle qt_thread_bundle; Connman connman; const auto manager = connman.manager(); - manager->onTechnologiesChanged([&](const auto& technologies) { - called = true; - ASSERT_FALSE(technologies.empty()); - for (const auto& tech : technologies) { - const auto props = tech->properties(); - EXPECT_FALSE(props.getName().empty()); - if (props.isConnected()) { - EXPECT_TRUE(props.isPowered()) - << "Technology is connected but not powered"; + manager->onTechnologiesChanged( + [&called, main_tid = qt_thread_bundle.main_tid, + loop_tid = qt_thread_bundle.loop_tid](const auto& technologies) { + called = true; + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + ASSERT_FALSE(technologies.empty()); + for (const auto& tech : technologies) { + const auto props = tech->properties(); + EXPECT_FALSE(props.getName().empty()); + if (props.isConnected()) { + EXPECT_TRUE(props.isPowered()) + << "Technology is connected but not powered"; + } + props.print(); } - props.print(); - } - }); + }); } ASSERT_TRUE(called) << "TechnologiesChanged callback was never called"; } @@ -33,18 +41,27 @@ TEST(Connman, PowerOnAllTechnologies) { bool called = false; { + const QtThreadBundle qt_thread_bundle; Connman connman; const auto manager = connman.manager(); - manager->onTechnologiesChanged([&](const auto& technologies) { - ASSERT_FALSE(technologies.empty()) << "No technologies returned"; - + manager->onTechnologiesChanged([&called, + main_tid = qt_thread_bundle.main_tid, + loop_tid = qt_thread_bundle.loop_tid]( + const auto& technologies) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + ASSERT_FALSE(technologies.empty()); // Power on all technologies for (const auto& tech : technologies) { - tech->onPropertyChanged([&](const auto& prop) { + tech->onPropertyChanged([main_tid, loop_tid](const auto& prop) { EXPECT_TRUE(prop.isPowered()) << "Technology " << prop.getName() << " was not powered ON"; + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); std::cout << "onPropertyChanged:\n"; prop.print(); }); @@ -52,7 +69,11 @@ TEST(Connman, PowerOnAllTechnologies) { const auto name = prop.getName(); if (!prop.isPowered()) { std::cout << "Powering on technology: " << name << '\n'; - tech->setPowered(true, [&, name](auto success) { + tech->setPowered(true, [&called, name, main_tid, + loop_tid](auto success) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); std::cout << "setPowered callback for " << name << ": " << (success ? "Success" : "Failure") << '\n'; EXPECT_TRUE(success) @@ -70,27 +91,36 @@ TEST(Connman, PowerOnAllTechnologies) { TEST(Connman, ScanWifiTechnology) { bool called = false; { + const QtThreadBundle qt_thread_bundle; Connman connman; const auto manager = connman.manager(); - manager->onTechnologiesChanged([&](const auto& technologies) { - ASSERT_FALSE(technologies.empty()) << "No technologies returned"; + manager->onTechnologiesChanged( + [&called, main_tid = qt_thread_bundle.main_tid, + loop_tid = qt_thread_bundle.loop_tid](const auto& technologies) { + ASSERT_FALSE(technologies.empty()) + << "No technologies returned"; - for (const auto& tech : technologies) { - const auto props = tech->properties(); - const auto name = props.getName(); - if (props.getType() == Type::Wifi) { - std::cout << "Scanning technology with name: " << name - << "\n"; - tech->scan([&called, name](bool success) { - called = true; - EXPECT_TRUE(success); - std::cout << "Technology " << name - << " scanned successfully.\n"; - }); + for (const auto& tech : technologies) { + const auto props = tech->properties(); + const auto name = props.getName(); + if (props.getType() == Type::Wifi) { + std::cout << "Scanning technology with name: " << name + << "\n"; + tech->scan( + [&called, name, main_tid, loop_tid](bool success) { + const auto callback_tid = + std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + called = true; + EXPECT_TRUE(success); + std::cout << "Technology " << name + << " scanned successfully.\n"; + }); + } } - } - }); + }); } ASSERT_TRUE(called) << "TechnologiesChanged callback was never called"; } diff --git a/tests/qt_thread_bundle.hpp b/tests/qt_thread_bundle.hpp new file mode 100644 index 0000000..c9a0c15 --- /dev/null +++ b/tests/qt_thread_bundle.hpp @@ -0,0 +1,50 @@ +#pragma once + +#include + +#include +#include + +struct QtThreadBundle { + std::thread::id main_tid; + std::thread::id loop_tid; + + QtThreadBundle() + : main_tid(std::this_thread::get_id()), + loop_(g_main_loop_new(nullptr, FALSE)), + qt_simulator_([this]() { + this->loop_tid = std::this_thread::get_id(); + g_idle_add(&QtThreadBundle::on_loop_started, this); + g_main_loop_run(this->loop_); + g_main_loop_unref(this->loop_); + }) { + { + std::unique_lock lock(mtx_); + cv_.wait(lock, [this] { return running_; }); + } + } + + ~QtThreadBundle() { + if (qt_simulator_.joinable()) { + g_main_loop_quit(loop_); + qt_simulator_.join(); + } + } + + private: + std::mutex mtx_; + std::condition_variable cv_; + bool running_{false}; + GMainLoop* loop_; + std::thread qt_simulator_; + + static auto on_loop_started(gpointer user_data) -> gboolean { + auto* self = static_cast(user_data); + { + std::lock_guard const lock(self->mtx_); + self->running_ = true; + } + self->cv_.notify_all(); + return G_SOURCE_REMOVE; + } +}; From 6c6fdd80a5ace9295cc0293331bd5608a05a2a93 Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Fri, 20 Mar 2026 17:05:19 +0100 Subject: [PATCH 08/12] gdbus: Add thread default context Create a thread default context and iterate the later on the worker thread. This solves #21. Signed-off-by: Eduardo Gonzalez --- include/amarula/dbus/gdbus.hpp | 2 ++ src/dbus/gdbus.cpp | 20 +++++++++++++------- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/include/amarula/dbus/gdbus.hpp b/include/amarula/dbus/gdbus.hpp index 157561c..c132866 100644 --- a/include/amarula/dbus/gdbus.hpp +++ b/include/amarula/dbus/gdbus.hpp @@ -18,6 +18,7 @@ class DBus { unsigned int pending_calls_{0}; GDBusConnection* connection_ = nullptr; GMainLoop* loop_{nullptr}; + GMainContext* ctx_{nullptr}; static auto on_loop_started(gpointer user_data) -> gboolean; @@ -35,6 +36,7 @@ class DBus { void onAnyAsyncStart(); [[nodiscard]] auto connection() const { return connection_; } + [[nodiscard]] auto context() { return ctx_; } }; } // namespace Amarula::DBus::G diff --git a/src/dbus/gdbus.cpp b/src/dbus/gdbus.cpp index a4fb224..e4962f8 100644 --- a/src/dbus/gdbus.cpp +++ b/src/dbus/gdbus.cpp @@ -19,8 +19,10 @@ void DBus::onAnyAsyncDone() { void DBus::onAnyAsyncStart() { ++pending_calls_; } -DBus::DBus(const std::string& bus_name, const std::string& object_path) { +DBus::DBus(const std::string& bus_name, const std::string& object_path) + : ctx_{g_main_context_new()} { GError* error = nullptr; + g_main_context_push_thread_default(ctx_); connection_ = g_bus_get_sync(G_BUS_TYPE_SYSTEM, nullptr, &error); if (connection_ == nullptr) { std::string const msg = error->message; @@ -54,14 +56,15 @@ DBus::DBus(const std::string& bus_name, const std::string& object_path) { } g_variant_unref(result); + g_main_context_pop_thread_default(ctx_); start(); -} - -DBus::~DBus() { { std::unique_lock lock(mtx_); cv_.wait(lock, [this] { return running_; }); } +} + +DBus::~DBus() { { std::lock_guard const lock(mtx_); running_ = false; @@ -76,6 +79,7 @@ DBus::~DBus() { if (connection_ != nullptr) { g_object_unref(connection_); } + g_main_context_unref(ctx_); } auto DBus::on_loop_started(gpointer user_data) -> gboolean { @@ -91,13 +95,15 @@ auto DBus::on_loop_started(gpointer user_data) -> gboolean { void DBus::start() { if (!running_) { glib_thread_ = std::thread([this]() { - loop_ = g_main_loop_new(nullptr, FALSE); - g_idle_add(&DBus::on_loop_started, this); + g_main_context_invoke(ctx_, &DBus::on_loop_started, this); + g_main_context_push_thread_default(ctx_); + loop_ = g_main_loop_new(ctx_, FALSE); g_main_loop_run(loop_); + g_main_context_pop_thread_default(ctx_); g_main_loop_unref(loop_); loop_ = nullptr; }); } } -} // namespace Amarula::DBus::G \ No newline at end of file +} // namespace Amarula::DBus::G From 4c2c5d0faf38f35385ca305ff04428686db3a425 Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Wed, 18 Mar 2026 16:26:18 +0100 Subject: [PATCH 09/12] gproxy: use g_main_context_invoke in proxy callMethod Wrap the callMethod method inside a g_main_context_invoke. The latter makes that the callbacks are executed in the worker thread provided by the library. This solves #21. Signed-off-by: Eduardo Gonzalez --- include/amarula/dbus/gproxy.hpp | 46 ++++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 6 deletions(-) diff --git a/include/amarula/dbus/gproxy.hpp b/include/amarula/dbus/gproxy.hpp index eabd425..cad9ada 100644 --- a/include/amarula/dbus/gproxy.hpp +++ b/include/amarula/dbus/gproxy.hpp @@ -238,14 +238,48 @@ class DBusProxy : public std::enable_shared_from_this> { callMethod(cancellable, "SetProperty", parameters, callback, user_data); } - void callMethod(GCancellable* cancellable, const gchar* arg_name, + void callMethod(GCancellable* cancellable, const std::string& arg_name, GVariant* parameters, GAsyncReadyCallback callback, gpointer user_data) { - g_dbus_proxy_call( - proxy_, arg_name, - (parameters != nullptr) ? parameters - : g_variant_new_tuple(nullptr, 0), - G_DBUS_CALL_FLAGS_NONE, -1, cancellable, callback, user_data); + struct Data { + GDBusProxy* proxy; + std::string arg_name; + GVariant* parameters; + GCancellable* cancellable; + GAsyncReadyCallback callback; + gpointer user_data; + }; + + auto data = std::make_unique( + Data{proxy_, arg_name, + (parameters != nullptr) + ? g_variant_ref_sink(parameters) + : g_variant_ref_sink(g_variant_new_tuple(nullptr, 0)), + (cancellable != nullptr) + ? static_cast(g_object_ref(cancellable)) + : nullptr, + callback, user_data}); + + g_main_context_invoke_full( + dbus_->context(), G_PRIORITY_DEFAULT, + [](gpointer user_data) -> gboolean { + auto* data = static_cast(user_data); + + g_dbus_proxy_call(data->proxy, data->arg_name.c_str(), + data->parameters, G_DBUS_CALL_FLAGS_NONE, -1, + data->cancellable, data->callback, + data->user_data); + + return G_SOURCE_REMOVE; + }, + data.release(), + [](gpointer user_data) { + std::unique_ptr data(static_cast(user_data)); + g_variant_unref(data->parameters); + if (data->cancellable != nullptr) { + g_object_unref(data->cancellable); + } + }); } }; From cad49c12731ec84fdc01ed7ed7bb2ebc06ff99ac Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Wed, 1 Apr 2026 10:12:55 +0200 Subject: [PATCH 10/12] gproxy.hpp: Initialize the glib proxy on worker thread Initialize the glib proxy member in the worker thread and connect the signals in the worker thread. All this make the callbacks of the connected signals to run on the worker thread. The creation of the proxy is blocked until the worker thread creates it. This solves #21. Add flags when proxy is created to improve creation time. Signed-off-by: Eduardo Gonzalez --- include/amarula/dbus/gproxy.hpp | 104 +++++++++++++++++++++++++++----- 1 file changed, 89 insertions(+), 15 deletions(-) diff --git a/include/amarula/dbus/gproxy.hpp b/include/amarula/dbus/gproxy.hpp index cad9ada..e829f2f 100644 --- a/include/amarula/dbus/gproxy.hpp +++ b/include/amarula/dbus/gproxy.hpp @@ -83,6 +83,32 @@ class DBusProxy : public std::enable_shared_from_this> { } protected: + template + void connectSignal(const std::string& signal_name, Callback callback, + gpointer user_data) { + struct Data { + GDBusProxy* proxy; + std::string signal_name; + Callback callback; + gpointer user_data; + }; + + auto* data = new Data{proxy_, signal_name, callback, user_data}; + + g_main_context_invoke_full( + dbus_->context(), G_PRIORITY_DEFAULT, + [](gpointer user_data) -> gboolean { + auto* data = static_cast(user_data); + + g_signal_connect(data->proxy, data->signal_name.c_str(), + G_CALLBACK(data->callback), data->user_data); + + return G_SOURCE_REMOVE; + }, + data, + [](gpointer user_data) { delete static_cast(user_data); }); + } + void updateProperties(GVariant* properties) { GVariantIter* iter = g_variant_iter_new(properties); GVariant* prop = nullptr; @@ -176,23 +202,71 @@ class DBusProxy : public std::enable_shared_from_this> { dbus_->onAnyAsyncDone(); } - explicit DBusProxy(DBus* dbus, const gchar* name, const gchar* obj_path, - const gchar* interface_name) + explicit DBusProxy(DBus* dbus, const std::string& name, + const std::string& obj_path, + const std::string& interface_name) : dbus_{dbus} { - GError* err = nullptr; - - proxy_ = g_dbus_proxy_new_sync(dbus_->connection(), - G_DBUS_PROXY_FLAGS_NONE, nullptr, name, - obj_path, interface_name, nullptr, &err); - if (proxy_ == nullptr) { - std::string const msg = - "Failed to create proxy: " + std::string(err->message); - g_error_free(err); - throw std::runtime_error(msg); + struct Data { + DBusProxy* proxy; + std::string name; + std::string obj_path; + std::string interface_name; + std::mutex mtx; + std::condition_variable cv; + bool done{false}; + std::string error; + }; + + auto data = Data{this, name, obj_path, interface_name}; + + g_main_context_invoke_full( + dbus_->context(), G_PRIORITY_HIGH, + [](gpointer user_data) -> gboolean { + auto* data = static_cast(user_data); + + GError* err = nullptr; + + auto flags = static_cast( + G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES | + G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS | + G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START | + G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START_AT_CONSTRUCTION); + + data->proxy->proxy_ = g_dbus_proxy_new_sync( + data->proxy->dbus_->connection(), flags, nullptr, + data->name.c_str(), data->obj_path.c_str(), + data->interface_name.c_str(), nullptr, &err); + + if (data->proxy->proxy_ == nullptr) { + data->error = + "Failed to create proxy: " + std::string(err->message); + g_error_free(err); + } else { + g_signal_connect( + data->proxy->proxy_, "g-signal::PropertyChanged", + G_CALLBACK(&DBusProxy::on_properties_changed_cb), + data->proxy); + } + + return G_SOURCE_REMOVE; + }, + &data, + [](gpointer user_data) { + auto* data = static_cast(user_data); + { + std::lock_guard const lock(data->mtx); + data->done = true; + } + data->cv.notify_all(); + }); + { + std::unique_lock lock(data.mtx); + data.cv.wait(lock, [&] { return data.done; }); + } + + if (!data.error.empty()) { + throw std::runtime_error(data.error); } - g_signal_connect(proxy_, "g-signal::PropertyChanged", - G_CALLBACK(&DBusProxy::on_properties_changed_cb), - this); } template From aaf42ad78ed46720c989de151df1104f68e42f77 Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Wed, 1 Apr 2026 10:45:49 +0200 Subject: [PATCH 11/12] gconnman_manager.cpp: Connect signals in worker thread This solves #21. Signed-off-by: Eduardo Gonzalez --- src/dbus/gconnman_manager.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/dbus/gconnman_manager.cpp b/src/dbus/gconnman_manager.cpp index 5f11a21..ef0a18d 100644 --- a/src/dbus/gconnman_manager.cpp +++ b/src/dbus/gconnman_manager.cpp @@ -49,14 +49,12 @@ Manager::Manager(DBus* dbus, const std::string& agent_path) get_technologies(); get_services(); - g_signal_connect(proxy(), "g-signal::TechnologyRemoved", - G_CALLBACK(&Manager::on_technology_added_removed_cb), - this); - g_signal_connect(proxy(), "g-signal::TechnologyAdded", - G_CALLBACK(&Manager::on_technology_added_removed_cb), - this); - g_signal_connect(proxy(), "g-signal::ServicesChanged", - G_CALLBACK(&Manager::on_services_changed_cb), this); + connectSignal("g-signal::TechnologyRemoved", + &Manager::on_technology_added_removed_cb, this); + connectSignal("g-signal::TechnologyAdded", + &Manager::on_technology_added_removed_cb, this); + connectSignal("g-signal::ServicesChanged", &Manager::on_services_changed_cb, + this); } void Manager::process_services_changed( From 677d7a15727398bd8a7937d4ad799f48ef5e3d1b Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Wed, 1 Apr 2026 14:52:00 +0200 Subject: [PATCH 12/12] gconnman_agent: Register object in worker thread Call the register object in the dbus in the worker thread. The latter makes the callback to be executed also in the worker thread. The creation of the agent block the thread until the worker thread register the object. This solves #21. Signed-off-by: Eduardo Gonzalez --- include/amarula/dbus/connman/gagent.hpp | 4 +- src/dbus/gconnman_agent.cpp | 59 ++++++++++++++++++++----- src/dbus/gconnman_manager.cpp | 3 +- 3 files changed, 52 insertions(+), 14 deletions(-) diff --git a/include/amarula/dbus/connman/gagent.hpp b/include/amarula/dbus/connman/gagent.hpp index 998e3e5..4c0f7d0 100644 --- a/include/amarula/dbus/connman/gagent.hpp +++ b/include/amarula/dbus/connman/gagent.hpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -15,8 +16,7 @@ class Agent { GDBusConnection *connection_{nullptr}; std::string path_{"/net/amarula/gconnman/agent"}; - explicit Agent(GDBusConnection *connection, - const std::string &path = std::string()); + explicit Agent(DBus *dbus, const std::string &path = std::string()); using RequestInputCallback = std::function; diff --git a/src/dbus/gconnman_agent.cpp b/src/dbus/gconnman_agent.cpp index 3cf559a..8078fcc 100644 --- a/src/dbus/gconnman_agent.cpp +++ b/src/dbus/gconnman_agent.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -28,8 +29,8 @@ static constexpr const char *INTROSPECTION_XML = " " ""; -Agent::Agent(GDBusConnection *connection, const std::string &path) - : connection_{connection} { +Agent::Agent(DBus *dbus, const std::string &path) + : connection_{dbus->connection()} { if (!path.empty()) { path_ = path; } @@ -42,16 +43,54 @@ Agent::Agent(GDBusConnection *connection, const std::string &path) throw std::runtime_error(msg); } - registration_id_ = g_dbus_connection_register_object( - connection, path_.c_str(), *(node_info_->interfaces), &INTERFACE_VTABLE, - this, nullptr, &err); + struct Data { + Agent *self; + std::mutex mtx; + std::condition_variable cv; + bool done{false}; + std::string error; + }; + + auto data = Data{this}; + + g_main_context_invoke_full( + dbus->context(), G_PRIORITY_HIGH, + [](gpointer user_data) -> gboolean { + auto *data = static_cast(user_data); + + GError *err = nullptr; + + data->self->registration_id_ = g_dbus_connection_register_object( + data->self->connection_, data->self->path_.c_str(), + *(data->self->node_info_->interfaces), &INTERFACE_VTABLE, + data->self, nullptr, &err); + + if (data->self->registration_id_ == 0) { + data->error = + "Failed to register agent: " + std::string(err->message); + g_error_free(err); + } + + return G_SOURCE_REMOVE; + }, + &data, + [](gpointer user_data) { + auto *data = static_cast(user_data); + { + std::lock_guard const lock(data->mtx); + data->done = true; + } + data->cv.notify_all(); + }); + + { + std::unique_lock lock(data.mtx); + data.cv.wait(lock, [&] { return data.done; }); + } - if (registration_id_ == 0) { - std::string const msg = - "Failed to register agent: " + std::string(err->message); - g_error_free(err); + if (!data.error.empty()) { g_dbus_node_info_unref(node_info_); - throw std::runtime_error(msg); + throw std::runtime_error(data.error); } } Agent::~Agent() { diff --git a/src/dbus/gconnman_manager.cpp b/src/dbus/gconnman_manager.cpp index ef0a18d..bfbcdb4 100644 --- a/src/dbus/gconnman_manager.cpp +++ b/src/dbus/gconnman_manager.cpp @@ -43,8 +43,7 @@ void ManaProperties::update(const gchar* key, GVariant* value) { Manager::Manager(DBus* dbus, const std::string& agent_path) : DBusProxy(dbus, SERVICE, MANAGER_PATH, MANAGER_INTERFACE), - agent_{ - std::unique_ptr(new Agent(dbus->connection(), agent_path))} { + agent_{std::unique_ptr(new Agent(dbus, agent_path))} { setup_agent(); get_technologies(); get_services();