Fix problems with broken receives #828

Author: ArminJo

README.md

@@ -48,12 +48,12 @@ Therefore you must change all `IrSender.begin(true);` by `IrSender.begin(IR_SEND
 In the new version you will send NEC commands not by 32 bit codes but by a (constant) 8 bit address and an 8 bit command.
 
 # FAQ
-- IR does not work right when I use Neopixels (aka WS2811/WS2812/WS2812B)<br/>
+- IR does not work right when I use Neopixels (aka WS2811/WS2812/WS2812B) or other libraries blocking interrupts for a longer time (> 50 us).<br/>
  Whether you use the Adafruit Neopixel lib, or FastLED, interrupts get disabled on many lower end CPUs like the basic Arduinos for longer than 50 µs.
 In turn, this stops the IR interrupt handler from running when it needs to. There are some solutions to this on some processors,
  [see this page from Marc MERLIN](http://marc.merlins.org/perso/arduino/post_2017-04-03_Arduino-328P-Uno-Teensy3_1-ESP8266-ESP32-IR-and-Neopixels.html)
 - The default IR timer on AVR's is timer 2. Since the **Arduino Tone library** as well as **analogWrite() for pin 3 and pin 11** requires timer 2,
- this functionality cannot be used simultaneously. You can use tone() but after the tone has stopped, you must call `IrReceiver.start()` or better `IrReceiver.start(<microsecondsOfToneDuration>)` to restore the timer settings for receive.<br/>
+ this functionality cannot be used simultaneously. You can use tone() but after the tone has stopped, you must call `IrReceiver.start()` or better `IrReceiver.start(<microsecondsOfToneDuration>)` to restore the timer settings for receive. Or you change the timer to timer 1 in private/IRTimer.cpp.h.<br/>
 If you can live with the NEC protocol, you can try the MinimalReceiver example, it requires no timer.
 - You can use **multiple IR receiver** by just connecting the output pins of several IR receivers together.
  The IR receivers use an NPN transistor as output device with just a 30k resistor to VCC.
@@ -133,7 +133,7 @@ Modify it by commenting them out or in, or change the values if applicable. Or d
 | `DO_NOT_USE_FEEDBACK_LED` | TinyIRReceiver.h | disabled | Enable it to disable the feedback LED function. |
 
 ### Modifying compile options with Arduino IDE
-First use *Sketch > Show Sketch Folder (Ctrl+K)*.<br/>
+First, use *Sketch > Show Sketch Folder (Ctrl+K)*.<br/>
 If you did not yet stored the example as your own sketch, then you are instantly in the right library folder.<br/>
 Otherwise you have to navigate to the parallel `libraries` folder and select the library you want to access.<br/>
 In both cases the library files itself are located in the `src` directory.<br/>

src/TinyIRReceiver.cpp.h

@@ -46,7 +46,6 @@
  * @{
  */
 //#define TRACE
-
 TinyIRReceiverStruct TinyIRReceiverControl;
 
 /**
@@ -70,7 +69,10 @@ ICACHE_RAM_ATTR
 IRAM_ATTR
 #endif
 void IRPinChangeInterruptHandler(void) {
-    // save IR input level - negative logic, true means inactive / IR pause
+    /*
+     * Save IR input level
+     * Negative logic, true / HIGH means inactive / IR space, LOW / false means IR mark.
+     */
     uint_fast8_t tIRLevel = digitalReadFast(IR_INPUT_PIN);
 
 #if !defined(DO_NOT_USE_FEEDBACK_LED) && defined(IR_FEEDBACK_LED_PIN)
@@ -81,7 +83,7 @@ void IRPinChangeInterruptHandler(void) {
      * 1. compute microseconds after last change
      */
     uint32_t tCurrentMicros = micros();
-    uint16_t tDeltaMicros = tCurrentMicros - TinyIRReceiverControl.LastChangeMicros;
+    uint16_t tMicrosOfMarkOrSpace = tCurrentMicros - TinyIRReceiverControl.LastChangeMicros;
     TinyIRReceiverControl.LastChangeMicros = tCurrentMicros;
 
     uint8_t tState = TinyIRReceiverControl.IRReceiverState;
@@ -98,26 +100,34 @@ void IRPinChangeInterruptHandler(void) {
 
     if (tIRLevel == LOW) {
         /*
-         * We receive a signal now
+         * We have a mark here
          */
-        if (tDeltaMicros > 2 * NEC_HEADER_MARK) {
+        if (tMicrosOfMarkOrSpace > 2 * NEC_HEADER_MARK) {
             // timeout -> must reset state machine
             tState = IR_RECEIVER_STATE_WAITING_FOR_START_MARK;
         }
         if (tState == IR_RECEIVER_STATE_WAITING_FOR_START_MARK) {
+            // We are at the beginning of the header mark, check timing at the next transition
             tState = IR_RECEIVER_STATE_WAITING_FOR_START_SPACE;
         }
 
         else if (tState == IR_RECEIVER_STATE_WAITING_FOR_FIRST_DATA_MARK) {
-            // Check start space length
-            if (tDeltaMicros >= lowerValue25Percent(NEC_HEADER_SPACE) && tDeltaMicros <= upperValue25Percent(NEC_HEADER_SPACE)) {
-                tState = IR_RECEIVER_STATE_WAITING_FOR_DATA_SPACE;
+            if (tMicrosOfMarkOrSpace >= lowerValue25Percent(NEC_HEADER_SPACE)
+                    && tMicrosOfMarkOrSpace <= upperValue25Percent(NEC_HEADER_SPACE)) {
+                /*
+                 * We have a valid data header space here -> initialize data
+                 */
                 TinyIRReceiverControl.IRRawDataBitCounter = 0;
                 TinyIRReceiverControl.IRRawData.ULong = 0;
                 TinyIRReceiverControl.IRRawDataMask = 1;
                 TinyIRReceiverControl.IRRepeatDetected = false;
-            } else if (tDeltaMicros >= lowerValue25Percent(NEC_REPEAT_HEADER_SPACE)
-                    && tDeltaMicros <= upperValue25Percent(NEC_REPEAT_HEADER_SPACE)) {
+                tState = IR_RECEIVER_STATE_WAITING_FOR_DATA_SPACE;
+            } else if (tMicrosOfMarkOrSpace >= lowerValue25Percent(NEC_REPEAT_HEADER_SPACE)
+                    && tMicrosOfMarkOrSpace <= upperValue25Percent(NEC_REPEAT_HEADER_SPACE)
+                    && TinyIRReceiverControl.IRRawDataBitCounter >= NEC_BITS) {
+                /*
+                 * We have a repeat header here and no broken receive before -> set repeat flag
+                 */
                 TinyIRReceiverControl.IRRepeatDetected = true;
                 tState = IR_RECEIVER_STATE_WAITING_FOR_DATA_SPACE;
             } else {
@@ -129,51 +139,56 @@ void IRPinChangeInterruptHandler(void) {
 
         else if (tState == IR_RECEIVER_STATE_WAITING_FOR_DATA_MARK) {
             // Check data space length
-            if (tDeltaMicros >= lowerValue(NEC_ZERO_SPACE) && tDeltaMicros <= upperValue(NEC_ONE_SPACE)) {
-                // Here we have a valid bit
+            if (tMicrosOfMarkOrSpace >= lowerValue(NEC_ZERO_SPACE) && tMicrosOfMarkOrSpace <= upperValue(NEC_ONE_SPACE)) {
+                // We have a valid bit here
                 tState = IR_RECEIVER_STATE_WAITING_FOR_DATA_SPACE;
-                if (tDeltaMicros >= 2 * NEC_UNIT) {
+                if (tMicrosOfMarkOrSpace >= 2 * NEC_UNIT) {
                     // we received a 1
                     TinyIRReceiverControl.IRRawData.ULong |= TinyIRReceiverControl.IRRawDataMask;
                 } else {
-                    // we received a 0
+                    // we received a 0 - empty code for documentation
                 }
+                // prepare for next bit
                 TinyIRReceiverControl.IRRawDataMask = TinyIRReceiverControl.IRRawDataMask << 1;
                 TinyIRReceiverControl.IRRawDataBitCounter++;
             } else {
                 // Wrong length -> reset state
                 tState = IR_RECEIVER_STATE_WAITING_FOR_START_MARK;
             }
         } else {
-            // error wrong state for the received level (should not happen!) -> reset state
+            // error wrong state for the received level (should not happen!) -> reset state (2 checks costs 10 bytes programming space)
             tState = IR_RECEIVER_STATE_WAITING_FOR_START_MARK;
         }
     }
 
     else {
         /*
-         * We receive a space now
+         * We have a space here
          */
         if (tState == IR_RECEIVER_STATE_WAITING_FOR_START_SPACE) {
-            // Check start bit length
-            if (tDeltaMicros >= lowerValue25Percent(NEC_HEADER_MARK) && tDeltaMicros <= upperValue25Percent(NEC_HEADER_MARK)) {
+            /*
+             * Check length of header mark here
+             */
+            if (tMicrosOfMarkOrSpace >= lowerValue25Percent(NEC_HEADER_MARK)
+                    && tMicrosOfMarkOrSpace <= upperValue25Percent(NEC_HEADER_MARK)) {
                 tState = IR_RECEIVER_STATE_WAITING_FOR_FIRST_DATA_MARK;
             } else {
-                // Wrong length -> reset state
+                // Wrong length of header mark -> reset state
                 tState = IR_RECEIVER_STATE_WAITING_FOR_START_MARK;
             }
         }
 
         else if (tState == IR_RECEIVER_STATE_WAITING_FOR_DATA_SPACE) {
-
-            // Check data bit length
-            if (tDeltaMicros >= lowerValue(NEC_BIT_MARK) && tDeltaMicros <= upperValue(NEC_BIT_MARK)) {
+            // Check data mark length
+            if (tMicrosOfMarkOrSpace >= lowerValue(NEC_BIT_MARK) && tMicrosOfMarkOrSpace <= upperValue(NEC_BIT_MARK)) {
+                /*
+                 * We have a valid mark here, check for transmission complete
+                 */
                 if (TinyIRReceiverControl.IRRawDataBitCounter >= NEC_BITS || TinyIRReceiverControl.IRRepeatDetected) {
                     /*
-                     * Code complete -> call callback
-                     * No parity check
+                     * Code complete -> call callback, no parity check!
                      */
-                    // Set state for new start
+                    // Reset state for new start
                     tState = IR_RECEIVER_STATE_WAITING_FOR_START_MARK;
 #if !defined(ARDUINO_ARCH_MBED)
                     interrupts();
@@ -187,18 +202,22 @@ void IRPinChangeInterruptHandler(void) {
                         TinyIRReceiverControl.IRRawData.UByte.MidLowByte = 0; // Address is the first 8 bit
                     }
 
+                    /*
+                     * Call user provided callback here
+                     */
                     handleReceivedTinyIRData(TinyIRReceiverControl.IRRawData.UWord.LowWord,
                             TinyIRReceiverControl.IRRawData.UByte.MidHighByte, TinyIRReceiverControl.IRRepeatDetected);
 
                 } else {
+                    // not finished yet
                     tState = IR_RECEIVER_STATE_WAITING_FOR_DATA_MARK;
                 }
             } else {
                 // Wrong length -> reset state
                 tState = IR_RECEIVER_STATE_WAITING_FOR_START_MARK;
             }
         } else {
-            // error wrong state for the received level (should not happen!) -> reset state
+            // error wrong state for the received level (should not happen!) -> reset state (2 checks costs 10 bytes programming space)
             tState = IR_RECEIVER_STATE_WAITING_FOR_START_MARK;
         }
     }

src/ir_NEC.cpp

@@ -316,9 +316,10 @@ bool IRrecv::decodeNEC() {
 
 /*
  * With Send sendNECMSB() you can send your old 32 bit codes.
- * To convert one into the other, you must reverse the byte positions and then reverse all positions of each byte.
+ * To convert one into the other, you must reverse the byte positions and then reverse all bit positions of each byte.
+ * Or write it as one binary string and reverse/mirror it.
  * Example:
- * 0xCB340102 byte reverse -> 0x020134CB bit reverse-> 40802CD3
+ * 0xCB340102 byte reverse -> 02 01 34 CB bit reverse-> 40 80 2C D3. Bit reverse e.g. for CB = 1100 1011 -> (reverse/mirror bits) 1101 0011 = D3.
  */
 void IRsend::sendNECMSB(uint32_t data, uint8_t nbits, bool repeat) {
     // Set IR carrier frequency