Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Neopixel Library is not processing the NEO_KHZ400 option for ESP8266 (Wemos D1 mini) #186

Open
mctrp opened this issue Dec 24, 2018 · 8 comments
Open

Neopixel Library is not processing the NEO_KHZ400 option for ESP8266 (Wemos D1 mini) #186

mctrp opened this issue Dec 24, 2018 · 8 comments

Comments

@mctrp
Copy link

mctrp commented Dec 24, 2018

Hi,

For the holidays I was working on a WS2811 LED strip (5 meter, 3 LEDs per WS2811 chip) together with a Wemos D1 mini (ESP8266) as a controller.

The Neopixel library offers the NEO_KHZ400 option to drive WS2811 type strips, since these don't use a 800 KHz pulse freq like the WS2812x do.

For some reason however the NEO_KHZ400 option is ignored when I use a board based on the ESP8266 mcu. I've tried to run a simple script (just turn on 1 led based on serial input) with an Arduino UNO and the LED strip performs as expected. Using the same exact code on the Wemos got no response. (I'm aware the Wemos has a 3.3v output voltage so I used a logic level converter to turn it into 5v)

Minimal test script:

// Based on NeoPixel Ring simple sketch (c) 2013 Shae Erisson
// released under the GPLv3 license to match the rest of the AdaFruit NeoPixel library

#include <Adafruit_NeoPixel.h>

#define PIN D2 //For the Wemos D1 mini
//#define PIN 5 //For the Arduino UNO

// How many NeoPixels are attached to the Arduino?
#define NUMPIXELS      50
#define brightness     100

// When we setup the NeoPixel library, we tell it how many pixels, and which pin to use to send signals.
// Note that for older NeoPixel strips you might need to change the third parameter--see the strandtest
// example for more information on possible values.
Adafruit_NeoPixel pixels = Adafruit_NeoPixel(NUMPIXELS, PIN, NEO_GRB + NEO_KHZ400);

void setup() {
  pinMode(PIN, OUTPUT);
  Serial.begin(115200);
  pixels.begin(); // This initializes the NeoPixel library.
  pixels.setPixelColor(0, pixels.Color(0,0,0));
  pixels.show();
}
void loop() {
    // Tigger on serial input to synchronize UNO and Wemos sending the same Neopixel command.
    if (Serial.available() > 0) {
        Serial.println(1, BIN);
        pixels.setPixelColor(0, pixels.Color(brightness,brightness,brightness));
        pixels.show();
        Serial.read(); // Read value to reset Serial.available
        delay(1000); // Debounce delay         
    }
}

I started debugging and after (quite) some time found the following difference in signal output:

normal_400khz

After zooming in a little bit you can clearly see a difference in the length of 1 bit between the UNO and the Wemos:

Arduino UNO:
normal_400khz_closeup_uno

Wemos:
normal_400khz_closeup_wemos

The UNO uses the expected frequency of 400KHz but the Wemos is using the 800KHz frequency.

After some digging around I found the following section in esp8266.c that is related to the timing of these signals. It should change the frequencies based on a boolean named "is800KHz".

#define CYCLES_800_T0H  (F_CPU / 2500000) // 0.4us
#define CYCLES_800_T1H  (F_CPU / 1250000) // 0.8us
#define CYCLES_800      (F_CPU /  800000) // 1.25us per bit
#define CYCLES_400_T0H  (F_CPU / 2000000) // 0.5uS
#define CYCLES_400_T1H  (F_CPU /  833333) // 1.2us
#define CYCLES_400      (F_CPU /  400000) // 2.5us per bit

  uint8_t *p, *end, pix, mask;
  uint32_t t, time0, time1, period, c, startTime, pinMask;

  pinMask   = _BV(pin);
  p         =  pixels;
  end       =  p + numBytes;
  pix       = *p++;
  mask      = 0x80;
  startTime = 0;

#ifdef NEO_KHZ400
  if(is800KHz) {
#endif
    time0  = CYCLES_800_T0H;
    time1  = CYCLES_800_T1H;
    period = CYCLES_800;
#ifdef NEO_KHZ400
  } else { // 400 KHz bitstream
    time0  = CYCLES_400_T0H;
    time1  = CYCLES_400_T1H;
    period = CYCLES_400;
  }
#endif

However it does not. If I force the use of the 400KHz timing scheme by changing the 800KHz to match the 400KHz ones like this:

#define CYCLES_800_T0H  (F_CPU / 2000000) 
#define CYCLES_800_T1H  (F_CPU /  833333) 
#define CYCLES_800      (F_CPU /  400000) 
#define CYCLES_400_T0H  (F_CPU / 2000000) 
#define CYCLES_400_T1H  (F_CPU /  833333) 
#define CYCLES_400      (F_CPU /  400000)

It does work and I get the following output on my logic analyser:

Overview:
forced_400khz

Arduio UNO:
forced_400khz_closeup_uno

Wemos:
forced_400khz_closeup_wemos

Now the signals seem to match in terms of frequency. Both on 400KHz.

I have the feeling it has to do with this "is800KHz" boolean that is not being set correctly somewhere. But I cannot seem to find if the problem lies in the esp8266.c itself or in Adafruit_NeoPixel.cpp that calls it.

Does anyone know where to look for the cause? Or am I missing something?

Arduino board: Wemos D1 Mini (ESP8266), Arduino UNO (genuine)
Arduino IDE version: 1.8.5
Neopixel library version: 2.4.2
@slootsky
Copy link

slootsky commented Dec 24, 2018 via email

@ladyada
Copy link
Member

ladyada commented Dec 24, 2018

yeah we dont really support 400khz - if you can submit a PR we'll take a look but like, we dont even know where to get 400khz neopixels anymore :D

@PaintYourDragon
Copy link
Contributor

I don't see anything seriously out of whack. Additionally, I don't have any 400 KHz pixels handy to test with, so I'm flying on instruments here...but...

I notice the function declaration for espShow() in Adafruit_NeoPixel.cpp (line 115) doesn't exactly match what's in esp8266.c (line 20). Last argument is uint8_t in former, boolean in latter. IN PRINCIPLE they should compile to the same thing, but maybe the ESP8266 is a strange case.

Try changing the code so it's the same in both places...either both uint8_t, or both boolean...see if the behavior is any different.

@mctrp
Copy link
Author

mctrp commented Dec 25, 2018 •
edited
Loading

Thank you for your responses! I seem to have found the last one in existence apparently :P

With your feedback I think I have found the culprit:

If I change this part of esp8266.c

#ifdef NEO_KHZ400
  if(is800KHz) {
#endif
    time0  = CYCLES_800_T0H;
    time1  = CYCLES_800_T1H;
    period = CYCLES_800;
#ifdef NEO_KHZ400
  } else { // 400 KHz bitstream
    time0  = CYCLES_400_T0H;
    time1  = CYCLES_400_T1H;
    period = CYCLES_400;
  }
#endif

to

  if(is800KHz) {
    time0  = CYCLES_800_T0H;
    time1  = CYCLES_800_T1H;
    period = CYCLES_800;
  } else { // 400 KHz bitstream
    time0  = CYCLES_400_T0H;
    time1  = CYCLES_400_T1H;
    period = CYCLES_400;
  }

The signal is correct. So it seems that the definition of NEO_KHZ400 does not carry over to the secondary c file. Is there a more elegant way of solving this? Otherwise I will submit a PR for the code change described above.

@PaintYourDragon
Copy link
Contributor

Oh snap! Good eye.

NEO_KHZ400 is ONLY not-defined on ATtiny devices. It's present on everything else, so I'm OK with the change as shown...feel free to PR, or I can make the change next time I'm in there.

Thanks for spotting it!

@Jmarinis
Copy link

I'd like to get this change implemented. Should I reference this thread in the PR?

@AstroZombieSG
Copy link

I just ran in to this today. I was working on an ATTiny85 and I grabbed a strip of neoPixels I had laying around. After some odd behavior of the LEDs (when plugged in to the ATTiny85 they flicker uncontrollably) I was able to identify a marking on the strip that said 2811, so I changed the setting to NEO_KHZ400 and received the compilation error. 'NEO_KHZ400' was not declared in this scope.

Seems like a trivial thing to fix, but also a very rare occurrence of anyone using these two combinations.

@SputnikSeven
Copy link

I also just ran into this issue today on an attiny85 and an older pololu addressable led.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@PaintYourDragon @ladyada @Jmarinis @slootsky @mctrp @AstroZombieSG @SputnikSeven