Improve consistency between HAL implementations

[ejpcmac]

Rationale

I’ve digged into Rust Embedded a few weeks ago, after lurking at it for some time. Before to say anything else, I want to thank you so much for your incredible work. The ecosystem is still young, yet already joyful to work with and easy to contribute to.

I’ve recently ported stm32f767_hal to Rust 2018 and added timers support, and written a tiny RTFM blinky firmware with it. In the effort of porting it to an STM32L0, I’ve started to hack on stm32l0xx_hal this week-end.

During this process, I’ve read code and examples from different HAL implementations, and have found inconsistencies between them, and sometimes between modules in the same crate. Yet, there are no implementation guidelines, so it’s not easy to know which way among the others is the more “correct”.

Examples of inconsistencies

pac vs stm32

Some crates, like the stm32f1xx_hal and now stm32f767_hal use pac to refer to the Peripheral Access Crate. Some others—seems to be a majority in the stm32-rs organisation—use stm32.

Periph::periph1(p.PERIPH1, ...) vs p.PERIPH1.periph1(...)

I’ve seen both versions, even in the same crate.

I tend to prefer the first one as it has less magic in it: you understand quickly that you initialise a peripheral, and need to pass it to the constructor so it can take the property. The second one, in the other hand, takes a bit less code to write but it’s not obvious where the periph1 associated function comes from, as it is not initially in the PAC. It’s only my view, but I fount it less easy to read, and has the too-much-magic drawbacks from the OOP world. Extending a type like to do 12.hz is cool, but I don’t think it has its place everywhere.

Non standardised but useful helpers

While adding timers to stm32f767_hal, I’ve grabbed clear_update_interrupt_flag from stm32f1xx_hal. However, porting my little firmware to the STM32L0, I’ve needed to use clear_irq instead.

The first one is more precise, while the second one is more obvious to the user.

---

There are some more inconsistencies for sure, but let’s stop here, it is just to get the idea.

Possible solution

The first solution I think of right now would be to maintain a HAL implementation guide somewhere, which would be easy to find for any new contributor. This way, contributing would be even easier a there would be less questions about which crate is a good model / has the more recent API.

I know there are difference between architectures, especially between vendors, so all cannot be standardised in a very general fashion. However, what can be standardised should be, and there should be some patterns to follow in a given architecture—for instance for all stm32-rs crates.

I’m willing to help on this subject if you mind. I can transform this issue in a more formal pre-RFC too if needed.

Again, all of this is awesome, and I’m pretty excited to take part of this revelution in embedded systems development :)

[therealprof]

Yet, there are no implementation guidelines, so it’s not easy to know which way among the others is the more “correct”.

Well, there's no right or wrong. Even if there were implementation guidelines, there's no one fits all approach to this since the register interface is vendor specific and those vendors have completely different ideas on what a good interface looks like. To implement your suggestion would mean to create one definitive interface everyone would have to implement, no matter how scary that implementation would be, cf. Arduino.

Some crates, like the stm32f1xx_hal and now stm32f767_hal use pac to refer to the Peripheral Access Crate. Some others—seems to be a majority in the stm32-rs organisation—use stm32.

Well, pac is officially coined lingo whereas stm32 is a convenience convention to simplify knowledge transfer between STM32 HAL impls and have a common name for multiple MCUs within the same family. I do not think there's a huge benefit using one over the other as a prefix but I certainly have no problem to accept patches defining both as usable aliases and moving the internals to use pac only while slowly phasing out stm32 for external use.

Periph::periph1(p.PERIPH1, ...) vs p.PERIPH1.periph1(...)

Do you have an example? The latter should not even be possible since that would be register periph1 of peripheral block PERIPH1 which only has a few methods like read and write available but cannot be constructed.

However, what can be standardised should be, and there should be some patterns to follow in a given architecture—for instance for all stm32-rs crates.

I think it will be hard or even impossible to define and enforce hard rules but if we can come up with best practises I would not object to publishing those.

[TeXitoi]

Methods on pac object are added using traits in the prelude of the hal.

I think we can homogenize at least the hal un stm32 by having a common interface, begining by the clock management and the time module.

[therealprof]

I think we can homogenize at least the hal un stm32 by having a common interface, begining by the clock management

I very much doubt that. Clocking is vastly different between families.

and the time module.

That should be possible.

[ejpcmac]

@therealprof Thank you for your detailed answer. I may think too much about details sometimes.

Well, there's no right or wrong. Even if there were implementation guidelines, there's no one fits all approach to this since the register interface is vendor specific and those vendors have completely different ideas on what a good interface looks like. To implement your suggestion would mean to create one definitive interface everyone would have to implement, no matter how scary that implementation would be, cf. Arduino.

I understand this. I was more thinking about things to avoid or ways to name / do the things if possible and natural on the given platform. But maybe that’s too much energy for something that is not relevent.

Some crates, like the stm32f1xx_hal and now stm32f767_hal use pac to refer to the Peripheral Access Crate. Some others—seems to be a majority in the stm32-rs organisation—use stm32.

Well, pac is officially coined lingo whereas stm32 is a convenience convention to simplify knowledge transfer between STM32 HAL impls and have a common name for multiple MCUs within the same family. I do not think there's a huge benefit using one over the other as a prefix but I certainly have no problem to accept patches defining both as usable aliases and moving the internals to use pac only while slowly phasing out stm32 for external use.

Ok, so I’ll go for pac on any refactoring, as I go. That’s one question less :)

Periph::periph1(p.PERIPH1, ...) vs p.PERIPH1.periph1(...)

Do you have an example? The latter should not even be possible since that would be register periph1 of peripheral block PERIPH1 which only has a few methods like read and write available but cannot be constructed.

As @TeXitoi said, it is added via extensions to the types, in the predule. You can see an example here. I think this one particularly should be discussed, to use one or the other, but not a mix of both ways.

Edit: Nevermind, reading more carefully the implementations has shown to me that the extension from the prelude in fact calls the Module::peripheral1(p.PERIPHERAL1, ...) thing. It’s mainly my mental model of all this which was fairly inconsistent.

However, what can be standardised should be, and there should be some patterns to follow in a given architecture—for instance for all stm32-rs crates.

I think it will be hard or even impossible to define and enforce hard rules but if we can come up with best practises I would not object to publishing those.

I’ll gather some ideas and try to identify emerging tendencies while doing my next contributions and reading code from different HAL crates. I’ll soon receive a RISC-V board from HiFive too, so it will be the occasion to see a different architecture and not to be too STM32-minded.

Where should I post it once I have something?

Edit: While my global mental model of HAL crates is building up, I see now more clearly the different generations and improvements. What would be needed is not really a best practice guide, but more something like an “onboarding guide” to do a quick tour of this for newcommers like me. I was too quick in my reading of the code, saw one way of doing the things, then got surprised by the older/newer concepts.

[jamesmunns]

Nominating this issue for "keep", I don't think we're close to defining "best practices", but this is an ask I've heard repeated recently. It might be good to re-discuss this soon.