Skip to content

Commit

Permalink
Refactor decodeCOOLIX() code & add another test case. (#1750)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@crankyoldgit
crankyoldgit authored Jan 29, 2022
1 parent f8e5662 commit c5b36c8
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 28 deletions.
54 changes: 26 additions & 28 deletions src/ir_Coolix.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
35 changes: 35 additions & 0 deletions test/ir_Coolix_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit c5b36c8

Please sign in to comment.