Go Down

Topic: Project 8 Digital HourGlass - I can't change the light direction! Help! (Read 3786 times) previous topic - next topic

nigelsoh

So, I've been trying the additional question at the end of the chapter, asking me to try change the direction of how the LED should move.
In my case, I have placed the hardware exactly as the book has done it, except that I've used a push-button instead of a tilt switch as the tilt switch couldn't be attached to the breadboard.

Here's my code:

Code: [Select]

// Project 8 - Digital Hourglass

const int switchPin = 8;
unsigned long previousTime = 0;
int switchState = 0;
int prevSwitchState = 0;
int led = 2;
int directions = 1;
long interval = 600;

void setup(){
  for(int x = 2; x<8; x++){
    pinMode(x, OUTPUT);
  }
 
  pinMode(switchPin, INPUT);
}

void loop(){
  unsigned long currentTime = millis();
 
  if(currentTime - previousTime > interval){
    previousTime = currentTime;
   
    digitalWrite(led, HIGH);
   
      led += directions;
   
 
    }
 
 
   
    switchState = digitalRead(switchPin);
    if(switchState != prevSwitchState){ // if there is a change in state of switch
      for(int x=2; x<8; x++){
        digitalWrite(x, LOW);
      }
     
      if(directions = 1){ // if the led is moving in ascending order,
        led = 7;          // restart from the last pin,
        directions = -1;  // move in descending order.
      }
      if(directions = -1){ // if the led is moving in descending order,
        led = 1;             // restart from the first pin,
        directions = 1;      // move in adescending order.
      }
     
      previousTime = currentTime;
    }
    prevSwitchState = switchState;
  }


But, for some reason or so, my LED instead of changing directions, just restarts from ascending order (pin 2-7).
Sometimes, it also changes without me touching the Arduino at all!

I believe the problem lies in the code, but I just can't figure out what's wrong! Please help me!

Thank you.

Fabio881

uhm...one of the first problems that I see in this code is: supposing that you never change the "switchStatus" variable, how do you prevent the "led" variable from becoming increasingly bigger and bigger?
In the first "if-condition" you have this:
Code: [Select]

digitalWrite(led, HIGH);
led += directions;

and (supposing that the second "if-condition" is not accessed) this will simply go on and on, right?
To rephrase: apart from making the LEDs going backwards, it's also not clear to me how do you actually control the led going in the ascending order until a certain value of "led", and not more.

2fjeff

I am not sure if Fabio881's post solved the problem or not to the issue.
I have approached the direction of the flashing lights in a different way.  I stuck to the code written in the book for this project as closely as possible and only made changes to the if(led==7){}statement.
Here is the original code from the book where nothing happens after all the LEDs are lit:
Code: [Select]
const int switchPin=8;
unsigned long previousTime=0;
int switchState=0;
int previous SwitchState=0;
int led=2;
long interval=600000;  //in my code I changed the interval to 10000 to speed everything up

void setup(){
  for(int x=2;x<8;x++){
    pinMode(x,OUTPUT);
  }
pinMode(switchPin,INPUT);
}
void loop(){
  unsigned long currentTime=millis();
  if(currentTime-previousTime>interval){
    previousTime=currentTime;
    digitalWrite(led,HIGH);
    led++;
    if(led==7){  //this is the area we need to modify.  The book has nothing in between the brackets so that you can program what will happen once all 6 LEDs are lit
    }
  }
  switchState=digitalRead(switchPin);
  if(switchState!=previousSwitchState){
    for(int x=2;x<8;x++){
      digitalWrite(x,LOW);
    }
    led=2;
   previousTime=CurrentTime;
  }
  previousSwitchState=switchState;
}


There is a small problem with the code as is straight from the book and has to do with line 18 & 19 of the code:
Code: [Select]
led++;
if(led==7){

if the code is left as is once the 5th LED lights up the the led variable will increase to 7 and whatever you put between the {} of the if(led==7) statement will occur before all LEDs are lit.  So to correct this we change the == signs to > sign and then things will work how we want them.  Now here is the code I placed within the if(led>7)statement:
Code: [Select]
if(led>7){
    // these statements will cause the LEDs to flash on and off from low to high than high to low
     for(int x=2;x<8;x++){
        digitalWrite(x,HIGH);  //will cause LEDs to flash on for 100millseconds each in succesion from 2-7
        delay(100);
        digitalWrite(x,LOW);
      }
      for(int x=7;x>1;x--){
        digitalWrite(x,HIGH);  //will cause LEDs to flash on for 100millseconds each in succesion from 7-2
        delay(100);
        digitalWrite(x,LOW);
      }
    }

 Now once the 6th LED comes on they will flash from the first to last LED in succession then Flash from the last LED to the first.  However, as the code is, after the LEDs do one cycle of the up and down flashing they will pause for an 1 interval before flashing again.  This happens every time. That is because the entire if(led>7){ statement is within the if(currentTime-previousTime>interval){ statement, and therefore we have to wait one full interval for the process to continue.
Code: [Select]
if(currentTime-previousTime>interval){
            previousTime=currentTime;
            digitalWrite(led,HIGH);
            led++;
            if(led>7){

  Fellow Arduino Community Member BadMrFresli discovered that if we separate our two if statements by putting a } after the led++; there will no longer be a pause and the lights will flash as intended. Here is what this section of code should look to work properly:
Code: [Select]
if(currentTime-previousTime>interval){
            previousTime=currentTime;
            digitalWrite(led,HIGH);
            led++;
         }
         if(led>7){
        // these statements will cause the LEDs to flash on and off from low to high than high to low
           for(int x=2;x<8;x++){
             digitalWrite(x,HIGH);  //will cause LEDs to flash on for 100millseconds each in succesion from 2-7
             delay(100);
             digitalWrite(x,LOW);
          }
          for(int x=7;x>1;x--){
            digitalWrite(x,HIGH);  //will cause LEDs to flash on for 100millseconds each in succesion from 7-2
            delay(100);
            digitalWrite(x,LOW);
         }
       }

by separating the if(LED>7) statement from the if(currentTime-previousTime>interval) statement it is no longer dependent on the program to go through an interval.  It is now only dependent on whether or not the variable led is greater than 7.
I hope this helps
If there are any further questions please post questions to this forum

BadMrFresli

Hello Everyone

I think the problem with nigelsohs code comes from the two if statements at the end.
One changes the value of the directions variable and the other changes it right back.
You could use an "if else" or "switch / case" statement instead.
Or tie "directions" and the startLED to the switchState variable. 
That way the positioning has an influence. (Just like with a real hourglass.)

Hope this helps

@fabio881: You could add something like this:

 
Code: [Select]
if(led > 7){                             
    led = 8;                               
  }
  if(led < 2) {
    led = 1;
  }


I did that to stop the in- / decreasing of the variable, to the point where it disturbs the functioning of the code.
To be fair, if you keep the original interval of 10min, that takes pretty long..



Go Up