* Fixed negative index in stack

[iosifpeterfi]

• Fixed a race condition leading to a loop when resume() is called
  in IRrecv and rawbuf is still filled

[ArminJo]

OK, Thanks 🥇
Can you explain the race condition or better add a comment why it is required at the statement irparams.rawlen = 0; ?

[ArminJo]

Thanks for finding the bug at sendRaw(durations + intros, repeats - 1, khz); 😀

[iosifpeterfi]

Let me explain the race condition.

Let's take as an example the following implementation from IRreceiveDumpV2.ino:

void loop() { if (IrReceiver.decode()) { // Grab an IR code dumpInfo(); // Output the results IrReceiver.printIRResultRawFormatted(&Serial); // Output the results in RAW format Serial.println(); // blank line between entries IrReceiver.printIRResultAsCArray(&Serial); // Output the results as source code array IrReceiver.printIRResultAsCVariables(&Serial); // Output address and data as source code variables IrReceiver.printIRResultAsPronto(&Serial); Serial.println(); // 2 blank lines between entries Serial.println(); IrReceiver.resume(); // Prepare for the next value } }

The example shows a check on IrReceiver.decode(), then a IrReceiver.resume() to resume.

In the sitation that RAW_BUFFER_LENGTH reached the code path is as follows:

In IRremote.cpp 135:
if (irparams.rawlen >= RAW_BUFFER_LENGTH) { // Flag up a read overflow; Stop the State Machine irparams.overflow = true; irparams.rcvstate = IR_REC_STATE_STOP; }
This stops the timer and sets overflow and rcvstate.

Then the decode() function runs and outputs info to Serial.

3. IrReceiver.resume() is invoked, and rcvstate is set to IR_REC_STATE_IDLE.

4. Timer starts again and irparams.rawlen is still bigger than RAW_BUFFER_LENGTH

Loops forever 1 to 4

resume() is called internally 3 times.

Twice at end of decode, if no decoder can parse the data, so then rawbuf can be discarded, however now is kept.

Once in in IRrecv::available() looks like it tries to 'skip' the overflowed buffer but instead it just ignores it and returns false.

The behavior now is to keep the buffer if the IR signal cannot be decoded or is too long. I think we need to discard it since it is either incomplete or none of the decoders can make anything of it. (Although decodeHash works with any data).

Because resume() is called always in situations when the buffer should be discarded, then is safe to set the rawlen to 0.

Another approach would be to create another function flush() for example which clears the buffer.
From implementation perspective. I'd always want to call flush() before resume().

[bengtmartensson]

A Pronto signal consists of an intro- and a repeat-segment. Any of these can be empty. See this description. On a physical remote, the intro is sent once when a button is pressed, and then the repeat is sent repeatedly as long as the button is held pressed. The sendPronto function models this behavior. It sends the intro once, and the repeat a certain number of times. The original behavior is the correct one, not the "fixed" one.

[iosifpeterfi]

@bengtmartensson After reviewing the ProntoSemantics it makes more sense.

If the function is called with times bigger than 1 and no value for repeats in Proto CFF, the it crashes at that line.
Let me move the check before that.

Sorry for the confusion.

[iosifpeterfi]

Let me know if now is better.

[ArminJo]

@bengtmartensson
I do not understand why the call sendRaw(durations, intros - 1, khz); al line 57 uses an odd number intros - 1, since intros = 2 * data[2] is even?
OK I got it (and dociumented it!)

[ArminJo]

@iosifpeterfi Thanks for the explanation!
I moved the irparams.rawlen handling to the state machine to keep it locally.

@bengtmartensson
I commented the code and extended the example and the dump format and tested both. Thanks!

[bengtmartensson]

@iosifpeterfi:

Let me know if now is better.

Looks fine!

@ArminJo:

I do not understand why the call sendRaw(durations, intros - 1, khz); al line 57 uses an odd number intros - 1, since intros = 2 * data[2] is even?
OK I got it

For those who did not "got it": sendRaw is handling sequences with an odd number of entries (starting and ending with a space mark), while the sequences in a Pronto are even number of durations (staring with a space mark, ending with a gap space).

[ArminJo]

For those who did not "got it": sendRaw is handling sequences with an odd number of entries (starting and ending with a space), while the sequences in a Pronto are even number of durations (staring with a space, ending with a gap).

But then the source is wrong :-(

    for (uint8_t i = 0; i < aLengthOfBuffer; i++) {
        if (i & 1) {
            space(aBufferWithMicroseconds[i]);
        } else {
            mark(aBufferWithMicroseconds[i]);
        }
    }

[bengtmartensson]

But then the source is wrong

Sorry, I got the terminology wrong. I changed in my previous message. OK now with you?

[ArminJo]

Thanks, I documented it anyway