-
Notifications
You must be signed in to change notification settings - Fork 79
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
Consider replacing mavlink-bindgen #287
Comments
I was concerned that the more complex codegenerator might lead to a lot longer compile times so I ran some (crude) benchmarks to see how the new parser performs.
*The last test on the newer generator has the features format-generated-code, emit-extensions and emit-description disabled (since this functionality does not need features). It seems that the new generator performs generally better then the old one but seems to struggle when more dialects are enabled. If someone want to run this themselves https://github.com/pv42/rust-mavlink/tree/new_bindgen (this is not pretty). |
Hi @G1gg1L3s, nice work! Is it possible for you to open a PR ? Serde is kind of a requirement for us to merge since some critical applications need it. |
Hey, @patrickelectric, sure! If you are okay with many breaking changes and a potentially huge PR, I can create it rightaway. As for the serde - I think it can be merged right after the PR. I already added some basic support, which you can find in a separate branch. However, it also contains some breaking changes. For example, enums cannot have |
Can you create an example folder for us to take a look in the API and test with some real vehicles ? Just to be sure that everything is working |
Sorry, could you elaborate? You mean compile some definitions and push them to the repo for you to see the generated code and try it out? |
Any updates on this? Would be interesting to get this moving forward to be able to use the newest mavlink specification |
Hi! First, thanks for your work here!
For the past two month, I've working on rewriting rust mavlink code generator because I found some bugs in the current version of mavlink-bindgen. I definitely had the same issues as reported in #285. I also had some messages being generated empty and problems with bitflags.
I tried to write a generator that is compatible with current rust-mavlink code and generates more or less the same principle and code structure as used in mavlink-bindgen because I hoped that it may be merged here someday.
You can find the code here - https://github.com/G1gg1L3s/mavgen
It has a number of features, for example:
The architecture is build more explicitly, similar to how compilers work: we have frontend (xml parser), intermediate representation (which gets checked for consistency, like "this messages refers to a valid enum" or "this enum is redefined", ec) and backend (or code generator). This allows to test the generator easily, and hopefully it will open options for more code generators (like maybe for different languages) or more gradual configuration.
The project has many tests for a number of cases: from xml parsing, to cycles in file imports, to tests for generator methods.
The project even includes integration tests with pymavlink to verify that both version can indeed exchange messages without corruptions.
The generator handles some exotic configurations, like arrays of enums, which we have in some recent mavlink definitions. I don't remember if rust-mavlink handles them, but I suspect it doesn't.
Many things are verified, like if enum fits into a message field, if two messages have the same name, etc.
It also has differences and some incompatibilities if compared with current mavlink-bindgen. For instance:
Generated name items try to follow rust conventions, so structs and enums are converted to PascalCase, where fields and modules are snake_case.
Generator targets
bitfield >= 2.0
, because older version was producing errors for me, even with rust-mavlink generator.For now, serde support is omitted, but it should be easy to add. Serde is included, but it generates structure a bit differently than the current version (due to name conflicts).The code always includes docs and is always formatted with
prettyplease
. Formatting actually takes a big chunk of generator's execution time.Enum sizes are generated as minimal as possible (instead of always using
#[repr(u32)]
).#[deprecated]
attributes are generated for the corresponding items.Codegen derives
Eq
on the message if it can.Field names that are reserved rust words (like
type
) are automatically escaped with raw idents (r#type
).It's divided into 3 crates: mavgen, mavgen-test and mavgen-cli to avoid bringing unneeded dependencies for CLI for example. The malink-rust uses features to hide cli, but personally I always forget to turn them on when compiling the binary.
And probably more small things I don't recall.
As the goal of the project was to make the codegen work with the current mavlink definitions, I think I achieved it. There are still some polishing and testing to do, for example, testing with V1 messages, adding proper handling of extension fields and serde support. But overall, I'm happy with the result.
So, that's why I'm suggesting merging this tool here. I believe it will be most useful under the mavlink organization for other people to improve and contribute. I understand that it's a big patch of work, so that's why I create this issue so we can discuss if you want to proceed, and if yes, how to plan this path.
Thank you!
The text was updated successfully, but these errors were encountered: