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.

Sign up for GitHub

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

Jump to bottom

generated internal conditional compilation gates #281

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

generated internal conditional compilation gates #281

wants to merge 34 commits into 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 •
edited
Loading

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
move feature check into build.rs for correctness
7fc9530 
previous check would not error if two were selected
@Crzyrndm
import the feature tables from macro branch
3bda397
@Crzyrndm
feature gate struct allows this to be a simple check->print loop
74afb53
@Crzyrndm
add L485 which shows up in the ST HAL
3f67d2a
@Crzyrndm
update peripheral feature tables based on L4 cube package v1.17.1
c4299b4
@Crzyrndm
update serial gates
f568cf6
@Crzyrndm
PAC support for UART4/5 is not great
802d688
@Crzyrndm
add RTC type3 gate
a2571d0
@Crzyrndm
add gates for device family (L4xx groups)
f8296c0
@Crzyrndm
update i2c gates
be416c3
@Crzyrndm
gpio f/g
1ae4871
@Crzyrndm
CAN pin gating
ebf99a8
@Crzyrndm
more GPIO F/G
0b96ba8
@Crzyrndm
unreachable instead of an (incorrect) register block
1d6abd5 
Reaching this arm would imply things have gone very wrong
@Crzyrndm
fix copy paste errors in device family gates
a39efd2
@Crzyrndm
add gates for pretty much every peripheral (including those that are …
40107da 
…always present)
@Crzyrndm
update lib.rs gating
919a532
@Crzyrndm
add a few missed peripherals (mostly L4+ exclusives)
153431e
@Crzyrndm
add gpioh and gpioi
284ad6d 
H has a couple of pins that are often reserved on many parts. Full port
on some larger footprints
I is only present on L49 and up in the larger footprints
@Crzyrndm
missed the L4q5 pka peripheral
17a0580
@Crzyrndm
update spi gates
8f5fd18
@Crzyrndm
timer gates
816d121
@Crzyrndm
fix l442 duplication
44e115f
@Crzyrndm
enable gates
a10f398
@Crzyrndm
fix spi gates
4260ae0
@Crzyrndm
update signature gates
9cac39e
@Crzyrndm
update rtc2 conditional
790ac09
@Crzyrndm
Copy link
Contributor Author

Crzyrndm commented Feb 9, 2022 •
edited
Loading

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
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

simplification(?) of 'stm32l4xx' feature detection
3 participants
@Crzyrndm @korken89 @DerFetzer