From 060deaebc2dd5755aa1422e0cd8e65782ba99066 Mon Sep 17 00:00:00 2001 From: Alejandro Hernandez Cordero Date: Wed, 4 Dec 2024 13:39:16 +0100 Subject: [PATCH 1/4] Windows support Signed-off-by: Alejandro Hernandez Cordero --- README.md | 5 ++ rmw_zenoh_cpp/src/detail/graph_cache.cpp | 5 +- rmw_zenoh_cpp/src/detail/rmw_client_data.cpp | 5 +- .../src/detail/rmw_context_impl_s.cpp | 76 ++++++++++--------- rmw_zenoh_cpp/src/detail/rmw_service_data.cpp | 8 +- .../src/detail/rmw_subscription_data.cpp | 6 +- .../src/detail/zenoh_router_check.cpp | 32 +++++--- rmw_zenoh_cpp/src/detail/zenoh_utils.hpp | 11 +++ rmw_zenoh_cpp/src/rmw_init.cpp | 2 +- rmw_zenoh_cpp/src/zenohd/main.cpp | 3 +- 10 files changed, 98 insertions(+), 55 deletions(-) diff --git a/README.md b/README.md index 164fe0f8..f922bf96 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,11 @@ For information about the Design please visit [design](docs/design.md) page. ## Requirements - [ROS 2](https://docs.ros.org): Rolling/Jazzy/Iron +### Windows + +```bash +choco install -y rust mingw +``` ## Setup diff --git a/rmw_zenoh_cpp/src/detail/graph_cache.cpp b/rmw_zenoh_cpp/src/detail/graph_cache.cpp index 776d343d..aa119512 100644 --- a/rmw_zenoh_cpp/src/detail/graph_cache.cpp +++ b/rmw_zenoh_cpp/src/detail/graph_cache.cpp @@ -13,6 +13,7 @@ // limitations under the License. #include +#include #include #include #include @@ -233,7 +234,7 @@ void GraphCache::handle_matched_events_for_put( update_event_counters( topic_info.name_, ZENOH_EVENT_PUBLICATION_MATCHED, - match_count_for_entity); + static_cast(match_count_for_entity)); if (is_entity_local(*entity) && match_count_for_entity > 0) { local_entities_with_events[entity].insert(ZENOH_EVENT_PUBLICATION_MATCHED); } @@ -260,7 +261,7 @@ void GraphCache::handle_matched_events_for_put( update_event_counters( topic_info.name_, ZENOH_EVENT_SUBSCRIPTION_MATCHED, - match_count_for_entity); + static_cast(match_count_for_entity)); if (is_entity_local(*entity) && match_count_for_entity > 0) { local_entities_with_events[entity].insert(ZENOH_EVENT_SUBSCRIPTION_MATCHED); } diff --git a/rmw_zenoh_cpp/src/detail/rmw_client_data.cpp b/rmw_zenoh_cpp/src/detail/rmw_client_data.cpp index 7bfe4c16..b185f388 100644 --- a/rmw_zenoh_cpp/src/detail/rmw_client_data.cpp +++ b/rmw_zenoh_cpp/src/detail/rmw_client_data.cpp @@ -473,7 +473,10 @@ rmw_ret_t ClientData::send_request( // capture shared_from_this() instead of this. num_in_flight_++; z_owned_closure_reply_t zn_closure_reply = - z_closure(client_data_handler, client_data_drop, this); + rmw_zenoh_cpp::make_z_closure( + static_cast(this), + &client_data_handler, + client_data_drop); z_get( context_impl->session(), z_loan(keyexpr_), "", diff --git a/rmw_zenoh_cpp/src/detail/rmw_context_impl_s.cpp b/rmw_zenoh_cpp/src/detail/rmw_context_impl_s.cpp index 50b29038..a356c0a0 100644 --- a/rmw_zenoh_cpp/src/detail/rmw_context_impl_s.cpp +++ b/rmw_zenoh_cpp/src/detail/rmw_context_impl_s.cpp @@ -33,6 +33,7 @@ #include "rmw_node_data.hpp" #include "zenoh_config.hpp" #include "zenoh_router_check.hpp" +#include "zenoh_utils.hpp" #include "rcpputils/scope_exit.hpp" #include "rmw/error_handling.h" @@ -78,11 +79,11 @@ class rmw_context_impl_s::Data final } // Check if shm is enabled. - z_owned_str_t shm_enabled = zc_config_get(z_loan(config), "transport/shared_memory/enabled"); - auto always_free_shm_enabled = rcpputils::make_scope_exit( - [&shm_enabled]() { - z_drop(z_move(shm_enabled)); - }); + // z_owned_str_t shm_enabled = zc_config_get(z_loan(config), "transport/shared_memory/enabled"); + // auto always_free_shm_enabled = rcpputils::make_scope_exit( + // [&shm_enabled]() { + // z_drop(z_move(shm_enabled)); + // }); // Initialize the zenoh session. session_ = z_open(z_move(config)); @@ -164,34 +165,34 @@ class rmw_context_impl_s::Data final // Initialize the shm manager if shared_memory is enabled in the config. shm_manager_ = std::nullopt; - if (shm_enabled._cstr != nullptr && - strcmp(shm_enabled._cstr, "true") == 0) - { - char idstr[sizeof(zid.id) * 2 + 1]; // 2 bytes for each byte of the id, plus the trailing \0 - static constexpr size_t max_size_of_each = 3; // 2 for each byte, plus the trailing \0 - for (size_t i = 0; i < sizeof(zid.id); ++i) { - snprintf(idstr + 2 * i, max_size_of_each, "%02x", zid.id[i]); - } - idstr[sizeof(zid.id) * 2] = '\0'; - // TODO(yadunund): Can we get the size of the shm from the config even though it's not - // a standard parameter? - shm_manager_ = - zc_shm_manager_new( - z_loan(session_), - idstr, - SHM_BUFFER_SIZE_MB * 1024 * 1024); - if (!shm_manager_.has_value() || - !zc_shm_manager_check(&shm_manager_.value())) - { - throw std::runtime_error("Unable to create shm manager."); - } - } - auto free_shm_manager = rcpputils::make_scope_exit( - [this]() { - if (shm_manager_.has_value()) { - z_drop(z_move(shm_manager_.value())); - } - }); + // if (shm_enabled._cstr != nullptr && + // strcmp(shm_enabled._cstr, "true") == 0) + // { + // char idstr[sizeof(zid.id) * 2 + 1]; // 2 bytes for each byte of the id, plus the trailing \0 + // static constexpr size_t max_size_of_each = 3; // 2 for each byte, plus the trailing \0 + // for (size_t i = 0; i < sizeof(zid.id); ++i) { + // snprintf(idstr + 2 * i, max_size_of_each, "%02x", zid.id[i]); + // } + // idstr[sizeof(zid.id) * 2] = '\0'; + // // TODO(yadunund): Can we get the size of the shm from the config even though it's not + // // a standard parameter? + // shm_manager_ = + // zc_shm_manager_new( + // z_loan(session_), + // idstr, + // SHM_BUFFER_SIZE_MB * 1024 * 1024); + // if (!shm_manager_.has_value() || + // !zc_shm_manager_check(&shm_manager_.value())) + // { + // throw std::runtime_error("Unable to create shm manager."); + // } + // } + // auto free_shm_manager = rcpputils::make_scope_exit( + // [this]() { + // if (shm_manager_.has_value()) { + // z_drop(z_move(shm_manager_.value())); + // } + // }); graph_guard_condition_ = std::make_unique(); graph_guard_condition_->implementation_identifier = rmw_zenoh_cpp::rmw_zenoh_identifier; @@ -200,8 +201,11 @@ class rmw_context_impl_s::Data final // Setup the liveliness subscriber to receives updates from the ROS graph // and update the graph cache. auto sub_options = zc_liveliness_subscriber_options_null(); - z_owned_closure_sample_t callback = z_closure( - graph_sub_data_handler, nullptr, this); + z_owned_closure_sample_t callback = + rmw_zenoh_cpp::make_z_closure( + static_cast(this), + graph_sub_data_handler, + nullptr); graph_subscriber_ = zc_liveliness_declare_subscriber( z_loan(session_), z_keyexpr(liveliness_str.c_str()), @@ -218,7 +222,7 @@ class rmw_context_impl_s::Data final } close_session.cancel(); - free_shm_manager.cancel(); + // free_shm_manager.cancel(); undeclare_z_sub.cancel(); } diff --git a/rmw_zenoh_cpp/src/detail/rmw_service_data.cpp b/rmw_zenoh_cpp/src/detail/rmw_service_data.cpp index 42366ef0..ea9c1892 100644 --- a/rmw_zenoh_cpp/src/detail/rmw_service_data.cpp +++ b/rmw_zenoh_cpp/src/detail/rmw_service_data.cpp @@ -36,6 +36,8 @@ #include "rmw/get_topic_endpoint_info.h" #include "rmw/impl/cpp/macros.hpp" +#include "zenoh_utils.hpp" + namespace rmw_zenoh_cpp { ///============================================================================== @@ -160,7 +162,11 @@ std::shared_ptr ServiceData::make( // TODO(Yadunund): Instead of passing a rawptr, rely on capturing weak_ptr // in the closure callback once we switch to zenoh-cpp. - z_owned_closure_query_t callback = z_closure(service_data_handler, nullptr, service_data.get()); + z_owned_closure_query_t callback = + rmw_zenoh_cpp::make_z_closure( + static_cast(service_data.get()), + &rmw_zenoh_cpp::service_data_handler, + nullptr); service_data->keyexpr_ = z_keyexpr_new(service_data->entity_->topic_info().value().topic_keyexpr_.c_str()); auto free_ros_keyexpr = rcpputils::make_scope_exit( diff --git a/rmw_zenoh_cpp/src/detail/rmw_subscription_data.cpp b/rmw_zenoh_cpp/src/detail/rmw_subscription_data.cpp index 54bf5487..cb791c92 100644 --- a/rmw_zenoh_cpp/src/detail/rmw_subscription_data.cpp +++ b/rmw_zenoh_cpp/src/detail/rmw_subscription_data.cpp @@ -38,6 +38,8 @@ #include "rmw/get_topic_endpoint_info.h" #include "rmw/impl/cpp/macros.hpp" +#include "zenoh_utils.hpp" + namespace rmw_zenoh_cpp { namespace @@ -224,7 +226,9 @@ bool SubscriptionData::init() { // TODO(Yadunund): Instead of passing a rawptr, rely on capturing weak_ptr // in the closure callback once we switch to zenoh-cpp. - z_owned_closure_sample_t callback = z_closure(sub_data_handler, nullptr, this); + z_owned_closure_sample_t callback = + rmw_zenoh_cpp::make_z_closure( + static_cast(this), sub_data_handler, nullptr); z_owned_keyexpr_t keyexpr = z_keyexpr_new(entity_->topic_info()->topic_keyexpr_.c_str()); auto always_free_ros_keyexpr = rcpputils::make_scope_exit( diff --git a/rmw_zenoh_cpp/src/detail/zenoh_router_check.cpp b/rmw_zenoh_cpp/src/detail/zenoh_router_check.cpp index 7d7afaa9..9028fb9d 100644 --- a/rmw_zenoh_cpp/src/detail/zenoh_router_check.cpp +++ b/rmw_zenoh_cpp/src/detail/zenoh_router_check.cpp @@ -23,26 +23,34 @@ #include "logging_macros.hpp" #include "liveliness_utils.hpp" +#include "zenoh_utils.hpp" + namespace rmw_zenoh_cpp { + +// Define callback +void callback_id(const struct z_id_t * id, void * ctx) +{ + const std::string id_str = liveliness::zid_to_str(*id); + RMW_ZENOH_LOG_INFO_NAMED( + "rmw_zenoh_cpp", + "Successfully connected to a Zenoh router with id %s.", id_str.c_str()); + // Note: Callback is guaranteed to never be called + // concurrently according to z_info_routers_zid docstring + (*(static_cast(ctx)))++; +} + ///============================================================================= rmw_ret_t zenoh_router_check(z_session_t session) { // Initialize context for callback int context = 0; - // Define callback - auto callback = [](const struct z_id_t * id, void * ctx) { - const std::string id_str = liveliness::zid_to_str(*id); - RMW_ZENOH_LOG_INFO_NAMED( - "rmw_zenoh_cpp", - "Successfully connected to a Zenoh router with id %s.", id_str.c_str()); - // Note: Callback is guaranteed to never be called - // concurrently according to z_info_routers_zid docstring - (*(static_cast(ctx)))++; - }; - - z_owned_closure_zid_t router_callback = z_closure(callback, nullptr /* drop */, &context); + z_owned_closure_zid_t router_callback = + rmw_zenoh_cpp::make_z_closure( + static_cast(&context), + &callback_id, + nullptr); if (z_info_routers_zid(session, z_move(router_callback))) { RMW_ZENOH_LOG_ERROR_NAMED( "rmw_zenoh_cpp", diff --git a/rmw_zenoh_cpp/src/detail/zenoh_utils.hpp b/rmw_zenoh_cpp/src/detail/zenoh_utils.hpp index a750cbf2..da49591a 100644 --- a/rmw_zenoh_cpp/src/detail/zenoh_utils.hpp +++ b/rmw_zenoh_cpp/src/detail/zenoh_utils.hpp @@ -68,6 +68,17 @@ class ZenohQuery final z_owned_query_t query_; std::chrono::nanoseconds::rep received_timestamp_; }; + +///============================================================================= +template +T make_z_closure(void * context, void (* call)(H *, void *), void (* drop)(void *)) +{ + T closure; + closure.context = context; + closure.call = call; + closure.drop = drop; + return closure; +} } // namespace rmw_zenoh_cpp #endif // DETAIL__ZENOH_UTILS_HPP_ diff --git a/rmw_zenoh_cpp/src/rmw_init.cpp b/rmw_zenoh_cpp/src/rmw_init.cpp index c61a44fa..ed2a98f0 100644 --- a/rmw_zenoh_cpp/src/rmw_init.cpp +++ b/rmw_zenoh_cpp/src/rmw_init.cpp @@ -89,7 +89,7 @@ rmw_init(const rmw_init_options_t * options, rmw_context_t * context) // If not already defined, set the logging environment variable for Zenoh sessions // to warning level by default. // TODO(Yadunund): Switch to rcutils_get_env once it supports not overwriting values. - if (setenv(ZENOH_LOG_ENV_VAR_STR, ZENOH_LOG_WARN_LEVEL_STR, 0) != 0) { + if (!rcutils_set_env_overwrite(ZENOH_LOG_ENV_VAR_STR, ZENOH_LOG_WARN_LEVEL_STR, 0)) { RMW_SET_ERROR_MSG("Error configuring Zenoh logging."); return RMW_RET_ERROR; } diff --git a/rmw_zenoh_cpp/src/zenohd/main.cpp b/rmw_zenoh_cpp/src/zenohd/main.cpp index 8ae25809..55866035 100644 --- a/rmw_zenoh_cpp/src/zenohd/main.cpp +++ b/rmw_zenoh_cpp/src/zenohd/main.cpp @@ -30,6 +30,7 @@ #include "../detail/zenoh_config.hpp" #include "../detail/liveliness_utils.hpp" +#include "rcutils/env.h" #include "rmw/error_handling.h" #include "rcpputils/scope_exit.hpp" @@ -63,7 +64,7 @@ int main(int argc, char ** argv) // If not already defined, set the logging environment variable for Zenoh router // to info level by default. // TODO(Yadunund): Switch to rcutils_get_env once it supports not overwriting values. - if (setenv(ZENOH_LOG_ENV_VAR_STR, ZENOH_LOG_INFO_LEVEL_STR, 0) != 0) { + if (!rcutils_set_env_overwrite(ZENOH_LOG_ENV_VAR_STR, ZENOH_LOG_INFO_LEVEL_STR, 0)) { RMW_SET_ERROR_MSG("Error configuring Zenoh logging."); return 1; } From e7a06d22227f985b776071ed34c9b60d2218a5fe Mon Sep 17 00:00:00 2001 From: Alejandro Hernandez Cordero Date: Wed, 4 Dec 2024 13:42:28 +0100 Subject: [PATCH 2/4] make linters happy Signed-off-by: Alejandro Hernandez Cordero --- rmw_zenoh_cpp/src/detail/rmw_context_impl_s.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rmw_zenoh_cpp/src/detail/rmw_context_impl_s.cpp b/rmw_zenoh_cpp/src/detail/rmw_context_impl_s.cpp index a356c0a0..a239d4d5 100644 --- a/rmw_zenoh_cpp/src/detail/rmw_context_impl_s.cpp +++ b/rmw_zenoh_cpp/src/detail/rmw_context_impl_s.cpp @@ -168,7 +168,8 @@ class rmw_context_impl_s::Data final // if (shm_enabled._cstr != nullptr && // strcmp(shm_enabled._cstr, "true") == 0) // { - // char idstr[sizeof(zid.id) * 2 + 1]; // 2 bytes for each byte of the id, plus the trailing \0 + // // 2 bytes for each byte of the id, plus the trailing \0 + // char idstr[sizeof(zid.id) * 2 + 1]; // static constexpr size_t max_size_of_each = 3; // 2 for each byte, plus the trailing \0 // for (size_t i = 0; i < sizeof(zid.id); ++i) { // snprintf(idstr + 2 * i, max_size_of_each, "%02x", zid.id[i]); From ae44d68f07322fdc2cdab119da5a75afc4be36d3 Mon Sep 17 00:00:00 2001 From: Alejandro Hernandez Cordero Date: Wed, 4 Dec 2024 13:46:37 +0100 Subject: [PATCH 3/4] make linters happy Signed-off-by: Alejandro Hernandez Cordero --- rmw_zenoh_cpp/src/detail/rmw_context_impl_s.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rmw_zenoh_cpp/src/detail/rmw_context_impl_s.cpp b/rmw_zenoh_cpp/src/detail/rmw_context_impl_s.cpp index a239d4d5..01930b43 100644 --- a/rmw_zenoh_cpp/src/detail/rmw_context_impl_s.cpp +++ b/rmw_zenoh_cpp/src/detail/rmw_context_impl_s.cpp @@ -169,7 +169,7 @@ class rmw_context_impl_s::Data final // strcmp(shm_enabled._cstr, "true") == 0) // { // // 2 bytes for each byte of the id, plus the trailing \0 - // char idstr[sizeof(zid.id) * 2 + 1]; + // char idstr[sizeof(zid.id) * 2 + 1]; // static constexpr size_t max_size_of_each = 3; // 2 for each byte, plus the trailing \0 // for (size_t i = 0; i < sizeof(zid.id); ++i) { // snprintf(idstr + 2 * i, max_size_of_each, "%02x", zid.id[i]); From b621ac73fdc2ed781a966ce5c3b8983d48b33c6c Mon Sep 17 00:00:00 2001 From: Alejandro Hernandez Cordero Date: Thu, 5 Dec 2024 11:58:27 +0100 Subject: [PATCH 4/4] Fix tricky issue on windows Signed-off-by: Alejandro Hernandez Cordero --- rmw_zenoh_cpp/src/detail/attachment_helpers.cpp | 2 +- rmw_zenoh_cpp/src/detail/rmw_service_data.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rmw_zenoh_cpp/src/detail/attachment_helpers.cpp b/rmw_zenoh_cpp/src/detail/attachment_helpers.cpp index 7733c918..81d48895 100644 --- a/rmw_zenoh_cpp/src/detail/attachment_helpers.cpp +++ b/rmw_zenoh_cpp/src/detail/attachment_helpers.cpp @@ -78,7 +78,7 @@ int64_t get_int64_from_attachment( errno = 0; char * endptr; - int64_t num = strtol(int64_str, &endptr, 10); + int64_t num = std::strtoll(int64_str, &endptr, 10); if (num == 0) { // This is an error regardless; the client should never send this return -1; diff --git a/rmw_zenoh_cpp/src/detail/rmw_service_data.cpp b/rmw_zenoh_cpp/src/detail/rmw_service_data.cpp index ea9c1892..f1fa957a 100644 --- a/rmw_zenoh_cpp/src/detail/rmw_service_data.cpp +++ b/rmw_zenoh_cpp/src/detail/rmw_service_data.cpp @@ -331,14 +331,14 @@ rmw_ret_t ServiceData::take_request( request_header->request_id.sequence_number = get_int64_from_attachment(&attachment, "sequence_number"); if (request_header->request_id.sequence_number < 0) { - RMW_SET_ERROR_MSG("Failed to get sequence_number from client call attachment"); + RMW_SET_ERROR_MSG("Failed to get sequence_number from service call attachment"); return RMW_RET_ERROR; } request_header->source_timestamp = get_int64_from_attachment( &attachment, "source_timestamp"); if (request_header->source_timestamp < 0) { - RMW_SET_ERROR_MSG("Failed to get source_timestamp from client call attachment"); + RMW_SET_ERROR_MSG("Failed to get source_timestamp from service call attachment"); return RMW_RET_ERROR; } if (!get_gid_from_attachment(