Implemented timeout handling for Properties.GetAll

AP-Sensing · Jan 30, 2021 · 5d8ca58 · 5d8ca58
1 parent a8a8e44
commit 5d8ca58
Show file tree

Hide file tree

Showing 6 changed files with 163 additions and 46 deletions.
diff --git 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
@@ -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<typename FuncT, typename ReturnT>
 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<ReturnT>::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<void(CallState)> f, StubBase& stub);
+
+   static
+   void _delete(void* p);
+
+   static
+   void pending_notify(DBusPendingCall* pc, void* data);
+
+   std::function<void(CallState)> f_;
+   StubBase& stub_;
+};
+
+
 }   // detail
 
 }   // dbus

diff --git 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<typename... T> friend struct ClientSignal;
    template<typename... T> 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<std::string> 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
@@ -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
@@ -12,6 +12,36 @@
 #include <cassert>
 
 
+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<void(D
 
     if (!is_oneway)
     {
-        int timeout = disp().request_timeout();
-
-        if (detail::request_specific_timeout.count() > 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<void(D
        dbus_connection_flush(disp().conn_);
     }
 
-    detail::request_specific_timeout = std::chrono::milliseconds(0);
-
     return PendingCall(dbus_message_get_serial(msg.get()), pending);
 }
 
@@ -221,14 +243,7 @@ message_ptr_t StubBase::send_request_and_block(const char* method_name, std::fun
 
     if (!is_oneway)
     {
-        int timeout = disp().request_timeout();
-
-        if (detail::request_specific_timeout.count() > 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
@@ -374,7 +374,7 @@ struct GetAllClient : simppl::dbus::Stub<Properties>
 
          // 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");