next topic »

[on:]

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);
}

[Reply #1 on:]

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'.

[Reply #2 on:]

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.

[Reply #3 on:]

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.

[Reply #4 on:]

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

[Reply #5 on:]

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.

[Reply #6 on:]

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

[Reply #7 on:]

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);
}

[Reply #8 on:]

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().

[Reply #9 on:]

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);
}

[Reply #10 on:]

Code:

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

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

[Reply #11 on:]

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

[Reply #12 on:]

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.

[Reply #13 on:]

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

[Reply #14 on:]

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