can someone double check my code?

Projects Programming
1

I have been makeing this project but i have difficulties, and cant find the problem
pins 11, and 12 are always on after the start they should only be on when pressing the coresponding button, or at the end if that color won.
its a game like oddball of halo ball changes hands team to team gaining points for each as they hold their teams button. 1st to rack up all the points wins.
the point counting works, but not the led (to let you know what button is being pressed) redPin, greenPin; or the white led that lights up if your pressing a team button, and you are behind in the score.
score set low to 10 for testing.

  Name: CrazySkull ( "oddball" type game)
  
  Description: 
 
 The circuit:
 * 
 * 
 * 
 
 Created AUG 2012
 By: Brad Nelson
 -Mainstream Productions-
 
 */

// Constants:
// Outputs:
const int redPin = 12;   //red led with resistor
const int greenPin = 11;  // green led and resitor
const int whitePin = 10; //white led resitor to 3v
const int sirenPin = 7; // set to pin 7 (off) pin 8 (on)
const int brightPin = 9; //relay to 1W led

// Inputs:

const int redButton = 5;  //10k pulldown
const int greenButton = 6; // 10k pulldown
const int startSensor = 4; //10k pulldown

// Variables:
// Read: (has something physicaly attached)
int redState = LOW;
int greenState = LOW;
// Made: (has nothing physicaly attached)
int pointValue = 10; // setable point value for game end (in arbitrary units)
int greenCount = 0;
int redCount = 0;



void setup() {
  // initialize the pins outputs:
  pinMode(redPin, OUTPUT);
  pinMode(greenPin, OUTPUT);
  pinMode(whitePin, OUTPUT);
  pinMode(sirenPin, OUTPUT);
  pinMode(brightPin, OUTPUT);
  
  // initialize the pins inputs:
  pinMode(redButton, INPUT);
  pinMode(greenButton, INPUT);
  pinMode(startSensor, INPUT);
  digitalWrite(redPin, HIGH);
  digitalWrite(greenPin, HIGH);
  digitalWrite(whitePin, HIGH);
  digitalWrite(brightPin, HIGH);
  digitalWrite(sirenPin, HIGH);
  delay(500);
  digitalWrite(sirenPin, LOW);
  delay(500);
  digitalWrite(sirenPin, HIGH);
  delay(500);
  digitalWrite(sirenPin, LOW);
  delay(500);
  digitalWrite(sirenPin, HIGH);
  delay(500);
  digitalWrite(sirenPin, LOW);
  digitalWrite(redPin, LOW);
  digitalWrite(greenPin, LOW);
  digitalWrite(whitePin, LOW);
}

void loop(){
  //
  greenState = digitalRead(greenButton);
  redState = digitalRead(redButton);
  
  
  if(redState == HIGH, greenState == LOW)
  {
    redTeam();                  // run redTeam
  }
  if(greenState == HIGH, redState == LOW)
  {
    greenTeam();                // run greenTeam
  }  
  if(pointValue <= redCount)
  {
    win();  
  }
  if(pointValue <= greenCount)
  {
    win();  
  }
}
//000000000000000000000000000000000000000
void redTeam(){
  redState = digitalRead(redButton);
  digitalWrite(redPin, HIGH);
  digitalWrite(greenPin, LOW);
  if(redState == HIGH)
  {
    redCount++;
    delay(500);
    if(redCount < greenCount)
      {
        digitalWrite(whitePin, HIGH);
      }
    if(redCount >= greenCount)
      {
        digitalWrite(whitePin, LOW);
      }   
  }
}  
void greenTeam(){
  greenState = digitalRead(greenButton);
  digitalWrite(greenPin, HIGH);
  digitalWrite(redPin, LOW);
  if(greenState == HIGH)
  {
    greenCount++;
    delay(500);
    if(greenCount < redCount)
      {
        digitalWrite(whitePin, HIGH);
      }
    if(greenCount >= redCount)
      {
        digitalWrite(whitePin, LOW);
      }   
  }
}  
  void win(){
   digitalWrite(brightPin, LOW);
   digitalWrite(whitePin, HIGH);
   if(redCount >= greenCount)
     {
       digitalWrite(redPin, HIGH);
       digitalWrite(greenPin, LOW);
     }
   digitalWrite(whitePin, HIGH);
   if(greenCount >= redCount)
     {
       digitalWrite(greenPin, HIGH);
       digitalWrite(redPin, LOW);
     }
   delay(500);
   digitalWrite(sirenPin,HIGH);
   digitalWrite(brightPin, HIGH);
   digitalWrite(whitePin, LOW);
   delay(500);
   digitalWrite(sirenPin,LOW);
   digitalWrite(brightPin, LOW);
   digitalWrite(whitePin, HIGH);
   delay(1000);
   digitalWrite(sirenPin,HIGH);
   digitalWrite(brightPin, HIGH);
   digitalWrite(whitePin, LOW);
   delay(500);
   digitalWrite(sirenPin,LOW);
   digitalWrite(brightPin, LOW);
   digitalWrite(whitePin, HIGH);
   delay(1000);
   digitalWrite(sirenPin,HIGH);
   digitalWrite(brightPin, HIGH);
   digitalWrite(whitePin, LOW);
   delay(500);
   digitalWrite(sirenPin,LOW);
   digitalWrite(brightPin, LOW);
   digitalWrite(whitePin, HIGH);
   delay(1000);
   digitalWrite(sirenPin,HIGH);
   digitalWrite(brightPin, HIGH);
   digitalWrite(whitePin, LOW);
   delay(2000);
   digitalWrite(sirenPin,LOW);
   digitalWrite(brightPin, LOW);
   digitalWrite(whitePin, HIGH);
  }
2

The compiler will probably complain about the missing /* at the start.

Your "if" conditions with commas may compile, but will not do what you want in most cases.

If "redCount" isn't less than "greenCount", it isn't necessary to test again to see if it is greater than or equal, a si please "else" will suffice.

3 
  if(redState == HIGH, greenState == LOW)

What do you think this is doing? It isn't, but I'm curious what you were trying to accomplish?

Perhaps you need to look at the && operator, and quit trying to invent shortcuts. The compiler certainly isn't going to understand them.

4

it has on i forgot to copy it i guess, and it compiles fine.
it is supposed to make sure it wont call the function if both buttons are pressed.
im not familiar with && ill look it up,but that part has been working.

5

but that part has been working.

OK. Define "working"

6

im not familiar with && ill look it up,but that part has been working.

But, you are complaining that the code isn't working. I guess we must be applying different meanings to the term "working".

How are your switches wired? It's so much simpler to use the internal pullup resistor, and wire one side of the switch to ground and the other side to the pin. LOW means pressed, then.

7

they are wired 5v to pin, and pin to ground with 10k resistor.
Working here means that the buttons read, and count, and cannont be pressed at the same time. as designed.
what is not working is the lights on pins 11, 12;
redPin and greenPin. they are always on.
i have had people check my circit for shorts etc. but we have found nothing. so i moved here after i couldnt (besideds convention/ comonalities) find anything opperationaly wrong(not as designed) with my code.
changeing to the && now.
and yes it would be better to use else. ill change that.

done

void loop(){
  //
  greenState = digitalRead(greenButton);
  redState = digitalRead(redButton);
  
  
  if(redState == HIGH && greenState == LOW)
  {
    redTeam();                  // run redTeam
  }
  if(greenState == HIGH && redState == LOW)
  {
    greenTeam();                // run greenTeam
  }  
  if(pointValue <= redCount || pointValue <= greenCount)
  {
    win();  
  }
}
//000000000000000000000000000000000000000
void redTeam(){
  redState = digitalRead(redButton);
  digitalWrite(redPin, HIGH);
  digitalWrite(greenPin, LOW);
  if(redState == HIGH)
  {
    redCount++;
    delay(500);
    if(redCount < greenCount)
      {
        digitalWrite(whitePin, HIGH);
      }
    else
      {
        digitalWrite(whitePin, LOW);
      }   
  }
}  
void greenTeam(){
  greenState = digitalRead(greenButton);
  digitalWrite(greenPin, HIGH);
  digitalWrite(redPin, LOW);
  if(greenState == HIGH)
  {
    greenCount++;
    delay(500);
    if(greenCount < redCount)
      {
        digitalWrite(whitePin, HIGH);
      }
    else
      {
        digitalWrite(whitePin, LOW);
      }   
  }
}

got some || or's in there as well

8

What's missing now is a call to Serial.begin() in setup() and calls to Serial.print() in loop(), to show when you are changing the state of the pins. If you write out that you are setting the pin low, but the light doesn't go off, then there is a hardware problem. If the statement that you are writing the pin low never appears in the serial monitor, then you have a software problem (or a hardware problem). But, lets figure out which is which, first.

9

i just ran this:

void setup() {
  // initialize the pins outputs:
  pinMode(redPin, OUTPUT);
  pinMode(greenPin, OUTPUT);
  pinMode(whitePin, OUTPUT);
  pinMode(sirenPin, OUTPUT);
  pinMode(brightPin, OUTPUT);
  
  // initialize the pins inputs:
  pinMode(redButton, INPUT);
  pinMode(greenButton, INPUT);
  pinMode(startSensor, INPUT);
  
  
}
void loop(){
  
  digitalWrite(redPin, HIGH);
  delay(1000);
  digitalWrite(redPin, LOW);
  digitalWrite(greenPin, HIGH);
  delay(1000);
  digitalWrite(greenPin, LOW);
  digitalWrite(whitePin, HIGH);
  delay(1000);
  digitalWrite(whitePin, LOW);
  
}

and all the led's checked out with the hardware so i ran my edited code with the "else &&, and ||" with simplified hardware(not installed onboard leds), and that checked out.
so ill test it with the more complicated set of hardware(installed in skull, external led's) but i expect it to work.
i dont get how the && could change that its in an entirely different part of the code but that may hove been the problem?

10

thebfs:
it is supposed to make sure it wont call the function if both buttons are pressed.

It does not achieve that.

11

yes it does it will only call if the states are different.
show me what im missing please if u belive different.

12

In your code in the first post you set reg and green HIGH aka ON in setup. Is this your your problem?.

Mark

13

How, exactly, do you play this game?