Add ledstrip_grb_rgb setting (GRB or RGB) to handle WS2811 or WS2812 … (#5255)

* Add ledstrip_grb_rgb setting (GRB or RGB) to handle WS2811 or WS2812 LED drivers

* Rename setting lookup table to lookupLedStripPackingOrder

* Fix call to ws2811UpdateStrip

* Fix unit test

* Use ledStripFormatRGB_e enumeration for RGB packing format

* Fix unit test

* Whoops. Make ledStripFormatRGB_e match lookupLedStripFormatRGB

* Applied review feedback

* Add documentation of ledstrip_grb_rgb

docs/LedStrip.md

@@ -38,6 +38,26 @@ It could be possible to be able to specify the timings required via CLI if users
 * [Adafruit NeoPixel Stick](https://www.adafruit.com/products/1426) (works well)
   * Measured current consumption in all white mode ~ 350 mA.
 
+
+### WS2811 vs WS2812
+
+The [WS2811](https://cdn-shop.adafruit.com/datasheets/WS2811.pdf) is a LED driver IC which is connected to an RGB LED. It accepts data in the form of 8 bits each of Red-Green-Blue.
+
+The [WS2812](https://cdn-shop.adafruit.com/datasheets/WS2812.pdf) is integrated into the package of a 50:50 LED rather than as a separate device.. It accepts data in the form of 8 bits each of Green-Red-Blue.
+
+It is thus possible, depending on the LED board/strip being used that either Red-Green-Blue or Green-Red-Blue encoding may be required. This may be controlled by setting the following.
+
+```
+set ledstrip_grb_rgb = RGB
+```
+or
+
+```
+set ledstrip_grb_rgb = GRB
+```
+
+The confirm the required setting simply set an LED to be green. If it lights up red, you have the wrong setting.
+
 ## Connections
 
 WS2812 LED strips generally require a single data line, 5V and GND.
@@ -601,4 +621,4 @@ This also means that you can make sure that each R,G and B LED in each LED modul
 
 After a short delay the LEDs will show the unarmed color sequence and or low-battery warning sequence.
 
-Also check that the feature `LED_STRIP` was correctly enabled and that it does not conflict with other features, as above.
+Also check that the feature `LED_STRIP` was correctly enabled and that it does not conflict with other features, as above.

src/main/drivers/light_ws2811strip.c

@@ -95,7 +95,8 @@ void ws2811LedStripInit(ioTag_t ioTag)
 
     const hsvColor_t hsv_white = { 0, 255, 255 };
     setStripColor(&hsv_white);
-    ws2811UpdateStrip();
+    // RGB or GRB ordering doesn't matter for white
+    ws2811UpdateStrip(LED_RGB);
 }
 
 bool isWS2811LedStripReady(void)
@@ -109,12 +110,23 @@ static int16_t ledIndex;
 #define USE_FAST_DMA_BUFFER_IMPL
 #ifdef USE_FAST_DMA_BUFFER_IMPL
 
-STATIC_UNIT_TESTED void fastUpdateLEDDMABuffer(rgbColor24bpp_t *color)
+STATIC_UNIT_TESTED void fastUpdateLEDDMABuffer(ledStripFormatRGB_e ledFormat, rgbColor24bpp_t *color)
 {
-    uint32_t grb = (color->rgb.g << 16) | (color->rgb.r << 8) | (color->rgb.b);
+    uint32_t packed_colour;
+
+    switch (ledFormat) {
+        case LED_RGB: // WS2811 drivers use RGB format
+            packed_colour = (color->rgb.r << 16) | (color->rgb.g << 8) | (color->rgb.b);
+            break;
+
+        case LED_GRB: // WS2812 drivers use GRB format
+        default:
+            packed_colour = (color->rgb.g << 16) | (color->rgb.r << 8) | (color->rgb.b);
+        break;
+    }
 
     for (int8_t index = 23; index >= 0; index--) {
-        ledStripDMABuffer[dmaBufferOffset++] = (grb & (1 << index)) ? BIT_COMPARE_1 : BIT_COMPARE_0;
+        ledStripDMABuffer[dmaBufferOffset++] = (packed_colour & (1 << index)) ? BIT_COMPARE_1 : BIT_COMPARE_0;
     }
 }
 #else
@@ -141,7 +153,7 @@ STATIC_UNIT_TESTED void updateLEDDMABuffer(uint8_t componentValue)
  * This method is non-blocking unless an existing LED update is in progress.
  * it does not wait until all the LEDs have been updated, that happens in the background.
  */
-void ws2811UpdateStrip(void)
+void ws2811UpdateStrip(ledStripFormatRGB_e ledFormat)
 {
     static rgbColor24bpp_t *rgb24;
 
@@ -160,10 +172,21 @@ void ws2811UpdateStrip(void)
         rgb24 = hsvToRgb24(&ledColorBuffer[ledIndex]);
 
 #ifdef USE_FAST_DMA_BUFFER_IMPL
-        fastUpdateLEDDMABuffer(rgb24);
+        fastUpdateLEDDMABuffer(ledFormat, rgb24);
 #else
-        updateLEDDMABuffer(rgb24->rgb.g);
-        updateLEDDMABuffer(rgb24->rgb.r);
+        switch (ledFormat) {
+            case LED_RGB: // WS2811 drivers use RGB format
+            updateLEDDMABuffer(rgb24->rgb.r);
+            updateLEDDMABuffer(rgb24->rgb.g);
+            break;
+
+        case LED_GRB: // WS2812 drivers use GRB format
+        default:
+            updateLEDDMABuffer(rgb24->rgb.g);
+            updateLEDDMABuffer(rgb24->rgb.r);
+            break;
+        }
+
         updateLEDDMABuffer(rgb24->rgb.b);
 #endif
 

src/main/drivers/light_ws2811strip.h

@@ -18,6 +18,7 @@
 #pragma once
 
 #include "drivers/io_types.h"
+#include "io/ledstrip.h"
 
 #define WS2811_LED_STRIP_LENGTH    32
 #define WS2811_BITS_PER_LED        24
@@ -36,7 +37,7 @@ void ws2811LedStripInit(ioTag_t ioTag);
 void ws2811LedStripHardwareInit(ioTag_t ioTag);
 void ws2811LedStripDMAEnable(void);
 
-void ws2811UpdateStrip(void);
+void ws2811UpdateStrip(ledStripFormatRGB_e ledFormat);
 
 void setLedHsv(uint16_t index, const hsvColor_t *color);
 void getLedHsv(uint16_t index, hsvColor_t *color);

src/main/interface/settings.c

@@ -293,6 +293,12 @@ static const char * const lookupOverclock[] = {
 };
 #endif
 
+#ifdef USE_LED_STRIP
+    static const char * const lookupLedStripFormatRGB[] = {
+        "GRB", "RGB"
+    };
+#endif
+
 const lookupTableEntry_t lookupTables[] = {
     { lookupTableOffOn, sizeof(lookupTableOffOn) / sizeof(char *) },
     { lookupTableUnit, sizeof(lookupTableUnit) / sizeof(char *) },
@@ -348,6 +354,9 @@ const lookupTableEntry_t lookupTables[] = {
 #ifdef USE_OVERCLOCK
     { lookupOverclock, sizeof(lookupOverclock) / sizeof(char *) },
 #endif
+#ifdef USE_LED_STRIP
+    { lookupLedStripFormatRGB, sizeof(lookupLedStripFormatRGB) / sizeof(char *) },
+#endif
 #ifdef USE_DUAL_GYRO
     { lookupTableGyro, sizeof(lookupTableGyro) / sizeof(char *) },
 #endif
@@ -730,6 +739,7 @@ const clivalue_t valueTable[] = {
 // PG_LED_STRIP_CONFIG
 #ifdef USE_LED_STRIP
     { "ledstrip_visual_beeper",     VAR_UINT8  | MASTER_VALUE | MODE_LOOKUP, .config.lookup = { TABLE_OFF_ON }, PG_LED_STRIP_CONFIG, offsetof(ledStripConfig_t, ledstrip_visual_beeper) },
+    { "ledstrip_grb_rgb",           VAR_UINT8  | MASTER_VALUE | MODE_LOOKUP, .config.lookup = { TABLE_RGB_GRB }, PG_LED_STRIP_CONFIG, offsetof(ledStripConfig_t, ledstrip_grb_rgb) },
 #endif
 
 // PG_SDCARD_CONFIG

src/main/interface/settings.h

@@ -77,6 +77,9 @@ typedef enum {
 #ifdef USE_OVERCLOCK
     TABLE_OVERCLOCK,
 #endif
+#ifdef USE_LED_STRIP
+    TABLE_RGB_GRB,
+#endif
 #ifdef USE_DUAL_GYRO
     TABLE_GYRO,
 #endif

src/main/io/ledstrip.c

@@ -1115,7 +1115,7 @@ void ledStripUpdate(timeUs_t currentTimeUs)
         bool updateNow = timActive & (1 << timId);
         (*layerTable[timId])(updateNow, timer);
     }
-    ws2811UpdateStrip();
+    ws2811UpdateStrip((ledStripFormatRGB_e)ledStripConfig()->ledstrip_grb_rgb);
 }
 
 bool parseColor(int index, const char *colorConfig)
@@ -1210,6 +1210,6 @@ static void ledStripDisable(void)
 {
     setStripColor(&HSV(BLACK));
 
-    ws2811UpdateStrip();
+    ws2811UpdateStrip((ledStripFormatRGB_e)ledStripConfig()->ledstrip_grb_rgb);
 }
 #endif

src/main/io/ledstrip.h

@@ -121,6 +121,12 @@ typedef enum {
     LED_OVERLAY_WARNING
 } ledOverlayId_e;
 
+// Enumeration to match the string options defined in lookupLedStripFormatRGB in settings.c
+typedef enum {
+    LED_GRB,
+    LED_RGB
+} ledStripFormatRGB_e;
+
 typedef struct modeColorIndexes_s {
     uint8_t color[LED_DIRECTION_COUNT];
 } modeColorIndexes_t;
@@ -146,6 +152,7 @@ typedef struct ledStripConfig_s {
     uint8_t ledstrip_visual_beeper; // suppress LEDLOW mode if beeper is on
     uint8_t ledstrip_aux_channel;
     ioTag_t ioTag;
+    ledStripFormatRGB_e ledstrip_grb_rgb;
 } ledStripConfig_t;
 
 PG_DECLARE(ledStripConfig_t, ledStripConfig);

src/test/unit/ledstrip_unittest.cc

@@ -308,7 +308,7 @@ void ws2811LedStripInit(ioTag_t ioTag) {
     UNUSED(ioTag);
 }
 
-void ws2811UpdateStrip(void) {}
+void ws2811UpdateStrip(ledStripFormatRGB_e) {}
 
 void setLedValue(uint16_t index, const uint8_t value) {
     UNUSED(index);