Passing Boolean Function Question

Using Arduino Programming Questions
1

I have a program this is checking the status of a boolean function in a while loop. I do have it working by assigning the boolean function to a boolean variable first and then putting the variable in the While loop. But I would have thought putting the boolean function directly into the while loop would have worked correctly. In the example code below, The first three lines work fine, but I would think I could eliminate the FanRunStatus variable and use the 4th line only.

Is that just the way it is or is there something better I can do?

Thanks
John

  bool FanRunStatus
  FanRunStatus = FanStatus();                     // Fan Running Status
  while(FanRunStatus)                             // Run the fan if appropriate
  while(FanStatus());

bool FanStatus()
{
bool EngineRunStatus;
bool RecentlyRunning;

EngineRunStatus = EngineStatus();
RecentlyRunning = AfterRunStatus();
    
  Serial.print("EngineStatus = "); Serial.print(EngineRunStatus); Serial.println();
  Serial.print("RecentlyRunningStatus = "); Serial.print(RecentlyRunning); Serial.println();
 
  if (EngineRunStatus || RecentlyRunning)
  {
    Serial.println("FanStatus() Complete as true");
    Serial.println();
    return true;  
  }
  if (EngineRunStatus == false &&  RecentlyRunning == false)
  {
    Serial.println("FanStatus() Complete as false");
    Serial.println();    
    return false;
  }

}
///// End of FanStatus() ////////////////////////////////////////////////////////////////
2

I will look carefully at something that compiles, or should, and runs and either does or does not do the right thing.

a7

3

Get rid of the trailing ;. It should look like this:

  while (FanStatus()) {
    // Your code here
  }
4

Or not. :wink:

a7

5

Definitely a mistake but not the problem. It still doesen't work

6

If you don't get a new value for FanRunStatus inside the while loop then this one becomes an infinite loop.

If that doesn't work as a while loop (assuming you remove the offending semicolon) then you have more mistake that you aren't showing.

Post an actual piece of code, not a paraphrased snippet. The mistake may not be where you think it is.

7

Then you're obviously doing something wrong. Unfortunately, you haven't posted a complete code, and I have absolutely no intention of guessing or playing 20 Questions with you.

8

Yes, you should use only the 4th line for the reason @Delta_G explained to you, the variable is never updated, the function is.

9

@Delta_G @MaximoEsfuerzo @gfvalvo @alto777

Well guys I owe you all an apology. It works as you say and I'm sorry to have wasted your time but I really have no idea what went wrong on several occasions previously. It couldn't have been the semi colon because it wouldn't have compiled. And for whatever reason it works now.

The program is 1100 lines long and very messy. In fact eliminating these extra variables is part of cleaning it up. So I made a shortened version to demonstrate the problem it showed that there was no problem. I can only suspect I had a different function being called in the while loop than the FanStatus(), and I do have several that look similar.

But what ever you got me out of a question that stumped me.

Thanks
John

void setup() 
{
  Serial.begin(115200);           
  while(!Serial);                        
  Serial.println("************** SetUp() Complete ***********"); Serial.println(); Serial.println();
}

void loop() 
{
  bool FanRunStatus     = FanStatus();                    // Fan Running Status
  
  //FanRunStatus = FanStatus();                     // Fan Running Status
  Serial.println("Begining off loop print outs ******************************************");
  Serial.print("FanRunStatus That Works = "); Serial.println(FanRunStatus);
  Serial.print("FanRunStatus That I thought dident Work = "); Serial.println(FanStatus());Serial.println();Serial.println();  


  while(FanRunStatus)                             // This works
  //while(FanStatus())                            // And This Works
  {
    Serial.print(" ***********In While Loop Fuction pass - Succesful "); Serial.println(); Serial.println();
    delay(5000);
  }
  Serial.print(" ***********Did Not get into While Loop - Un-Succesful "); Serial.println(); Serial.println();
  delay(5000);
}

bool FanStatus()
{
  //Function printouts and external fuction calls commented out for demo
  
  bool EngineRunStatus;
  bool RecentlyRunning;

  //EngineRunStatus = EngineStatus();
  //RecentlyRunning = AfterRunStatus();

  EngineRunStatus = true;               // Hard set to true so no need for demo
  RecentlyRunning = true;               // Hard set to true so no need for demo
    
  //Serial.print("EngineStatus = "); Serial.print(EngineRunStatus); Serial.println();
  //Serial.print("RecentlyRunningStatus = "); Serial.print(RecentlyRunning); Serial.println();
 
  if (EngineRunStatus || RecentlyRunning)
  {
    //Serial.println("FanStatus() Complete as true");
    //Serial.println();
    return true;  
  }
  if (EngineRunStatus == false &&  RecentlyRunning == false)
  {
    //Serial.println("FanStatus() Complete as false");
    //Serial.println();    
    return false;
  }
}
///// End of FanStatus() ////////////////////////////////////////////////////////////////type or paste code here
10

Yes, it probably was, and it would compile. This is perfectly valid C++ code, it just didn't do what you wanted.

  while(FanStatus());
11

@gfvalvo

Interesting results. It does compile but goes the wrong thing.

Serial output stops before entry into the while loop. See serial output below.

So I guess that is what happened, but its kind of discouraging as I miss semicolens all the time and without the compiler to pick it up I can anticipate trouble. Hopefully this was a good enough learning experience for me to remember to triple check my while loops.

Thanks
John

21:59:14.484 -> ************** SetUp() Complete ***********
21:59:14.484 ->
21:59:14.484 ->
21:59:14.484 -> Begining off loop print outs ******************************************
21:59:14.484 -> FanRunStatus That Works = 1
21:59:14.484 -> FanStatus() That I thought dident Work = 1
21:59:14.484 ->
21:59:14.484 ->

If I take the comma away the serial output adds a new line every 5 sec.

22:07:09.773 -> ************** SetUp() Complete ***********
22:07:09.773 ->
22:07:09.773 ->
22:07:09.773 -> Begining off loop print outs ******************************************
22:07:09.773 -> FanRunStatus That Works = 1
22:07:09.773 -> FanStatus() That I thought dident Work = 1
22:07:09.773 ->
22:07:09.773 ->
22:07:09.773 -> ***********In While Loop Fuction pass - Succesful
22:07:09.773 ->
22:07:14.747 -> ***********In While Loop Fuction pass - Succesful
22:07:14.747 ->
22:07:19.755 -> ***********In While Loop Fuction pass - Succesful
22:07:19.755 ->
22:07:24.762 -> ***********In While Loop Fuction pass - Succesful
22:07:24.762 ->
22:07:29.769 -> ***********In While Loop Fuction pass - Succesful
22:07:29.769 ->
22:07:34.778 -> ***********In While Loop Fuction pass - Succesful
22:07:34.778 ->

12 
bool FanStatus()
{
bool EngineRunStatus;
bool RecentlyRunning;

EngineRunStatus = EngineStatus();
RecentlyRunning = AfterRunStatus();
    
  Serial.print("EngineStatus = "); Serial.print(EngineRunStatus); Serial.println();
  Serial.print("RecentlyRunningStatus = "); Serial.print(RecentlyRunning); Serial.println();
 
  if (EngineRunStatus || RecentlyRunning)
  {
    Serial.println("FanStatus() Complete as true");
    Serial.println();
    return true;  
  }
  if (EngineRunStatus == false &&  RecentlyRunning == false)
  {
    Serial.println("FanStatus() Complete as false");
    Serial.println();    
    return false;
  }

}
///// End of FanStatus() ////////////////////////////////////////////////////////////////

This function should always return true or false. Good news: it does...
But it took me a while to check all cases (4 in total).
Better would be:

bool FanStatus()
{
bool EngineRunStatus;
bool RecentlyRunning;

EngineRunStatus = EngineStatus();
RecentlyRunning = AfterRunStatus();
    
  Serial.print("EngineStatus = "); Serial.print(EngineRunStatus); Serial.println();
  Serial.print("RecentlyRunningStatus = "); Serial.print(RecentlyRunning); Serial.println();
 
  if (EngineRunStatus || RecentlyRunning)
  {
    Serial.println("FanStatus() Complete as true");
    Serial.println();
    return true;  
  }
  Serial.println("FanStatus() Complete as false");
  Serial.println();    
  return false;
}
///// End of FanStatus() ////////////////////////////////////////////////////////////////

Now it is immediately clear that the function will always return true or false (no possibility that you missed a case).
It also saves unneeded checks (=code and runtime).

And even shorter:

bool FanStatus()
{
  Serial.print("EngineStatus = "); Serial.print(EngineRunStatus); Serial.println();
  Serial.print("RecentlyRunningStatus = "); Serial.print(RecentlyRunning); Serial.println();
 
  if (EngineStatus() || AfterRunStatus())
  {
    Serial.println("FanStatus() Complete as true");
    Serial.println();
    return true;  
  }
  Serial.println("FanStatus() Complete as false");
  Serial.println();    
  return false;
}
///// End of FanStatus() ////////////////////////////////////////////////////////////////
13

@build_1971 Exactly. Somewhere along the line I went down the path of assigning functions to variables because I had a problem. This whole thread started as I was trying to get rid of all those variables to clean up the code. As far as the extra if statements, they help me think about conditions I don't want to see and can be removed once I'm comfortable with the code.

14

I have no idea what you mean by this. Do you perhaps mean that you wrote functions that returned values that returned values that were then assigned to variables ? That would make more sense

15

NP. With as little of it as there is left (!), many of us choose to spend it here.

I was going to tell you to go to the IDE preferences and turn up the verbosity and warnings. I checked and it still would not complain or warn about this

  while (FanStatus());
    // stuff if FanStatus goes false seems to always get execute

Empty bodies are no problem, nothing to worry about, no warning. This is the C/C++ gun and its notorious ability to shoot you the foot. So be careful.

My tests did, however, warn about a real problem that you may have just been lucky enough that it didn't hurt:

warning: control reaches end of non-void function [-Wreturn-type]

A function that promises to return a value should, or that value will just be whatever junk happens to be on the stack when the function returns.

This is why putting any return in the middle of a function is not advised, even though it is seen more than never. In any case, take care to be sure that all ways out of a function execute a return statement as the last step. It shouldn't be too hard to do that with there being only one return, at the bottom, and the result will be more readable code.

You can use

  return;

for functions returning void. Lotsa ppl don't bother.

a7