Merge pull request #394 from teknynja/esp32_rmt_memory_allocation_fix_safe

RMT Buffer Allocation Fix (Thread-Safe) for Issue #375

Author: dhalbert

Adafruit_NeoPixel.cpp

@@ -92,6 +92,11 @@ Adafruit_NeoPixel::Adafruit_NeoPixel(uint16_t n, int16_t p, neoPixelType t)
   }
   init = true;
 #endif
+#if defined(ESP32)
+#if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 0, 0)
+  espInit();
+#endif
+#endif
 }
 
 /*!
@@ -389,6 +394,7 @@ extern "C" IRAM_ATTR void espShow(uint16_t pin, uint8_t *pixels,
 #elif defined(ESP32)
 extern "C" void espShow(uint16_t pin, uint8_t *pixels, uint32_t numBytes,
                         uint8_t type);
+
 #endif // ESP8266
 
 #if defined(K210)

Adafruit_NeoPixel.h

@@ -208,6 +208,15 @@ static const uint8_t PROGMEM _NeoPixelGammaTable[256] = {
     218, 220, 223, 225, 227, 230, 232, 235, 237, 240, 242, 245, 247, 250, 252,
     255};
 
+/* Declare external methods required by the Adafruit_NeoPixel implementation
+    for specific hardware/library versions
+*/
+#if defined(ESP32)
+#if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 0, 0)
+extern "C" void espInit();
+#endif
+#endif
+
 /*!
     @brief  Class that stores state and functions for interacting with
             Adafruit NeoPixels and compatible devices.

esp.c

@@ -31,40 +31,93 @@
 #endif
 
 
-
 #ifdef HAS_ESP_IDF_5
 
+static SemaphoreHandle_t show_mutex = NULL;
+
 void espShow(uint8_t pin, uint8_t *pixels, uint32_t numBytes, boolean is800KHz) {
-  rmt_data_t led_data[numBytes * 8];
+  // Note: Because rmtPin is shared between all instances, we will
+  //  end up releasing/initializing the RMT channels each time we
+  //  invoke on different pins. This is probably ok, just not
+  //  efficient. led_data is shared between all instances but will
+  //  be allocated with enough space for the largest instance; data
+  //  is not used beyond the mutex lock so this should be fine.
+
+#define SEMAPHORE_TIMEOUT_MS 50
+
+  static rmt_data_t *led_data = NULL;
+  static uint32_t led_data_size = 0;
+  static int rmtPin = -1;
+
+  if (show_mutex && xSemaphoreTake(show_mutex, SEMAPHORE_TIMEOUT_MS / portTICK_PERIOD_MS) == pdTRUE) {
+    uint32_t requiredSize = numBytes * 8;
+    if (requiredSize > led_data_size) {
+      free(led_data);
+      if (led_data = (rmt_data_t *)malloc(requiredSize * sizeof(rmt_data_t))) {
+        led_data_size = requiredSize;
+      } else {
+        led_data_size = 0;
+      }
+    } else if (requiredSize == 0) {
+      // To release RMT resources (RMT channels and led_data), call
+      //  .updateLength(0) to set number of pixels/bytes to zero,
+      //  then call .show() to invoke this code and free resources.
+      free(led_data);
+      led_data = NULL;
+      if (rmtPin >= 0) {
+        rmtDeinit(rmtPin);
+        rmtPin = -1;
+      }
+      led_data_size = 0;
+    }
 
-  if (!rmtInit(pin, RMT_TX_MODE, RMT_MEM_NUM_BLOCKS_1, 10000000)) {
-    log_e("Failed to init RMT TX mode on pin %d", pin);
-    return;
-  }
+    if (led_data_size > 0 && requiredSize <= led_data_size) {
+      if (pin != rmtPin) {
+        if (rmtPin >= 0) {
+          rmtDeinit(rmtPin);
+          rmtPin = -1;
+        }
+        if (!rmtInit(pin, RMT_TX_MODE, RMT_MEM_NUM_BLOCKS_1, 10000000)) {
+          log_e("Failed to init RMT TX mode on pin %d", pin);
+          return;
+        }
+        rmtPin = pin;
+      }
 
-  int i=0;
-  for (int b=0; b < numBytes; b++) {
-    for (int bit=0; bit<8; bit++){
-      if ( pixels[b] & (1<<(7-bit)) ) {
-        led_data[i].level0 = 1;
-        led_data[i].duration0 = 8;
-        led_data[i].level1 = 0;
-        led_data[i].duration1 = 4;
-      } else {
-        led_data[i].level0 = 1;
-        led_data[i].duration0 = 4;
-        led_data[i].level1 = 0;
-        led_data[i].duration1 = 8;
+      if (rmtPin >= 0) {
+        int i=0;
+        for (int b=0; b < numBytes; b++) {
+          for (int bit=0; bit<8; bit++){
+            if ( pixels[b] & (1<<(7-bit)) ) {
+              led_data[i].level0 = 1;
+              led_data[i].duration0 = 8;
+              led_data[i].level1 = 0;
+              led_data[i].duration1 = 4;
+            } else {
+              led_data[i].level0 = 1;
+              led_data[i].duration0 = 4;
+              led_data[i].level1 = 0;
+              led_data[i].duration1 = 8;
+            }
+            i++;
+          }
+        }
+
+        rmtWrite(pin, led_data, numBytes * 8, RMT_WAIT_FOR_EVER);
       }
-      i++;
     }
-  }
 
-  //pinMode(pin, OUTPUT);  // don't do this, will cause the rmt to disable!
-  rmtWrite(pin, led_data, numBytes * 8, RMT_WAIT_FOR_EVER);
+    xSemaphoreGive(show_mutex);
+  }
 }
 
-
+// To avoid race condition initializing the mutex, all instances of
+//  Adafruit_NeoPixel must be constructed before launching and child threads
+void espInit() {
+  if (!show_mutex) {
+    show_mutex = xSemaphoreCreateMutex();
+  }
+}
 
 #else
 
@@ -219,6 +272,6 @@ void espShow(uint8_t pin, uint8_t *pixels, uint32_t numBytes, boolean is800KHz)
 }
 
 #endif // ifndef IDF5
- 
+
 
 #endif // ifdef(ESP32)