From 39df1b4b15ce68e75d9d0554055a1596be07cb23 Mon Sep 17 00:00:00 2001 From: Sola85 Date: Wed, 16 Jul 2025 11:39:40 +0200 Subject: [PATCH 1/6] enable dma in rmt only when necessary --- ports/espressif/common-hal/pulseio/PulseIn.c | 31 ++++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/ports/espressif/common-hal/pulseio/PulseIn.c b/ports/espressif/common-hal/pulseio/PulseIn.c index f7e500790562a..78d1ba3ac1af2 100644 --- a/ports/espressif/common-hal/pulseio/PulseIn.c +++ b/ports/espressif/common-hal/pulseio/PulseIn.c @@ -74,14 +74,20 @@ static bool _done_callback(rmt_channel_handle_t rx_chan, void common_hal_pulseio_pulsein_construct(pulseio_pulsein_obj_t *self, const mcu_pin_obj_t *pin, uint16_t maxlen, bool idle_state) { + + // Only use dma when necessary, since dma-rmt might not be available. + // If dma is not available, this will raise an error below. + bool use_dma = maxlen/2 > SOC_RMT_MEM_WORDS_PER_CHANNEL; + self->buffer = (uint16_t *)m_malloc_without_collect(maxlen * sizeof(uint16_t)); if (self->buffer == NULL) { m_malloc_fail(maxlen * sizeof(uint16_t)); } // We add one to the maxlen version to ensure that two symbols at lease are // captured because we may skip the first portion of a symbol. - self->raw_symbols_size = MIN(64, maxlen / 2 + 1) * sizeof(rmt_symbol_word_t); - self->raw_symbols = (rmt_symbol_word_t *)m_malloc_without_collect(self->raw_symbols_size); + self->raw_symbols_size = (maxlen / 2 + 1) * sizeof(rmt_symbol_word_t); + // RMT DMA mode cannot access PSRAM -> ensure raw_symbols is in internal ram + self->raw_symbols = (rmt_symbol_word_t *)port_malloc(self->raw_symbols_size, true); if (self->raw_symbols == NULL) { m_free(self->buffer); m_malloc_fail(self->raw_symbols_size); @@ -109,17 +115,30 @@ void common_hal_pulseio_pulsein_construct(pulseio_pulsein_obj_t *self, const mcu .clk_src = RMT_CLK_SRC_DEFAULT, // 2 us resolution so we can capture 65ms pulses. The RMT period is only 15 bits. .resolution_hz = 1000000 / 2, - .mem_block_symbols = SOC_RMT_MEM_WORDS_PER_CHANNEL, + .mem_block_symbols = use_dma ? self->raw_symbols_size : SOC_RMT_MEM_WORDS_PER_CHANNEL, + .flags.with_dma = use_dma; }; - // If we fail here, the buffers allocated above will be garbage collected. - CHECK_ESP_RESULT(rmt_new_rx_channel(&config, &self->channel)); + // If we fail here, the self->buffer will be garbage collected. + esp_err_t result = rmt_new_rx_channel(&config, &self->channel); + if (result != ESP_OK) { + port_free(self->raw_symbols); + if (result == ESP_ERR_NOT_SUPPORTED){ + mp_raise_ValueError_varg(MP_ERROR_TEXT("Invalid %q"), MP_QSTR_maxlen); + } else { + raise_esp_error(result); + } + } rmt_rx_event_callbacks_t rx_callback = { .on_recv_done = _done_callback }; rmt_rx_register_event_callbacks(self->channel, &rx_callback, self); rmt_enable(self->channel); - rmt_receive(self->channel, self->raw_symbols, self->raw_symbols_size, &rx_config); + result = rmt_receive(self->channel, self->raw_symbols, self->raw_symbols_size, &rx_config); + if (result != ESP_OK) { + port_free(self->raw_symbols); + raise_esp_error(result); + } } bool common_hal_pulseio_pulsein_deinited(pulseio_pulsein_obj_t *self) { From 4abc9125ab86b6197dc0fe870cf88d50d94746d5 Mon Sep 17 00:00:00 2001 From: Sola85 Date: Wed, 16 Jul 2025 12:56:38 +0200 Subject: [PATCH 2/6] Document limitations of PulseIn --- shared-bindings/pulseio/PulseIn.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/shared-bindings/pulseio/PulseIn.c b/shared-bindings/pulseio/PulseIn.c index d9e6bfd7aecc3..07383b3dcb1cb 100644 --- a/shared-bindings/pulseio/PulseIn.c +++ b/shared-bindings/pulseio/PulseIn.c @@ -33,6 +33,11 @@ //| :param bool idle_state: Idle state of the pin. At start and after `resume` //| the first recorded pulse will the opposite state from idle. //| +//| **Limitations**: The `maxlen` parameter is limited depending on the specific board: +//| - ESP32 and ESP32-S2: Maximum `maxlen` is 128. +//| - ESP32-S3 and ESP32-P4: The first `PulseIn` instance can use `maxlen` up to available RAM; all subsequent instances are limited to 96. +//| - All other ESP32 variants: Maximum `maxlen` is 96. +//| //| Read a short series of pulses:: //| //| import pulseio @@ -54,7 +59,7 @@ //| pulses.clear() //| //| # Resume with an 80 microsecond active pulse -//| pulses.resume(80)""" +//| pulses.resume(80) //| ... //| static mp_obj_t pulseio_pulsein_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *all_args) { From 9c3f0662d31563755907550f4cf70e9c29d755e2 Mon Sep 17 00:00:00 2001 From: Sola85 Date: Wed, 16 Jul 2025 22:28:13 +0200 Subject: [PATCH 3/6] Fix formatting --- ports/espressif/common-hal/pulseio/PulseIn.c | 8 ++++---- shared-bindings/pulseio/PulseIn.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ports/espressif/common-hal/pulseio/PulseIn.c b/ports/espressif/common-hal/pulseio/PulseIn.c index 78d1ba3ac1af2..fc566eba5c540 100644 --- a/ports/espressif/common-hal/pulseio/PulseIn.c +++ b/ports/espressif/common-hal/pulseio/PulseIn.c @@ -76,8 +76,8 @@ void common_hal_pulseio_pulsein_construct(pulseio_pulsein_obj_t *self, const mcu uint16_t maxlen, bool idle_state) { // Only use dma when necessary, since dma-rmt might not be available. - // If dma is not available, this will raise an error below. - bool use_dma = maxlen/2 > SOC_RMT_MEM_WORDS_PER_CHANNEL; + // If dma is not available, this will raise an error below. + bool use_dma = maxlen / 2 > SOC_RMT_MEM_WORDS_PER_CHANNEL; self->buffer = (uint16_t *)m_malloc_without_collect(maxlen * sizeof(uint16_t)); if (self->buffer == NULL) { @@ -116,13 +116,13 @@ void common_hal_pulseio_pulsein_construct(pulseio_pulsein_obj_t *self, const mcu // 2 us resolution so we can capture 65ms pulses. The RMT period is only 15 bits. .resolution_hz = 1000000 / 2, .mem_block_symbols = use_dma ? self->raw_symbols_size : SOC_RMT_MEM_WORDS_PER_CHANNEL, - .flags.with_dma = use_dma; + .flags.with_dma = use_dma }; // If we fail here, the self->buffer will be garbage collected. esp_err_t result = rmt_new_rx_channel(&config, &self->channel); if (result != ESP_OK) { port_free(self->raw_symbols); - if (result == ESP_ERR_NOT_SUPPORTED){ + if (result == ESP_ERR_NOT_SUPPORTED) { mp_raise_ValueError_varg(MP_ERROR_TEXT("Invalid %q"), MP_QSTR_maxlen); } else { raise_esp_error(result); diff --git a/shared-bindings/pulseio/PulseIn.c b/shared-bindings/pulseio/PulseIn.c index 07383b3dcb1cb..fdf4e0e5489f7 100644 --- a/shared-bindings/pulseio/PulseIn.c +++ b/shared-bindings/pulseio/PulseIn.c @@ -59,7 +59,7 @@ //| pulses.clear() //| //| # Resume with an 80 microsecond active pulse -//| pulses.resume(80) +//| pulses.resume(80)""" //| ... //| static mp_obj_t pulseio_pulsein_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *all_args) { From 90f1f5ddc1b0f63ea491cdb650627d518c17f727 Mon Sep 17 00:00:00 2001 From: Sola85 Date: Thu, 17 Jul 2025 10:29:21 +0200 Subject: [PATCH 4/6] Fix formatting --- ports/espressif/common-hal/pulseio/PulseIn.c | 6 +++--- shared-bindings/pulseio/PulseIn.c | 3 +-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/ports/espressif/common-hal/pulseio/PulseIn.c b/ports/espressif/common-hal/pulseio/PulseIn.c index fc566eba5c540..cddf6f5dffc6f 100644 --- a/ports/espressif/common-hal/pulseio/PulseIn.c +++ b/ports/espressif/common-hal/pulseio/PulseIn.c @@ -77,8 +77,8 @@ void common_hal_pulseio_pulsein_construct(pulseio_pulsein_obj_t *self, const mcu // Only use dma when necessary, since dma-rmt might not be available. // If dma is not available, this will raise an error below. - bool use_dma = maxlen / 2 > SOC_RMT_MEM_WORDS_PER_CHANNEL; - + bool use_dma = maxlen > 128; + self->buffer = (uint16_t *)m_malloc_without_collect(maxlen * sizeof(uint16_t)); if (self->buffer == NULL) { m_malloc_fail(maxlen * sizeof(uint16_t)); @@ -87,7 +87,7 @@ void common_hal_pulseio_pulsein_construct(pulseio_pulsein_obj_t *self, const mcu // captured because we may skip the first portion of a symbol. self->raw_symbols_size = (maxlen / 2 + 1) * sizeof(rmt_symbol_word_t); // RMT DMA mode cannot access PSRAM -> ensure raw_symbols is in internal ram - self->raw_symbols = (rmt_symbol_word_t *)port_malloc(self->raw_symbols_size, true); + self->raw_symbols = (rmt_symbol_word_t *)port_malloc(self->raw_symbols_size, use_dma); if (self->raw_symbols == NULL) { m_free(self->buffer); m_malloc_fail(self->raw_symbols_size); diff --git a/shared-bindings/pulseio/PulseIn.c b/shared-bindings/pulseio/PulseIn.c index fdf4e0e5489f7..f5a58def097b4 100644 --- a/shared-bindings/pulseio/PulseIn.c +++ b/shared-bindings/pulseio/PulseIn.c @@ -34,9 +34,8 @@ //| the first recorded pulse will the opposite state from idle. //| //| **Limitations**: The `maxlen` parameter is limited depending on the specific board: -//| - ESP32 and ESP32-S2: Maximum `maxlen` is 128. +//| - Most ESP32 variants: Maximum `maxlen` is 128. //| - ESP32-S3 and ESP32-P4: The first `PulseIn` instance can use `maxlen` up to available RAM; all subsequent instances are limited to 96. -//| - All other ESP32 variants: Maximum `maxlen` is 96. //| //| Read a short series of pulses:: //| From 90cd973ce9b6fb946db5c7788516da1b35d31bcc Mon Sep 17 00:00:00 2001 From: Sola85 Date: Thu, 17 Jul 2025 15:01:30 +0200 Subject: [PATCH 5/6] update documentation --- shared-bindings/pulseio/PulseIn.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/shared-bindings/pulseio/PulseIn.c b/shared-bindings/pulseio/PulseIn.c index f5a58def097b4..4f4af3ec60c7c 100644 --- a/shared-bindings/pulseio/PulseIn.c +++ b/shared-bindings/pulseio/PulseIn.c @@ -33,9 +33,9 @@ //| :param bool idle_state: Idle state of the pin. At start and after `resume` //| the first recorded pulse will the opposite state from idle. //| -//| **Limitations**: The `maxlen` parameter is limited depending on the specific board: -//| - Most ESP32 variants: Maximum `maxlen` is 128. -//| - ESP32-S3 and ESP32-P4: The first `PulseIn` instance can use `maxlen` up to available RAM; all subsequent instances are limited to 96. +//| **Limitations**: The `maxlen` parameter is limited depending on the specific board. +//| On most ESP32 variants the limit is 128. On ESP32-S3 and ESP32-P4 the first `PulseIn` instance +//| can use `maxlen` up to available RAM; all subsequent instances are limited to 128. //| //| Read a short series of pulses:: //| From f817c93e998aa66e3bbd6bc0b49e7e390755f063 Mon Sep 17 00:00:00 2001 From: Sola85 Date: Thu, 17 Jul 2025 15:01:57 +0200 Subject: [PATCH 6/6] fix whitespace --- shared-bindings/pulseio/PulseIn.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared-bindings/pulseio/PulseIn.c b/shared-bindings/pulseio/PulseIn.c index 4f4af3ec60c7c..1df5773114493 100644 --- a/shared-bindings/pulseio/PulseIn.c +++ b/shared-bindings/pulseio/PulseIn.c @@ -34,7 +34,7 @@ //| the first recorded pulse will the opposite state from idle. //| //| **Limitations**: The `maxlen` parameter is limited depending on the specific board. -//| On most ESP32 variants the limit is 128. On ESP32-S3 and ESP32-P4 the first `PulseIn` instance +//| On most ESP32 variants the limit is 128. On ESP32-S3 and ESP32-P4 the first `PulseIn` instance //| can use `maxlen` up to available RAM; all subsequent instances are limited to 128. //| //| Read a short series of pulses::