RMT initialization error leaves the channel in unrecoverable state (IDFGH-8732)

[ertong]

Answers checklist.

• I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
• I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
• I have searched the issue tracker for a similar issue and not found a similar issue.

General issue report

If for some reason rmt_driver_install returns ESP_ERR_NOT_FOUND, it becomes not possible to retry and make a successful call to rmt_driver_install or rmt_driver_uninstall.

In rmt_driver_install there is a code like

    if (rmt_contex.rmt_driver_channels == 0) {
        // first RMT channel using driver
        err = rmt_isr_register(rmt_driver_isr_default, &rmt_contex.hal, intr_alloc_flags, &(rmt_contex.rmt_driver_intr_handle));
    }
    if (err == ESP_OK) {
        rmt_contex.rmt_driver_channels |= BIT(channel);
    }

In case of success, the bit is set into rmt_driver_channels.
But in case of error, it is not and p_rmt_obj[channel] remains allocated.

Then, in rmt_driver_uninstall we have a precondition check, that forces bit to be set AND p_rmt_obj[channel] is not null.

    ESP_RETURN_ON_FALSE(rmt_contex.rmt_driver_channels & BIT(channel), ESP_ERR_INVALID_STATE, TAG, "No RMT driver for this channel");
    if (p_rmt_obj[channel] == NULL) {
        return ESP_OK;
    }

So, in case of error, nether rmt_driver_install, nor rmt_driver_uninstall cannot be called, due to invalid state.

rmt_driver_install should take care that
rmt_driver_channels is set if and only if p_rmt_obj[channel] != NULL

[suda-morris]

@ertong Nice catch! Yes, here is a real resource leak. rmt_contex.rmt_driver_channels should be used to only control the "shared interrupt"' lifecycle, not the channel object's. We will provide the fix as soon as possible.

For now, you can apply the following changes:

diff --git a/components/driver/deprecated/rmt_legacy.c b/components/driver/deprecated/rmt_legacy.c
index dbbc1886e6..3017a3b5c5 100644
--- a/components/driver/deprecated/rmt_legacy.c
+++ b/components/driver/deprecated/rmt_legacy.c
@@ -924,7 +924,7 @@ esp_err_t rmt_driver_uninstall(rmt_channel_t channel)
 {
     esp_err_t err = ESP_OK;
     ESP_RETURN_ON_FALSE(channel < RMT_CHANNEL_MAX, ESP_ERR_INVALID_ARG, TAG, RMT_CHANNEL_ERROR_STR);
-    ESP_RETURN_ON_FALSE(rmt_contex.rmt_driver_channels & BIT(channel), ESP_ERR_INVALID_STATE, TAG, "No RMT driver for this channel");
+    // we allow to call this uninstall function on the same channel for multiple times
     if (p_rmt_obj[channel] == NULL) {
         return ESP_OK;
     }
@@ -944,7 +944,7 @@ esp_err_t rmt_driver_uninstall(rmt_channel_t channel)

     _lock_acquire_recursive(&(rmt_contex.rmt_driver_isr_lock));
     rmt_contex.rmt_driver_channels &= ~BIT(channel);
-    if (rmt_contex.rmt_driver_channels == 0) {
+    if (rmt_contex.rmt_driver_channels == 0 && rmt_contex.rmt_driver_intr_handle) {
         rmt_module_disable();
         // all channels have driver disabled
         err = rmt_isr_deregister(rmt_contex.rmt_driver_intr_handle);
@@ -952,10 +952,6 @@ esp_err_t rmt_driver_uninstall(rmt_channel_t channel)
     }
     _lock_release_recursive(&(rmt_contex.rmt_driver_isr_lock));

-    if (err != ESP_OK) {
-        return err;
-    }
-
     if (p_rmt_obj[channel]->tx_sem) {
         vSemaphoreDelete(p_rmt_obj[channel]->tx_sem);
         p_rmt_obj[channel]->tx_sem = NULL;
@@ -987,7 +983,6 @@ esp_err_t rmt_driver_uninstall(rmt_channel_t channel)
 esp_err_t rmt_driver_install(rmt_channel_t channel, size_t rx_buf_size, int intr_alloc_flags)
 {
     ESP_RETURN_ON_FALSE(channel < RMT_CHANNEL_MAX, ESP_ERR_INVALID_ARG, TAG, RMT_CHANNEL_ERROR_STR);
-    ESP_RETURN_ON_FALSE((rmt_contex.rmt_driver_channels & BIT(channel)) == 0, ESP_ERR_INVALID_STATE, TAG, "RMT driver already installed for channel");

     esp_err_t err = ESP_OK;

[ertong]

Thank you.
Still trying to debug why "RMT initialization error" arises in the first place (very floating issue), ... but for this issue, the fix looks ok (not sure whether it is correct to ignore the result of rmt_isr_deregister, but it is possible that it is ok).