next topic »

[on:]

hello, i'm not sure why this isn't working and was hoping someone could clue me in..

i've got a momentary pushbutton, 4 LEDs and a small relay hooked up to the UNO at the moment. here's what i want to do:

1. first (on) button push: relay closes, LEDs fade on to 255. this is working.

2. after LEDs hit 255, they begin to randomly blink, fast. this is working as well.

3. second (off) button push: relay opens, LEDs fade out, loop breaks. this fade-out effect/break is not working, it just drops everything to 0 immediately. i've tried pretty much everything i could think of to mod this code to open the relay then fade the LEDs to 0 after the button push, but i'm just fumbling in the dark here. here's an early iteration,  i'd really appreciate any help =)

Code:

#define LED 3
#define BUTTON 2
#define LED2 13
#define LED3 5
#define LED4 9
#define relay 12


int i = 0;
int state = 0;
int val = 0;

//A function to see if the button has been pressed, returns an int
int CheckButton()
{
  return(digitalRead(BUTTON) == HIGH);
}     

void setup()
{
  pinMode(LED, OUTPUT);
  pinMode(LED2, OUTPUT);
  pinMode(LED3, OUTPUT);
  pinMode(LED4, OUTPUT);
  pinMode(BUTTON, INPUT);
  pinMode(relay, OUTPUT);
}

void loop()
{   

  //Check to see if the button has been pressed.
  if(digitalRead(BUTTON) == HIGH)
  {
    //While the digital read statement is true (high) continue the loop
    while(digitalRead(BUTTON) == HIGH)
      continue;

    //state is true as state is 0 and if we do !state then it is 1
    state = !state; 
  }

  //If state is 1 which it will be as we wont make it 0. If we dont use state then we have to keep pressing the button
  if(state)
  {
    for (i = 0; i < 255; i++)
    {
      //If checkbutton is true it means the button has been pressed
      if(CheckButton())
      {
        //Turn off the LED and break the loop
        analogWrite(LED, 0);
        analogWrite(LED2, 0);
        analogWrite(LED3, 0);
        analogWrite(LED4, 0);
        digitalWrite(relay, 0);

        break;
      }

      //Continue fading
      analogWrite(LED, i);
      analogWrite(LED2, i);
      analogWrite(LED3, i);
      analogWrite(LED4, i);
      delay(15);     
      digitalWrite(relay, 1);
    }

    for (i != 255; i > 0; i--)
    {
      //If the checkbutton is true it means the button has been pressed
      if(CheckButton())
      {
        //Turn off the LED and break the loop
        analogWrite(LED, 0);
        analogWrite(LED2, 0);
        analogWrite(LED3, 0);
        analogWrite(LED4, 0);                     
        digitalWrite(relay, 0);
        break;
      }

      //Continue fading
      analogWrite(LED, random(20, 254));         
      analogWrite(LED2, random(20, 254));
      analogWrite(LED3, random(20, 254));
      analogWrite(LED4, random(20, 254));
      delay(30);
      digitalWrite(relay, 1);
    }



  }
}

[Reply #1 on:]

Code:

int CheckButton()
{
  return(digitalRead(BUTTON) == HIGH);
}   

The comparison between the value read from the switch (put the button back on the shirt you stole it from) and HIGH is either true or false. So the return type from this function should be boolean, not int.

Why is your
      code randomly indented?
  Don't you think
                      it would be easier
         to read if it was
    indented properly?
  Using Tools + Auto format before
                      posting code is a nice touch.

I think that you need to learn about edge detection. Just determining that the switch pin IS HIGH or LOW is not sufficient. First, you need to determine if the state is, or is not, the same as the last time you checked, Of course, this means that you need to keep track of the previous state.

If the current state is not equal to the previous state, a transition (from pressed to released or from released to pressed) has occurred. The current state (pressed or released) defines which transition it was.

3. second (off) button push: relay opens, LEDs fade out, loop breaks. this fade-out effect/break is not working, it just drops everything to 0 immediately.

Perhaps because that's exactly what you told it to do:

Code:

          for (i = 0; i < 255; i++)
            {
           //If checkbutton is true it means the button has been pressed
        if(CheckButton())
        {
            //Turn off the LED and break the loop
          analogWrite(LED, 0);
          analogWrite(LED2, 0);
          analogWrite(LED3, 0);
          analogWrite(LED4, 0);
          digitalWrite(relay, 0);

[Reply #2 on:]

sorry, fixed the formatting, i had no idea there was an auto formatter, hah..

i realize that the posted code just turns everything off, this is the base i've been working off of.. i've been through many different mods of this and none worked, so i figured i would post something that made sense instead of something hilariously wrong. i know why this isn't working.. but it is doing most of what i want it to.

i'll look into edge detection. it seems like i've used the store button state before to do something, but it's been a while.

[Reply #3 on:]

Some functions would be useful, too.

3. second (off) button push: relay opens, LEDs fade out, loop breaks. this fade-out effect/break is not working, it just drops everything to 0 immediately.

Create a function to fade the LEDs to 0. Call that function when the fade out should start. There is no reason to check switch states in that function, if the function is supposed to just fade the LEDs to 0.

If the LEDs are to fade from there current value to 0, pass the current value(s) to the function, as a starting point for the fade out.

[Reply #4 on:]

awesome, thanks for the function info, i didn't realize there was such a thing and that little tidbit of ignorance has made my limited coding very, very messy 

it's doing exactly what i want now, but the break command doesn't seem to be working, it restarts instead of turning off after the function.. i've only been able to find the barest of info on break, any idea where i might look for more?

Code:

#define LED 3
#define BUTTON 2
#define LED2 13
#define LED3 5
#define LED4 9
#define relay 12


int i = 0;
int state = 0;
int val = 0;

//A function to see if the button has been pressed, returns an int
int CheckButton()
{
  return(digitalRead(BUTTON) == HIGH);
}     

void setup()
{
  pinMode(LED, OUTPUT);
  pinMode(LED2, OUTPUT);
  pinMode(LED3, OUTPUT);
  pinMode(LED4, OUTPUT);
  pinMode(BUTTON, INPUT);
  pinMode(relay, OUTPUT);
}

void loop()
{   

  //Check to see if the button has been pressed.
  if(digitalRead(BUTTON) == HIGH)
  {
    //While the digital read statement is true (high) continue the loop
    while(digitalRead(BUTTON) == HIGH)
      continue;

    //state is true as state is 0 and if we do !state then it is 1
    state = !state; 
  }

  //If state is 1 which it will be as we wont make it 0. If we dont use state then we have to keep pressing the button
  if(state)
  {
    for (i = 0; i < 255; i++)
    {
      //If checkbutton is true it means the button has been pressed
      if(CheckButton())
      {
        //Turn off the LED and break the loop
        analogWrite(LED, 0);
        analogWrite(LED2, 0);
        analogWrite(LED3, 0);
        analogWrite(LED4, 0);
        digitalWrite(relay, 0);

        break;
      }

      //Continue fading
      analogWrite(LED, i);
      analogWrite(LED2, i);
      analogWrite(LED3, i);
      analogWrite(LED4, i);
      delay(5);     
      digitalWrite(relay, 1);
    }

    for (i = 255; i > 0; i--)
    {
      //If the checkbutton is true it means the button has been pressed
      if(CheckButton())
      {
        //Turn off the LED and break the loop
        powerDown();
        break;       
      }     

      //Continue fading
      analogWrite(LED, random(20, 254));         
      analogWrite(LED2, random(20, 254));
      analogWrite(LED3, random(20, 254));
      analogWrite(LED4, random(20, 254));
      delay(30);
      digitalWrite(relay, 1);
    }
  }
}

void powerDown()
{
  {           
    for (i != 255; i > 0; i--)
    {
      //Relay OFF, LEDs fade to 0
      analogWrite(LED, i);
      analogWrite(LED2, i);
      analogWrite(LED3, i);
      analogWrite(LED4, i);                     
      digitalWrite(relay, 0);
      delay(5);

    }

  }

}

[Reply #5 on:]

ok so "break" in the "if" loop is only breaking the "if" loop, instead of the whole void loop. it seems to me like the "break" command should be in the function, so that after the function is called the break occurs, but again, it just breaks the function. the compiler doesn't seem to like a "break" command in the main loop, although i don't think i'd want it there anyway, it needs to be looping.. so how do you break the main loop conditionally?

argh.

[Reply #6 on:]

got it. it's not the prettiest thing, but it gets the job done. thanks for the assistance )

Code:

#define LED 3
#define BUTTON 2
#define LED2 13
#define LED3 5
#define LED4 9
#define relay 12


int i = 0;
int state = 0;
int val = 0;

//A function to see if the button has been pressed, returns an int
int CheckButton()
{
  return(digitalRead(BUTTON) == HIGH);
}     

void setup()
{
  pinMode(LED, OUTPUT);
  pinMode(LED2, OUTPUT);
  pinMode(LED3, OUTPUT);
  pinMode(LED4, OUTPUT);
  pinMode(BUTTON, INPUT);
  pinMode(relay, OUTPUT);
}

void loop()
{   

  //Check to see if the button has been pressed.
  if(digitalRead(BUTTON) == HIGH)
  {
    //While the digital read statement is true (high) continue the loop
    while(digitalRead(BUTTON) == HIGH)
      continue;

    //state is true as state is 0 and if we do !state then it is 1
    state = !state; 
  }

  //If state is 1 which it will be as we wont make it 0. If we dont use state then we have to keep pressing the button
  if(state)
  {
    for (i = 0; i < 255; i++)
    {
      //If checkbutton is true it means the button has been pressed
      if(CheckButton())
      {
        //Turn off the LED and break the loop
        analogWrite(LED, 0);
        analogWrite(LED2, 0);
        analogWrite(LED3, 0);
        analogWrite(LED4, 0);
        digitalWrite(relay, 0);

        break;
      }

      //Continue fading
      analogWrite(LED, i);
      analogWrite(LED2, i);
      analogWrite(LED3, i);
      analogWrite(LED4, i);
      delay(5);     
      digitalWrite(relay, 1);
    }

    for (i = 255; i > 0; i--)
    {
      //If the checkbutton is true it means the button has been pressed
      if(CheckButton())
      {
        //Turn off the LED and break the loop
        powerDown();
        state = 0;
             break; 
      }     

      //Continue fading
      analogWrite(LED, random(20, 254));         
      analogWrite(LED2, random(20, 254));
      analogWrite(LED3, random(20, 254));
      analogWrite(LED4, random(20, 254));
      delay(30);
      digitalWrite(relay, 1);
    }
  }
}

void powerDown()
{
  {           
    for (i != 255; i > 0; i--)
    {
      {
      //Relay OFF, LEDs fade to 0
      analogWrite(LED, i);
      analogWrite(LED2, i);
      analogWrite(LED3, i);
      analogWrite(LED4, i);                     
      digitalWrite(relay, 0);
      delay(5);
      }
      analogWrite(LED, 0);
        analogWrite(LED2, 0);
        analogWrite(LED3, 0);
        analogWrite(LED4, 0);
        digitalWrite(relay, 0);
    }
    }

  }

[Reply #7 on:]

ok so "break" in the "if" loop is only breaking the "if" loop,

The if statement is not a loop. There is nothing to break out of. The break function causes a jump to the first instruction of the end of the for/while statement (each of which cause looping).

The function you wrote has extra curly braces, and is poorly indented. Use the Tools + Auto Format to line stuff up nicely, and you'll see the extra braces. Remove them, and auto format again.

[Reply #8 on:]

@Zach
We like code posted properly (using the # icon). If you want to play the game, you are welcome to, if you post code properly.

Code:

  //Check to see if the button has been pressed.
  if(digitalRead(BUTTON) == HIGH)

That does not check if the switch HAS BEEN pressed. If checks whether the switch IS pressed. Big difference.

[Reply #9 on:]

I am sorry; my bad.

You could modify your post...