RMT refactoring based on IDF #6024

SuGlider · 2021-12-15T04:32:58Z

Summary

RMT HAL refactoring based on IDF.

Impact

Improves RMT by adding IDF v4.4 support.
Receiving RMT can handle any size of data.
rmtInit() has a new parameter - RxBufferSize - to hold any number of data when receiving RMT.
rmtWrite() has a new parameter - wait_tx_done - to block writing until sending all data.

Related links

fix #5905

me-no-dev · 2021-12-15T09:20:15Z

cores/esp32/esp32-hal-rmt.c

+
+// USE SOC_RMT_CHANNELS_PER_GROUP
+// LOOK to https://github.com/espressif/esp-idf/blob/master/components/hal/include/hal/rmt_types.h#L32
+


correct :) https://github.com/espressif/arduino-esp32/blob/master/tools/sdk/esp32/include/soc/esp32/include/soc/soc_caps.h#L195-L202

#include "soc/soc_caps.h" #define MAX_CHANNELS (SOC_RMT_GROUPS * SOC_RMT_CHANNELS_PER_GROUP) #define MAX_DATA_PER_CHANNEL SOC_RMT_MEM_WORDS_PER_CHANNEL #define MAX_DATA_PER_ITTERATION (MAX_DATA_PER_CHANNEL - 2)

It is then chip independent. We should try to have as few chips specific things as possible

me-no-dev · 2021-12-15T09:25:07Z

cores/esp32/esp32-hal-rmt.c

+   rmt_get_status(channel, &status);
+   rmt_get_source_clk(channel, &srcClk);
+
+   ESP_LOGD(tag, "Status for RMT channel %d", channel);


do not use ESP_LOGx in Arduino. Use log_x

me-no-dev · 2021-12-15T10:48:59Z

cores/esp32/esp32-hal-rmt.c

-    RMT.apb_conf.fifo_mask = 1;
+        rmt_config_t config = RMT_DEFAULT_ARD_CONFIG_RX(pin, channel, buffers);
+        ESP_ERROR_CHECK(rmt_config(&config));
+        ESP_ERROR_CHECK(rmt_driver_install(config.channel, 1024, 0)); //, rxBufferSize, 0));


rxBufferSize is ignored here?

Solved by keeping 1024 to all SoCs. Tested it with ESP32, S2 and C3.
One possible solution is to use default values in the function signature, but that would require changing to change esp32-hal-rmt.c to cpp instead.

me-no-dev · 2021-12-15T10:54:17Z

Overall very good, but here are a few things:

do not change the API, let's think how we can accommodate rxBufferSize while keeping init the same.
same goes for write. Add a new function rmtWriteBlocking
clean up all unused definitions and variables
convert the ones left to soc_caps (I gave example in a comment)

SuGlider · 2021-12-17T16:27:17Z

Overall very good, but here are a few things:

* do not change the API, let's think how we can accommodate `rxBufferSize` while keeping init the same.

* same goes for write. Add a new function `rmtWriteBlocking`

* clean up all unused definitions and variables

* convert the ones left to `soc_caps` (I gave example in a comment)

OK. All done.
rmtInit() is back to the original. I just removed rxBufferSize and it is being initialized with 1024 as buffer size always.
1024 works fine in all the SoCs - even with RMT_MEM_512 in ESP32 for RX.

me-no-dev · 2021-12-20T11:09:05Z

cores/esp32/esp32-hal-rmt.c

-        return _rmtSendOnce(rmt, data, size, false);
-    }
+    RMT_MUTEX_LOCK(channel);
+    ESP_ERROR_CHECK(rmt_tx_stop(channel));


ESP_ERROR_CHECK is also an IDF API that boils down to ESP_LOGx

Actually ESP_ERROR_CHECK boils down directly to esp_rom_printf(), which seems to be a ROM function, thus, it should be fine for Arduino.
What do you say? Let's keep it?
I like the error message with all necessary details about code line, function name and so on.

https://github.com/espressif/esp-idf/blob/master/components/esp_common/include/esp_err.h#L121-L127
https://github.com/espressif/esp-idf/blob/master/components/esp_system/esp_err.c#L23-L33

what it looks like is that it will abort and produce some IDF formatted message. Let's stick to Arduino constructs and log_e if not ESP_OK

OK. Done.
I removed ESP_ERROR_CHECK() from IDF function calls that only checked IDF parameters and left it on the function call that really could return error on IDF operations.

Jason2866 · 2021-12-21T14:33:47Z

Compile with Arduino Lib Builder does fail with this PR

SuGlider · 2021-12-21T22:11:10Z

Compile with Arduino Lib Builder does fail with this PR

With IDF 4.4?

Could you please share the log?

Jason2866 · 2021-12-21T22:28:23Z

@SuGlider Here is the log. Yes with IDF4.4. Lib Builder https://github.com/Jason2866/esp32-arduino-lib-builder
logs_701.zip

SuGlider · 2021-12-22T00:13:27Z

@SuGlider Here is the log. Yes with IDF4.4. Lib Builder https://github.com/Jason2866/esp32-arduino-lib-builder logs_701.zip

Thanks @Jason2866!
I just submitted a fixing PR.

@me-no-dev @VojtechBartoska -
I think that the CI may not have the same compilation options as in the Lib Builder, otherwise this problem should have been caught in the CI testing. Could you please check it?

RMT refactoring based on IDF

c0ba4d7

SuGlider requested a review from me-no-dev December 15, 2021 04:44

SuGlider self-assigned this Dec 15, 2021

me-no-dev reviewed Dec 15, 2021

View reviewed changes

SuGlider and others added 3 commits December 17, 2021 12:58

Merge branch 'espressif:master' into rmt-refactoring

e0a36c5

RMT refactoring based on IDF

ae01a6a

RMT refactoring based on IDF

aa69265

SuGlider mentioned this pull request Dec 17, 2021

RMT: using rmt_write_sample async with all 8 channels has signficant staggered starts of last few channels #2885

Closed

me-no-dev reviewed Dec 20, 2021

View reviewed changes

VojtechBartoska added this to the 2.0.2 milestone Dec 20, 2021

SuGlider and others added 3 commits December 20, 2021 20:28

Merge branch 'master' into rmt-refactoring

fc41032

Merge branch 'espressif:master' into rmt-refactoring

e3a062e

RMT refactoring based on IDF

0d49840

me-no-dev merged commit 7cf1623 into espressif:master Dec 21, 2021

SuGlider mentioned this pull request Dec 22, 2021

RMT Lib Builder Fix #6051

Closed

SuGlider mentioned this pull request Dec 22, 2021

Fixes Lib Builder compiling errors #6052

Merged

Jason2866 mentioned this pull request Jan 30, 2022

Resets with IDF 4.4/Arduino Core 2.x platform on network request wled/WLED#2518

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RMT refactoring based on IDF #6024

RMT refactoring based on IDF #6024

SuGlider commented Dec 15, 2021

me-no-dev Dec 15, 2021

SuGlider Dec 17, 2021

me-no-dev Dec 15, 2021

SuGlider Dec 17, 2021

me-no-dev Dec 15, 2021

SuGlider Dec 17, 2021

me-no-dev commented Dec 15, 2021

SuGlider commented Dec 17, 2021 •

edited

Loading

me-no-dev Dec 20, 2021

SuGlider Dec 20, 2021 •

edited

Loading

me-no-dev Dec 21, 2021

SuGlider Dec 21, 2021

Jason2866 commented Dec 21, 2021

SuGlider commented Dec 21, 2021

Jason2866 commented Dec 21, 2021 •

edited

Loading

SuGlider commented Dec 22, 2021


		// USE SOC_RMT_CHANNELS_PER_GROUP
		// LOOK to https://github.com/espressif/esp-idf/blob/master/components/hal/include/hal/rmt_types.h#L32

RMT refactoring based on IDF #6024

RMT refactoring based on IDF #6024

Conversation

SuGlider commented Dec 15, 2021

Summary

Impact

Related links

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

me-no-dev commented Dec 15, 2021

SuGlider commented Dec 17, 2021 • edited Loading

Choose a reason for hiding this comment

SuGlider Dec 20, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jason2866 commented Dec 21, 2021

SuGlider commented Dec 21, 2021

Jason2866 commented Dec 21, 2021 • edited Loading

SuGlider commented Dec 22, 2021

SuGlider commented Dec 17, 2021 •

edited

Loading

SuGlider Dec 20, 2021 •

edited

Loading

Jason2866 commented Dec 21, 2021 •

edited

Loading