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

@@ -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

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"))

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_

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"));