From 44d9f550b40eb48798c88b48a822c1b953b3b3f4 Mon Sep 17 00:00:00 2001
From: Maxim Mikityanskiy <maxtram95@gmail.com>
Date: Sat, 3 Aug 2024 02:44:44 +0300
Subject: [PATCH] Gree: Fix reporting vertical swing

toCommonSwingV() is only called when SwingAuto is false, but it converts
kGreeSwingLastPos to kAuto. It doesn't make sense, because:

1. kGreeSwingLastPos means that swinging is stopped (i.e. the shutter
   remains in its last position), which corresponds to kOff.
2. kAuto shouldn't be returned from this function at all, because it's
   handled separately in toCommon() when SwingAuto is true.
3. As can be seen in setSwingVertical(), when automatic is false, the
   valid set of positions includes kGreeSwingLastPos, but not
   kGreeSwingAuto.

Fix the logic by amending toCommonSwingV() according to the
considerations above. It fixes parsing of received IR packets when the
user disables vertical swinging from the remote (tested with YAP1FB).

For consistency and robustness, educate setSwingVertical() and
convertSwingV() about the supported kGreeSwingLastPos mode.

Add a unit test for the described bug.
---
 src/ir_Gree.cpp       | 4 +++-
 test/ir_Gree_test.cpp | 5 +++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/ir_Gree.cpp b/src/ir_Gree.cpp
index 2c44cfe52..3d2da38f7 100644
--- a/src/ir_Gree.cpp
+++ b/src/ir_Gree.cpp
@@ -375,6 +375,7 @@ void IRGreeAC::setSwingVertical(const bool automatic, const uint8_t position) {
   uint8_t new_position = position;
   if (!automatic) {
     switch (position) {
+      case kGreeSwingLastPos:
       case kGreeSwingUp:
       case kGreeSwingMiddleUp:
       case kGreeSwingMiddle:
@@ -503,6 +504,7 @@ uint8_t IRGreeAC::convertFan(const stdAc::fanspeed_t speed) {
 /// @return The native equivalent of the enum.
 uint8_t IRGreeAC::convertSwingV(const stdAc::swingv_t swingv) {
   switch (swingv) {
+    case stdAc::swingv_t::kOff:     return kGreeSwingLastPos;
     case stdAc::swingv_t::kHighest: return kGreeSwingUp;
     case stdAc::swingv_t::kHigh:    return kGreeSwingMiddleUp;
     case stdAc::swingv_t::kMiddle:  return kGreeSwingMiddle;
@@ -562,7 +564,7 @@ stdAc::swingv_t IRGreeAC::toCommonSwingV(const uint8_t pos) {
     case kGreeSwingMiddle:     return stdAc::swingv_t::kMiddle;
     case kGreeSwingMiddleDown: return stdAc::swingv_t::kLow;
     case kGreeSwingDown:       return stdAc::swingv_t::kLowest;
-    default:                   return stdAc::swingv_t::kAuto;
+    default:                   return stdAc::swingv_t::kOff;
   }
 }
 
diff --git a/test/ir_Gree_test.cpp b/test/ir_Gree_test.cpp
index e0160577e..d2d8354ed 100644
--- a/test/ir_Gree_test.cpp
+++ b/test/ir_Gree_test.cpp
@@ -701,6 +701,11 @@ TEST(TestGreeClass, toCommon) {
   ASSERT_FALSE(ac.toCommon().filter);
   ASSERT_FALSE(ac.toCommon().beep);
   ASSERT_EQ(-1, ac.toCommon().clock);
+
+  // Test kGreeSwingLastPos following the pattern in IRac::gree().
+  ASSERT_EQ(kGreeSwingLastPos, ac.convertSwingV(stdAc::swingv_t::kOff));
+  ac.setSwingVertical(false, kGreeSwingLastPos);
+  ASSERT_EQ(stdAc::swingv_t::kOff, ac.toCommon().swingv);
 }
 
 TEST(TestGreeClass, Issue814Power) {