Handling interrupts properly when using multiple inputs

I'm basically making something close to the many nightlights people have posted online. I have the circuit wired up for a single RGB LED on pins 9,10,11 so I can use PWM. I also have a 10k trimpot hooked to analog 0, a light dependant resistor hooked on analog 1, and a simple push button wired into digital 2. I attached the diagram as a PDF for reference.


What I am trying to do is make the arduino turn on the LED when the lights in the room go out, but I also want the light to have an adjustment so that the brightness can be turned down. Lastly, I added a pushbutton to serve as an interrupt so that I can add different programs(I'm using cases for each program in the code) and have someone be able to cycle through them. The code that I have is working, but there are some issues I'm not quite sure how to resolve. One of the programs is a slow color swirl, and when that program is running I can't adjust on the fly what the brightness level should be. Also, during this program if I hit the interrupt button it won't cycle to the next program until the full program completes its routine. This holds true for the rest of the programs as well, which are much simpler and are just a single solid color. When these programs are running and I hit the interrupt button it wont go on to the next program until the light is applied to the photosensor(LDR). I think in this case I need a way to make the interrupt break the while test I am running, but I am not sure how to do that. I'll post my code below. I have tried to add some comments and rough guidance. Any help or suggestions to optimize the code would be greatly appreciated.

#define REDPIN 9        //pin definitions and global variable definitions
#define GREENPIN 10
#define BLUEPIN 11
const int photocellPin = 1;
const int trimPin = 0;
const int buttonPin = 2;
int photocellReading;
int brightness;

int x = 0;           //interrupt counter

unsigned long button_time = 0;                              //stabilizes the signla from a button push
unsigned long last_button_time = 0;                         //to ignore unwanted noise
 
void setup() {                                              ///setup shit like define in/outputs and establish serial connection if desired
  //Serial.begin(9600);
  pinMode(REDPIN, OUTPUT);
  pinMode(GREENPIN, OUTPUT);
  pinMode(BLUEPIN, OUTPUT);
  pinMode(trimPin, INPUT);
  pinMode(photocellPin, INPUT);
  pinMode(buttonPin, INPUT_PULLUP);                    //sets internal pullup resistor
  attachInterrupt(0, increment, FALLING);
}
 
 
void loop() {                                          //main loop 
  
  //Serial.print("Analog reading = ");
  //Serial.println(photocellReading);                  
  digitalWrite(buttonPin, HIGH);
  
  
  int r, g, b;  
  photocellReading = analogRead(photocellPin);  
  brightness = analogRead(trimPin);
  int FADESPEED;                    
  int Time = 7000; 
  
  switch (x) {
                   //******colorswirl program******\\
    case 0:
     while (photocellReading < 500){
     // fade from blue to violet
     for (r = 0; r < brightness/4; r++) { 
     analogWrite(REDPIN, r);
     FADESPEED = Time/brightness;
     delay(FADESPEED);
    } 
    // fade from violet to red
     for (b = brightness/4; b > 0; b--) { 
     analogWrite(BLUEPIN, b); 
     delay(FADESPEED);
    } 
    // fade from red to yellow
     for (g = 0; g < brightness/4; g++) { 
     analogWrite(GREENPIN, g);
     delay(FADESPEED);
    } 
    // fade from yellow to green
     for (r = brightness/4; r > 0; r--) { 
     analogWrite(REDPIN, r);
     delay(FADESPEED);
    } 
    // fade from green to teal
     for (b = 0; b < brightness/4; b++) { 
     analogWrite(BLUEPIN, b);
     delay(FADESPEED);
    } 
    // fade from teal to blue
     for (g = brightness/4; g > 0; g--) { 
     analogWrite(GREENPIN, g);
     delay(FADESPEED);
    }
    break;
    case 1:                                              //Red
      while (photocellReading < 500){
        brightness = analogRead(trimPin);
        analogWrite(REDPIN, brightness/4);
        photocellReading = analogRead(photocellPin);
      }
      break;
    case 2:
      while (photocellReading < 500){                    //Green
        brightness = analogRead(trimPin);
        analogWrite(GREENPIN, brightness/4);
        photocellReading = analogRead(photocellPin);
      }
      break;
    case 3:
      while (photocellReading < 500){                   //Blue
        brightness = analogRead(trimPin);
        analogWrite(BLUEPIN, brightness/4);
        photocellReading = analogRead(photocellPin);
      }
      break;
    case 4:
      while (photocellReading < 500){                  //Magenta
        brightness = analogRead(trimPin);
        analogWrite(REDPIN, brightness/8);
        analogWrite(BLUEPIN, brightness/4);
        photocellReading = analogRead(photocellPin);
      }
      break;
    case 5:
      while (photocellReading < 500){                 //Cyan
        brightness = analogRead(trimPin);
        analogWrite(BLUEPIN, brightness/4);
        analogWrite(GREENPIN, brightness/4);
        photocellReading = analogRead(photocellPin);
      }
      break; 
    case 6:
      while (photocellReading < 500){                //Yellow
        brightness = analogRead(trimPin);
        analogWrite(REDPIN, brightness/4);
        analogWrite(GREENPIN, brightness/4);
        photocellReading = analogRead(photocellPin);
      }
      break;
    }
  }
  analogWrite(REDPIN, 0);
  analogWrite(GREENPIN, 0);
  analogWrite(BLUEPIN,0);  
  delay(2);
}


// Interrupt service routine for button interrupt on digital pin 2
void increment() 
{
  button_time = millis();
  //check to see if increment() was called in the last 250 milliseconds
  if (button_time - last_button_time > 250)
  {
    if (x <= 5){
       x++;
       analogWrite(REDPIN, 0);
       analogWrite(BLUEPIN, 0);
       analogWrite(GREENPIN,0);
       loop();
    }
    else {
       x = 0;
      last_button_time = button_time;
      analogWrite(REDPIN, 0);
       analogWrite(BLUEPIN, 0);
       analogWrite(GREENPIN,0);
       loop();
    }
  }
}

Calling the loop() function from inside an ISR seems a bizarre thing to do, as does doing 3 analogWrites which take a comparatively long time to execute. The usual technique when using an ISR is for it to set a variable to indicate that it has been triggered and for the value of the variable to be acted upon in the main program.

This is only possible if the delay() function is not used in the main program because this blocks execution of other code until the delay() is complete and your code is full of them. Consider using millis() as a timer using the technique used in the BlinkWithoutDelay example program.

if (x <= 5){
x++;
analogWrite(REDPIN, 0);
analogWrite(BLUEPIN, 0);
analogWrite(GREENPIN,0);
loop();
}
else {
x = 0;
last_button_time = button_time;
analogWrite(REDPIN, 0);
analogWrite(BLUEPIN, 0);
analogWrite(GREENPIN,0);
loop();
}

Yikes, that has to cause a stack overflow eventually. How long does that code run for?

Just do a return.


Rob

Awesome thanks for the pointers. Not a coder, nor an electronics dude, so after I looked up what stack overflow meant, I would say you are correct. I knew there were probably better approaches such as a timer (millis) and return. I just didnt know the proper syntax. Its a learning process. On a side note, there were actually pretty interesting effects achieved from calling the main program inside the ISR. Non-desirable ones, but effects nonetheless.. Thanks again

Cheers