From 5d8ca58933c6983224226b9f84c4c267817f3a2d Mon Sep 17 00:00:00 2001 From: Martin Haefner Date: Sat, 30 Jan 2021 10:46:45 +0100 Subject: [PATCH] Implemented timeout handling for Properties.GetAll --- CMakeLists.txt | 1 + include/simppl/detail/holders.h | 64 +++++++++++++++++++++++++++++---- include/simppl/stubbase.h | 48 +++++++++++++++++-------- src/properties.cpp | 31 ++++++++++++++++ src/stubbase.cpp | 61 +++++++++++++++++++------------ tests/properties.cpp | 4 ++- 6 files changed, 163 insertions(+), 46 deletions(-) create mode 100644 src/properties.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 1a603b1..54fabd8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -59,6 +59,7 @@ add_library(simppl SHARED src/serialization.cpp src/bool.cpp src/holders.cpp + src/properties.cpp ) # Provide a namespaced alias add_library(Simppl::simppl ALIAS simppl) diff --git a/include/simppl/detail/holders.h b/include/simppl/detail/holders.h index 3ec41f7..9098c3f 100644 --- a/include/simppl/detail/holders.h +++ b/include/simppl/detail/holders.h @@ -11,6 +11,11 @@ namespace simppl namespace dbus { +// forward decl +struct StubBase; +struct CallState; + + namespace detail { @@ -19,12 +24,12 @@ struct InterimCallbackHolder { typedef HolderT holder_type; - InterimCallbackHolder(const InterimCallbackHolder& rhs) + /*InterimCallbackHolder(const InterimCallbackHolder& rhs) : pc_(std::move(rhs.pc_)) { // NOOP - } - + }*/ + InterimCallbackHolder& operator=(const InterimCallbackHolder&) = delete; explicit inline @@ -33,11 +38,36 @@ struct InterimCallbackHolder { // NOOP } - + PendingCall pc_; }; +struct InterimGetAllPropertiesCallbackHolder +{ + /*inline + InterimGetAllPropertiesCallbackHolder(const InterimGetAllPropertiesCallbackHolder& rhs, StubBase& stub) + : pc_(std::move(rhs.pc_)) + , stub_(stub) + { + // NOOP + }*/ + + InterimGetAllPropertiesCallbackHolder& operator=(const InterimGetAllPropertiesCallbackHolder&) = delete; + + InterimGetAllPropertiesCallbackHolder(const PendingCall& pc, StubBase& stub) + : pc_(std::move(pc)) + , stub_(stub) + { + // NOOP + } + + PendingCall pc_; + StubBase& stub_; +}; + + + template struct CallbackHolder { @@ -51,7 +81,7 @@ struct CallbackHolder { // NOOP } - + static inline void _delete(void* p) { @@ -68,10 +98,10 @@ struct CallbackHolder assert(that->f_); CallState cs(*msg); - + DBusMessageIter iter; dbus_message_iter_init(msg.get(), &iter); - + GetCaller::type::template evalResponse(iter, that->f_, cs); } @@ -126,6 +156,26 @@ struct PropertyCallbackHolder FuncT f_; }; + +struct GetAllPropertiesHolder +{ + GetAllPropertiesHolder(const GetAllPropertiesHolder&) = delete; + GetAllPropertiesHolder& operator=(const GetAllPropertiesHolder&) = delete; + + + GetAllPropertiesHolder(std::function f, StubBase& stub); + + static + void _delete(void* p); + + static + void pending_notify(DBusPendingCall* pc, void* data); + + std::function f_; + StubBase& stub_; +}; + + } // detail } // dbus diff --git a/include/simppl/stubbase.h b/include/simppl/stubbase.h index 094e876..f517934 100644 --- a/include/simppl/stubbase.h +++ b/include/simppl/stubbase.h @@ -28,11 +28,34 @@ namespace dbus struct Dispatcher; struct ClientSignalBase; struct ClientPropertyBase; +struct StubBase; + + +// TODO move away from here +namespace detail +{ +struct GetAllProperties +{ + typedef detail::InterimGetAllPropertiesCallbackHolder getall_properties_holder_type; + + GetAllProperties(simppl::dbus::StubBase& stub); + + GetAllProperties& operator[](int flags); + + void operator()(); + + getall_properties_holder_type + async(); + + simppl::dbus::StubBase& stub_; +}; + +} struct StubBase { - typedef detail::InterimGetAllPropertiesCallbackHolder getall_properties_holder_type; + typedef detail::GetAllProperties::getall_properties_holder_type getall_properties_holder_type; template friend struct ClientSignal; template friend struct ClientMethod; @@ -42,6 +65,7 @@ struct StubBase friend struct Dispatcher; friend struct ClientPropertyBase; friend struct detail::GetAllPropertiesHolder; + friend struct detail::GetAllProperties; StubBase(const StubBase&) = delete; StubBase& operator=(const StubBase&) = delete; @@ -91,26 +115,16 @@ struct StubBase /** * Implementation of org.freedesktop.DBus.Properties.GetAll(). Blocking call. * - * Before calling this method all Properties callbacks shall be installed. - * The properties callbacks will then be called before this function returns. - * - * Issues a call like this: - * dbus-send --print-reply --dest=test.Properties.s /test/Properties/s org.freedesktop.DBus.Properties.GetAll string:test.Properties - * - * @TODO complete async support - */ - void get_all_properties(); - - /** - * Implementation of org.freedesktop.DBus.Properties.GetAll(). Asynchronous call. - * * Before calling this method all Properties callbacks shall be installed. You may install * only some of the property callbacks. If a callback is not registered the property will * just omitted. * * The asynchronous return will arrive as soon as call the property callbacks are evaluated. + * + * Issues a call like this: + * dbus-send --print-reply --dest=test.Properties.s /test/Properties/s org.freedesktop.DBus.Properties.GetAll string:test.Properties */ - getall_properties_holder_type get_all_properties_async(); + detail::GetAllProperties get_all_properties; protected: @@ -156,6 +170,9 @@ struct StubBase */ simppl::dbus::CallState get_all_properties_handle_response(DBusMessage& response); + void get_all_properties_request(); + getall_properties_holder_type get_all_properties_request_async(); + std::vector ifaces_; char* objectpath_; std::string busname_; @@ -185,3 +202,4 @@ void operator>>(simppl::dbus::detail::InterimGetAllPropertiesCallbackHolder&& r, #endif // SIMPPL_STUBBASE_H + diff --git a/src/properties.cpp b/src/properties.cpp new file mode 100644 index 0000000..e4bbee1 --- /dev/null +++ b/src/properties.cpp @@ -0,0 +1,31 @@ +#include "simppl/stubbase.h" +#include "simppl/timeout.h" + + +simppl::dbus::detail::GetAllProperties::GetAllProperties(simppl::dbus::StubBase& stub) + : stub_(stub) +{ + // NOOP +} + + +simppl::dbus::detail::GetAllProperties& simppl::dbus::detail::GetAllProperties::operator[](int flags) +{ + if (flags & (1<<0)) + detail::request_specific_timeout = timeout.timeout_; + + return *this; +} + + +void simppl::dbus::detail::GetAllProperties::operator()() +{ + stub_.get_all_properties_request(); +} + + +simppl::dbus::detail::GetAllProperties::getall_properties_holder_type +simppl::dbus::detail::GetAllProperties::async() +{ + return stub_.get_all_properties_request_async(); +} diff --git a/src/stubbase.cpp b/src/stubbase.cpp index b15fdc7..844f7cd 100644 --- a/src/stubbase.cpp +++ b/src/stubbase.cpp @@ -12,6 +12,36 @@ #include +namespace { + +struct TimeoutRAIIHelper +{ + TimeoutRAIIHelper(simppl::dbus::Dispatcher& disp) + : timeout_(disp.request_timeout()) + { + if (simppl::dbus::detail::request_specific_timeout.count() > 0) + timeout_ = simppl::dbus::detail::request_specific_timeout.count(); + } + + ~TimeoutRAIIHelper() + { + simppl::dbus::detail::request_specific_timeout = std::chrono::milliseconds(0); + } + + operator int() + { + return timeout_; + } + + int timeout_; +}; + +} // namespace + + +// --------------------------------------------------------------------- + + using namespace std::literals::chrono_literals; @@ -22,7 +52,8 @@ namespace dbus { StubBase::StubBase() - : objectpath_(nullptr) + : get_all_properties(*this) + , objectpath_(nullptr) , conn_state_(ConnectionState::Disconnected) , disp_(nullptr) , signals_(nullptr) @@ -93,7 +124,7 @@ Dispatcher& StubBase::disp() } -void StubBase::get_all_properties() +void StubBase::get_all_properties_request() { message_ptr_t msg = make_message(dbus_message_new_method_call(busname().c_str(), objectpath(), "org.freedesktop.DBus.Properties", "GetAll")); DBusPendingCall* pending = nullptr; @@ -103,8 +134,7 @@ void StubBase::get_all_properties() encode(iter, iface()); - // TODO timeout handling here - dbus_connection_send_with_reply(conn(), msg.get(), &pending, DBUS_TIMEOUT_USE_DEFAULT); + dbus_connection_send_with_reply(conn(), msg.get(), &pending, TimeoutRAIIHelper(disp())); dbus_pending_call_block(pending); @@ -155,7 +185,7 @@ simppl::dbus::CallState StubBase::get_all_properties_handle_response(DBusMessage } -StubBase::getall_properties_holder_type StubBase::get_all_properties_async() +StubBase::getall_properties_holder_type StubBase::get_all_properties_request_async() { message_ptr_t msg = make_message(dbus_message_new_method_call(busname().c_str(), objectpath(), "org.freedesktop.DBus.Properties", "GetAll")); DBusPendingCall* pending = nullptr; @@ -167,8 +197,7 @@ StubBase::getall_properties_holder_type StubBase::get_all_properties_async() encode(iter, iface()); } - // TODO timeout handling here - dbus_connection_send_with_reply(conn(), msg.get(), &pending, DBUS_TIMEOUT_USE_DEFAULT); + dbus_connection_send_with_reply(conn(), msg.get(), &pending, TimeoutRAIIHelper(disp())); return getall_properties_holder_type(PendingCall(dbus_message_get_serial(msg.get()), pending), *this); } @@ -186,12 +215,7 @@ PendingCall StubBase::send_request(const char* method_name, std::function 0) - timeout = detail::request_specific_timeout.count(); - - dbus_connection_send_with_reply(disp().conn_, msg.get(), &pending, timeout); + dbus_connection_send_with_reply(disp().conn_, msg.get(), &pending, TimeoutRAIIHelper(disp())); } else { @@ -202,8 +226,6 @@ PendingCall StubBase::send_request(const char* method_name, std::function 0) - timeout = detail::request_specific_timeout.count(); - - dbus_connection_send_with_reply(disp().conn_, msg.get(), &pending, timeout); - - detail::request_specific_timeout = std::chrono::milliseconds(0); + dbus_connection_send_with_reply(disp().conn_, msg.get(), &pending, TimeoutRAIIHelper(disp())); dbus_pending_call_block(pending); diff --git a/tests/properties.cpp b/tests/properties.cpp index 9ba9ea0..4a27ecd 100644 --- a/tests/properties.cpp +++ b/tests/properties.cpp @@ -374,7 +374,7 @@ struct GetAllClient : simppl::dbus::Stub // now call - callbacks will be called in background just before // this callback gets called... - get_all_properties_async() >> [this](simppl::dbus::CallState cs){ + get_all_properties.async() >> [this](simppl::dbus::CallState cs){ EXPECT_TRUE((bool)cs); // the other two callbacks are already evaluated... @@ -518,6 +518,8 @@ TEST(Properties, getall_blocking) // now call - callbacks will be called in background c.get_all_properties(); + c.get_all_properties[42](); + EXPECT_EQ(ival, 4711); EXPECT_EQ(sval, "Hallo Welt");