lavallc / nrf51-neopixel

WS2812(Neopixel) library for the Nordic Semiconductor 51822 Bluetooth Low Energy ARM SoC.
MIT License
60 stars 22 forks source link

Library does not work with RedBearLab nRF51822 #1

Open kontextbewusst opened 9 years ago

kontextbewusst commented 9 years ago

Hi there,

thank you very much for this library. We were hoping to use it in a project with a RBL nRF51822 and a small Neopixel ring, but all it does is turn on all of the LEDs and set them to white. No matter what configuration we tried, the Neopixel ring never showed the colour pattern that we set in code.

Can you give me some hints on how to debug this, or how to change your code to make it work?

Thanks!

esalem commented 9 years ago

We noticed some issues with LEDs not responding properly and updated the number of asm calls to correct the timing. The github neopixel.h needs to be updated to our(Lava's) most recent version below. Let me know if that solves your issue.

define NEOPIXEL_SEND_ONE NRF_GPIO->OUTSET = (1UL << PIN); \

    __ASM ( \
            " NOP\n\t" \
            " NOP\n\t" \
            " NOP\n\t" \
            " NOP\n\t" \
            " NOP\n\t" \
            " NOP\n\t" \
            " NOP\n\t" \
            " NOP\n\t" \
            " NOP\n\t" \
        ); \
    NRF_GPIO->OUTCLR = (1UL << PIN); \

define NEOPIXEL_SEND_ZERO NRF_GPIO->OUTSET = (1UL << PIN); \

    __ASM (  \
            " NOP\n\t"  \
        );  \
    NRF_GPIO->OUTCLR = (1UL << PIN);  \
    __ASM ( \
            " NOP\n\t" \
            " NOP\n\t" \
            " NOP\n\t" \
            " NOP\n\t" \
            " NOP\n\t" \
            " NOP\n\t" \
            " NOP\n\t" \
            " NOP\n\t" \
        );
ericbarch commented 9 years ago

Just pushed up the change to the repo, thanks esalem.

kontextbewusst - can you give that a shot?

kontextbewusst commented 9 years ago

Thanks for the quick response, I'll try it when I am back in office on Monday!

kontextbewusst commented 9 years ago

I just tried the new header, same result as with the previous version, unfortunately. To give a bit more context, this is my code:

#include <stdbool.h>
#include "neopixel.h"

neopixel_strip_t m_strip;
uint8_t dig_pin_num = 16;
uint8_t leds_per_strip = 16;
uint8_t error;
uint8_t led_to_enable = 5;
uint8_t red = 255;
uint8_t green = 0;
uint8_t blue = 159;

int main(void) {
    neopixel_init(&m_strip, dig_pin_num, leds_per_strip);
    neopixel_clear(&m_strip);
    error = neopixel_set_color_and_show(&m_strip, led_to_enable, red, green, blue);
    if (error) {
        // led_to_enable was not within number leds_per_strip
    }
    while (true) {
        // stay alive
    }
}

I have also attached a photo of the resulting light pattern... not quite what you would expect.

img_2267

ericbarch commented 9 years ago

kontextbewusst - are you running this with the radio enabled? If so, are you able to test the radio notification callback method?

kontextbewusst commented 9 years ago

@ericbarch no, the code that I posted is my complete main.c, there is no SoftDevice involved.

bickster commented 9 years ago

@kontextbewusst Have you been able to get the neopixel ring working with the RedBear nRF51822 yet?

peetonn commented 9 years ago

it doesn't work for nRF51822 on BLE nano either. hey guys, can you please share your ideas behind these adjustments - https://github.com/lavallc/nrf51-neopixel/blob/master/neopixel.h#L13 ? how should I check these timings to be correct on my platform (16Mhz), for my compiler (armgcc) and the code which sets the color (I use yours for now)? thanks!

esalem commented 9 years ago

The assembly command " NOP\n\t" is a single no operation command. Based on the processor time it takes for a single operation, there are a given number of NOPS to match the specs for the Neopixel (ws2812b) per their datasheet. The changes that occurred to the code in the last patch were done because it was drifting outside of spec'ed timing and needed to be adjusted by one NOP. The most help I can supply at the moment is to add more NOPs for higher clocks and less NOPs for lower clocks. Check the datasheet and compare the assembly code to the number of NOPs to get a jist of how long one NOP takes. http://www.adafruit.com/datasheets/WS2812.pdf

As an example: 0 code ,high voltage time 0.35us +/-150ns 0 code , low voltage time 0.8us +/- 150ns NEOPIXEL_SEND_ZERO 1 NOP high 8 NOPS low

This means 800 / 8 = about 100ns per NOP...but realistically there is time for the pin to change states. So, the 0 code high voltage is only 1 NOP which might be thought to be 100 ns(spec says 350ns). Meaning time to flip states could be about 200ns.

Hope this helps you update it for your platforms.

bickster commented 9 years ago

@peetonn Check out this post on the RedBear site. I've been asking RedBear for their help. https://redbearlab.zendesk.com/entries/73654999-BLE-Nano-with-Neopixel-WS2812-mbed?page=1#post_29972715 Sounds like we have a similar problem.

I've also asked the mbed community for help as well. https://developer.mbed.org/questions/54210/RedBear-nRF51822-driving-NeoPixels/

esalem commented 9 years ago

I have not looked into the newer version of the Softdevice beyond S110. I would imagine that since RedBear is using S130 there may be a conflict since this library directly couples with the softdevice.

I believe the clock speed is the same(don't quote me on that), so I would recommend trying the library without the soft device enabled.Try the example code, but remove the neopixel_clear and destroy calls. If that works, then there has been updates to the ble_radio_callback. If the non softdevice example doesn't work then there are timing issues that need to be resolved with an oscilloscope or logic analyzer. maybe @ericbarch can look into them since he has been more up to date on the lastest nrf51822 stuff and probably has the hardware.

bickster commented 9 years ago

FYI. RedBear posted some code that worked for me using the Nano and the nRF51822. https://redbearlab.zendesk.com/entries/73654999-BLE-Nano-with-Neopixel-WS2812-mbed?page=1#post_29972715

Can anyone see how it's different from this library? The RedBear code can only turn on 1 LED. I would like to turn on more than 1.

esalem commented 9 years ago

Just a quick look at it shows this:

uint8_t rgb[3] = {g, r, b};
uint8_t *p = rgb;
uint8_t *end = p + 3;

while (p < end)
{
    uint8_t pix = *p++;

... The rgb values you pass goes into the array used for the first pixel. But the pointer to the data passed into the "pix" variable is increased which refers to a new area of memory that never got the rgb variables. If this is idea correct, then to fix it you need to state that for all the memory you will be using there are rgb values:

    uint8_t rgb[3] = {g, r, b};
    uint8_t *p = rgb;
    uint8_t *end = p + 3;
    for (uint8_t * i = p; i < p + sizeof(rgb) * numleds; i+=sizeof(rgb)) {
         i[0] = g;
        i[1] = r;
        i[2] = b;
    }
   //Now the data is all setup... granted I realize some of this is c++ code and numleds was never created. 
    //But hopefully you understand.

You need to setup the data beforehand which is why the lava library creates structs to store all the data:

///this represents their rgb array
//the redbear forum code creates one of these and then just moves along a pointer to uninitialized memory.
//Really, you need an array of this array filled with data.
typedef union {
        struct {
            uint8_t g, r, b;
        }simple;
    uint8_t grb[3];
} color_t;

//this is what the redbear forum code is missing.
//the neopixel.c allocates and initializes rgb values for an array pointed to by "leds"
typedef struct {
    uint8_t pin_num;
    uint16_t num_leds;
    color_t *leds;
} neopixel_strip_t;

Sorry for the runon information and lack of final answer. I really hate writing the actual code because people have many different applications and I'm sure many will read this. I also jump between languages too often to remember what syntax is what. I just hope someone reading it found the thoughts useful.

peetonn commented 9 years ago

A small update from me - I made it to work by combining Lava's code with the code posted on ReadBearLab forum. Just update these macro in neopixel.h:

#define NEOPIXEL_SEND_ONE   NRF_GPIO->OUTSET = (1UL << PIN); \
                NRF_GPIO->OUTSET = (1UL << PIN); \
                NRF_GPIO->OUTSET = (1UL << PIN); \
                NRF_GPIO->OUTCLR = (1UL << PIN);
#define NEOPIXEL_SEND_ZERO  NRF_GPIO->OUTSET = (1UL << PIN); \
                NRF_GPIO->OUTCLR = (1UL << PIN); \
                NRF_GPIO->OUTCLR = (1UL << PIN); \
                NRF_GPIO->OUTCLR = (1UL << PIN); \
                NRF_GPIO->OUTCLR = (1UL << PIN);

These timings work for my BLE nano. I use cross-compiling using armgcc v4.9.3, nRF51822 SDK v8.1.0 and SoftDevice v110. It works fine, however I can see small flickering on closer look. I'll check timings later with oscilloscope if I find one.

bickster commented 9 years ago

Thanks @esalem @peetonn With the code changes you just posted I was able to get Lava's code with the BLE nano working using mbed.

peetonn commented 9 years ago

@bickster nice! also, I'm wondering - why does the color need to be set in radio callback and only when radio_active == false? is there any justification for that in documentation?

bickster commented 9 years ago

I haven't experienced any issues with the Bluetooth radio being on. We have an iOS app that connects to the Nano and sends/receives messages while the Nano illuminates the NeoPixel strip. I forked the this library. I'll post my changes soon.

esalem commented 9 years ago

@peetonn The LEDs are written via a bitbashed protocol(just timing the pins high and low) rather than an interrupt/timer based protocol. This means that whenever an interrupt occurs it will stop the writing to the neopixels. This would not only write to less of the LEDs, but potentially mess up the rgb value to that last LED it was writing to when interrupted. This is usually corrected by another write to the LEDs within a short period of time(think refresh rate). However, the human eye can see this "flicker" if you will. If you are lucky and only using a few LEDs in a strip you may never see this issue. However, you use 40 LEDs like us and the time adds up.

~1.1us per bit of data x 8 bits per byte = 8.8 us x 3 bytes per LED(RGB) = 26.4 x 40 LEDs = 1.056 ms (note Milliseconds now)

1 millisecond is a long time to be blocking everything on a micro from doing anything. Realistically its longer. For some timer and low priority interrupts we can just do the DISABLE_INTERRUPTS() macro that nordic has(not the actual macro, but there is one to do this) and your interrupts won't intervene... BUT

Here's the big but. The radio is on a schedule and WILL interrupt as it has the highest priority given by the soft device. So now we have two periods of time. Radio(BLE comms) is NOTACTIVE and radio is ACTIVE because the radio can be on for awhile-think milliseconds. So, you enable the interrupt and toggle the bit everytime the radio comes on and goes off. This lava library makes sure to write to the LEDs when the Radio has turned off and it guarantees we have a lot of time to write to them before it will interrupt. It also gives a predictable refresh rate dependent on the Radio speed.

Oh and one more note. There is another catch that really got us. The radio turns on exactly as predicted, but there are interrupts that trigger for it just before hand meaning you need to be done with whatever write to LEDs well before the radio is scheduled to turn on. IIRC it was like 1 ms before the radio turned on. Now, this could all be different with the new softdevice, but with S110 the radio was a blocking function that interrupted whatever processing(writing to LEDs) that we were doing.

In the end, everything depends on BLE comms rate, number of leds written to, and luck(unless you practice safe programming).

peetonn commented 9 years ago

right, interruptions!! thanks, @esalem for comprehensive answer!

peetonn commented 9 years ago

wrote this test code to iterate through 7 LEDs sequentially in radio callback (there are ):

static int counter = 1;
static int ledId = 0;
void radio_callback_handler(bool radio_active)
{
    if (!radio_active)
    {
        if (counter%10 == 0)
        {
            clearLedAll();
            ledId = (ledId + 1) % (NeoStrip.num_leds-1);
        }
        else
            neopixel_set_color(ledId, 0xff, 0x00, 0x00);
        counter++;
        neopixel_show(&NeoStrip);
    }
}

interestingly, the frequency of iterating through all LEDs is different in different states: when device is not connected it's about 7 LEDs per second, however, when connection is established (using Light Blue app on OS X) everything slows down - LEDs are lit for about 10 seconds and off for about 1 second. I suspect radio callbacks have different frequencies in different modes. Could that be true?

esalem commented 9 years ago

That is because the BLE connection comms interval is different than advertising. When connected to a smart device, usually the smart device will dictate the frequency of communication. I believe iOS device are around 20ms while androids tend to allow the other device to decide(you should have a preferred connection rate you can set). This differs from the unconnected state which is decided solely by your code/device and is called the advertising interval.