mavlink / rust-mavlink

MAVLink library for Rust.
https://mavlink.github.io/rust-mavlink/mavlink/
Apache License 2.0
151 stars 78 forks source link

Better feedback on feature combinations #263

Open danieleades opened 1 month ago

danieleades commented 1 month ago

Would be great to have some documentation on valid combinations of features. This repo makes heavy use of features and some of these are mutually exclusive or have other relationships between them.

These should be documented and invalid combinations should have clear panic messages.

Would also be good to review that all of the valid combinations are tested in CI


i note that Cargo.toml contains these comments:

# NOTE: Only one of 'embedded' and 'embedded-hal-02' features can be enabled.
# Use "embedded' feature to enable embedded-hal=1.0 (embedded-io and embedded-io-async is part of embedded-hal).
# Use 'embedded-hal-0.2' feature to enable deprecated embedded-hal=0.2.3 (some hals is not supports embedded-hal=1.0 yet).

additionally, the following feature combinations fail to compile (not exhaustive):

the following combinations do compile (not exhaustive):

danieleades commented 1 month ago

Should also consider setting a custom default rust-analyzer command that doesn't emit every single target

pv42 commented 1 month ago

To also improve the documentation using #![feature(doc_auto_cfg)] to add those "Available on crate feature featurename only." badges would be a good idea. Apparently this is a nightly feature but docs.rs is on nightly so #![cfg_attr(docsrs, feature(doc_auto_cfg))] would work.

danieleades commented 1 month ago

@patrickelectric can you provide any context on what combinations of features are allowed?

patrickelectric commented 1 month ago

This was probably broken on the pass PRs, some work need to be done to fix that besides adding tests in CI.

danieleades commented 1 month ago

This was probably broken on the pass PRs, some work need to be done to fix that besides adding tests in CI.

I guess i'm trying to understand what is expected to work

patrickelectric commented 1 month ago

This was probably broken on the pass PRs, some work need to be done to fix that besides adding tests in CI.

I guess i'm trying to understand what is expected to work

At the moment, check the combinations in CI. For conflict configuration is still necessary to document.