Added windows support #273
Conversation
Signed-off-by: Alejandro Hernandez Cordero <[email protected]>
Signed-off-by: Alejandro Hernandez Cordero <[email protected]>
Signed-off-by: Alejandro Hernandez Cordero <[email protected]>
Signed-off-by: Alejandro Hernández Cordero <[email protected]>
zenoh_c_vendor/CMakeLists.txt
Outdated
| @@ -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 | |||
There was a problem hiding this comment.
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.
rmw_zenoh_cpp/src/zenohd/main.cpp
Outdated
| @@ -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) { | |||
There was a problem hiding this comment.
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.
| @@ -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; | |||
There was a problem hiding this comment.
Could you explain this change?
There was a problem hiding this comment.
z_closure is a define which I believe use syntax that it's not compatible with the windows compiler
There was a problem hiding this comment.
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().
There was a problem hiding this comment.
@ahcorde I think you may have missed this suggestion.
There was a problem hiding this comment.
There are different z_owned_closure_*_t, I can create a template function. Where is the right file to include this new function ?
There was a problem hiding this comment.
That would be great! I'd put the function in detail/rmw_data_types.hpp right above where the sub_data_handler is defined!
Signed-off-by: Alejandro Hernández Cordero <[email protected]>
Signed-off-by: Alejandro Hernández Cordero <[email protected]>
Signed-off-by: Alejandro Hernández Cordero <[email protected]>
Require this PR ros2/rcutils#473 in rcutils
The code just compile, I didn't try the code yet.