Coding Simultaneous Output to Neopixel and an LED

Hello!

First time to this forum, forgive me if I’ve done anything procedurally incorrect.

So, I’ve created a circuit using a Bleufruit Feather nRF52 as a controller, and I’m having minor issues with my code.

I wrote my code so that when I press a button on my phone, a green LED will turn on for a couple seconds and a neopixel ring will simultaneously pulse green before turning off. The same occurs with red. The problem is that the neopixel ring pulses after the green or red LED has completed its temporary flash. I’ve posted a query on adafruit and I was told it was due to the 64 step sequence I wrote within the neopixel code.

I was directed to this link: https://www.arduino.cc/en/Tutorial/BlinkWithoutDelay

Unfortunately, I’m hardly savvy when it comes to coding, and I still don’t understand what I must write in order to sync up my LEDs and my neopixels.

Thanks in advance for any help; the code is included!

-J

#include <bluefruit.h>
#include <Arduino.h>
#include <Adafruit_NeoPixel.h>

BLEUart bleuart;

// Function prototypes for packetparser.cpp
uint8_t readPacket (BLEUart *ble_uart, uint16_t timeout);
float   parsefloat (uint8_t *buffer);
void    printHex   (const uint8_t * data, const uint32_t numBytes);

// Packet buffer
extern uint8_t packetbuffer[];

//Constants
const int ledPinON =  A1;  
const int ledPinOFF = A3;

#define PIN 15
Adafruit_NeoPixel strip = Adafruit_NeoPixel(12, PIN, NEO_RGBW + NEO_KHZ800);

void setup(void)
{
  pinMode(ledPinON, OUTPUT);
  pinMode(ledPinOFF, OUTPUT);

  digitalWrite(ledPinON, LOW);
  digitalWrite(ledPinOFF, LOW);
  
  Serial.begin(115200);
  Serial.println(F("Adafruit Bluefruit52 Controller App Example"));
  Serial.println(F("-------------------------------------------"));

  Bluefruit.begin();
  // Set max power. Accepted values are: -40, -30, -20, -16, -12, -8, -4, 0, 4
  Bluefruit.setTxPower(4);
  Bluefruit.setName("Bluefruit52");

  bleuart.begin();

  startAdv();

  Serial.println(F("Please use Adafruit Bluefruit LE app to connect in Controller mode"));
  Serial.println(F("Then activate/use the sensors, color picker, game controller, etc!"));
  Serial.println();  

  strip.begin();
  strip.show();// Initialize all pixels to 'off'
  strip.setBrightness(64);
  
}

void startAdv(void)
{
  // Advertising packet
  Bluefruit.Advertising.addFlags(BLE_GAP_ADV_FLAGS_LE_ONLY_GENERAL_DISC_MODE);
  Bluefruit.Advertising.addTxPower();
  
  Bluefruit.Advertising.addService(bleuart);
  
  Bluefruit.ScanResponse.addName();
  
  Bluefruit.Advertising.restartOnDisconnect(true);
  Bluefruit.Advertising.setInterval(32, 244);    // in unit of 0.625 ms
  Bluefruit.Advertising.setFastTimeout(30);      // number of seconds in fast mode
  Bluefruit.Advertising.start(0);                // 0 = Don't stop advertising after n seconds  
}

void loop(void)
{
  uint8_t len = readPacket(&bleuart, 500);
  if (len == 0) return;

  if (packetbuffer[1] == 'B') {
    uint8_t buttnum = packetbuffer[2] - '0';
    boolean pressed = packetbuffer[3] - '0';
    
    if (pressed) 
    {
      if(buttnum == 5)  {
        //Serial.println("button1");
        digitalWrite(ledPinON, HIGH);
        delay(1000);
        digitalWrite(ledPinON,LOW);

        brightenON();
        darkenON();
      } 
      
      if(buttnum == 6){
        //Serial.println("button2");
        digitalWrite(ledPinOFF, HIGH);
        delay(1000);
        digitalWrite(ledPinOFF,LOW);

        brightenOFF();
        darkenOFF();
      }
    }
  }
  }

void brightenON() {
  uint16_t i, j;
  
  for (j = 0; j < 64; j++) {
    for (i = 0; i < strip.numPixels(); i++) {
      strip.setPixelColor(i, j, 0, 0, 0);
    }
    strip.show();
    delay(15);
  }
}

void darkenON() {
  Serial.begin(9600);
  uint16_t i, j;

  for (j = 64; j > 0; j--) {
    for (i = 0; i < strip.numPixels(); i++) {
      strip.setPixelColor(i, j, 0, 0, 0);
    }
    strip.show();
    delay(15);
    Serial.println(j);
  }
}

void brightenOFF() {
  uint16_t i, j;

  for (j = 0; j < 64; j++) {
    for (i = 0; i < strip.numPixels(); i++) {
      strip.setPixelColor(i, 0, j, 0, 0);
    }
    strip.show();
    delay(15);
  }
}

void darkenOFF() {
  Serial.begin(9600);
  uint16_t i, j;

  for (j = 64; j > 0; j--) {
    for (i = 0; i < strip.numPixels(); i++) {
      strip.setPixelColor(i, 0, j, 0, 0);
    }
    strip.show();
    delay(15);
    Serial.println(j);
  }
}

You must remove all delays and for loops from your code, and replace the functions you want to do with a state machine.

http://www.thebox.myzen.co.uk/Tutorial/State_Machine.html

Alternatively it seems to me you could just turn on the LED, do the pulsing and then turn off the LED. Again no delay needed between the turn on and off just the call to your pulsing ring function.

Grumpy_Mike:
You must remove all delays and for loops from your code

Not all for loops, wouldn’t you agree Mike?

For example:

  for (j = 0; j < 64; j++) {
    for (i = 0; i < strip.numPixels(); i++) {
      strip.setPixelColor(i, j, 0, 0, 0);
    }
    strip.show();
    delay(15);
  }

The inner for loop is fine. The outer for loop needs to go. The fact that that the outer for loop has a delay in it is the clue to that.

Not all for loops, wouldn't you agree Mike?

Agreed, but I don’t think he needs a state machine at all if he just calls that function after turning on the LED and turn off the LED when it returns.

However I am not sure why he wants to set the colour to black 64 times, once would do.

Yes, is confusing!

Using this function signature:

strip.setPixelColor(i, 0, j, 0, 0);

implies that the neopixels are RGBW type. I have only ever seen rings made of the regular RGB type.

EDIT: scratch that, AdaFruit now make a range of RGBW rings, including a 12-led ring!

Grumpy_Mike:
However I am not sure why he wants to set the colour to black 64 times, once would do.

Where is that in the OP's code?

Sorry that was your example in reply#2.

Ok, so, JKSu, back to Mike's instructions in post #1 and the blinkWithoutDelay example. To be able to do this, you are going to need to become a bit more code savvy. We are not here to do it for you but we do want to encourage you to take those next steps and guide you towards the answer. So give it a go and post what you come up with, even if it won't compile (and if it won't compile, post the entire list of errors from the IDE, also in code tags).

The advice you got from the AdaFruit forum was not correct, or perhaps you misunderstood the answer. It is not the 64 steps that is the problem, its the use of delay(). All delay does is waste a bit of time. Nothing else can happen until the delay is over. The other forum directed you to the blinkWithoutDelay example, so I suspect they did understand your problem but you did not understand the answer.

The idea of blink without delay is to remove all delays (or at least all but the very short ones). Any steps that used to happen after the delay had finished must be moved to another part of the sketch, where they can be triggered at the appropriate time. How does the sketch know when the appropriate time is? By recording the time (the value of millis()) when an event (such as a button press) occurs, then checking the time regularly and calculating how much time has passed since the event. A bit like using a clock with a second hand. Once the required time has passed, the remaining actions can be done.

@PaulRB @Grumpy_Mike

Thank you both for such a fast response, and for the time taken to answer my question - it is much appreciated! Apologies for my delay as well, I didn't receive a notification.

As for your advice, I have not yet had time to implement your suggestions; I simply wanted to post a quick response to let you know I haven't abandoned!

The 64 step, for clarity, is a loop to adjust the brightness from 0-64 and 64-0 again, which is how my NeoPixel ring pulses.

Also, this is what the rep at Adafruit prescribed exactly:

"That's a side-effect of how your code is written. The functions that animate the NeoPixels like this
one run a complete 64-step sequence before execution returns to loop() so the CPU can check for
more incoming control packets.

If you want to read input while an animation is running, you'll need to rearrange the code so it
only displays a single frame of the animation before checking for BLE input again.

The basic structure you want is demonstrated in the BlinkWithoutDelay example."

And you're correct Paul that I'm not great with code, the code is a Frankenstein of my own input and other example codes.

My code to pulse my NeoPixel currently resides in a for loop outside of the main void(loop). Should this be the proper way of creating a pulse, or am I able to place this code within the main void(loop) as opposed to outside?

Thanks in advance for your patience!

No, putting your code in a for loop in a function outside loop() does nothing to fix the problem. It just moves the problem! Feel free to create your own functions if you find they make your code more readable, or allow you to avoid repeating code, the are lots of good reasons for using them. But they won’t magically fix your current problem. You have to get rid of the delay() calls (very short ones are sometimes useful and ok). Each section of code, whether its inside loop() or in a separate function called from loop(), must execute quickly so that other sections of code can also be executed in quick succession. Often, the only thing a section of code will do is check the time (millis()) and decide it is not yet time to perform their action and so they just finish, allowing another section of code to run.

I thought the problem was that your LED on / off and the ring pulse were not happening at the same time, but it seems like it is not and you want to be able to read input while the ring is pulsing and if something new comes along either abandon the pulsing and / or start a new pulsing sequence. Can you confirm if that is what you want?

The key piece of advice from AdaFruit is:-

If you want to read input while an animation is running, you'll need to rearrange the code so it
only displays a single frame of the animation before checking for BLE input again.

So the technique for doing this is to remove the for loop and the delay and do it by hand. That is the blink without delay technique for calling the pulsing task which just makes one step and then returns. So this means each time the pulsing task is entered what was the for loop's index variable gets incremented, and if this is not at its limit the next animation step is made. If it is at its limit to raise a flag ( set a boolean variable ) which stops this task from being called by the time out on the blink without delay task calling part.

@Grumpy_Mike

That is my problem yes: I would like my LEDs to turn on immediately as the pulse animation happens in the ring. The LEDs will be replaced later with another component; for now they are simply there so I know that there is voltage in those branches of my circuit.

In short, I have two inputs. An up arrow button on my phone and a down arrow button on my phone. If I press the up-arrow, the green LED should light up and the neopixel ring should begin its pulse animation at the same time. The same will be the case for the down-arrow and the red LED.

I will fiddle around tonight and see what I can produce!

@PaulRB

I did attempt to implement a time check (millis()), but to no avail. I'll have to revisit that then.

I would like my LEDs to turn on immediately as the pulse animation happens in the ring.

So given that at the moment you get:-

The problem is that the neopixel ring pulses after the green or red LED has completed its temporary flash

Then replace:-

if (pressed)
    {
      if(buttnum == 5)  {
        //Serial.println("button1");
        digitalWrite(ledPinON, HIGH);
        delay(1000);
        digitalWrite(ledPinON,LOW);

        brightenON();
        darkenON();
      }

with

if (pressed)
    {
      if(buttnum == 5)  {
        //Serial.println("button1");
        digitalWrite(ledPinON, HIGH);

        brightenON();
        darkenON();

        digitalWrite(ledPinON,LOW);

      }

and likewise for the other buttnum vaalue

So, I actually managed to get the LEDs to initiate simultaneously with the NeoPixel, using the millis() function.

@Grumpy_Mike I suppose that the remedy you just prescribed would result in the same.

My issue is, and I realize that I did not specify this prior, that the LED stays on for as long as it takes for the NeoPixel to complete its pulse (which I defined as 3 seconds). I actually want the LED to stay on for about 1 second and the NeoPixel pulse cycle to complete after 3 seconds.

I attempted to code the Neopixel pulse with a separate millis() clause; however, the LED and neopixel duration still seem set to complete over the same time. I also tried to replace the delay(15) within the pulse codes with a millis() condition; however, this would simply prevent any of the components from working in the circuit (perhaps it was written incorrectly).

Here is a copy of the [void loop(void)] code that has the LED on as long as the NeoPixel takes to pulse.

void loop(void)
{
  uint8_t len = readPacket(&bleuart, 500);
  if (len == 0) return;
   
  if (packetbuffer[1] == 'B') {
    uint8_t buttnum = packetbuffer[2] - '0';
    boolean pressed = packetbuffer[3] - '0';

    unsigned long currentMillis = millis();
    
    if (pressed) 
    {
      if(buttnum == 5) {
        //Serial.println("button1");
        digitalWrite(ledPinON, HIGH);
        brightenON();
        darkenON();
      }

      if((buttnum == 5) && (currentMillis - previousMillis >= LEDtime)) {
        //delay(1000);
        digitalWrite(ledPinON,LOW);
        //previousMillis = currentMillis;
      } 
      
      if(buttnum == 6){
        //Serial.println("button2");
        digitalWrite(ledPinOFF, HIGH);
        
        brightenOFF();
        darkenOFF();
      }

      if((buttnum == 6) && (currentMillis - previousMillis >= LEDtime)) {
        //delay(1000);
        digitalWrite(ledPinOFF,LOW);
        previousMillis = currentMillis;
      }
    }
  }
}
 

void brightenON() {
  uint16_t i, j;

  for (j = 0; j < 64; j++) {
    for (i = 0; i < strip.numPixels(); i++) {
      strip.setPixelColor(i, j, 0, 0, 0);
    }
    strip.show();
    delay(15);
  }
}

void darkenON() {
  Serial.begin(9600);
  uint16_t i, j;

  for (j = 64; j > 0; j--) {
    for (i = 0; i < strip.numPixels(); i++) {
      strip.setPixelColor(i, j, 0, 0, 0);
    }
    strip.show();
    delay(15);
    Serial.println(j);
  }
}

void brightenOFF() {
  uint16_t i, j;

  for (j = 0; j < 64; j++) {
    for (i = 0; i < strip.numPixels(); i++) {
      strip.setPixelColor(i, 0, j, 0, 0);
    }
    strip.show();
    delay(15);
  }
}

void darkenOFF() {
  Serial.begin(9600);
  uint16_t i, j;

  for (j = 64; j > 0; j--) {
    for (i = 0; i < strip.numPixels(); i++) {
      strip.setPixelColor(i, 0, j, 0, 0);
    }
    strip.show();
    delay(15);
    Serial.println(j);
  }
}

So I suppose in summary, my revised question is: how do I make it so that the LED and neoPixel turn on simultaneously, provided that the LED remain on for 1 second and the neoPixel pulse through a period of 3 seconds?

You are not getting what we are saying. Your code structure has hardly changed and there are still 4 delay() calls. You need to forget this code for a while. Go and study the blinkWithoutDelay example until you understand the principle it teaches.

My issue is, and I realize that I did not specify this prior, that the LED stays on for as long as it takes for the NeoPixel to complete its pulse (which I defined as 3 seconds). I actually want the LED to stay on for about 1 second and the NeoPixel pulse cycle to complete after 3 seconds.

It would have helped enormously if you had said that at the beginning, so no my code modifacatiin would not have worked.

See my reply #4 in this thread for an example of the Neopixel demo program written as a state machine. FastLED/NeoPixel examples that are non-blocking? - LEDs and Multiplexing - Arduino Forum

Then read again what we have been saying in this thread.