[BUG] LPC1768: Incorrect Neopixel WS2812B timing in library

[chrisqwertz]

Description

Using the timing values in the Neopixel fork by @p3p results in glitches in a Neopixel strip that has more than 4 LEDS in the strip

Steps to Reproduce

Enable NEOPIXEL_LED and PRINTER_EVENT_LEDS.

In my case the configuration is as follows:

#define NEOPIXEL_LED
#if ENABLED(NEOPIXEL_LED)
  #define NEOPIXEL_TYPE   NEO_GRB // NEO_GRBW / NEO_GRB - four/three channel driver type (defined in Adafruit_NeoPixel.h)
  #define NEOPIXEL_PIN    P1_21        // LED driving pin
  #define NEOPIXEL_PIXELS 10       // Number of LEDs in the strip
  //#define NEOPIXEL_IS_SEQUENTIAL   // Sequential display for temperature change - LED by LED. Disable to change all LEDs at once.
  #define NEOPIXEL_BRIGHTNESS 255  // Initial brightness (0-255)
  //#define NEOPIXEL_STARTUP_TEST  // Cycle through colors at startup
#endif

Expected behavior: All LEDs changing at the same time to same color

Actual behavior: Ugly multicolor glitches predominantly after the 4 LED in the strip

Additional Information

Change timing values according to WS2812B datasheet fixes all pixel errors and glitches:

#elif defined(TARGET_LPC1768)
  uint8_t  *ptr, *end, p, bitMask;
  ptr     =  pixels;
  end     =  ptr + numBytes;
  p       = *ptr++;
  bitMask =  0x80;

#ifdef NEO_KHZ400 // 800 KHz check needed only if 400 KHz support enabled
  if(is800KHz) {
#endif
    for(;;) {
      if(p & bitMask) {
        // data ONE high
        // min: 550 typ: 700 max: 5,500
        gpio_set(pin);
        time::delay_ns(800);   /* was 550 */
        // min: 450 typ: 600 max: 5,000
        gpio_clear(pin);
        time::delay_ns(450);
      } else {
        // data ZERO high
        // min: 200  typ: 350 max: 500
        gpio_set(pin);
        time::delay_ns(400);    /* was 200*/
        // data low
        // min: 450 typ: 600 max: 5,000
        gpio_clear(pin);
        time::delay_ns(850);    /* was 450*/
      }
      if(bitMask >>= 1) {
        // Move on to the next pixel
        asm("nop;");
      } else {
        if(ptr >= end) break;
        p       = *ptr++;
        bitMask = 0x80;
      }
    }
#ifdef NEO_KHZ400
  } else { // 400 KHz bitstream
    // ToDo!
  }
#endif

[Bob-the-Kuhn]

Where is this file? Is it .piolibdeps\Adafruit NeoPixel\Adafruit_NeoPixel.cpp?

DUE also has a Neopixel timing problem. It would be nice if both were solved.

I'm a bit surprised that the problems start at LED 4 and not 2. The data sheet says that the chip re-times the signal before passing it on. Its probably because we're pushing the bits faster than the nominal bit rate and the re-timing isn't perfect.

[p3p]

This is my NeoPixel fork used by LPC176x, The timings where chosen after a fair bit of research to be as fast as possible but still work well, it takes a long time to update a string of LEDs and I wanted to minimise that. It was tested on a a very long strip of (RGB) LEDs and works very well on all of mine .. but I must have pushed the margins to far using minimums and counting on overhead to give me the margin.

https://wp.josh.com/2014/05/13/ws2812-neopixels-are-not-so-finicky-once-you-get-to-know-them/ documents just how loose the spec actually is.

[Bob-the-Kuhn]

Here's my wish list:

1. Make this Neopixel library the default for Marlin
2. Change the #else // Other ARM architecture -- Presumed Arduino Due section

FROM:

  #define SCALE      VARIANT_MCK / 2UL / 1000000UL
  #define INST       (2UL * F_CPU / VARIANT_MCK)
  #define TIME_800_0 ((int)(0.40 * SCALE + 0.5) - (5 * INST))
  #define TIME_800_1 ((int)(0.80 * SCALE + 0.5) - (5 * INST))
  #define PERIOD_800 ((int)(1.25 * SCALE + 0.5) - (5 * INST))
  #define TIME_400_0 ((int)(0.50 * SCALE + 0.5) - (5 * INST))
  #define TIME_400_1 ((int)(1.20 * SCALE + 0.5) - (5 * INST))
  #define PERIOD_400 ((int)(2.50 * SCALE + 0.5) - (5 * INST))

  int             pinMask, time0, time1, period, t;

TO:

  #define SCALE      VARIANT_MCK / 2UL / 1000000UL
  #define INST       (2UL * F_CPU / VARIANT_MCK)
  #define TIME_800_0 ((int)(0.40 * SCALE + 0.5) + (3 * INST))
  #define TIME_800_1 ((int)(0.80 * SCALE + 0.5) + (3 * INST))
  #define PERIOD_800 ((int)(1.25 * SCALE + 0.5) + (3 * INST))
  #define TIME_400_0 ((int)(0.50 * SCALE + 0.5) + (3* INST))
  #define TIME_400_1 ((int)(1.20 * SCALE + 0.5) + (3 * INST))
  #define PERIOD_400 ((int)(2.50 * SCALE + 0.5) + (3 * INST))

  uint32_t       pinMask, time0, time1, period, t;

[Bob-the-Kuhn]

Here's the file with both changes. Adafruit_NeoPixel.zip

[chrisqwertz]

@p3p Can I close this?

This is working reliably for me:

#elif defined(TARGET_LPC1768)
  uint8_t  *ptr, *end, p, bitMask;
  ptr     =  pixels;
  end     =  ptr + numBytes;
  p       = *ptr++;
  bitMask =  0x80;

#ifdef NEO_KHZ400 // 800 KHz check needed only if 400 KHz support enabled
  if(is800KHz) {
#endif
    for(;;) {
      if(p & bitMask) {
        // data ONE high
        // min: 550 typ: 700 max: 5,500
        gpio_set(pin);
        time::delay_ns(700);
        // min: 450 typ: 600 max: 5,000
        gpio_clear(pin);
        time::delay_ns(350);
      } else {
        // data ZERO high
        // min: 200  typ: 350 max: 500
        gpio_set(pin);
        time::delay_ns(300);
        // data low
        // min: 450 typ: 600 max: 5,000
        gpio_clear(pin);
        time::delay_ns(750);
      }
      if(bitMask >>= 1) {
        // Move on to the next pixel
        time::delay_ns(50);
      } else {
        if(ptr >= end) break;
        p       = *ptr++;
        bitMask = 0x80;
        time::delay_ns(50);
      }
    }
#ifdef NEO_KHZ400
  } else { // 400 KHz bitstream
    // ToDo!
  }
#endif

[boelle]

Lack of Activity This issue is being closed due to lack of activity. If you have solved the issue, please let us know how you solved it. If you haven't, please tell us what else you've tried in the meantime, and possibly this issue will be reopened.

[github-actions[bot]]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.