Skip to content
This repository has been archived by the owner on Mar 4, 2019. It is now read-only.

API tailored for specific microcontrollers. #9

Closed
protomors opened this issue Jun 25, 2017 · 9 comments
Closed

API tailored for specific microcontrollers. #9

protomors opened this issue Jun 25, 2017 · 9 comments

Comments

@protomors
Copy link

I am opening the issue here but it applies to other device crates as well.
SVD file from which this library is generated describes the whole stm32f103 family. But not every peripheral available in all devices. For example basic timers are present only in high-density and XL-density line.
So to catch errors like usage of unavailable peripherals and to make library smaller I made separate SVD file for each type of microcontroller that I using. I even removed configuration bits for GPIO pins that are unavailable on given package. And it works surprisingly well. Now a program that tries to use unavailable peripherals or pins simply will not compile. It makes porting to different device much easier.
I would like to share my work but don't want to create clutter with crates like "stm32f103c8". Maybe better approach will be to somehow combine different variants of API into single crate with ability to select specific microcontroller? Is it possible to use features for that?

@japaric
Copy link
Owner

japaric commented Jun 28, 2017

Is it possible to use features for that?

It is possible to use Cargo features for that but I think it's not a good solution because those features are binary so in theory there's nothing stopping you from enabling several device features at once like +stm32f103c8 +stm32f103vg which doesn't make much sense.

I think a better solution would be include cfg attributes like #[cfg(device = "stm32f103c8")] in the svd2rust generated code. Then you could use a Cargo configuration file to specify which device you are using:

$ cat .cargo/config
[build]
rustflags = [
    "--cfg", 'device="stm32f103vg"'
]

Of course there's nothing stopping you from passing several device= switches to rustc but it should be pretty obvious to spot the problem:

$ cat .cargo/config
[build]
rustflags = [
    "--cfg", 'device="stm32f103c8"'
    "--cfg", 'device="stm32f103vg"',
]

With Cargo features the problem is not so obvious because Cargo features are additive. If the stm32f103xx appears more than once in your dependency graph and each instance has different features (devices) enabled then Cargo will compile the stm32f103xx with the union of all the features. But the information about which feature is enabled is scattered across different crates so it's hard to track down the source of the problem.

Now the question is how could we modify svd2rust to include these #[cfg] attributes. I don't think SVDs provide a field for this kind of thing so maybe we can use an auxiliary file thas specifies what's available on what device and what's not.

Since you have been working with different devices in the stm32f103 family what differences have you found between those devices. Are all the differences just at the peripheral level (e.g. TIM6 is not available in low density devices), or are there also differences at the register level? In any case it probably makes sense to make the file substractive: as in it specifies which peripheral / register is not available for device X. For example it could look like this:

# stm32f103xx.toml
[device.stm32f103c8]
# these are not available for this device
peripherals = ["TIM6", "TIM7"]

Then the generated code would look like this:

#[cfg(not(device = "stm32f103c8")]
pub const TIM6: Peripheral<tim6::RegisterBlock> = ..;
..

Thoughts?

@protomors
Copy link
Author

It is possible to use Cargo features for that but I think it's not a good solution because those features are binary so in theory there's nothing stopping you from enabling several device features at once like
+stm32f103c8 +stm32f103vg which doesn't make much sense.

You are right. I already seen several crates facing the same problem. Cargo really should implement some enum-like functionality for crate settings.

# stm32f103xx.toml
[device.stm32f103c8]
# these are not available for this device
peripherals = ["TIM6", "TIM7"]

I like the idea to declare type of microcontroller as set of peripherals. It will make possible to include in API only peripherals that you are using (will it reduce a compile time?).
But I was thinking about much simpler approach - just including in the device crate separate files for each microcontroller and compiling only selected one. Of cource it will make device crates even larger but it should be simpler to implement. It needs separate SVD file for each type of microcontroller but does not take much time to write them (I simply delete sections from general file). Maybe automating this process in svd2rust can be the firs step? Like if you pass to svd2rust list of SVD files it will generate crate for all devices with ability to select one. Either way looks like to implement this the right way we will need changes to svd2rus.

Since you have been working with different devices in the stm32f103 family what differences have you found between those devices. Are all the differences just at the peripheral level (e.g. TIM6 is not available in low density devices), or are there also differences at the register level?

As far as I know differences are only in memory size and availability of some peripherals. At least I am unaware of any example where the same peripheral behaves differently. Registers are the same in whole family but it makes problem even more annoying. Because trying to use unavailable peripheral does not cause exceptions. Registers are there but writing to them does nothing. This is why such mistakes are hard to spot and I decided to modify SVD file.

So you could describe differences at peripheral level. But I did not only that but also removed all configuration bits related to peripheral. For example TIM6EN from register APB1ENR. And also I removed all configuration bits related to pins that are not present on given package. So for example when porting from 64 pin part to 48 pin looking at compiler errors it will be easy to tell which pins need to be remapped.

I still don't know if it was worth it. Implementing such level of granularity using configuration file and conditional compilation probably will be too cumbersome.

@japaric
Copy link
Owner

japaric commented Jun 28, 2017

Cargo really should implement some enum-like functionality for crate settings.

Maybe. IMO that may lead to proliferation of non composable crates as in crate A
depends on the crate f103 with feature "stm32f103c8" enabled, and crate B
depends on crate f103 with feature "stm32f103vg" enabled so now you can't use
crate A and B at the same time even if you were going to use an API from both
crates that was actually common to both the stm32f103c8 and stm32f103vg devices.
That may be a case where #[cfg(device = "..")] may do better than enum-like
Cargo features, but who knows.

I like the idea to declare type of microcontroller as set of peripherals.

Hmm. Idea: create one Cargo feature per peripheral and add it to the stm32f103
crate. If the feature is enabled the API for that peripheral will be included,
otherwise it won't. The additive nature of Cargo features should work OK with
this model.

However that sounds like too many Cargo features to manage and to reason about.

It will make possible to include in API only peripherals that you are using
(will it reduce a compile time?).

It would reduce compile time of the stm32f103xx crate itself but that's a
marginal gain because the stm32f103xx crate most likely only needs to be
compiled once per development session. It's the crate that you are working on
what will get re-compiled way more often.

In any case this would require a bunch of #[cfg] attributes in the stm32f103xx
crate. To toggle those #[cfg] you would have to use either Cargo features or the
--cfg device=.. mechanism I proposed.

But I was thinking about much simpler approach - just including in the device
crate separate files for each microcontroller and compiling only selected one

At that point what would be difference from just having one crate per device?
With one crate per device you would save yourself the trouble of having to deal
with a bunch of Cargo features.

But I did not only that but also removed all configuration bits related to
peripheral. For example TIM6EN from register APB1ENR. And also I removed all
configuration bits related to pins that are not present on given package. So
for example when porting from 64 pin part to 48 pin looking at compiler errors
it will be easy to tell which pins need to be remapped.

Yeah, that sounds more involved and less automatable. IMO if you are using a HAL
/ higher level API instead of directly using registers then just removing a whole
peripheral should cause enough compiler errors to get your attention. Or IOW, I
don't think additional errors about missing configuration registers add too
much.

The pin remapping problem, though, is interesting. I hope we'll come up with
some mechanism that not only tells you that you are using a pin that doesn't
exist but also if you are using peripherals that have overlapping pins (that
would be an error). I don't have a solution for that though.

@protomors
Copy link
Author

Hmm. Idea: create one Cargo feature per peripheral and add it to the stm32f103
crate. If the feature is enabled the API for that peripheral will be included,
otherwise it won't. The additive nature of Cargo features should work OK with
this model.

This is exactly what I had in mind. And other crates should depend on only required features. For expample USART HAL could depend on USART, RCC, GPIO and optionally DMA peripherals. We still can have features like "stm32f103c8" for convenience. But they will be declared only as a set of peripherals.

At that point what would be difference from just having one crate per device?
With one crate per device you would save yourself the trouble of having to deal
with a bunch of Cargo features.

This was my original proposal. :)
Creating separate crate for each device is the simplest approach. And I even can help with writing SVD files for STM parts. I was just worried that we will end up with a LOT of similar crates and they will be harder to maintain. And as you described it will be a problem if other crates start to depend on specific device. It is ok for end application and board support crates but for HAL or helper libraries it wold be better to depend on specific peripherals.

Yeah, that sounds more involved and less automatable. IMO if you are using a HAL
/ higher level API instead of directly using registers then just removing a whole
peripheral should cause enough compiler errors to get your attention. Or IOW, I
don't think additional errors about missing configuration registers add too
much.

Yes. For now idea of separating at peripheral level sounds best to me. At least it can be automated and is relatively easy to understand. But if you decide to take this approach then this issue belongs to svd2rust.

The pin remapping problem, though, is interesting. I hope we'll come up with
some mechanism that not only tells you that you are using a pin that doesn't
exist but also if you are using peripherals that have overlapping pins (that
would be an error). I don't have a solution for that though.

I think that things like pin remapping or clock settings are to high level to be handled by device crate. SVD files just don't provide enough information. In the future it would be nice to have tool like CubeMX which generates project and initialization code for peripherals. And let this tool handle such conflicts.

@japaric
Copy link
Owner

japaric commented Jun 29, 2017

This is exactly what I had in mind. And other crates should depend on only
required features.

I don't know. Sounds complicated to manage. Too many features to reason about as
I said before. It would be great if something like this was possible:

// stm32f103xx
#[cfg(peripheral = "usart1")]
pub const USART1: Peripheral<..> = ..;

#[cfg(peripheral = "spi1")]
pub const SPI1: Peripheral<..> = ..;
// stm32f103xx-hal

extern crate embedded_hal as hal;
extern crate stm32f103xx;

struct Serial1<'a>(&'a stm32f103xx::USART1);
impl hal::Serial for Serial1<'a> { .. }

struct Spi1<'a>(&'a stm32f103xx::SPI1);
impl hal::Spi for Spi1<'a> { .. }
// app

extern crate stm32f103xx_hal;

fn main() {
    // only uses Serial1
}

Here when building app you declare that the target device has a USART1
peripheral but not a SPI1 peripheral. app should compile because it only uses
USART1 through Serial1. However, this won't compile because stm32f103xx-hal uses
SPI1 which doesn't exist because it was cfg-ed away from stm32f103xx.

Of course you can make this work if you add #[cfg] attributes to the
stm32f103xx-hall crate but that's exactly what I want to avoid. I want to avoid
sprinkling a bunch of #[cfg]s in every single crate between stm32f103xx and app
but I think rustc is not going to let me get away with that :-(.

This was my original proposal. :)

Yeah, I think we should not do it that way. Main reason being that this
duplicates a bunch of code. The worst part being that stm32f103c8::TIM2 and
stm32f103vg::TIM2 are different types so HAL traits would have to be
implemented for one crate and for the other; this effectively multiplies the
amount of work to do.

In general, we should try to reduce the number of types that mean the same thing
as much as possible. There has even been an idea floating around about creating
common crates to share peripheral definitions common across different device
families from the same vendor. cf. rust-embedded/svd2rust#96

I think that things like pin remapping or clock settings are to high level to
be handled by device crate.

I agree. I was wondering if that tool / mechanism could also solve the problem
of detecting uses of a peripheral that's not available for a device with minimal
changes in the device crates we have today.


Another idea to deal with this: we leave the stm32f103xx crate as it is,
exposing an API for every peripheral the biggest chip may have, and create
device specific crates like stm32f103c8 that simply re-export the part of
stm32f103xx's API that's available for that device. With code:

// stm32f103c8

extern crate stm32f103xx;

pub use stm32f103xx::{TIM1, TIM2, TIM3, TIM4};

// not available, don't re-export
// pub use stm32f103xx::{TIM6, TIM7};

// ..

With this approach we only have to implement the HAL traits for the general
family crate, e.g. stm32f103xx, and not for the device specific crates. When
writing an application you would have to use a device specific crate to avoid
using non-existent peripherals. This assumes that HAL implementations use
newtypes that must be constructed from references to peripherals like the
blue-pill crate does. (I know some people would like to get rid of any reference
to the actual peripherals from the HAL).

@protomors
Copy link
Author

The worst part being that stm32f103c8::TIM2 and
stm32f103vg::TIM2 are different types

I didn't even thought about it (still very new to rust). For me separate crates for each device worked so well simply because I used device crates directly in my application. Am I understanding right that TIM2, TIM3 and TIM4 are also different types, even though they are functionally identical? And not only in the same device, they are the same in F101-F107 families (and maybe even in some others). I will look into embedded-hal and blue-pill crates to understand how you are dealing with this problem.

Another idea to deal with this: we leave the stm32f103xx crate as it is,
exposing an API for every peripheral the biggest chip may have, and create
device specific crates like stm32f103c8 that simply re-export the part of
stm32f103xx's API that's available for that device.

In this case crate stm32f10xxx could make more sense. As far as I know peripherals in all 100-107 families are compatible. Even if there are differences SMT tries their hardest to not cause conflicts. For example look at the description of RCC_APB2ENR register in stm32f100 and stm32f103 reference manuals. Configuration bits for the same peripherals are at the same positions. And if some peripherals are not present in given family at all - corresponding bits are marked as reserved (accessing them will not cause any errors and they will always read as 0).

Sadly, looks like I am not experienced enough in rust to choose the best solution. But will help with what I can.

@japaric
Copy link
Owner

japaric commented Jun 30, 2017

Am I understanding right that TIM2, TIM3 and TIM4 are also different types, even though they are functionally identical?

They are different types but they provide the exact same API because they deref to the same type: tim2::RegisterBlock. As Deref is a trait you can write a generic function that works for TIM2, TIM3 and TIM4:

fn foo<T>(tim: &T) where T: Deref<tim2::RegisterBlock> {
    let tim2: &tim2::RegisterBlock = tim.deref();
    tim2.cr1.write(..);
}

// all these work. TIM2 has type &TIM2, etc.
foo(&TIM2);
foo(&TIM3);
foo(&TIM4);

I will look into embedded-hal and blue-pill crates to understand how you are dealing with this problem.

You probably should read about the "newtype pattern" as well.

Sadly, looks like I am not experienced enough in rust to choose the best solution.

This is tricky stuff. I'll open an issue in svd2rust to get more opinions / ideas on this matter.

@protomors
Copy link
Author

Thank you for the example.

I'll open an issue in svd2rust to get more opinions / ideas on this matter.

It is good idea. More people will see the issue there. And looks like it belongs to svd2rust anyway.

@japaric
Copy link
Owner

japaric commented Jul 6, 2017

closing in favor of rust-embedded/svd2rust#122

@japaric japaric closed this as completed Jul 6, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants