Make it possible to use any RGB sequence for panels with unusual mapp…

…ing.

o Replace swap_green_blue with a new option led_rgb_sequence to give
  any mapping (e.g. some have red and green swapped, that would be "GRB")
o Still support --led_swap_green_blue, but don't advertise it anymore.
o Drop swap_green_blue field completely from the API and replace with led_rgb_sequence.
o Add option to Python wrapper.

README.md

@@ -286,12 +286,12 @@ You might want this if started from an init script at boot-time.
 
 ```
 --led-inverse             : Switch if your matrix has inverse colors on.
---led-swap-green-blue     : Switch if your matrix has green/blue swapped on.
+--led-rgb-sequence        : Switch if your matrix has led colors swapped (Default: "RGB")
 ```
 
 These are if you have a different kind of LED panel in which the logic of the
-color bits is reversed (`--led-inverse`) or where the green and blue colors
-are swapped (`--led-swap-green-blue`). You know it when you see it.
+color bits is reversed (`--led-inverse`) or where the Red, Green and Blue LEDs
+are mixed up (`--led-rgb-sequence`). You know it when you see it.
 
 Troubleshooting
 ---------------

examples-api-use/README.md

@@ -26,7 +26,7 @@ Options:
         --led-scan-mode=<0..1>    : 0 = progressive; 1 = interlaced (Default: 0).
         --led-show-refresh        : Show refresh rate.
         --led-inverse             : Switch if your matrix has inverse colors on.
-        --led-swap-green-blue     : Switch if your matrix has green/blue swapped on.
+        --led-rgb-sequence        : Switch if your matrix has led colors swapped (Default: "RGB")
         --led-pwm-lsb-nanoseconds : PWM Nanoseconds for LSB (Default: 130)
         --led-no-hardware-pulse   : Don't use hardware pin-pulse generation.
         --led-slowdown-gpio=<0..2>: Slowdown GPIO. Needed for faster Pis and/or slower panels (Default: 1).

include/led-matrix-c.h

@@ -95,6 +95,11 @@ struct RGBLedMatrixOptions {
    */
   int scan_mode;
 
+  /* In case the internal sequence of mapping is not "RGB", this contains the
+   * real mapping. Some panels mix up these colors.
+   */
+  const char *led_rgb_sequence;     /* Corresponding flag: --led-rgb-sequence */
+
   /** The following are boolean flags, all off by default **/
 
   /* Allow to use the hardware subsystem to create pulses. This won't do
@@ -103,7 +108,7 @@ struct RGBLedMatrixOptions {
    */
   unsigned disable_hardware_pulsing:1;
   unsigned show_refresh_rate:1;  /* Corresponding flag: --led-show-refresh    */
-  unsigned swap_green_blue:1;    /* Corresponding flag: --led-swap-green-blue */
+  // unsigned swap_green_blue:1; /* deprecated, use led_sequence instead */
   unsigned inverse_colors:1;     /* Corresponding flag: --led-inverse         */
 };
 

include/led-matrix.h

@@ -111,9 +111,13 @@ class RGBMatrix : public Canvas {
     // non-standard wirings.
     // Flag: --led-hardware-pulse
     bool disable_hardware_pulsing;
-    bool show_refresh_rate;  // Flag: --led-show-refresh
-    bool swap_green_blue;    // Flag: --led-swap-green-blue
-    bool inverse_colors;     // Flag: --led-inverse
+    bool show_refresh_rate;    // Flag: --led-show-refresh
+    // bool swap_green_blue; (Deprecated: use led_sequence instead)
+    bool inverse_colors;       // Flag: --led-inverse
+
+    // In case the internal sequence of mapping is not "RGB", this contains the
+    // real mapping. Some panels mix up these colors.
+    const char *led_rgb_sequence;  // Flag: --led-rgb-sequence
   };
 
   // Create an RGBMatrix.

lib/Makefile

@@ -38,12 +38,6 @@ HARDWARE_DESC?=regular
 # Flag: --led-inverse
 #DEFINES+=-DINVERSE_RGB_DISPLAY_COLORS
 
-# Some panels out there seem to have Green and Blue swapped (even though the
-# writing on connector looks correct). This option will swap these colors if
-# you happen to have gotten such panel.
-# Flag: --led-swap-green-blue
-#DEFINES+=-DRGB_SWAP_GREEN_BLUE
-
 # For curiosity reasons and while tweaking values for LSB_PWM_NANOSECONDS,
 # uncomment to see refresh rate in terminal.
 # Flag: --led-show-refresh

lib/framebuffer-internal.h

@@ -61,7 +61,7 @@ class Framebuffer {
 public:
   Framebuffer(int rows, int columns, int parallel,
               int scan_mode,
-              bool swap_green_blue, bool inverse_color,
+              const char* led_sequence, bool inverse_color,
               PixelMapper **mapper);
   ~Framebuffer();
 
@@ -101,6 +101,13 @@ class Framebuffer {
 private:
   static const struct HardwareMapping *hardware_mapping_;
 
+  // This returns the gpio-bit for given color (one of 'R', 'G', 'B'). This is
+  // returning the right value in case led_sequence_ is _not_ "RGB"
+  gpio_bits_t GetGpioFromLedSequence(char col,
+                                     gpio_bits_t default_r,
+                                     gpio_bits_t default_g,
+                                     gpio_bits_t default_b);
+
   void InitDefaultDesignator(int x, int y, PixelDesignator *designator);
   inline void  MapColors(uint8_t r, uint8_t g, uint8_t b,
                          uint16_t *red, uint16_t *green, uint16_t *blue);
@@ -110,7 +117,7 @@ class Framebuffer {
   const int columns_;  // Number of columns. Number of chained boards * 32.
 
   const int scan_mode_;
-  const bool swap_green_blue_;
+  const char *const led_sequence_;  // Some LEDs are mapped differently.
   const bool inverse_color_;
 
   uint8_t pwm_bits_;   // PWM bits to display.

lib/framebuffer.cc

@@ -20,6 +20,7 @@
 #include "framebuffer-internal.h"
 
 #include <assert.h>
+#include <ctype.h>
 #include <math.h>
 #include <stdint.h>
 #include <stdio.h>
@@ -63,14 +64,14 @@ const struct HardwareMapping *Framebuffer::hardware_mapping_ = NULL;
 
 Framebuffer::Framebuffer(int rows, int columns, int parallel,
                          int scan_mode,
-                         bool swap_green_blue, bool inverse_color,
+                         const char *led_sequence, bool inverse_color,
                          PixelMapper **mapper)
   : rows_(rows),
     parallel_(parallel),
     height_(rows * parallel),
     columns_(columns),
     scan_mode_(scan_mode),
-    swap_green_blue_(swap_green_blue), inverse_color_(inverse_color),
+    led_sequence_(led_sequence), inverse_color_(inverse_color),
     pwm_bits_(kBitPlanes), do_luminance_correct_(true), brightness_(100),
     double_rows_(rows / SUB_PANELS_), row_mask_(double_rows_ - 1),
     shared_mapper_(mapper) {
@@ -270,11 +271,8 @@ inline void Framebuffer::MapColors(
 
 void Framebuffer::Fill(uint8_t r, uint8_t g, uint8_t b) {
   uint16_t red, green, blue;
-  if (!swap_green_blue_) {
-    MapColors(r, g, b, &red, &green, &blue);
-  } else {
-    MapColors(r, g, b, &red, &blue, &green);
-  }
+  MapColors(r, g, b, &red, &green, &blue);
+
   const struct HardwareMapping &h = *hardware_mapping_;
   gpio_bits_t all_r = h.p0_r1 | h.p0_r2 | h.p1_r1 | h.p1_r2 | h.p2_r1 | h.p2_r2;
   gpio_bits_t all_g = h.p0_g1 | h.p0_g2 | h.p1_g1 | h.p1_g2 | h.p2_g1 | h.p2_g2;
@@ -306,11 +304,7 @@ void Framebuffer::SetPixel(int x, int y, uint8_t r, uint8_t g, uint8_t b) {
   if (pos < 0) return;  // non-used pixel marker.
 
   uint16_t red, green, blue;
-  if (!swap_green_blue_) {
-    MapColors(r, g, b, &red, &green, &blue);
-  } else {
-    MapColors(r, g, b, &red, &blue, &green);
-  }
+  MapColors(r, g, b, &red, &green, &blue);
 
   uint32_t *bits = bitplane_buffer_ + pos;
   const int min_bit_plane = kBitPlanes - pwm_bits_;
@@ -330,42 +324,62 @@ void Framebuffer::SetPixel(int x, int y, uint8_t r, uint8_t g, uint8_t b) {
   }
 }
 
+// Strange LED-mappings such as RBG or so are handled here.
+gpio_bits_t Framebuffer::GetGpioFromLedSequence(char col,
+                                                gpio_bits_t default_r,
+                                                gpio_bits_t default_g,
+                                                gpio_bits_t default_b) {
+  const char *pos = strchr(led_sequence_, col);
+  if (pos == NULL) pos = strchr(led_sequence_, tolower(col));
+  if (pos == NULL) {
+    fprintf(stderr, "LED sequence '%s' does not contain any '%c'.\n",
+            led_sequence_, col);
+    abort();
+  }
+  switch (pos - led_sequence_) {
+  case 0: return default_r;
+  case 1: return default_g;
+  case 2: return default_b;
+  }
+  return default_r;  // String too long, should've been caught earlier.
+}
+
 void Framebuffer::InitDefaultDesignator(int x, int y, PixelDesignator *d) {
   const struct HardwareMapping &h = *hardware_mapping_;
   uint32_t *bits = ValueAt(y & row_mask_, x, 0);
   d->gpio_word = bits - bitplane_buffer_;
   d->r_bit = d->g_bit = d->b_bit = 0;
   if (y < rows_) {
     if (y < double_rows_) {
-      d->r_bit = h.p0_r1;
-      d->g_bit = h.p0_g1;
-      d->b_bit = h.p0_b1;
+      d->r_bit = GetGpioFromLedSequence('R', h.p0_r1, h.p0_g1, h.p0_b1);
+      d->g_bit = GetGpioFromLedSequence('G', h.p0_r1, h.p0_g1, h.p0_b1);
+      d->b_bit = GetGpioFromLedSequence('B', h.p0_r1, h.p0_g1, h.p0_b1);
     } else {
-      d->r_bit = h.p0_r2;
-      d->g_bit = h.p0_g2;
-      d->b_bit = h.p0_b2;
+      d->r_bit = GetGpioFromLedSequence('R', h.p0_r2, h.p0_g2, h.p0_b2);
+      d->g_bit = GetGpioFromLedSequence('G', h.p0_r2, h.p0_g2, h.p0_b2);
+      d->b_bit = GetGpioFromLedSequence('B', h.p0_r2, h.p0_g2, h.p0_b2);
     }
   }
   else if (y >= rows_ && y < 2 * rows_) {
     if (y - rows_ < double_rows_) {
-      d->r_bit = h.p1_r1;
-      d->g_bit = h.p1_g1;
-      d->b_bit = h.p1_b1;
+      d->r_bit = GetGpioFromLedSequence('R', h.p1_r1, h.p1_g1, h.p1_b1);
+      d->g_bit = GetGpioFromLedSequence('G', h.p1_r1, h.p1_g1, h.p1_b1);
+      d->b_bit = GetGpioFromLedSequence('B', h.p1_r1, h.p1_g1, h.p1_b1);
     } else {
-      d->r_bit = h.p1_r2;
-      d->g_bit = h.p1_g2;
-      d->b_bit = h.p1_b2;
+      d->r_bit = GetGpioFromLedSequence('R', h.p1_r2, h.p1_g2, h.p1_b2);
+      d->g_bit = GetGpioFromLedSequence('G', h.p1_r2, h.p1_g2, h.p1_b2);
+      d->b_bit = GetGpioFromLedSequence('B', h.p1_r2, h.p1_g2, h.p1_b2);
     }
   }
   else {
     if (y - 2*rows_ < double_rows_) {
-      d->r_bit = h.p2_r1;
-      d->g_bit = h.p2_g1;
-      d->b_bit = h.p2_b1;
+      d->r_bit = GetGpioFromLedSequence('R', h.p2_r1, h.p2_g1, h.p2_b1);
+      d->g_bit = GetGpioFromLedSequence('G', h.p2_r1, h.p2_g1, h.p2_b1);
+      d->b_bit = GetGpioFromLedSequence('B', h.p2_r1, h.p2_g1, h.p2_b1);
     } else {
-      d->r_bit = h.p2_r2;
-      d->g_bit = h.p2_g2;
-      d->b_bit = h.p2_b2;
+      d->r_bit = GetGpioFromLedSequence('R', h.p2_r2, h.p2_g2, h.p2_b2);
+      d->g_bit = GetGpioFromLedSequence('G', h.p2_r2, h.p2_g2, h.p2_b2);
+      d->b_bit = GetGpioFromLedSequence('B', h.p2_r2, h.p2_g2, h.p2_b2);
     }
   }
 

lib/led-matrix-c.cc

@@ -62,7 +62,7 @@ struct RGBLedMatrix *led_matrix_create_from_options(
     OPT_COPY_IF_SET(scan_mode);
     OPT_COPY_IF_SET(disable_hardware_pulsing);
     OPT_COPY_IF_SET(show_refresh_rate);
-    OPT_COPY_IF_SET(swap_green_blue);
+    OPT_COPY_IF_SET(led_rgb_sequence);
     OPT_COPY_IF_SET(inverse_colors);
 #undef OPT_COPY_IF_SET
   }
@@ -87,7 +87,7 @@ struct RGBLedMatrix *led_matrix_create_from_options(
     ACTUAL_VALUE_BACK_TO_OPT(scan_mode);
     ACTUAL_VALUE_BACK_TO_OPT(disable_hardware_pulsing);
     ACTUAL_VALUE_BACK_TO_OPT(show_refresh_rate);
-    ACTUAL_VALUE_BACK_TO_OPT(swap_green_blue);
+    ACTUAL_VALUE_BACK_TO_OPT(led_rgb_sequence);
     ACTUAL_VALUE_BACK_TO_OPT(inverse_colors);
 #undef ACTUAL_VALUE_BACK_TO_OPT
   }

lib/led-matrix.cc

@@ -164,17 +164,12 @@ RGBMatrix::Options::Options() :
     show_refresh_rate(false),
 #endif
 
-#ifdef RGB_SWAP_GREEN_BLUE
-    swap_green_blue(true),
-#else
-    swap_green_blue(false),
-#endif
-
 #ifdef INVERSE_RGB_DISPLAY_COLORS
-    inverse_colors(true)
+    inverse_colors(true),
 #else
-    inverse_colors(false)
+    inverse_colors(false),
 #endif
+  led_rgb_sequence("RGB")
 {
   // Nothing to see here.
 }
@@ -251,7 +246,7 @@ FrameCanvas *RGBMatrix::CreateFrameCanvas() {
                                               32 * params_.chain_length,
                                               params_.parallel,
                                               params_.scan_mode,
-                                              params_.swap_green_blue,
+                                              params_.led_rgb_sequence,
                                               params_.inverse_colors,
                                               &shared_pixel_mapper_));
   if (created_frames_.empty()) {

lib/options-initialize.cc

@@ -141,6 +141,9 @@ static bool FlagInit(int &argc, char **&argv,
       if (ConsumeStringFlag("gpio-mapping", it, end,
                             &mopts->hardware_mapping, &err))
         continue;
+      if (ConsumeStringFlag("rgb-sequence", it, end,
+                            &mopts->led_rgb_sequence, &err))
+        continue;
       if (ConsumeIntFlag("rows", it, end, &mopts->rows, &err))
         continue;
       if (ConsumeIntFlag("chain", it, end, &mopts->chain_length, &err))
@@ -160,8 +163,19 @@ static bool FlagInit(int &argc, char **&argv,
         continue;
       if (ConsumeBoolFlag("inverse", it, &mopts->inverse_colors))
         continue;
-      if (ConsumeBoolFlag("swap-green-blue", it, &mopts->swap_green_blue))
+      // We don't have a swap_green_blue option anymore, but we simulate the
+      // flag for a while.
+      bool swap_green_blue;
+      if (ConsumeBoolFlag("swap-green-blue", it, &swap_green_blue)) {
+        if (strlen(mopts->led_rgb_sequence) == 3) {
+          char *new_sequence = strdup(mopts->led_rgb_sequence);
+          new_sequence[0] = mopts->led_rgb_sequence[0];
+          new_sequence[1] = mopts->led_rgb_sequence[2];
+          new_sequence[2] = mopts->led_rgb_sequence[1];
+          mopts->led_rgb_sequence = new_sequence;  // leaking. Ignore.
+        }
         continue;
+      }
       bool allow_hardware_pulsing = !mopts->disable_hardware_pulsing;
       if (ConsumeBoolFlag("hardware-pulse", it, &allow_hardware_pulsing)) {
         mopts->disable_hardware_pulsing = !allow_hardware_pulsing;
@@ -331,8 +345,8 @@ void PrintMatrixFlags(FILE *out, const RGBMatrix::Options &d,
           "\t--led-%sshow-refresh        : %show refresh rate.\n"
           "\t--led-%sinverse             "
           ": Switch if your matrix has inverse colors %s.\n"
-          "\t--led-%sswap-green-blue     : Switch if your matrix has green/blue "
-          "swapped %s.\n"
+          "\t--led-rgb-sequence        : Switch if your matrix has led colors "
+          "swapped (Default: \"RGB\")\n"
           "\t--led-pwm-lsb-nanoseconds : PWM Nanoseconds for LSB "
           "(Default: %d)\n"
           "\t--led-%shardware-pulse   : %sse hardware pin-pulse generation.\n",
@@ -341,7 +355,6 @@ void PrintMatrixFlags(FILE *out, const RGBMatrix::Options &d,
           d.pwm_bits, d.brightness, d.scan_mode,
           d.show_refresh_rate ? "no-" : "", d.show_refresh_rate ? "Don't s" : "S",
           d.inverse_colors ? "no-" : "",    d.inverse_colors ? "off" : "on",
-          d.swap_green_blue ? "no-" : "",  d.swap_green_blue ? "off" : "on",
           d.pwm_lsb_nanoseconds,
           !d.disable_hardware_pulsing ? "no-" : "",
           !d.disable_hardware_pulsing ? "Don't u" : "U");
@@ -405,6 +418,19 @@ bool RGBMatrix::Options::Validate(std::string *err_in) const {
     success = false;
   }
 
+  if (led_rgb_sequence == NULL || strlen(led_rgb_sequence) != 3) {
+    err->append("led-sequence needs to be three characters long.\n");
+    success = false;
+  } else {
+    if ((!strchr(led_rgb_sequence, 'R') && !strchr(led_rgb_sequence, 'r'))
+        || (!strchr(led_rgb_sequence, 'G') && !strchr(led_rgb_sequence, 'g'))
+        || (!strchr(led_rgb_sequence, 'B') && !strchr(led_rgb_sequence, 'b'))) {
+      err->append("led-sequence needs to contain all of letters 'R', 'G' "
+                  "and 'B'\n");
+      success = false;
+    }
+  }
+
   if (!success && !err_in) {
     // If we didn't get a string to write to, we write things to stderr.
     fprintf(stderr, "%s", err->c_str());

python/rgbmatrix/Makefile

@@ -10,3 +10,6 @@ all : core.cpp graphics.cpp
 
 %.cpp : %.pyx
 	$(CYTHON) --cplus -o $@ $^
+
+clean:
+	rm -rf core.cpp graphics.cpp