Variable Seems to Change on It's Own

I have 4 adafruit 25 pixel strands connected to an Arduino Uno. Previously I had a button with a pull-up resistor connected to pin2 as an interrupt, but as I originally thought that might be related to my issue I've removed everything for now (including the button and related hardware connected to the Arduino).

So I've completely stripped down my code to make it as simple as possible and I'm still getting this very strange issue. Basically, the program correctly sets the 100 pixels one at a time to an off-white color. For the first 100 iterations of loop(), mode is 0 and the animation runs fine. Once it hits the end of the strand, mode changes to 30 and I can't seem to figure out why.

The output being as you would expect:

0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/0/30/30/30/30/30/30/30/30/30/30/30/30/30/30/30/30/30/30/30/30/30/30/30/30/... etc

The idea being that each press of the button should increment mode and cause the strand to switch to the next pattern. It should be able to continue animating indefinitely while performing other tasks on each frame. For now I just have removed all but one pattern, but the purpose of the switch is to allow many patterns.

#include "SPI.h"
#include "Adafruit_WS2801.h"

int dataPin  = 3;    // Yellow wire on Adafruit Pixels
int clockPin = 5;    // Green wire on Adafruit Pixels

Adafruit_WS2801 strip = Adafruit_WS2801(100, dataPin, clockPin);

int const pixels = 100;
int a[pixels]; //brightness
int r[pixels];
int g[pixels];
int b[pixels];

int mode = 0;
int currentLED = 0;

  
void setup() {  
  strip.begin();
  Serial.begin(9600);
  
  clear();
  
  strip.show();
}


void loop() {    
  Serial.print(mode);
  Serial.print("/");
  
  // drawing
  switch (mode) {
    case 0:
      colorWipe(255, 220, 255, 30);
      break;
  }
  
  // update the entire strand
  for (int i; i < pixels; i++) {
    strip.setPixelColor(i, Color(r[i] * (a[i] / 255) , g[i] * (a[i] / 255) , b[i] * (a[i] / 255 )));
  }
  
  // show the changes
  strip.show();
}

void clear() {
  for (int i = 0; i < pixels; i++) {
    r[i] = 0;
    b[i] = 0;
    g[i] = 0;
    a[i] = 0;
  }
}

// fill the dots one after the other with said color
void colorWipe(byte myA, byte myR, byte myG, byte myB) {
  a[currentLED] = myA;
  r[currentLED] = myR;
  g[currentLED] = myG;
  b[currentLED] = myB;
  
  currentLED++;
  if (currentLED > pixels) {
    currentLED = 0;
  }
}

/* Helper functions */

// Create a 24 bit color value from R,G,B
uint32_t Color(byte r, byte g, byte b)
{
  uint32_t c;
  c = r;
  c <<= 8;
  c |= g;
  c <<= 8;
  c |= b;
  return c;
}

I'm completely at a loss what else I can try. There is no longer any code that modifies the value of mode so I don't even see where to start troubleshooting.

Thanks for any suggestions

suggestions: where you got for ( int i ; i<pixels ; i++ )
is probably unwise. i might have an indeterminate initial value, you should probably have int i=0 ; i<pixels

your "mode" variable is declared after the arrays. This makes it vulnerable to being overwritten if you write
past the end of the array accidentally or on purpose. Try declaring "mode" before the arrays.

Ah I didn't catch the incorrectly declared i.

It looks like the order of declarations fixed my problem. Very interesting, I never would have thought but it does make sense. It looks like the actual error is in the colorWipe function where it should be resetting currentLED to 0 when it is equal to 100 and not 101;

Thanks for the help, would have taken me a while to figure that out.

Very interesting, I never would have thought but it does make sense.

Unlike global variables, local variables are not initialized, unless YOU cause that to happen.

It's better to get in the habit of initializing every variable - local or global.

It's also better to minimize the use of global variables. pixels and currentLed should be passed to colorWipe().

One final note. a, r, g, and b never hold values that are larger than a byte, do they? You could save 20% of SRAM by making the type of those arrays byte, instead of int.

your "mode" variable is declared after the arrays. This makes it vulnerable to being overwritten if you write
past the end of the array accidentally or on purpose. Try declaring "mode" before the arrays.

Rather than cover up the bug, why not fix it? Here is the code concerned:

  currentLED++;
  if (currentLED > pixels) {
    currentLED = 0;
  }

See anything wrong? I do!

[EDIT: I hadn't fully read reply#2 when I wrote this - looks like you may have fixed this already.]

PaulS:
Unlike global variables, local variables are not initialized, unless YOU cause that to happen.

It's better to get in the habit of initializing every variable - local or global.

It's also better to minimize the use of global variables. pixels and currentLed should be passed to colorWipe().

I wasn't sure how else to declare and initialize the arrays, should I just declare the arrays inside setup() where I initialize them? Or is there a way to set every entry in an array to the same variable? I looked around a little bit for how to do that but the for loop seemed the best way.

I'm not sure I fully understand local vs global, would I declare these variables in setup() to make them local? I know variables declared inside a function will be local, but those two will be needed by most of my patterns.

PaulS:
One final note. a, r, g, and b never hold values that are larger than a byte, do they? You could save 20% of SRAM by making the type of those arrays byte, instead of int.

Thanks for the tip, though I was aware of this one. Originally I had arrays storing entire 24 bit colors and when I'm tinkering I tend to leave things as they were in case I wanted to go back, but I think I'm pretty happy with this (unless you have any suggestions on how I would go about "buffering" the entire strand). The only thing I was going to change was to use HSB instead of RGB + Brightness, which would save even more space.

dc42:
Rather than cover up the bug, why not fix it? Here is the code concerned:

You mean, is there something else wrong other than what I mentioned in my reply? I made the change I mentioned and it isn't writing to entry 100 anymore, but if there is something else wrong there I don't see it.

should I just declare the arrays inside setup() where I initialize them?

Where do you initialize a, r, g, and b in setup? I don't see that happening.

I'm not sure I fully understand local vs global, would I declare these variables in setup() to make them local?

Yes, but, then they would be local to setup() where they are not used, rather than local to loop, where they are used.

Leaving them as global is probably best, until you understand scope better.

PaulS:
Where do you initialize a, r, g, and b in setup? I don't see that happening.

Maybe I'm unclear on what initialize means. I thought clear()—which sets every value in the array to 0—would be considered initializing it. Wouldn't I just have to use a loop similar for loop to initialize the arrays?

PaulS:
Yes, but, then they would be local to setup() where they are not used, rather than local to loop, where they are used.

This makes sense, but if I declare them in loop then won't they will be declared again every frame? Should I be declaring them at the beginning of loop() and then have a separate looping function inside of loop() (so that loop() essentially only runs once)? I'm getting a little off topic here, I should probably spend some more time tinkering before asking so many questions.

Maybe I'm unclear on what initialize means. I thought clear()—which sets every value in the array to 0—would be considered initializing it. Wouldn't I just have to use a loop similar for loop to initialize the arrays?

clear() may be called from setup(), but the initialization is done IN clear(), not IN setup().

but if I declare them in loop then won't they will be declared again every frame?

Unless they are declared static.

But, its easier to just leave them global.

PaulS:
clear() may be called from setup(), but the initialization is done IN clear(), not IN setup().

But, its easier to just leave them global.

Oh, right. I think I just got confused—my original point was just that they are being initialized, but I realize that was right after you said global variables are automatically initialized anyway.

Thanks again for the help. I'm still wondering what dc42 meant, but I think he just missed my reply.

Variable Seems to Change on It's Own

That magic.