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

Mavlink v2 extension are broken for v0.13 for the mavlink crate #246

Closed
pv42 opened this issue Jul 30, 2024 · 3 comments
Closed

Mavlink v2 extension are broken for v0.13 for the mavlink crate #246

pv42 opened this issue Jul 30, 2024 · 3 comments

Comments

@pv42
Copy link
Contributor

pv42 commented Jul 30, 2024

The feature emit-extensions that is supposed to enable mavlink v2 extension does not currently work for versions 0.13.x.
This can be seen when comaparing the documentation (which enables all features) of version 0.12.2 and 0.13.1 on docs.rs for COMMAND_ACK_DATA:
https://docs.rs/mavlink/0.13.1/mavlink/common/struct.COMMAND_ACK_DATA.html only has the command and result fields
while https://docs.rs/mavlink/0.12.2/mavlink/common/struct.COMMAND_ACK_DATA.html also contains the extension fields.

The reason for this is that the feature when enabled on the mavlink crate will not be set on the mavlink-bindgencrate which uses it when generating code.
This can be fixed by adding the feature dependency in mavlink/Cargo.toml:

@@ -88,7 +88,7 @@ serde_arrays = { version = "0.1.0", optional = true }
 
 "format-generated-code" = []
 "emit-description" = []
-"emit-extensions" = []
+"emit-extensions" = ["mavlink-bindgen/emit-extensions"]
 "std" = ["mavlink-core/std"]
 "udp" = ["mavlink-core/udp"]
 "tcp" = ["mavlink-core/tcp"]

and adding the feature itself to mavlink-bindgen in mavlink-bindgen/Cargo.toml:

@@ -27,3 +27,4 @@ anstyle-parse = { version = "=0.2.1", optional=true }
 
 [features]
 cli = ["dep:clap", "dep:clap_lex", "dep:clap_builder", "dep:anstyle", "dep:anstyle-query", "dep:anstyle-parse"]
+emit-extensions=[]

There is a further issue when doing that, since the parser will use serde(default = "crate::RustDefault::rust_default")) and RustDefault is a pub (crate) is a trait in mavlink-core but required in the mavlink crate when using emit-extensions.
The solution to this is making it fully public and importing it in mavlink/lib.rs. This does not create additional dependencies since mavlink already depends on mavlink-bindgen.

@@ -3,3 +3,6 @@
 include!(concat!(env!("OUT_DIR"), "/mod.rs"));
 
 pub use mavlink_core::*;
+
+#[cfg(feature = "emit-extensions")]
+#[allow(unused_imports)]
+pub (crate) use mavlink_core::utils::RustDefault;
@patrickelectric
Copy link
Member

Hi @pv42! Thanks for creating the issues, I'm with no time at the moment to fix it myself, but I'm able to review and merge code, be free to submit PRs.

@pv42
Copy link
Contributor Author

pv42 commented Jul 30, 2024

Thanks for the quick reply.
I just noticed that the emit-description feature seems to have the same issue, but since it only generates doc it does not break any builds.
I will probably submit a PR at least for this issue.

@pv42 pv42 mentioned this issue Aug 3, 2024
@pv42 pv42 mentioned this issue Aug 11, 2024
@amsmith-pro
Copy link
Contributor

amsmith-pro commented Aug 19, 2024

Just ran into this as well, thanks for submitting a PR @pv42

@pv42 pv42 closed this as completed Aug 23, 2024
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

No branches or pull requests

3 participants