From f83ab5290c95bacfcd68d0062491f74f094e0f60 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Mon, 4 Nov 2024 19:12:44 -0500 Subject: [PATCH 1/2] Fix the received_timestamp for client data. (#305) These should be timestamped at the time they are received, not at the time that they are taken (that might be much later). Signed-off-by: Chris Lalancette --- rmw_zenoh_cpp/src/detail/rmw_data_types.cpp | 11 +++++------ rmw_zenoh_cpp/src/detail/rmw_data_types.hpp | 1 + rmw_zenoh_cpp/src/detail/zenoh_utils.cpp | 11 ++++++++++- rmw_zenoh_cpp/src/detail/zenoh_utils.hpp | 6 +++++- rmw_zenoh_cpp/src/rmw_zenoh.cpp | 4 +--- 5 files changed, 22 insertions(+), 11 deletions(-) diff --git a/rmw_zenoh_cpp/src/detail/rmw_data_types.cpp b/rmw_zenoh_cpp/src/detail/rmw_data_types.cpp index c1e56bfd..acf0bc70 100644 --- a/rmw_zenoh_cpp/src/detail/rmw_data_types.cpp +++ b/rmw_zenoh_cpp/src/detail/rmw_data_types.cpp @@ -12,17 +12,13 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include -#include +#include #include #include #include -#include "liveliness_utils.hpp" #include "logging_macros.hpp" -#include "rcpputils/scope_exit.hpp" - #include "rmw/error_handling.h" #include "rmw/impl/cpp/macros.hpp" @@ -188,7 +184,10 @@ void client_data_handler(z_owned_reply_t * reply, void * data) return; } - client_data->add_new_reply(std::make_unique(reply)); + std::chrono::nanoseconds::rep received_timestamp = + std::chrono::system_clock::now().time_since_epoch().count(); + + client_data->add_new_reply(std::make_unique(reply, received_timestamp)); // Since we took ownership of the reply, null it out here *reply = z_reply_null(); } diff --git a/rmw_zenoh_cpp/src/detail/rmw_data_types.hpp b/rmw_zenoh_cpp/src/detail/rmw_data_types.hpp index 5205f3cf..5605d7f0 100644 --- a/rmw_zenoh_cpp/src/detail/rmw_data_types.hpp +++ b/rmw_zenoh_cpp/src/detail/rmw_data_types.hpp @@ -31,6 +31,7 @@ #include "rosidl_runtime_c/type_hash.h" #include "event.hpp" +#include "liveliness_utils.hpp" #include "message_type_support.hpp" #include "rmw_wait_set_data.hpp" #include "service_type_support.hpp" diff --git a/rmw_zenoh_cpp/src/detail/zenoh_utils.cpp b/rmw_zenoh_cpp/src/detail/zenoh_utils.cpp index db07d01e..2e6c8e0c 100644 --- a/rmw_zenoh_cpp/src/detail/zenoh_utils.cpp +++ b/rmw_zenoh_cpp/src/detail/zenoh_utils.cpp @@ -82,9 +82,12 @@ const z_query_t ZenohQuery::get_query() const } ///============================================================================= -ZenohReply::ZenohReply(const z_owned_reply_t * reply) +ZenohReply::ZenohReply( + const z_owned_reply_t * reply, + std::chrono::nanoseconds::rep received_timestamp) { reply_ = *reply; + received_timestamp_ = received_timestamp; } ///============================================================================= @@ -102,4 +105,10 @@ std::optional ZenohReply::get_sample() const return std::nullopt; } + +///============================================================================= +std::chrono::nanoseconds::rep ZenohReply::get_received_timestamp() const +{ + return received_timestamp_; +} } // namespace rmw_zenoh_cpp diff --git a/rmw_zenoh_cpp/src/detail/zenoh_utils.hpp b/rmw_zenoh_cpp/src/detail/zenoh_utils.hpp index c57fc87e..f7ec26b2 100644 --- a/rmw_zenoh_cpp/src/detail/zenoh_utils.hpp +++ b/rmw_zenoh_cpp/src/detail/zenoh_utils.hpp @@ -17,6 +17,7 @@ #include +#include #include #include @@ -37,14 +38,17 @@ create_map_and_set_sequence_num(int64_t sequence_number, GIDCopier gid_copier); class ZenohReply final { public: - ZenohReply(const z_owned_reply_t * reply); + ZenohReply(const z_owned_reply_t * reply, std::chrono::nanoseconds::rep received_timestamp); ~ZenohReply(); std::optional get_sample() const; + std::chrono::nanoseconds::rep get_received_timestamp() const; + private: z_owned_reply_t reply_; + std::chrono::nanoseconds::rep received_timestamp_; }; // A class to store the queries made by clients. diff --git a/rmw_zenoh_cpp/src/rmw_zenoh.cpp b/rmw_zenoh_cpp/src/rmw_zenoh.cpp index df2529a6..31956676 100644 --- a/rmw_zenoh_cpp/src/rmw_zenoh.cpp +++ b/rmw_zenoh_cpp/src/rmw_zenoh.cpp @@ -1896,9 +1896,7 @@ rmw_take_response( return RMW_RET_ERROR; } - auto now = std::chrono::system_clock::now().time_since_epoch(); - auto now_ns = std::chrono::duration_cast(now); - request_header->received_timestamp = now_ns.count(); + request_header->received_timestamp = latest_reply->get_received_timestamp(); *taken = true; From fbdd17a06e8a3dba8cd1379b1d8704095b79dc84 Mon Sep 17 00:00:00 2001 From: yadunund Date: Tue, 5 Nov 2024 08:13:00 +0800 Subject: [PATCH 2/2] Fix compile on OSX (#306) * Fix -Wpessimizing-move Signed-off-by: Yadunund * Compile with apple clang Signed-off-by: Yadunund * Fix -Wunused-lambda-capture Signed-off-by: Yadunund * Fix -Wgnu-zero-variadic-macro-arguments Signed-off-by: Yadunund --------- Signed-off-by: Yadunund --- rmw_zenoh_cpp/src/detail/graph_cache.cpp | 4 ++-- rmw_zenoh_cpp/src/detail/rmw_service_data.cpp | 2 +- rmw_zenoh_cpp/src/rmw_init_options.cpp | 2 +- rmw_zenoh_cpp/src/rmw_zenoh.cpp | 7 ++++++- 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/rmw_zenoh_cpp/src/detail/graph_cache.cpp b/rmw_zenoh_cpp/src/detail/graph_cache.cpp index f815aedd..22f1b153 100644 --- a/rmw_zenoh_cpp/src/detail/graph_cache.cpp +++ b/rmw_zenoh_cpp/src/detail/graph_cache.cpp @@ -1316,8 +1316,8 @@ void GraphCache::set_querying_subscriber_callback( std::unordered_map >::iterator cb_it = querying_subs_cbs_.find(sub_keyexpr); if (cb_it == querying_subs_cbs_.end()) { - querying_subs_cbs_[sub_keyexpr] = std::move( - std::unordered_map{}); + querying_subs_cbs_[sub_keyexpr] = + std::unordered_map{}; cb_it = querying_subs_cbs_.find(sub_keyexpr); } cb_it->second.insert(std::make_pair(sub_keyxpr_hash, std::move(cb))); diff --git a/rmw_zenoh_cpp/src/detail/rmw_service_data.cpp b/rmw_zenoh_cpp/src/detail/rmw_service_data.cpp index 50fe9357..9be89277 100644 --- a/rmw_zenoh_cpp/src/detail/rmw_service_data.cpp +++ b/rmw_zenoh_cpp/src/detail/rmw_service_data.cpp @@ -141,7 +141,7 @@ std::shared_ptr ServiceData::make( RMW_ZENOH_LOG_ERROR_NAMED( "rmw_zenoh_cpp", "Unable to generate keyexpr for liveliness token for the service %s.", - service_name); + service_name.c_str()); return nullptr; } diff --git a/rmw_zenoh_cpp/src/rmw_init_options.cpp b/rmw_zenoh_cpp/src/rmw_init_options.cpp index 2b79566a..783c5a6a 100644 --- a/rmw_zenoh_cpp/src/rmw_init_options.cpp +++ b/rmw_zenoh_cpp/src/rmw_init_options.cpp @@ -98,7 +98,7 @@ rmw_init_options_copy(const rmw_init_options_t * src, rmw_init_options_t * dst) return ret; } auto free_discovery_options = rcpputils::make_scope_exit( - [&tmp, allocator]() { + [&tmp]() { rmw_ret_t tmp_ret = rmw_discovery_options_fini(&tmp.discovery_options); static_cast(tmp_ret); }); diff --git a/rmw_zenoh_cpp/src/rmw_zenoh.cpp b/rmw_zenoh_cpp/src/rmw_zenoh.cpp index 31956676..761fbdcf 100644 --- a/rmw_zenoh_cpp/src/rmw_zenoh.cpp +++ b/rmw_zenoh_cpp/src/rmw_zenoh.cpp @@ -1438,7 +1438,12 @@ rmw_create_client( allocator->deallocate(client_data, allocator->state); }); - RMW_TRY_PLACEMENT_NEW(client_data, client_data, return nullptr, rmw_zenoh_cpp::rmw_client_data_t); + RMW_TRY_PLACEMENT_NEW( + client_data, + client_data, + return nullptr, + rmw_zenoh_cpp::rmw_client_data_t, + ); auto destruct_client_data = rcpputils::make_scope_exit( [client_data]() { RMW_TRY_DESTRUCTOR_FROM_WITHIN_FAILURE(