espressif / esp-idf

Espressif IoT Development Framework. Official development framework for Espressif SoCs.
Apache License 2.0
13.49k stars 7.26k forks source link

I2S ADC channel order is unexpected/doubled up (IDFGH-1408) #3686

Closed detly closed 1 year ago

detly commented 5 years ago

Environment

Problem Description

I'm trying to use the I2S direct-from-ADC functionality in my firmware, with multiple channels.

Note: I have modified the IDF code with the following changes:

  1. Exposed (removed static, added to header) adc_set_i2s_data_len() from components/driver/rtc_module.c via components/driver/include/driver/adc.h
  2. Exposed adc_set_i2s_data_pattern() (same).
  3. Removed the call to _i2s_adc_mode_recover() in i2s_adc_enable() in components/driver/i2s.c

These changes are necessary to allow connecting multiple ADCs to the I2S module (see #2980 , @montaguk's comment).

I have some example code that simply reads the I2S data as it is generated and extracts the channel it originated from. The Technical Manual says:

ESP32 supports up to a 12-bit SAR ADC resolution. The 16-bit data in DMA is composed of the ADC result and some necessary information related to the scanning mode:

  • For single mode, only 4-bit information on channel selection is added.

So this is done with code like:

(data & 0xF0) >> 4

...applied to every second char, offset by one (to get the MSB).

But when I actually run my code (below), I do not see alternating channels eg 5, 6, 5, 6. I see channels doubled up and sometimes skipped eg. 5, 6, 6, 5, 5, 6, 6, 5, 6.

Expected Behavior

The Technical Manual says:

The pattern tables contain the measurement rules mentioned above. Each table has 16 items which store information on channel selection, resolution and attenuation. When scanning starts, the controller reads measurement rules one-by-one from a pattern table. For each controller the scanning sequence includes 16 different rules at most, before repeating itself.

So given the setup:

    adc_set_i2s_data_len(ADC_UNIT_1, 2);
    adc_set_i2s_data_pattern(ADC_UNIT_1, 0, ADC_CHANNEL_6, ADC_WIDTH_BIT_12, ADC_ATTEN_DB_11);
    adc_set_i2s_data_pattern(ADC_UNIT_1, 1, ADC_CHANNEL_5, ADC_WIDTH_BIT_12, ADC_ATTEN_DB_11);

I would expect the I2S-triggered scanning to simply go 6, 5, 6, 5, 6, 5, and the resulting I2S data to have that same pattern.

Actual Behavior

My code prints:

I (30673) i2s-test: CH: 6 5 5 6 6 5 5 6
I (30673) i2s-test: CH: 6 5 5 6 6 5 5 6
I (30673) i2s-test: CH: 6 5 5 6 6 5 5 6
I (30683) i2s-test: CH: 6 5 5 6 6 5 5 6
I (30683) i2s-test: CH: 6 5 5 6 6 5 5 6
I (30693) i2s-test: CH: 6 5 6 6 5 6 6 5
I (30693) i2s-test: CH: 6 6 5 6 5 5 6 5
I (30703) i2s-test: CH: 5 6 6 5 5 6 6 5

Note the repetition of each channel.

Steps to repropduce

Build and run the code I link to with idf.py build and idf.py flash monitor. Don't forget to make the modifications I mention above.

Code to reproduce this issue

I have put the entire example project on Gitlab here. But here is the essential setup code for the peripherals:

static void app_i2s_adc_init()
{
    int i2s_num = I2S_NUM_0;

    i2s_config_t i2s_config = {
        .mode = I2S_MODE_MASTER | I2S_MODE_RX | I2S_MODE_ADC_BUILT_IN,
        .sample_rate =  24000,
        .bits_per_sample = 16,
        .communication_format = I2S_COMM_FORMAT_I2S_MSB,
        .channel_format = I2S_CHANNEL_FMT_RIGHT_LEFT,
        .intr_alloc_flags = 0,
        .dma_buf_count = 2,
        .dma_buf_len = 1024,
        .use_apll = 1,
        .fixed_mclk = 0
    };

    i2s_driver_install(i2s_num, &i2s_config, 0, NULL);
    i2s_set_adc_mode(ADC_UNIT_1, ADC1_CHANNEL_6);

    adc_set_i2s_data_len(ADC_UNIT_1, 2);
    adc_set_i2s_data_pattern(ADC_UNIT_1, 0, ADC_CHANNEL_6, ADC_WIDTH_BIT_12, ADC_ATTEN_DB_11);
    adc_set_i2s_data_pattern(ADC_UNIT_1, 1, ADC_CHANNEL_5, ADC_WIDTH_BIT_12, ADC_ATTEN_DB_11);
}

I would also like to know what the proper communication_format is to use for direct-from-ADC operation, because it does seem to affect the result.

Debug Logs

Here's the log of the first set of reads:

I (0) cpu_start: App cpu up.
I (225) heap_init: Initializing. RAM available for dynamic allocation:
I (232) heap_init: At 3FFAE6E0 len 00001920 (6 KiB): DRAM
I (238) heap_init: At 3FFB2F10 len 0002D0F0 (180 KiB): DRAM
I (244) heap_init: At 3FFE0440 len 00003AE0 (14 KiB): D/IRAM
I (250) heap_init: At 3FFE4350 len 0001BCB0 (111 KiB): D/IRAM
I (257) heap_init: At 400883B0 len 00017C50 (95 KiB): IRAM
I (263) cpu_start: Pro cpu start user code
I (281) cpu_start: Starting scheduler on PRO CPU.
I (0) cpu_start: Starting scheduler on APP CPU.
I (283) I2S: DMA Malloc info, datalen=blocksize=4092, dma_buf_count=2
I (283) I2S: APLL: Req RATE: 24000, real rate: 11999.990, BITS: 16, CLKM: 1, BCK_M: 8, MCLK: 3071997.500, SCLK: 383999.687500, diva: 1, divb: 0
I (303) i2s-test: repeat #0
I (303) i2s-test: enabling I2S ADC
I (303) I2S: APLL: Req RATE: 24000, real rate: 11999.990, BITS: 16, CLKM: 1, BCK_M: 8, MCLK: 3071997.500, SCLK: 383999.687500, diva: 1, divb: 0
I (323) i2s-test: CH: 5 6 6 5 5 6 6 5
I (323) i2s-test: CH: 5 6 6 5 5 6 6 5
I (323) i2s-test: CH: 5 6 6 5 5 6 6 5
I (333) i2s-test: CH: 6 6 5 6 6 5 5 6
I (333) i2s-test: CH: 6 5 5 6 6 5 5 6
I (343) i2s-test: CH: 6 5 5 6 6 5 5 6
I (343) i2s-test: CH: 6 5 5 6 6 5 5 6
I (353) i2s-test: CH: 6 5 5 6 6 5 5 6
I (353) i2s-test: disabling I2S ADC

Happy to provide more information if it helps.

davydnorris commented 5 years ago

I think you have a problem with your rate - the real rate is half your desired rate.

It may be just the recent bug around rate calculation but I would double check that first

detly commented 5 years ago

@davydnorris I was confused by that too. Is it in this issue tracker?

(I'm not sure that accounts for it anyway, because the I2S/ADC interaction doesn't need to know what the desired sample rate is. It just gets the data from the ADC reads as triggered by the WS line, whatever the sample rate is.)

davydnorris commented 5 years ago

See https://github.com/espressif/esp-idf/pull/3648, but I think maybe you have some other problem as the errors here are not half.

I may be completely off track here as I thought you were reading an external ADC sensor, but you're using the in-built one. However I would chase that rate problem down - it seems very weird

detly commented 5 years ago

Verified that it's present in master.

detly commented 5 years ago

@davydnorris I filed a separate bug for that, #3692 .

detly commented 5 years ago

Running this on a commit where the sample rate problem is not present still shows the channel ordering issue.

detly commented 5 years ago

So avoiding the APLL issue, I have verified with an oscilloscope that the WS signal is running at 24kHz as expected. The channel ordering is still doubled up.

detly commented 5 years ago

@davydnorris Turns out the weird sample rate being reported is simply a side effect of the fact that the clocks work a bit differently in ADC mode (WS is always half the frequency of the BCK). The debug print still calculates everything assuming WS = BCK/(bits*channels). But the WS signal is actually correct as long as the PLL D2 is used and not the APLL.

davydnorris commented 5 years ago

thanks for digging into this - I have never used the built in ADC on the ESP32 so this is all news to me!

X-Ryl669 commented 5 years ago

Have you seen #3170 ? If you use apll, you need do also set a fixed mclk:

   .use_apll = 1,
   .fixed_mclk= 2000000,
detly commented 5 years ago

@x-ryl669 yep, see #3692. Problem exists regardless of APLL.

xiongyumail commented 4 years ago

@detly I found that the adc channel is normal when using mono.

i2s_set_clk(i2s_num, SAMPLE_RATE, I2S_BITS_PER_SAMPLE_16BIT, I2S_CHANNEL_MONO);

image

image

X-Ryl669 commented 4 years ago

Why is it swapping values in the middle (5 6 5 6 then 6 5 6 5) ?

detly commented 4 years ago

I found that the adc channel is normal when using mono.

Look closely, there's a doubled-up channel 6 read in the middle of your output.

At any rate, I can't contribute much to this issue any more. I scrapped the project and blacklisted ESP due to multiple not-fit-for-purpose issues such as this one.

ddomax commented 4 years ago

@detly I found that the adc channel is normal when using mono.

i2s_set_clk(i2s_num, SAMPLE_RATE, I2S_BITS_PER_SAMPLE_16BIT, I2S_CHANNEL_MONO);
  • 2 channel:

image

  • 8 channel:

image

When the I2S bus works in I2S_CHANNEL_FMT_RIGHT_LEFT mode, each data will be repeated once to cause the data rate to double, but the actual sampling rate is unchanged. However, when using SAR ADC1 to perform multi-channel scanning according to the specified pattern table, as in your test results, there will be random channel sequence changes, or some channels will be repeatedly scanned suddenly. This problem causes non-uniformity in the sampling interval, and it is clear that the chip does not work according to the characteristics given in TRM.

To reproduce this problem, code attched to the link https://www.esp32.com/viewtopic.php?t=8335&start=10 can be used. Here is a part of the output of the example code when the problem occurs: ch 6 0.160000v ch 7 0.160000v ch 0 0.160000v ch 3 0.160000v ch 3 0.160000v ch 7 0.160000v ch 7 0.208308v ch 0 0.197573v ch 3 0.160000v ch 6 0.229011v ch 7 0.202173v ch 0 0.188371v ch 3 0.160000v ch 6 0.208308v ch 7 0.196806v ch 0 0.179170v


i2s_adc_scale.tar.gz

X-Ryl669 commented 4 years ago

I've read in one of the issues (don't remember which) here that the HW sampler needs 2 clocks to restart sampling when it's done a batch (instead of restarting immediately), so it's always missing one sample, that's probably the reason for the repetition, in fact, it has missed one value.

If you had a non constant voltage source, you'd see the effect.

detly commented 4 years ago

I've read in one of the issues (don't remember which) here that the HW sampler needs 2 clocks to restart sampling when it's done a batch (instead of restarting immediately), so it's always missing one sample, that's probably the reason for the repetition, in fact, it has missed one value.

I read this too, I think it was a PR, not an issue. If this is a case, it absolutely needs to be documented front and centre in the TRM and the API, because it is totally at odds with every other embedded system out there, and means that the ADC-to-I2S functionality — an advertised feature of this system — is more or less useless for a great many applications. On top of that, letting engineers waste this much time and effort trying to debug it, instead of simply being honest about the limitation, is unacceptable.

If you had a non constant voltage source, you'd see the effect.

I actually do, I see it quite clearly with a sine tone generator or audio calibration signal.

detly commented 4 years ago

@X-Ryl669 was it this comment on PR #1991?

X-Ryl669 commented 4 years ago

Yes, exactly. Thanks for digging this out again.

Younes-SadatNejad commented 4 years ago

Hello, I hope you are doing well. I believe that what is being discussed in here is the underlying issue of what I have posted:

https://github.com/espressif/arduino-esp32/issues/3939

Could you kindly please take a look at this issue and let me know what you think? My knowledge on I2S and firmware are very limited, and so I`m not sure if this is the same issue and if there is a solution for it?

Thank you

kelvincesar commented 3 years ago

@detly About the modifications that you've done to sample multiple channels, does it is still possible to do it on current esp32 firmware?

giPeteR commented 3 years ago

This actually helped a lot:

I've read in one of the issues (don't remember which) here that the HW sampler needs 2 clocks to restart sampling when it's done a batch (instead of restarting immediately), so it's always missing one sample, that's probably the reason for the repetition, in fact, it has missed one value.

But when I saw it I thought of tossing everything out the (closed) window since it seemed like a HW limitation. and hundreds of hours down the drain. Now it's actually working here.

X-Ryl669 commented 3 years ago

Thanks for explaining your engineering dissection. I'm eager to see the actual code to make it work flawlessly.