Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added windows support #273

Open
wants to merge 8 commits into
base: rolling
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
7 changes: 6 additions & 1 deletion rmw_zenoh_cpp/src/detail/zenoh_router_check.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,12 @@ rmw_ret_t zenoh_router_check(z_session_t session)
};

rmw_ret_t ret = RMW_RET_OK;
z_owned_closure_zid_t router_callback = z_closure(callback, nullptr /* drop */, &context);
// z_owned_closure_zid_t router_callback = z_closure(callback, nullptr /* drop */, &context);
z_owned_closure_zid_t router_callback;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

z_closure is a define which I believe use syntax that it's not compatible with the windows compiler

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. It would be good to document this. Also let's add a function make_z_closure() to the anonymous namesapce that constructs and returns the z_owned_closure_zid_t. That way we implement it once somewhere and we simply update the z_closure() calls everywhere to make_z_closure().

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ahcorde I think you may have missed this suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are different z_owned_closure_*_t, I can create a template function. Where is the right file to include this new function ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be great! I'd put the function in detail/rmw_data_types.hpp right above where the sub_data_handler is defined!

router_callback.context = static_cast<void *>(&context);
router_callback.call = callback;
router_callback.drop = nullptr;

if (z_info_routers_zid(session, z_move(router_callback))) {
RMW_ZENOH_LOG_ERROR_NAMED(
"rmw_zenoh_cpp",
Expand Down
9 changes: 7 additions & 2 deletions rmw_zenoh_cpp/src/rmw_init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,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) != 0) {
RMW_SET_ERROR_MSG("Error configuring Zenoh logging.");
return RMW_RET_ERROR;
}
Expand Down Expand Up @@ -359,7 +359,12 @@ rmw_init(const rmw_init_options_t * options, rmw_context_t * context)
// z_move(callback),
// &sub_options);
auto sub_options = zc_liveliness_subscriber_options_null();
z_owned_closure_sample_t callback = z_closure(graph_sub_data_handler, nullptr, context->impl);
// z_owned_closure_sample_t callback = z_closure(graph_sub_data_handler, nullptr, context->impl);
z_owned_closure_sample_t callback;
callback.context = static_cast<void *>(context->impl);
callback.call = graph_sub_data_handler;
callback.drop = nullptr;

context->impl->graph_subscriber = zc_liveliness_declare_subscriber(
z_loan(context->impl->session),
z_keyexpr(liveliness_str.c_str()),
Expand Down
27 changes: 20 additions & 7 deletions rmw_zenoh_cpp/src/rmw_zenoh.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1445,9 +1445,14 @@ rmw_create_subscription(
allocator->deallocate(type_hash_c_str, allocator->state);
});

// Everything above succeeded and is setup properly. Now declare a subscriber
// Everything above succeeded and is setup properly. Now declare a subscriber
// with Zenoh; after this, callbacks may come in at any time.
z_owned_closure_sample_t callback = z_closure(rmw_zenoh_cpp::sub_data_handler, nullptr, sub_data);
// z_owned_closure_sample_t callback = z_closure(
// rmw_zenoh_cpp::sub_data_handler, nullptr, sub_data);
z_owned_closure_sample_t callback;
callback.context = static_cast<void *>(sub_data);
callback.call = rmw_zenoh_cpp::sub_data_handler;
callback.drop = nullptr;
z_owned_keyexpr_t keyexpr = ros_topic_name_to_zenoh_key(
node->context->actual_domain_id,
topic_name,
Expand Down Expand Up @@ -2494,8 +2499,12 @@ rmw_send_request(
// and any number.
opts.consolidation = z_query_consolidation_latest();
opts.value.payload = z_bytes_t{data_length, reinterpret_cast<const uint8_t *>(request_bytes)};
z_owned_closure_reply_t zn_closure_reply =
z_closure(rmw_zenoh_cpp::client_data_handler, rmw_zenoh_cpp::client_data_drop, client_data);
// z_owned_closure_reply_t zn_closure_reply =
// z_closure(rmw_zenoh_cpp::client_data_handler, rmw_zenoh_cpp::client_data_drop, client_data);
z_owned_closure_reply_t zn_closure_reply;
zn_closure_reply.context = static_cast<void *>(client_data);
zn_closure_reply.call = rmw_zenoh_cpp::client_data_handler;
zn_closure_reply.drop = rmw_zenoh_cpp::client_data_drop;
z_get(
z_loan(context_impl->session),
z_loan(client_data->keyexpr), "",
Expand Down Expand Up @@ -2865,9 +2874,13 @@ rmw_create_service(
return nullptr;
}

z_owned_closure_query_t callback = z_closure(
rmw_zenoh_cpp::service_data_handler, nullptr,
service_data);
// z_owned_closure_query_t callback = z_closure(
// rmw_zenoh_cpp::service_data_handler, nullptr,
// service_data);
z_owned_closure_query_t callback;
callback.context = static_cast<void *>(service_data);
callback.call = rmw_zenoh_cpp::service_data_handler;
callback.drop = nullptr;
// Configure the queryable to process complete queries.
z_queryable_options_t qable_options = z_queryable_options_default();
qable_options.complete = true;
Expand Down
3 changes: 2 additions & 1 deletion rmw_zenoh_cpp/src/zenohd/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) != 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be better to leave the switch to rcutils_set_env_overwrite/rcutils_set_env2 once the upstream PR is reviewed and merged. Given that it may not be backported to iron/jazzy, it might be best if we use a combination of rcutils_get_env check first followed by rcutils_set_env to get the same behavior as rcutils_set_env_overwrite.

RMW_SET_ERROR_MSG("Error configuring Zenoh logging.");
return 1;
}
Expand Down
2 changes: 1 addition & 1 deletion zenoh_c_vendor/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ set(ZENOHC_CARGO_FLAGS "--no-default-features$<SEMICOLON>--features=zenoh/shared
# - https://github.com/eclipse-zenoh/zenoh/pull/1150 (fix deadlock issue https://github.com/ros2/rmw_zenoh/issues/182)
ament_vendor(zenoh_c_vendor
VCS_URL https://github.com/eclipse-zenoh/zenoh-c.git
VCS_VERSION 548ee8dde0f53a58c06e68a2949949b31140c36c
VCS_VERSION 134dbfa06ca212def5fb51dd8e816734dfd8dff6
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With #269 merged, the commit version is the latest zenoh-c so we don't have to change the commit here to an older one.

CMAKE_ARGS
"-DZENOHC_CARGO_FLAGS=${ZENOHC_CARGO_FLAGS}"
)
Expand Down
Loading