Turn traits and execution model modules inside out

[eldruin]

A proposal for discussion.

Advantadges

• Place for execution-model-independent type definitions
  • For example useful for SPI modes
• Fits better with users who might think of peripheral first, then execution model. - @Sh3Rm4n
• Helps discoverability, since the modules are not "hidden" behind their execution model. - @Rahix

Disadvantadges

• Breaks every last embedded-hal-dependent code
• Users may want to import several embedded_hal::xxx::blocking modules, for example, although there is no much reason to do so.
• There might be confusion between embedded_hal::xxx::nb and embedded_hal::nb.

Open questions

• Reexport execution-model-independent types in blocking/nb submodules?

Alternatives

Keep the structure as-is

[rust-highfive]

r? @therealprof

(rust-highfive has picked a reviewer for you, use r? to override)

[Sh3Rm4n]

Even though this change might be quite disruptive, I really like this approach.

I imagine, normally when a user uses e-h they will first choose the interface they want to use (spi, serial, ...) and then they'll choose the implementation, whether it's blocking, nb based or async.

Sometimes, using mutliple implementations simulatiously could also happen. This change would make the use statements cleaner. Instead of

use emedded_hal::{
    nb::serial::Write,
    blocking::serial::Write,
};

it would be like this:

use emedded_hal::serial::{
    nb::Write,
    blocking::Write,
};

[therealprof]

@ryankurte Are you aware of this proposal? Any concerns? Otherwise we'd go ahead merge it.

[ryankurte]

thanks for the reminder, totally fell off the stack...

it is an interesting exercise to consider alternative layouts and this change is neat that it creates an obvious space for common definitions, however, apart from that i'm not convinced there's a strong argument for this, or that the refactoring is a net improvement over the existing layout compared to say, adding a common module.

so i'm opposed to this as it seems to me to be substantial churn for predominantly a stylistic preference. while this is absolutely the time to make breaking changes, it does impact a bunch of downstream users, and i don't see that it makes anyone's e-h experience measurably or demonstrably better as justification for this.

Users may want to import several embedded_hal::xxx::blocking modules, for example, although there is no much reason to do so.

I imagine, normally when a user uses e-h they will first choose the interface they want to use (spi, serial, ...) and then they'll choose the implementation, whether it's blocking, nb based or async.

my experience is probably the opposite of the comments on this PR. ime you tend to import a whole pile of peripheral definitions when wiring things up, what you use is primarily defined by the runtime available to you, and it's unlikely (*, except for serial atm) one would choose to mix-and-match those from blocking/nb(/async).

there's also talk of feature-gating the upcoming async feature, which is totally doable either way but, much simpler with the current layout.

[GrantM11235]

Even if this PR is not accepted, there will still be a lot of churn due to #278.

For the record, I do think that this refactor is a net improvement.

[Rahix]

I would say we shouldn't let "downstream churn" hold up progression of the crate design for embedded-hal. The amount of work it takes each maintainer to upgrade is going to be tangible and as we're using Rust here, there also need not be any fears of accidentally breaking something in the process. Improving usability should be the top priority right now, especially as we're still in a place where breaking changes don't hurt anyone.

Regarding the actual changeset: I also think this is a good idea because it improves discoverability. Some modules only exist in one of the two (three?) categories so it would be harder to see them if hidden behind an nb or blocking module first. Having all the supported "concepts" visible on the landing page of the documentation (i.e. in the crate root) help a lot with this, in my opinion.

[ryankurte]

discovery is a fair point, though we also control the docs landing page / can put what we want on it / could steer people to the appropriate traits for their runtime. i am still not convinced it's an improvement but i also don't have any substantial concerns so, if the rest of @rust-embedded/hal are onboard don't let me stand in the way ^_^

[therealprof]

@ryankurte Thanks for unblocking this.

I am a fan of the revised model for the reasons mentioned. So let's get the party started. 🎉

bors r+

[bors]

Build succeeded:

• ci-linux (1.35.0, x86_64-unknown-linux-gnu)
• ci-linux (stable, thumbv6m-none-eabi)
• ci-linux (stable, thumbv7m-none-eabi)
• ci-linux (stable, x86_64-unknown-linux-gnu)
• ci-linux-test (1.36.0, x86_64-unknown-linux-gnu)
• ci-linux-test (stable)
• fmt