Command in "if" statement executed without condition being met

I'm a beginner Arduino user, and I'm writing a program to modify the output of a motor for a robot. In one of my functions, I have the following if statement:

if(output>155){

Serial.print("Flag 1\n");
output=155;

}

When I run the code, the print statement will be executed only when output>155. However, the command "output=155" appears to be executed for each iteration the code runs, regardless of whether the condition "output>155" is met. Any suggestions?

brownsugar:
Any suggestions?

Yes. Post all your code. You've either copied this wrong or you have an error elsewhere in your code.

Thank you for your reply. Below is the complete code. The specific “if” statement occurs in the “forward” function.

const int trigPinFront = 31;    //subscript "Front" refers to US sensor on front of car
const int echoPinFront = 30;

const int trigPinRight = 41;    //subscript "Right" refers to US sensor on right side of car
const int echoPinRight = 40;

const int trigPinLeft=51;
const int echoPinLeft=50;

const int AIABack = 10;  // (pwm) Pins for back wheels
const int AIBBack = 11;  // 
const int BIABack = 6;  // 
const int BIBBack = 3;  // 
const int buttonPin=26;

int PIDflag=0;

long durationFront, cmFront, inchesFront; 
long durationRight, cmRight, inchesRight;
long durationLeft, cmLeft, inchesLeft;
void forward(int inchesRight, int inchesLeft, int inchesFront);

int enable = -1;
byte speed = 155;  // change this (0-255) to control the speed of the motorsin 
byte minimumspeed=100;

int buttonState=0;

void setup() {
 
 Serial.begin(9600);
 
 pinMode(trigPinFront, OUTPUT);  
 pinMode(echoPinFront, INPUT);
 pinMode(trigPinRight, OUTPUT);  
 pinMode(echoPinRight, INPUT);  
 pinMode(trigPinLeft, OUTPUT);  
 pinMode(echoPinLeft, INPUT);
   
 pinMode(AIABack, OUTPUT); 
 pinMode(AIBBack, OUTPUT);
 pinMode(BIABack, OUTPUT);
 pinMode(BIBBack, OUTPUT);

 pinMode(buttonPin, INPUT);
}



void loop() {


buttonState=digitalRead(buttonPin);
 
 cmFront, inchesFront = usSensorFront();
 cmRight, inchesRight = usSensorRight();
 cmLeft,  inchesLeft  = usSensorLeft();

 if(buttonState == HIGH){
   delay(500);
   //forward(inchesRight, inchesLeft, inchesFront);
   if(enable == 1){
     enable = -1;
     Serial.print("Enable = -1\n");
   }
   else if(enable == -1){
     enable = 1;
     Serial.print("Enable = 1\n");
   }
  }

 if(enable == 1){
   forward(inchesRight, inchesLeft, inchesFront);
 }

 if(enable == -1){
   pause();
 }

 //forward(inchesRight, inchesLeft, inchesFront);   
 //delay(500);
 //backward();
   
}




/////////////////////////////////////////////////////////////////////////////////////////////
// The following functions were written to activate forward, backward, left, and right motion

void pause()

{
 analogWrite(AIABack, 255);
 analogWrite(AIBBack, 255);
 analogWrite(BIABack, 255);
 analogWrite(BIBBack, 255);
}

void backward()

{
 analogWrite(AIABack, speed);
 analogWrite(AIBBack, 0);
 analogWrite(BIABack, 0);
 analogWrite(BIBBack, speed);
}



void forward(int inchesRight, int inchesLeft, int inchesFront)
{ 
 int rightdistance=12;      // Inches
 int leftdistance=12;       // Inches

 byte speedRight = round(speed*inchesLeft/leftdistance);
 byte speedLeft = round(speed*inchesRight/rightdistance);

 byte output;
 double error, errorSum, errorDir, errorLast;
 double kp, ki, kd, ABS;
 unsigned long lastTime, now; 
 double timeChange;
 

 kp=1;
 ki=0.001;
 kd=1;

 now = millis();
 
 if(PIDflag=1){
   
   timeChange = now-lastTime;

   error = inchesRight - inchesLeft;

   errorSum  = (error+errorLast)*timeChange;
   errorDir  = (error - errorLast)/timeChange;

   output = round(kp*error + ki*errorSum + kd*errorDir);

   if(output<0){
     output=-output;
   }
   
   
   Serial.print("Ouput0: ");
   Serial.print(output);
   Serial.println();    

   if(output>155){
     Serial.print("Flag 1\n");
     output=155;   
     }
   Serial.print("Ouput1: ");
   Serial.print(output);
   Serial.println();

   if(output<100){
     output=100;
   }
     
   if(error<0){
     speedLeft=output;
     //Serial.print("Turn left\n");
   }
   if(error>=0){

     speedRight=output;
     //Serial.print("Turn right\n");
   }

   /*
   Serial.print("Error: ");
   Serial.print(error);
   Serial.print("\n");
   Serial.print("errorSum: ");
   Serial.print(errorSum);
   Serial.print("\n");
   Serial.print("errorDir: ");
   Serial.print(errorDir);
   Serial.print("\n");*/
   Serial.print("Output2: ");
   Serial.print(output);
   Serial.println();
 }


 analogWrite(AIABack, 0);
 analogWrite(AIBBack, speedRight);
 analogWrite(BIABack, speedLeft);
 analogWrite(BIBBack, 0);

 errorLast=error;
 lastTime=now;
 PIDflag=1;  
}

void left()

{

 analogWrite(AIABack, 0);
 analogWrite(AIBBack, speed);
 analogWrite(BIABack, 0);
 analogWrite(BIBBack, speed);
 
}



void right()

{

 analogWrite(AIABack, speed);
 analogWrite(AIBBack, 0);
 analogWrite(BIABack, speed);
 analogWrite(BIBBack, 0);
}


/////////////////////////////////////////////////////////////////////////////////////////////
// Function for reading front ultrasonic sensor
long usSensorFront()  {
 // The sensor is triggered by a HIGH pulse of 10 or more microseconds.
 // Give a short LOW pulse beforehand to ensure a clean HIGH pulse:
 digitalWrite(trigPinFront, LOW);
 delayMicroseconds(5);
 digitalWrite(trigPinFront, HIGH);
 delayMicroseconds(10);
 digitalWrite(trigPinFront, LOW);

 // Read the signal from the sensor: a HIGH pulse whose
 // duration is the time (in microseconds) from the sending
 // of the ping to the reception of its echo off of an object.
 pinMode(echoPinFront, INPUT);
 durationFront = pulseIn(echoPinFront, HIGH);

 // convert the time into a distance
 cmFront = (durationFront/2) / 29.1;
 inchesFront = (durationFront/2) / 74; 

 return cmFront, inchesFront;
} 

/////////////////////////////////////////////////////////////////////////////////////////////
// Function for reading right ultrasonic sensor
long usSensorRight()  {
 // The sensor is triggered by a HIGH pulse of 10 or more microseconds.
 // Give a short LOW pulse beforehand to ensure a clean HIGH pulse:
 digitalWrite(trigPinRight, LOW);
 delayMicroseconds(5);
 digitalWrite(trigPinRight, HIGH);
 delayMicroseconds(10);
 digitalWrite(trigPinRight, LOW);

 // Read the signal from the sensor: a HIGH pulse whose
 // duration is the time (in microseconds) from the sending
 // of the ping to the reception of its echo off of an object.
 pinMode(echoPinRight, INPUT);
 durationRight = pulseIn(echoPinRight, HIGH);

 // convert the time into a distance
 cmRight = (durationRight/2) / 29.1;
 inchesRight = (durationRight/2) / 74; 

 
 return cmRight, inchesRight;
} 

/////////////////////////////////////////////////////////////////////////////////////////////
// Function for reading left ultrasonic sensor
long usSensorLeft()  {
 // The sensor is triggered by a HIGH pulse of 10 or more microseconds.
 // Give a short LOW pulse beforehand to ensure a clean HIGH pulse:
 digitalWrite(trigPinLeft, LOW);
 delayMicroseconds(5);
 digitalWrite(trigPinLeft, HIGH);
 delayMicroseconds(10);
 digitalWrite(trigPinLeft, LOW);

 // Read the signal from the sensor: a HIGH pulse whose
 // duration is the time (in microseconds) from the sending
 // of the ping to the reception of its echo off of an object.
 pinMode(echoPinLeft, INPUT);
 durationLeft = pulseIn(echoPinLeft, HIGH);

 // convert the time into a distance
 cmLeft = (durationLeft/2) / 29.1;
 inchesLeft = (durationLeft/2) / 74; 
 
 return cmLeft, inchesLeft;
}

Read here and pay particular attention to point 7.

I promise that if statement is not where output is being set to 155. You must have an issue with your PID algorithm. That's a properly formed if statement. Try printing output right after it gets set by the pid code and see what it is there.

Actually, you've got a print in there right before and after that line. Can you show the output that makes you think it's changing in there?

If output is type byte, how can it be less than 0?

if(output<0){
output=-output;
}

Yes, here's a sample of the output from first two iterations:

(first iteration)
Ouput0: 147
Ouput1: 155

(second iteration)
Ouput0: 161
Flag 1
Ouput1: 155

This demonstrates what I was talking about when I said that the "Flag 1" print command executes when the "if" statement conditions are met, but the output value changes regardless.

    if(PIDflag=1){

Probably not what you intended.

Yeah, here's another error. It doesn't work like this, but this isn't the one we're chasing.

cmFront, inchesFront = usSensorFront();
  cmRight, inchesRight = usSensorRight();
  cmLeft,  inchesLeft  = usSensorLeft();

Delta_G:
Yeah, here's another error. It doesn't work like this, but this isn't the one we're chasing.

cmFront, inchesFront = usSensorFront();

cmRight, inchesRight = usSensorRight();
  cmLeft,  inchesLeft  = usSensorLeft();

Those cm values aren't even used anywhere. Just drop them.
Returning the same result in two different units does not make any sense anyway.

Damned if I can see it. That shouldn't be happening as far as I can tell.

OP
I am curious, did you write this code.

Most of it I wrote, other parts I got from the internet. The entire "forward" function was written by me.

strange

ive no idea why its doing it and after playing with it for a few minutes I found that the

if (output >=155) {
output=155;
}

is not being executed yet the output does change to 155

if I // out the output=155; then the output does not change

if I added

output=0; before the if then it worked as you would expect so I presumed that output was being compiled wrong (way above my pay level)

so I changed

byte output=0; and that didn’t work

then I tried

static byte output=0; and that worked correctly which indicates the compiler is doing it for a reason I just don’t know why

anyway change

byte output; to static byte output=0; or move byte output=0; to global and try it and it should work (in my tests it did)

hopefully my gammon will see this post.

Hes explained on a few posts how the compiler sees a piece of code and decides that the "if" or what ever will always be true then it just scrubs that from the code. So I presume that the compiler thinks that the output at this point will always be more than 155 so it just scrubbed the if and said output = 155.

then again I may be wrong as normal

You might be able to test that by making output volatile instead of static. Compiler won't try to optimize the if out if output is volatile.