need help fixing code

Projects Programming
1

im not a coder, but I managed to modify some code i found online to work with my arduino micro. I have a video game controller made up completely of buttons. the purpose of the code is so that when opposite directions are input simultaneously, the arduino will output which ever was input last, so if i hold down the left button, and then simultaneously hold down the right button, the output will change from left to right. the code was originally written for left/right only, but I added up/down. it actually works really well for the intended purpose, but the unintended consequence is that I can no longer go diagonal, so for example if im pressing left, and then down, it will only go left. Id really appreciate any help or a fix. like i said im not a coder, and it is pretty challenging for me to follow.

int up_input = 2;
int down_input = 3;
int up_output = 11;
int down_output = 12;



int left_input = 0;   //RX
int right_input = 1;  //TX
int left_output = 8;
int right_output = 9;

 
volatile boolean b = false;
volatile boolean c = false;
 
void setup()
{
 
pinMode(up_input, INPUT);
pinMode(down_input, INPUT);
pinMode(up_output, OUTPUT);
pinMode(down_output, OUTPUT);

pinMode(left_input, INPUT);
pinMode(right_input, INPUT);
pinMode(left_output, OUTPUT);
pinMode(right_output, OUTPUT);
 
attachInterrupt(1, up, CHANGE);
attachInterrupt(0, down, CHANGE);

attachInterrupt(2, left, CHANGE);
attachInterrupt(3, right, CHANGE);
}
 
void up(){
b = true;
}
 
void down(){
b= true;
}
 
void left(){
c= true;
}

void right(){
c= true;
}
 
void loop()
{
 
digitalWrite(up_input, HIGH);
digitalWrite(down_input, HIGH);
digitalWrite(up_output, HIGH);
digitalWrite(down_output, HIGH);
b=false;

digitalWrite(left_input, HIGH);
digitalWrite(right_input, HIGH);
digitalWrite(left_output, HIGH);
digitalWrite(right_output, HIGH);
c=false;
 
detachInterrupt(1);
while(digitalRead(up_input)==LOW && b==false)
{
digitalWrite(up_output, LOW);
}
attachInterrupt(1, up, CHANGE);
 
digitalWrite(up_input, HIGH);
digitalWrite(down_input, HIGH);
digitalWrite(up_output, HIGH);
digitalWrite(down_output, HIGH);
b=false;
 
detachInterrupt(0);
while(digitalRead(down_input)==LOW && b==false)
{
digitalWrite(down_output, LOW);
}
attachInterrupt(0, down, CHANGE);


detachInterrupt(2);
while(digitalRead(left_input)==LOW && c==false)
{
digitalWrite(left_output, LOW);
}
attachInterrupt(2, left, CHANGE);
 
digitalWrite(left_input, HIGH);
digitalWrite(right_input, HIGH);
digitalWrite(left_output, HIGH);
digitalWrite(right_output, HIGH);
c=false;
 
detachInterrupt(3);
while(digitalRead(right_input)==LOW && c==false)
{
digitalWrite(right_output, LOW);
}
attachInterrupt(3, right, CHANGE);
}
2

1)Dont use pins 0 an 1, they are for serial, I guess you can, just not recommended. You are using them for interrupts, so nvm.
2)Once you use digitalWrite() the output stays as what you set it, you dont need to continuously call it. You don't need those while loops.
3)Same goes with attachInterrupt, call it once, and you dont need to call it again.
4)Don't use int if you only need a byte.
5)Why are you digitalWrite()'ing to an input?

Just do something like this

...

void setup(){
  ...
}

void loop(){
  //nothing
}

//you probably want to add some digitalWrite()'s in here too...
void up(){
  if(digitalRead(upInput) == LOW){
    upIsPressed = true; //up is being pressed
    if(downIsPressed == true)
      downIsPressed = false; //up was most recent press, disable down press
  }
  else {
    upIsPressed = false; //up was released
    if(digitalRead(downInput) == LOW)
      downIsPressed = true; //down is still being pressed, so we need to reset it to true
  }
}

void down(){
  if(digitalRead(downInput) == LOW){
    downIsPressed = true; //down is being pressed
    if(upIsPressed == true)
      upIsPressed = false; //down was most recent press, disable up press
  }
  else {
    downIsPressed = false; //down was released
    if(digitalRead(upInput) == LOW)
      upIsPressed = true; //up is still being pressed, so we need to reset it to true
  }
}

//do the same for left() and right()
3

Why are you even using interrupts?

4

Im guessing he wants to know which was the most recently pressed, and while you can do that without interrupts, it is easier to do it with them.

5

Ps991:
1)Dont use pins 0 an 1, they are for serial, I guess you can, just not recommended. You are using them for interrupts, so nvm.

Just in case anyone else is confused...

Interrupts 0 and 1 use pins 2 and 3

And your suggestion not to use pins 0 and 1 is generally good advice.

I overlooked the fact that the OP has a Micro. I had assumed he was using an Uno.

...R

6

Just in case anyone else is confused...

Interrupts 0 and 1 use pins 2 and 3

This is on a Micro with a 32u4

7

AWOL:
This is on a Micro with a 32u4

Thanks. I had missed that. I have edited my Post.

...R

8

wow, great community you guys have here. so much help so fast. I'll try your suggestions Ps991. like I said, I'm not a coder, and it will still be a struggle figuring out how to work in your code. I don't see where it actually writes to the outputs? I assume I still need to add that part. and of course setup part. Thanks everybody

9

Ive been looking at this and my head is spinning. with Ps991's code am i going to still use the interrupts? I cant figure out how and where to put in the digital writes. Do I really want the loop to contain nothing? Any help would be greatly appreciated. Thanks a lot

10

This is what I came up with. just going with up/down for now. Its very erratic as if up and down are both constantly outputting all the time. Any help would be appreciated, thanks.

int up_input = 2;
int down_input = 3;
int up_output = 11;
int down_output = 12;



int left_input = 0;   //RX
int right_input = 1;  //TX
int left_output = 8;
int right_output = 9;

volatile boolean upIsPressed = false;
volatile boolean downIsPressed = false;

void setup(){

pinMode(up_input, INPUT);
pinMode(down_input, INPUT);
pinMode(up_output, OUTPUT);
pinMode(down_output, OUTPUT);

pinMode(left_input, INPUT);
pinMode(right_input, INPUT);
pinMode(left_output, OUTPUT);
pinMode(right_output, OUTPUT);
 
attachInterrupt(1, up, CHANGE);
attachInterrupt(0, down, CHANGE);

attachInterrupt(2, left, CHANGE);
attachInterrupt(3, right, CHANGE);

}

void loop(){
  //nothing
}

//you probably want to add some digitalWrite()'s in here too...
void up(){
  if(digitalRead(up_input) == LOW){
    upIsPressed = true; //up is being pressed
    digitalWrite(up_output, LOW);
    if(downIsPressed == true)
      downIsPressed = false; //up was most recent press, disable down press
      digitalWrite(down_output, HIGH);
  }
  else {
    upIsPressed = false; //up was released
    digitalWrite(up_output, HIGH);
    if(digitalRead(down_input) == LOW)
      downIsPressed = true; //down is still being pressed, so we need to reset it to true
      digitalWrite(down_output, LOW);
  }
}

void down(){
  if(digitalRead(down_input) == LOW){
    downIsPressed = true; //down is being pressed
    digitalWrite(down_output, LOW);
    if(upIsPressed == true)
      upIsPressed = false; //down was most recent press, disable up press
      digitalWrite(up_output, HIGH);
  }
  else {
    downIsPressed = false; //down was released
    digitalWrite(down_output, HIGH);
    if(digitalRead(up_input) == LOW)
      upIsPressed = true; //up is still being pressed, so we need to reset it to true
      digitalWrite(up_output, LOW);
  }
}

//do the same for left() and right()

void left(){
// add later
}



void right(){
//add later
}
11 
    if(downIsPressed == true)
      downIsPressed = false; //up was most recent press, disable down press
      digitalWrite(down_output, HIGH);

You are missing the { and }. This happens 4 times in your code.

Make it this...

    if(downIsPressed == true){
      downIsPressed = false; //up was most recent press, disable down press
      digitalWrite(down_output, HIGH);
     }

And yes, you are still using the interrupts. And you were correct to add the digitalWrite()'s in there.

12

I haven't tried the corrected code yet, but just for kicks, on my previous mostly working code I changed the "while"s to "if"s and it created erratic behavior when I held down up/down together. ( in certain games I intend to hold down down, while tapping the up button, same for left/right)

13

That's not what you said here

so if i hold down the left button, and then simultaneously hold down the right button, the output will change from left to right

If you want that new functionality, then it would be simpler, just don't set the other button false.

There is another thing you should look up when you are done. Look up Debouncing
Debouncing is when you press a button and at the very moment it makes contact, sometimes it will appear as if the button got pressed many times in a very short amount of time. Here is an accurate picture

14

there is no contradiction. when both buttons are pressed simultaneously, whichever button was pressed last is the one that controls the output. so when the second button is pressed, it seems to me that the first has to be set to false, right?

15

It is normal for button pins to be "low" when pressed, because of the pull-up resistor circuitry.

However, it is less normal for output pins to be "low" when activated. Are you sure you want that ?

16

fluffhead, tell me EXACTLY how you want it to output in all situations.

For example, pressing this outputs this. Lets try it with left and right.
LEFT = output high for left
RIGHT = output high for right
HOLDING RIGHT, PRESSING LEFT = output high for right, output high for left if left is pressed
HOLDING LEFT, PRESSING RIGHT = output high for left, output high for right if right is pressed

If this is what you want, then it is much simpler because left() and right() do not need to interfere with each other.