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

ESP32 RMT driver exposes no errors when reading #1032

Open
tve opened this issue Feb 13, 2023 · 1 comment
Open

ESP32 RMT driver exposes no errors when reading #1032

tve opened this issue Feb 13, 2023 · 1 comment

Comments

@tve
Copy link
Contributor

tve commented Feb 13, 2023

Build environment: Arch Linux
Target device: ESP32

Description
When using an RMT device in rx mode there is no provision for errors. The HW is pretty cryptic but the ESP-IDF does expose an rmt_get_status call (which is not in the esp32 tech ref man...) and the rmt_reg.h header file shows that there are bits for "full" and "owner error", plus a couple more that are probably not relevant (yet).

The problem I'm having is that the RMT memory is overflowing due to what looks like too long a packet to it. The result is that when I read I get 0 samples. There's no way for my code to detect that it actually got too many samples.

In my use-case this is a problem because I need to tune the sensitivity of the radio feeding the signal in response to the number of pulses detected. E.g., if there are "too many" pulses it's too sensitive and picks up too much noise and I need to decrease the sensitivity. The reverse for "too few" pulses. The problem then is that 0 pulses could mean way too sensitive or way too insensitive.

Steps to Reproduce
This problem is not entirely easy to reproduce. The best I know is to generate a PWM burst on one pin and feed that into an RMT pin. Generate a burst of <64 PWM transitions and you get the expected result reading. Generate >64 PWM transitions and read returns 0.

Expected behavior
I expect some error indication.

Part of me posting here as opposed to just adding the necessary code is that I'm wondering what the right way to add error indications is. In an ideal world:

  • rx would have a onReadable and an onError callback
  • an overrun would trigger the onError callback

Given the current state of the interface, I see a couple of options:

  • add a status method that calls rmt_get_status and can return an "overflow" status
  • make read call rmt_get_status and throw in case of error
  • support onError and make read call rmt_set_status and make the onError callback when appropriate

NB: I'm not 100% sure of this, but I believe the ringbufferSize is a bit of a mis-feature. Given that the RMT driver hard-codes config.mem_block_num = 1; it is impossible to receive any transmission with more than 64 pulses (128 transitions). Each call to read pulls off one entry from the ring buffer, which is one "packet", which is limited to those 64 pulses. I don't see a way to fill the default ringbuffer size of 1024. (Being able to increase the mem_block_num would be highly desirable.)

Update: it looks like the problem is more complicated than I thought. "Packets" longer than 64 pulses get silently truncated to 64 and I'm not seeing that the truncation is in any way signaled by ESP-IDF. The problem I'm really hitting is "infinite packets", i.e., never hitting the idle threshold and therefore never getting any packet.

I added a rmt_get_status call to read and throw an error if the mem_full bit is set. That does not get triggered by packets longer than 64 pulses (presumably the status gets cleared rapidly by esp-idf reading the packet into the ring buffer). However, it gets triggered by infinitely long packets (e.g. just put a PWM signal into the RMT pin). So it helps detect infinite streams of pulses but not overruns.

Other users have hit this problem (in ESP-IDF) trying to receive servo PWM pulses or measuring PWM duty cycle, e.g. https://esp32.com/viewtopic.php?f=13&t=5122&start=10

Trying to step back...

  • I do believe Moddable should support setting mem_block_num
  • It would be nice to produce an error when the RX is effectively hung due to too many packets, but maybe that's too much of an edge-case to include (the Espressif HW and IDF docs are so unclear, I ran into this head-long right at the outset when I wanted to test my first pieces of RMT code by feeding it a PWM output and it took me a while to understand what was going on).
  • The memory overrun with a packet longer than 64 pulses is detectable by the fact that the buffer doesn't end with a zero-length pulse (this can be done at the app level as well).

Since I mentioned that I added the status check, here's my diff:

--- a/modules/pins/rmt/esp32/rmt.c
+++ b/modules/pins/rmt/esp32/rmt.c
@@ -190,6 +190,12 @@ void xs_rmt_read(xsMachine *the){
        int bufferByteLength = xsmcGetArrayBufferLength(xsArg(0));
        data = (uint8_t*)xsmcToArrayBuffer(xsArg(0));
 
+       uint32_t status;
+       rmt_get_status(rmt->channel, &status);
+       if (status & (1<<28)) {
+               xsUnknownError("rx overrun");
+       }
+
        err = receive(rmt, data, bufferByteLength, &count, &phase);
        if (err == -1) xsRangeError("ringbuffer has too much data");
        xsResult = xsmcNewObject();
@phoddie
Copy link
Collaborator

phoddie commented Feb 14, 2023

The Espressif RMT I/O unit is a strange beast. It does a lot of useful stuff, but it never quite feels obvious to work with.

This module is pre-dates ECMA-419 and so doesn't follow that style. There's a lot that would be different if it did. For any large changes, reworking the implementation for that world would be my preference.

For a quick fix for your immediate problems, reporting the overrun as you propose seems OK (as much as I dislike throwing from read unless the caller made a mistake). An alternative could be to add a status or error property to the returned object. That wouldn't change the behavior for any existing use and would quietly propagate the information you want.

All that said, @acarle knows more about this code than I do. I defer to him.

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

2 participants