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

Vendor zenohd #64

Closed
Yadunund opened this issue Nov 15, 2023 · 5 comments · Fixed by #101
Closed

Vendor zenohd #64

Yadunund opened this issue Nov 15, 2023 · 5 comments · Fixed by #101
Assignees

Comments

@Yadunund
Copy link
Member

Right now users need to manually install zenohd router.

We could have zenoh_c_vendor patch the Cargo.toml.in file in zenoh-c to include zenohd as a binary artifact to be built. But this requires some updates to cargo which are discussed here.

Other viable options may be discussed below.

@JEnoch
Copy link
Contributor

JEnoch commented Nov 15, 2023

I see 2 alternative options.

Context:

zenohd is "just" a main.rs file that depends on zenoh crate.
It performs the following tasks:

  • it parses the command line arguments, reading a config file if provided and creates a Config with "router" mode.
  • it instanciates a PluginManager for plugins and storages management
  • it creates an AdminSpace that replies to queries on admin space (key expr: @/router/<router_id>/**)
  • it creates a Runtime with the Config. A Runtime is the core part of a Zenoh, managing the transports, the protocol, but also the routing tables in case "router" or "peer" mode is configured.

Alternative 1: your own Rust router

This repo could implement its own main.rs copying or adapting the zenohd's main.rs

Pros:

  • no need of cargo updates
  • you can adapt the router to your need, enabling/disabling some Rust features, adding/removing command line argument arguments or forcing some configuration.

Cons:

  • you have to maintain it and possibly cherry-pick changes made in zenohd's main.rs

Alternative 2: your own "C" router

Whatever the language, a Zenoh Session embeds a Runtime. And a Session can be configured with "router" mode and will behave as a zenohd router (except plugins and admin space parts)

Thus, this repo could implement in C or C++ a process that creates a zenoh-c Session in "router" mode.

Pros:

  • zenoh-c is already built for rmw_zenoh_cpp, thus no extra Rust build
  • you can adapt the router to your need (same as alternative 1)

Cons:

  • you have to maintain it and possibly cherry-pick changes made in zenohd's main.rs
  • no plugins nor admin space, since PluginManager and AdminSpace are not exposed in zenoh-c

@francocipollone
Copy link
Collaborator

Thanks @JEnoch for adding the context and purposing two alternatives here:

For the record, last week I just drafted a "temporarily" solution ( #63) while investigating this. (It will probably be reimplemented or discarded)

Alternative 1

I have some concerns about this alternative:

  • If we have our own main.rs we will want to compile it. We can embed a cargo build command into the CMake machinery to properly automatize this. However, this will require us to depend on the zenoh crate (we should have a .toml somewhere for its dependencies).
    • So we will be building zenoh again (we already were building it via zenoh-c-vendor --> zenoh-c )
    • Not sure how this will behave in the ros build farm (@clalancette might have more info here)
  • Taking into account what was just stated, it would be quite similar to Bring zenoh router via vendor. #63 regarding building zenoh again.

Please let me know if I am wrong: a new Rust/Cargo user here 😇

Alternative 2

Overall sounds like a good alternative in order to maintain all the solution under the same language without being mixing dependencies and types of packages.

  • no plugins nor admin space, since PluginManager and AdminSpace are not exposed in zenoh-c

I can't foresee for the moment if the use of plugins can be a thing for us in the future, probably not, however it will definitely limit the user to use solely the configuration we provide. It might be ok.

  • PluginManager and AdminSpace are not exposed in zenoh-c

    Let's say we go with this option and we end up needing plugins in the future. Is adding C bindings for those entities something doable? Or is it "private" in the zenoh-rust implementation so it would demand a huge refactor?

@francocipollone
Copy link
Collaborator

francocipollone commented Nov 16, 2023

From F2F chat during weekly sync, we agreed on merging current #63 for now and then proceeding with Alternative 2.
For completitude: PluginManager and AdminSpace features should be able to be brought to the zenoh-c in the future if needed.

@francocipollone
Copy link
Collaborator

I just drafted a proof of concept here: https://github.com/ros2/rmw_zenoh/tree/francocipollone/router_via_zenoh_session

Context

Basically I am testing alternative 2: Creating zenoh session as usual + router configuration.

Results

Results are ok. It is enough for our case at least.
I think it is safe to proceed this way. In the future, if we need to support plugins we will need those bindings into the zenoh-c API. For the moment should be enough.

Next steps

If we are ok with proceeding this way:

  • Polish code a bit:
    • This method (here) can be extended to provide a way to select whether you want to get the router config file or the "peer" config file.
    • We could also provide an env variable for passing a custom zenoh router config file (code-wise we will get that for free after extending the method.)

CC @clalancette @Yadunund . Let me know your thoughts.

@francocipollone
Copy link
Collaborator

As commented during weekly meetings this effort is put on hold until implementation of this rmw is in a more mature state, so we make sure all the requirements for the zenoh router are granted (plugins, etc)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants