A bug in my code got me curoius about the root cause

Hi,

I'm working on a simple project with NodeMCU and IR receiver module.
The IR module detect the key and the board just print something .

My code:

const int ir_pin = 4; // NodeMCU D2

#include <IRremote.h>

const int BUTTON_POWER = 0x12;

void setup() {
  Serial.begin(115200);
  IrReceiver.begin(ir_pin, ENABLE_LED_FEEDBACK);
}

int decode_button(IRData data) {
  if (
      (data.protocol == NEC) &&
      (!(data.flags & IRDATA_FLAGS_IS_REPEAT))
     )
    {
      IrReceiver.printIRResultShort(&Serial);  
      return data.command;
    }
}

void loop() {
  if (IrReceiver.decode()) {
    int button = decode_button(IrReceiver.decodedIRData);
    Serial.println(button);
    if (button == BUTTON_POWER) {
      Serial.println("got power");
    }
    IrReceiver.resume();
  }
  
}

During the work I noticed that after i created the 'decode_button' function, the board started to behaved weird and the code inside the 'if' function in 'decode_button' was always executed and printed stuff no matter what key I pressed or remote I used.
Before that everything worked fine.

After some debugging I noticed i forgot to return a value from that function when the 'if' is false, so I added 'return 0;' at the end and that's when everything worked again.
Now I enabled the warnings in the compiler, but that got me interested how is that even possible?
How can a missing return value can make the 'if' to always execute?

Output from the code above with a different remote:

Protocol=SAMSUNG Address=0x707 Command=0x7 Raw-Data=0xF8070707 32 bits LSB first
7
Protocol=SAMSUNG Address=0x707 Command=0xB Raw-Data=0xF40B0707 32 bits LSB first
11
Protocol=SAMSUNG Address=0x707 Command=0xF Raw-Data=0xF00F0707 32 bits LSB first
15

Thanks.

Educated guess would be that if decode_button() does not return a value then button will end up with whatever value happens to be left in RAM from the last time button was declared. As it is only a short sketch I would think button gets the same value it had from the previous time it was declared.

So to put it another way, if decode_button() does not return a value then you don't know what value button has.

right, return value from the stack that the function is responsible for updating prior to returning

at qualcomm and i assume all professional developers always enable -Wall and -Werror.

i'll guess that these options aren't enabled by default to accept legacy code compiled w/o them

Ran across this problem recently in another discussion https://forum.arduino.cc/t/arduino-nano-twice-read-from-eeprom-gives-crash/895755

In that case, the function was declared to return a String, but had no return, and the returned value was never used. Not sure what the compiler was doing, but the 2nd time the function was called the program would crash. Likely falls under "undefined behavior" from the compiler.

The issue I talk about here is that the 'if' clause is true always true:

if  (
      (data.protocol == NEC) &&
      (!(data.flags & IRDATA_FLAGS_IS_REPEAT))
     )

When you can clearly see that it's false in the output because I'm using a SAMSUNG remote in this case.

I find it odd that a missing return value can cause this issue.

I updated added more prints to my code to better show the issue:

const int ir_pin = 4; // NodeMCU D2

#include <IRremote.h>

const int BUTTON_POWER = 0x12;
 
void setup() {
  Serial.begin(115200);
  IrReceiver.begin(ir_pin, ENABLE_LED_FEEDBACK);
}


int decode_button(IRData data) {
  Serial.print("Protocol is ");
  Serial.print(data.protocol);
  Serial.print(" , Expected: ");
  Serial.println(NEC);
  if (
      (data.protocol == NEC) &&
      (!(data.flags & IRDATA_FLAGS_IS_REPEAT))
     )
    {
      Serial.println("Inside 'if' clause");
      IrReceiver.printIRResultShort(&Serial);  
      return data.command;
    }
}

void loop() {
  if (IrReceiver.decode()) {
    int button = decode_button(IrReceiver.decodedIRData);
    Serial.print("decode_button return value: ");
    Serial.println(button);
    if (button == BUTTON_POWER) {
      Serial.println("got power");
    }
    Serial.println();
    IrReceiver.resume();
  }
  
}

The output now is:

Protocol is 16 , Expected: 7
Inside 'if' clause
Protocol=SAMSUNG Address=0x707 Command=0x7 Raw-Data=0xF8070707 32 bits LSB first
decode_button return value: 7

Protocol is 16 , Expected: 7
Inside 'if' clause
Protocol=SAMSUNG Address=0x707 Command=0xF Raw-Data=0xF00F0707 32 bits LSB first
decode_button return value: 15

Protocol is 16 , Expected: 7
Inside 'if' clause
Protocol=SAMSUNG Address=0x707 Command=0xF Raw-Data=0xF00F0707 32 bits LSB first
decode_button return value: 15

You can see that the first condition of the 'if' is false because the protocol is not the one I expect, but we are still get inside the 'if'.

I suggest you make the prints inside the if a bit more specific, so print the actual values that got you there, so print data.protocol, NEC, data.flags, IRDATA_FLAGS_IS_REPEAT
I suspect they won't be what you think they are. I stress, print them inside the if.

I can't try your code as I don't have the right hardware, sorry.

Maybe the compiler sees only one return statement and therefore always enables this code.

If you include a return 0 as last statement in decode_button(), do you have the same behavior?

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