search

yuri91 / ili9341-rs

A WIP, no_std, generic driver for the ILI9341 (and ILI9340C) TFT LCD display
Apache License 2.0
57 stars 50 forks source link

Adding new async API for DMA #45

Open ValouBambou opened 6 months ago

ValouBambou commented 6 months ago

Hi, I was working on a PR for embedded_graphics that will enable a AsyncDrawTarget that we could use in the driver in order to provide an async API. For example, this could enable the driver to use the DMA async implementation from embassy.

The implementation of this trait in the driver could be similar to DrawTarget but using AsyncWriteOnlyDataCommand generic trait bound instead of WriteOnlyDataCommand. This could be relatively easy once AsyncDrawTarget will be available publicly and could bring us some better performances.

ValouBambou commented 6 months ago

I have done a quick implementation on my fork to add this feature, but there is a typical issue of code duplication with sync and async code. I can't see a good way of avoiding it since this is a language current issue :/

yuri91 commented 6 months ago

We don't need to wait for embedded_graphics necessarily. We can already offer the async api to users that don't need the high level drawing methods.

About the duplication: yeah, not great. I looked at your fork and indeed it's pretty much copy-paste minus the .await and the method names having the _async suffix.

There are macros like https://github.com/fMeow/maybe-async-rs (but I think others too) to avoid duplication, but I am not sure if they are practical to use.

markszente commented 6 months ago

I can confirm that maybe_async_cfg is used in multiple crates for embedded development. Practicality-wise, well, it's fairly straightforward to set up and helps avoid code duplication.