Go Down

Topic: 3 switches, code for each switch, trouble figuring out how to read each switch.. (Read 1 time) previous topic - next topic

medic29

This is for the same project...it is the code to make the leds go from the inside out, but for some reason I have the earlier problem again where the ledEPin and ledFPin apparently are still firing when I want them to turn off, thus they only dim until it cycles again.  I have looked through the code and studied it, changed things and played with it, re-written it, etc.  If someone can take a look and see what I'm missing I would appreciate it.
Code: [Select]

// test code utilizing blink without delay.
// written 11/13/12.

const int ledAPin = 7;      // the number of the LED pin
const int ledBPin = 8;      // the number of the 2nd LED pin
const int ledCPin = 9;      // the number of the 3rd LED pin
const int ledDPin = 10;     // the number of the 4th LED pin
const int ledEPin = 11;     // the number of the 5th LED pin
const int ledFPin = 12;     // the number of the 6th LED pin

// Variables will change:

long previousMillis = 0;        // will store last time LED was updated


long ledAOn = 2500;           // total time ledA is on
long ledBOn = 300;              // the time ledB will come on
long ledCOn = 600;
long ledDOn = 900;
long ledEOn = 1200;
long ledFOn = 1500;
long ledGOn = 1800;
long totalTime = 3000;
long totalTimeCenter = 2800;


void setup() {
  // set the digital pin as output:
  pinMode(ledAPin, OUTPUT);     
  pinMode(ledBPin, OUTPUT);
  pinMode(ledCPin, OUTPUT);
  pinMode(ledDPin, OUTPUT);
  pinMode(ledEPin, OUTPUT);
  pinMode(ledFPin, OUTPUT);
}

void loop()
{

  unsigned long currentMillis = millis();

    if(currentMillis - previousMillis > totalTimeCenter) {
      previousMillis = currentMillis;
    }

    if(currentMillis - previousMillis < ledEOn){
      if(currentMillis - previousMillis < ledGOn)
      digitalWrite(ledCPin, HIGH);
      digitalWrite(ledDPin, HIGH);
    }
     
    if(currentMillis - previousMillis > ledBOn){
      if(currentMillis - previousMillis < ledEOn)
        digitalWrite(ledBPin, HIGH);
        digitalWrite(ledEPin, HIGH);
    }

    if(currentMillis - previousMillis > ledCOn){
      if(currentMillis - previousMillis < ledEOn)
        digitalWrite(ledAPin, HIGH);
        digitalWrite(ledFPin, HIGH);
    }
   
    if(currentMillis - previousMillis > ledGOn){
     digitalWrite(ledAPin, LOW);
     digitalWrite(ledBPin, LOW);
     digitalWrite(ledCPin, LOW);
     digitalWrite(ledDPin, LOW);
     digitalWrite(ledEPin, LOW);
     digitalWrite(ledFPin, LOW);
}

   if(currentMillis - previousMillis > totalTimeCenter) {
     previousMillis = currentMillis;
  }
}


thanks in advance.
Sincere thanks for all help given.

Arrch




A couple of things...

Remove the commented out code, no need for it to waste space.
Create a variable that stores the result of currentMillis - previousMillis, and use that in your comparisons; no need to do the arithmetic that many times.
Use arrays for the pins and intervals; probably an array of a simple struct simple struct:
Code: [Select]
struct ledData {
 int pin;
 unsigned long on;
 unsigned long off;
};


I would also use a variable as a flag that gets set when you turn all the LEDs off, and cleared otherwise. Have a single if statement that checks the flag before even bothering to check if it's been long enough. Psuedocode:
Code: [Select]
if the flag cleared
 get the time difference
 check each led to see if it should be on


if it's been long enough
  turn all leds off
  set the flag
else
  clear the flag

   


I need to try to figure out what you are talking about as far as setting flags, etc.  As far as setting a variable to replace doing the arrythmatic, that would be easy enough, but either way doesn't affect the code and/or the functioning of the code. Not a bad idea to make the code look cleaner.  


"setting and clearing a flag" isn't complicated; the concept is implemented using a simple variable and assigning it a some value that you check it against later on.

The functionality won't change, but when you're dealing with devices with limited resources, it's always a good idea to not make it do unnecessarily repetitive calculations.

Arrch

Code: [Select]
   if(currentMillis - previousMillis > ledBOn){
     if(currentMillis - previousMillis < ledEOn)
       digitalWrite(ledBPin, HIGH);
       digitalWrite(ledEPin, HIGH);
   }

   if(currentMillis - previousMillis > ledCOn){
     if(currentMillis - previousMillis < ledEOn)
       digitalWrite(ledAPin, HIGH);
       digitalWrite(ledFPin, HIGH);
   }


Did you forget brackets around the nested if statements? As you have it written, A and B are turned HIGH when both of their conditions are met, while F and E are turned only if the first condition is met, and regardless of the nested condition.

medic29


Code: [Select]
   if(currentMillis - previousMillis > ledBOn){
     if(currentMillis - previousMillis < ledEOn)
       digitalWrite(ledBPin, HIGH);
       digitalWrite(ledEPin, HIGH);
   }

   if(currentMillis - previousMillis > ledCOn){
     if(currentMillis - previousMillis < ledEOn)
       digitalWrite(ledAPin, HIGH);
       digitalWrite(ledFPin, HIGH);
   }


Did you forget brackets around the nested if statements? As you have it written, A and B are turned HIGH when both of their conditions are met, while F and E are turned only if the first condition is met, and regardless of the nested condition.


Yep, that is probably it.  I didn't realize I needed them, but it makes sense.  Thanks!!
Sincere thanks for all help given.

Go Up