Trying to Switch between 2 Functions, Switch code not working

So I finally got a code that’s functional in the sense that there is a mode of solid green and a mode of blinking green. However, when I try to use the switch to switch between them they don’t switch. Only one is ever constant at a time. If I set the switch to High its the blinking green, if I set the switch to LOW in the code it’s the constant green.

#include <Adafruit_NeoPixel.h>
#ifdef __AVR__
  #include <avr/power.h>
#endif

#define PIN 1


Adafruit_NeoPixel strip = Adafruit_NeoPixel(30, PIN, NEO_GRB + NEO_KHZ800);
const int buttonPin = 4; 
int buttonState = 0;


void setup() {
  // This is for Trinket 5V 16MHz, you can remove these three lines if you are not using a Trinket
  #if defined (__AVR_ATtiny85__)
    if (F_CPU == 16000000) clock_prescale_set(clock_div_1);
  #endif
  // End of trinket special code


  strip.begin();
  strip.show(); // Initialize all pixels to 'off'
pinMode(buttonPin, INPUT_PULLUP); //
}


void loop(){
  
buttonState = digitalRead(buttonPin);
if (buttonState == LOW) {
  
colorWipe(strip.Color(0, 255, 0),0);

}
else {
rainbowCycle(20);
  }
}

void colorWipe(uint32_t c, uint8_t wait) {
  for(uint16_t i=0; i<strip.numPixels(); i++) {
    strip.setPixelColor(i, c);
    strip.show();
    delay(wait);
  }
}
 
// Slightly different, this makes the rainbow equally distributed throughout
void rainbowCycle(uint8_t wait) {
  uint16_t i, j;

  for(j=0; j<256*5; j++) { // 5 cycles of all colors on wheel
    for(i=0; i< strip.numPixels(); i++) {
      strip.setPixelColor(i, Wheel(((i * 256 / strip.numPixels()) + j) & 255));
    }
    strip.show();
    delay(0);
  }
}

// Input a value 0 to 255 to get a color value.
// The colours are a transition r - g - b - back to r.
uint32_t Wheel(byte WheelPos) {
  WheelPos = 255 - WheelPos;
  if(WheelPos < 85) {
    return strip.Color(0, 255,0);
  }
  if(WheelPos < 170) {
    WheelPos -= 85;
    return strip.Color(0, WheelPos * 3,0);
  }
  WheelPos -= 170;
  return strip.Color(0, 255 - WheelPos * 3, 0);
}

//void colorWipe(uint32_t c, uint8_t wait) {

The compiler is right. colorWipe() wasn't declared. It was commented out with //

The // comment style only comments one line. It doesn't follow the { and } pairs to comment out a whole block. So by putting in that single //, you've effectively removed a { from your program. But the matching } is still there. You will have a LOT of errors.

Expand the error window at the bottom of your Arduino screen. Scroll up to the top to find the first error. Fix that one first. That will solve a lot of the lower errors.

The compiler is very smart. If you define a function but never use it, it won't get put into the final executable code. So you don't need to comment-out unneeded functions. Although it is a good idea to prune them away eventually, so you don't have a lot of unreachable code that you have to read past.

MorganS:
The compiler is right. colorWipe() wasn’t declared. It was commented out with //

The // comment style only comments one line. It doesn’t follow the { and } pairs to comment out a whole block. So by putting in that single //, you’ve effectively removed a { from your program. But the matching } is still there. You will have a LOT of errors.

Expand the error window at the bottom of your Arduino screen. Scroll up to the top to find the first error. Fix that one first. That will solve a lot of the lower errors.

The compiler is very smart. If you define a function but never use it, it won’t get put into the final executable code. So you don’t need to comment-out unneeded functions. Although it is a good idea to prune them away eventually, so you don’t have a lot of unreachable code that you have to read past.

#include <Adafruit_NeoPixel.h>
#ifdef __AVR__
  #include <avr/power.h>
#endif

#define PIN 1

Adafruit_NeoPixel strip = Adafruit_NeoPixel(30, PIN, NEO_GRB + NEO_KHZ800);

const int buttonPin = 1; // I chose pin 1 for the switch
int buttonState = 0; // I copied this as well as it's required for the switch to work



void setup() {
  // This is for Trinket 5V 16MHz, you can remove these three lines if you are not using a Trinket
  #if defined (__AVR_ATtiny85__)
    if (F_CPU == 16000000) clock_prescale_set(clock_div_1);
  #endif
  // End of trinket special code
  strip.begin();
strip.show();
pinMode(buttonPin, INPUT_PULLUP);

}

void loop() {


buttonState = digitalRead(buttonPin); 
if (buttonState == HIGH) {
// Fill the dots one after the other with a color
void colorWipe(uint32_t c, uint8_t wait) {
  for(uint16_t i=0; i<strip.numPixels(); i++) {
    strip.setPixelColor(0,255,0);
    strip.show();
    delay(wait);
  }
}

//void rainbow(uint8_t wait) {
  uint16_t i, j;

  for(j=0; j<256; j++) {
    for(i=0; i<strip.numPixels(); i++) {
      strip.setPixelColor(i, Wheel((i+j) & 255));
    }
    strip.show();
    delay(wait);
  }

} else {
// Slightly different, this makes the rainbow equally distributed throughout
void rainbowCycle(uint8_t wait) {
  uint16_t i, j;

  for(j=0; j<256*5; j++) { // 5 cycles of all colors on wheel
    for(i=0; i< strip.numPixels(); i++) {
      strip.setPixelColor(0,255,0);
    }
    strip.show();
    delay(wait);
  }
}

//Theatre-style crawling lights.
//void theaterChase(uint32_t c, uint8_t wait) {
  for (int j=0; j<10; j++) {  //do 10 cycles of chasing
    for (int q=0; q < 3; q++) {
      for (uint16_t i=0; i < strip.numPixels(); i=i+3) {
        strip.setPixelColor(i+q, c);    //turn every third pixel on
      }
      strip.show();

      delay(wait);

      for (uint16_t i=0; i < strip.numPixels(); i=i+3) {
        strip.setPixelColor(i+q, 0);        //turn every third pixel off
      }
    }
  }
}

//Theatre-style crawling lights with rainbow effect
//void theaterChaseRainbow(uint8_t wait) {
  for (int j=0; j < 256; j++) {     // cycle all 256 colors in the wheel
    for (int q=0; q < 3; q++) {
      for (uint16_t i=0; i < strip.numPixels(); i=i+3) {
        strip.setPixelColor(i+q, Wheel( (i+j) % 255));    //turn every third pixel on
      }
      strip.show();

      delay(wait);

      for (uint16_t i=0; i < strip.numPixels(); i=i+3) {
        strip.setPixelColor(i+q, 0);        //turn every third pixel off
      }
    }
  }
}

// Input a value 0 to 255 to get a color value.
// The colours are a transition r - g - b - back to r.
uint32_t Wheel(byte WheelPos) {
  WheelPos = 255 - WheelPos;
  if(WheelPos < 85) {
    return strip.Color(255 - WheelPos * 3, 0, WheelPos * 3);
  }
  if(WheelPos < 170) {
    WheelPos -= 85;
    return strip.Color(0, WheelPos * 3, 255 - WheelPos * 3);
  }
  WheelPos -= 170;
  return strip.Color(WheelPos * 3, 255 - WheelPos * 3, 0);
}

So I removed the // and now the error I’m getting is Arduino: 1.8.5 (Windows 10), Board: “Adafruit Trinket (ATtiny85 @ 16MHz)”

C:\Users\Iram\AppData\Local\Temp\arduino_modified_sketch_30678\strandtest.ino: In function ‘void loop()’:

strandtest:33: error: a function-definition is not allowed here before ‘{’ token

void colorWipe(uint32_t c, uint8_t wait) {

^

strandtest:115: error: expected ‘}’ at end of input

}

^

strandtest:115: error: expected ‘}’ at end of input

exit status 1
a function-definition is not allowed here before ‘{’ token

This report would have more information with
“Show verbose output during compilation”
option enabled in File → Preferences.

Yeah you've got that function definition right smack in the middle of the loop function. You can't do that.

Delta_G:
Yeah you've got that function definition right smack in the middle of the loop function. You can't do that.

If this makes it any clearer I have literally no idea what I'm doing. I'm trying to work off of this tutorial so I'm basically just copying and pasting what it says and where it says.

Withencroft:
so I'm basically just copying and pasting what it says and where it says.

OK, well that severely limits what you will be able to do. It would help to learn some of the basics of C++ so you can at least understand what a function is and what the { and } do. You've got a lot of stuff pasted in the wrong places and I don't want to rewrite the whole thing for you.

Delta_G:
OK, well that severely limits what you will be able to do. It would help to learn some of the basics of C++ so you can at least understand what a function is and what the { and } do. You’ve got a lot of stuff pasted in the wrong places and I don’t want to rewrite the whole thing for you.

I am trying but this is all really foreign to me.

#include <Adafruit_NeoPixel.h>
#ifdef __AVR__
  #include <avr/power.h>
#endif

#define PIN 1


Adafruit_NeoPixel strip = Adafruit_NeoPixel(30, PIN, NEO_GRB + NEO_KHZ800);
const int buttonPin = 1; 
int buttonState = 0;


void setup() {
  // This is for Trinket 5V 16MHz, you can remove these three lines if you are not using a Trinket
  #if defined (__AVR_ATtiny85__)
    if (F_CPU == 16000000) clock_prescale_set(clock_div_1);
  #endif
  // End of trinket special code


  strip.begin();
  strip.show(); // Initialize all pixels to 'off'
pinMode(buttonPin, INPUT_PULLUP); //
}

void loop() {

  buttonState = digitalRead(buttonPin);
if (buttonState == HIGH) {



// Fill the dots one after the other with a color}
void colorWipe(uint32_t c, uint8_t wait) {
  for(uint16_t i=0; i<strip.numPixels(); i++) {
    strip.setPixelColor(i, c)
    strip.show();
    delay(wait);
    }
  }

} else {

// Slightly different, this makes the rainbow equally distributed throughout
void rainbowCycle(uint8_t wait) {
  uint16_t i, j;

  for(j=0; j<256*5; j++) { // 5 cycles of all colors on wheel
    for(i=0; i< strip.numPixels(); i++) {
      strip.setPixelColor(i, Wheel(((i * 256 / strip.numPixels()) + j) & 255));
    }
    strip.show();
    delay(wait);
  }
}

// Input a value 0 to 255 to get a color value.
// The colours are a transition r - g - b - back to r.
uint32_t Wheel(byte WheelPos) {
  WheelPos = 255 - WheelPos;
  if(WheelPos < 85) {
    return strip.Color(0, 255,0);
  }
  if(WheelPos < 170) {
    WheelPos -= 85;
    return strip.Color(0, WheelPos * 3,);
  }
  WheelPos -= 170;
  return strip.Color(0, 255 - WheelPos * 3, 0);
}

Is this any better? I deleted the extra code of the functions I don’t want instead of just // everything. I am still getting an error though saying “a function-definition is not allowed here before ‘{’ token” Right after the colorwipe part

Yes, you still have the definition of the colorWipe function inside the loop function. You can’t do that. Put the function definition outside of the loop function and call the function from loop.

void colorWipe(uint32_t c, uint8_t wait) {
  for(uint16_t i=0; i<strip.numPixels(); i++) {
    strip.setPixelColor(i, c)
    strip.show();
    delay(wait);
    }
  }

that part is a function definition. that’s not how you run the function. That’s how you tell it what to do when you call this function. This can’t be inside the loop function.

void colorWipe(uint32_t c, uint8_t wait) {
  for(uint16_t i=0; i<strip.numPixels(); i++) {
    strip.setPixelColor(i, c)
    strip.show();
    delay(wait);
    }
  }


void loop(){
  if (buttonState == HIGH){
     colorWipe(someNumber, someOtherNumber);
  }
}

Delta_G:
Yes, you still have the definition of the colorWipe function inside the loop function. You can’t do that. Put the function definition outside of the loop function and call the function from loop.

void colorWipe(uint32_t c, uint8_t wait) {

for(uint16_t i=0; i<strip.numPixels(); i++) {
    strip.setPixelColor(i, c)
    strip.show();
    delay(wait);
    }
  }




that part is a function definition. that's not how you run the function. That's how you tell it what to do when you call this function. This can't be inside the loop function. 




void colorWipe(uint32_t c, uint8_t wait) {
  for(uint16_t i=0; i<strip.numPixels(); i++) {
    strip.setPixelColor(i, c)
    strip.show();
    delay(wait);
    }
  }

void loop(){
  if (buttonState == HIGH){
    colorWipe(someNumber, someOtherNumber);
  }
}

Okay okay I think I understand what you’re saying. The Loop is just listing what’s going to happen and the details of those functions need to be outside of that.

#include <Adafruit_NeoPixel.h>
#ifdef __AVR__
  #include <avr/power.h>
#endif

#define PIN 1


Adafruit_NeoPixel strip = Adafruit_NeoPixel(30, PIN, NEO_GRB + NEO_KHZ800);
const int buttonPin = 4; 
int buttonState = 0;


void setup() {
  // This is for Trinket 5V 16MHz, you can remove these three lines if you are not using a Trinket
  #if defined (__AVR_ATtiny85__)
    if (F_CPU == 16000000) clock_prescale_set(clock_div_1);
  #endif
  // End of trinket special code


  strip.begin();
  strip.show(); // Initialize all pixels to 'off'
pinMode(buttonPin, INPUT_PULLUP); //
}


void loop(){
  
if (buttonState == HIGH) {

  
strip.setBrightness(brightness); // I added this. Found in the Überguide
colorWipe(strip.Color(0, 255, 0), 0); // I set the last number to 0
brightness = brightness + fadeAmount;
if (brightness <= 0 || brightness >= 255) {
fadeAmount = -fadeAmount;
}
//delay(0); // I commented this part out by adding two slashes in front
}
else {
rainbowCycle(20);
  }
}

void colorWipe(uint32_t c, uint8_t wait) {
  for(uint16_t i=0; i<strip.numPixels(); i++) {
    strip.setPixelColor(i, c);
    strip.show();
    delay(wait);
  }
}
 
// Slightly different, this makes the rainbow equally distributed throughout
void rainbowCycle(uint8_t wait) {
  uint16_t i, j;

  for(j=0; j<256*5; j++) { // 5 cycles of all colors on wheel
    for(i=0; i< strip.numPixels(); i++) {
      strip.setPixelColor(i, Wheel(((i * 256 / strip.numPixels()) + j) & 255));
    }
    strip.show();
    delay(wait);
  }
}

// Input a value 0 to 255 to get a color value.
// The colours are a transition r - g - b - back to r.
uint32_t Wheel(byte WheelPos) {
  WheelPos = 255 - WheelPos;
  if(WheelPos < 85) {
    return strip.Color(0, 255,0);
  }
  if(WheelPos < 170) {
    WheelPos -= 85;
    return strip.Color(0, WheelPos * 3,);
  }
  WheelPos -= 170;
  return strip.Color(0, 255 - WheelPos * 3, 0);
}

I did that and now it’s saying brightness was not declared in this scope. If I try to eliminate the brightness part it then says colorwipe is not declared

OK, what number should it start with for brightness? Where did you tell it that?

int buttonState = 0;

See how you created a variable for buttonState?

You need to do that for brightness.

Also, the colorWipe function you have defined takes a single unsigned long (a 32 bit number) for the color. You try to pass it a set of parenthesis with three numbers in it.

Delta_G:
OK, what number should it start with for brightness? Where did you tell it that?

int buttonState = 0;

See how you created a variable for buttonState?

You need to do that for brightness.

Also, the colorWipe function you have defined takes a single unsigned long (a 32 bit number) for the color. You try to pass it a set of parenthesis with three numbers in it.

The brightness was a part of the tutorial so I ejected it and kept the simple colorswipe.

The code is fully functional with no errors now but when I loaded it into the trinket it only does one of the functions, when I switch the switch it doesn’t move to the other code. I feel so close but not quite there.

#include <Adafruit_NeoPixel.h>
#ifdef __AVR__
  #include <avr/power.h>
#endif

#define PIN 1


Adafruit_NeoPixel strip = Adafruit_NeoPixel(30, PIN, NEO_GRB + NEO_KHZ800);
const int buttonPin = 4; 
int buttonState = 0;


void setup() {
  // This is for Trinket 5V 16MHz, you can remove these three lines if you are not using a Trinket
  #if defined (__AVR_ATtiny85__)
    if (F_CPU == 16000000) clock_prescale_set(clock_div_1);
  #endif
  // End of trinket special code


  strip.begin();
  strip.show(); // Initialize all pixels to 'off'
pinMode(buttonPin, INPUT_PULLUP); //
}


void loop(){
  
if (buttonState == HIGH) {
  
colorWipe(strip.Color(0, 255, 0),0);

}
else {
rainbowCycle(20);
  }
}

void colorWipe(uint32_t c, uint8_t wait) {
  for(uint16_t i=0; i<strip.numPixels(); i++) {
    strip.setPixelColor(i, c);
    strip.show();
    delay(wait);
  }
}
 
// Slightly different, this makes the rainbow equally distributed throughout
void rainbowCycle(uint8_t wait) {
  uint16_t i, j;

  for(j=0; j<256*5; j++) { // 5 cycles of all colors on wheel
    for(i=0; i< strip.numPixels(); i++) {
      strip.setPixelColor(i, Wheel(((i * 256 / strip.numPixels()) + j) & 255));
    }
    strip.show();
    delay(0);
  }
}

// Input a value 0 to 255 to get a color value.
// The colours are a transition r - g - b - back to r.
uint32_t Wheel(byte WheelPos) {
  WheelPos = 255 - WheelPos;
  if(WheelPos < 85) {
    return strip.Color(0, 255,0);
  }
  if(WheelPos < 170) {
    WheelPos -= 85;
    return strip.Color(0, WheelPos * 3,0);
  }
  WheelPos -= 170;
  return strip.Color(0, 255 - WheelPos * 3, 0);
}

From testing it with colors it seems the rainbow cycle function is the one that's working but the colorswipe one is not.

Delta_G:
Also, the colorWipe function you have defined takes a single unsigned long (a 32 bit number) for the color. You try to pass it a set of parenthesis with three numbers in it.

colorWipe(strip.Color(0, 255, 0),0);

Because of thte way the comma operator works this becomes :

colorWipe(strip.Color(0,0);

Did the tutorial you're following call colorWipe with three numbers in parenthesis like that?

See the thing about coding is that you have to read the directions and instructions and documentation. You can't just make up how you think it should work. You have to follow things precisely.

Again, I beg you to take a day or two and run through a basic C++ tutorial and learn some of the basics of the language. Otherwise you're just going to get frustrated.

void loop() {
buttonState = digitalRead(buttonPin); //read how the switch is set
if (buttonState == HIGH) { // If switch is right
colorWipe(strip.Color(255, 51, 0), 0); // set to orange
} else { // If switch is left
rainbowCycle(20);
}
}

This is copied directly from the tutorial book.

"As always, I just had to change some of the void codes to run the animations like I wanted. For the
colorWipe mode, I set the very last number to 0, so it instantly filled all pixels with my color instead of
turning them on one by one. And finally I also adjusted the colors of the rainbowCycle to get a pretty
orange. For this I simply turned the blue channel off and added 1/5 of red to the green channel."

The paragraph of explanation she gave for this section.

Where did you find this tutorial?

If it defines the function like this:

void colorWipe(uint32_t c, uint8_t wait) {

and then tries to call it like this:

colorWipe(strip.Color(255, 51, 0), 0);

Then you should probably throw that tutorial in the rubbish bin.

Somehow I kind of doubt that this is the case.

The function definition says that it expects two arguments, the first is a 32 bit unsigned variable and the second is a 8 bit unsigned variable. That's what the function expects to see. Passing it a bunch of numbers in parenthesis when that isn't what it expects is just not something you should expect to work.

.Color() is a helper method that takes 3 integers for red, green and blue and returns the 32-bit representation of that color.

I think it is being used correctly here.

The braces {} are really important in C code. When you start, your brain is not keyed to look for them. For experienced coders, it is obvious what is "inside" and "outside". Keep working and you will get used to looking for those important signposts in the code.

MorganS:
.Color() is a helper method that takes 3 integers for red, green and blue and returns the 32-bit representation of that color.

OK. Didn't see that one defined.

MorganS:
.Color() is a helper method that takes 3 integers for red, green and blue and returns the 32-bit representation of that color.

I think it is being used correctly here.

The braces {} are really important in C code. When you start, your brain is not keyed to look for them. For experienced coders, it is obvious what is "inside" and "outside". Keep working and you will get used to looking for those important signposts in the code.

Okay thank you. So the code I have now works technically in the fact that there is a solid green mode and a blinking green mode however I can't get the switch to switch between them.

Do you have a pull down resistor on the button input?

Wanting a button to be HIGH when pressed is backwards of normal. Normally you would wire the button to go LOW when pressed and use pinMode INPUT_PULLUP. That way you won’t need any external resistor

Delta_G:
Do you have a pull down resistor on the button input?

Wanting a button to be HIGH when pressed is backwards of normal. Normally you would wire the button to go LOW when pressed and use pinMode INPUT_PULLUP. That way you won’t need any external resistor

No, it wasn't in the schematic so I didn't add it. So adding a resistor or changing how the button is wired should make it work?