Hot take: the way avr-hal uses features is frustrating

[innermatrix]

As I am sure you know, the recommended way to use features is to make them additive, and to avoid mutually exclusive features. However, avr-hal relies heavily on non-additive mutually exclusive features. I find this to be a pain.

This setup works great as long as you are building a crate that generates one executable for one board. It doesn't work great if you are

• writing a library on top of avr-hal, because it forces the library into the same disrecommended mutually-exclusive feature behavior.
• writing a crate that outputs multiple executables for different MCUs

The core issue — as I see it — is not that avr-hal is using features, it's the way it's using them. Specifically, avr-hal has some symbols whose implementation is selected mutually-exclusively using a feature — for example, Pins in atmega-hal. The fact that atmega-hal supports multiple MCUs, but can only ever export one Pins symbol, is at the root of the conflict.

IMO a better way to manage this would be to use features as they are intended: a feature flag selects whether a piece of functionality is included in the library, but allows multiple features to be selected. For example, I should be able to select both atmega32u4 and atmega328 features — and if I do, instead of them fighting over who gets to control the global Pins symbol, I should get both atmega32u4::Pins and atmega328::Pins symbols.

Taking this all the way up to the board level, what this would mean is that I would be able to use arduino-hal.features = ["atmega32u4", "atmega328"] when importing arduino-hal— which would indicate that I want arduino-hal to include support for both of those MCUs (using features additively, as they were intended) — and then in my main.rs I would write

#[cfg(any(feature = "atmega328"))]
use arduino_hal::atmega328 as board;

#[cfg(any(feature = "atmega32u4"))]
use arduino_hal::atmega32u4 as board;

I think this would completely eliminate all the shenanigans with mutually-exclusive features, and simplify the experience for anyone trying to use avr-hal in a non-trivial way.

Thoughts?

[stappersg]

On Sun, Nov 17, 2024 at 02:02:09PM -0800, Iris Artin wrote: Thoughts?

Patches welcome

[innermatrix]

@stappersg I have started this work in #603

That PR only changes the feature-specific pins! macros in attiny-hal and atmega-hal. I have follow-on PRs that I already started working on for refactoring other aspects — you can see the current PR stack at innermatrix#1 (comment).

My current overall plan is:

• Maintain the current behavior (with conflicting MCU-specific or board-specific globals) behind a feature flag (named deprecated-globals)
• Enhance each crate so that if deprecated-globals is off the crate can be built to support any number of MCUs/boards

After I am done with that we can figure out what the plan is for if/when to fully remove the deprecated-globals functionality; that would be a breaking change, and I would leave decisions about that up to the maintainers. Up until that point I plan to maintain backwards compatibility.

I am structuring this as a PR stack, of which the PR above is the first. The full PR stack is linked in #603 if you want to peek ahead to where I am going with this and offer early feedback, but you can also ignore it for now if you prefer 🙂 If there is some other structure you'd prefer for this work, let me know.

[stappersg]

On Wed, Nov 20, 2024 at 05:53:21PM -0800, Iris Artin wrote: @stappersg I have started this work in #603

Yep. Due "Notify all" I saw it already. Please don't wait for me and above all: Explore what you want to explore. Groeten Geert Stappers -- Silence is hard to parse

[innermatrix]

Alrighty, I have a coherent plan and will soon be sending along some PRs that put this into place.

• v0.1.x: current version
  • Changes: tweaks to macro_rules! pins to line up some future work. Make attiny_hal::pins! and atmega_hal::pins! feature-agnostic #603
  • Impact on current users of the crate: none
• v0.2.x: in-progress, PRs coming in the next few hours / days
  • Changes
    • Conflicting globals (such as crate::Pins) made optional via the deprecated-globals feature in attiny-hal (Refactored attiny_hal for additive features #605), atmega-hal (Refactored atmega_hal for additive features #606), and arduino-hal (forthcoming).
    • The deprecated-globals feature is on by default.
    • It becomes possible to build attiny-hal, atmega-hal, and arduino-hal to support multiple MCUs/boards
  • Impact on current users of the crate
    • Compiler warning letting them know that they are using deprecated-globals, and that it will be removed in the future.
    • Recommended action: replace attiny_hal::Pins with use attiny_hal::attiny85 as mcu; and mcu::Pins and similar.
    • Alternately, users can add deprecated-globals to dependency features, and continue with no code changes
• v0.3.x: forthcoming
  • Changes: deprecated-globals turned off by default.
  • Impact on current users of the crate
    • Compiler error if they are using deprecated globals and haven't added deprecated-globals to dependency features
    • Recommended action: same as for v0.2
    • They can still add deprecated-globals to dependency features, and continue with no code changes
• v0.4.x: unspecified future
  • Changes: deprecated-globals removed
  • Impact on current users of the crate
    • Compiler error if they are using deprecated globals
    • Required action: same as for v0.2