Skip to content

Commit

Permalink
Refactor decodeMitsubishiAC() (#1532)
Browse files Browse the repository at this point in the history 
* Use standard code methods to decode it rather than spaghetti.
* Remove Unit Test cases that tried to match bad data. We are just going to ignore bad signals from now on.
* Ensure repeated messages are identical.
* Do the message compliance tests earlier so if it isn't a valid message we can quit earlier.

Fixes #1523
  • Loading branch information
@crankyoldgit
crankyoldgit authored Jul 14, 2021
1 parent 178940c commit 2d4659c
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 238 deletions.
149 changes: 39 additions & 110 deletions src/ir_Mitsubishi.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,120 +257,49 @@ void IRsend::sendMitsubishiAC(const unsigned char data[], const uint16_t nbytes,
bool IRrecv::decodeMitsubishiAC(decode_results *results, uint16_t offset,
const uint16_t nbits,
const bool strict) {
if (results->rawlen <= ((kMitsubishiACBits * 2) + 2) + offset) {
DPRINTLN("Shorter than shortest possibly expected.");
return false; // Shorter than shortest possibly expected.
}
if (strict && nbits != kMitsubishiACBits) {
DPRINTLN("Request is out of spec.");
return false; // Request is out of spec.
}
for (uint8_t i = 0; i < kMitsubishiACStateLength; i++) results->state[i] = 0;
bool failure = false;
uint8_t rep = 0;
do {
failure = false;
// Header:
// Sometime happens that junk signals arrives before the real message
bool headerFound = false;
while (!headerFound &&
offset < (results->rawlen - (kMitsubishiACBits * 2 + 2))) {
headerFound =
matchMark(results->rawbuf[offset], kMitsubishiAcHdrMark) &&
matchSpace(results->rawbuf[offset + 1], kMitsubishiAcHdrSpace);
offset += 2;
}
if (!headerFound) {
DPRINTLN("Header mark not found.");
return false;
}
DPRINT("Header mark found at #");
DPRINTLN(offset - 2);
// Decode byte-by-byte:
match_result_t data_result;
for (uint8_t i = 0; i < kMitsubishiACStateLength && !failure; i++) {
results->state[i] = 0;
data_result =
matchData(&(results->rawbuf[offset]), 8, kMitsubishiAcBitMark,
kMitsubishiAcOneSpace, kMitsubishiAcBitMark,
kMitsubishiAcZeroSpace,
_tolerance + kMitsubishiAcExtraTolerance, 0, false);
if (data_result.success == false) {
failure = true;
DPRINT("Byte decode failed at #");
DPRINTLN((uint16_t)i);
} else {
results->state[i] = data_result.data;
offset += data_result.used;
DPRINT((uint16_t)results->state[i]);
DPRINT(",");
}
DPRINTLN("");
}
// HEADER validation:
if (failure || results->state[0] != 0x23 || results->state[1] != 0xCB ||
results->state[2] != 0x26 || results->state[3] != 0x01 ||
results->state[4] != 0x00) {
DPRINTLN("Header mismatch.");
failure = true;
} else {
// DATA part:

// FOOTER checksum:
if (!IRMitsubishiAC::validChecksum(results->state)) {
DPRINTLN("Checksum error.");
failure = true;
}
}
if (rep != kMitsubishiACMinRepeat && failure) {
bool repeatMarkFound = false;
while (!repeatMarkFound &&
offset < (results->rawlen - (kMitsubishiACBits * 2 + 4))) {
repeatMarkFound =
matchMark(results->rawbuf[offset], kMitsubishiAcRptMark) &&
matchSpace(results->rawbuf[offset + 1], kMitsubishiAcRptSpace);
offset += 2;
}
if (!repeatMarkFound) {
DPRINTLN("First attempt failure and repeat mark not found.");
return false;
// Compliance
if (strict && nbits != kMitsubishiACBits) return false; // Out of spec.
// Do we need to look for a repeat?
const uint16_t expected_repeats = strict ? kMitsubishiACMinRepeat : kNoRepeat;
// Enough data?
if (results->rawlen <= (nbits * 2 + kHeader + kFooter) *
(expected_repeats + 1) + offset - 1) return false;
uint16_t save[kStateSizeMax];
// Handle repeats if we need too.
for (uint16_t r = 0; r <= expected_repeats; r++) {
// Header + Data + Footer
uint16_t used = matchGeneric(results->rawbuf + offset, results->state,
results->rawlen - offset, nbits,
kMitsubishiAcHdrMark, kMitsubishiAcHdrSpace,
kMitsubishiAcBitMark, kMitsubishiAcOneSpace,
kMitsubishiAcBitMark, kMitsubishiAcZeroSpace,
kMitsubishiAcRptMark, kMitsubishiAcRptSpace,
r < expected_repeats, // At least?
_tolerance + kMitsubishiAcExtraTolerance,
0, false);
if (!used) return false; // No match.
offset += used;
if (r) { // Is this a repeat?
// Repeats are expected to be exactly the same.
if (std::memcmp(save, results->state, nbits / 8) != 0) return false;
} else { // It is the first message.
// Compliance
if (strict) {
// Data signature check.
static const uint8_t signature[5] = {0x23, 0xCB, 0x26, 0x01, 0x00};
if (std::memcmp(results->state, signature, 5) != 0) return false;
// Checksum verification.
if (!IRMitsubishiAC::validChecksum(results->state)) return false;
}
// Save a copy of the state to compare with.
std::memcpy(save, results->state, nbits / 8);
}
rep++;
// Check if the repeat is correct if we need strict decode:
if (strict && !failure) {
DPRINTLN("Strict repeat check enabled.");
// Repeat mark and space:
if (!matchMark(results->rawbuf[offset++], kMitsubishiAcRptMark) ||
!matchSpace(results->rawbuf[offset++], kMitsubishiAcRptSpace)) {
DPRINTLN("Repeat mark error.");
return false;
}
// Header mark and space:
if (!matchMark(results->rawbuf[offset++], kMitsubishiAcHdrMark) ||
!matchSpace(results->rawbuf[offset++], kMitsubishiAcHdrSpace)) {
DPRINTLN("Repeat header error.");
return false;
}
// Payload:
for (uint8_t i = 0; i < kMitsubishiACStateLength; i++) {
data_result =
matchData(&(results->rawbuf[offset]), 8, kMitsubishiAcBitMark,
kMitsubishiAcOneSpace, kMitsubishiAcBitMark,
kMitsubishiAcZeroSpace,
_tolerance + kMitsubishiAcExtraTolerance, 0, false);
if (data_result.success == false ||
data_result.data != results->state[i]) {
DPRINTLN("Repeat payload error.");
return false;
}
offset += data_result.used;
}
} // strict repeat check
} while (failure && rep <= kMitsubishiACMinRepeat);
}

// Success.
results->decode_type = MITSUBISHI_AC;
results->bits = nbits;
return !failure;
return true;
}
#endif // DECODE_MITSUBISHI_AC

Expand Down
128 changes: 0 additions & 128 deletions test/ir_Mitsubishi_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -824,134 +824,6 @@ TEST(TestDecodeMitsubishiAC, DecodeRealExample) {
EXPECT_STATE_EQ(expected, irsend.capture.state, kMitsubishiACBits);
}

// Tests for decodeMitsubishiAC() when the first payload has an error.
TEST(TestDecodeMitsubishiAC, DecodeRealExampleRepeatNeeded) {
IRsendTest irsend(kGpioUnused);
IRrecv irrecv(kGpioUnused);
irsend.begin();

irsend.reset();
// Mitsubishi AC "Power On, 16C, low fan, vane auto move".
uint16_t rawData[583] = {
3476, 1708, 416, 1264, 420, 1260, 414, 400, 448, 390, 446, 392, 444, 1236,
440, 400, 446, 392, 446, 1234, 440, 1266, 418, 396, 442, 1264, 420, 394,
444, 394, 442, 1264, 422, 1260, 414, 398, 440, 1266, 418, 1264, 420, 392,
446, 392, 444, 1264, 422, 392, 446, 392, 446, 1260, 414, 400, 448, 390,
446, 392, 444, 394, 442, 396, 442, 398, 440, 424, 412, 400, 448, 390, 446,
392, 446, 392, 444, 394, 442, 396, 442, 396, 440, 398, 438, 400, 448, 390,
446, 392, 446, 392, 444, 396, 442, 396, 440, 398, 440, 400, 438, 400, 448,
390, 446, 392, 444, 1236, 440, 1266, 418, 394, 442, 396, 440, 398, 438,
402, 446, 1232, 444, 396, 440, 1268, 418, 394, 442, 396, 440, 398,
// space 699 is not recognizable:
440, 400, 448, 390, 448, 1232, 442, 1266, 420, 394, 444, 1264, 699, 1260,
416, 396, 440, 398, 450, 1230, 444, 396, 442, 398, 440, 1266, 418, 1264,
422, 1258, 416, 1266, 418, 394, 442, 396, 440, 398, 440, 398, 438, 400,
446, 392, 446, 392, 446, 392, 444, 396, 442, 396, 440, 398, 438, 398, 438,
400, 448, 392, 446, 392, 444, 394, 444, 396, 442, 396, 440, 398, 438, 400,
448, 390, 448, 392, 444, 394, 444, 394, 442, 396, 442, 396, 440, 398, 438,
400, 448, 390, 446, 392, 446, 392, 444, 394, 442, 396, 442, 396, 440, 398,
438, 400, 448, 390, 446, 392, 444, 394, 444, 394, 442, 396, 440, 398, 440,
398, 438, 400, 448, 390, 446, 392, 444, 394, 444, 394, 442, 396, 440, 398,
438, 400, 438, 400, 448, 392, 446, 392, 444, 394, 442, 396, 442, 396, 440,
398, 438, 1240, 444, 1264, 422, 390, 446, 392, 446, 1260, 414, 1268, 418,
1264, 422, 12984, 3478, 1708, 418, 1264, 422, 1234, 442, 398, 448, 390,
446, 392, 446, 1234, 440, 400, 448, 390, 446, 1234, 442, 1266, 420, 392,
444, 1264, 420, 392, 446, 394, 444, 1236, 448, 1260, 416, 398, 440, 1266,
418, 1262, 422, 390, 446, 392, 444, 1234, 440, 400, 448, 392, 446, 1234,
440, 398, 450, 390, 446, 392, 444, 394, 444, 394, 442, 396, 442, 398, 440,
400, 438, 400, 448, 390, 446, 392, 444, 394, 442, 396, 442, 396, 440, 398,
438, 400, 448, 390, 446, 392, 446, 392, 444, 394, 442, 396, 442, 396, 440,
398, 438, 400, 448, 416, 420, 392, 444, 1234, 440, 1240, 446, 394, 442,
396, 440, 398, 438, 400, 448, 1232, 444, 396, 440, 1240, 446, 394, 442,
396, 440, 398, 440, 400, 448, 390, 446, 1232, 444, 1238, 446, 394, 444,
1236, 448, 1232, 442, 396, 440, 398, 448, 1232, 444, 396, 440, 398, 438,
1242, 444, 1238, 448, 1234, 442, 1240, 444, 394, 442, 396, 440, 398, 438,
400, 448, 390, 446, 394, 444, 420, 416, 394, 444, 396, 440, 398, 440, 398,
438, 400, 448, 418, 420, 418, 418, 394, 442, 396, 442, 396, 440, 424, 412,
400, 448, 390, 446, 392, 446, 420, 418, 420, 416, 396, 440, 398, 440, 424,
412, 426, 420, 418, 420, 392, 444, 394, 444, 422, 416, 422, 414, 398, 440,
426, 422, 388, 448, 416, 420, 418, 418, 422, 416, 422, 414, 424, 414, 398,
438, 426, 422, 418, 420, 390, 446, 418, 418, 420, 416, 396, 440, 424, 412,
426, 412, 400, 446, 418, 420, 420, 418, 420, 416, 422, 414, 422, 414, 424,
412, 426, 422, 390, 446, 1232, 442, 1240, 446, 394, 444, 394, 442, 1238,
446, 1234, 440, 1240, 444}; // UNKNOWN F6FDB82B

irsend.sendRaw(rawData, 583, 33);
irsend.makeDecodeResult();

ASSERT_TRUE(irrecv.decode(&irsend.capture));
EXPECT_EQ(MITSUBISHI_AC, irsend.capture.decode_type);
EXPECT_EQ(kMitsubishiACBits, irsend.capture.bits);
uint8_t expected[kMitsubishiACStateLength] = {
0x23, 0xCB, 0x26, 0x01, 0x00, 0x00, 0x18, 0x0A, 0x36,
0x79, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xE6};
EXPECT_STATE_EQ(expected, irsend.capture.state, kMitsubishiACBits);
}

// Tests for decodeMitsubishiAC() when the repeat mark is wrong.
TEST(TestDecodeMitsubishiAC, DecodeRealExampleRepeatMarkError) {
IRsendTest irsend(kGpioUnused);
IRrecv irrecv(kGpioUnused);
irsend.begin();

irsend.reset();
// Mitsubishi AC "Power On, 16C, low fan, vane auto move".
uint16_t rawData[583] = {
3476, 1708, 416, 1264, 420, 1260, 414, 400, 448, 390, 446, 392, 444, 1236,
440, 400, 446, 392, 446, 1234, 440, 1266, 418, 396, 442, 1264, 420, 394,
444, 394, 442, 1264, 422, 1260, 414, 398, 440, 1266, 418, 1264, 420, 392,
446, 392, 444, 1264, 422, 392, 446, 392, 446, 1260, 414, 400, 448, 390,
446, 392, 444, 394, 442, 396, 442, 398, 440, 424, 412, 400, 448, 390, 446,
392, 446, 392, 444, 394, 442, 396, 442, 396, 440, 398, 438, 400, 448, 390,
446, 392, 446, 392, 444, 396, 442, 396, 440, 398, 440, 400, 438, 400, 448,
390, 446, 392, 444, 1236, 440, 1266, 418, 394, 442, 396, 440, 398, 438,
402, 446, 1232, 444, 396, 440, 1268, 418, 394, 442, 396, 440, 398, 440,
400, 448, 390, 448, 1232, 442, 1266, 420, 394, 444, 1264, 422, 1260, 416,
396, 440, 398, 450, 1230, 444, 396, 442, 398, 440, 1266, 418, 1264, 422,
1258, 416, 1266, 418, 394, 442, 396, 440, 398, 440, 398, 438, 400, 446,
392, 446, 392, 446, 392, 444, 396, 442, 396, 440, 398, 438, 398, 438, 400,
448, 392, 446, 392, 444, 394, 444, 396, 442, 396, 440, 398, 438, 400, 448,
390, 448, 392, 444, 394, 444, 394, 442, 396, 442, 396, 440, 398, 438, 400,
448, 390, 446, 392, 446, 392, 444, 394, 442, 396, 442, 396, 440, 398, 438,
400, 448, 390, 446, 392, 444, 394, 444, 394, 442, 396, 440, 398, 440, 398,
438, 400, 448, 390, 446, 392, 444, 394, 444, 394, 442, 396, 440, 398, 438,
400, 438, 400, 448, 392, 446, 392, 444, 394, 442, 396, 442, 396, 440, 398,
438, 1240, 444, 1264, 422, 390, 446, 392, 446,
// Repeat mark (1111) wrong:
1260, 414, 1268, 418, 1264, 422, 1111, 347, 1708, 418, 1264, 422, 1234,
442, 398, 448, 390, 446, 392, 446, 1234, 440, 400, 448, 390, 446, 1234,
442, 1266, 420, 392, 444, 1264, 420, 392, 446, 394, 444, 1236, 448, 1260,
416, 398, 440, 1266, 418, 1262, 422, 390, 446, 392, 444, 1234, 440, 400,
448, 392, 446, 1234, 440, 398, 450, 390, 446, 392, 444, 394, 444, 394,
442, 396, 442, 398, 440, 400, 438, 400, 448, 390, 446, 392, 444, 394, 442,
396, 442, 396, 440, 398, 438, 400, 448, 390, 446, 392, 446, 392, 444, 394,
442, 396, 442, 396, 440, 398, 438, 400, 448, 416, 420, 392, 444, 1234,
440, 1240, 446, 394, 442, 396, 440, 398, 438, 400, 448, 1232, 444, 396,
440, 1240, 446, 394, 442, 396, 440, 398, 440, 400, 448, 390, 446, 1232,
444, 1238, 446, 394, 444, 1236, 448, 1232, 442, 396, 440, 398, 448, 1232,
444, 396, 440, 398, 438, 1242, 444, 1238, 448, 1234, 442, 1240, 444, 394,
442, 396, 440, 398, 438, 400, 448, 390, 446, 394, 444, 420, 416, 394, 444,
396, 440, 398, 440, 398, 438, 400, 448, 418, 420, 418, 418, 394, 442, 396,
442, 396, 440, 424, 412, 400, 448, 390, 446, 392, 446, 420, 418, 420, 416,
396, 440, 398, 440, 424, 412, 426, 420, 418, 420, 392, 444, 394, 444, 422,
416, 422, 414, 398, 440, 426, 422, 388, 448, 416, 420, 418, 418, 422, 416,
422, 414, 424, 414, 398, 438, 426, 422, 418, 420, 390, 446, 418, 418, 420,
416, 396, 440, 424, 412, 426, 412, 400, 446, 418, 420, 420, 418, 420, 416,
422, 414, 422, 414, 424, 412, 426, 422, 390, 446, 1232, 442, 1240, 446,
394, 444, 394, 442, 1238, 446, 1234, 440, 1240, 444}; // UNKNOWN F6FDB82B

irsend.sendRaw(rawData, 583, 33);
irsend.makeDecodeResult();

ASSERT_TRUE(irrecv.decode(&irsend.capture));
EXPECT_EQ(MITSUBISHI_AC, irsend.capture.decode_type);
EXPECT_EQ(kMitsubishiACBits, irsend.capture.bits);
uint8_t expected[kMitsubishiACStateLength] = {
0x23, 0xCB, 0x26, 0x01, 0x00, 0x00, 0x18, 0x0A, 0x36,
0x79, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xE6};
EXPECT_STATE_EQ(expected, irsend.capture.state, kMitsubishiACBits);
}

// Tests for decodeMitsubishiAC() when first payload has an error and the
// repeat mark is wrong.
TEST(TestDecodeMitsubishiAC, DecodeRealExampleRepeatNeededButError) {
Expand Down

0 comments on commit 2d4659c

Please sign in to comment.