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

Consider merging (some) HAL crates #94

Open
Rahix opened this issue Oct 26, 2020 · 8 comments
Open

Consider merging (some) HAL crates #94

Rahix opened this issue Oct 26, 2020 · 8 comments
Labels
help wanted Extra attention is needed mcu-support Support for a new Microcontroller

Comments

@Rahix
Copy link
Owner

Rahix commented Oct 26, 2020

From my perspective as the maintainer of avr-hal, the HAL crates are becoming more and more of a burden to maintain because any global change needs to be reflected in each one. There is a huge amount of code-duplication across the HAL crates which is not helping with this.

I think it makes sense to look into merging at least some of the HAL crates, for chips where the differences are small enough that #[cfg()]s can be used to differentiate between them.

The big question is where to draw the lines, what MCUs should go into which HAL crate:

  • The nuclear solution would be to put all devices of ATmega/ATtiny into a crate. This would leave us with the minimum number of crates but a mess of #[cfg()]s to manage them.
  • On the other end of the spectrum, we could just go by the families as described by Microchip in their Overview (PDF). For the current set of supported MCUs this would not give us much advantage over the current situation, though.

IMO we need to larger grouping than the "Microchip Families" but just forcing all ATmega/ATtiny devices into a single crate also looks scary. The latter might still be manageable now, but looking into the future, with more and more devices being supported, I can see it becoming a large mess.

Not yet sure what to do ...

Proposals

"Nuclear" Split, atmega/atxmega/attiny

The nuclear solution would be to put all devices of ATmega/ATtiny into a crate. This would leave us with the minimum number of crates but a mess of #[cfg()]s to manage them.

Pros

  • Least amount of crates
  • (Probably) least amount of code duplication

Cons

  • Many #[cfg()]s to differentiate

Split by Architecture

Group devices by the instruction set they use.

Pros

  • Well defined separation.

Cons

  • There are similarities cross-architecture which means we'd still have a lot of duplicated code.

Split by Generation/Series

Original Comment:

The last digit marks the series of chips. We can group them like this (for example):

  • no digit
  • 8, 4, 5
  • 0, 1
  • U*

Pros

  • Devices in each series/generation are very similar so it is mostly about presence vs non-presence of a certain peripheral.
  • This also means not many special cases to consider.

Cons

  • Still ends up with a lot of crates (at least 4 for atmega, atxmega and attiny unclear)
  • Blurry lines where generations end/where to split. Only 0-series is a well defined difference.

Split by Family

On the other end of the spectrum, we could just go by the families as described by Microchip in their Overview (PDF). For the current set of supported MCUs this would not give us much advantage over the current situation, though.

Pros

  • Well defined separation.

Cons

  • Lots of crates, pretty much where we're at right now.
@Rahix Rahix added help wanted Extra attention is needed mcu-support Support for a new Microcontroller labels Oct 26, 2020
@Rahix Rahix added this to the crates.io Release milestone Oct 26, 2020
@jaxter184
Copy link

This wikipedia article groups the ATtinys into 4 groups (not including the newer generation, which is pretty clear about its grouping). I'm not sure what exactly the process is for that, but it seems to be mostly by architecture? I'm not sure how useful it would be to use that in this case, but if the goal is just to reduce the number of crates, it seems like a reasonable metric. Otherwise, I'm a fan of splitting by ATmega/ATtiny

@Rahix
Copy link
Owner Author

Rahix commented Dec 1, 2020

Hmm, I also found this Stackoverflow Answer which might give a good base:

"4" The last digit marks the series of chips. Within a series they are often pin-compatible and share a similar set of features. Some prominent series are:

  • 'no digit' - these are the first generation chips with 8 to 128 kiB Flash
  • '8' - a series from 4 to 32 kiB Flash, all in the same housing. More or less an improved version of the original chips
  • '4' similar to '8', but in larger packages with more pins (~40 instead of ~30) and up to 128 kiB Flash
  • '5' similar to '4', but with more timers and PWM channels
  • '0','1' rather old family with large packages (60 - 100 pins) and up to 256 kiB Flash.
  • '9' with integrated LCD controller
  • 'U2', 'U4' are the two sizes of USB-enabled controllers
  • '08', '09' newest family with additional configurable logic and more CPU-bypassing features
  • '50', '90' the largest chips with 100 pins, but few peripherals

I think we can group some of them even more. For a start, I'd (without looking deeper) group the ATmega devices like this:

  • no digit
  • 8, 4, 5
  • 0, 1
  • U*

for the others, we don't currently have any devices supported, so we can take care of them later. For the ATtiny devices, I think we can try to merge the info from your link with this kind of 'generation' numbering scheme, to derive a sensible partitioning.

What also needs to be evaluated is how to name those crates ... Not sure what would be intuitive.

@MarkuBu
Copy link

MarkuBu commented Dec 3, 2020

It is pretty difficult. I remember when I wrote a C library about 18 years ago it was difficult with the different ATmegas. Nowadays, with the new devices from Microchip, it becomes even worse, because their peripheral is totally different.

You can try to put pin compatible devices like ATmega8/48/88.. into one crate. Also all 40 pin, all 64 pin and 100 pin. But even there it becomes a mess sooner or later.

I assume that the original Atmel AVRs and the new Microchip AVRs need to be completely separated from each other. Even the IO ports are different. You need to write new drivers.

@Rahix
Copy link
Owner Author

Rahix commented Dec 3, 2020

I remember when I wrote a C library about 18 years ago it was difficult with the different ATmegas.

Luckily we have one very big advantage in Rust: We can create very rich abstractions without impacting the generated code at all (= zero cost). I think it should not be too difficult to build HAL drivers that hide all this mess from downstream users. We just have to carefully evaluate where to draw the lines.

Nowadays, with the new devices from Microchip, it becomes even worse, because their peripheral is totally different.

[...]

I assume that the original Atmel AVRs and the new Microchip AVRs need to be completely separated from each other. Even the IO ports are different. You need to write new drivers.

Yeah, I totally agree. I think those are what Microchip calls the 0-series devices (e.g. ATmegaXX08/09).

I want the basic driver functionality (user facing API) to be the exact same for old and new devices. For any additional features found in the newer ones, additional API can be provided alongside.

This was referenced Dec 10, 2020
@jaxter184
Copy link

Seems to me like all the relevant information has been collected, and all that's left is to make a decision.

I'm not sure if this assumption has already been made, but I think it's safe to assume that Microchip's future AVR chips will have well-defined boundaries (series 0,1,2, etc.), which can easily be categorized into their respective crates. For the rest, I think littering the code with #[cfg()]s is a small price to pay for consolidating 8+ crates into 2. If we're going to use them anyway, might as well use them a bunch.

Based on my understanding, the only thing this changes from a user perspective is how the crate is imported in Cargo.toml (i.e. whether or not it has a feature flag). From a contributor perspective, I don't think things would get much more complex since all the crates are in one repo anyway. Also, as you mentioned in the initial post, one of the benefits of merging crates is that the code becomes easier to change and maintain due to reduced code duplication, which seems very valuable to me.

Anyway, my opinions:

  • "Nuclear" Split: probably the best tradeoff, splitting them is hard, and the simplicity of two big crates with clear boundaries seems very nice (i guess more than that if you include series-0/1/2)
  • Split by Architecture: the option I'm most emotionally attached to, and similar to how the stm32 crates do it, but honestly doesn't help that much with code duplication as far as I can tell
  • Split by Generation/Series: the blurry lines splitting generations makes this a bit difficult to actually implement.
  • Split by Family: too many crates imo

tl;dr: I support either the nuclear split or splitting by architecture

Rahix added a commit that referenced this issue Feb 7, 2021
Start with the new unified HAL crate for all ATmega MCUs.  For now, only
support ATmega328P with just a GPIO-pins implementation.

Ref: #94
@Rahix
Copy link
Owner Author

Rahix commented Feb 7, 2021

Okay, I decided to go with the "nuclear" split for now in #129. We can always resplit later on if this turns out to be infeasible, but at least it gets us moving forward from the current situation. Sorry for the long delay...

@jaxter184
Copy link

Glad to hear the decision has been made! Just to clarify, are the newer series-0/1/2 chips getting their own category? and if so, are they each getting a crate or are they all being combined into one big atxmega crate?

@Rahix
Copy link
Owner Author

Rahix commented Feb 9, 2021

Glad to hear the decision has been made!

Right, sorry for the long wait...

Just to clarify, are the newer series-0/1/2 chips getting their own category? and if so, are they each getting a crate or are they all being combined into one big atxmega crate?

Yeah, they will at least be separate from ATmega of course. Whether it makes sense to split them into individual series, I am not yet sure - I would suggest we start with putting them all in one atxmega-hal crate and then see how that works out. We can always resplit later if it turns out to be the better design.

Rahix added a commit that referenced this issue Feb 13, 2021
Start with the new unified HAL crate for all ATmega MCUs.  Start with
support for ATmega328P's GPIO pins and USART peripheral.

Instead of, like the previous HAL, exposing `pac::Peripherals`, the HAL
now defines its own `Peripherals` struct.  This allows us to more
cleanly implement more complex peripheral support.  For example, instead
of individual fields for each port (PORTB, PORTC, PORTD, ...), the
`hal::Peripherals` only has a single field `pins`, wrapping all of the
above.

Ref: #94
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed mcu-support Support for a new Microcontroller
Projects
None yet
Development

No branches or pull requests

3 participants