Skip to content

[draft] Re-organizing cargo features to support more devices#716

Closed
TethysSvensson wants to merge 1 commit intoatsamd-rs:masterfrom
TethysSvensson:tethys/variant-cargo-feature
Closed

[draft] Re-organizing cargo features to support more devices#716
TethysSvensson wants to merge 1 commit intoatsamd-rs:masterfrom
TethysSvensson:tethys/variant-cargo-feature

Conversation

@TethysSvensson
Copy link
Copy Markdown
Contributor

Summary

This is a draft PR, to open a discussion about how different variants of the same peripheral are organized in the code.

Motivation

Currently a most peripherals with more than one variant decides which variant to use based on the thumbv6 or thumbv7 features.

However, this approach does not work for devices such as the l22, which is a thumbv6 device has peripherals that behave both like the thumbv6 and the thumbv7 variants. A few peripherals are even completely different from either of the two.

Implementation

This draft PR proposes solving this problem by introducing more cargo flags for variant-implementations. In this PR I have converted adc.rs and serial_number.rs from the thumbv6/thumbv7 style to this alternative style.

If you like this style of organizing the code, I am up for converting the entire thumbv6/thumbv7 hierarchy to this format.

Alternatives

This use of cargo features is not really the intended one. It is somewhat cumbersome to make sure that the feature lists are both correct and complete. Currently the features are mostly used to decide which peripherals are present, and not as much which variant -- and even so the feature list is already unusually long.

One alternative solution would be to implement a proc-macro, which kept a list of all peripherals supported by each device, including which variant.

This proc-macro could be invoked as #[hal_feature(serial-numbers)], #[hal_feature(serial-numbers-variant1)] or #[hal_feature(serial-numbers-variant1a)].

This proc-macro would then expand to #[cfg(any(feature = "device1", feature = "device2", ..))].

@TethysSvensson TethysSvensson force-pushed the tethys/variant-cargo-feature branch from 456fa93 to edfdaa8 Compare January 14, 2024 20:37
@jbeaurivage
Copy link
Copy Markdown
Contributor

@TethysSvensson, @bradleyharden, you can find a very minimal proof of concept of what a custom proc-macro would look like here: https://github.com/jbeaurivage/atsamd/tree/proc-macro. I'm still in the process of discovering what we would and wouldn't be capable of doing.
One major limitation I found is that attribute macros can be used in much fewer places compared to a regular #[cfg].

@jbeaurivage
Copy link
Copy Markdown
Contributor

Superseded by #728

@jbeaurivage jbeaurivage closed this Mar 8, 2024
@TethysSvensson TethysSvensson deleted the tethys/variant-cargo-feature branch March 9, 2024 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants