Pages: 1 [2]   Go Down
Author Topic: 3 switches, code for each switch, trouble figuring out how to read each switch..  (Read 1014 times)
0 Members and 1 Guest are viewing this topic.
Central Illinois
Offline Offline
Newbie
*
Karma: 0
Posts: 9
trying to understand programming the arduino
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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:
// 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.
Logged

Sincere thanks for all help given.

California
Offline Offline
Faraday Member
**
Karma: 88
Posts: 3376
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset


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:
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:
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.
Logged

California
Offline Offline
Faraday Member
**
Karma: 88
Posts: 3376
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Code:
   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.
« Last Edit: November 14, 2012, 04:02:02 pm by Arrch » Logged

Central Illinois
Offline Offline
Newbie
*
Karma: 0
Posts: 9
trying to understand programming the arduino
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Code:
   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!!
Logged

Sincere thanks for all help given.

Pages: 1 [2]   Go Up
Jump to: