embedded_hal::i2c::I2c no longer implemented for async I2c in esp-hal 0.21

[tomried]

Bug description

In commit 00ad9b5eed876bb34a9a4daee8ee587a60425e6d, the embedded_hal::i2c::I2c trait was removed from async I2c and is now only implemented for blocking I2c.

To Reproduce

Try using something requiring the embedded_hal::i2c::I2c trait with an async I2c bus.

Expected behavior

embedded_hal::i2c::I2c should be implemented for blocking and async I2c, as it was in 0.20.1.

Environment

ESP32-S3 with esp-hal 0.21

[jessebraham]

FYI we published a patch release today which restores this trait implementation.

[bugadani]

I'm reopening this, just because we are pretty inconsistent with ehal traits in I2C. We implement ehal1::I2c for any mode, but ehal02 traits only for Blocking. We should probably be a bit more consistent, or at least decide whether we want to be intentionally inconsistent.

[bugadani]

I think there are three questions here, that we should answer and extend that answer to all our peripherals:

• Do we want to keep embedded-hal 0.2 support? I think yes, it's not a huge amount of work and some old drivers may need it.
• Do we want to implement blocking embedded-hal (be it 0.2 or 1.0) traits for Async peripherals? I think we should.
• Do we want to provide blocking inherent methods on Async APIs? We could as _blocking functions, but depending on trait impls it may not be necessary.

If the answer is "yes" to the third, we'd end up with the following, which has some interesting implications to generic code (user code couldn't be generic over Mode, users would be forced to use embedded-hal traits to use blocking APIs of I2C drivers of unknown mode).

impl I2c<Blocking> {
  fn transfer();
}
impl I2c<Async> {
  async fn transfer();
  fn transfer_blocking();
}

The internal implementations would need to be decoupled from the actual API, but that is a relatively small task. Then we could delegate public API as well as trait impls to these internal implementations.

[jessebraham]

* Do we want to keep embedded-hal 0.2 support? I think yes, it's not a huge amount of work and some old drivers may need it.

I seem to be in the minority, as I have shared this opinion before, but I think there is little value in continuing to implement these. Many of the traits are not even useful, and the 1.0.0 release has been out for nearly a year now. At some point we need to rip off the band-aid and force people to get with the times. Especially if we include these in our release (whenever this happens), we're committing to carrying them around for the duration of the support period.

[MabezDev]

Closing in favour of #2590