next topic »

[on:]

In the following code, the while loop only runs once, then stops?

//declare variables
int TX_LED = 3;
int RX_LED = 4;
int LC_LED = 5;
int LC_IN = 0;
int AB_IN = 1;
int MB_IN = 3;
int AMP_RELAY = 6;
int loadcell;
int ampbatt;
int mainbatt;
int compread = -1;
int compCONNECT;
int count = 0;

void setup()
{
  Serial.begin(9600); //Begin arduino-computer serial communication at 9600baud.
 
  //declare pin modes
  pinMode(TX_LED, OUTPUT);
  pinMode(RX_LED, OUTPUT);
  pinMode(LC_LED, OUTPUT);
  pinMode(AMP_RELAY, OUTPUT);
 
  //turn on amp relay
  digitalWrite(AMP_RELAY, HIGH);
 
  //flash LEDs rapidly 3x
  while (count < 4)
  {
    TX_FLASH(10);
    RX_FLASH(10);
    LC_FLASH(10);
    delay(200);
    count = count + 1;
  }
 
  while (compCONNECT = 0);
  {
    Serial.println("12345");
    TX_FLASH(100);
    delay(1000);
    compread = Serial.read();
   
    if (compread > 48)
    {
     compCONNECT = 1;
     RX_FLASH(100);
    }
    else
    {
      compCONNECT = 0;
    }
  }
}

void loop()
{
}

int TX_FLASH(int time)
{
  digitalWrite(TX_LED, HIGH); //turn on TX_LED
  delay(time); //wait TIME
  digitalWrite(TX_LED, LOW); //turn off TX_LED
}

int RX_FLASH(int time)
{
  digitalWrite(RX_LED, HIGH); //turn on RX_LED
  delay(time); //wait TIME
  digitalWrite(RX_LED, LOW); //turn off RX_LED
}

int LC_FLASH(int time)
{
  digitalWrite(LC_LED, HIGH); //turn on LC_LED
  delay(time); //wait TIME
  digitalWrite(LC_LED, LOW); //turn off LC_LED
}

[Reply #1 on:]

First, use CODE tags when you post and refer to the sticky that you should have read.

Second, which while loop? You have more than 1...

Lastly:

Code:

while (compCONNECT = 0);

if not the proper way to compare two values. The comparison operator is ==. You are using the assignment operator.

[Reply #2 on:]

Also, the semi-colon after the while statement is what the while loop "sees" as the loop body. remove it.

[Reply #3 on:]

Also, the semi-colon after the while statement is what the while loop "sees" as the loop body. remove it.

Good catch.

A couple of improvements:

Code:

int TX_FLASH(int time)
{
  digitalWrite(TX_LED, HIGH); //turn on TX_LED
  delay(time); //wait TIME
  digitalWrite(TX_LED, LOW); //turn off TX_LED
}

int RX_FLASH(int time)
{
  digitalWrite(RX_LED, HIGH); //turn on RX_LED
  delay(time); //wait TIME
  digitalWrite(RX_LED, LOW); //turn off RX_LED
}

int LC_FLASH(int time)
{
  digitalWrite(LC_LED, HIGH); //turn on LC_LED
  delay(time); //wait TIME
  digitalWrite(LC_LED, LOW); //turn off LC_LED
}

Can be reduced to:

Code:

void FLASH_SOMETHING(int led, unisgned long time)
{
  digitalWrite(led, HIGH);
  delay(time); //wait TIME
  digitalWrite(led, LOW);
}

 

Code:

if (compread > 48)

I gather you aren't sending it 48, you're sending it '1', '2', '3', etc.? Functionally it's the same, but this is more clear as to your intentions:

 

Code:

if (compread > '0')

Code:

count = count + 1;

The language is called C++. Feel free to use such operator:

Code:

count++;

[Reply #4 on:]

Can be reduced to:

Which the compiler will complain about, since it is missing a return type.

[Reply #5 on:]

Can be reduced to:

Which the compiler will complain about, since it is missing a return type.

No it's not...

[Reply #6 on:]

Thanks for the responses.  Was very pressed for time and totally forgot about code snippets   Also, very new to C++, very used to VB.net, so I was used to:

Code:

count = count + 1

  Will go through and change that.  Arrch, thanks for bring those to my attention.  Its works now