An idea of using `groupName` as cfg feature

[duskmoon314]

Motivation

Recently I am working on two MCU sharing most of the peripherals. So I searched for issues to find if there were any excellent solutions and found #393. It seems hard to parse multiple SVD files and merge them into one crate automatically. Thus I came up with the idea of using groupName as the cfg feature to partly solve this problem. This idea has concerns and may not be a very good idea. But #393 is still open, so I want to share and discuss this idea.

Design

In the spec of SVD, every peripheral can have a child element named groupName:

child elements	description	type	occurrence
groupName	Define a name under which the System Viewer is showing this peripheral.	xs:Name	0..1

The idea is to add a new argument named --group_name_as_feature or something shorter and meaningful and it is off-by-default. When the argument is given, then generate a #[cfg(feature = "<group_name>"] before the definition of peripheral if it has this child element.

That is if we have an SVD file with the following definition of a uart:

<peripheral>
  <name>UART0</name>
  <description>Universal Asynchronous Receivers/Transmitters</description>
  <groupName>Interface</groupName>
  <baseAddress>0x96000000</baseAddress>
  ...

Then the generated code is:

#[doc = "Universal Asynchronous Receivers/Transmitters"]
#[cfg(feature = "interface")]
pub struct UART0 {
    _marker: PhantomData<*const ()>,
}
#[cfg(feature = "interface")]
unsafe impl Send for UART0 {}
...

Also, use toml_edit to add cfg to Cargo.toml:

  [features]
  rt = ["riscv-rt"]
+ interface = []
+ group_1 = []
+ group_2 = []
+ default = ["interface", "group_1", "group_2"]

And the maintainer of this crate should add something like the below to the Cargo.toml:

[features]
rt = ["riscv-rt"]
interface = []
default = ["interface"]
+ mcu1 = ["interface", "group_1"]
+ mcu2 = ["interface", "group_2"]

Possible Usecase

Suppose Alice sells two chips with most of the peripherals are same except one has HDMI and other video peripherals supported.

Bob wants to make an SVD file to describe both of them at the same time. With this feature, Bob can give HDMI a groupName such as video and get the following features in Cargo.toml:

[features]
video = []
mcu = []
mcu-with-video =["video"]

Maybe this feature can be extended to support multiple arch in the same crate. But for now, I haven't got a good idea of how to implement it elegantly.

Concern

Just like discussions in #433 and #254, adding cfg may cause usability and composability problems. Maintainers should spend more time maintaining Cargo.toml if they enable this feature. And the only benefit I see for now is to generate a crate that supports multiple MCU after someone merge multiple SVD file by hand.

I wonder if there are any other benefits. If the benefits exist, is it worth adopting this design?

Overall, this idea is very similar to #254 except for using groupName as the cfg feature. Thus the advantages and disadvantages are alike.

Note: The implementation may be highly similar to #254, not hard but need more discussion. A try is at duskmoon314/svd2rust group_name_as_feature

[burrbull]

Yeah, you need generate some Cargo.features.toml which can them be merged into Cargo.toml

[duskmoon314]

Yeah, you need generate some Cargo.features.toml which can them be merged into Cargo.toml

I think that using toml_edit to add the cfg feature automatically may be easier to use. Maybe generate something like:

[features]
# other features, e.g. rt = ["riscv/rt"]
group_1 = []
group_2 = []
default = ["group_1", "group_2"]

Advantage: There is less work to do for the maintainers of the crate.

Disadvantage: May overwrite some feature names if accidentally use the same name with the SVD file.

[burrbull]

Have you seen this old PR and discussion? #254

[duskmoon314]

Have you seen this old PR and discussion? #254

Oh, my bad. Didn't read the section about Cargo.toml generation carefully before.

Since an SVD file may contain dozens of groupName, it is reasonable to generate a Cargo.toml like the implementation in #254. However, in doing so, svd2rust users may find it annoying if they frequently update an SVD file and overwrite the Cargo.toml again and again.

Besides, #254 disable all peripherals by default, which I think is not compatible with the current behavior.

[therealprof]

I just had a nice private discussion about this.

One detail that didn't quite stand out was that the proposal is to have an off-by-default svd2rust flag to control the generation of this slightly different output to cater for the mentioned usecases. Due to that I think a lot of the discussion in #254 doesn't apply to this new proposal.

There're probably some kinks which need ironing out but I don't have any objections to this approach per se.

[therealprof]

@duskmoon314 We discussed this in todays meeting and people are quite excited about the idea and convinced that it might work nicely as proposed. Any chance you have time to follow up with a PR to implement this idea?

[duskmoon314]

Sure!

I've skimmed the discussion briefly. I have to say that I have not yet thought carefully about how to solve the problem of multiple peripherals with the same name. 😢

Something needs to think more about:

• what to do when people want peripherals with the same name but different groupName
• how to support multiple targets (e.g. Cortex-A and RISC-V in one file)