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

generated internal conditional compilation gates #281

Open
wants to merge 34 commits into
base: master
Choose a base branch
from

Conversation

Crzyrndm
Copy link
Contributor

closes #277

This uses a build script to generate internal conditional compilation gates. This allows keeping the cargo features clean, while internally being able to use readable checks like

#[cfg(any(
    condition = "family_L4x1",
    condition = "family_L4x2",
    condition = "family_L4x3",
))]

or

#[cfg(condition = "peripheral_usart3")]

The build script is dependency free and simple (just names and a boolean condition) so there is basically no impact on compile time and very little to understand. Currently generates two sets of "conditions"

condition = "peripheral_<name>"
condition = "family_<name>"

peripheral presense mapping is derived from the STM32 Cube repository. I chose to map all peripherals (even those present on every device) for maximum clarity. the peripheral name used is the lower case of the peripheral #define from that repository

suggestions on naming very welcome (I consider "condition" to be a placeholder)

@Crzyrndm Crzyrndm changed the title #277 feature derive build rs generated internal conditional compilation gates Dec 25, 2021
@korken89
Copy link
Collaborator

Very nice improvement!
Would you see this as ready for merge? Seems to work fine for me.

@Crzyrndm
Copy link
Contributor Author

Crzyrndm commented Dec 27, 2021

Feature wise, I believe so. The list of peripherals and which MCU they are present on is fairly complete (based on the C HAL) and I can't see any major downsides outside of this being a somewhat obscure capability of the build script (and the peripheral list being somewhat long ofc).

The style of the gating I am less sure on.

  • is 'condition' a good prefix (should it even have a prefix)? Some potential alternatives (I consider the current style a placeholder so opinions are very welcome)
    • cfg(peripheral_uart4) / cfg(peripheral_UART4)
    • cfg(peripheral = "uart4")
    • cfg(derived = "...") / cfg(generated = "...") / cfg(option = "...")
    • ???
  • should peripheral/family names be uppercase to match what is in the C-HAL / PAC?
  • is family_L41_42 a good naming scheme for those groupings?
  • should finer details (e.g. what type of RTC / USB peripheral is present) get their own grouping? This group is probably going to grow over time. To me it would make sense to at least have a separated list for it, not sure what prefix to use for 'RTC is type 3' or 'USB is device only'
  • This can be somewhat awkward to mix into the macros that declare a peripheral. One of the motivating factors for compiling a complete list of peripherals (other than removing any later uncertainty) was potentially being able to make the gating easier / more standardised. This would raise style questions if it were possible (e.g. using uppercase for peripherals)

@Crzyrndm
Copy link
Contributor Author

Crzyrndm commented Feb 9, 2022

Coming back to this after a few weeks away. Updated naming, cleaned up the build script with an eye toward future additions and generally just looking with fresh eyes

Each gate group now has its own unique name rather than all using condition = "prefix_..."

  • #[cfg(family = "L43_44")] // group by the second digit
  • #[cfg(family = "L4x2")] // group by the third digit
  • #[cfg(has_peripheral = "adc1")] // peripheral presence
  • #[cfg(has_peripheral_variant = "rtc_type3")] // peripheral subsets / variants

Naming is still a little shaky but it made no sense to standardise the left side of the gate and I have no idea what I was thinking with that idea...

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 this pull request may close these issues.

simplification(?) of 'stm32l4xx' feature detection
3 participants