Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

One Wire Interface #445

Closed
DaneSlattery opened this issue Jul 2, 2024 · 6 comments
Closed

One Wire Interface #445

DaneSlattery opened this issue Jul 2, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@DaneSlattery
Copy link
Contributor

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
Copy link
Collaborator

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.

@Vollbrecht Vollbrecht added the enhancement New feature or request label Jul 4, 2024
@DaneSlattery
Copy link
Contributor Author

Thank you for the feedback. I have begun my implementation by thinly wrapping the interface exposed by the esp-idf component and working towards "searching" the onewire bus for implementing devices such as the DS18b20 temperature probe. I consider the first PR to be complete when

  1. Everything builds on all supported targets.
  2. The competing features are mutually exclusive
  3. The wrapper can find devices on the bus.

After that, I will probably throw up a follow-up PR that implements the DS18b20 temperature probe, similar to how the Ethernet modules supports different chips (LAN8720X, etc).

@xiaguangbo
Copy link
Contributor

I'm using io simulate onewire to support ds18b20, It would be great if could use hardware to support it.

@xiaguangbo
Copy link
Contributor

I will master code copy to local, but build faild

error[E0412]: cannot find type `onewire_device_iter_handle_t` in this scope
  --> /home/xiaguangbo/.cargo/registry/src/index.crates.io-6f17d22bba15001f/esp-idf-hal-0.44.1/src/onewire.rs:45:13
   |
45 |     search: onewire_device_iter_handle_t,

onewire_device_iter_handle_t from https://components.espressif.com/components/espressif/onewire_bus/versions/1.0.2?

how do?

@DaneSlattery
Copy link
Contributor Author

Hi @xiaguangbo , please see the comments on top of this file about the component inclusion and patch. Remember to also run cargo clear, sometimes it's a bit finnicky with building new versions of esp-idf-sys.

@DaneSlattery
Copy link
Contributor Author

I think we can resolve this ticket?

@github-project-automation github-project-automation bot moved this from Todo to Done in esp-rs Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

No branches or pull requests

3 participants