stm32l4xx-hal
stm32l4xx-hal copied to clipboard
simplification(?) of 'stm32l4xx' feature detection
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
- 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
- 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 anelse ifis 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_ADC2macro, 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
- Note that from my quick testing it didn't work with my
- ...?
Proposed action
-
Create the pub(crate) module (
feature/feature_detection/device_features/ ...) with the aim of providing macros which other modules will use (mostly) instead offeature = "..."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, ...)
- pair of macros, e.g.
-
Investigate how
#if ... #else ...can be supported in a somewhat composable manner
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