Is there any way to make this code cleaner?

Projects Programming
1

Okay hey guys,
I'm currently using a Stellaris Launchpad (It's similar to Arduino)
I'm using the software called Energia to program it pretty much exactly like the Arduino.
My current code is this:

int redLED = RED_LED;
int greenLED = GREEN_LED;
int blueLED = BLUE_LED;
int Time = 30;

const int buttonINC = PUSH1;
const int buttonDEC = PUSH2;
int buttonIState = 0;
int buttonDState = 0;

void setup()  { 
  pinMode(buttonINC, INPUT_PULLUP);     
  pinMode(buttonDEC, INPUT_PULLUP);
}

void checka() {
  buttonIState = digitalRead(buttonINC);
  if (buttonIState == HIGH) {
  Time += 2;
  }

}

void checkb() {
  buttonDState = digitalRead(buttonDEC);
  if (buttonDState == HIGH) {
  Time -= 2;
  }

}

void check() {
    checka();
    checkb();
    if(Time < 1)
    {
     Time = 2; 
    }
}
void loop()  {
  check();
  for(int fadeValue = 0 ; fadeValue <= 255; fadeValue +=5) { 
    analogWrite(redLED, fadeValue);         
    delay(Time);                            
  } 
  check();
  for(int fadeValue = 255 ; fadeValue >= 0; fadeValue -=5) { 
    analogWrite(redLED, fadeValue);
    delay(Time);                            
  } 
  check();
  for(int fadeValue = 0 ; fadeValue <= 255; fadeValue +=5) { 
    analogWrite(greenLED, fadeValue);         
    delay(Time);                            
  } 
  check();
  for(int fadeValue = 255 ; fadeValue >= 0; fadeValue -=5) { 
    analogWrite(greenLED, fadeValue);         
    delay(Time);                            
  } 
  check();
  for(int fadeValue = 0 ; fadeValue <= 255; fadeValue +=5) { 
    analogWrite(blueLED, fadeValue);         
    delay(Time);                            
  } 
  check();
  for(int fadeValue = 255 ; fadeValue >= 0; fadeValue -=5) { 
    analogWrite(blueLED, fadeValue);         
    delay(Time);                            
  }
  
}

PUSH1, PUSH2 are the built in buttons on the board.
RED_LED,GREEN_LED,BLUE_LED are used when using the RGB LED built onto the board.
Other then that it's mostly like the Arduino when it comes to the code, I am not the best at this language and I'm working my best to make the most cleanest and efficient code.
Basically what this does is it fades in Red, then fades out, fades in green, fades out green, fades in blue, fades out blue, and repeats, I can use the 2 buttons to change the speeds...
Also I was wondering if it was possible to make a interrupt sorta thing, where the button update are pretty much instant, like I don't have to wait for it to be checked it will change the speed automatically, hope you guys can help, if you need any more info just ask, I appreciate the help very much!
:smiley:

2

See this : http://arduino.cc/en/Reference/AttachInterrupt
But only two pins on the ATmega328P can be used.
The other pins can also be used, but in a different way. I'm sure there must be a library for that.

The minimum and maximum time could be used in a different way.

const int Time_min = 2;
const int Time_max = 1000;
...
// increment
if (Time <= (Time_max - 2))
  Time += 2;
...
// decrement
if (Time >= (Time_min + 2))
  Time -= 2;
...

You could write smaller (but less readable) code like this:

const int Time_min = 2;
const int Time_max = 1000;
...
// increment
Time = min (Time + 2,Time_max);
...
// decrement
Time = max (Time - 2,Time_min);
...
3

I would rewrite it this way:

void check_buttons(unsigned char inc, unsigned char dec) {
  if (digitalRead(inc)) Time += 2; //read increment button
  if (digitalRead(dec)) Time -= 2; //read decrement button
  if (Time < 1) Time = 2; //what about 1?
}

...
  check_buttons(buttonInc, buttonDec); //check buttons and bound Time
...
4

bloodless2010:
Also I was wondering if it was possible to make a interrupt sorta thing, where the button update are pretty much instant, like I don't have to wait for it to be checked it will change the speed automatically, hope you guys can help, if you need any more info just ask, I appreciate the help very much!
:smiley:

The answer comes up so often that there really should be a sticky for it.

What you're trying to do is control two activities concurrently i.e. detecting button transitions, and doing timed changes to the LED output states.

By far the best way to achieve this is to restructure your sketch so that it is non-blocking (your code does not stop and wait for something to happen). The 'blink without delay' example sketch gives a simple demonstration of this technique and shows how to carry out activities on a timed basis without stopping the sketch to wait for time to pass.

If you adopted this approach you'd write some button handling code that polled the input states and detected button transitions (pressed/unpressed) and updated the configuration accordingly, and some timing code similar to blink without delay that was called repeatedly. Each time it is called it determines whether it is time to change the outputs, and if so it changes them.

Once you have got your head around this non-blocking way of handling events you will find it can be extended to support as many different types of activity as you like, with minimal dependency between them.