loading

What is wrong with the program...?

I am trying to make a quizbowl(jepoardy) buzzer system, and I am using an arduino.  I have a program that does a few buttons, and I can add more buttons by just adding more if else, but when I clear it( pin0 and pin3) you have to wait about a second to buzz in, otherwise it doesn't register.  Just a note: the buttons are set up so that I can use 6 pins and control 9 buttons.  It is hard to explain but the pins are setup so that two of the pins sense the current to correspond to the button pressed.  I don't know what else to add, I think it is that is doesn't register that the clear button is no longer pressed, but that might not be it.  Here is the program:
const int pin0 = 14;
const int pin1 = 15;
const int pin2 = 16;
const int pin3 = 17;
const int pin4 = 18;
const int pin5 = 19;
const int led1 = 13;
const int led2 = 12;
const int led3 = 11;

int pin0s = 0;
int pin1s = 0;
int pin2s = 0;
int pin3s = 0;
int pin4s = 0;
int pin5s = 0;

void setup()
{
pinMode(pin0, INPUT);
pinMode(pin1, INPUT);
pinMode(pin2, INPUT);
pinMode(pin3, INPUT);
pinMode(pin4, INPUT);
pinMode(pin5, INPUT);
pinMode(led1, OUTPUT);
pinMode(led2, OUTPUT);
pinMode(led3, OUTPUT);
}

void loop ()
{
  pin0s = digitalRead(pin0);
  pin1s = digitalRead(pin1);
  pin2s = digitalRead(pin2);
  pin3s = digitalRead(pin3);
  pin4s = digitalRead(pin4);
  pin5s = digitalRead(pin5);
 
  if (pin1s == HIGH && pin3s == HIGH)
  {
    digitalWrite(led1, HIGH);
    while(pin0s == LOW || pin3s == LOW)
    {
      pin0s = digitalRead(pin0);
      pin3s = digitalRead(pin3);
    }
    digitalWrite(led1,LOW);
    while(pin0s == HIGH || pin3s == HIGH)
    {
      pin0s = digitalRead(pin0);
      pin1s = digitalRead(pin1);
      pin2s = digitalRead(pin2);
      pin3s = digitalRead(pin3);
      pin4s = digitalRead(pin4);
      pin5s = digitalRead(pin5);
    }
      pin0s = digitalRead(pin0);
  pin1s = digitalRead(pin1);
  pin2s = digitalRead(pin2);
  pin3s = digitalRead(pin3);
  pin4s = digitalRead(pin4);
  pin5s = digitalRead(pin5);
 
  }
 else if(pin3s == HIGH && pin2s == HIGH)
 {
    digitalWrite(led2, HIGH);
    while(pin0s == LOW || pin3s == LOW)
    {
      pin0s = digitalRead(pin0);
      pin3s = digitalRead(pin3);
    }
    digitalWrite(led2,LOW);
    while(pin0s == HIGH || pin3s == HIGH)
    {
      pin0s = digitalRead(pin0);
      pin1s = digitalRead(pin1);
      pin2s = digitalRead(pin2);
      pin3s = digitalRead(pin3);
      pin4s = digitalRead(pin4);
      pin5s = digitalRead(pin5);
    }
      pin0s = digitalRead(pin0);
  pin1s = digitalRead(pin1);
  pin2s = digitalRead(pin2);
  pin3s = digitalRead(pin3);
  pin4s = digitalRead(pin4);
  pin5s = digitalRead(pin5);
}

else if (pin0s == HIGH && pin4s == HIGH)
{
    digitalWrite(led3, HIGH);
    while(pin0s == LOW || pin3s == LOW)
    {
      pin0s = digitalRead(pin0);
      pin3s = digitalRead(pin3);
    }
    digitalWrite(led3,LOW);
    while(pin0s == HIGH || pin3s == HIGH)
    {
      pin0s = digitalRead(pin0);
      pin1s = digitalRead(pin1);
      pin2s = digitalRead(pin2);
      pin3s = digitalRead(pin3);
      pin4s = digitalRead(pin4);
      pin5s = digitalRead(pin5);
    }
      pin0s = digitalRead(pin0);
  pin1s = digitalRead(pin1);
  pin2s = digitalRead(pin2);
  pin3s = digitalRead(pin3);
  pin4s = digitalRead(pin4);
  pin5s = digitalRead(pin5);
}}


Please help!
Thanks in Advance!


sort by: active | newest | oldest
frollard7 years ago
wow...comment your code, and name your variables something meaningful!

I've sat here for 10 minutes trying to comprehend whats going on...

My understanding (of what it should do, not what it does do):
6 buttons 3 leds

When 'ready' is pressed then you loop waiting for a button
if a button is pressed, kick out of the loop and return the value of the button pressed, and do not accept any other buttons until reset is pressed again.

loop.



Recommendations:
Name the pins and variables something meaningful like 'ResetButton' or 'player1button' etc.  Name the status leds 'ReadyLED', etc.

https://www.instructables.com/answers/How-do-I-use-a-wait-until-command-in-the-ardui/  (someone doing the same as you)

Another addition:  It is possible if you poll the buttons all within one loop that multiple ones will be hit at exactly the same microsecond (rare but possible), in which case you may wish to randomize which player gets the buzz.

Lastly: how do you output which player got the buzz?
JaredsProjects (author)  frollard7 years ago
I was determining which player got the buzz by seeing which two analog inputs are true at the same time.  The reason why two of the inputs have to be true, is that I needed the digital pins for leds and a piezo, so I had to use the 6 analog pins for 9 buttons.  So the switches are setup so that the 5 volts connects to two of the pins and the ground(that is connected by resistor).  There are diodes in place to prevent from false readings.  The reason why I named the variables pin1, pin2... was because no single pin was a single button.  Sorry that I didn't add any comments, I will try to start adding comments.  Also it only controls the 3 player and the reset button now, because it wasn't clearing quickly, so I decided that I should ask and find out what was wrong with the program before finishing. 

Thank you for you help!
Running out of pins is not uncommon - but instead of using so many pins to run one led each; try running a matrix, or a shift register (cheap and easy).  3 pins plus shift register(s) can make 8++ outputs using included reference code.

Even the inputs can be matrixed with diodes if you really want to reduce pin usage.  (think how a keyboard works, a hundred keys with only 10-20 pins, same thing)

Totally agreed with what seandogue says below:

make procedures to make the main loop simple, and have the procedures do the hard work.

1. setup
2. clear everything
3. Wait for a 'ready' button
4. while <check for buttons> keep looping checking the buttons
5. evaluate who pressed the button
6. light up their led, and start a timer
7. wait for reset button or timer to expire,
8. listen for 'score' buttons, if you want, or a reset button
9 goto 4

another method for checking the buttons in a 'fair' manner that ensures everyone has the same odds if they happen to hit at the same time:
8 'buttons' are hooked to output pins, each through a button, then all tie to one common input.

winner = 0
i = 1
while winner = 0

   digitalwrite pin(i) = high
  ' delay(1)  'optional, might need to wait for the pin to sense the output going       high
   if digitalread inputpin = high then
     winner = i
         end if
   digitalwrite pin(i) = low
i = i+1
if i = 9 then i=1
end while loop

as in, keep checking the input pin while pinging each player, hundreds of times per second.  This way, it randomizes who wins in the event of 2 players hitting at exactly the same time.  Still not 100% fair, but balanced, player 1 has an equal advantage/disadvantage ratio with 3 people more likely to be picked and 3 people less likely to be picked.
seandogue7 years ago
well for starters, it's way too linear.

The program doesn't take advantage of the concept of subroutines to reduce its footprint, and it doesn't appear to have tried to minimize the polling routine. I gave up trying to follow it pretty quickly, aside from realizing that it was too difficult to follow (not spending 10 minutes to figure what it's supposed to be doing)

As frollard said, there are no comments. Bad form. Programs should always be commented.

As frollard said, the variable names are less than helpful in figuring out what's going on.

Why are you doing const int pin(x) = something ?

I'll freely admit I don't work with the arduino, but it appears that you're trying to redfine the pin or lock it to a value. this makes little sense to me.

Further, polling of the various inputs can be done in a more efficient manner by using a while or for loop.

pseudo code

for i = 0 to 5 {
digitalRead(i)
}


this makes for clean code

in fact, you can go one step further and do a define statement

#define (check_inputs) {
for i = 0 to 5 {
digitalRead(i)
            }
}

then just call check_inputs from the program to see what's been pressed


sorry for any weirdness in the text formatting...this embedded editor the Instructables Progamming staff included really sucks rocks bigtime
I wanted to post a bunch of pseudocode to explain how to proceduralize the program to be less linear....Great suggestion.
btw, nice new pic. Is that you and your wife?
not sure how I missed this message...

Yes; thats our wedding day, May of this (2009) year.
with the accidental cropping the instructables does, it makes a nice icon. Looks a bit like an old dutch painting from the 18th century.
If you look at my wife's dress, you can see the shadow of my uncle holding his slr (bald head with arm coming up and around holding lens from above :D)
Lol...so you can...I didn't notice that at first...

just wanted to share this quik rendition of a detail on your pic as a painting so you see what I mean..Renoir-eque and all...
A little pointillism here or there and it looks good!  Shame that ibles compresses jpg so far that everything look renoir :D

I have a sepia tone conversion of this pic printed for my mother - its significantly more cool-looking with all the branches essentially in grayscale.
I just did a quick hack using Paint dot net using a single pass with the "oil painting" effect after a resize and dpi increase .. I haven't gotten Photoshop re-installed since my last system crash...too much other work and not enough time or it really would have looked like a real painting...just did a sepia transform. tone...yes..it does look rather "Period".
Re: up the resolution; the original is something like 20 megapixel...not so good for an avatar :D
 Lol...no...not the best size for uploading to instructables. I usually try to keep my uploads to <100K when I think about it
...oh, its because its only 6 minutes old :D
Yeah, thing is, AI's posting the same routines over and over again in that code. I don't think it's necessary, and it does little more than make it hard to read and debug, with what I evaluate to be too much redundancy in operations.

The while loops are fine though for the polling, although imo, there are far too many polls taking place per iteration.