Two opposing if statements conditions met at the same time

I am writing some simple code to assess whether a sensor is outputting a signal correctly or not. The sensor outputs a PWM signal and should output a pulse every 700 milliseconds +/- 30% (i.e between 580 & 906). If the period between pulses is within the 580 - 906 limits, a green LED should turn on and a yellow LED should go off. If the period is out with these limits a red LED should go on and the yellow LED should go off.

The code calculates the period between two pulses correctly using the first section of code (I'm getting 725 milliseconds returned on the serial monitor). However, for some reason the "if" statements that decide which while loop to go into do not work, as both conditions are met simultaneously, i.e both sensorFailed and sensorPassed become 100.

I don't understand how both of these conditions can be met at the same time especially when I know that the period is being calculated accurately. Can anyone see a bug in my code, that I cannot see??

/////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
/////////////////////////////////////////THIS CODE TESTS THE SENSORS FOR 2 FAILURE TYPES, CONSTANT HIGH VOLTAGE AND CONSTANT LOW VOLTAGE (I.E CONSTANT 0V OR CONSTANT 3V)///////////////////////////////////////////////////////////////////////////////////////
//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

int highTime;
unsigned long startTimer = 0;
int startTest = 0;
unsigned long timer = 0;

int arrayIndex = 0;

int highTimes[10];
static byte sensorPassed = 0;
static byte sensorFailed = 0;

int sensorSignal = 0;

void setup() {  


  Serial.begin(115200);
  pinMode(48, OUTPUT); //Broken sensor LED pin (Red)
  pinMode(52, OUTPUT); //Good sensor LED pin (Green)
  pinMode(51, OUTPUT); //Started testing pin (Yellow)



}

void loop() 
{  
  
  sensorSignal = analogRead(A0); //Read analog sensor output 
  digitalWrite(51, HIGH); //yellow LED on, to indicate that testing has started
  //delay(10000);

    
  /*Serial.print(sensorSignal);
  Serial.println();*/

  if(sensorSignal < 50)
  {
    sensorFailed = 100;
  }
    
 
     
//--------------------------------------------------------------------------THIS LOOP TESTS IF THE SENSOR IS CONSTANTLY OUTPUTTING A HIGH SIGNAL (I.E CONSTANT HIGH SIGNAL TEST)---------------------------------------------------------------------------------------------------------------------------
//---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------


     
       if(sensorSignal < 800 && startTest == 0)
       {
        startTest = 1;  //start test = 1 to start next if loop
       }

       if(sensorSignal > 800 && startTest == 1)
       {
         startTimer = millis(); 
         startTest = 2;   //start test = 2 to start next if loop
       }
       
       if(sensorSignal < 800 && startTest == 2)
       {
        startTest = 3;    //start test = 3 to start next if loop
       }

       if(sensorSignal > 800 && startTest == 3)
       {
        startTest = 0;
        timer = millis();
        highTime = timer - startTimer;  //time recorded at end of second pulse - end of second pulse 
        highTimes[arrayIndex] = sensorFailed; // !!!!!!USED TO COLLECT DATA, DOES NOT PERFORM ANY FUCNTION IN THE CODE!!!!!
        arrayIndex = arrayIndex + 1; // !!!!!!USED TO COLLECT DATA, DOES NOT PERFORM ANY FUCNTION IN THE CODE!!!!!
       }

   

        
        if (arrayIndex > 10)//MAX ARRAY SIZE = 1023 // !!!!!!USED TO COLLECT DATA, DOES NOT PERFORM ANY FUCNTION IN THE CODE!!!!!
          {
            for (int z = 0; z < 10; z = z + 1)
            {     
              Serial.println(highTimes[z]);
            }
            arrayIndex = 0;
          }                                         
          

       if(highTime > 580 && highTime < 906)  // assess whether period is within 700 milliseconds +/- 30% limits
       {
        sensorPassed = 100;
       }

      

       if (highTime < 580 || highTime > 906 && sensorPassed != 100)  // assess whether period is outwith 700 milliseconds +/- 30% limits
       {
        sensorFailed = 100;
       }

       while(sensorFailed = 100)
        {
            digitalWrite(52 ,HIGH); //turn red LED on to show the sensor is broken 
            digitalWrite(51, LOW); //turn yellow LED low to show testing has finished    
        }

       while(sensorPassed = 100)
        {
          digitalWrite(48 ,HIGH); //turn green LED on to show the sensor is good 
          digitalWrite(51, LOW); //turn yellow LED low to show testing has finished
        } 
        
       
      
        
        
}

Try

if( (sensorSignal >= 50) && (sensorSignal < 800+ && (startTest == 0))

There is nothing in the code preventing this state, so, when this happens, only the first while loop is executed and you are stuck there forever.

Why do you have 2 separate variables for passing and failing, and why do you make them equal to 100? One single, boolean variable is enough: make it true if sensor passes and false if sensor fails.

Then:

if (sensor_ok)
{
    // do this
}
else
{
    // do that
}

I have tried it using an if…else statement. I have also tried it using sensorPassed = 1 rather than 100. I’ve also used one variable which changes for passed/failed. I.e originally I had just set the startTest variable to 5 or 6 depending on whether it passed or failed.

All of these variations still presented the same issue where both conditions were met at the same time.

This section of the code works fine as it calculates the period correctly I.e it is always 700 milliseconds.

I started to rewrite this to make it clearer and then I got confused by what you are trying to do. Implementing a few "best practices" can be helpful.

  1. Remove all the "magic numbers" and replace them with definitions that make sense. e.g.
const float PULSE_PERIOD = 700;
const float PULSE_PERIOD_LOWER = PULSE_PERIOD * 0.7;
const float PULSE_PERIOD_UPPER = PULSE_PERIOD * 1.3;
  1. Use "else if" on the statements that are complementary so it's immediately obvious which ones can execute.
  2. what do you expect this if (highTime < 580 || highTime > 906 && sensorPassed != 100) to do? It might not do what you expect. Use parentheses to make your logic clearer and don't rely on operator precedence.

Clear code is far, far easier to debug. It's worth taking the time to do it right.

In reply to section 3, I want it to check if the value is above or below the 700 +/- 30% and check that the “sensorPassed” has not already been changed from 0 -> 100

In reply to the remaining points, I will take that on board for next time. Sorry if it wasn’t clear enough.

So, this is what you meant?

if ( ((highTime < 580) || (highTime > 906)) && (sensorPassed != 100))
{
}

Yes.

Consider that here you are storing 100 in sensorFailed and then the while loop is executed because the condition evaluates to 100 (an assignment statement returns whatever value is stored in the variable being assigned). As a condition, Zero is considered false, everything else is considered true.

Try this, to see how it works:

int a = 100;
void setup() {
  Serial.begin(115200);
  Serial.println("\nTesting integer values as conditions");
  Serial.print("The value of a is " ); Serial.println(a);

  if (a = 5) {
    Serial.println("a=5");
  }
  else {
    Serial.println("a is not 5");
  }

  Serial.print("The value of a is now " ); Serial.println(a);

  if (3) {
    Serial.println("3 is TRUE");
  } else {
    Serial.println("3 is FALSE");
  }

  if (0) {
    Serial.println("0 is TRUE");
  } else {
    Serial.println("0 is FALSE");
  }

  Serial.println("Printing integers from -2 to 2:");
  for (int i = -2; i < 3; i++ ) {
    if (i) { // this evaluates to false when i is 0, true otherwise
      Serial.println(i);
    }
  }

  while (a = 10) { // this will change the value of a and evaluate to 10
    Serial.println("while loop was executed with (a=10)");
    break;
  }
  Serial.print("The value of a is now " ); Serial.println(a);

  while (a = 0) { // this will change the value of a and evaluate to 0
    Serial.println("while loop was executed with (a=0)");
    break;
  }
  Serial.print("The value of a is now " ); Serial.println(a);

}

void loop() {

}

Instead, use

while (sensorFailed == 100)

The same applies to the other while.

Incidentally, when there is nothing inside the loop that changes the value of a while condition, once it enters the loop it never exits (I used break in the exampe above to, well, break the loop). Your while loops then execute either 0 times, or ad infinitum.

Talking about loops, I noticed you refer to if instructions as loops. They aren't.

And my question is, how do you know both sensorPassed and sensorFailed become 100?

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.