From 3cd6e9e89e8227266b80e1269c56dc635e198998 Mon Sep 17 00:00:00 2001 From: Denis Biryukov Date: Thu, 12 Sep 2024 17:21:53 +0200 Subject: [PATCH] added support for z_hello_clone; added support for background declarations; added support for z_id_t to string; added support for session_close_options; --- include/zenoh/api/hello.hxx | 20 +++++++++++ include/zenoh/api/id.hxx | 8 +++-- include/zenoh/api/publisher.hxx | 3 -- include/zenoh/api/queryable.hxx | 21 ++++++++++-- include/zenoh/api/session.hxx | 42 +++++++++++++++-------- include/zenoh/api/subscriber.hxx | 28 +++++++++++---- tests/universal/network/pub_sub.cxx | 6 ++++ tests/universal/network/queryable_get.cxx | 6 ++++ zenoh-c | 2 +- zenoh-pico | 2 +- 10 files changed, 107 insertions(+), 31 deletions(-) diff --git a/include/zenoh/api/hello.hxx b/include/zenoh/api/hello.hxx index e1dc1ba2..1ec28c9f 100644 --- a/include/zenoh/api/hello.hxx +++ b/include/zenoh/api/hello.hxx @@ -62,5 +62,25 @@ class Hello : public Owned<::z_owned_hello_t> { #endif return locators; } + + /// @brief Copy contructor + Hello(const Hello& other) : Owned(nullptr) { ::z_hello_clone(&this->_0, interop::as_loaned_c_ptr(other)); }; + + /// @brief Move constructor + Hello(Hello&& other) = default; + + /// @name Operators + + /// @brief Assignment operator. + Hello& operator=(const Hello& other) { + if (this != &other) { + ::z_drop(z_move(this->_0)); + ::z_hello_clone(&this->_0, interop::as_loaned_c_ptr(other)); + } + return *this; + }; + + /// @brief Move assignment operator. + Hello& operator=(Hello&& other) = default; }; } // namespace zenoh \ No newline at end of file diff --git a/include/zenoh/api/id.hxx b/include/zenoh/api/id.hxx index 08b8be23..6fccabf5 100644 --- a/include/zenoh/api/id.hxx +++ b/include/zenoh/api/id.hxx @@ -16,6 +16,7 @@ #include #include #include +#include #include "../zenohc.hxx" #include "base.hxx" @@ -44,9 +45,10 @@ class Id : public Copyable<::z_id_t> { /// @warning This API has been marked as unstable: it works as advertised, but it may be changed in a future release. /// @brief Print ``Id`` in the hex format. inline std::ostream& operator<<(std::ostream& os, const Id& id) { - auto id_ptr = reinterpret_cast(&id)->id; - for (size_t i = 0; id_ptr[i] != 0 && i < 16; i++) - os << std::hex << std::setfill('0') << std::setw(2) << static_cast(id_ptr[i]); + ::z_owned_string_t s; + ::z_id_to_string(interop::as_copyable_c_ptr(id), &s); + os << std::string_view(::z_string_data(::z_loan(s)), ::z_string_len(::z_loan(s))); + ::z_drop(::z_move(s)); return os; } } // namespace zenoh \ No newline at end of file diff --git a/include/zenoh/api/publisher.hxx b/include/zenoh/api/publisher.hxx index daea66f6..a572adb4 100644 --- a/include/zenoh/api/publisher.hxx +++ b/include/zenoh/api/publisher.hxx @@ -101,13 +101,10 @@ class Publisher : public Owned<::z_owned_publisher_t> { "Failed to perform delete_resource operation"); } -#ifdef ZENOHCXX_ZENOHC /// @brief Get the key expression of the publisher. - /// @note zenoh-c only. const KeyExpr& get_keyexpr() const { return interop::as_owned_cpp_ref(::z_publisher_keyexpr(interop::as_loaned_c_ptr(*this))); } -#endif #if defined(ZENOHCXX_ZENOHC) && defined(UNSTABLE) /// @warning This API has been marked as unstable: it works as advertised, but it may be changed in a future /// release. diff --git a/include/zenoh/api/queryable.hxx b/include/zenoh/api/queryable.hxx index 3c36fce3..a1fcd270 100644 --- a/include/zenoh/api/queryable.hxx +++ b/include/zenoh/api/queryable.hxx @@ -27,6 +27,15 @@ class QueryableBase : public Owned<::z_owned_queryable_t> { QueryableBase(zenoh::detail::null_object_t) : Owned(nullptr){}; QueryableBase(::z_owned_queryable_t* q) : Owned(q){}; + public: + /// @brief Undeclares queryable. + /// @param err if not null, the result code will be written to this location, otherwise ZException exception will be + /// thrown in case of error. + void undeclare(ZResult* err = nullptr) && { + __ZENOH_RESULT_CHECK(::z_undeclare_queryable(interop::as_moved_c_ptr(*this)), err, + "Failed to undeclare queryable"); + } + friend class zenoh::Session; }; } // namespace detail @@ -40,13 +49,15 @@ class Queryable : public detail::QueryableBase { public: using QueryableBase::QueryableBase; - + using QueryableBase::undeclare; friend class Session; }; /// A Zenoh queryable. Constructed by ``Session::declare_queryable`` method. /// @tparam Handler Streaming handler exposing data. If `void`, no handler access is provided and instead data is being -/// processed inside the callback. +/// processed inside the callback. Dropping handler-less queryable does not disable the callback. The corresponding +/// messages are still received and processed unti the corresponding session is destroyed or closed. If callback needs +/// to be disabled undeclare method should be called instead. template class Queryable : public detail::QueryableBase { Handler _handler; @@ -67,7 +78,13 @@ class Queryable : public detail::QueryableBase { /// @brief Return handler to queryable data stream. const Handler& handler() const { return _handler; }; + using QueryableBase::undeclare; friend class Session; + + ~Queryable() { + ZResult err; + std::move(*this).undeclare(&err); + } }; namespace interop { diff --git a/include/zenoh/api/session.hxx b/include/zenoh/api/session.hxx index 3d1d2b57..1d79fcaa 100644 --- a/include/zenoh/api/session.hxx +++ b/include/zenoh/api/session.hxx @@ -57,6 +57,12 @@ class Session : public Owned<::z_owned_session_t> { static SessionOptions create_default() { return {}; } }; + /// @brief Options to be passed when closing a ``Session``. + struct SessionCloseOptions { + /// @name Fields + static SessionCloseOptions create_default() { return {}; } + }; + /// @name Constructors /// @brief Create a new Session. @@ -66,7 +72,8 @@ class Session : public Owned<::z_owned_session_t> { /// thrown in case of error. Session(Config&& config, SessionOptions&& options = SessionOptions::create_default(), ZResult* err = nullptr) : Owned(nullptr) { - __ZENOH_RESULT_CHECK(::z_open(&this->_0, interop::as_moved_c_ptr(config)), err, "Failed to open session"); + __ZENOH_RESULT_CHECK(::z_open(&this->_0, interop::as_moved_c_ptr(config), nullptr), err, + "Failed to open session"); #ifdef ZENOHCXX_ZENOHPICO if (err != nullptr && *err != Z_OK) return; if (options.start_background_tasks) { @@ -76,7 +83,7 @@ class Session : public Owned<::z_owned_session_t> { this->start_lease_task(&err_inner); } if (err_inner == Z_OK) return; - ::z_close(::z_move(this->_0)); + ::z_close(::z_move(this->_0), nullptr); __ZENOH_RESULT_CHECK(err_inner, err, "Failed to start background tasks"); } #else @@ -88,9 +95,13 @@ class Session : public Owned<::z_owned_session_t> { /// @brief Create a new Session with custom SHM client set. /// @param config Zenoh session ``Config``. /// @param shm_storage Storage with custom SHM clients. + /// @param options Options to pass to session creation operation. /// @param err if not null, the result code will be written to this location, otherwise ZException exception will be /// thrown in case of error. - Session(Config&& config, const ShmClientStorage& shm_storage, ZResult* err = nullptr) : Owned(nullptr) { + Session(Config&& config, const ShmClientStorage& shm_storage, + SessionOptions&& options = SessionOptions::create_default(), ZResult* err = nullptr) + : Owned(nullptr) { + (void)options; __ZENOH_RESULT_CHECK(::z_open_with_custom_shm_clients(&this->_0, interop::as_moved_c_ptr(config), interop::as_loaned_c_ptr(shm_storage)), err, "Failed to open session"); @@ -112,24 +123,17 @@ class Session : public Owned<::z_owned_session_t> { /// @brief A factory method equivalent to a ``Session`` constructor for custom SHM clients list. /// @param config Zenoh session ``Config``. /// @param shm_storage Storage with custom SHM clients. + /// @param options Options to pass to session creation operation. /// @param err if not null, the result code will be written to this location, otherwise ZException exception will be /// thrown in case of error. /// @return ``Session`` object. In case of failure it will be return in invalid state. - static Session open(Config&& config, const ShmClientStorage& shm_storage, ZResult* err = nullptr) { - return Session(std::move(config), shm_storage, err); + static Session open(Config&& config, const ShmClientStorage& shm_storage, + SessionOptions&& options = SessionOptions::create_default(), ZResult* err = nullptr) { + return Session(std::move(config), shm_storage, std::move(options), err); } #endif /// @name Methods - - /// @brief Create a shallow copy of the session. - /// @return a new ``Session`` instance. - Session clone() const { - Session s(zenoh::detail::null_object); - ::z_session_clone(&s._0, interop::as_loaned_c_ptr(*this)); - return s; - } - #if defined UNSTABLE /// @warning This API has been marked as unstable: it works as advertised, but it may be changed in a future /// release. @@ -879,5 +883,15 @@ class Session : public Owned<::z_owned_session_t> { __ZENOH_RESULT_CHECK(z_timestamp_new(&t, interop::as_loaned_c_ptr(*this)), err, "Failed to create timestamp"); return interop::into_copyable_cpp_obj(t); } + + /// @brief Close and invalidate the session. This also undeclares all non-undeclared Subscriber and Queryable + /// callbacks. + /// @param options options to pass to close operation. + /// @param err if not null, the result code will be written to this location, otherwise ZException exception will be + /// thrown in case of error. + void close(SessionCloseOptions&& options = SessionCloseOptions::create_default(), ZResult* err = nullptr) && { + (void)options; + __ZENOH_RESULT_CHECK(::z_close(interop::as_moved_c_ptr(*this), nullptr), err, "Failed to close the session"); + } }; } // namespace zenoh \ No newline at end of file diff --git a/include/zenoh/api/subscriber.hxx b/include/zenoh/api/subscriber.hxx index fccc68f6..ee04a1e7 100644 --- a/include/zenoh/api/subscriber.hxx +++ b/include/zenoh/api/subscriber.hxx @@ -29,13 +29,17 @@ class SubscriberBase : public Owned<::z_owned_subscriber_t> { SubscriberBase(::z_owned_subscriber_t* s) : Owned(s){}; public: -#ifdef ZENOHCXX_ZENOHC /// @brief Get the key expression of the subscriber - /// @note zenoh-c only. const KeyExpr& get_keyexpr() const { return interop::as_owned_cpp_ref(::z_subscriber_keyexpr(interop::as_loaned_c_ptr(*this))); } -#endif + /// @brief Undeclares subscriber. + /// @param err if not null, the result code will be written to this location, otherwise ZException exception will be + /// thrown in case of error. + void undeclare(ZResult* err = nullptr) && { + __ZENOH_RESULT_CHECK(::z_undeclare_subscriber(interop::as_moved_c_ptr(*this)), err, + "Failed to undeclare subscriber"); + } friend class zenoh::Session; }; @@ -48,12 +52,18 @@ class Subscriber : public detail::SubscriberBase { protected: using SubscriberBase::SubscriberBase; friend class Session; + + public: + using SubscriberBase::get_keyexpr; + using SubscriberBase::undeclare; }; /// A Zenoh subscriber. Destroying subscriber cancels the subscription. /// Constructed by ``Session::declare_subscriber`` method. /// @tparam Handler Streaming handler exposing data. If `void`, no handler access is provided and instead data is being -/// processed inside the callback. +/// processed inside the callback. Dropping handler-less subscriber does not disable the callback. The corresponding +/// messages are still received and processed unti the corresponding session is destroyed or closed. If callback needs +/// to be disabled undeclare method should be called instead. template class Subscriber : public detail::SubscriberBase { Handler _handler; @@ -71,13 +81,17 @@ class Subscriber : public detail::SubscriberBase { : SubscriberBase(interop::as_owned_c_ptr(s)), _handler(std::move(handler)) {} /// @name Methods - -#ifdef ZENOHCXX_ZENOHC using SubscriberBase::get_keyexpr; -#endif + using SubscriberBase::undeclare; + /// @brief Return the handler to subscriber data stream. const Handler& handler() const { return _handler; }; friend class Session; + + ~Subscriber() { + ZResult err; + std::move(*this).undeclare(&err); + } }; namespace interop { diff --git a/tests/universal/network/pub_sub.cxx b/tests/universal/network/pub_sub.cxx index 4bc2d86d..5ad85b30 100644 --- a/tests/universal/network/pub_sub.cxx +++ b/tests/universal/network/pub_sub.cxx @@ -77,6 +77,12 @@ void pub_sub(Talloc& alloc) { assert(received_messages[0].second == "first"); assert(received_messages[1].first == "zenoh/test"); assert(received_messages[1].second == "second"); +#ifdef ZENOHCXX_ZENOHC // TODO: remove once pico supports background declarations + /// check that drop does not undeclare + assert(!subscriber_dropped); + std::move(session2).close(); + std::this_thread::sleep_for(1s); +#endif assert(subscriber_dropped); } diff --git a/tests/universal/network/queryable_get.cxx b/tests/universal/network/queryable_get.cxx index 1c39a55f..2ba21e88 100644 --- a/tests/universal/network/queryable_get.cxx +++ b/tests/universal/network/queryable_get.cxx @@ -94,6 +94,12 @@ void queryable_get() { qd = {"zenoh/test/1", "err", 3}; assert(queries[2] == qd); +#ifdef ZENOHCXX_ZENOHC // TODO: remove once pico supports background declarations + /// check that drop does not undeclare + assert(!queryable_dropped); + std::move(session1).close(); + std::this_thread::sleep_for(1s); +#endif assert(queryable_dropped); assert(replies.size() == 2); diff --git a/zenoh-c b/zenoh-c index ad18d186..837ebca1 160000 --- a/zenoh-c +++ b/zenoh-c @@ -1 +1 @@ -Subproject commit ad18d1860b9322ce277e6bd00fb1128eb2d68a94 +Subproject commit 837ebca1bee268fb7e626f89212363e400ca9a3a diff --git a/zenoh-pico b/zenoh-pico index 9e926b5b..ee9b5bbe 160000 --- a/zenoh-pico +++ b/zenoh-pico @@ -1 +1 @@ -Subproject commit 9e926b5b517336d2a78d54b9515f420241fc7dc1 +Subproject commit ee9b5bbe10dba92f3276d52eae6a36e8d6cfdb06