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

simplification(?) of 'stm32l4xx' feature detection #277

Open
Crzyrndm opened this issue Dec 18, 2021 · 1 comment · May be fixed by #281
Open

simplification(?) of 'stm32l4xx' feature detection #277

Crzyrndm opened this issue Dec 18, 2021 · 1 comment · May be fixed by #281

Comments

@Crzyrndm
Copy link
Contributor

Crzyrndm commented Dec 18, 2021

As it currently stands, detection of device specific features is rather unwieldy (prone to repetition, potential errors, verbosity, ...).
NOTE: for simplicity, I will use detection of ADC2 peripheral in any examples.

There are few specific issues currently

  1. Lack of abstraction: Lack of clarity because the condition is a list of devices, not a named condition (e.g. has ADC2 peripheral)
    a. A feature gate is often needed in multiple locations (e.g. struct definition + function impl), requiring repetition of the (not completely clear) condition in many places
  2. It is not unusual to need an if ... else ... style condition, and #[cfg(...)] provides nothing to support this. More repetition (note that clarity is much worse when an else if is needed because the condition will be slightly modified)

Potential solutions

Derived features

Looking through other STM32 HAL implmentations, the STM32F4 HAL crate makes heavy use of derived features. However using primary/secondary features (both a part of the crate API) for metaprogramming seems like a poor choice if it can be avoided. cargo.toml is not really intended to house complexity

const conditions

const had fairly limited internal benefit as it doesn't allow for code that won't compile and/or removing struct/function declarations.

Macros

Expansion inside a #[cfg(any(...))] conditions doesn't appear to be supported. What I could get to work was a macro that encompassed the entire expression, condition and all.

Macros can be used anywhere (depending on how the macro is written), and don't create API issues.
A simple level of abstraction can be created by wrapping the conditional expression in a macro e.g.

macro_rules! has_ADC2 {
  ($ex:expr) => {
    #[ cfg(any( feature = "stm32l412", feature = "stm32l22", ... )) ] $ex;
  }
}

has_ADC2!{ <code for ADC2> }

Other crates

  • The cfg-if crate allows for the equivalent to C #if ... #else ..., is small, maintained by Rust libs team and is widely used judging by the crates.io download counters. This would solve the second issue listed above.
    • Note that from my quick testing it didn't work with my has_ADC2 macro, so potentially will be better off with something internal. I'm probably doing something dumb here since I'm writing macros for the first time today to try this out
  • ...?

Proposed action

  1. Create the pub(crate) module (feature / feature_detection / device_features / ...) with the aim of providing macros which other modules will use (mostly) instead of feature = "..." directly. Initial set to focus on peripheral presense (e.g. has_ADC2!), trialling a few different forms for bikeshedding

    • pair of macros, e.g. feature::has_ADC2!(...) / feature::no_ADC2!(...)
    • single macro with condition, e.g. feature::if_ADC2!(present, ...) / feature::if_ADC2!(absent, ...)
  2. Investigate how #if ... #else ... can be supported in a somewhat composable manner

@Crzyrndm
Copy link
Contributor Author

Crzyrndm commented Dec 24, 2021

I think I've found the "correct" way fo doing this while investigating build scripts a bit more. https://doc.rust-lang.org/cargo/reference/build-scripts.html#rustc-cfg

In its simplest form

// in build.rs
// feature presence can be checked through CARGO_FEATURE_<name> (<name> is uppercased)
if let Ok(_) = env::var("CARGO_FEATURE_STM32L476") {
    println!("cargo:rustc-cfg=L476");
    // or
    println!(r#"cargo:rustc-cfg=gen_feature=L476"#);
}

// in lib.rs
#[cfg(all(L476, feature = "rt"))]
// or
#[cfg(all(gen_feature = "L476", feature = "rt"))]

With a few additional helpers, this can easily generate any combination of internal "features" and as can be seen with ^^ example, composes just fine with standard features.

Stylistically, you can use any fey (gen_feature ^^) or no key at all.

  • I prefer not to use the "feature" key to make it clear this isn't from cargo.toml features
  • I think leaving unkeyed vcalues for the language (e.g. #[cfg(test)]) is sensible
  • I don't have an obvious good alternative key name (derived, generated, custom, build, option, ...?)
    This will be easy to resolve right before merge, so only a minor issue for now

@Crzyrndm Crzyrndm linked a pull request Dec 25, 2021 that will close this issue
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.

1 participant