Harden LED configuration and color packing

src/led_color_order.c

@@ -0,0 +1,59 @@
+// Copyright 2026 Michael Conrad
+//
+// This file is part of the Refloat VESC package.
+//
+// Refloat VESC package is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by the
+// Free Software Foundation, either version 3 of the License, or (at your
+// option) any later version.
+//
+// Refloat VESC package is distributed in the hope that it will be useful, but
+// WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+// or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+// more details.
+//
+// You should have received a copy of the GNU General Public License along with
+// this program. If not, see <http://www.gnu.org/licenses/>.
+
+#include "led_color_order.h"
+
+static uint32_t color_grb(uint8_t w, uint8_t r, uint8_t g, uint8_t b) {
+    (void) w;
+    return ((uint32_t) g << 16) | ((uint32_t) r << 8) | b;
+}
+
+static uint32_t color_grbw(uint8_t w, uint8_t r, uint8_t g, uint8_t b) {
+    return ((uint32_t) g << 24) | ((uint32_t) r << 16) | ((uint32_t) b << 8) | w;
+}
+
+static uint32_t color_rgb(uint8_t w, uint8_t r, uint8_t g, uint8_t b) {
+    (void) w;
+    return ((uint32_t) r << 16) | ((uint32_t) g << 8) | b;
+}
+
+static uint32_t color_wrgb(uint8_t w, uint8_t r, uint8_t g, uint8_t b) {
+    return ((uint32_t) w << 24) | ((uint32_t) r << 16) | ((uint32_t) g << 8) | b;
+}
+
+bool led_color_order_resolve(LedColorOrder order, uint8_t *bits, LedColorConverter *converter) {
+    switch (order) {
+    case LED_COLOR_GRB:
+        *bits = 24;
+        *converter = color_grb;
+        return true;
+    case LED_COLOR_GRBW:
+        *bits = 32;
+        *converter = color_grbw;
+        return true;
+    case LED_COLOR_RGB:
+        *bits = 24;
+        *converter = color_rgb;
+        return true;
+    case LED_COLOR_WRGB:
+        *bits = 32;
+        *converter = color_wrgb;
+        return true;
+    }
+
+    return false;
+}

src/led_color_order.h

@@ -0,0 +1,27 @@
+// Copyright 2026 Michael Conrad
+//
+// This file is part of the Refloat VESC package.
+//
+// Refloat VESC package is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by the
+// Free Software Foundation, either version 3 of the License, or (at your
+// option) any later version.
+//
+// Refloat VESC package is distributed in the hope that it will be useful, but
+// WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+// or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+// more details.
+//
+// You should have received a copy of the GNU General Public License along with
+// this program. If not, see <http://www.gnu.org/licenses/>.
+
+#pragma once
+
+#include "conf/datatypes.h"
+
+#include <stdbool.h>
+#include <stdint.h>
+
+typedef uint32_t (*LedColorConverter)(uint8_t w, uint8_t r, uint8_t g, uint8_t b);
+
+bool led_color_order_resolve(LedColorOrder order, uint8_t *bits, LedColorConverter *converter);

src/led_driver.c

@@ -166,20 +166,6 @@ static void deinit_hw(const PinHwConfig *cfg) {
     disable_dma_stream(cfg);
 }
 
-inline static uint8_t color_order_bits(LedColorOrder order) {
-    switch (order) {
-    case LED_COLOR_GRBW:
-    case LED_COLOR_WRGB:
-        return 32;
-    case LED_COLOR_GRB:
-    case LED_COLOR_RGB:
-        return 24;
-    }
-
-    // the switch above should be exhaustive, just silence the warning
-    return 24;
-}
-
 void led_driver_init(LedDriver *driver) {
     driver->bitbuffer_length = 0;
     driver->bitbuffer = NULL;
@@ -192,6 +178,7 @@ bool led_driver_setup(
         log_error("Invalid LED pin configured: %u", pin);
         return false;
     }
+
     driver->pin_hw_config = &pin_hw_configs[pin];
 
     driver->bitbuffer_length = 0;
@@ -207,7 +194,7 @@ bool led_driver_setup(
 
         driver->strips[i] = strip;
         offsets[i] = driver->bitbuffer_length;
-        driver->bitbuffer_length += color_order_bits(strip->color_order) * strip->length;
+        driver->bitbuffer_length += strip->color_bits * strip->length;
     }
 
     // An extra array item to set the output to 0 PWM
@@ -239,24 +226,6 @@ inline static uint8_t cgamma(uint8_t c) {
     return (c * c + c) / 256;
 }
 
-static uint32_t color_grb(uint8_t w, uint8_t r, uint8_t g, uint8_t b) {
-    unused(w);
-    return (g << 16) | (r << 8) | b;
-}
-
-static uint32_t color_grbw(uint8_t w, uint8_t r, uint8_t g, uint8_t b) {
-    return (g << 24) | (r << 16) | (b << 8) | w;
-}
-
-static uint32_t color_rgb(uint8_t w, uint8_t r, uint8_t g, uint8_t b) {
-    unused(w);
-    return (r << 16) | (g << 8) | b;
-}
-
-static uint32_t color_wrgb(uint8_t w, uint8_t r, uint8_t g, uint8_t b) {
-    return (w << 24) | (r << 16) | (g << 8) | b;
-}
-
 void led_driver_paint(LedDriver *driver) {
     if (!driver->bitbuffer) {
         return;
@@ -268,23 +237,6 @@ void led_driver_paint(LedDriver *driver) {
             break;
         }
 
-        uint32_t (*color_conv)(uint8_t, uint8_t, uint8_t, uint8_t);
-        switch (strip->color_order) {
-        case LED_COLOR_GRB:
-            color_conv = color_grb;
-            break;
-        case LED_COLOR_GRBW:
-            color_conv = color_grbw;
-            break;
-        case LED_COLOR_RGB:
-            color_conv = color_rgb;
-            break;
-        case LED_COLOR_WRGB:
-            color_conv = color_wrgb;
-            break;
-        }
-
-        uint8_t bits = color_order_bits(strip->color_order);
         uint16_t *strip_bitbuffer = driver->strip_bitbuffs[i];
 
         for (uint32_t j = 0; j < strip->length; ++j) {
@@ -294,10 +246,11 @@ void led_driver_paint(LedDriver *driver) {
             uint8_t g = cgamma((color >> 8) & 0xFF);
             uint8_t b = cgamma(color & 0xFF);
 
-            color = color_conv(w, r, g, b);
+            color = strip->color_conv(w, r, g, b);
 
-            for (int8_t bit = bits - 1; bit >= 0; --bit) {
-                strip_bitbuffer[bit + j * bits] = color & 0x1 ? WS2812_ONE : WS2812_ZERO;
+            for (int8_t bit = strip->color_bits - 1; bit >= 0; --bit) {
+                strip_bitbuffer[bit + j * strip->color_bits] =
+                    color & 0x1 ? WS2812_ONE : WS2812_ZERO;
                 color >>= 1;
             }
         }

src/led_strip.c

@@ -22,12 +22,20 @@
 void led_strip_init(LedStrip *strip) {
     strip->data = NULL;
     strip->length = 0;
-    strip->color_order = LED_COLOR_GRB;
+    led_color_order_resolve(LED_COLOR_GRB, &strip->color_bits, &strip->color_conv);
     strip->reverse = false;
 }
 
-void led_strip_configure(LedStrip *strip, const CfgLedStrip *cfg) {
+bool led_strip_configure(LedStrip *strip, const CfgLedStrip *cfg) {
+    uint8_t color_bits;
+    LedColorConverter color_conv;
+    if (!led_color_order_resolve(cfg->color_order, &color_bits, &color_conv)) {
+        return false;
+    }
+
     strip->length = cfg->count;
-    strip->color_order = cfg->color_order;
+    strip->color_bits = color_bits;
+    strip->color_conv = color_conv;
     strip->reverse = cfg->reverse;
+    return true;
 }

src/led_strip.h

@@ -17,8 +17,9 @@
 
 #pragma once
 
-#include "conf/datatypes.h"
+#include "led_color_order.h"
 
+#include <stdbool.h>
 #include <stdint.h>
 
 #define STRIP_COUNT 3
@@ -35,12 +36,13 @@ typedef union {
 typedef struct {
     uint32_t *data;
     uint8_t length;
-    LedColorOrder color_order;
+    uint8_t color_bits;
+    LedColorConverter color_conv;
     bool reverse;
     float brightness;
     TransitionData trans_data;
 } LedStrip;
 
 void led_strip_init(LedStrip *strip);
 
-void led_strip_configure(LedStrip *strip, const CfgLedStrip *cfg);
+bool led_strip_configure(LedStrip *strip, const CfgLedStrip *cfg);

src/leds.c

@@ -836,6 +836,15 @@ void leds_init(Leds *leds) {
     led_driver_init(&leds->led_driver);
 }
 
+static bool configure_strip(LedStrip *strip, const CfgLedStrip *cfg, const char *name) {
+    if (led_strip_configure(strip, cfg)) {
+        return true;
+    }
+
+    log_error("Invalid %s LED color order: %u.", name, cfg->color_order);
+    return false;
+}
+
 void leds_setup(Leds *leds, CfgHwLeds *hw_cfg, const CfgLeds *cfg) {
     uint8_t status_offset = 0;
     uint8_t front_offset = 0;
@@ -846,17 +855,23 @@ void leds_setup(Leds *leds, CfgHwLeds *hw_cfg, const CfgLeds *cfg) {
     size_t strip_i = 0;
     for (uint8_t i = 1; i <= STRIP_COUNT; ++i) {
         if (hw_cfg->status.order == i && hw_cfg->status.count > 0) {
-            led_strip_configure(&leds->status_strip, &hw_cfg->status);
+            if (!configure_strip(&leds->status_strip, &hw_cfg->status, "status")) {
+                return;
+            }
             status_offset = current_offset;
             current_offset += leds->status_strip.length;
             strip_array[strip_i++] = &leds->status_strip;
         } else if (hw_cfg->front.order == i && hw_cfg->front.count > 0) {
-            led_strip_configure(&leds->front_strip, &hw_cfg->front);
+            if (!configure_strip(&leds->front_strip, &hw_cfg->front, "front")) {
+                return;
+            }
             front_offset = current_offset;
             current_offset += leds->front_strip.length;
             strip_array[strip_i++] = &leds->front_strip;
         } else if (hw_cfg->rear.order == i && hw_cfg->rear.count > 0) {
-            led_strip_configure(&leds->rear_strip, &hw_cfg->rear);
+            if (!configure_strip(&leds->rear_strip, &hw_cfg->rear, "rear")) {
+                return;
+            }
             rear_offset = current_offset;
             current_offset += leds->rear_strip.length;
             strip_array[strip_i++] = &leds->rear_strip;