Odd compiler behavior, skipping parts of code [solved]

I’m attempting to write a library for the CrystalFontz CFA533 display. The display is a CFA533-TMI-KC. I wrote one several years ago and I’m rewriting it to behave more as a drop-in replacement for the standard liquid crystal display library. The issue I’m having is with the compiler apparently optimizing away critical portions of code. I’m including excerpts of my code in the hope that someone might see what’s wrong. I am planning on putting the library on github, but haven’t done so yet.

The demo code brightens the LCD and dims the keypad and then reverses it. That works fine. So, at the end of the first ‘i’ loop the LCD is still dark. The second ‘i’ loop sets both keypad and LCD to the same value and should end with them both at full brightness. It works fine as long as I use Serial.print in my library to print one of the arguments. If I comment out the Serial.print line then the second ‘i’ loop does not change the brightness on the display. I can serial print comments in the second ‘i’ loop and prove that the loop is running, but it doesn’t affect the display.

My testing is being done with an Arduino Mega2560 and the sketch is only using 2% of program space and 10% of dynamic memory (Mostly printing feedback on the serial port).

I’m stumped.

The library prototype.

void setBacklight(uint8_t backlight, uint8_t kp_backlight = 101);

This is a section of the library where I’m having issues.

void CFA533::setBacklight(uint8_t backlight, uint8_t kp_backlight)
{
 if(kp_backlight==101)
 kp_backlight = backlight;
 if(backlight > 100)
 backlight = 100;
 if(kp_backlight > 100)
 kp_backlight = 100;
 _dataOut.data[0] = backlight;
 _dataOut.data[1] = kp_backlight;
 _dataOut.command = CFA533_BACKLIGHT;
 _dataOut.dataLength = 2;
 
 Serial.print(backlight); // Does not work properly if this is commented out!
 
 send_verify_return_packet(_dataOut, DELAY_50mSec);
}

This is a portion of the demo code that is having issues.

for(int i = 0; i < 3; i++)
  {
    for(int j = 0; j < 101; j += 4)
    {
      lcd2.setBacklight(j, 100 - j);
    }
    for(int j = 0; j < 101; j += 4)
    {
      lcd2.setBacklight(100 - j, j);
    }

  }
  
    for(int i = 0; i < 101; i += 4)
    {
      lcd2.setBacklight(i,i);
      delay(250);
    }

Sounds like memory issue. I can see that you’re using arrays; any chance that you’re writing outside the boundary of an array (note that an array with 10 elements has indices 0…9). Be aware that the problem does not necessarily has to be in the code that you showed, it can be anywhere.

Use of String (capital S) anywhere in your code (specificaly concatenation) can be an other culprit.

Compiler can optimize code away, if they figure out values are not used. This can happen in loops which write to the same memory location. Compiler do not know about peripherals unless they are told. When you use Serial.print function the compiler figures out it needs to write all backlight values in the loop because they end up in a peripheral and not just local memory.
Maybe this is not true for the send_verify_return_packet function.

Maybe the use of serial.print is altering timings - giving the loop a li. Try replacing it with a delay(10) and see if that's what's making things work ok. I mean - your loops don't have any timings in them, so the backlight will just go from 0 to 100 pretty much instantly anyway.

Other problem might be methods clobbering memory that doesn't belong to them, but we'd have to see the declaration of _dataOut to verify that.

There are various other possible problems that we don't really have enough context do diagnose. For instance: you might be attempting to do this inside an interrupt.

I see the second loop does have a delay. 1/4 second, 25 steps, six seconds total. Is that what you were aiming for? Maybe it's working just fine, but you are not expecting it to take that long and you're getting impatient with it?

Appreciate the feedback. In answer to some of the replies.

I'm not using strings, other than literals. Relatively sure my arrays aren't being overwritten. The code will run all day without crashing, just the unexpected behavior that is very consistent when it manifests.

I've also tried substituting a delay for the Serial.print line and it does not make it work correctly. The delay in the second loop was added to try to make sure that I wasn't missing seeing something. The current demo code is just for me to test functions as I complete them in the library, and so it's not pretty.

I'm afraid the compiler seems to be optimizing some parts of my code away. I have another project at work that I've had similar issues with. Simple code seems to behave fine, the problem seems to only show in more complex projects.

I've posted the code on github. Complete library along with the current state of the example.

I think you're suffering from an overly aggressive optimizer that is keying off the end condition of your loops (ftree-scev-cprop). Try designating your variables as "volatile", or wrapping your code like this.

I downloaded your files and compiled it with compiler all warnings (File -> Preferences -> Settings) and get the following warnings:

..\CFA533demo.ino: In function 'void loop()':

..\CFA533demo.ino:73:21: warning: variable 'backlight' set but not used [-Wunused-but-set-variable]

  uint8_t contrast, backlight, kp_backlight;

                    ^~~~~~~~~
..\CFA533demo.ino:73:32: warning: variable 'kp_backlight' set but not used [-Wunused-but-set-variable]

  uint8_t contrast, backlight, kp_backlight;

                               ^~~~~~~~~~~~

This is likely why the compiler is optimizing the code away. As JimEli said try writing the values to "volatile" variables.

The compiler only knows memory (RAM). When you write to the same memory location without reading it the compiler can assume the writes are not necessary and can keep the value in the processor registers or remove the code altogether. This usualy depends on the compiler optimization settings. With volatile the compiler is forced to write to the location anyways.

There are a few other warnings you might want to check for potential additional issues with your library.

Thanks guys. I'll check on those things. I have the warnings turned on now, didn't see that in the options. Hopefully can get back to finishing out the library and not dealing with ghost issues.

Actually, just tried it and realized those were variables in the example code where it was supposed to read the values and then set them back to original settings. That wasn't working, which was why I changed to the loop that should drive them both back to full bright. So, I guess I'm going to have to try forcing it not to optimize in critical sections of code.

Think it's fixed. The warning for notValid being uninitialized was the hint. In my send_verify_return_packet routine, it was not being initialized before being used in a while statement. Why it worked most of the time anyway is beyond me, but have it initialized now and getting the program to behave properly and consistently.