small usability issue with esp_hal::init()

[liebman]

If you want to set the clock you can't do:

let peripherals = esp_hal::init(esp_hal::Config {
    cpu_clock: CpuClock::max(),
);

because it's marked with #[non_exhaustive] one must go this route:

let mut config = esp_hal::Config::default();
config.cpu_clock = CpuClock::max();
let peripherals = esp_hal::init(config);

[bugadani]

This is completely intentional. If we're going to introduce new options, we don't want to break existing code.

The style I prefer is:

let peripherals = esp_hal::init({
    let mut config = esp_hal::Config::default();
    config.cpu_clock = CpuClock::max();
    config
});

Or I think you should be able to do something like:

let peripherals = esp_hal::init(esp_hal::Config {
    cpu_clock: CpuClock::max(),
    ..Default::default()
});

[liebman]

or something like this maybe?

let peripherals = esp_hal::init(esp_hal::Config::default()
    .with_cpu_clock(CpuClock::max()));
});

The rust error told you to "look for a new() method" or something like that.

[liebman]

let peripherals = esp_hal::init(esp_hal::Config {
    cpu_clock: CpuClock::max(),
    ..Default::default()
});

Of the two you posed I prefer ^^

[bugadani]

Builders are fine I guess, but unless there's extreme interest for them, I'd love to avoid adding more code for a single use struct :(

[bjoernQ]

IMHO the ..Default::default() way is the most idiomatic and also convenient enough for now

[MabezDev]

As @bugadani mentioned, this is by design so we can add fields and not break code in the future. Imo ..Default::default() is nice enough for now, we can revisit this at a later date if the number of fields/options do get unwieldy.

[liebman]

That does not seem to work:

error[E0639]: cannot create non-exhaustive struct using struct expression
  --> board/src/board.rs:52:41
   |
52 |           let peripherals = esp_hal::init(esp_hal::Config {
   |  _________________________________________^
53 | |             cpu_clock: CpuClock::max(),
54 | |             ..Default::default()
55 | |         });
   | |_________^

For more information about this error, try `rustc --explain E0639`.
error: could not compile `board` (lib) due to 1 previous error

[MabezDev]

Ah, you're right. I'm hoping it's something that will be allowed in the future. It currently only works on structs defined within the crate, see: https://rust-lang.github.io/rfcs/2008-non-exhaustive.html#functional-record-updates.

In which case, @bugadani's first option works and it's not too much more code. I personally don't see the advantage of a builder pattern, but I'm open to hear other options.

[bugadani]

That's sad, but hopefully it's still not the end of the world. Again, we have to balance stability, lines of code (yes, really!) and useability and I think we're in a local maximum of the 3.