Fix decoding issue on most protocols.

- Fixes Issue #243 - [bugfix] Incorrect assumption on minimum entry length when decoding. Off by one. - [bugfix] Make matching the trailing gap/space on commands optional if we run out of buffer. - Fix a wayward Serial.println() debug statement that got missed. - Add unit tests to add coverage for the bug found. - Bump version to v2.0.1
crankyoldgit · Jun 14, 2017 · bc387a8 · bc387a8
1 parent 235eb67
commit bc387a8
Show file tree

Hide file tree

Showing 21 changed files with 102 additions and 30 deletions.
diff --git a/.gitignore b/.gitignore
@@ -26,3 +26,7 @@ lib/readme.txt
 test/*.o
 test/*.a
 test/*_test
+.pioenvs
+.piolibdeps
+.clang_complete
+.gcc-flags.json
diff --git a/library.json b/library.json
@@ -1,6 +1,6 @@
 {
   "name": "IRremoteESP8266",
-  "version": "2.0.0",
+  "version": "2.0.1",
   "keywords": "infrared, ir, remote, esp8266",
   "description": "Send and receive infrared signals with multiple protocols (ESP8266)",
   "repository":

diff --git a/library.properties b/library.properties
@@ -1,5 +1,5 @@
 name=IRremoteESP8266
-version=2.0.0
+version=2.0.1
 author=Sebastien Warin, Mark Szabo, Ken Shirriff, David Conran
 maintainer=Mark Szabo, David Conran, Sebastien Warin, Roi Dayan, Massimiliano Pinto
 sentence=Send and receive infrared signals with multiple protocols (ESP8266)

diff --git a/src/IRrecv.cpp b/src/IRrecv.cpp
@@ -226,7 +226,7 @@ bool IRrecv::decode(decode_results *results, irparams_t *save) {
 #if DECODE_DENON
   // Denon needs to preceed Panasonic as it is a special case of Panasonic.
 #ifdef DEBUG
-  Serial.println("Attempting Denon decode");
+  DPRINTLN("Attempting Denon decode");
 #endif
   if (decodeDenon(results, DENON_48_BITS) ||
       decodeDenon(results, DENON_BITS) ||

diff --git a/src/ir_Coolix.cpp b/src/ir_Coolix.cpp
@@ -87,7 +87,7 @@ bool IRrecv::decodeCOOLIX(decode_results *results, uint16_t nbits,
                           bool strict) {
   // The protocol sends the data normal + inverted, alternating on
   // each byte. Hence twice the number of expected data bits.
-  if (results->rawlen < 2 * 2 * nbits + HEADER + FOOTER)
+  if (results->rawlen < 2 * 2 * nbits + HEADER + FOOTER - 1)
     return false;  // Can't possibly be a valid COOLIX message.
   if (strict && nbits != COOLIX_BITS)
     return false;  // Not strictly an COOLIX message.
@@ -131,7 +131,8 @@ bool IRrecv::decodeCOOLIX(decode_results *results, uint16_t nbits,
   // Footer
   if (!matchMark(results->rawbuf[offset++], COOLIX_BIT_MARK))
     return false;
-  if (!matchAtLeast(results->rawbuf[offset], COOLIX_MIN_GAP))
+  if (offset <= results->rawlen &&
+      !matchAtLeast(results->rawbuf[offset], COOLIX_MIN_GAP))
     return false;
 
   // Compliance

diff --git a/src/ir_Denon.cpp b/src/ir_Denon.cpp
@@ -95,7 +95,7 @@ bool IRrecv::decodeDenon(decode_results *results, uint16_t nbits, bool strict) {
     // NOTE: I don't this following protocol actually exists.
     //       Looks like a partial version of the Sharp protocol.
     // Check we have enough data
-    if (results->rawlen < 2 * nbits + HEADER + FOOTER)
+    if (results->rawlen < 2 * nbits + HEADER + FOOTER - 1)
       return false;
     if (strict && nbits != DENON_LEGACY_BITS)
       return false;

diff --git a/src/ir_Dish.cpp b/src/ir_Dish.cpp
@@ -86,7 +86,7 @@ void IRsend::sendDISH(uint64_t data, uint16_t nbits, uint16_t repeat) {
 //   http://lirc.sourceforge.net/remotes/echostar/301_501_3100_5100_58xx_59xx
 //   https://github.com/marcosamarinho/IRremoteESP8266/blob/master/ir_Dish.cpp
 bool IRrecv::decodeDISH(decode_results *results, uint16_t nbits, bool strict) {
-  if (results->rawlen < 2 * nbits + HEADER + FOOTER)
+  if (results->rawlen < 2 * nbits + HEADER + FOOTER - 1)
     return false;  // Not enough entries to be valid.
   if (strict && nbits != DISH_BITS)
     return false;  // Not strictly compliant.

diff --git a/src/ir_JVC.cpp b/src/ir_JVC.cpp
@@ -98,7 +98,7 @@ uint16_t IRsend::encodeJVC(uint8_t address, uint8_t command) {
 bool IRrecv::decodeJVC(decode_results *results, uint16_t nbits,  bool strict) {
   if (strict && nbits != JVC_BITS)
     return false;  // Must be called with the correct nr. of bits.
-  if (results->rawlen < 2 * nbits + 2)
+  if (results->rawlen < 2 * nbits + FOOTER - 1)
     return false;  // Can't possibly be a valid JVC message.
 
   uint64_t data = 0;
@@ -132,7 +132,8 @@ bool IRrecv::decodeJVC(decode_results *results, uint16_t nbits,  bool strict) {
   // Footer
   if (!matchMark(results->rawbuf[offset++], JVC_BIT_MARK))
     return false;
-  if (!matchAtLeast(results->rawbuf[offset], JVC_MIN_GAP))
+  if (offset <= results->rawlen &&
+      !matchAtLeast(results->rawbuf[offset], JVC_MIN_GAP))
     return false;
 
   // Success

diff --git a/src/ir_LG.cpp b/src/ir_LG.cpp
@@ -137,7 +137,7 @@ uint32_t IRsend::encodeLG(uint16_t address, uint16_t command) {
 // Ref:
 //   https://funembedded.wordpress.com/2014/11/08/ir-remote-control-for-lg-conditioner-using-stm32f302-mcu-on-mbed-platform/
 bool IRrecv::decodeLG(decode_results *results, uint16_t nbits, bool strict) {
-  if (results->rawlen < 2 * nbits + 4 && results->rawlen != 4)
+  if (results->rawlen < 2 * nbits + HEADER + FOOTER - 1 && results->rawlen != 4)
     return false;  // Can't possibly be a valid LG message.
   if (strict && nbits != LG_BITS && nbits != LG32_BITS)
     return false;  // Doesn't comply with expected LG protocol.
@@ -168,7 +168,8 @@ bool IRrecv::decodeLG(decode_results *results, uint16_t nbits, bool strict) {
   // Footer
   if (!matchMark(results->rawbuf[offset++], LG_BIT_MARK))
     return false;
-  if (!matchAtLeast(results->rawbuf[offset], LG_MIN_GAP))
+  if (offset <= results->rawlen &&
+      !matchAtLeast(results->rawbuf[offset], LG_MIN_GAP))
     return false;
 
   // Repeat

diff --git a/src/ir_Mitsubishi.cpp b/src/ir_Mitsubishi.cpp
@@ -93,7 +93,7 @@ void IRsend::sendMitsubishi(uint64_t data, uint16_t nbits, uint16_t repeat) {
 //   GlobalCache's Control Tower's Mitsubishi TV data.
 bool IRrecv::decodeMitsubishi(decode_results *results, uint16_t nbits,
                               bool strict) {
-  if (results->rawlen < 2 * nbits + FOOTER)
+  if (results->rawlen < 2 * nbits + FOOTER - 1)
     return false;  // Shorter than shortest possibly expected.
   if (strict && nbits != MITSUBISHI_BITS)
     return false;  // Request is out of spec.
@@ -119,7 +119,8 @@ bool IRrecv::decodeMitsubishi(decode_results *results, uint16_t nbits,
   // Footer
   if (!matchMark(results->rawbuf[offset++], MITSUBISHI_BIT_MARK, 30))
     return false;
-  if (!matchAtLeast(results->rawbuf[offset], MITSUBISHI_MIN_GAP))
+  if (offset <= results->rawlen &&
+      !matchAtLeast(results->rawbuf[offset], MITSUBISHI_MIN_GAP))
     return false;
 
   // Compliance

diff --git a/src/ir_NEC.cpp b/src/ir_NEC.cpp
@@ -119,7 +119,7 @@ uint32_t IRsend::encodeNEC(uint16_t address, uint16_t command) {
 // Ref:
 //   http://www.sbprojects.com/knowledge/ir/nec.php
 bool IRrecv::decodeNEC(decode_results *results, uint16_t nbits, bool strict) {
-  if (results->rawlen < 2 * nbits + HEADER + FOOTER &&
+  if (results->rawlen < 2 * nbits + HEADER + FOOTER - 1 &&
       results->rawlen != NEC_RPT_LENGTH)
     return false;  // Can't possibly be a valid NEC message.
   if (strict && nbits != NEC_BITS)
@@ -161,7 +161,8 @@ bool IRrecv::decodeNEC(decode_results *results, uint16_t nbits, bool strict) {
   // Footer
   if (!matchMark(results->rawbuf[offset++], NEC_BIT_MARK))
       return false;
-  if (!matchAtLeast(results->rawbuf[offset], NEC_MIN_GAP))
+  if (offset <= results->rawlen &&
+      !matchAtLeast(results->rawbuf[offset], NEC_MIN_GAP))
     return false;
 
   // Compliance

diff --git a/src/ir_Panasonic.cpp b/src/ir_Panasonic.cpp
@@ -127,7 +127,7 @@ uint64_t IRsend::encodePanasonic(uint16_t manufacturer,
 //   http://www.hifi-remote.com/wiki/index.php?title=Panasonic
 bool IRrecv::decodePanasonic(decode_results *results, uint16_t nbits,
                              bool strict, uint32_t manufacturer) {
-  if (results->rawlen < 2 * nbits + HEADER + FOOTER)
+  if (results->rawlen < 2 * nbits + HEADER + FOOTER - 1)
     return false;  // Not enough entries to be a Panasonic message.
   if (strict && nbits != PANASONIC_BITS)
     return false;  // Request is out of spec.
@@ -155,7 +155,8 @@ bool IRrecv::decodePanasonic(decode_results *results, uint16_t nbits,
   // Footer
   if (!match(results->rawbuf[offset++], PANASONIC_BIT_MARK))
     return false;
-  if (!matchAtLeast(results->rawbuf[offset], PANASONIC_MIN_GAP))
+  if (offset <= results->rawlen &&
+      !matchAtLeast(results->rawbuf[offset], PANASONIC_MIN_GAP))
     return false;
 
   // Compliance

diff --git a/src/ir_RC5_RC6.cpp b/src/ir_RC5_RC6.cpp
@@ -354,7 +354,7 @@ int16_t IRrecv::getRClevel(decode_results *results,  uint16_t *offset,
 // TODO(anyone):
 //   Serious testing of the RC-5X and strict aspects needs to be done.
 bool IRrecv::decodeRC5(decode_results *results, uint16_t nbits, bool strict) {
-  if (results->rawlen < MIN_RC5_SAMPLES + HEADER) return false;
+  if (results->rawlen < MIN_RC5_SAMPLES + HEADER - 1) return false;
 
   // Compliance
   if (strict && nbits != RC5_BITS && nbits != RC5X_BITS)

diff --git a/src/ir_RCMM.cpp b/src/ir_RCMM.cpp
@@ -141,7 +141,8 @@ bool IRrecv::decodeRCMM(decode_results *results, uint16_t nbits, bool strict) {
   // Footer decode
   if (!matchMark(results->rawbuf[offset++], RCMM_BIT_MARK))
     return false;
-  if (!matchAtLeast(results->rawbuf[offset], RCMM_MIN_GAP))
+  if (offset <= results->rawlen &&
+      !matchAtLeast(results->rawbuf[offset], RCMM_MIN_GAP))
     return false;
 
   // Compliance

diff --git a/src/ir_Samsung.cpp b/src/ir_Samsung.cpp
@@ -102,7 +102,7 @@ uint32_t IRsend::encodeSAMSUNG(uint8_t customer, uint8_t command) {
 //  http://elektrolab.wz.cz/katalog/samsung_protocol.pdf
 bool IRrecv::decodeSAMSUNG(decode_results *results, uint16_t nbits,
                            bool strict) {
-  if (results->rawlen < 2 * nbits + 4)
+  if (results->rawlen < 2 * nbits + HEADER + FOOTER - 1)
     return false;  // Can't possibly be a valid Samsung message.
   if (strict && nbits != SAMSUNG_BITS)
     return false;  // We expect Samsung to be 32 bits of message.
@@ -129,7 +129,8 @@ bool IRrecv::decodeSAMSUNG(decode_results *results, uint16_t nbits,
   // Footer
   if (!matchMark(results->rawbuf[offset++], SAMSUNG_BIT_MARK))
     return false;
-  if (!matchAtLeast(results->rawbuf[offset], SAMSUNG_MIN_GAP))
+  if (offset <= results->rawlen &&
+      !matchAtLeast(results->rawbuf[offset], SAMSUNG_MIN_GAP))
     return false;
 
   // Compliance

diff --git a/src/ir_Sanyo.cpp b/src/ir_Sanyo.cpp
@@ -183,7 +183,7 @@ bool IRrecv::decodeSanyoLC7461(decode_results *results, uint16_t nbits,
 // Ref:
 //   https://github.com/z3t0/Arduino-IRremote/blob/master/ir_Sanyo.cpp
 bool IRrecv::decodeSanyo(decode_results *results, uint16_t nbits, bool strict) {
-  if (results->rawlen < 2 * nbits + HEADER)
+  if (results->rawlen < 2 * nbits + HEADER - 1)
     return false;  // Shorter than shortest possible.
   if (strict && nbits != SANYO_SA8650B_BITS)
     return false;  // Doesn't match the spec.

diff --git a/src/ir_Sharp.cpp b/src/ir_Sharp.cpp
@@ -173,7 +173,7 @@ void IRsend::sendSharp(uint16_t address, uint16_t command, uint16_t nbits,
 //   http://www.hifi-remote.com/johnsfine/DecodeIR.html#Sharp
 bool IRrecv::decodeSharp(decode_results *results, uint16_t nbits, bool strict,
                          bool expansion) {
-  if (results->rawlen < 2 * nbits + FOOTER)
+  if (results->rawlen < 2 * nbits + FOOTER - 1)
     return false;  // Not enough entries to be a Sharp message.
   // Compliance
   if (strict) {
@@ -209,7 +209,8 @@ bool IRrecv::decodeSharp(decode_results *results, uint16_t nbits, bool strict,
   // Footer
   if (!match(results->rawbuf[offset++], SHARP_BIT_MARK))
     return false;
-  if (!matchAtLeast(results->rawbuf[offset], SHARP_GAP))
+  if (offset <= results->rawlen &&
+      !matchAtLeast(results->rawbuf[offset], SHARP_GAP))
     return false;
 
   // Compliance

diff --git a/src/ir_Sony.cpp b/src/ir_Sony.cpp
@@ -113,7 +113,7 @@ uint32_t IRsend::encodeSony(uint16_t nbits, uint16_t command,
 // Ref:
 // http://www.sbprojects.com/knowledge/ir/sirc.php
 bool IRrecv::decodeSony(decode_results *results, uint16_t nbits, bool strict) {
-  if (results->rawlen < 2 * nbits + HEADER)
+  if (results->rawlen < 2 * nbits + HEADER - 1)
     return false;  // Message is smaller than we expected.
 
   // Compliance

diff --git a/src/ir_Whynter.cpp b/src/ir_Whynter.cpp
@@ -77,7 +77,7 @@ void IRsend::sendWhynter(uint64_t data, uint16_t nbits, uint16_t repeat) {
 //   https://github.com/z3t0/Arduino-IRremote/blob/master/ir_Whynter.cpp
 bool IRrecv::decodeWhynter(decode_results *results, uint16_t nbits,
                            bool strict) {
-  if (results->rawlen < 2 * nbits + 2 * HEADER + FOOTER)
+  if (results->rawlen < 2 * nbits + 2 * HEADER + FOOTER - 1)
      return false;  // We don't have enough entries to possibly match.
 
   // Compliance
@@ -114,7 +114,8 @@ bool IRrecv::decodeWhynter(decode_results *results, uint16_t nbits,
   // Footer
   if (!matchMark(results->rawbuf[offset++], WHYNTER_BIT_MARK))
     return false;
-  if (!matchAtLeast(results->rawbuf[offset], WHYNTER_MIN_GAP))
+  if (offset <= results->rawlen &&
+      !matchAtLeast(results->rawbuf[offset], WHYNTER_MIN_GAP))
     return false;
 
   // Success

diff --git a/test/IRsend_test.cpp b/test/IRsend_test.cpp
@@ -100,18 +100,40 @@ TEST(TestSendRaw, GeneralUse) {
 
   irsend.reset();
   irsend.sendRaw(rawData, 67, 38);
-  // Add some extra space at the end of the command to allow it to match.
-  irsend.addGap(22000);
   irsend.makeDecodeResult();
   EXPECT_EQ(
       "m8950s4500"
       "m550s1650m600s1650m550s550m600s500m600s550m550s550m600s1650m550s1650"
       "m600s1650m600s1650m550s1700m550s550m600s550m550s550m600s500m600s550"
       "m550s1650m600s1650m600s1650m550s550m600s500m600s500m600s550m550s550"
       "m600s1650m550s1650m600s1650m600s500m650s1600m600s500m600s550m550s550"
-      "m600s22000", irsend.outputStr());
+      "m600", irsend.outputStr());
   ASSERT_TRUE(irrecv.decodeNEC(&irsend.capture, NEC_BITS, false));
   EXPECT_EQ(NEC, irsend.capture.decode_type);
   EXPECT_EQ(32, irsend.capture.bits);
   EXPECT_EQ(0xC3E0E0E8, irsend.capture.value);
 }
+
+// Incorrect handling of decodes from Raw. i.e. There is no gap recorded at
+// the end of a command when using the interrupt code. sendRaw() best emulates
+// this for unit testing purposes. sendGC() and sendXXX() will add the trailing
+// gap. Users won't see this in normal use.
+TEST(TestSendRaw, NoTrailingGap) {
+  IRsendTest irsend(4);
+  IRrecv irrecv(4);
+  irsend.begin();
+
+  irsend.reset();
+  uint16_t rawData[67] = {9000, 4500, 650, 550, 650, 1650, 600, 550, 650, 550,
+                          600, 1650, 650, 550, 600, 1650, 650, 1650, 650, 1650,
+                          600, 550, 650, 1650, 650, 1650, 650, 550, 600, 1650,
+                          650, 1650, 650, 550, 650, 550, 650, 1650, 650, 550,
+                          650, 550, 650, 550, 600, 550, 650, 550, 650, 550,
+                          650, 1650, 600, 550, 650, 1650, 650, 1650, 650, 1650,
+                          650, 1650, 650, 1650, 650, 1650, 600};
+  irsend.sendRaw(rawData, 67, 38);
+  irsend.makeDecodeResult();
+  EXPECT_TRUE(irrecv.decodeNEC(&irsend.capture));
+  EXPECT_EQ(NEC, irsend.capture.decode_type);
+  EXPECT_EQ(NEC_BITS, irsend.capture.bits);
+}
diff --git a/test/ir_NEC_test.cpp b/test/ir_NEC_test.cpp
@@ -213,3 +213,39 @@ TEST(TestDecodeNEC, LongerNECDecodeWithoutStrict) {
   irsend.makeDecodeResult();
   EXPECT_FALSE(irrecv.decodeNEC(&irsend.capture, 64, false));
 }
+
+// Incorrect decoding reported in Issue #243
+// Incorrect handling of decodes when there is no gap recorded at
+// the end of a command when using the interrupt code. sendRaw() best emulates
+// this for unit testing purposes. sendGC() and sendXXX() will add the trailing
+// gap. Users won't see this in normal use.
+TEST(TestDecodeNEC, NoTrailingGap_Issue243) {
+  IRsendTest irsend(4);
+  IRrecv irrecv(4);
+  irsend.begin();
+
+  irsend.reset();
+  uint16_t rawData[67] = {9000, 4500, 650, 550, 650, 1650, 600, 550, 650, 550,
+                          600, 1650, 650, 550, 600, 1650, 650, 1650, 650, 1650,
+                          600, 550, 650, 1650, 650, 1650, 650, 550, 600, 1650,
+                          650, 1650, 650, 550, 650, 550, 650, 1650, 650, 550,
+                          650, 550, 650, 550, 600, 550, 650, 550, 650, 550,
+                          650, 1650, 600, 550, 650, 1650, 650, 1650, 650, 1650,
+                          650, 1650, 650, 1650, 650, 1650, 600};
+  irsend.sendRaw(rawData, 67, 38);
+  irsend.makeDecodeResult();
+  EXPECT_TRUE(irrecv.decodeNEC(&irsend.capture));
+  EXPECT_EQ(NEC, irsend.capture.decode_type);
+  EXPECT_EQ(NEC_BITS, irsend.capture.bits);
+  EXPECT_EQ(0x4BB640BF, irsend.capture.value);
+  EXPECT_EQ(0x6DD2, irsend.capture.address);
+  EXPECT_EQ(0x2, irsend.capture.command);
+
+  irsend.reset();
+  irsend.sendRaw(rawData, 67, 38);
+  irsend.makeDecodeResult();
+  ASSERT_TRUE(irrecv.decode(&irsend.capture));
+  EXPECT_EQ(NEC, irsend.capture.decode_type);
+  EXPECT_EQ(NEC_BITS, irsend.capture.bits);
+  EXPECT_EQ(0x4BB640BF, irsend.capture.value);
+}