Pages: [1] 2   Go Down
Author Topic: Button triggered Macro, problems with interrupt and delay  (Read 1162 times)
0 Members and 1 Guest are viewing this topic.
Offline Offline
Newbie
*
Karma: 0
Posts: 15
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

So I'm building a macro game controller, my intent is that I push a button and the Arduino will run through a macro sequence, ideally I'd like to be able to push the button a 2nd time to immediately stop the macro even if it hasn't completed. pushing it a 3rd time would start the macro sequence back at the beginning again.

I've got the hardware side complete and confirmed working.
I wrote a small program that turns an LED on and off via an interrupt and then I wrote another one that runs through my sequence as part of the main loop, both of those worked great (and confirmed that my hardware is setup properly) but I'm having problems now that I'm trying to combine them.

Initially when I tried running my macro sequence via an interrupt it would just skip over the calls to "delay" so I swapped that out for a manual calculation of delay using millis() but I can't seem to get that working and I don't know why. I would appreciate any help or advice.

Here is my sketch:

Code:
/*
 SNES Controller Test
 presses each of the buttons on the controller when the go button is pushed
 should also stop the sequence if the go button is pushed a second time
 */
 
//set names for the output pins
int A = 4;
int B = 0;
int X = 1;
int Y = 2;

int R = 12;
int L = 7;

int Select = 13;

int Up = 11;
int Down = 10;
int Left = 9;
int Right = 8;

int Go = 1; //interrupt 1 pin #3

volatile int state = HIGH; // The go button state toggle
unsigned long lastInterruptTime = 0; //set the interrupt counter

// the setup routine runs once when you press reset:
void setup() {
  // initialize the digital pins as an output.
  pinMode(A, OUTPUT);
  pinMode(B, OUTPUT);
  pinMode(X, OUTPUT);
  pinMode(Y, OUTPUT);
  pinMode(L, OUTPUT);
  pinMode(R, OUTPUT);
  pinMode(Select, OUTPUT);
  pinMode(Up, OUTPUT);
  pinMode(Down, OUTPUT);
  pinMode(Left, OUTPUT);
  pinMode(Right, OUTPUT);
  
  //setup interrupt    
  attachInterrupt(Go, buttonPushed, LOW);
  
  //make sure everything is in the starting state that we want
  allClear();
}

//sets all outputs to off (high)
void allClear(){
  digitalWrite(Up, HIGH);
  digitalWrite(Down, HIGH);
  digitalWrite(Left, HIGH);
  digitalWrite(Right, HIGH);
  digitalWrite(A, HIGH);
  digitalWrite(B, HIGH);
  digitalWrite(X, HIGH);
  digitalWrite(Y, HIGH);
  digitalWrite(L, HIGH);
  digitalWrite(R, HIGH);
  digitalWrite(Select, HIGH);
}

// the loop routine runs over and over again forever:
void loop() {
 wait();
}

// the interrupt routine run with the go button is pushed
void buttonPushed(){
  unsigned long interruptTime = millis(); //check the interrupt time
  
  // If interrupts come faster than 50ms, assume it's a bounce and ignore
  if (interruptTime - lastInterruptTime > 50){
    state = !state; //change state
    if (state){
      doStuff(); //run the sequence
    } else {
      allClear(); //clear the outputs
    }
    lastInterruptTime = interruptTime; //set the last interrupt time
  }
}

// my non "delay" delay routine
void myDelay(int interval){
  unsigned long previousMillis = millis(); //get the start time
  unsigned long currentMillis = millis(); //get the current time
  while (previousMillis + interval < currentMillis){ //loop until the start time plus the desired delay is reached
    currentMillis = millis(); //get the current time
  }
}

void wait(){
  myDelay(1000); // wait for 1 second
}

// the output sequence routine
void doStuff(){
  digitalWrite(Up, LOW);
  myDelay(1000);
  digitalWrite(Up, HIGH);
  digitalWrite(Down, LOW);
  myDelay(1000);
  digitalWrite(Down, HIGH);
  digitalWrite(Left, LOW);
  myDelay(1000);
  digitalWrite(Left, HIGH);
  digitalWrite(Right, LOW);
  myDelay(1000);
  digitalWrite(Right, HIGH);
  digitalWrite(A, LOW);
  myDelay(1000);
  digitalWrite(A, HIGH);
  digitalWrite(B, LOW);
  myDelay(1000);
  digitalWrite(B, HIGH);
  digitalWrite(X, LOW);
  myDelay(1000);
  digitalWrite(X, HIGH);
  digitalWrite(Y, LOW);
  myDelay(1000);
  digitalWrite(Y, HIGH);
  digitalWrite(L, LOW);
  myDelay(1000);
  digitalWrite(L, HIGH);
  digitalWrite(R, LOW);
  myDelay(1000);
  digitalWrite(R, HIGH);
  digitalWrite(Select, LOW);
  myDelay(1000);
  digitalWrite(Select, HIGH);
}
« Last Edit: December 28, 2012, 11:14:44 am by twistedsymphony » Logged

UK
Offline Offline
Shannon Member
****
Karma: 223
Posts: 12630
-
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

You can't use delay() in an interrupt, and you should not be trying to. Interrupts are designed for things that happen very quickly, and your replay sequence does not happen very quickly.

I suggest you restructure your code into two parts:

A function to poll the switch input state, do any debouncing necessary and detect button transitions (up to down). Use this to toggle the state of your sketch between 'replay running' and 'replay not running'. The function should not call delay() or do anything that would prevent it returning promptly. Call this function once in loop().

A function which is called repeatedly while the replay is running. It should use the techniques demonstrated in 'blink without delay' (and also used in your current interrupt handler) to decide whether it is time to perform the next output action, and perform it if so. This function should be called once in loop() if the state is 'replay running'.
Logged

I only provide help via the forum - please do not contact me for private consultancy.

Offline Offline
Newbie
*
Karma: 0
Posts: 15
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

thanks! I'll give the restructuring a shot.

Though the "myDelay" function in my code above is modeled after the "blink without delay" program.

it doesn't seem to actually work though, I'm not sure why.
« Last Edit: December 28, 2012, 11:42:35 am by twistedsymphony » Logged

UK
Offline Offline
Shannon Member
****
Karma: 223
Posts: 12630
-
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

thanks! I'll give the restructuring a shot.

Though the "myDelay" function in my code above is modeled after the "blink without delay" program.

it doesn't seem to actually work though, I'm not sure why.

Yeah, but it's not designed to be used inside an interrupt.
Logged

I only provide help via the forum - please do not contact me for private consultancy.

Offline Offline
Newbie
*
Karma: 0
Posts: 15
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

If I comment out the interrupt code and put my dostuff function in the main loop  it still doesn't work.
Logged

Seattle, WA USA
Offline Offline
Brattain Member
*****
Karma: 644
Posts: 50524
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Though the "myDelay" function in my code above is modeled after the "blink without delay" program
It may be modeled after the blink without delay example, it is still a blocking function. It does nothing different from what the regular delay() function does.
Logged

Global Moderator
Melbourne, Australia
Offline Offline
Brattain Member
*****
Karma: 506
Posts: 19141
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

You are better of replacing this:

Code:
// the loop routine runs over and over again forever:
void loop() {
  wait();
}

with some kind of test for the button push, and then react accordingly.

eg.

Code:
void loop ()
  {
  static byte oldButtonState = HIGH;
  byte newButtonState = digitalRead (theButton);

  if (oldButtonState != newButtonState)
    {
    if (newButtonState == LOW) 
       buttonPushed ();
    oldButtonState = newButtonState;
    }  // end of button down
  }  // end of loop
Logged

http://www.gammon.com.au/electronics

Please post technical questions on the forum - not to me by personal message. Thanks a lot.

Offline Offline
Newbie
*
Karma: 0
Posts: 15
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

ok so I've re-written my sketch per everyone's suggestions... it works!

My only questions now are:
Is there a more elegant way to do what I'm doing between each step in the doStuff function?

I'm interested in loading in the sequence from a separate file and ideally programming the state of all 12 outputs plus the delay time in a single line for ease of reading... I have some ideas but I don't like any of them any suggestions on the best way to go about this?



Code:
/*
 SNES Controller Test
 presses each of the buttons on the controller when the go button is pushed
 should also stop the sequence if the go button is pushed a second time
 */
 
//set names for the output pins
int A = 4;
int B = 0;
int X = 1;
int Y = 2;

int R = 12;
int L = 7;

int Select = 13;

int Up = 11;
int Down = 10;
int Left = 9;
int Right = 8;

int Go = 3;

byte state = HIGH; // The macro state 0=running 1=stopped
byte lastButtonState = HIGH; // The go button state toggle 0=depressed
unsigned long lastPushTime = 0; //the milliseconds since the last time the button was pushed
unsigned long currentMillis = 0; //get the delay start time in milliseconds
unsigned long previousMillis = 0; //get the current time in milliseconds

// the setup routine runs once when you press reset:
void setup() {
  // initialize the digital pins as an output.
  pinMode(A, OUTPUT);
  pinMode(B, OUTPUT);
  pinMode(X, OUTPUT);
  pinMode(Y, OUTPUT);
  pinMode(L, OUTPUT);
  pinMode(R, OUTPUT);
  pinMode(Select, OUTPUT);
  pinMode(Up, OUTPUT);
  pinMode(Down, OUTPUT);
  pinMode(Left, OUTPUT);
  pinMode(Right, OUTPUT);
  
  // initialize the digital pins as an input.
  pinMode(Go, INPUT);
  
  //make sure everything is in the starting state that we want
  allClear();
}

//sets all outputs to off (high)
void allClear(){
  digitalWrite(Up, HIGH);
  digitalWrite(Down, HIGH);
  digitalWrite(Left, HIGH);
  digitalWrite(Right, HIGH);
  digitalWrite(A, HIGH);
  digitalWrite(B, HIGH);
  digitalWrite(X, HIGH);
  digitalWrite(Y, HIGH);
  digitalWrite(L, HIGH);
  digitalWrite(R, HIGH);
  digitalWrite(Select, HIGH);
}

// the loop routine runs over and over again forever:
void loop() {
  checkPushed(); //check if the button was pushed
  if (!state){ //if the state is set to run
    doStuff(); //run the sequence
    state = HIGH; //reset the state so we don't run again
  }
  allClear(); //set all the outputs to the starting position
}

// the interrput routine run with the go button is pushed
void checkPushed(){
  byte buttonState = digitalRead(Go);
  if (lastButtonState && !buttonState){
    unsigned long pushTime = millis(); //check the time
  
    // If pushed again within 60ms, assume it's a bounce and ignore
    if (pushTime - lastPushTime > 60){
      state = !state; //change state
      lastPushTime = pushTime; //set the last check time
    }
  }
  lastButtonState = buttonState;
}

// check for the button push
void myDelay(unsigned long interval){
  currentMillis = millis(); //get the current time
  previousMillis = currentMillis; //get the start time
  while (currentMillis - previousMillis < interval){ //loop until the desired delay is reached
    currentMillis = millis(); //get the current time
    checkPushed(); //check if the button has been pushed
    if (state){ //if the button push triggered a state change
      return; //exit the delay function
    }
  }
}


// the output sequence routine
void doStuff(){
  digitalWrite(Up, LOW); //start of first step in the sequence
  myDelay(1000); //continue running step while checking for a state change
  if (state){ //if there was a state change
    return; //quit the sequence
  }
 digitalWrite(Up, HIGH); //end of first step in the sequence
 
  digitalWrite(Down, LOW);
  myDelay(1000);
  if (state){
    return;
  }
  digitalWrite(Down, HIGH);
  
  digitalWrite(Left, LOW);
  myDelay(1000);
  if (state){
    return;
  }
  digitalWrite(Left, HIGH);
  
  digitalWrite(Right, LOW);
  myDelay(1000);
  if (state){
    return;
  }
  digitalWrite(Right, HIGH);
  
  digitalWrite(A, LOW);
  myDelay(1000);
  if (state){
    return;
  }
  digitalWrite(A, HIGH);
  
  digitalWrite(B, LOW);
  myDelay(1000);
  if (state){
    return;
  }
  digitalWrite(B, HIGH);
  
  digitalWrite(X, LOW);
  myDelay(1000);
  if (state){
    return;
  }
  digitalWrite(X, HIGH);
  
  digitalWrite(Y, LOW);
  myDelay(1000);
  if (state){
    return;
  }
  digitalWrite(Y, HIGH);
  
  digitalWrite(L, LOW);
  myDelay(1000);
  if (state){
    return;
  }
  digitalWrite(L, HIGH);
  
  digitalWrite(R, LOW);
  myDelay(1000);
  if (state){
    return;
  }
  digitalWrite(R, HIGH);
  
  digitalWrite(Select, LOW);
  myDelay(1000);
  if (state){
    return;
  }
  digitalWrite(Select, HIGH);
}
« Last Edit: December 28, 2012, 11:29:58 pm by twistedsymphony » Logged

Seattle, WA USA
Offline Offline
Brattain Member
*****
Karma: 644
Posts: 50524
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Is there a more elegant way to do what I'm doing between each step in the doStuff function?
I don't know about more elegant, but state is not a boolean. It should be, it you are going to use it as such. It should be assigned the values true or false. It should also have a better name, like stuffToDo. It should also be assigned the correct values at the correct times.

Code:
if(!stuffToDo)
   doStuff();
looks pretty stupid, doesn't it?

No to mention which the doStuff() function needs a better name. Something meaningful, like A348fGhw().
Logged

Offline Offline
Newbie
*
Karma: 0
Posts: 15
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

so I found another project that used an external file to separate the logic from the macro and it seems to work very well but now I'm having a weird bug....

So when I run my old "doStuff" function everything works great, I get a low signal on each output one at a time each for 1 second.

when I run my new function instead, which loops through an array and should be doing the exact same thing as "doStuff" the pin for pin_B, and pin_X don't work... I also sometimes get pin_X flashing low along at the same time I loop to the next step (just a quick low back to high, rather than staying low for the full 1 second).

I'm sure it's not the hardware since this doesn't happen on doStuff, Also I'm outputting the raw data that I'm processing to serial and everything seem to be correct.... I'm at a loss as to why this is happening

Code:
/*
 runs through an automated sequence when the button is pushed
 */
 
#include <avr/pgmspace.h> //this is so we can use flash memory instead of SRAM

//set the binary button settings 4 bytes
#define A     0x80000000 //B10000000000000000000000000000000
#define B     0x40000000 //B01000000000000000000000000000000
#define X     0x20000000 //B00100000000000000000000000000000
#define Y     0x10000000 //B00010000000000000000000000000000
#define LB    0x08000000 //B00001000000000000000000000000000
#define RB    0x04000000 //B00000100000000000000000000000000
#define Select  0x02000000 //B00000010000000000000000000000000
#define Up    0x01000000 //B00000001000000000000000000000000
#define Down  0x00800000 //B00000000100000000000000000000000
#define Left  0x00400000 //B00000000010000000000000000000000
#define Right 0x00200000 //B00000000001000000000000000000000

PROGMEM prog_uint32_t steps[] = {
A+1000,
B+1000,
X+1000,
Y+1000,
LB+1000,
RB+1000,
Back+1000,
Up+1000,
Down+1000,
Left+1000,
Right+1000,
};

long sizeOfSeq = (sizeof(steps) / sizeof(steps[0])); //determine the length of the sequence

//set names for the output pins
int pin_A = 4;
int pin_B = 0;
int pin_X = 1;
int pin_Y = 2;

int pin_RB = 12;
int pin_LB = 7;

int pin_Select = 13;

int pin_Up = 11;
int pin_Down = 10;
int pin_Left = 9;
int pin_Right = 8;

int pin_Go = 3;

byte state = HIGH; // The macro state 0=running 1=stopped
byte lastButtonState = HIGH; // The go button state toggle 0=depressed
unsigned long lastPushTime = 0; //the millis since the last time the button was pushed
unsigned long currentMillis = 0; //get the start time
unsigned long previousMillis = 0; //get the current time

// the setup routine runs once when you press reset:
void setup() {
  // initialize the digital pins as an output.
  pinMode(pin_A, OUTPUT);
  pinMode(pin_B, OUTPUT);
  pinMode(pin_X, OUTPUT);
  pinMode(pin_Y, OUTPUT);
  pinMode(pin_LB, OUTPUT);
  pinMode(pin_RB, OUTPUT);
  pinMode(pin_Select, OUTPUT);
  pinMode(pin_Up, OUTPUT);
  pinMode(pin_Down, OUTPUT);
  pinMode(pin_Left, OUTPUT);
  pinMode(pin_Right, OUTPUT);
  
  //setup interrupt    
  pinMode(pin_Go, INPUT);

  //make sure everything is in the starting state that we want
  digitalWrite(pin_Select, LOW); //try to get it set off
  allClear();
}

//sets all outputs to off (high)
void allClear(){
  digitalWrite(pin_Up, HIGH);
  digitalWrite(pin_Down, HIGH);
  digitalWrite(pin_Left, HIGH);
  digitalWrite(pin_Right, HIGH);
  digitalWrite(pin_A, HIGH);
  digitalWrite(pin_B, HIGH);
  digitalWrite(pin_X, HIGH);
  digitalWrite(pin_Y, HIGH);
  digitalWrite(pin_LB, HIGH);
  digitalWrite(pin_RB, HIGH);
  digitalWrite(pin_Select, HIGH);
}

// the loop routine runs over and over again forever:
void loop() {
  checkPushed(); //check if the button was pushed
  if (state == LOW){
    //doStuff();
    runSeq();
    allClear();
    state = HIGH;
  }
}

// the interrput routine run with the go button is pushed
void checkPushed(){
  byte buttonState = digitalRead(pin_Go);
  if (lastButtonState == HIGH && buttonState == LOW){
    unsigned long pushTime = millis(); //check the time
  
    // If pushed again within 60ms, assume it's a bounce and ignore
    if (pushTime - lastPushTime > 60){
      state = !state; //change state
      lastPushTime = pushTime; //set the last check time
    }
  }
  lastButtonState = buttonState;
}

// check for the button push
void myDelay(unsigned long interval){
  currentMillis = millis(); //get the current time
  previousMillis = currentMillis; //get the start time
  while (currentMillis - previousMillis < interval){ //loop until the start time plus the desired delay is reached
    currentMillis = millis(); //get the current time
    checkPushed();
    if (state){
      return;
    }
  }
}

void runSeq(){
  unsigned long seq = 0;
  Serial.begin(9600);
  for(int i=0; i<sizeOfSeq; i++){ //loop through each step in the sequence
    seq = pgm_read_dword(&(steps[i])); //reads the steps from the included file

    Serial.print("seq ");
    Serial.print(i, DEC);
    Serial.print(": ");
    Serial.println(seq, BIN);

    if ((seq&A    ) == A    ) {digitalWrite(pin_A    , LOW);} else {digitalWrite(pin_A    , HIGH);}
    if ((seq&B    ) == B    ) {digitalWrite(pin_B    , LOW);} else {digitalWrite(pin_B    , HIGH);}
    if ((seq&X    ) == X    ) {digitalWrite(pin_X    , LOW);} else {digitalWrite(pin_X    , HIGH);}
    if ((seq&Y    ) == Y    ) {digitalWrite(pin_Y    , LOW);} else {digitalWrite(pin_Y    , HIGH);}
    if ((seq&LB   ) == LB   ) {digitalWrite(pin_LB   , LOW);} else {digitalWrite(pin_LB   , HIGH);}
    if ((seq&RB   ) == RB   ) {digitalWrite(pin_RB   , LOW);} else {digitalWrite(pin_RB   , HIGH);}
    if ((seq&Select) == Select) {digitalWrite(pin_Select , LOW);} else {digitalWrite(pin_Select , HIGH);}
    if ((seq&Up   ) == Up   ) {digitalWrite(pin_Up   , LOW);} else {digitalWrite(pin_Up   , HIGH);}
    if ((seq&Down ) == Down ) {digitalWrite(pin_Down , LOW);} else {digitalWrite(pin_Down , HIGH);}
    if ((seq&Left ) == Left ) {digitalWrite(pin_Left , LOW);} else {digitalWrite(pin_Left , HIGH);}
    if ((seq&Right) == Right) {digitalWrite(pin_Right, LOW);} else {digitalWrite(pin_Right, HIGH);}
    
    myDelay(seq&0x000fffff);
    if (state){
      return;
    }
  }
}

// the output sequence routine
void doStuff(){
  digitalWrite(pin_A, LOW);
  myDelay(1000);
  if (state){
    return;
  }
  digitalWrite(pin_A, HIGH);
  
  digitalWrite(pin_B, LOW);
  myDelay(1000);
  if (state){
    return;
  }
  digitalWrite(pin_B, HIGH);
  
  digitalWrite(pin_X, LOW);
  myDelay(1000);
  if (state){
    return;
  }
  digitalWrite(pin_X, HIGH);
  
  digitalWrite(pin_Y, LOW);
  myDelay(1000);
  if (state){
    return;
  }
  digitalWrite(pin_Y, HIGH);
  
  digitalWrite(pin_LB, LOW);
  myDelay(1000);
  if (state){
    return;
  }
  digitalWrite(pin_LB, HIGH);
  
  digitalWrite(pin_RB, LOW);
  myDelay(1000);
  if (state){
    return;
  }
  digitalWrite(pin_RB, HIGH);
  
  digitalWrite(pin_Select, LOW);
  myDelay(1000);
  if (state){
    return;
  }
  digitalWrite(pin_Select, HIGH);
  
  digitalWrite(pin_Up, LOW);
  myDelay(1000);
  if (state){
    return;
  }
 digitalWrite(pin_Up, HIGH);
 
  digitalWrite(pin_Down, LOW);
  myDelay(1000);
  if (state){
    return;
  }
  digitalWrite(pin_Down, HIGH);
  
  digitalWrite(pin_Left, LOW);
  myDelay(1000);
  if (state){
    return;
  }
  digitalWrite(pin_Left, HIGH);
  
  digitalWrite(pin_Right, LOW);
  myDelay(1000);
  if (state){
    return;
  }
  digitalWrite(pin_Right, HIGH);
}
« Last Edit: December 29, 2012, 03:42:59 pm by twistedsymphony » Logged

Global Moderator
Melbourne, Australia
Offline Offline
Brattain Member
*****
Karma: 506
Posts: 19141
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Code:
void runSeq(){
  unsigned long seq = 0;
  Serial.begin(9600);

Do Serial.begin in setup, not every time through loop.
Logged

http://www.gammon.com.au/electronics

Please post technical questions on the forum - not to me by personal message. Thanks a lot.

Offline Offline
Newbie
*
Karma: 0
Posts: 15
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

moved... though that doesn't seem to change the results.
Logged

Global Moderator
Melbourne, Australia
Offline Offline
Brattain Member
*****
Karma: 506
Posts: 19141
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Is this right? ...

Code:
    seq = pgm_read_dword(&(steps[i])); //reads the steps from the included file

Anyway, I wouldn't bother putting such a small array into PROGMEM.
Logged

http://www.gammon.com.au/electronics

Please post technical questions on the forum - not to me by personal message. Thanks a lot.

Offline Offline
Newbie
*
Karma: 0
Posts: 15
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

the only reason it's small is because I'm testing it out, the final version will have hundreds of elements in the array.
Logged

Global Moderator
Melbourne, Australia
Offline Offline
Brattain Member
*****
Karma: 506
Posts: 19141
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

What do your debugging displays tell you? (perhaps paste them here).
Logged

http://www.gammon.com.au/electronics

Please post technical questions on the forum - not to me by personal message. Thanks a lot.

Pages: [1] 2   Go Up
Jump to: