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.

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

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

Open
ivmarkov opened this issue Jul 8, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@ivmarkov
Copy link
Collaborator

ivmarkov commented Jul 8, 2024

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

  • Annotate these with #[non_exhaustive]; this way, the user code would be forced to destructure such structs with .. and to have a _ => "match all" on such enums, which - in turn - would allow us to add new members to these types without this being a breaking change for the user
  • For structs specifically - implement a public constructor function (as #[non_exhaustive] structs cannot be constructed directly by the user) and optionally - a builder pattern (to be decided)

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.

@github-project-automation github-project-automation bot moved this to Todo in esp-rs Jul 8, 2024
@Vollbrecht Vollbrecht added the enhancement New feature or request label Jul 8, 2024
@Vollbrecht
Copy link
Collaborator

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

@ivmarkov
Copy link
Collaborator Author

ivmarkov commented Jul 8, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Todo
Development

No branches or pull requests

2 participants