From 142a9501f4d6809613527b02257d9b69c39aab7d Mon Sep 17 00:00:00 2001 From: crankyoldgit Date: Thu, 12 Aug 2021 23:30:00 +1000 Subject: [PATCH] SamsungAc: Use `sendExtended()` going forward. * As the `checksum()` calculation for extended states has been fixed in #1554, use `sendExtended()` to calculate & send the extended state instead of the `sendOn()` & `sendOff()`. - This should allow for custom off messages. e.g. clean. - It should also reduce the number of messages sent, and the beeps generated by the A/C on receiving the message. - And make the intended transaction quicker. * Update/adjust unit tests accordingly. * Refactored `sendExtended()` - Use `memcpy()`. - Use less stack space / memory. For #1484 --- src/ir_Samsung.cpp | 54 ++++++++++++++++----------------- test/IRac_test.cpp | 9 ++---- test/ir_Samsung_test.cpp | 65 +++++++++++++++++----------------------- 3 files changed, 57 insertions(+), 71 deletions(-) diff --git a/src/ir_Samsung.cpp b/src/ir_Samsung.cpp index e811af758..9e1ad4669 100644 --- a/src/ir_Samsung.cpp +++ b/src/ir_Samsung.cpp @@ -280,8 +280,8 @@ IRSamsungAc::IRSamsungAc(const uint16_t pin, const bool inverted, /// @param[in] initialPower Set the initial power state. True, on. False, off. void IRSamsungAc::stateReset(const bool forcepower, const bool initialPower) { static const uint8_t kReset[kSamsungAcExtendedStateLength] = { - 0x02, 0x92, 0x0F, 0x00, 0x00, 0x00, 0xF0, 0x01, 0x02, 0xAE, 0x71, 0x00, - 0x15, 0xF0}; + 0x02, 0x92, 0x0F, 0x00, 0x00, 0x00, 0xF0, + 0x01, 0x02, 0xAE, 0x71, 0x00, 0x15, 0xF0}; std::memcpy(_.raw, kReset, kSamsungAcExtendedStateLength); _forcepower = forcepower; _lastsentpowerstate = initialPower; @@ -355,18 +355,14 @@ void IRSamsungAc::checksum(void) { /// @note Use for most function/mode/settings changes to the unit. /// i.e. When the device is already running. void IRSamsungAc::send(const uint16_t repeat, const bool calcchecksum) { - if (calcchecksum) checksum(); - // Do we need to send a the special power on/off message? - if (getPower() != _lastsentpowerstate || _forcepower) { - _forcepower = false; // It will now been sent, so clear the flag if set. - if (getPower()) { - sendOn(repeat); - } else { - sendOff(repeat); - return; // No point sending anything else if we are turning the unit off. - } + // Do we need to send a the special power on/off message? i.e. An Extended Msg + if (getPower() != _lastsentpowerstate || _forcepower) { // We do. + sendExtended(repeat, calcchecksum); + _forcepower = false; // It has now been sent, so clear the flag if set. + } else { // No, it's just a normal message. + if (calcchecksum) checksum(); + _irsend.sendSamsungAC(_.raw, kSamsungAcStateLength, repeat); } - _irsend.sendSamsungAC(_.raw, kSamsungAcStateLength, repeat); } /// Send the extended current internal state as an IR message. @@ -376,20 +372,24 @@ void IRSamsungAc::send(const uint16_t repeat, const bool calcchecksum) { /// Samsung A/C requires an extended length message when you want to /// change the power operating mode of the A/C unit. void IRSamsungAc::sendExtended(const uint16_t repeat, const bool calcchecksum) { + static const uint8_t extended_middle_section[kSamsungAcSectionLength] = { + 0x01, 0xD2, 0x0F, 0x00, 0x00, 0x00, 0x00}; if (calcchecksum) checksum(); - uint8_t extended_state[kSamsungAcExtendedStateLength] = { - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x01, 0xD2, 0x0F, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; - // Copy/convert the internal state to an extended state. - for (uint16_t i = 0; i < kSamsungAcSectionLength; i++) - extended_state[i] = _.raw[i]; - for (uint16_t i = kSamsungAcSectionLength; i < kSamsungAcStateLength; i++) - extended_state[i + kSamsungAcSectionLength] = _.raw[i]; - // extended_state[8] seems special. This is a guess on how to calculate it. - extended_state[8] = (extended_state[1] & 0x9F) | 0x40; + // Copy/convert the internal state to an extended state by + // copying the second section to the third section, and inserting the extended + // middle (second) section. + std::memcpy(_.raw + 2 * kSamsungAcSectionLength, + _.raw + kSamsungAcSectionLength, + kSamsungAcSectionLength); + std::memcpy(_.raw + kSamsungAcSectionLength, extended_middle_section, + kSamsungAcSectionLength); // Send it. - _irsend.sendSamsungAC(extended_state, kSamsungAcExtendedStateLength, repeat); + _irsend.sendSamsungAC(_.raw, kSamsungAcExtendedStateLength, repeat); + // Now revert it by copying the third section over the second section. + std::memcpy(_.raw + kSamsungAcSectionLength, + _.raw + 2* kSamsungAcSectionLength, + kSamsungAcSectionLength); + _lastsentpowerstate = getPower(); // Remember the last power state sent. } /// Send the special extended "On" message as the library can't seem to @@ -397,7 +397,7 @@ void IRSamsungAc::sendExtended(const uint16_t repeat, const bool calcchecksum) { /// @param[in] repeat Nr. of times the message will be repeated. /// @see https://github.com/crankyoldgit/IRremoteESP8266/issues/604#issuecomment-475020036 void IRSamsungAc::sendOn(const uint16_t repeat) { - const uint8_t extended_state[21] = { + const uint8_t extended_state[kSamsungAcExtendedStateLength] = { 0x02, 0x92, 0x0F, 0x00, 0x00, 0x00, 0xF0, 0x01, 0xD2, 0x0F, 0x00, 0x00, 0x00, 0x00, 0x01, 0xE2, 0xFE, 0x71, 0x80, 0x11, 0xF0}; @@ -410,7 +410,7 @@ void IRSamsungAc::sendOn(const uint16_t repeat) { /// @param[in] repeat Nr. of times the message will be repeated. /// @see https://github.com/crankyoldgit/IRremoteESP8266/issues/604#issuecomment-475020036 void IRSamsungAc::sendOff(const uint16_t repeat) { - const uint8_t extended_state[21] = { + const uint8_t extended_state[kSamsungAcExtendedStateLength] = { 0x02, 0xB2, 0x0F, 0x00, 0x00, 0x00, 0xC0, 0x01, 0xD2, 0x0F, 0x00, 0x00, 0x00, 0x00, 0x01, 0x02, 0xFF, 0x71, 0x80, 0x11, 0xC0}; diff --git a/test/IRac_test.cpp b/test/IRac_test.cpp index cc6de5a2f..9aa29e14a 100644 --- a/test/IRac_test.cpp +++ b/test/IRac_test.cpp @@ -1514,14 +1514,9 @@ TEST(TestIRac, Samsung) { ac._irsend.makeDecodeResult(); EXPECT_TRUE(capture.decode(&ac._irsend.capture)); ASSERT_EQ(SAMSUNG_AC, ac._irsend.capture.decode_type); + // We expect an extended state because of `dopower`. ASSERT_EQ(kSamsungAcExtendedBits, ac._irsend.capture.bits); - // However, we expect a plain "on" state as it should be sent before the - // desired state. - char expected_on[] = - "Power: On, Mode: 1 (Cool), Temp: 24C, Fan: 0 (Auto), Swing: Off, " - "Beep: Off, Clean: Off, Quiet: Off, Powerful: Off, Breeze: Off, " - "Light: On, Ion: Off"; - ASSERT_EQ(expected_on, IRAcUtils::resultAcToString(&ac._irsend.capture)); + ASSERT_EQ(expected, IRAcUtils::resultAcToString(&ac._irsend.capture)); ASSERT_TRUE(IRAcUtils::decodeToState(&ac._irsend.capture, &r, &p)); } diff --git a/test/ir_Samsung_test.cpp b/test/ir_Samsung_test.cpp index 9e47a537a..a4e0503cd 100644 --- a/test/ir_Samsung_test.cpp +++ b/test/ir_Samsung_test.cpp @@ -1270,6 +1270,8 @@ TEST(TestDecodeSamsung36, SyntheticExample) { } // https://github.com/crankyoldgit/IRremoteESP8266/issues/604 +// This has been superceeded in a way by the ability to calculate extended msg +// checksums correctly. See #1484, #1538, & #1554 TEST(TestIRSamsungAcClass, Issue604SendPowerHack) { IRSamsungAc ac(0); ac.begin(); @@ -1277,7 +1279,7 @@ TEST(TestIRSamsungAcClass, Issue604SendPowerHack) { std::string freqduty = "f38000d50"; - std::string poweron = + std::string settings_section1 = "m690s17844" "m3086s8864" "m586s436m586s1432m586s436m586s436m586s436m586s436m586s436m586s436" @@ -1287,45 +1289,27 @@ TEST(TestIRSamsungAcClass, Issue604SendPowerHack) { "m586s436m586s436m586s436m586s436m586s436m586s436m586s436m586s436" "m586s436m586s436m586s436m586s436m586s436m586s436m586s436m586s436" "m586s436m586s436m586s436m586s436m586s1432m586s1432m586s1432m586s1432" - "m586s2886" + "m586s2886"; + std::string settings_section2 = "m3086s8864" "m586s1432m586s436m586s436m586s436m586s436m586s436m586s436m586s436" "m586s436m586s1432m586s436m586s436m586s1432m586s436m586s1432m586s1432" - "m586s1432m586s1432m586s1432m586s1432m586s436m586s436m586s436m586s436" - "m586s436m586s436m586s436m586s436m586s436m586s436m586s436m586s436" - "m586s436m586s436m586s436m586s436m586s436m586s436m586s436m586s436" - "m586s436m586s436m586s436m586s436m586s436m586s436m586s436m586s436" - "m586s436m586s436m586s436m586s436m586s436m586s436m586s436m586s436" - "m586s2886" - "m3086s8864" - "m586s1432m586s436m586s436m586s436m586s436m586s436m586s436m586s436" - "m586s436m586s1432m586s436m586s436m586s436m586s1432m586s1432m586s1432" - "m586s436m586s1432m586s1432m586s1432m586s1432m586s1432m586s1432m586s1432" + "m586s436m586s1432m586s1432m586s1432m586s436m586s1432m586s436m586s1432" "m586s1432m586s436m586s436m586s436m586s1432m586s1432m586s1432m586s436" - "m586s436m586s436m586s436m586s436m586s436m586s436m586s436m586s1432" - "m586s1432m586s436m586s436m586s436m586s1432m586s436m586s436m586s436" + "m586s436m586s436m586s436m586s436m586s1432m586s1432m586s1432m586s436" + "m586s1432m586s436m586s436m586s1432m586s1432m586s436m586s436m586s436" "m586s436m586s436m586s436m586s436m586s1432m586s1432m586s1432m586s1432" "m586s100000"; - std::string settings = - "m690s17844" + std::string extended_section = "m3086s8864" - "m586s436m586s1432m586s436m586s436m586s436m586s436m586s436m586s436" - "m586s436m586s1432m586s436m586s436m586s1432m586s436m586s436m586s1432" + "m586s1432m586s436m586s436m586s436m586s436m586s436m586s436m586s436" + "m586s436m586s1432m586s436m586s436m586s1432m586s436m586s1432m586s1432" "m586s1432m586s1432m586s1432m586s1432m586s436m586s436m586s436m586s436" "m586s436m586s436m586s436m586s436m586s436m586s436m586s436m586s436" "m586s436m586s436m586s436m586s436m586s436m586s436m586s436m586s436" "m586s436m586s436m586s436m586s436m586s436m586s436m586s436m586s436" - "m586s436m586s436m586s436m586s436m586s1432m586s1432m586s1432m586s1432" - "m586s2886" - "m3086s8864" - "m586s1432m586s436m586s436m586s436m586s436m586s436m586s436m586s436" - "m586s436m586s1432m586s436m586s436m586s1432m586s436m586s1432m586s1432" - "m586s436m586s1432m586s1432m586s1432m586s436m586s1432m586s436m586s1432" - "m586s1432m586s436m586s436m586s436m586s1432m586s1432m586s1432m586s436" - "m586s436m586s436m586s436m586s436m586s1432m586s1432m586s1432m586s436" - "m586s1432m586s436m586s436m586s1432m586s1432m586s436m586s436m586s436" - "m586s436m586s436m586s436m586s436m586s1432m586s1432m586s1432m586s1432" - "m586s100000"; + "m586s436m586s436m586s436m586s436m586s436m586s436m586s436m586s436" + "m586s2886"; std::string text = "Power: On, Mode: 1 (Cool), Temp: 23C, Fan: 4 (Med), " "Swing: On, Beep: Off, Clean: Off, Quiet: Off, " "Powerful: Off, Breeze: Off, Light: On, Ion: Off"; @@ -1336,33 +1320,38 @@ TEST(TestIRSamsungAcClass, Issue604SendPowerHack) { ac.setFan(kSamsungAcFanMed); ac.send(); EXPECT_EQ(text, ac.toString()); - EXPECT_EQ(freqduty + settings, ac._irsend.outputStr()); + EXPECT_EQ(freqduty + settings_section1 + settings_section2, + ac._irsend.outputStr()); // Ensure the power state is changed by changing it and sending it. ac.off(); ac.send(); ac._irsend.reset(); // Clear the capture buffer. // Now trigger a special power message by using a power method. ac.on(); - ac.send(); // This should result in two messages. 1 x extended + 1 x normal. + ac.send(); // This should result in an extended message. EXPECT_EQ(text, ac.toString()); - EXPECT_EQ(freqduty + poweron + settings, ac._irsend.outputStr()); + EXPECT_EQ(freqduty + settings_section1 + extended_section + settings_section2, + ac._irsend.outputStr()); ac._irsend.reset(); // Clear the capture buffer. // Subsequent sending should be just the "settings" message. ac.send(); EXPECT_EQ(text, ac.toString()); - EXPECT_EQ(freqduty + settings, ac._irsend.outputStr()); + EXPECT_EQ(freqduty + settings_section1 + settings_section2, + ac._irsend.outputStr()); ac._irsend.reset(); // Clear the capture buffer. ac.setPower(true); // Note: The power state hasn't changed from previous. ac.send(); // This should result in a normal setting message. EXPECT_EQ(text, ac.toString()); - EXPECT_EQ(freqduty + settings, ac._irsend.outputStr()); + EXPECT_EQ(freqduty + settings_section1 + settings_section2, + ac._irsend.outputStr()); ac._irsend.reset(); // Clear the capture buffer. ac.setPower(false); ac.setPower(true); // Note: The power state hasn't changed from the last sent ac.send(); // This should result in a normal setting message. EXPECT_EQ(text, ac.toString()); - EXPECT_EQ(freqduty + settings, ac._irsend.outputStr()); + EXPECT_EQ(freqduty + settings_section1 + settings_section2, + ac._irsend.outputStr()); ac.stateReset(); // Normal `stateReset` defaults to send the power message // on first send. @@ -1372,11 +1361,13 @@ TEST(TestIRSamsungAcClass, Issue604SendPowerHack) { ac.setFan(kSamsungAcFanMed); ac.send(); EXPECT_EQ(text, ac.toString()); - EXPECT_EQ(freqduty + poweron + settings, ac._irsend.outputStr()); + EXPECT_EQ(freqduty + settings_section1 + extended_section + settings_section2, + ac._irsend.outputStr()); ac._irsend.reset(); // Clear the capture buffer. ac.send(); // Subsequent send() should just be a settings message. EXPECT_EQ(text, ac.toString()); - EXPECT_EQ(freqduty + settings, ac._irsend.outputStr()); + EXPECT_EQ(freqduty + settings_section1 + settings_section2, + ac._irsend.outputStr()); } TEST(TestIRSamsungAcClass, toCommon) {