Skip to content

Commit

Permalink
refactor: remove floating_slot_t tag and use return_slot_t instead (#439
Browse files Browse the repository at this point in the history 
)

Make all the API consistent by using return_slot_t-based overloads for returning the slots to clients, and overloads without that tag for "floating" slots. floating_slot_t tag was previously added for API backwards compatibility reasons, but now is a (counter-)duplicate to the return_slot_t.
  • Loading branch information
@sangelovic
sangelovic committed Apr 24, 2024
1 parent a6b6490 commit 6eb0cff
Show file tree
Hide file tree
Showing 10 changed files with 107 additions and 81 deletions.
4 changes: 3 additions & 1 deletion docs/using-sdbus-c++.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1798,6 +1798,8 @@ sdbus-c++ v2 is a major release that comes with a number of breaking API/ABI/beh
* `createDefaultBusConnection()` has been renamed to `createBusConnection()`.
* `IObject::removeObjectManager()` and `IObject::hasObjectManager()` were removed. Clients should now use the slot-returning `IObject::addObjectManager()` to control the `ObjectManager` interface lifetime.
* `floating_slot_t` tag was removed from `IConnection::addObjectManager()`, the function is now by default floating-slot-based.
* Slot-returning `IConnection::addMatch()` has gotten the `return_slot_t` tag parameter, while `floating_slot_t` was removed from the floating slot-based overload of the method.
* Slot-returning `IConnection::addMatchAsync()` has gotten the `return_slot_t` tag parameter, while `floating_slot_t` was removed from the floating slot-based overload of the method.
* Change in behavior: `Proxy`s now by default call `createBusConnection()` to get a connection when the connection is not provided explicitly by the caller, so they connect to either the session bus or the system bus depending on the context (as opposed to always to the system bus like before).
* Callbacks taking `const sdbus::Error* error` were changed to take `std::optional<sdbus::Error>`, which better expresses the intent and meaning.
* `getInterfaceName()`, `getMemberName()`, `getSender()`, `getPath()` and `getDestination()` methods of `Message` class now return `const char*` instead of `std::string`, for efficiency reasons.
Expand All @@ -1808,7 +1810,7 @@ sdbus-c++ v2 is a major release that comes with a number of breaking API/ABI/beh
* CMake options got `SDBUSCPP_` prefix for better usability and minimal risk of conflicts in downstream CMake projects. `SDBUSCPP_INSTALL` CMake option was added.
* CMake components got `sdbus-c++-` prefix.
Some of these changes required correspoding adaptations in the sdbus-c++ codegen. Hence, your **C++ bindings (if any) must be re-generated** with the new sdbus-c++-xml2cpp v2 in order to use them with sdbus-c++ v2 API.
An important note: **C++ bindings generated from XML files must be re-generated** with the new `sdbus-c++-xml2cpp` when migrating to sdbus-c++ v2.0.
Conclusion
----------
Expand Down
64 changes: 40 additions & 24 deletions include/sdbus-c++/IConnection.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,11 +257,10 @@ namespace sdbus {
[[nodiscard]] virtual Slot addObjectManager(const ObjectPath& objectPath, return_slot_t) = 0;

/*!
* @brief Installs a match rule for messages received on this bus connection
* @brief Installs a floating match rule for messages received on this bus connection
*
* @param[in] match Match expression to filter incoming D-Bus message
* @param[in] callback Callback handler to be called upon processing an inbound D-Bus message matching the rule
* @return RAII-style slot handle representing the ownership of the subscription
*
* The method installs a match rule for messages received on the specified bus connection.
* The syntax of the match rule expression passed in match is described in the D-Bus specification.
Expand All @@ -270,70 +269,87 @@ namespace sdbus {
* sends a control message requesting the match to be added to the broker and waits until the broker
* confirms the match has been installed successfully.
*
* Simply let go of the slot instance to uninstall the match rule from the bus connection. The slot
* must not outlive the connection for the slot is associated with it.
* The method installs a floating match rule for messages received on the specified bus connection.
* Floating means that the bus connection object owns the match rule, i.e. lifetime of the match rule
* is bound to the lifetime of the bus connection.
*
* For more information, consult `man sd_bus_add_match`.
*
* @throws sdbus::Error in case of failure
*/
[[nodiscard]] virtual Slot addMatch(const std::string& match, message_handler callback) = 0;
virtual void addMatch(const std::string& match, message_handler callback) = 0;

/*!
* @brief Installs a floating match rule for messages received on this bus connection
* @brief Installs a match rule for messages received on this bus connection
*
* @param[in] match Match expression to filter incoming D-Bus message
* @param[in] callback Callback handler to be called upon processing an inbound D-Bus message matching the rule
* @return RAII-style slot handle representing the ownership of the subscription
*
* The method installs a floating match rule for messages received on the specified bus connection.
* Floating means that the bus connection object owns the match rule, i.e. lifetime of the match rule
* is bound to the lifetime of the bus connection.
* The method installs a match rule for messages received on the specified bus connection.
* The syntax of the match rule expression passed in match is described in the D-Bus specification.
* The specified handler function callback is called for each incoming message matching the specified
* expression. The match is installed synchronously when connected to a bus broker, i.e. the call
* sends a control message requesting the match to be added to the broker and waits until the broker
* confirms the match has been installed successfully.
*
* Refer to the @c addMatch(const std::string& match, message_handler callback) documentation for more
* information.
* The lifetime of the match rule is bound to the lifetime of the returned slot instance. Destroying
* the slot instance implies uninstalling of the match rule from the bus connection.
*
* For more information, consult `man sd_bus_add_match`.
*
* @throws sdbus::Error in case of failure
*/
virtual void addMatch(const std::string& match, message_handler callback, floating_slot_t) = 0;
[[nodiscard]] virtual Slot addMatch(const std::string& match, message_handler callback, return_slot_t) = 0;

/*!
* @brief Asynchronously installs a match rule for messages received on this bus connection
* @brief Asynchronously installs a floating match rule for messages received on this bus connection
*
* @param[in] match Match expression to filter incoming D-Bus message
* @param[in] callback Callback handler to be called upon processing an inbound D-Bus message matching the rule
* @param[in] installCallback Callback handler to be called upon processing an inbound D-Bus message matching the rule
* @return RAII-style slot handle representing the ownership of the subscription
*
* This method operates the same as `addMatch()` above, just that it installs the match rule asynchronously,
* in a non-blocking fashion. A request is sent to the broker, but the call does not wait for a response.
* The `installCallback' callable is called when the response is later received, with the response message
* from the broker as parameter. If it's an empty function object, a default implementation is used that
* terminates the bus connection should installing the match fail.
*
* Refer to the @c addMatch(const std::string& match, message_handler callback) documentation, and consult
* `man sd_bus_add_match`, for more information.
* The method installs a floating match rule for messages received on the specified bus connection.
* Floating means that the bus connection object owns the match rule, i.e. lifetime of the match rule
* is bound to the lifetime of the bus connection.
*
* For more information, consult `man sd_bus_add_match_async`.
*
* @throws sdbus::Error in case of failure
*/
[[nodiscard]] virtual Slot addMatchAsync(const std::string& match, message_handler callback, message_handler installCallback) = 0;
virtual void addMatchAsync(const std::string& match, message_handler callback, message_handler installCallback) = 0;

/*!
* @brief Asynchronously installs a floating match rule for messages received on this bus connection
* @brief Asynchronously installs a match rule for messages received on this bus connection
*
* @param[in] match Match expression to filter incoming D-Bus message
* @param[in] callback Callback handler to be called upon processing an inbound D-Bus message matching the rule
* @param[in] installCallback Callback handler to be called upon processing an inbound D-Bus message matching the rule
* @return RAII-style slot handle representing the ownership of the subscription
*
* The method installs a floating match rule for messages received on the specified bus connection.
* Floating means that the bus connection object owns the match rule, i.e. lifetime of the match rule
* is bound to the lifetime of the bus connection.
* This method operates the same as `addMatch()` above, just that it installs the match rule asynchronously,
* in a non-blocking fashion. A request is sent to the broker, but the call does not wait for a response.
* The `installCallback' callable is called when the response is later received, with the response message
* from the broker as parameter. If it's an empty function object, a default implementation is used that
* terminates the bus connection should installing the match fail.
*
* The lifetime of the match rule is bound to the lifetime of the returned slot instance. Destroying
* the slot instance implies the uninstalling of the match rule from the bus connection.
*
* Refer to the @c addMatch(const std::string& match, message_handler callback, message_handler installCallback)
* documentation for more information.
* For more information, consult `man sd_bus_add_match_async`.
*
* @throws sdbus::Error in case of failure
*/
virtual void addMatchAsync(const std::string& match, message_handler callback, message_handler installCallback, floating_slot_t) = 0;
[[nodiscard]] virtual Slot addMatchAsync( const std::string& match
, message_handler callback
, message_handler installCallback
, return_slot_t ) = 0;

/*!
* @brief Retrieves the unique name of a connection. E.g. ":1.xx"
Expand Down
3 changes: 1 addition & 2 deletions include/sdbus-c++/Message.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,8 +266,7 @@ namespace sdbus {
MethodCall() = default;

MethodReply send(uint64_t timeout) const;
void send(void* callback, void* userData, uint64_t timeout, floating_slot_t) const;
[[nodiscard]] Slot send(void* callback, void* userData, uint64_t timeout) const;
[[nodiscard]] Slot send(void* callback, void* userData, uint64_t timeout, return_slot_t) const;

MethodReply createReply() const;
MethodReply createErrorReply(const sdbus::Error& error) const;
Expand Down
44 changes: 18 additions & 26 deletions src/Connection.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,12 @@ uint64_t Connection::getMethodCallTimeout() const
return timeout;
}

Slot Connection::addMatch(const std::string& match, message_handler callback)
void Connection::addMatch(const std::string& match, message_handler callback)
{
floatingMatchRules_.push_back(addMatch(match, std::move(callback), return_slot));
}

Slot Connection::addMatch(const std::string& match, message_handler callback, return_slot_t)
{
SDBUS_THROW_ERROR_IF(!callback, "Invalid match callback handler provided", EINVAL);

Expand All @@ -240,12 +245,15 @@ Slot Connection::addMatch(const std::string& match, message_handler callback)
return {matchInfo.release(), [](void *ptr){ delete static_cast<MatchInfo*>(ptr); }};
}

void Connection::addMatch(const std::string& match, message_handler callback, floating_slot_t)
void Connection::addMatchAsync(const std::string& match, message_handler callback, message_handler installCallback)
{
floatingMatchRules_.push_back(addMatch(match, std::move(callback)));
floatingMatchRules_.push_back(addMatchAsync(match, std::move(callback), std::move(installCallback), return_slot));
}

Slot Connection::addMatchAsync(const std::string& match, message_handler callback, message_handler installCallback)
Slot Connection::addMatchAsync( const std::string& match
, message_handler callback
, message_handler installCallback
, return_slot_t )
{
SDBUS_THROW_ERROR_IF(!callback, "Invalid match callback handler provided", EINVAL);

Expand All @@ -266,11 +274,6 @@ Slot Connection::addMatchAsync(const std::string& match, message_handler callbac
return {matchInfo.release(), [](void *ptr){ delete static_cast<MatchInfo*>(ptr); }};
}

void Connection::addMatchAsync(const std::string& match, message_handler callback, message_handler installCallback, floating_slot_t)
{
floatingMatchRules_.push_back(addMatchAsync(match, std::move(callback), std::move(installCallback)));
}

void Connection::attachSdEventLoop(sd_event *event, int priority)
{
#ifndef SDBUS_basu
Expand Down Expand Up @@ -460,7 +463,8 @@ void Connection::deleteSdEventSource(sd_event_source *s)
Slot Connection::addObjectVTable( const ObjectPath& objectPath
, const InterfaceName& interfaceName
, const sd_bus_vtable* vtable
, void* userData )
, void* userData
, return_slot_t )
{
sd_bus_slot *slot{};

Expand Down Expand Up @@ -546,24 +550,11 @@ MethodReply Connection::callMethod(const MethodCall& message, uint64_t timeout)
return reply;
}

void Connection::callMethod(const MethodCall& message, void* callback, void* userData, uint64_t timeout, floating_slot_t)
{
// TODO: Think of ways of optimizing these three locking/unlocking of sdbus mutex (merge into one call?)
auto timeoutBefore = getEventLoopPollData().timeout;
message.send(callback, userData, timeout, floating_slot);
auto timeoutAfter = getEventLoopPollData().timeout;

// An event loop may wait in poll with timeout `t1', while in another thread an async call is made with
// timeout `t2'. If `t2' < `t1', then we have to wake up the event loop thread to update its poll timeout.
if (timeoutAfter < timeoutBefore)
notifyEventLoopToWakeUpFromPoll();
}

Slot Connection::callMethod(const MethodCall& message, void* callback, void* userData, uint64_t timeout)
Slot Connection::callMethod(const MethodCall& message, void* callback, void* userData, uint64_t timeout, return_slot_t)
{
// TODO: Think of ways of optimizing these three locking/unlocking of sdbus mutex (merge into one call?)
auto timeoutBefore = getEventLoopPollData().timeout;
auto slot = message.send(callback, userData, timeout);
auto slot = message.send(callback, userData, timeout, return_slot);
auto timeoutAfter = getEventLoopPollData().timeout;

// An event loop may wait in poll with timeout `t1', while in another thread an async call is made with
Expand Down Expand Up @@ -638,7 +629,8 @@ Slot Connection::registerSignalHandler( const char* sender
, const char* interfaceName
, const char* signalName
, sd_bus_message_handler_t callback
, void* userData )
, void* userData
, return_slot_t )
{
sd_bus_slot *slot{};

Expand Down
20 changes: 12 additions & 8 deletions src/Connection.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,13 @@ namespace sdbus::internal {
void setMethodCallTimeout(uint64_t timeout) override;
[[nodiscard]] uint64_t getMethodCallTimeout() const override;

[[nodiscard]] Slot addMatch(const std::string& match, message_handler callback) override;
void addMatch(const std::string& match, message_handler callback, floating_slot_t) override;
[[nodiscard]] Slot addMatchAsync(const std::string& match, message_handler callback, message_handler installCallback) override;
void addMatchAsync(const std::string& match, message_handler callback, message_handler installCallback, floating_slot_t) override;
void addMatch(const std::string& match, message_handler callback) override;
[[nodiscard]] Slot addMatch(const std::string& match, message_handler callback, return_slot_t) override;
void addMatchAsync(const std::string& match, message_handler callback, message_handler installCallback) override;
[[nodiscard]] Slot addMatchAsync( const std::string& match
, message_handler callback
, message_handler installCallback
, return_slot_t ) override;

void attachSdEventLoop(sd_event *event, int priority) override;
void detachSdEventLoop() override;
Expand All @@ -123,7 +126,8 @@ namespace sdbus::internal {
Slot addObjectVTable( const ObjectPath& objectPath
, const InterfaceName& interfaceName
, const sd_bus_vtable* vtable
, void* userData ) override;
, void* userData
, return_slot_t ) override;

[[nodiscard]] PlainMessage createPlainMessage() const override;
[[nodiscard]] MethodCall createMethodCall( const ServiceName& destination
Expand All @@ -142,8 +146,7 @@ namespace sdbus::internal {
, const char* signalName ) const override;

MethodReply callMethod(const MethodCall& message, uint64_t timeout) override;
void callMethod(const MethodCall& message, void* callback, void* userData, uint64_t timeout, floating_slot_t) override;
Slot callMethod(const MethodCall& message, void* callback, void* userData, uint64_t timeout) override;
Slot callMethod(const MethodCall& message, void* callback, void* userData, uint64_t timeout, return_slot_t) override;

void emitPropertiesChangedSignal( const ObjectPath& objectPath
, const InterfaceName& interfaceName
Expand All @@ -163,7 +166,8 @@ namespace sdbus::internal {
, const char* interfaceName
, const char* signalName
, sd_bus_message_handler_t callback
, void* userData ) override;
, void* userData
, return_slot_t ) override;

private:
using BusFactory = std::function<int(sd_bus**)>;
Expand Down
Loading

0 comments on commit 6eb0cff

Please sign in to comment.