esp-rs / esp-idf-hal

embedded-hal implementation for Rust on ESP32 and ESP-IDF
https://docs.esp-rs.org/esp-idf-hal/
Apache License 2.0
433 stars 170 forks source link

Annotate configuration structs & enums with #[non_exhaustive]; add Enum::Other catch-all to configuration enums #451

Open ivmarkov opened 1 month ago

ivmarkov commented 1 month ago

ESP IDF has a source backwards compatibility policy that does allow adding new members to C structs and enums, where this is not considered a backwards incompatible change.

To model this in the esp-idf-hal and esp-idf-svc crates we need to do roughly the following:

For Rust struct/enum wrappers of "raw" esp-idf-sys types

For raw C enums from esp-idf-sys's bindings

When converting these to the actual Rust enums, have a "catch-all" non-panicking _ => leg in the match statement to convert any potentially new yet unmapped values to a generic Rust enum Other or Unknown value (which needs to be introduced if it is not already there). Ideally, this value should carry the raw esp-idf-sys constant.

==========

The above would be a one-time breaking change but it would allow us to model member addition to configuration types as a non-breaking change in future.

Vollbrecht commented 1 month ago

Do you want to tackle that now or after we try to experiment with the monorepo?

ivmarkov commented 1 month ago

Haven't thought about that. Monorepo + workspace would certainly be a tad easier to tackle this - like any cross-crate changes.