Boolean not set when used as last line in an IF statement?

Projects Programming
1

Hi all,

I see the following weird thing:

If I use a boolean to be set as last action inside an IF statement, it will not be set, but when I put something behind it, like in my example the NightRunActiveIndicator = true; it WILL work?!?

The variables are all declared in the portion of the code before setup() and loop(), but after some #includes .

This is just a very small part of the code used inside loop()

So like this it will work, NightRunActiveIndicator WILL be set:

// Begin of Check if we need to start the Nightly Run  
  if ( ( NightStartTime != 0 ) && ( millis() >  NightStartTime)  && ( AccDone == true ) ) {
        AccDone = false;
        Load = ( MaxLoad - FireAdjustHigh) ;
        
        NightRunActive = true;
        NightRunActiveIndicator = true;
        
        lcd.setCursor(0, 1);
        lcd.print("Nightly Run!!       ");
        lcd.write(1);  
}
// End of Check if we need to start the Nightly Run

And like this it will NOT work, NightRunActiveIndicator will NOT be set:

// Begin of Check if we need to start the Nightly Run  
  if ( ( NightStartTime != 0 ) && ( millis() >  NightStartTime)  && ( AccDone == true ) ) { 
        AccDone = false;
        Load = ( MaxLoad - FireAdjustHigh) ;
        
        lcd.setCursor(0, 1);
        lcd.print("Nightly Run!!       ");
        lcd.write(1); 
        
        NightRunActive = true;
        NightRunActiveIndicator = true; 
      
}
// End of Check if we need to start the Nightly Run

So changing the order of these 5 lines of code makes all the difference?

I use an Arduino Pro Mini and Arduino IDE 1.8.13

Is this a known issue or do I something wrong in my code?

Saludos,

Leo

2

Please post your complete, compilable sketch, or a minimal version of it which exhibits the issue. Add it as an attachment if it is large.

3

Everybody always wants to blame the compiler but in my 35 years of software development I can count on less than one hand that it actually is.

Please post your entire sketch. Chances are there is something else wrong.

4

Hi all,

In short what it this is:

I have a wireless display that shows the result of our Grid Tied solarpanels. As soon as there is Surplus (more solar than our own consumption) it would go into the grid.

Arrange for payment of your surplus energy here in Spain is really difficult, so I build an Arduino controlled dimmer, that will control an electric waterheater in such a way, that all the surplus energy will go into the waterheater. It also keeps track of how much energy went in there. It will do a nightrun to make sure all of the water will be heated properly to avoid luke warm water and legionella desease.

The site would not let me put the entire code here (over 9000 characters) so I added the complete code in the attachment below.
If there is a better way of showing you the code, please let me know.

In another part of the code I used another way of dealing with the declaration of the boolean at the end of an IF-statement, look for "int wfs=0" , there I used a little For-Next loop to set it.
But again, that way again it is not the last statement at the end of the IF....

Saludos,

Leo

Leo_Kroonenburg_Dimmer_Code.txt (93.6 KB)

5

Below the warnings when I compile your code; please set Compiler Warning to all under File -> Preferences. Your symptoms (So changing the order of these 5 lines of code makes all the difference?) are a clear indication of memory problems and the below warnings are an indication of that.
```
*C:\Users\sterretje\AppData\Local\Temp\arduino_modified_sketch_939144\sketch_oct06a.ino: In function 'void Create_Xmit_String()':

C:\Users\sterretje\AppData\Local\Temp\arduino_modified_sketch_939144\sketch_oct06a.ino:1105:30: warning: null argument where non-null required (argument 2) [-Wnonnull]

strcat(controlmessage, '\0');                                        // add terminator character ??

^

Archiving built core (caching) in: C:\Users\sterretje\AppData\Local\Temp\arduino_cache_732344\core\core_arduino_avr_uno_340ee7b99d1d6851600b80336d8bbfb5.a
C:\Users\sterretje\AppData\Local\Arduino15\packages\arduino\hardware\avr\1.6.21\cores\arduino\main.cpp: In function 'main':

C:\Users\sterretje\AppData\Local\Temp\arduino_modified_sketch_939144\sketch_oct06a.ino:955:47: warning: iteration 43 invokes undefined behavior [-Waggressive-loop-optimizations]

lastcontrolmessage[i] = controlmessage[i];

^

C:\Users\sterretje\AppData\Local\Temp\arduino_modified_sketch_939144\sketch_oct06a.ino:954:5: note: containing loop

for (int i = 0 ; i <= MesLen; i++ )    {                              // copy the string to laststring in V8.11 changed length of copy to i <= MesLen, so copy all 43 positions instead of only 42

^

Sketch uses 22502 bytes (69%) of program storage space. Maximum is 32256 bytes.
Global variables use 1437 bytes (70%) of dynamic memory, leaving 611 bytes for local variables. Maximum is 2048 bytes.
_*_ _*Further you might want to get rid of String objects (capital S); I don't use them but this e.g. looks odd*_ _**_
*String mystring(tempc, 1) ;

String(mystring).toCharArray(DateTime, 5);                              // Convert temperature (4 position) to an char-array[] called DateTime !! must be 5 long, because of 21.3 + Temination Character
  if ( String(mystring).length() == 3 )  strcat(controlmessage, "0");  // If the array has a length of 3, the temperature must be < 9.9, so we add one 'zero' first*
```
mystring is a String, in the next line you create a new String with the same content.

6

@Satbeginner - if what you claim is true then it should be possible to write a simple minimal sketch to prove it.

I believe that the title of this thread should actually be "Boolean not set when used as the last line in an IF statement in my program"

7

Hi @sterretje and @UKHeliBob,

Your suggestions clearly show I am a newbie, in my case Compiler Warnings was still set to "None", and although the program 'works' it is now clear to me I have work to do.

I never got a proper training in 'C' programming, I did put everything together by finding loads of examples for different types of problems online.

When I now compile (with Compiler Warnings set to All) I too have a long list of messages, mostly about the string 'controlmessage'.
If that string is written beyond it's end, it might and or will affect other parts of the program.

In the coming days I will try to get rid of the compiler warnings in my program and clean up the "String mystring(tempc, 1) ;" stuff and try to reproduce the thing in a minimal program.

Please let us put my question on hold until I have done both.

Many thanks for the suggestions, I will let you know my progress in a few days.

Saludos,

Leo

8

Hi @sterretje and @UKHeliBob,

my first basic test shows that booleans as last statement inside an IF statement do work as they should work.

I made this simple program:

boolean WaitForSolar = false; 

void setup() {
  Serial.begin(9600); 
  Serial.println ("Begin program");  
}

void loop() {

    if (WaitForSolar == false ){
        Serial.print ( "WaitForSolar before 1st change is: "),Serial.println (WaitForSolar);
        WaitForSolar = true;
    }

    
delay(1000);
Serial.print ( "WaitForSolar is now: ");Serial.println (WaitForSolar);
delay(1000);


    if (WaitForSolar == true ){
        Serial.print ( "WaitForSolar before change back is now: "),Serial.println (WaitForSolar);
        WaitForSolar = false;
    }

delay(1000);   
Serial.print ( "WaitForSolar is now: ");Serial.println (WaitForSolar);    
delay(1000);    
}

This proves the compiler does work, so even more reasons to check my own program.

I will let you know about that later.

Thanks again,

Leo

9

Hi @sterretje and @UKHeliBob,

Most if not all of the errors regarding the handling of the controlmessage were related to trying to copy the entire string, including the termination character.
The string has a useful length of 42 characters, but I was copying the termination character on position 43 as well.

Also I used strcat to build the string and also put the termination character on position 43, now I use strcat up to position 42, and use controlmessage[43] = '\0';

In the attached file the updated code that compiles without errors now.
I changed back the 'tricks' regarding the booleans at the end of the IF-statement.
A quick test using Serial output shows they now DO are set :slight_smile:

I still use the String (capital S), I will see if I can do thing differently as well.

I will test this version for functionality tomorrow and let you know.

Again, thanks,

Leo

Leo_Kroonenburg_Dimmer_Code_No_Compiler_errors_anymore.txt (95.8 KB)

10

Use of Strings leads to memory problems and random program crashes, because of poor memory management.

They are almost never necessary, so it is best to get rid of them entirely.

11

Yep I always recommend to never use Arduino Strings...You can use char arrays instead :smiley:

12

Hi all,

after the last modification to get rid of the compiler warnings I tested the code in real life.
It mostly worked, but again, one boolean was not set... after the start of the NightRun the NightRunActiveIndicator stayed false.

I now put setting both booleans in a separate routine to test this.
Again, in the attachment the changed, complete code with the call to the added SetNightRunFlags() routine.

The funny thing is, that all other booleans do work, but this one still was set as last action in an IF-statement...

The result of this test will be known tomorrow morning, I will let you know.

Saludos,

Leo

P.S. The 'Strings' change I will do later
@TheUNOGuy, btw, just read your piece on Strings.. I now realize I must change my code quickly to et rid of them :slight_smile:

Leo Kroonenburg changed name of boolean - no compiler warnings and without use of Strings .txt (94.7 KB)

13

Hi all,

maybe as expected, all tricks changing the position of the line where the NightRunActiveIndicator was set did not help, at all.
So, to exclude the mentioned 'Strings' mess-up I now changed the program so all use of Strings (Capital S) is removed.
Attached again the code, again with no Compiler Warnings, and no use of Strings.

Since the use of that one boolean only shows in the morning, I can only report back tomorrow morning :slight_smile:

Thanks for the tips and suggestions,

Leo

Leo Kroonenburg no compiler warnings and without use of Strings .txt (94.3 KB)

14

Hi all,

so the story continues.....

It still does not work!?

The boolean NightRunActiveIndicator stays false, whatever I try?
Not even at the end of an IF-statement anymore, but it stays false always.

This boolean is set in the same routine as the boolean NightRunActive, but that one DOES change from false to true and back.

I looked at the scope of these booleans, it is all the same, they are all global.

So to exclude also the weird things, I now changed the name of NightRunActiveIndicator into NRAI.
(maybe too long?? maybe the Compiler mixes up these names because they begin with the same letters? ....?)

I installed the software and will report back tomorrow...

Again the complete code is in the attachment.

Saludos,

Leo

Leo Kroonenburg changed name of boolean - no compiler warnings and without use of Strings .txt (94.7 KB)

15

I know that it is commented out but lines like this

//    lcd.print("W"),lcd.print(WaitForSolar),lcd.print("G"),lcd.print(GetSolar),lcd.print("C"),lcd.print(DoCalculationIndicator),lcd.print("A"),lcd.print(AccDoneIndicator),lcd.print("N"),lcd.print(NRAI);

make the code almost impossible to read

If the line were not commented out what do you think that it would do, noting that the commands are separated by commas ?

maybe too long?? maybe the Compiler mixes up these names because they begin with the same letters? ....?

Neither of these is true

16

UKHeliBob:
I know that it is commented out but lines like this

//    lcd.print("W"),lcd.print(WaitForSolar),lcd.print("G"),lcd.print(GetSolar),lcd.print("C"),lcd.print(DoCalculationIndicator),lcd.print("A"),lcd.print(AccDoneIndicator),lcd.print("N"),lcd.print(NRAI);

make the code almost impossible to read

If the line were not commented out what do you think that it would do, noting that the commands are separated by commas ?
Neither of these is true

Hi, thanks for you reply.

Since I am still learning, I am never sure the lines of code I change actually do work, that is why I sometimes leave my original line in there, to be cleaned out later.

The line with all the print-statements will execute all individual prints sequentially to the LCD, like eg. W1G0C0A0N0, but was changed into something to print either the '1' if a boolean is true, and a "." when false.

About my original question/problem:
Among others, I use these two booleans, ActiveNightRun and ActiveNightRunIndicator.

The first one I use to see if the state of the program should execute the NightRun part of the code, the Indicator boolean I use to keep showing it actually DID execute the NightRun, also after the NightRun.

But.... That NightRunActiveIndicator always is showing 'false'.

First I thought is was because it was the last one set before the end of an IF-statement, hence the title of my post.

There was another boolean showing the same behaviour, WaitForSolar. I first tried to put that in a small 4x look to set it 4 times in a row, that worked for that boolean.

Later, I found it did not matter where I would set the boolean NightRunActiveIndicator, it still would be false.

Other suggestion was it might be messed up by the use of Strings (Capital S), so I changed that code too.

I looked if I made mistakes in the scope of these booleans, but saw nothing weird.

Now the WaitForSolar boolean DOES work, normal, as it should be, but the NightRunActiveIndicator still does not.
That's why I changed the name of this boolean, because I don't know what else to try.

Saludos,

Leo

17

I sometimes leave my original line in there, to be cleaned out later.

It is not the fact that the line is there. I understand that and do the same myself, it is more that you have put so many commands on the same line and if you do that you need to be sure that you know what using a comma between program statements does

18

I can barely find the code in amongst all those comments; someone was a tad overzealous. The problem with over-commenting code is that, assuming the comments were correct in the first place, over time with many edits the comments invariably diverge from the code and become a liability.

char tbs[3];
...
  sprintf(tbs, "%03d", Difference);                         // fill tbs with three positions of Difference, while adding leading zero's if necesary

I suspect this is why NightRunActiveIndicator is overwritten. The tbs buffer is only 3 bytes long, but the sketch is writing at least 4 bytes to it (including the terminating null). BTW, %03d is a minimum field width - worth remembering.
The cautious approach would be to expand this buffer to 7 bytes, but 4 might be enough.

19

arduarn:
I can barely find the code in amongst all those comments; someone was a tad overzealous. The problem with over-commenting code is that, assuming the comments were correct in the first place, over time with many edits the comments invariably diverge from the code and become a liability.

char tbs[3];

...
 sprintf(tbs, "%03d", Difference);                         // fill tbs with three positions of Difference, while adding leading zero's if necesary



I suspect this is why *NightRunActiveIndicator* is overwritten. The *tbs* buffer is only 3 bytes long, but the sketch is writing *at least* 4 bytes to it (including the terminating null). BTW, *%03d* is a minimum field width - worth remembering. 
The cautious approach would be to expand this buffer to 7 bytes, but 4 might be enough.

Hello @Arduarn,

Yeah, I know, more comments than code :-), but I never learned to be a programmer, so I do not instantly recognize the meaning of lines of code when I read it, also not to forget why I used this code, that's why I put so many comments.

I see what you mean by this "char tbs[3];" and will change it in my code and put it to the test.
I think I missed that, especially while it did not cause any visible problems on the LCD.

Thank you for looking at my code, I will let you know. 8)
The "real life test" is during the night, so I can tell you tomorrow morning.

Saludos,

Leo

Leo Kroonenburg changed size of tbs - no compiler warnings and without use of Strings .txt (95.2 KB)

20

Is it feasible to run your code faster so that you can run a full test in an hour rather than overnight? I assume that if you're actually running this with the solar array connected up, you actually do need to wait for darkness. If so, I'd be tempted to write a simulator using another Arduino just to enable faster testing.

There are a number of things in your code that could be improved: any time you have variables like Average6, it is an indication that you should be using arrays, which I see you are using elsewhere in your code.

This:

  if ( DoCalculationIndicator   == true )

is more commonly written as:

  if ( DoCalculationIndicator )

Your way will work fine, but it's redundant and looks slightly odd.

Your loop function is huge and therefore hard to follow. Consider breaking some of it out into separate functions.

Having global variables with single character names is not good practice. Using i is especially risky because it is commonly used in for loops, so it you borrow some code from somewhere you may have caused yourself unnecessary pain.