-
Notifications
You must be signed in to change notification settings - Fork 8
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
Expose bindings #31
base: develop
Are you sure you want to change the base?
Expose bindings #31
Conversation
84a5d63
to
d90d529
Compare
oh, worth mentioning i dropped the |
069a9f4
to
709af28
Compare
709af28
to
3593efc
Compare
Can you show me an example what the raw/direct API looks like and what the nicer/generated one would be like? That would help me understand what you mean. |
absolutely. easiest way is navigate to the the api exposed by the main crate is unchanged from the existing implementation. ( |
the original crate has two layers
in this PR i've split those two layers into separate crates. So i guess the question now is
there are other reasons for using a wrapping crate though
obviously the price you pay is greater maintenance burden |
i was looking at publishing this. looks like people are camping on every possible permutation of "mavlink" and "sdk" on crates.io already... |
mavsdk_rust taken? |
Haven't looked very closely, but right now Just wondering because for me, ideally we would do the same as in the other language bindings (mavsdk-java, mavsdk-python, mavsdk-swift), i.e. generate most of the code from the templates. And there gRPC is an implementation detail. Actually, for this repo (mavsdk-rust), I would even think it may be worth looking into bypassing gRPC entirely and generating code that calls C++ directly. Does that make sense? |
yes,
that definitely makes sense. in that case you would use an FFI rather than gRPC, right? with a library such as cxx. You'd use templates to generate the glue code, and the Rust types. Not something I have any experience with, but certainly you'd have some big savings in binary size and likely performance by doing that. |
i'm going to mark this as a draft, as I don't necessarily expect it should be merged. It's been useful as a discussion! |
I mean on my end it can be merged like this 👍. @julianoes what do you think? I just wanted to mention the templates discussion. I don't know Rust, so I cannot write the code, but I'm happy to help with the templates. Question would be whether we want to continue with gRPC or use FFI and call C++ directly (I would vote for the latter), and then you would need to write a few representative functions before starting looking at the templates, I would say. |
3593efc
to
b6bde80
Compare
this is a sketch of a different repository layout, which may be a partial solution to the discussion in #23 (comment)
there's no additional overhead (in either complexity or compile time) to split the repo into two crates.
The first crate
mavsdk-bindings
directly exposes the generated gRPC bindings. This satisfies the requirement that generated code is always perfectly aligned with the proto definitions, without any additional effort to maintain.the second crate is a higher-level wrapper which can
there's not additional effort to publish both, so consumers of the API would be free to choose their guarantees.
or maybe the raw bindings turn out to be good enough that the wrapper doesn't add any value...
if you want to be really clever, you could also stick the separate plugins behind
cfg
flags, so that you only need to pay for what you need (although you'll still get the fullprost
package, which i assume is the bulk of the binary size)