Programming help

Hi
i am working on a arduino project with a group. I was responsible for the physical construction while one of my team mates was responsible for the coding aspect. My team mate has left the course and so i am left to code the box which is due in the next two weeks. While my supervisor has offered full marks on the assignment due to my contribution being complete, i would still like to complete the project.
Because of my low coding skill i was hoping for some help with programming my box.

Part 1
The design of the box is that until the button is pressed nothing happens.
Once the button is held down the program activates.

While the button is pressed the three green and 1 red led light up. If the button is released the green lights will turn off 1 by 1 each second untill only the red is left.

Part 2
Once the red led is left it should start blinking and the arduino should pulse the buzzer until the button is held down.
if the button is held down while the green lights are turning off it should interrupt the program and return to the held state.

with my basic skills i have programmed part 1 but cannot figure out how to get part 2 to work.
i believe i need to use interupts but do not know how to program them.
would someone be able to help?

green leds are 5,6,7
red led is 8

my code is pasted below.

void setup() {
  /* Button LED */
  pinMode(2, OUTPUT);
  /* Button */
  pinMode(3, INPUT_PULLUP);

  /* LEDs Descending */
  pinMode(5, OUTPUT);
  pinMode(6, OUTPUT);
  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);

  /* Buzzer */
  pinMode(11, OUTPUT);
}

// the loop function runs over and over again forever
void loop() {
  for (;;) {
    if (digitalRead(3) == LOW) {
      delay(100);
      if (digitalRead(3) == LOW)
        break;
    }
  }

  digitalWrite(11, LOW);

  digitalWrite(2, LOW);

  digitalWrite(5, HIGH);
  digitalWrite(6, HIGH); 
  digitalWrite(7, HIGH);
  digitalWrite(8, HIGH);

  for (;;) {
    if (digitalRead(3) == HIGH) {
      delay(100);
      if (digitalRead(3) == HIGH)
        break;
    }
  }

  for (uint8_t i = 5; i < 8; i++) {
    delay(1000);
    digitalWrite(i, LOW);
  }

  digitalWrite(11, HIGH);
}

any help would be greatly appreciated

At any point in time your program will be in one of several states and the code for that state will need to be executed. That lends itself to using what has become known as a state machine.

At first it sounds scary but it need not be. I would start by working out what the states will be, what the code is doing when in each state and assigning a number to each state. For each state decide what conditions need to be set before entering the state, such as the start time for states needing timing and the conditions, such as changes of input and/or time passing, that will cause the state to end and the destination state when the current one ends. Note that there may be different destination states depending on the exit conditions of the current state.

Do all of this without writing any code. Then start with what I will call state 0 (doing nothing waiting for input. Personally I would use switch/case and give the states names but you can use if/else too.

Each time through loop() execute the code for the current state, ie test the input. When the input is detected turn on the LEDs and move to state 1 (red and green on). The exit condition will be release of the button which will cause a move to the next state.

It is important not to introduce delay()s into the code for the states so use millis() for timing to keep the program moving. You may find Using millis() for timing. A beginners guide, Several things at the same time and the BlinkWithoutDelay example in the IDE helpful.

Adventmatrix:
i believe i need to use interupts but do not know how to program them.

Absolutly not :wink:

You're problem is (as usual) that you use delay(). Have look at Blink without delay. The idea is that you leave the loop() running as fast as possible and only act when you need to.

The design of the box is that until the button is pressed nothing happens.

That can NOT possibly work. Part of what the Arduino program needs to do is read the state of the pin that the switch is connected to. So, the program must be running AT ALL TIMES.

What it does when the switch IS pressed, or BECOMES pressed, is for you to code.

Once the button is held down

Why does the switch (leave the buttons on your shirt; they will be useless sewn onto the Arduino) have to be held?

What, exactly, should happen when the switch becomes pressed? What, exactly, should happen when the switch becomes released? You are aware of the state change detection example, right?

Ok i've just tried to use machine states (i think).

now the button is lit up but nothing happens when i press or hold the button.
Once the button is pressed it should switch state to case 2 HELD.

here is the code

#define OFF 0
#define HELD 1
#define RELEASE 2
#define BUZZ 3
#define DELAY 1000
#define DELAY2 100

static unsigned int time;
static unsigned int button;
static unsigned int Delay;

uint8_t fsm_state = OFF;

void setup() {
  /* Button LED */
  pinMode(2, OUTPUT);
  /* Button */
  pinMode(3, INPUT_PULLUP);

  /* LEDs Descending */
  pinMode(5, OUTPUT);
  pinMode(6, OUTPUT);
  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);

  /* Buzzer */
  pinMode(11, OUTPUT);

  time = 0;


}

// the loop function runs over and over again forever
 //state machine
void loop(){
  button = digitalRead(3);
  switch (fsm_state)
  {
    case OFF:  //Statements to execute every time LED_OFF is reached
      digitalWrite(2, HIGH);
      digitalWrite(5, LOW);
      digitalWrite(6, LOW);
      digitalWrite(7, LOW);
      digitalWrite(8, LOW);
      break;
          default:
      break;
  }
  digitalRead(3);
   switch (fsm_state)
  {      
    case HELD:   //Statements to execute every time LED_ON is reached
    if(button == HIGH)
    digitalWrite(13, LOW);
      digitalWrite(2, LOW);
      digitalWrite(5, HIGH);
      digitalWrite(6, HIGH);
      digitalWrite(7, HIGH);
      digitalWrite(8, HIGH);
      break;
    default:
      break;
  }
   switch (fsm_state)
  {
          case RELEASE:   //Statements to execute every time LED_ON is reached
    if(button == LOW)
      digitalWrite(2, HIGH);
      DELAY2;
      digitalWrite(5, LOW);
      DELAY;
      digitalWrite(6, LOW);
      DELAY;
      digitalWrite(7, LOW);
fsm_state = BUZZ;
      break;
    default:
      break;
  }
   switch (fsm_state)
  {
            case BUZZ:   //Statements to execute every time LED_ON is reached
      digitalWrite(11, HIGH);
      digitalWrite(8, HIGH);
      DELAY2;
      digitalWrite(11, LOW);
      digitalWrite(8, LOW);
      DELAY2;
  }

  }

arduino compiles with no complaint so the code is correctly written, but im figuring not correctly programmed.

PaulS:
That can NOT possibly work. Part of what the Arduino program needs to do is read the state of the pin that the switch is connected to. So, the program must be running AT ALL TIMES.

What it does when the switch IS pressed, or BECOMES pressed, is for you to code.
Why does the switch (leave the buttons on your shirt; they will be useless sewn onto the Arduino) have to be held?

What, exactly, should happen when the switch becomes pressed? What, exactly, should happen when the switch becomes released? You are aware of the state change detection example, right?

So the design is as follows:

state 0: Once the box is turned on the LED arcade button lights up, then does nothing untill the button is activated.

State 1: once the button activates the button led turns off and the three green and 1 red turn on, and stay static.

state 2: if the button is released the green leds turn off in turn after 1 second after each other.

state 3: if all the green leds are off and the red is on, the red led starts blinking and the buzzer starts going on and off at half second intervels.

if the button is held down at anytime i want it to default back to the state 1

I have not looked in detail but your use of switch/case is flawed

If should be

switch(state variable)
case X
code for case X
break

case Y
code for case Y
break

case Z
code for case Z
break
end of switch

  digitalRead(3);

If you don't care what digitalRead() returns, why bother calling it?

@LarryD's state machine and timer tutorial demonstrates the basics of what you need to implement your box. Not everything in the tutorial necessarily applies to your situation but, the essentials are there.

PaulS:

  digitalRead(3);

If you don't care what digitalRead() returns, why bother calling it?

Mate i have no coding knowledge so just stating that something won't work without giving me any indication of why it won't work doesn't help,
i need guidance not cryptic hints.

I have gotten the state machines to work now and switch on button state.except for in case 'release'

i want it to read the button and if its low switch back to state 'held' if the button is unchanged carry on.
i just can't get it to do that, it just carries on regardless until it switches to 'buzz'

#define OFF 0
#define HELD 1
#define RELEASE 2
#define BUZZ 3
#define DELAY 1000
#define DELAY2 100

static unsigned int time;
static unsigned int button;
static unsigned int Delay;

uint8_t fsm_state = OFF;

void setup() {
  /* Button LED */
  pinMode(2, OUTPUT);
  /* Button */
  pinMode(3, INPUT_PULLUP);

  /* LEDs Descending */
  pinMode(5, OUTPUT);
  pinMode(6, OUTPUT);
  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);

  /* Buzzer */
  pinMode(11, OUTPUT);

  time = 0;


}

// the loop function runs over and over again forever
 //state machine
void loop(){
  button = digitalRead(3);
  switch (fsm_state)
  {
    case OFF:  //Statements to execute every time LED_OFF is reached
      digitalWrite(2, HIGH);
      digitalWrite(5, LOW);
      digitalWrite(6, LOW);
      digitalWrite(7, LOW);
      digitalWrite(8, LOW);
      digitalRead(button);
if (button == LOW)
      fsm_state = HELD;
break;

  }
      switch (fsm_state)
      {
    case HELD:   //Statements to execute every time LED_ON is reached
    if(button == LOW)
    {
    digitalWrite(13, LOW);
      digitalWrite(2, LOW);
      digitalWrite(5, HIGH);
      digitalWrite(6, HIGH);
      digitalWrite(7, HIGH);
      digitalWrite(8, HIGH);
      delay (100);
      digitalRead(button);
     if (button == HIGH)
      fsm_state = RELEASE;
      break;
    }
          case RELEASE:   //Statements to execute every time LED_ON is reached
    if(button == HIGH)
      {
      digitalWrite(2, HIGH);
      digitalWrite(5, HIGH);
      digitalWrite(6, HIGH);
      digitalWrite(7, HIGH);
      digitalWrite(8, HIGH);
      delay (1000);
      digitalRead(button);
      if (button == LOW)
      fsm_state = HELD;
      digitalWrite(5, LOW);
      delay (1000);
      digitalRead(button);
      if (button == LOW)
      fsm_state = HELD;
      digitalWrite(6, LOW);
      delay (1000);
      digitalRead(button);
      if (button == LOW)
      fsm_state = HELD;
      digitalWrite(7, LOW);
}
fsm_state = BUZZ;
      break;

            case BUZZ:   //Statements to execute every time LED_ON is reached
      digitalWrite(11, HIGH);
      digitalWrite(8, HIGH);
      delay(100);
      digitalWrite(11, LOW);
      digitalWrite(8, LOW);
      delay (100);
      digitalRead(button);
if (button == LOW)
      fsm_state = HELD;
  }

  }

Delta_G:
You have the same issue in many other places. You seem to think that

digitalread(3);

is going to magically place the value read into the variable button.

You obviously understood that how to make that work at the top of loop. I don't understand how it fell apart later. But if your line of code doesn't start out with

button =

and then something you aren't changing the value of button.

So your telling me i don't need to use digitalread(button) before an if statement determined by the buttons position. i.e.

digitalread (button);
if (button == HIGH)

Delta_G:
If you want the if statement to have an updated value from the button then you have to read the button again. BUT you have to save the value you read or you're just using the value that you read at the top of loop.

Listen, I guess you're a bit dense so I'll just spoon feed you this one. Trying to help you see it isn't getting anywhere.

If you want the variable button to have the value you read from the pin then you have to write:

button = digitalRead(3);

Just writing

digitalRead(3);

reads the pin and throws away the result.

Right now the only place you read the button is at the top of loop. But there are several places in your code where you seem to expect button to have a new value after you call

digitalRead(3);

But that DOES NOT put any value into button. Nope. The button variable will still have the same value it got when you actually did it right the one time at the top of loop.

If you bothered to read the whole topic instead of being an ASS by calling me dense you would know i have ZERO coding experience.

i am working on this from a couple of handout i got from someone who is doing an arduino coding course.

The reason i posted to this forum is so i could get help not so i can be insulted by an elitist ass.

The handouts i have say i need to put digitalread(*) before in IF statement.
I've been working from what is obviously an incorrect worksheet.

so if you can't be nice to someone who has obviously got no coding skills and is try to learn as best he can then get lost

Check that the Flow Chart (given below) has contained every step of your program logic except the Interrupt Service Routine which you have not clearly provided. If yes, it can be helpful for you to code your program yourself giving attention to all the clues that have been given in the previous posts of this thread. While converting this Flow Chart into codes, please remember these points:
1. Keep the labels in the comment field after //
2. If you have problem to use while-do and do-while loops, you may use goto statement for the time being. Once the program works, replace all these goto statements by the appropriate loop structures.

3. You have not mentioned the DPin connection of the button; It can be taken as being connected with DPin-4/INTO with internal pull-up resistor enabled. So, the inactive level (when button is not closed) of the button is LH.
4. Use millis() function instead of delay(1000) function while blinking/checking button status; otherwise, you will most likely miss active states )button closure) of the button.
.

Few Example Codings for the Flow Chart

void setup()
{
     //;                                                   START:
     pinMode(8, OUTPUT);                        //L1:
     .............................. 
     pinMode(4, INPUT_PULLUP);             //button is connected with DPin-4 with internal pull-up
     ...............................

     bool n = digitalRead(4);                    //L2: //Read buttton status (1-bit and save in 1-bit variable n
     if (n != LOw)                                    //button is not closed
     {
         goto L2;
      }
      .....................................

      n = digitalRead(4);                   //L5: Read button status
      if (n != HIGH)
     {
         goto L5;                                      //button is not opened
     }
     ......................................
}


void loop()
{

}


ISR(INT0_vect)      //Interrupt Service Routine (ISR) due to interrupt caused by button close
{

}

This is incredibly bad advice.

I agree; but, seeing the programming level of the OP from OFF screen I could only conceive this unwanted advice. :slight_smile:

all of the other states in my code are working except for the following section.

         case RELEASE:   //Statements to execute every time LED_ON is reached
    if(button == HIGH)
      {
        button = digitalRead(3);
      digitalWrite(2, HIGH);
      digitalWrite(5, HIGH);
      digitalWrite(6, HIGH);
      digitalWrite(7, HIGH);
      digitalWrite(8, HIGH);
      delay (100);
      if (button == LOW)
      fsm_state = HELD;
      digitalWrite(5, LOW);
      delay (1000);
      if (button == LOW)
      fsm_state = HELD;
      digitalWrite(6, LOW);
      delay (1000);
      if (button == LOW)
      fsm_state = HELD;
      digitalWrite(7, LOW);
      delay (1000);
}
fsm_state = BUZZ;
      break;

i want it to check the button between each light switching off. if the button is not held down carry on.
if the button has been held down switch back to state HELD.
i was hoping what i wrote above in the code would work but it does not

Delta_G:
OK, So in this code snippet let's look again.

You start with

if(button == HIGH)

So we know the button variable has the value HIGH when you first enter here.

Next line you read the pin and store into button: Yay, you got it!

button = digitalRead(3);

Then you do some digitalWrites and a delay and check the value of the button:

if (button == LOW)

fsm_state = HELD;
      digitalWrite(5, LOW);
      delay (1000);




Note that without the braces only the first line is conditional (another place where you really gotta go do a tutorial on C++). So if the button is LOW we set the state to HELD. Then we digitalWrite a pin (regardless of the state of button) and call a delay:

Then you have:


if (button == LOW)
      fsm_state = HELD;
      digitalWrite(5, LOW);
      delay (1000);




You check the button state again. Do you think it might have changed in that one second? It can't. It will have the same value you last read into it. So if it was LOW to write to pin 5 it will still be LOW to write to pin 6. 

Finally at the end of this case you set the fsm_state variable to BUZZ. So why bother ever setting it to HELD if you're just going to set it right back to BUZZ? It wasn't ever HELD in any place that made sense. It was only HELD for a little while you wrote a couple of pins and called some delay. But before you ever get round to use it you set it back to BUZZ.

what i want it to do is if the button state has changed to LOW i want it to switch to state 2 (called HELD).
if the button state is still set to HIGH i want it to continue on as it were in state 3.

when it gets to the end of the code and nothing has changed i switches to state 4 (BUZZ) which it does perfectly fine.

its the switching to state 2 if the button has changed that i can't get to work.

So i figured it out.

because it was going line by line it would set the state but not switch untill the state had run its course.

so now each light turning off is a seperate state and the end of each state reads the button and then ends the state and moves on dependant on what the state has been set to dependant on the button state.

#define OFF 0
#define HELD 1
#define RELEASE 2
#define BUZZ 3
#define Countdown1 4
#define Countdown2 5
#define Countdown3 6

static unsigned int time;
static unsigned int button;
static unsigned int Delay;

uint8_t fsm_state = OFF;

void setup() {
  /* Button LED */
  pinMode(2, OUTPUT);
  /* Button */
  pinMode(3, INPUT_PULLUP);

  /* LEDs Descending */
  pinMode(5, OUTPUT);
  pinMode(6, OUTPUT);
  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);

  /* Buzzer */
  pinMode(11, OUTPUT);

  time = 0;


}

// the loop function runs over and over again forever
 //state machine
void loop(){
  button = digitalRead(3);
  switch (fsm_state)
  {
    case OFF:  //Statements to execute every time LED_OFF is reached
      digitalWrite(2, HIGH);
      digitalWrite(5, LOW);
      digitalWrite(6, LOW);
      digitalWrite(7, LOW);
      digitalWrite(8, LOW);
if (button == LOW)
      fsm_state = HELD;
break;

  }
      switch (fsm_state)
      {
    case HELD:   //Statements to execute every time LED_ON is reached
    if(button == LOW)
    {
    digitalWrite(13, LOW);
      digitalWrite(2, LOW);
      digitalWrite(5, HIGH);
      digitalWrite(6, HIGH);
      digitalWrite(7, HIGH);
      digitalWrite(8, HIGH);
      delay (100);
     if (button == HIGH)
      fsm_state = RELEASE;
      break;
    }
          case RELEASE:   //Statements to execute every time LED_ON is reached
    if(button == HIGH)
      {
        button = digitalRead(3);
        if (button == LOW)
        fsm_state = HELD;
        else if (button == HIGH)
      digitalWrite(2, HIGH);
      digitalWrite(5, HIGH);
      digitalWrite(6, HIGH);
      digitalWrite(7, HIGH);
      digitalWrite(8, HIGH);
      delay (100);
      if (button == LOW)
      fsm_state = HELD;
      else if (button == HIGH)
      fsm_state = Countdown1;
      }
      break;
      
case Countdown1:
      {
      digitalWrite(5, LOW);
      delay (1000);
   button = digitalRead(3);
      if (button == LOW)
      fsm_state = HELD;
      else if (button == HIGH)
      fsm_state = Countdown2;
      }
      break;

      case Countdown2:
      {
      digitalWrite(6, LOW);
      delay (1000);
     button = digitalRead(3);
      if (button == LOW)
      fsm_state = HELD;
      else if (button == HIGH)
      fsm_state = Countdown3;
      }
break;
      
       case Countdown3:
      {
      digitalWrite(7, LOW);
      delay (1000);
     button = digitalRead(3);
      if (button == LOW)
      fsm_state = HELD;
      else if (button == HIGH)
      fsm_state = BUZZ;
     
}
      break;

            case BUZZ:   //Statements to execute every time LED_ON is reached
      digitalWrite(11, HIGH);
      digitalWrite(8, HIGH);
      delay(100);
      digitalWrite(11, LOW);
      digitalWrite(8, LOW);
      delay (100);
if (button == LOW)
      fsm_state = HELD;
  }

  }

Delta_G:
Listen, I guess you're a bit dense so I'll just spoon feed you this one.

Adventmatrix:
...instead of being an ASS...

Adventmatrix:
if i wanted to talk to assholes i'd be on reddit, i came here looking for help not criticism from an elitist ass.

No more personal attacks. No more cussing. Only warning.

This thread is an example of why people should start at the beginning and work through examples until they understand the concepts before moving on to more complex projects.

Adventmatrix: take a few steps backward. Hook a button up to your Arduino. Use it to turn the LED on and off depending on the state of the button (pressed or not pressed.) Then hook several LEDs to several digital pins. Program the Arduino so that pressing the button makes each LED turn on then off, in sequence. Then program the Arduino so that holding the button makes the LEDs go on and off in sequence as long as the button is held, and turns the LEDs off when released. At this point, you understand how buttons - correction, switches - work.

Then you will have the tools to proceed with your complex project. Skipping the early steps robs you of understanding fundamental concepts required to think through the steps needed to program your project.

The alternative is to convince your software person to return to the team. Incentivize them! Learning these rudimentary skills is essential to any modern engineering related job, unless you are a Widlar level genius. Then you can do whatever you want, no matter what life's moderators say. :wink: