From f964a2c6f7bc51017759d3c5f25cd63b134dfc91 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Wed, 2 Mar 2022 20:15:56 -0300 Subject: [PATCH 1/3] Fixes rmtDeinit() and tests RX/TX before operations --- cores/esp32/esp32-hal-rmt.c | 88 ++++++++++++++++++++++++++++--------- 1 file changed, 67 insertions(+), 21 deletions(-) diff --git a/cores/esp32/esp32-hal-rmt.c b/cores/esp32/esp32-hal-rmt.c index 9c619eea2c8..ea4cc8dc61b 100644 --- a/cores/esp32/esp32-hal-rmt.c +++ b/cores/esp32/esp32-hal-rmt.c @@ -96,6 +96,7 @@ struct rmt_obj_s void * arg; TaskHandle_t rxTaskHandle; bool rx_completed; + bool tx_not_rx; }; /** @@ -109,15 +110,15 @@ static xSemaphoreHandle g_rmt_objlocks[MAX_CHANNELS] = { }; static rmt_obj_t g_rmt_objects[MAX_CHANNELS] = { - { false, NULL, 0, 0, 0, NULL, NULL, NULL, NULL, true}, - { false, NULL, 0, 0, 0, NULL, NULL, NULL, NULL, true}, - { false, NULL, 0, 0, 0, NULL, NULL, NULL, NULL, true}, - { false, NULL, 0, 0, 0, NULL, NULL, NULL, NULL, true}, + { false, NULL, 0, 0, 0, NULL, NULL, NULL, NULL, true, true}, + { false, NULL, 0, 0, 0, NULL, NULL, NULL, NULL, true, true}, + { false, NULL, 0, 0, 0, NULL, NULL, NULL, NULL, true, true}, + { false, NULL, 0, 0, 0, NULL, NULL, NULL, NULL, true, true}, #if MAX_CHANNELS > 4 - { false, NULL, 0, 0, 0, NULL, NULL, NULL, NULL, true}, - { false, NULL, 0, 0, 0, NULL, NULL, NULL, NULL, true}, - { false, NULL, 0, 0, 0, NULL, NULL, NULL, NULL, true}, - { false, NULL, 0, 0, 0, NULL, NULL, NULL, NULL, true}, + { false, NULL, 0, 0, 0, NULL, NULL, NULL, NULL, true, true}, + { false, NULL, 0, 0, 0, NULL, NULL, NULL, NULL, true, true}, + { false, NULL, 0, 0, 0, NULL, NULL, NULL, NULL, true, true}, + { false, NULL, 0, 0, 0, NULL, NULL, NULL, NULL, true, true}, #endif }; @@ -258,6 +259,10 @@ bool rmtSetCarrier(rmt_obj_t* rmt, bool carrier_en, bool carrier_level, uint32_t if (!rmt || low > 0xFFFF || high > 0xFFFF) { return false; } + if (!rmt->tx_not_rx) { + log_e("Can't write on a RX RMT Channel"); + return false; + } size_t channel = rmt->channel; RMT_MUTEX_LOCK(channel); @@ -271,6 +276,10 @@ bool rmtSetFilter(rmt_obj_t* rmt, bool filter_en, uint32_t filter_level) if (!rmt || filter_level > 0xFF) { return false; } + if (rmt->tx_not_rx) { + log_e("Can't read on a TX RMT Channel"); + return false; + } size_t channel = rmt->channel; RMT_MUTEX_LOCK(channel); @@ -284,6 +293,10 @@ bool rmtSetRxThreshold(rmt_obj_t* rmt, uint32_t value) if (!rmt || value > 0xFFFF) { return false; } + if (rmt->tx_not_rx) { + log_e("Can't read on a TX RMT Channel"); + return false; + } size_t channel = rmt->channel; RMT_MUTEX_LOCK(channel); @@ -306,14 +319,17 @@ bool rmtDeinit(rmt_obj_t *rmt) RMT_MUTEX_LOCK(rmt->channel); // force stopping rmt processing - rmt_rx_stop(rmt->channel); - rmt_tx_stop(rmt->channel); + if (rmt->tx_not_rx) { + rmt_tx_stop(rmt->channel); + } else { + rmt_rx_stop(rmt->channel); + if(rmt->rxTaskHandle){ + vTaskDelete(rmt->rxTaskHandle); + rmt->rxTaskHandle = NULL; + } + } - if(rmt->rxTaskHandle){ - vTaskDelete(rmt->rxTaskHandle); - rmt->rxTaskHandle = NULL; - } - rmt_driver_uninstall(rmt->channel); + rmt_driver_uninstall(rmt->channel); size_t from = rmt->channel; size_t to = rmt->buffers + rmt->channel; @@ -330,6 +346,7 @@ bool rmtDeinit(rmt_obj_t *rmt) #if !CONFIG_DISABLE_HAL_LOCKS if(g_rmt_objlocks[from] != NULL) { vSemaphoreDelete(g_rmt_objlocks[from]); + g_rmt_objlocks[from] = NULL; } #endif @@ -341,7 +358,10 @@ bool rmtLoop(rmt_obj_t* rmt, rmt_data_t* data, size_t size) if (!rmt) { return false; } - + if (!rmt->tx_not_rx) { + log_e("Can't write on a RX RMT Channel"); + return false; + } int channel = rmt->channel; RMT_MUTEX_LOCK(channel); rmt_tx_stop(channel); @@ -356,7 +376,10 @@ bool rmtWrite(rmt_obj_t* rmt, rmt_data_t* data, size_t size) if (!rmt) { return false; } - + if (!rmt->tx_not_rx) { + log_e("Can't write on a RX RMT Channel"); + return false; + } int channel = rmt->channel; RMT_MUTEX_LOCK(channel); rmt_tx_stop(channel); @@ -371,7 +394,10 @@ bool rmtWriteBlocking(rmt_obj_t* rmt, rmt_data_t* data, size_t size) if (!rmt) { return false; } - + if (!rmt->tx_not_rx) { + log_e("Can't write on a RX RMT Channel"); + return false; + } int channel = rmt->channel; RMT_MUTEX_LOCK(channel); rmt_tx_stop(channel); @@ -386,7 +412,10 @@ bool rmtReadData(rmt_obj_t* rmt, uint32_t* data, size_t size) if (!rmt) { return false; } - + if (rmt->tx_not_rx) { + log_e("Can't read on a TX RMT Channel"); + return false; + } rmtReadAsync(rmt, (rmt_data_t*) data, size, NULL, false, 0); return true; } @@ -397,6 +426,10 @@ bool rmtBeginReceive(rmt_obj_t* rmt) if (!rmt) { return false; } + if (rmt->tx_not_rx) { + log_e("Can't read on a TX RMT Channel"); + return false; + } int channel = rmt->channel; RMT_MUTEX_LOCK(channel); @@ -421,6 +454,10 @@ bool rmtRead(rmt_obj_t* rmt, rmt_rx_data_cb_t cb, void * arg) if (!rmt || !cb) { return false; } + if (rmt->tx_not_rx) { + log_e("Can't read on a TX RMT Channel"); + return false; + } int channel = rmt->channel; rmt->arg = arg; @@ -449,8 +486,12 @@ bool rmtEnd(rmt_obj_t* rmt) int channel = rmt->channel; RMT_MUTEX_LOCK(channel); - rmt_rx_stop(channel); - rmt->rx_completed = true; + if (rmt->tx_not_rx) { + rmt_tx_stop(channel); + } else { + rmt_rx_stop(channel); + rmt->rx_completed = true; + } RMT_MUTEX_UNLOCK(channel); return true; } @@ -460,6 +501,10 @@ bool rmtReadAsync(rmt_obj_t* rmt, rmt_data_t* data, size_t size, void* eventFlag if (!rmt) { return false; } + if (rmt->tx_not_rx) { + log_e("Can't read on a TX RMT Channel"); + return false; + } int channel = rmt->channel; // No limit on size with IDF ;-) @@ -568,6 +613,7 @@ rmt_obj_t* rmtInit(int pin, bool tx_not_rx, rmt_reserve_memsize_t memsize) rmt->data_size = 0; rmt->rx_completed = false; rmt->events = NULL; + rmt->tx_not_rx = tx_not_rx; #if !CONFIG_DISABLE_HAL_LOCKS if(g_rmt_objlocks[channel] == NULL) { From 81c567f9259059f0c437306ca4bebf2d6e2ac506 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Thu, 3 Mar 2022 09:04:44 -0300 Subject: [PATCH 2/3] Optimizes final binary size --- cores/esp32/esp32-hal-rmt.c | 79 ++++++++++++++----------------------- 1 file changed, 29 insertions(+), 50 deletions(-) diff --git a/cores/esp32/esp32-hal-rmt.c b/cores/esp32/esp32-hal-rmt.c index ea4cc8dc61b..91ac5df73ad 100644 --- a/cores/esp32/esp32-hal-rmt.c +++ b/cores/esp32/esp32-hal-rmt.c @@ -249,6 +249,25 @@ static bool _rmtCreateRxTask(rmt_obj_t* rmt) return true; } +// Helper function to test if a RMT channel is correctly assigned to TX or RX, issuing a error message if necessary +// return true when it is correctly assigned, false otherwise +static bool _rmtCheckTXnotRX(rmt_obj_t* rmt, bool tx_not_rx) +{ + if (!rmt) { // also returns false on NULL + return false; + } + + if (rmt->tx_not_rx == tx_not_rx) { // matches expected RX/TX channel + return true; + } + + if (tx_not_rx) { // expected TX channel + log_e("Can't write on a RX RMT Channel"); + } else{ // expected RX channel + log_e("Can't read on a TX RMT Channel"); + } + return false; // missmatched +} /** * Public method definitions @@ -256,11 +275,7 @@ static bool _rmtCreateRxTask(rmt_obj_t* rmt) bool rmtSetCarrier(rmt_obj_t* rmt, bool carrier_en, bool carrier_level, uint32_t low, uint32_t high) { - if (!rmt || low > 0xFFFF || high > 0xFFFF) { - return false; - } - if (!rmt->tx_not_rx) { - log_e("Can't write on a RX RMT Channel"); + if (!_rmtCheckTXnotRX(rmt, RMT_TX_MODE) || low > 0xFFFF || high > 0xFFFF) { return false; } size_t channel = rmt->channel; @@ -273,11 +288,7 @@ bool rmtSetCarrier(rmt_obj_t* rmt, bool carrier_en, bool carrier_level, uint32_t bool rmtSetFilter(rmt_obj_t* rmt, bool filter_en, uint32_t filter_level) { - if (!rmt || filter_level > 0xFF) { - return false; - } - if (rmt->tx_not_rx) { - log_e("Can't read on a TX RMT Channel"); + if (!_rmtCheckTXnotRX(rmt, RMT_RX_MODE) || filter_level > 0xFF) { return false; } size_t channel = rmt->channel; @@ -290,11 +301,7 @@ bool rmtSetFilter(rmt_obj_t* rmt, bool filter_en, uint32_t filter_level) bool rmtSetRxThreshold(rmt_obj_t* rmt, uint32_t value) { - if (!rmt || value > 0xFFFF) { - return false; - } - if (rmt->tx_not_rx) { - log_e("Can't read on a TX RMT Channel"); + if (!_rmtCheckTXnotRX(rmt, RMT_RX_MODE) || value > 0xFFFF) { return false; } size_t channel = rmt->channel; @@ -355,11 +362,7 @@ bool rmtDeinit(rmt_obj_t *rmt) bool rmtLoop(rmt_obj_t* rmt, rmt_data_t* data, size_t size) { - if (!rmt) { - return false; - } - if (!rmt->tx_not_rx) { - log_e("Can't write on a RX RMT Channel"); + if (!_rmtCheckTXnotRX(rmt, RMT_TX_MODE)) { return false; } int channel = rmt->channel; @@ -373,11 +376,7 @@ bool rmtLoop(rmt_obj_t* rmt, rmt_data_t* data, size_t size) bool rmtWrite(rmt_obj_t* rmt, rmt_data_t* data, size_t size) { - if (!rmt) { - return false; - } - if (!rmt->tx_not_rx) { - log_e("Can't write on a RX RMT Channel"); + if (!_rmtCheckTXnotRX(rmt, RMT_TX_MODE)) { return false; } int channel = rmt->channel; @@ -391,13 +390,9 @@ bool rmtWrite(rmt_obj_t* rmt, rmt_data_t* data, size_t size) bool rmtWriteBlocking(rmt_obj_t* rmt, rmt_data_t* data, size_t size) { - if (!rmt) { + if (!_rmtCheckTXnotRX(rmt, RMT_TX_MODE)) { return false; } - if (!rmt->tx_not_rx) { - log_e("Can't write on a RX RMT Channel"); - return false; - } int channel = rmt->channel; RMT_MUTEX_LOCK(channel); rmt_tx_stop(channel); @@ -409,11 +404,7 @@ bool rmtWriteBlocking(rmt_obj_t* rmt, rmt_data_t* data, size_t size) bool rmtReadData(rmt_obj_t* rmt, uint32_t* data, size_t size) { - if (!rmt) { - return false; - } - if (rmt->tx_not_rx) { - log_e("Can't read on a TX RMT Channel"); + if (!_rmtCheckTXnotRX(rmt, RMT_RX_MODE)) { return false; } rmtReadAsync(rmt, (rmt_data_t*) data, size, NULL, false, 0); @@ -423,11 +414,7 @@ bool rmtReadData(rmt_obj_t* rmt, uint32_t* data, size_t size) bool rmtBeginReceive(rmt_obj_t* rmt) { - if (!rmt) { - return false; - } - if (rmt->tx_not_rx) { - log_e("Can't read on a TX RMT Channel"); + if (!_rmtCheckTXnotRX(rmt, RMT_RX_MODE)) { return false; } int channel = rmt->channel; @@ -451,11 +438,7 @@ bool rmtReceiveCompleted(rmt_obj_t* rmt) bool rmtRead(rmt_obj_t* rmt, rmt_rx_data_cb_t cb, void * arg) { - if (!rmt || !cb) { - return false; - } - if (rmt->tx_not_rx) { - log_e("Can't read on a TX RMT Channel"); + if (!_rmtCheckTXnotRX(rmt, RMT_RX_MODE)) { return false; } int channel = rmt->channel; @@ -498,11 +481,7 @@ bool rmtEnd(rmt_obj_t* rmt) bool rmtReadAsync(rmt_obj_t* rmt, rmt_data_t* data, size_t size, void* eventFlag, bool waitForData, uint32_t timeout) { - if (!rmt) { - return false; - } - if (rmt->tx_not_rx) { - log_e("Can't read on a TX RMT Channel"); + if (!_rmtCheckTXnotRX(rmt, RMT_RX_MODE)) { return false; } int channel = rmt->channel; From 914ea2d4463acf0bf212315df7115701df6ad87e Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Thu, 3 Mar 2022 09:17:04 -0300 Subject: [PATCH 3/3] Typo --- cores/esp32/esp32-hal-rmt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cores/esp32/esp32-hal-rmt.c b/cores/esp32/esp32-hal-rmt.c index 91ac5df73ad..a220119d316 100644 --- a/cores/esp32/esp32-hal-rmt.c +++ b/cores/esp32/esp32-hal-rmt.c @@ -249,7 +249,8 @@ static bool _rmtCreateRxTask(rmt_obj_t* rmt) return true; } -// Helper function to test if a RMT channel is correctly assigned to TX or RX, issuing a error message if necessary +// Helper function to test if an RMT channel is correctly assigned to TX or RX, issuing an error message if necessary +// Also test RMT pointer for NULL and returns false in case it is NULL // return true when it is correctly assigned, false otherwise static bool _rmtCheckTXnotRX(rmt_obj_t* rmt, bool tx_not_rx) {