Remove `new_..._async` constructors

[bugadani]

These constructors usually could be formulated as new(...).into_async(). This would even allow us to configure the priority of the async interrupt handler, although that's one more function :)

---

The majority of the work has been completed in #2430 - timer APIs, and TX/RX only PARL_IO remain to be updated.

[Dominaezzz]

Oh I'm drafting something for this actually.
I generally think interrupt setup should be separate from drivers.

[MabezDev]

I like this idea in general, but I'd like to see the design fleshed out a little more before we commit to it.

Just some loose thoughts:

• Does it make sense to be able to go back and forth between Blocking and Async, i.e will into_blocking also exist?
• Regarding the interrupt handler priority, I think we should have a story here from day one. I suppose fn into_async(self, Priority) -> Self is one option, but is there anything else we might want to configure here? DMA prio perhaps for some peripherals? Maybe that's better done in the top level esp_hal config though.

[bugadani]

Does it make sense to be able to go back and forth between Blocking and Async, i.e will into_blocking also exist?

Yes, although I'd imagine it would be rarely used. IMO the Async API should be a superset of the blocking API (that is, in terms of the general driver, disregarding interrupt functions, which should only be part of Blocking). Transitioning back to manual interrupt handling may be rare, but it's not a reason to disallow (or to require dropping and re-creating the driver).

flowchart TD
    construct
    Blocking
    Async

    construct --> Blocking
    Blocking -- "into_async(self)" ---> Async
    Async -- "into_blocking(self)" ---> Blocking

Loading

Regarding the interrupt handler priority, I think we should have a story here from day one. I suppose fn into_async(self, Priority) -> Self is one option, but is there anything else we might want to configure here? DMA prio perhaps for some peripherals? Maybe that's better done in the top level esp_hal config though.

IMHO the async API should have a with_priority (I'm undecided generally about set_ functions that take &mut self in addition) function to set priorities, similar to most of the driver configuration. DMA priority is the responsibility of the DMA channel API. I don't think separating some of the driver config into a global project config is a good idea.

---

With Dominic's "let's move the interrupt control out of the driver" idea we would have the following options:

flowchart TD
    construct
    Blocking
    Async
    Split["Blocking + external interrupt control"]

    construct --> Blocking
    Blocking -- "split(self) -> (Split, InterruptControl)" --> Split
    Split -- "unsplit(Split, InterruptControl)" --> Blocking
    Split -- "unsplit_into_async(Split, InterruptControl)" --> Async
    Blocking -- "into_async(self)" ---> Async
    Async -- "into_blocking(self)" ---> Blocking
    Async -- "split_into_blocking(self) -> (Split, InterruptControl)" --> Split

Loading

This looks daunting but is strictly additive I believe.

[Dominaezzz]

There might be a problematic area in here for DMA peripherals, where both the peripheral and the DMA channel have interrupt handlers which can provide asyncness.
Does the transitioning from blocking to async also affect the channel in the driver?
Do drivers really need the DMA channel to be async for them to provide async features?
My concern is that the answer isn't the same for all drivers.
I think I2S and PARL_IO drivers want the DMA channel to be async, but that should not be strictly necessary. (Was going to look into this at some point)

[bugadani]

My PoV is that if the user binds a dma channel (like for SPI) then we use that over other async options. If not, then not - quite obviously.

[Dominaezzz]

Transitioning back to manual interrupt handling may be rare

Just to share my use case for transitioning back to manual interrupt handling.
For drivers like SPI and I8080, the devices I talk to sometimes need some initialization steps to be executed before you can start using them properly.
Whilst doing this init I'd like to use async, so my init delays and such can be async as well. Once initialization is done, I can move on to doing manual interrupt handling to perform some queueing.

[MabezDev]

I think the benefits of into_async are clear, and IMO we should move forward with this to avoid exponential amounts of constructors on some drivers. I'm not yet convinced about the interrupt splitting, but as @bugadani pointed out those changes would be additive so we can figure out what that looks like later.

I'll let this sit till Monday to allow other views and concerns to be heard - @esp-rs/espressif

[bjoernQ]

I like the idea to create basic drivers which can be brought into async / blocking mode (they start living as blocking, probably)

I also think holding DMA channels in a certain mode could make this more complex (but it's early and I'm just one coffee into the day). I have seen discussions about treating DMA channels more like plain-old peripherals - might influence design discissions here, too

[MabezDev]

With #2430 & the follow up with #2461, the only remaining drivers with new_async are the low level timers.

This begs the question, should the low-level timers (TIMG & Systimer) be just that, and only implement the Timer trait and nothing more? This would allow us to consolidate our timer logic into the timer structs themselves (Periodic and OneShot), and I believe it opens up some clean up around AnyTimer (cc: @bugadani as you know more of the details here).

If we do the above, we could still implement the blocking traits on the low level timers - do we want to do that? Personally, I think probably not, it's better to be consistent imo.

[bjoernQ]

Personally, I think probably not, it's better to be consistent imo.

+1

[MabezDev]

I'm leaning towards removing the impls on the timers, and therefore the mode parameters too, and only implemented the eh/eha traits on the top level timer driver objects.

I'll let this sit a little while longer, just to be sure, but I'd like leave enough time to do the impl before the 0.22 milestone is complete.

[bugadani]

but I'd like leave enough time to do the impl before the 0.22 milestone is complete.

Ambitious :D

[bugadani]

TX/RX only PARL_IO remain to be updated.

I just realized that PARL_IO isn't a technical issue - tx/rx halves can be separately configured since their DMA channels are independent and have independent IN/OUT interrupts.

[MabezDev]

I'm leaning towards removing the impls on the timers, and therefore the mode parameters too, and only implemented the eh/eha traits on the top level timer driver objects.

Let's make the timers dumb again and consolidate the logic in the timer driver structs.

[MabezDev]

The last two remaining new_async methods have been removed 🚀