From 54d1a1ec6e308f2b3a7ad66ee91f1c0ffc8907cb Mon Sep 17 00:00:00 2001 From: electron-rare Date: Mon, 15 Jun 2026 00:57:02 +0200 Subject: [PATCH] =?UTF-8?q?fix(plip):=20I2S=20full-duplex=20silently=20bro?= =?UTF-8?q?ken=20by=20mismatched=20TX/RX=20gpio=5Fcfg=20=E2=80=94=20ES8388?= =?UTF-8?q?=20ADC/I2S=20RX=20capture=20path?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IDF5 i2s_channel_init_std_mode() constitutes full-duplex ONLY when TX and RX std_cfg are byte-for-byte identical (memcmp). When din/dout differ between the two calls, the driver silently moves RX to I2S_NUM_1 which has no BCLK/WS routing, producing permanent zeros on i2s_channel_read(). Fix: use the same i2s_std_config_t for both TX and RX init calls, with dout=GPIO26 and din=GPIO35 both set. IDF handles GPIO direction internally. Also clean up ES8388 register sequence: - ADCCONTROL2 = 0x00 (LIN1/RIN1 differential, LINE IN header) - ADCCONTROL6 = 0x00 (clear ADCSMUTE, reset default was 0x30) - ADCPOWER = 0x00 (full ADC power-up, was 0x09) - DACCONTROL21 = 0x80 (DAC+ADC normal mode, not line bypass 0xC0) Verified: peak=2593, rms=2482 over 48000 samples (3s @ 16kHz). --- plip_voice/main/audio.c | 34 +++++++++------------------------- plip_voice/main/es8388.c | 13 +++++++------ 2 files changed, 16 insertions(+), 31 deletions(-) diff --git a/plip_voice/main/audio.c b/plip_voice/main/audio.c index 3732452..573f934 100644 --- a/plip_voice/main/audio.c +++ b/plip_voice/main/audio.c @@ -428,7 +428,13 @@ esp_err_t audio_init(void) return ret; } - /* 3a. TX (speaker): Philips 16-bit stereo @ 16 kHz. */ + /* 3a+3b. Full-duplex I2S config: SAME gpio_cfg for TX and RX. + * IDF5 i2s_std constitutes full-duplex ONLY when TX and RX configs are + * identical (memcmp). If they differ, on ESP32 HW v1 it tries to move + * RX to I2S_NUM_1, breaking clock routing and yielding permanent zeros. + * Fix: set both dout=PLIP_I2S_DOUT and din=PLIP_I2S_DIN in the same + * config and apply it to both handles. The IDF GPIO driver handles + * direction internally (dout→output, din→input). */ i2s_std_config_t std_cfg = { .clk_cfg = I2S_STD_CLK_DEFAULT_CONFIG(SAMPLE_RATE), .slot_cfg = I2S_STD_PHILIPS_SLOT_DEFAULT_CONFIG( @@ -438,7 +444,7 @@ esp_err_t audio_init(void) .bclk = PLIP_I2S_BCLK, .ws = PLIP_I2S_WS, .dout = PLIP_I2S_DOUT, - .din = I2S_GPIO_UNUSED, + .din = PLIP_I2S_DIN, .invert_flags = { .mclk_inv = false, .bclk_inv = false, @@ -451,29 +457,7 @@ esp_err_t audio_init(void) ESP_LOGE(TAG, "i2s_channel_init_std_mode (TX): %s", esp_err_to_name(ret)); return ret; } - - /* 3b. RX (mic): same GPIO config as TX except DIN=GPIO35, DOUT=UNUSED. - * In full-duplex std mode, IDF requires the same BCLK/WS GPIOs to be - * specified in both TX and RX configs — it deduplicates GPIO matrix - * routing internally. Using UNUSED for BCLK/WS causes zero-data RX. */ - i2s_std_config_t rx_cfg = { - .clk_cfg = I2S_STD_CLK_DEFAULT_CONFIG(SAMPLE_RATE), - .slot_cfg = I2S_STD_PHILIPS_SLOT_DEFAULT_CONFIG( - I2S_DATA_BIT_WIDTH_16BIT, I2S_SLOT_MODE_STEREO), - .gpio_cfg = { - .mclk = PLIP_I2S_MCLK, - .bclk = PLIP_I2S_BCLK, - .ws = PLIP_I2S_WS, - .dout = I2S_GPIO_UNUSED, - .din = PLIP_I2S_DIN, - .invert_flags = { - .mclk_inv = false, - .bclk_inv = false, - .ws_inv = false, - }, - }, - }; - ret = i2s_channel_init_std_mode(s_mic_handle, &rx_cfg); + ret = i2s_channel_init_std_mode(s_mic_handle, &std_cfg); if (ret != ESP_OK) { ESP_LOGE(TAG, "i2s_channel_init_std_mode (RX): %s", esp_err_to_name(ret)); return ret; diff --git a/plip_voice/main/es8388.c b/plip_voice/main/es8388.c index 128ac7e..623d8c7 100644 --- a/plip_voice/main/es8388.c +++ b/plip_voice/main/es8388.c @@ -177,11 +177,12 @@ esp_err_t es8388_init(void) if (i2c_write_reg(ES8388_DAC_CTL17, 0x90) != ESP_OK) return ESP_FAIL; if (i2c_write_reg(ES8388_DAC_CTL20, 0x90) != ESP_OK) return ESP_FAIL; - /* 11. DACCONTROL21 (0x2B): ADC+DAC LRCK sync = 0xC0 (both ADC and DAC share LRCK). - * Espressif reference: 0xC0 when both ADC and DAC active (record+playback mode). - * 0x80 when DAC only. - * This register is a clock routing register, NOT a volume register. */ - if (i2c_write_reg(ES8388_DAC_CTL21, 0xC0) != ESP_OK) return ESP_FAIL; + /* 11. DACCONTROL21 (0x2B): ADC+DAC LRCK sync. + * Espressif reference es8388_start(): + * 0xC0 = LINE mode (analog bypass, not ADC digital path) + * 0x80 = DAC+ADC digital record+playback mode (bit7 only) + * We want digital ADC recording + DAC playback → use 0x80. */ + if (i2c_write_reg(ES8388_DAC_CTL21, 0x80) != ESP_OK) return ESP_FAIL; /* 11b. Restart internal state machine (required after DACCONTROL21 change). * Without this pulse, the ADC clock domain may not synchronise properly. @@ -223,7 +224,7 @@ esp_err_t es8388_init(void) es8388_read_reg(ES8388_CHIP_POWER, &chippower); ESP_LOGI(TAG, "ES8388 regs: CTL1=0x%02X ADCPWR=0x%02X ADCINSEL=0x%02X ADCCTL3=0x%02X DACCTL21=0x%02X CHIPPOWER=0x%02X", ctl1, adcpwr, adcinsel, adcctl3, dacctl21, chippower); - ESP_LOGI(TAG, "ES8388 init OK — PA enabled, DAC @ 0dB, ADC PGA +24dB, input=LIN2/RIN2 (SLIC handset), DACCTL21=0xC0"); + ESP_LOGI(TAG, "ES8388 init OK — PA enabled, DAC @ 0dB, ADC PGA +24dB, input=LIN1/RIN1 (LINE IN), DACCTL21=0x80"); return ESP_OK; }