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

Depreciation message for rmt.h #28

Open
klaus-liebler opened this issue Jun 9, 2022 · 21 comments
Open

Depreciation message for rmt.h #28

klaus-liebler opened this issue Jun 9, 2022 · 21 comments

Comments

@klaus-liebler
Copy link

Hi David,

when compiling with idf5.0 (master branch), I get a depreciation message for rmt.h:

esp-idf/components/driver/deprecated/driver/rmt.h:18:2: warning: #warning "The legacy RMT driver is deprecated, please use driver/rmt_tx.h and/or driver/rmt_rx.h" [-Wcpp]

Regards,

Klaus

@DavidAntliff
Copy link
Owner

Hi Klaus, thanks for the report. I'll investigate switching over to the new driver when I move to IDF 5.0.

@edg2411
Copy link

edg2411 commented Aug 1, 2022

On owb_rmt.c file: changing the rmt_set_pin function to rmt_set_gpio and adding a boolean 0 flag as a last parameter works for me. ( it was finding devices but not showing the readings temp, now it works just fine!)

@paulfaid
Copy link

I also applied that change locally. I was getting crashes when I tried addressing a 1wire device that was no longer connected (testing my error handling) and applying this change fixed these crashes.

@DavidAntliff
Copy link
Owner

The IDF v5.0.1 build should be ok in master now (rmt_set_pin() -> rmt_set_gpio(..., 0). Thanks to @mjcross for fixing that up.

As for the driver/rmt.h deprecation - I will need to come back and address this another time, as it's a non-trivial change. I'm leaving this URL here for my own reference later: https://docs.espressif.com/projects/esp-idf/en/latest/esp32/migration-guides/release-5.x/5.0/peripherals.html#rmt-driver

@mjcross
Copy link

mjcross commented Mar 27, 2023

Thanks for accepting the PR 👍
I've already been looking at the new RMT driver and I'll let you know how I get on.
Also in case it's useful, my 1-Wire driver for the RPi Pico / RP2040 has just been merged into raspberrypi/pico-examples:develop.

@bbinet
Copy link

bbinet commented Mar 27, 2023

FYI, there is now an example for 1-wire (based on rmt peripheral) on esp-idf itself:
https://github.com/espressif/esp-idf/tree/master/examples/peripherals/rmt/onewire_ds18b20

@mjcross
Copy link

mjcross commented Mar 27, 2023

FYI, there is now an example for 1-wire (based on rmt peripheral) on esp-idf itself

Ah! Many thanks for that - I'd looked for an Espressif example but couldn't see one

@mjcross
Copy link

mjcross commented Mar 27, 2023

Update: I’ve read the docs for the new driver/rmt and I think it is doable. Going to be a rewrite rather than a patch, but obviously keeping the same api.
What do you think @DavidAntliff - are you happy for me to have a bash at it?

@DavidAntliff
Copy link
Owner

@mjcross sure, if it's original code and it continues to work with the esp32-ds18b20 component (single, multiple & parasitic), and the associated example app, I'd be happy to consider merging such a PR. Thank you for offering.

It is interesting that Espressif now have their own OWB example, using RMT. The question of whether esp32-owb is still needed, in light of that, is worth asking. Although their example is coupled to the DS18B20, whereas this component is more general-purpose. That said I've never used it with anything other than a DS18B20.

@mjcross
Copy link

mjcross commented Mar 27, 2023

I asked myself the same question about whether esp-owb still has a place. IMHO whilst the Espressif example serves well as example code, it seems a bit over-engineered for everyday use. So yes, personally I’d say a slimmed down version has a place.
Concerning the support for parasitic power by the way, is that implemented for both RMT and GPIO or just the latter? I haven’t looked in detail but I didn’t think it worked on RMT - but I could be completely wrong on that.

@mjcross
Copy link

mjcross commented Mar 27, 2023

By the way, when I said a ‘rewrite’ I was only proposing to replace the current owb_rmt.c and .h, leaving everything else the same (well that is the objective, anyhow) :-)
I’ll carry on and see how far I get. 👍🏻

@DavidAntliff
Copy link
Owner

Yes, the IDF example is probably less useful for integrators as it's designed for teaching, mostly. Anyway, having a choice is good. If you need to use the example to learn how to implement this, by all means, but please do try to keep the new code original. I know that can be difficult when both codebases are practically doing the same thing, but I also don't want to run into licensing issues.

To be frank I've never used the parasitic feature beyond writing the support for it, so I actually can't remember! I've just looked through the code and I can't see any indication that it's not designed to work with RMT, but perhaps there is an issue and it hasn't been brought up formally. If you have the ability to test this, please do, but if you can't let me know and I can rig something up and give it a go.

Copy that on "rewrite" - I understood you :)

Thanks again, I appreciate you taking an interest in this component and being a contributor. Happy to put a credit in the README once done, if you are OK with that.

@mjcross
Copy link

mjcross commented Mar 27, 2023

Completely agree on the importance of steering clear of licensing issues; and I’m fine with that. I wrote the Pico driver from scratch including the pesky ROM search code, so it’s pretty familiar turf apart from the RMT aspect.
The new RMT driver is a bit involved but the documentation is pretty clear. One exception to that is the description of how to write your own Encoder, which I didn’t really follow ~ but I think the pre-rolled ones will do fine for what we need.
I’ve never used the parasitic mode either (or come across anyone who has). I’ve definitely got the kit to try it at some point, but I’ll defer that until later.
Will keep you posted as to progress.

@mjcross
Copy link

mjcross commented Mar 30, 2023

Shared a private repo with you @DavidAntliff, for the work in progress

@mjcross
Copy link

mjcross commented Apr 2, 2023

Basic framework done; OWB bus reset and slave detection are now working.

@mjcross
Copy link

mjcross commented Apr 5, 2023

Everything now seems to work fine so I'll start preparing a PR

@mjcross
Copy link

mjcross commented Apr 6, 2023

Now works with esp32-ds18b20-example:

  • fixed a simple bug in write_bits() that prevented it from sending single bits
  • spent way too long chasing what I thought was a buffer overflow, but turned out to be a Heisenbug related to me setting over-short device timeouts

Further testing underway, to be on the safe side

mjcross pushed a commit to mjcross/esp32-owb that referenced this issue Apr 6, 2023
@NRollo
Copy link

NRollo commented Jun 2, 2023

@mjcross, I take it you are satisfied with the 0e1821c commit or is testing still ongoing?

@mjcross
Copy link

mjcross commented Jun 2, 2023

Yes I'm happy with it - in fact I've been using it in production for a few weeks now.
If are OK with a simpler API then you might also find it useful to take a look at https://github.com/mjcross/esp32_onewire

@NRollo
Copy link

NRollo commented Jun 5, 2023

@mjcross Thanks for the information, the 'esp32_onewire' looks interesting, what is the differences between that and 'esp32-owb' (what would I miss compared to 'esp32-owb')?

@mjcross
Copy link

mjcross commented Jun 5, 2023

@NRollo You're welcome. They both use exactly the same code to talk to the bus, but esp32_onewire is a stripped down version that has a simpler API. It's designed for straightforward use-cases that don't need CRC checks or parasitic power.

If you compare the two examples esp32_onewire/main/main.c and DavidAntliff/esp32-ds18b20-example/main/app_main.c then I think you'll get the picture.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants