Re-export prelude of / whole crate `avr-hal-generic` in `mcu/*-hal` crates

icanwalkonwater commented 2 years ago

Why ?

Currently, when working with the crates of this repository, doing anything more than a blinky requires adding a bunch of dependencies that are already present in the upstream crates, just hidden.

For example, in a very basic app that uses only the serial and logs panics to it, I need all of these:

[dependencies]
# For interrupt::disable
avr-device = { version = "0.3", features = ["atmega1284p"] }
# For the uart hal
atmega-hal = { git = "https://github.com/icanwalkonwater/avr-hal.git", features = ["rt", "atmega1284p"] }
# For its prelude (uWrite + void)
avr-hal-generic = { git = "https://github.com/icanwalkonwater/avr-hal.git" }
# For uwriteln!
ufmt = "0.2"

Each one of these crates is already in the dependency tree of atmega-hal and is thus prone to version conflicts. The goal of this issue is to propose a solution for this.

Reexport `avr-hal-generic`'s prelude

The prelude of avr-hal-generic currently reexports traits from embedded-hal, ufmt and void, which are compiled either way and are useful to have in downstream crates.

The proposal is to create a prelude for the mcu/*-hal which reexports avr-hal-generic::prelude::*. Something like

// mcu/atmega-hal/src/lib.rs

pub mod prelude {
    pub use avr-hal-generic::prelude::*;
}

And maybe the same things in further downstream crates like arduino-hal.

This will also prevent version conflicts related issues as well. #303 #282

Reexport `avr-hal-generic`

In addition to that, reexporting the whole avr-hal-generic crates will be great, this was discarded as useless #98 but I'd argue that this is a similar case to the pac reexport in mcu crates and the hal one in arduino-hal, it allows users to drop down a level (or up in this case) and get access to all the traits and reexports of this crate to write more generic code.

This is similar to what they do on the stm32-rs project, for example in stm32f3xx-hal, the nb crate traits are implemented a bit everywhere and reexported at the root.

SebastianJL commented 1 year ago

The same would make sense for arduino-hal in my opinion. Yesterday I wrote a struct for handling a seven segment led. This element is used to display a single digit and has segments named A through G. With arduino_hal it looks a little something like this

struct SevenSegmentLed<A, B, C, D, E, F, G>
where
    A: PinOps,
    // Shortened for brevity.
{
    segment_a: Pin<Output, A>,
    // Shortened for brevity.
}

The problem is, that the Trait bound PinOps can only be found in avr-hal-generic.

Rahix commented 1 year ago

Reexport avr-hal-generic's prelude

:+1: from me. I'm actually surprised this isn't the case already... It probably got lost at some point during the refactoring.

Reexport avr-hal-generic

Hm, I'd rather see the pieces from avr-hal-generic re-exported in the appropriate locations. To some degree, this is already happening: For example in the adc module or in the i2c module:

https://github.com/Rahix/avr-hal/blob/eeef51bde41ab1a5ca846229e77c981d68897f5a/mcu/atmega-hal/src/adc.rs#L4

https://github.com/Rahix/avr-hal/blob/eeef51bde41ab1a5ca846229e77c981d68897f5a/mcu/atmega-hal/src/i2c.rs#L3

I think it is more sensible to add the missing re-exports in this way than to expose the entire avr-hal-generic crate's structure to the outside. It will make downstream code much more ugly if they have to pull in imports from all over the place rather than a single import for all i2c related items, for example.

From a quick glance, it does seem that these re-exports are missing in places, though. To begin, PinOps isn't re-exported in the port module in either atmega-hal nor the BSP arduino-hal crate. I think the end-goal should be having all items from avr-hal-generic re-exported somewhere appropriate in the HAL crates.

SebastianJL commented 1 year ago

I think it is more sensible to add the missing re-exports in this way than to expose the entire avr-hal-generic crate's structure to the outside.

I agree, seems like more work but if well done would be the better solution.

Rahix commented 1 year ago

I started with fixing the prelude and PinOps in #350. If you stumble over any other missing re-exports, please either report them here or, even better, send a PR :)

Rahix commented 1 year ago

I think this is all solved now, if not, let me know!

Rahix / avr-hal