Increase readability and consistency in src/gpio.rs

[mattiasgronlund]

Currently the gpio macro in src/gpio.rs has a lot of copy paste and future improvements will probably require even more copy paste.

For example the implementations for the embedded-hal traits currently consist of a large set of copy paste code.

As moving back from an erased type state to a non erased type state is a pain (and currently not even supported for the gpio pins (as I understand it), it seems like it would be beneficial to allow the pin states to be changed regardless if they are erased or not. But with current rust that would require a lot of copy-paste code as, so these implementations should be extracted into separated macros used by the top gpio macro. See e.g. stm32l0xx-hal for the amount of copy paste needed.

If we instead replaced the copy paste of code in the gpio macro with sub macros the code will be much easier to read and understand.

I do not know if we are allowed to use the paste crate as it as I understand require quite a new rust compiler (but at least works with a stable compiler). Using paste we could simplify the parameters to the gpio macro as many parameters are actually identifiers which could be constructed using the paste crate from one single identifier.

[teskje]

I agree that in its current state the GPIO code is pretty much unmaintainable. I have plans on refactoring it (see #122), so I'm very much interested in discussing all the things we can do better :) As of now I haven't looked too much at what the other HALs are doing, but I'm sure we can already find a lot of good ideas there.

I don't see an issue with using the paste crate. It will probably increase compile times a bit, but not too much I would think, since we already have proc macros in our dependencies. Regarding the compiler version, we don't specify an MSRV for this crate so I assume anything that's stable goes. But the other maintainers now better than me. In any case, if paste can help us get rid of boilerplate, I'm all for using it. Same goes for cfg-if by the way.

However, rather than trying to somehow make our current use of macros and #[cfg] bearable, I'd like to focus on reducing them altogether. I don't think we can do completely without them, but we should be able to do a lot more than now.

A lot of the functionality that's currently implemented via macros can probably be replaced by traits and generics. I haven't experimented with that yet, but my idea is that we should be able to implement a common trait for all the pin registers (via a macro) and then implement all the common functionality based on this trait.

For reducing #[cfg]s the path taken by the stm32l0xx-hal looks very promising (see #122). There are only five different GPIO versions for the whole F3 family of MCUs, so it should be possible to reduce the number of needed feature gates to five too. In addition, parsing the CubeMX DB also gives us all the AF mappings we need, so we can use code generation to get rid of what's currently the largest part of the GPIO macros. Of course this all relies on the data from the CubeMX DB to be actually correct, but I'm optimistic :)

I can't say much to the erased pin types since I've never needed or used them. I'm also not quite sure what they are usually needed for, but I guess it's about being able to store an arbitrary pin in cases where one doesn't care about its type?

[Sh3Rm4n]

I, too, think, that this module can be improved.

I can't say much to the erased pin types since I've never needed or used them. I'm also not quite sure what they are usually needed for, but I guess it's about being able to store an arbitrary pin in cases where one doesn't care about its type?

Generally these are used to define more generic APIs, where the exact type is not as important and would constrain the API to much, e.g. creating a pin array. Also, this can be used to avoid long type annotations. So it can be quite useful in some scenarios.

As moving back from an erased type state to a non erased type state is a pain (and currently not even supported for the gpio pins (as I understand it), [...]

That's a good point. This is just missing, atm.

The difference about the fully erased pins compared to the original types, is that the information about the GPIO itself is stored at runtime instead of at compile time with the full type available.

[...] it seems like it would be beneficial to allow the pin states to be changed regardless if they are erased or not.

I don't know if anything technical speaks against that. It seems like a useful addition though.

paste seems like a really useful addition! I see, that this could improve the method madness in the gpio module.

Regarding the compiler version, we don't specify an MSRV for this crate so I assume anything that's stable goes. But the other maintainers now better than me. In any case, if paste can help us get rid of boilerplate, I'm all for using it. Same goes for cfg-if by the way.

I'm all in for using these crates as long as they are compatible with current stable. I don't know about any MSRV guidelines in the stm32-rs group, though. But if we would introduce crates like paste, which require a rather new rustc, I would like to see a MSRV, which is build against in the CI and made visible to the user, just to avoid confusion.

In addition, parsing the CubeMX DB also gives us all the AF mappings we need, so we can use code generation to get rid of what's currently the largest part of the GPIO macros. Of course this all relies on the data from the CubeMX DB to be actually correct, but I'm optimistic :)

The CubeMX approach looks really promising and would reduce a lot of manual work and potential errors. I've heard that the CubeMX DB is not error free though. But I'd take that risk.

[mattiasgronlund]

I really like the idea of using CubeMX DB.

I also like the idea of having traits for the functionality.

I have also more and more realized that stm32xyyy devices to a very big extent share a few common variants of most peripherals, so these traits and their implementations would probably be possible to share between the different device crates. I do not know how the discussions has been in this regard, but I think a lot of effort could be saved going that route.

[Sh3Rm4n]

These similarities were noticed in the community. For instance the time.rs module is an obvious candidate to put behind a trait (crate), which can be shared between the HAL implementations.

But AFAIK, these efforts are not followed yet, to avoid preliminary constrained APIs and give the different HAL implementations the possibilities to experiment. The price to pay is to invest more effort to implement the same feature over the different HALs.
But these are discussions we could kickstart on the new meta repository, as the stm32-rs group is still loosely organized.

[teskje]

If the CubeMX DB approach actually works, i.e., the data in the DB is (reasonably) correct and can be used to auto-generate the pin mappings, it might actually be not that hard to generalize such an implementation to all stm32 families. I imagine you'd simply tell the code-gen to include all the other GPIO peripheral versions too and that's it. We could put that into a stm32-gpio crate and each HAL would select the GPIO versions it actually needs via features.

That assumes that all the GPIO peripherals are pretty much equivalent in terms of how they work, which seems to be the case AFAICT.

[Sh3Rm4n]

Speaking of CubeMX DB, I justed want to link to cube-parse, which was successfully used for the stm32l0xx-hal.

I have not played around with it, yet. But this seems really promising.

[Sh3Rm4n]

As of #129 I would consider this issue as resolved for now. Incremental improvements can be always done in the future.
But the reliance on macros will stand, as is the only sane way to ensure a type safe gpio implementation.

Anyways, if improvements on the macro capabilities are made, this code can be made more efficient / readable.