From 626302edb4b006d6cba62efbaf5698d653e2ec1d Mon Sep 17 00:00:00 2001 From: crankyoldgit Date: Sun, 30 Jan 2022 01:00:01 +1000 Subject: [PATCH] Refactor `decodeCOOLIX()` code & add another test case. * Remove the old auto scaling version of the code. It made it harder to diagnose problems. * Use `decodeGeneric()` as much as we can, and decode in whole 8-bit chuncks at a time. * Try to make the code more understandable and smaller. * Standardise on the same extra tolerance for Coolix protocols. * Add a unit test to confirm the library now matches the provided example that was previously slight out of tolerance. Fixes #1748 --- src/ir_Coolix.cpp | 54 ++++++++++++++++++++--------------------- test/ir_Coolix_test.cpp | 35 ++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 28 deletions(-) diff --git a/src/ir_Coolix.cpp b/src/ir_Coolix.cpp index 252d61a92..0358b89a5 100644 --- a/src/ir_Coolix.cpp +++ b/src/ir_Coolix.cpp @@ -32,7 +32,7 @@ const uint16_t kCoolixHdrSpaceTicks = 16; const uint16_t kCoolixHdrSpace = kCoolixHdrSpaceTicks * kCoolixTick; // 4416us const uint16_t kCoolixMinGapTicks = kCoolixHdrMarkTicks + kCoolixZeroSpaceTicks; const uint16_t kCoolixMinGap = kCoolixMinGapTicks * kCoolixTick; // 5244us -const uint8_t kCoolix48ExtraTolerance = 5; // Percent +const uint8_t kCoolixExtraTolerance = 5; // Percent using irutils::addBoolToString; using irutils::addIntToString; @@ -648,41 +648,39 @@ bool IRrecv::decodeCOOLIX(decode_results *results, uint16_t offset, return false; // We can't possibly capture a Coolix packet that big. // Header - if (!matchMark(results->rawbuf[offset], kCoolixHdrMark)) return false; - // Calculate how long the common tick time is based on the header mark. - uint32_t m_tick = results->rawbuf[offset++] * kRawTick / kCoolixHdrMarkTicks; - if (!matchSpace(results->rawbuf[offset], kCoolixHdrSpace)) return false; - // Calculate how long the common tick time is based on the header space. - uint32_t s_tick = results->rawbuf[offset++] * kRawTick / kCoolixHdrSpaceTicks; + if (!matchMark(results->rawbuf[offset++], kCoolixHdrMark)) return false; + if (!matchSpace(results->rawbuf[offset++], kCoolixHdrSpace)) return false; // Data // Twice as many bits as there are normal plus inverted bits. - for (uint16_t i = 0; i < nbits * 2; i++, offset++) { - bool flip = (i / 8) % 2; - if (!matchMark(results->rawbuf[offset++], kCoolixBitMarkTicks * m_tick)) - return false; - if (matchSpace(results->rawbuf[offset], kCoolixOneSpaceTicks * s_tick)) { - if (flip) - inverted = (inverted << 1) | 1; - else - data = (data << 1) | 1; - } else if (matchSpace(results->rawbuf[offset], - kCoolixZeroSpaceTicks * s_tick)) { - if (flip) - inverted <<= 1; - else - data <<= 1; + for (uint16_t i = 0; i < nbits * 2; i += 8) { + const bool flip = (i / 8) % 2; + uint64_t result = 0; + // Read the next byte of data. + const uint16_t used = matchGeneric(results->rawbuf + offset, &result, + results->rawlen - offset, 8, + 0, 0, // No Header + kCoolixBitMark, kCoolixOneSpace, // Data + kCoolixBitMark, kCoolixZeroSpace, + 0, 0, // No Footer + false, + _tolerance + kCoolixExtraTolerance, + 0, true); + if (!used) return false; // Didn't match a bytes worth of data. + offset += used; + if (flip) { // The inverted byte. + inverted <<= 8; + inverted |= result; } else { - return false; + data <<= 8; + data |= result; } } // Footer - if (!matchMark(results->rawbuf[offset++], kCoolixBitMarkTicks * m_tick)) - return false; + if (!matchMark(results->rawbuf[offset++], kCoolixBitMark)) return false; if (offset < results->rawlen && - !matchAtLeast(results->rawbuf[offset], kCoolixMinGapTicks * s_tick)) - return false; + !matchAtLeast(results->rawbuf[offset], kCoolixMinGap)) return false; // Compliance uint64_t orig = data; // Save a copy of the data. @@ -744,7 +742,7 @@ bool IRrecv::decodeCoolix48(decode_results *results, uint16_t offset, kCoolixBitMark, kCoolixOneSpace, kCoolixBitMark, kCoolixZeroSpace, kCoolixBitMark, kCoolixMinGap, - true, _tolerance + kCoolix48ExtraTolerance, 0, true)) + true, _tolerance + kCoolixExtraTolerance, 0, true)) return false; // Success diff --git a/test/ir_Coolix_test.cpp b/test/ir_Coolix_test.cpp index 62e0d6b0e..ba3610bc7 100644 --- a/test/ir_Coolix_test.cpp +++ b/test/ir_Coolix_test.cpp @@ -600,6 +600,41 @@ TEST(TestDecodeCoolix, RealCaptureExample) { EXPECT_EQ(0x0, irsend.capture.command); } +TEST(TestDecodeCoolix, Issue1748Example) { + IRsendTest irsend(kGpioUnused); + IRrecv irrecv(kGpioUnused); + + // Ref: https://github.com/crankyoldgit/IRremoteESP8266/issues/1748#issuecomment-1024907551 + const uint16_t powerOffRawData[199] = { + 4642, 4502, 514, 1706, 516, 624, 488, 1704, 514, 1702, 516, 624, 488, 624, + 488, 1702, 514, 626, 488, 620, 488, 1702, 490, 620, 512, 620, 488, 1728, + 488, 1704, 514, 624, 488, 1704, 514, 620, 488, 1702, 490, 1722, 488, 1724, + 514, 1728, 488, 600, 512, 1728, 490, 1706, 512, 1698, 488, 646, 486, 622, + 462, 646, 488, 624, 488, 1704, 514, 626, 488, 628, 460, 1724, 514, 1702, + 514, 1724, 462, 646, 488, 624, 488, 624, 488, 626, 486, 602, 488, 646, + 460, 648, 486, 626, 488, 1704, 486, 1724, 488, 1748, 488, 1704, 514, 1708, + 488, 5312, 4648, 4494, 488, 1704, 486, 646, 486, 1698, 512, 1700, 488, + 646, 462, 646, 486, 1728, 462, 648, 484, 622, 462, 1724, 510, 622, 488, + 626, 488, 1702, 514, 1728, 490, 626, 488, 1730, 462, 646, 488, 1704, 512, + 1724, 486, 1698, 514, 1728, 488, 626, 488, 1728, 488, 1704, 514, 1700, + 512, 620, 486, 620, 488, 620, 486, 626, 490, 1728, 488, 626, 488, 628, + 460, 1750, 488, 1728, 488, 1704, 488, 646, 488, 620, 488, 624, 488, 626, + 488, 626, 462, 646, 462, 644, 488, 626, 488, 1728, 490, 1704, 486, 1724, + 514, 1724, 488, 1728, 488}; // COOLIX48 B24D7B84E01F + + irsend.begin(); + + irsend.reset(); + + irsend.sendRaw(powerOffRawData, 199, 38000); + irsend.makeDecodeResult(); + ASSERT_TRUE(irrecv.decode(&irsend.capture)); + EXPECT_EQ(COOLIX, irsend.capture.decode_type); + EXPECT_EQ(kCoolixBits, irsend.capture.bits); + EXPECT_EQ(kCoolixOff, irsend.capture.value); + EXPECT_EQ(0x0, irsend.capture.address); + EXPECT_EQ(0x0, irsend.capture.command); +} // Tests to debug/fix: // https://github.com/crankyoldgit/IRremoteESP8266/issues/624