State machines logic for Rock Band project

Hello,

It was suggested to me that I use a state machine for what Im trying to accomplish. Here is a link to the first forum I was in RockBand KickPedal help please! - #14 by barnabeblunderbuss - Project Guidance - Arduino Forum. What im trying to make happen is when I place a magnet near the Reed I need the LED to blink once and stay off until I re apply the magnet. Here is a video of whats is happening now https://www.facebook.com/aaron.sigler.35/videos/1052267774804016/?l=1902319110972754836. As you can see the LED comes and stays on and even when I remove the magnet. And when I re apply it you see the LED get dimmer and then bright when I remove it again. Any help would be greatly appreciated.

looks like the led is being turned on and off real fast so it gives the impression that its dim. Post the code you are using for one of us to see whats going on

OMG i'm an idiot I meant to post it earlier.

#define S_IDLE 1
#define S_KICKON 2
#define S_WAITON 3
#define S_KICKOFF 4

int Reed = 0;

void setup()
{
  pinMode(4,OUTPUT);
  pinMode(2,INPUT);
}

void loop()
{
  Reed = digitalRead(2);
  static int state = S_IDLE; // Start in idle state
  
  switch(state)
  {
    case S_IDLE: // Start state do nothing
      if (Reed == HIGH){ // If Magnet is near Reed
      state = S_KICKON;
      }
      break;
    case S_KICKON: // Pedal is down for first time, game hears kick
      digitalWrite(4,HIGH); // 8mm is hearing constant kick
        state = S_WAITON;
      break;
    case S_WAITON: // Pedal is down and wanting to wait, game heard kick
      if (Reed == HIGH) { // If Magnet is near Reed
        state = S_KICKOFF;
      }
      break;
    case S_KICKOFF: // Pedal is down and waiting, game hears nothing
      digitalWrite(4,LOW); // 8mm is hearing nothing
      if (Reed == LOW) { // If Magnet is away from Reed
        digitalWrite(4,LOW);
      }
      else {
        state = S_KICKON;
      }
  }
}

your code makes no real sense. I see notes like Pedal is down and waiting but there nothing coded that says the code is going to wait for anything

the code practically says when magnet is near reed turn the led on and off every 3 loops of the program which will be real fast

  case S_KICKOFF: // Pedal is down and waiting, game hears nothing
    
      digitalWrite(4, LOW); // this line is wrong and will be exectuted
      //no matter what state the magnet is. 
      
      if (Reed == LOW) { // If Magnet is away from Reed
        digitalWrite(4, LOW);
      }
      else {
        state = S_KICKON;

I think this is along the lines of what you want

you will have to reset the pin numbers

byte reedinput = 4;
byte led = 13;

byte prevreed = 0;
byte reed = 0;
byte ledflash=0;
unsigned long prevmillis_1 = 0;
unsigned long prevmillis_2 = 0;
void setup()
{
  pinMode(led, OUTPUT);
  pinMode(reedinput, INPUT);//research INPUT_PULLUP
}

void loop()
{
  reed = digitalRead(reedinput);

  if (prevreed != reed) {//state changed
    if (millis() - prevmillis_1 > 40) {//debounce 40ms might not be required
      if (reed == HIGH) {//compare switch 
        ledflash = 1;//flag used later
      } else {//switch was LOW
        ledflash = 0;
      }
      prevmillis_1 = millis();
      prevreed = reed;
    }
  }


  if (ledflash == 1) {
    if (millis() - prevmillis_2 < 20) {//20 equals time it will flash on onetime 20ms
      digitalWrite(led, HIGH);//turn led on
    } else {
      digitalWrite(led, LOW);
    }
  } else {
    digitalWrite(led, LOW);//added in case you want long flash
    prevmillis_2 = millis();
  }
}

p.s state machines are great but you have to have something controlling the change of state

gpop1:
your code makes no real sense. I see notes like Pedal is down and waiting but there nothing coded that says the code is going to wait for anything

It makes plenty of sense as a state machine. The fact that loop() is called repeatedly takes care of the waiting - it waits in a particular state.

Main thing I'm noticing is that there is no reference to millis() - nothing that will make a blink happen.

I would have expected:

IDLE -> if reed, then note the time, turn the LED on, and move to blink-on state.
BLINK_ON -> if no reed, then turn LED off and move back to IDLE. Else, if the blink has timed out then turn LED off and move to BLINK_OFF
BLINK_OFF -> if no reed, move back to IDLE.

This logic means that the LED will cut short if the reed goes off during the blink period. If you want a constant-length blink that's triggered by the reed, then you need two state machines: one to do the blinking, and one to keep track of the reed going high. Happilly, these machines are simple enough that you really only want a boolean for each (and an unsigned long for the LED). Alternatively, you can do it as a machine with four states - a compound machine that's the product of the two simpler machines.

At the end of the day, you need two booleans and a timeout unsigned long. But you can understand them as a pair of two-state state machines.

boolean ledOn;
unsigned int ledStart;
boolean reed;

loop() {
  boolean reed_now = digitalRead(2);
  if(reed_now && !reed) {
    if(!ledOn) {
      digitalWrite(LED, HIGH);
      ledOn = true;
    }
    ledStart = millis();
  }

  if(ledOn && millis() - ledStart > 100) {
      digitalWrite(LED, LOW);
      ledOn = false;
  }

  reed = reed_now;
}

You need a break; at the end of your last case statement.
I also suggest a default case, for if your state becomes anything more than 4 or less than 0.

Thanks guys

gpop1 so if I use the code you have provided I should just have to move the LED to pin 13 and the Reed to pin 4?

Henry_Best:
You need a break; at the end of your last case statement.

break is not a requirement. It is usually good design to have a break, but without it the execution will just fall through to the next statement, so is not actually necessary for the last case statement.

barnabeblunderbuss:
Thanks guys

gpop1 so if I use the code you have provided I should just have to move the LED to pin 13 and the Reed to pin 4?

no you need to change the numbers to what ever suits you. I programmed using 13 as that's a onboard led and I had a breadboard already set up with a button on pin 4. so I used them to test the code before I posted it.
I forgot to put the numbers back before I copied the code to post.