atsamd-rs / atsamd

Target atsamd microcontrollers using Rust
https://matrix.to/#/#atsamd-rs:matrix.org
Apache License 2.0
559 stars 200 forks source link

Mutually incompatible BSP features lead to testing difficulties #704

Open ianrrees opened 11 months ago

ianrrees commented 11 months ago

Our Feather M0 BSP has features for some specific variants of the board, for instance the rfm feature for "RadioFruits", adalogger if the SD card shield is added, express for Feather M0 Express. These variants use the same ATSAMD21 microcontroller, but have different wiring.

Currently, we gate some of the provided BSP pins like this:

#[cfg(all(feature = "adalogger", not(feature = "rfm"), not(feature = "express")))]
PA21 {
    /// SD card detect
    name: sd_cd
    aliases: {
        PullUpInput: SdCd
    }
},

In particular when updating the HAL, it can be helpful to build all the examples for BSPs by doing cargo build --all-features --examples, however this fails when one of the examples uses the SdCd defined above. For most of our BSPs, this same approach is taken by CI, but currently we don't have an automated check that covers all the Feather M0 BSP examples.

Possible approaches to address this:

  1. Separate the Feather M0 BSP in to separate BSPs for the variants
  2. Extend the CI approach to build the mutually-incompatible variants separately
  3. Instead of feature-gating pins, feature-gate aliases

Bonus points: extend CI to check that each example gets built

ianrrees commented 2 months ago

Looking at this with a fresh set of eyes, I'm leaning toward approach 1 above. Cargo features just don't seem like a good way to express hardware variations, since they are purely additive. We've used the features like this to make it easier to support a wide variety of hardware, but even with this usage the current model is quite labour intensive.

So, rather than bending the CI process to support this use of features, I think that we'd be better off putting effort in to some other solution to make BSP maintenance easier.