Is an interrupt the right choice?

Hello!

I have a neopixel led strip and my goal is to make it bluetooth connected but this led to a problem where the effects (specifically the built-in "rainbow effect") makes it change after the rainbow effect is done(multiple seconds), not when it recieves the bluetooth input.

I've managed to make it able to change between static colors but whats the meaning of using the neopixel if i cant have dank effects right? :wink: so my question is if i can use either the input of the bluetooth module to somehow interrupt everything and in that case how or should i rewrite the rainbow function? my guess is it some sort of delay is making it queue the input and after that switch the effect.

im at the point now where i tried to remove the checking of the bluetooth input in the "void loop" and instead made it into a void BLINPUT as i didnt know if two of the same functions would disturb one another. in short, it doesnt work now :slight_smile:

Also, i removed the delay on the original rainbow effect and replaced it with a code i found on the forum for testing purposes to try and see if it worked, the "delay" worked but the problem with the interrupt still persists

#include <Adafruit_NeoPixel.h>
#include <SoftwareSerial.h>
SoftwareSerial BT(2, 3);
#define pixelpin 6
#define ledcount 4
#define brightness 100

Adafruit_NeoPixel strip(ledcount, pixelpin, NEO_GRB + NEO_KHZ800);

char value;

void BLINPUT() {
    if(BT.available()) {
     value = BT.read();
    BT.print(value);
    }
  }
  


void setup() {
  attachInterrupt(digitalPinToInterrupt(3), BLINPUT, CHANGE);
  
  BT.begin(9600);
  
  BT.print("Ready");
  
  strip.setBrightness(brightness);  

  strip.begin();
  
  strip.show();
  

}



void loop() {
  // put your main code here, to run repeatedly:
//if(BT.available()) {
     //value = BT.read();
    //BT.print(value);
      
  if (value=='a') {
for(int i = 0; i<strip.numPixels(); i++){
   strip.setPixelColor(i, 255, 0, 0);
}
      strip.show();
  }

if (value=='b'){
for(int i = 0; i<strip.numPixels(); i++){
   strip.setPixelColor(i, 0, 255, 0);
 
}
      strip.show();  
}

if (value=='c') {
for(int i = 0; i<strip.numPixels(); i++){
   strip.setPixelColor(i, 0, 0, 255);
}
      strip.show();
  }
  
if (value=='d') {
      rainbow(50);
  }  
}
  void altdelay(int del) {
  unsigned long myPrevMillis = millis();
  while (millis()- myPrevMillis <= del);
  }
  
void rainbow(int wait) {
  // Hue of first pixel runs 5 complete loops through the color wheel.
  // Color wheel has a range of 65536 but it's OK if we roll over, so
  // just count from 0 to 5*65536. Adding 256 to firstPixelHue each time
  // means we'll make 5*65536/256 = 1280 passes through this outer loop:
  for(long firstPixelHue = 0; firstPixelHue < 5*65536; firstPixelHue += 256) {
    for(int i=0; i<strip.numPixels(); i++) { // For each pixel in strip...
      // Offset pixel hue by an amount to make one full revolution of the
      // color wheel (range of 65536) along the length of the strip
      // (strip.numPixels() steps):
      int pixelHue = firstPixelHue + (i * 65536L / strip.numPixels());
      // strip.ColorHSV() can take 1 or 3 arguments: a hue (0 to 65535) or
      // optionally add saturation and value (brightness) (each 0 to 255).
      // Here we're using just the single-argument hue variant. The result
      // is passed through strip.gamma32() to provide 'truer' colors
      // before assigning to each pixel:
      strip.setPixelColor(i, strip.gamma32(strip.ColorHSV(pixelHue)));
}
    

    strip.show(); // Update strip with new contents
    altdelay(50);  // Pause for a moment
  }
}

Im doing all this with an Arduino Uno, HC-06 module and a led strip of 4 neopixel if its important!

Thanks!

No, an interrupt is not the correct choice. Also your altdelay function is no better than using delay. It seems you’ve completely misunderstood the purpose of using millis() for timing.

The correct way to do this is to change the rainbow effect so that it does ONE iteration of the outer “for(long firstPixelHue...” loop each time it is called...

long firstPixelHue = 0;

void rainbow() {
  for(int i=0; i<strip.numPixels(); i++) { // For each pixel in strip...
    int pixelHue = firstPixelHue + (i * 65536L / strip.numPixels());
    strip.setPixelColor(i, strip.gamma32(strip.ColorHSV(pixelHue)));
  }
  strip.show(); // Update strip with new contents
  firstPixelHue += 256;
  if (firstPixelHue >= 5*65536) firstPixelHue = 0;
}

Now use the blink without delay technique from loop() to call rainbow() every 50ms when the rainbow effect needs to be displayed.

Now loop() can both wait for new Bluetooth input while at the same time performing a single step of the rainbow animation every 50ms.

 strip.setBrightness(brightness);  

  strip.begin();

I'm pretty sure you be better off calling begin() before any other calls to your strip.

-jim lee

pcbbc:
No, an interrupt is not the correct choice. Also your altdelay function is no better than using delay. It seems you’ve completely misunderstood the purpose of using millis() for timing.

The correct way to do this is to change the rainbow effect so that it does ONE iteration of the outer “for(long firstPixelHue...” loop each time it is called...

long firstPixelHue = 0;

void rainbow() {
  for(int i=0; i<strip.numPixels(); i++) { // For each pixel in strip...
    int pixelHue = firstPixelHue + (i * 65536L / strip.numPixels());
    strip.setPixelColor(i, strip.gamma32(strip.ColorHSV(pixelHue)));
  }
  strip.show(); // Update strip with new contents
  firstPixelHue += 256;
  if (firstPixelHue >= 5*65536) firstPixelHue = 0;
}




Now use the blink without delay technique from loop() to call rainbow() every 50ms when the rainbow effect needs to be displayed.

Now loop() can both wait for new Bluetooth input while at the same time performing a single step of the rainbow animation every 50ms.

Hi, thanks for your response, havent been able to respond until now.
Your right, i most likely know what blink without delay does, but not how to apply it to my own code!
I've read myself blind on the example and havent gotten anywiser so i'll figure something out.

However, without testing the code, will this make it run continously until next command is received?

thanks again, i'll do some homework on the blinkwithout delay

ax3lh:
However, without testing the code, will this make it run continuously until next command is received?

Yes.

You need to get out of the mindset of linear programming (where you perform each step in turn and is therefore "blocking code")...

Wait for a key to be be pressed
If the key is the rainbow pattern...
  loop
    Do one step of the rainbow pattern
    Wait X ms

And start thinking about an event/state driven model...

If there is a key available...
  Read the key
  If the key is a request for the "rainbow pattern"...
    Set the state to "display rainbow pattern"

If the current state is "display rainbow pattern"...
  If X ms has elapsed since last time we did any work...
    Do one step of the rainbow pattern
    Advance the rainbow counter so next time we display the next colour in the sequence

Now your code appears to do both things at the same time:
a) Check for serial input
b) Display the rainbow pattern

If a key press is received mid-sequence the program can react to it and change the current state so the rest of the program does something else.

//libraries
#include <Adafruit_NeoPixel.h>
#include <SoftwareSerial.h>
//BT Initialisation
SoftwareSerial BT(2, 3);

//define
#define pixelpin 6
#define ledcount 4
#define brightness 100

//Neopixel
Adafruit_NeoPixel strip(ledcount, pixelpin, NEO_GRB + NEO_KHZ800);
//Milli timer
unsigned long previousMillis = 0;
const long rainbowtimer = 50;


//Variables
char value;

void rainbow() {
     if (currentMillis - previousMillis >= rainbowtimer) {
           previousMillis = currentMillis; 
  for(long firstPixelHue = 0; firstPixelHue < 5*65536; firstPixelHue += 256) {
    for(int i=0; i<strip.numPixels(); i++) { 
    int pixelHue = firstPixelHue + (i * 65536L / strip.numPixels());
      strip.setPixelColor(i, strip.gamma32(strip.ColorHSV(pixelHue)));
    }
    strip.show(); // Update strip with new contents
  firstPixelHue += 256;
  if (firstPixelHue >= 5*65536) firstPixelHue = 0;

  }
      }
}
    

void BLINPUT() {
    if(BT.available()) {
     value = BT.read();
    BT.print(value);
    }
  }
  


void setup() {
  //Bluetooth setup
  BT.begin(9600);
  BT.print("Ready");
  
//Neopixel initialisation
  strip.begin();
  strip.setBrightness(brightness);  
  strip.show();
 
}



void loop() {
   //put your main code here, to run repeatedly:
 unsigned long currentMillis = millis();
   
if(BT.available()) {
     value = BT.read();
    BT.print(value);
      
  if (value=='a') {
for(int i = 0; i<strip.numPixels(); i++){
   strip.setPixelColor(i, 255, 0, 0);
}
      strip.show();
  }

if (value=='b'){
for(int i = 0; i<strip.numPixels(); i++){
   strip.setPixelColor(i, 0, 255, 0);
 
}
      strip.show();  
}

if (value=='c') {
for(int i = 0; i<strip.numPixels(); i++){
   strip.setPixelColor(i, 0, 0, 255);
}
      strip.show();
  }
  
if (value=='d') {
  rainbow();
  }

  if (value=='o') {
    strip.clear();
    strip.show();
    }
}
}

Hello again , after trying multiple things i am once again asking for advice.

I'm pretty sure im missing something important. When i try to pass this code down it complains as the function is called before the unsigned long in the loop. if i move the function down the if statement will complain for the same reason. have i done something right, am i on the right track or did i just make it worse?

The timer for the rainbow function must be on top to nest the rainbow function in the timer also, right? or am i lost?

also tried to make the code a less strain for the eyes, dont know if it actually did it cleaner though lol

Thanks!

pcbbc:
Yes.

You need to get out of the mindset of linear programming (where you perform each step in turn and is therefore "blocking code")...

Wait for a key to be be pressed

If the key is the rainbow pattern...
  loop
    Do one step of the rainbow pattern
    Wait X ms




And start thinking about an event/state driven model...


If there is a key available...
  Read the key
  If the key is a request for the "rainbow pattern"...
    Set the state to "display rainbow pattern"

If the current state is "display rainbow pattern"...
  If X ms has elapsed since last time we did any work...
    Do one step of the rainbow pattern
    Advance the rainbow counter so next time we display the next colour in the sequence



Now your code appears to do both things at the same time:
a) Check for serial input
b) Display the rainbow pattern

If a key press is received mid-sequence the program can react to it and change the current state so the rest of the program does something else.

pcbbc:

A variables SCOPE is defined by the set of { } they are defined in.

Move...

unsigned long currentMillis = millis();

To the start of function...

void rainbow() {

You do not access 'currentMillis' from loop() so you do not need it there (although as I envisaged this you would have done the currentMillis/previousMillis check from loop - doesn't matter, you can do it either way).

Also remove this loop...

for (long firstPixelHue = 0; firstPixelHue < 5 * 65536; firstPixelHue += 256) {

This code is looping and therefore blocking.

Create a global (as I indicated in my snippet)...

long firstPixelHue = 0;

The looping is replaced by...

firstPixelHue += 256;
if (firstPixelHue >= 5 * 65536) firstPixelHue = 0;

...which gets ready for the next "tick" of the previousMillis timer, and does the next step of the rainbow animation.

Finally you need this code...

   if (value == 'd') {
      rainbow();
    }

...to be execute EVERY time through loop().

Currently it is only being executed ONCE immediately after a character is read from the serial port.