search

esp-rs / esp-idf-hal

embedded-hal implementation for Rust on ESP32 and ESP-IDF
https://docs.esp-rs.org/esp-idf-hal/
Apache License 2.0
417 stars 172 forks source link

One Wire Interface #445

Open DaneSlattery opened 1 week ago

DaneSlattery commented 1 week ago

The current RMT wrapper targets the ESP-IDF library v4 interface, which does not support a TX and RX RMT driver on the same pin.

However, the v5 library does support TX and RX mode on the same pin, and provides an implementation as an ESP-IDF component in the form of a one-wire peripheral.

There currently exists a few bit-banging implementations based on embedded-hal

  1. one-wire-bus
  2. onewire

but both only work with blocking delays, and can block a CPU for at least 750ms when reading a 12-bit temperature sensor. I find that even in a separate thread, this delay can impact other tasks on the CPU due to these severe delays (the WDT is always watching). Neither of these libraries function correctly when async delays are used (I tried, based on embedded-hal-async)-perhaps because the timing constraints for bit banging does not suit an asynchronous execution model.

I have had some luck adding in the esp-idf onewire components through the dark magic of bind-gen (Really, what you have done there blows my mind, and I would love to read about how that system functions), and propose that this is upstreamed into esp-idf-hal as a OneWire peripheral module within the rmt module.

An alternative is to rather implement the RMT interface provided by ESP-IDF v5, and then re-build the layers of the RMT peripheral to support a TxRxRMTDriver, but this is much more of a gargantuan task.

Before I take a crack at it, I think we would have to discuss:

  1. Should ESP-IDF components become modules in the esp-idf-hal. I see components such as MDNS and Websockets are included in the bindings.h of esp-idf-sys, and wrapped in esp-idf-svc.
  2. Would it be better to target an rmt re-write based on ESP-IDF v5?

Please let me know your thoughts, and if this is a feature worth contributing!

Vollbrecht commented 1 week ago

A short headsup: The esp-idf component term is a bit overloaded, as in its simplest form it only means a directory that defines a CmakeList.txt and has a certain form. In esp-idf every subdir inside the components are basically that. If you need access to any of that we are totally fine to add it into the default binding.h file. You can also include your own C components the same way with esp-idf-sys.

The next step are components that also defines a idf_components.yml as they generally live outside the esp-idf repository. They can be fetched remotely via the esp-idf component registry or also provided locally. In general we call this class of components "remote_components" somewhat sloppy.

Switching from ESP-IDF v4 to v5 they split the old "/driver" component that included all peripheral hardware drivers into a component for every peripheral. In the case for the rmt driver it now lives in components/esp_driver_rmt.

In the case of the remote_component onewire_bus you brought up, it currently lives inside the official maintained extra_components repository.

Now getting back to the topic!

I think a general onewire bus driver provided by us can be useful, so if you want to work on it i am ok to integrate it.

To the question -> Should you reuse the onewire_bus and write wrappers for it, or write a driver based on the rmt headers is in general up to you as long as we make it fit in our general style of API. E.g the public API should follow similar naming and schema as the other Drivers.

I would be hesitant to accept a wrapper for a "random" maintained "remote component", but in this case its officially maintained as it lives in this special extra-component repo, so from that case i am fine to use it.

One big caveat though that you have to look for either way, we still have one open problem with the transition from all the "legacy" drivers. ESP-IDF forces to not use legecy and new drivers the same time. We have this upstream issue still open and for further info also this issue . The tldr is currently we need to gate the old from the new driver. We dont have a feature flag for the legacy rmt driver currently, but till upstream fixes the linked issue we need a feature for the legacy-rmt driver simmilar to the adc-oneshot drives we introduces recently, so a user cannot use the legacy-rmt driver and your provided wrapper that is most likely based on the new rmt api at the same time.