search

ros2 / rmw_zenoh

RMW for ROS 2 using Zenoh as the middleware
Apache License 2.0
185 stars 34 forks source link

Use a zenoh session to run the router #101

Closed clalancette closed 8 months ago

clalancette commented 8 months ago

The main benefit here is that we stop vendoring zenohd, which should improve our compile times. While in here, we also rewrite the keyboard handling so it will work on both Linux and Windows (though only tested on Linux so far).

This is mostly the work of @francocipollone ; I just did the keyboard handling part. This should fix #64

clalancette commented 8 months ago

The code here works great. Did we come to a conclusion regarding tradeoffs between rust vs C based impl as discussed here?

As of right now, I don't think we need any router features that we can't get via the configuration file. Given how far along we are in the implementation, it seems to me that we probably won't need it; the one possible exception is security, which we haven't thought about at all. The good news is that if we merge this, then we have both options in the history; we have the ability to run the Rust-based one (the commits prior to this), and we'll have this one. So worst case, we could always revert this and go back to the Rust-based router.

If we want to stick to the C approach, I think it would be nice to refactor detail/zenoh_config.hpp to be flexible enough to return configs for sessions or routers. Perhaps even look for an envar for the router config as well.

Yeah, that would work. That way the user could also configure the router, which is one of our long-term goals anyway. I'll take a look at doing that.

clalancette commented 8 months ago

Thanks for clarifying. Happy to merge first and work on reusing the config generator code in a separate PR. Maybe we can open a ticket so we don't forget.

Yep, good call. I've opened up #102 to track that.