From 056a0d00aec12ad953b12d86b07de504aa1d325d Mon Sep 17 00:00:00 2001 From: David Conran Date: Sun, 23 Jun 2019 17:10:07 +1000 Subject: [PATCH] Handle A/Cs with toggles better. (#758) * Add class method to send from a state_t type. - If given the previous state, try to handle toggles intelligently. * Move compare state function out of example code into class library. * Unit tests covering the new functionality. --- examples/IRMQTTServer/IRMQTTServer.ino | 19 +---- src/IRac.cpp | 70 ++++++++++++++++ src/IRac.h | 5 +- test/IRac_test.cpp | 107 +++++++++++++++++++++++++ 4 files changed, 183 insertions(+), 18 deletions(-) diff --git a/examples/IRMQTTServer/IRMQTTServer.ino b/examples/IRMQTTServer/IRMQTTServer.ino index c6a9e55e8..fd3bda2d8 100644 --- a/examples/IRMQTTServer/IRMQTTServer.ino +++ b/examples/IRMQTTServer/IRMQTTServer.ino @@ -2639,7 +2639,7 @@ void loop(void) { if (lockMqttBroadcast && statListenTime.elapsed() > kStatListenPeriodMs) { unsubscribing(MqttClimateStat + '+'); mqttLog("Finished listening for previous state."); - if (cmpClimate(climate, climate_prev)) { // Something changed. + if (IRac::cmpStates(climate, climate_prev)) { // Something changed. mqttLog("The state was recovered from MQTT broker. Updating."); sendClimate(climate_prev, climate, MqttClimateStat, true, false, false); @@ -3262,17 +3262,6 @@ stdAc::state_t updateClimate(stdAc::state_t current, const String str, return result; } -// Compare two AirCon states (climates). -// Returns: True if they differ, False if they don't. -bool cmpClimate(const stdAc::state_t a, const stdAc::state_t b) { - return a.protocol != b.protocol || a.model != b.model || a.power != b.power || - a.mode != b.mode || a.degrees != b.degrees || a.celsius != b.celsius || - a.fanspeed != b.fanspeed || a.swingv != b.swingv || - a.swingh != b.swingh || a.quiet != b.quiet || a.turbo != b.turbo || - a.econo != b.econo || a.light != b.light || a.filter != b.filter || - a.clean != b.clean || a.beep != b.beep || a.sleep != b.sleep; -} - bool sendClimate(const stdAc::state_t prev, const stdAc::state_t next, const String topic_prefix, const bool retain, const bool forceMQTT, const bool forceIR, @@ -3367,11 +3356,7 @@ bool sendClimate(const stdAc::state_t prev, const stdAc::state_t next, // Turn IR capture off if we need to. if (irrecv != NULL) irrecv->disableIRIn(); // Stop the IR receiver #endif // IR_RX && DISABLE_CAPTURE_WHILE_TRANSMITTING - lastClimateSucceeded = commonAc->sendAc( - next.protocol, next.model, next.power, next.mode, - next.degrees, next.celsius, next.fanspeed, next.swingv, next.swingh, - next.quiet, next.turbo, next.econo, next.light, next.filter, next.clean, - next.beep, next.sleep, -1); + lastClimateSucceeded = commonAc->sendAc(next, &prev); #if IR_RX && DISABLE_CAPTURE_WHILE_TRANSMITTING // Turn IR capture back on if we need to. if (irrecv != NULL) irrecv->enableIRIn(); // Restart the receiver diff --git a/src/IRac.cpp b/src/IRac.cpp index 5d83ca35c..29010690e 100644 --- a/src/IRac.cpp +++ b/src/IRac.cpp @@ -789,6 +789,48 @@ void IRac::whirlpool(IRWhirlpoolAc *ac, const whirlpool_ac_remote_model_t model, } #endif // SEND_WHIRLPOOL_AC +// Create a new state base on desired & previous states but handle +// any state changes for options that need to be toggled. +// Args: +// desired: The state_t structure describing the desired a/c state. +// prev: Ptr to the previous state_t structure. +// +// Returns: +// A stdAc::state_t with the needed settings. +stdAc::state_t IRac::handleToggles(const stdAc::state_t desired, + const stdAc::state_t *prev) { + stdAc::state_t result = desired; + // If we've been given a previous state AND the it's the same A/C basically. + if (prev != NULL && desired.protocol == prev->protocol && + desired.model == prev->model) { + // Check if we have to handle toggle settings for specific A/C protocols. + switch (desired.protocol) { + case decode_type_t::COOLIX: + if ((desired.swingv == stdAc::swingv_t::kOff) ^ + (prev->swingv == stdAc::swingv_t::kOff)) // It changed, so toggle. + result.swingv = stdAc::swingv_t::kAuto; + else + result.swingv = stdAc::swingv_t::kOff; // No change, so no toggle. + result.turbo = desired.turbo ^ prev->turbo; + result.light = desired.light ^ prev->light; + result.clean = desired.clean ^ prev->clean; + result.sleep = (desired.sleep ^ prev->sleep) ? 0 : -1; + break; + case decode_type_t::WHIRLPOOL_AC: + result.power = desired.power ^ prev->power; + break; + case decode_type_t::PANASONIC_AC: + // CKP models use a power mode toggle. + if (desired.model == panasonic_ac_remote_model_t::kPanasonicCkp) + result.power = desired.power ^ prev->power; + break; + default: + {}; + } + } + return result; +} + // Send A/C message for a given device using common A/C settings. // Args: // vendor: The type of A/C protocol to use. @@ -1065,6 +1107,34 @@ bool IRac::sendAc(const decode_type_t vendor, const int16_t model, return true; // Success. } +// Send A/C message for a given device using state_t structures. +// Args: +// desired: The state_t structure describing the desired new a/c state. +// prev: Ptr to the previous state_t structure. +// +// Returns: +// boolean: True, if accepted/converted/attempted. False, if unsupported. +bool IRac::sendAc(const stdAc::state_t desired, const stdAc::state_t *prev) { + stdAc::state_t final = this->handleToggles(desired, prev); + return this->sendAc(final.protocol, final.model, final.power, final.mode, + final.degrees, final.celsius, final.fanspeed, + final.swingv, final.swingh, final.quiet, final.turbo, + final.econo, final.light, final.filter, final.clean, + final.beep, final.sleep, final.clock); +} + +// Compare two AirCon states. +// Returns: True if they differ, False if they don't. +// Note: Excludes clock. +bool IRac::cmpStates(const stdAc::state_t a, const stdAc::state_t b) { + return a.protocol != b.protocol || a.model != b.model || a.power != b.power || + a.mode != b.mode || a.degrees != b.degrees || a.celsius != b.celsius || + a.fanspeed != b.fanspeed || a.swingv != b.swingv || + a.swingh != b.swingh || a.quiet != b.quiet || a.turbo != b.turbo || + a.econo != b.econo || a.light != b.light || a.filter != b.filter || + a.clean != b.clean || a.beep != b.beep || a.sleep != b.sleep; +} + stdAc::opmode_t IRac::strToOpmode(const char *str, const stdAc::opmode_t def) { if (!strcasecmp(str, "AUTO") || !strcasecmp(str, "AUTOMATIC")) diff --git a/src/IRac.h b/src/IRac.h index 34fb3702b..8779de2fa 100644 --- a/src/IRac.h +++ b/src/IRac.h @@ -41,7 +41,8 @@ class IRac { const bool light, const bool filter, const bool clean, const bool beep, const int16_t sleep = -1, const int16_t clock = -1); - + bool sendAc(const stdAc::state_t desired, const stdAc::state_t *prev = NULL); + static bool cmpStates(const stdAc::state_t a, const stdAc::state_t b); static bool strToBool(const char *str, const bool def = false); static int16_t strToModel(const char *str, const int16_t def = -1); static stdAc::opmode_t strToOpmode( @@ -240,5 +241,7 @@ void daikin216(IRDaikin216 *ac, const bool turbo, const bool light, const int16_t sleep = -1, const int16_t clock = -1); #endif // SEND_WHIRLPOOL_AC +static stdAc::state_t handleToggles(const stdAc::state_t desired, + const stdAc::state_t *prev = NULL); }; // IRac class #endif // IRAC_H_ diff --git a/test/IRac_test.cpp b/test/IRac_test.cpp index 208afbc2e..cb94346c8 100644 --- a/test/IRac_test.cpp +++ b/test/IRac_test.cpp @@ -885,6 +885,113 @@ TEST(TestIRac, Whirlpool) { ASSERT_EQ(expected, ac.toString()); } +TEST(TestIRac, cmpStates) { + stdAc::state_t a, b; + a.protocol = decode_type_t::COOLIX; + a.model = -1; + a.power = true; + a.celsius = true; + a.degrees = 25; + a.mode = stdAc::opmode_t::kAuto; + a.fanspeed = stdAc::fanspeed_t::kAuto; + a.swingh = stdAc::swingh_t::kOff; + a.swingv = stdAc::swingv_t::kOff; + a.quiet = false; + a.turbo = false; + a.light = false; + a.econo = false; + a.beep = false; + a.filter = false; + a.clean = false; + a.quiet = false; + a.sleep = -1; + a.clock = -1; + + ASSERT_FALSE(IRac::cmpStates(a, a)); + ASSERT_TRUE(IRac::cmpStates(a, b)); + + b = a; + ASSERT_FALSE(IRac::cmpStates(a, b)); + + // Check we don't compare the clock. + b.clock = 1234; + ASSERT_FALSE(IRac::cmpStates(a, b)); + + // Now make them different. + b.power = false; + ASSERT_TRUE(IRac::cmpStates(a, b)); +} + +TEST(TestIRac, handleToggles) { + stdAc::state_t desired, prev, result; + desired.protocol = decode_type_t::COOLIX; + desired.model = -1; + desired.power = true; + desired.celsius = true; + desired.degrees = 25; + desired.mode = stdAc::opmode_t::kAuto; + desired.fanspeed = stdAc::fanspeed_t::kAuto; + desired.swingh = stdAc::swingh_t::kOff; + desired.swingv = stdAc::swingv_t::kOff; + desired.quiet = false; + desired.turbo = false; + desired.light = false; + desired.econo = false; + desired.beep = false; + desired.filter = false; + desired.clean = false; + desired.quiet = false; + desired.sleep = -1; + desired.clock = -1; + + // The states should be the same as we gave no previous state. + EXPECT_FALSE(IRac::cmpStates(desired, IRac::handleToggles(desired))); + // The states should be the same as we gave no settings that changed. + prev = desired; + EXPECT_FALSE(IRac::cmpStates(desired, IRac::handleToggles(desired, &prev))); + // Change something that isn't a toggle. + desired.degrees = 26; + ASSERT_TRUE(IRac::cmpStates(desired, prev)); + // Still shouldn't change. + EXPECT_FALSE(IRac::cmpStates(desired, IRac::handleToggles(desired, &prev))); + prev.turbo = true; // This requires a toggle. + result = IRac::handleToggles(desired, &prev); + EXPECT_TRUE(IRac::cmpStates(desired, result)); + EXPECT_TRUE(result.turbo); + desired.turbo = true; // As the desired setting hasn't changed from previous + // the result should not have turbo set, as it is + // a toggle setting. + result = IRac::handleToggles(desired, &prev); + EXPECT_TRUE(IRac::cmpStates(desired, result)); + EXPECT_FALSE(result.turbo); + + // Go back to the same states. + prev = desired; + ASSERT_FALSE(IRac::cmpStates(desired, prev)); + // Test swing, as it is more complicated. + result = IRac::handleToggles(desired, &prev); + EXPECT_EQ(stdAc::swingv_t::kOff, result.swingv); + desired.swingv = stdAc::swingv_t::kAuto; + result = IRac::handleToggles(desired, &prev); + EXPECT_NE(stdAc::swingv_t::kOff, result.swingv); + + prev = desired; // Pretend it was sent and time has passed. + ASSERT_FALSE(IRac::cmpStates(desired, prev)); + ASSERT_NE(stdAc::swingv_t::kOff, desired.swingv); + + // User changes setting but it's still an "on" setting, as this device + // only has a binary on/off for swingv. Nothing should change. + desired.swingv = stdAc::swingv_t::kHigh; + result = IRac::handleToggles(desired, &prev); + ASSERT_EQ(stdAc::swingv_t::kOff, result.swingv); // i.e No toggle. + + prev = desired; // Pretend it was sent and time has passed. + // User changes setting to off. i.e. It is no longer on, so it should toggle. + desired.swingv = stdAc::swingv_t::kOff; + result = IRac::handleToggles(desired, &prev); + ASSERT_NE(stdAc::swingv_t::kOff, result.swingv); // i.e A toggle. +} + TEST(TestIRac, strToBool) { EXPECT_TRUE(IRac::strToBool("ON")); EXPECT_TRUE(IRac::strToBool("1"));