Added MitsubishiAC VaneLeft #1572

hideosasaki · 2021-08-23T14:26:51Z

Hi,
I am a user of Mitsubishi A/C MSZ-ZW4017S-W, which has two vanes. While evaluating the library I noticed that IRMitsubishiAC::setVane moved only the right vane. So I added setVaneLeft method.
This is the first time to contribute open-source projects for me. Thank you so much for correcting my mistakes.
Hideo

NiKiZe · 2021-08-23T14:44:50Z

Nice start! ありがとうございます!

Just a quick initial look.
Please add your device to the Supports section in the .h file.
There is a few checks that is failing, can you take a look?

Maybe we can also add some captures from original remote and add tests?
There is also some string generation that we would like to have. Let us know if this is something that you want to tackle, and if you need guidance. Otherwise we are happy to do these parts before merging.

src/ir_Mitsubishi.h

NiKiZe · 2021-08-23T14:48:08Z

src/ir_Mitsubishi.h

+    // Byte 14
+    uint8_t          :4;
+    uint8_t          :4;
+    // Byte 15
+    uint8_t          :4;
+    uint8_t          :4;


For now these can be kept as :8; instead of double? :4; or is there some specific reason that it is split?

NiKiZe · 2021-08-23T14:50:22Z

src/ir_Mitsubishi.cpp

+uint8_t IRMitsubishiAC::getVaneLeft(void) const {
+  return _.VaneLeft;
+}
+


I think this fits in 80 columns on one line?

NiKiZe · 2021-08-23T14:51:17Z

src/ir_Mitsubishi.cpp

+  uint8_t pos = std::min(position, kMitsubishiAcVaneAutoMove);  // bounds check
+  _.VaneLeft = pos;


Temporary pos variable is unnecessary

hideosasaki · 2021-08-23T22:49:43Z

@NiKiZe
Thank you for reviewing! I committed some fixes you pointed out. And I am sorry for not being able to provide the real capture, because I haven't got any IR receivable devices but only have M5Stick-C.

crankyoldgit

Congrats on your effort. We appreciate it!

crankyoldgit · 2021-08-24T13:20:33Z

test/ir_Mitsubishi_test.cpp

@@ -652,6 +652,26 @@ TEST(TestMitsubishiACClass, VaneMode) {
  EXPECT_EQ(kMitsubishiAcVaneAutoMove - 1, ac.getVane());
 }

+TEST(TestMitsubishiACClass, VaneMode) {


A test with the same name already exists. e.g. VaneMode
You need to change it for your new test.
e.g.

TEST(TestMitsubishiACClass, VaneLeft) {

Changed to VaneLeft

crankyoldgit · 2021-08-24T13:25:20Z

src/ir_Mitsubishi.cpp

@@ -480,6 +480,16 @@ uint8_t IRMitsubishiAC::getWideVane(void) const {
  return _.WideVane;
 }

+/// Set the requested Left Vane (Vertical Swing) operation mode of the a/c unit.


Is it "Vertical Swing" or "Horizontal Swing"? Your description Mitsubishi144Protocol (VaneLeft) indicates it's a "SwingH(Left)". Hence the question.

it's Vertical Swing. The comment was wrong. See the attached photo on the first post.

crankyoldgit · 2021-08-24T13:25:40Z

src/ir_Mitsubishi.cpp

+  _.VaneLeft = std::min(position, kMitsubishiAcVaneAutoMove);  // bounds check
+}
+
+/// Get the Left Vane (Vertical Swing) mode of the A/C.


Ditto other comment.

src/ir_Mitsubishi.cpp

crankyoldgit · 2021-08-26T00:53:24Z

One of the unit tests is failing.
It just needs to be updated.
See https://github.com/crankyoldgit/IRremoteESP8266/pull/1572/checks?check_run_id=3419003709#step:4:2931

hideosasaki · 2021-08-26T04:24:10Z

@crankyoldgit
I fixed the test case, just removed the VaneLeft setting from. I had no choice to do the bad thing because I haven't got the IR receivable device.

test/ir_Mitsubishi_test.cpp

crankyoldgit

Approved

* Fix a code linter warning. * Improve documentation. * Improve a unit test. Ref #1572

_v2.7.20 (20210828)_ **[Bug Fixes]** - Make `strToSwingH()` match "Right Max" (#1550 #1551) **[Features]** - Experimental Bose remote support (#1579) - Added MitsubishiAC VaneLeft (#1572 #1576) - HAIER_AC176: Add experimental detailed support (#1480 #1571) - Detailed support for Tornado/Sanyo 88-bit A/C protocol (#1503 #1568) - Add support for new `TROTEC_3550` A/C protocol (#1563 #1566 #1507) - SamsungAc: Use `sendExtended()` going forward. (#1484 #1562) - SamsungAc: Redo/fix checksum calculations. (#1538 #1554) - LG: Add support for `AKB73757604` model (#1531 #1545) - Daikin176: Add support for Unit Id. (#1543 #1544) - Daikin2: Add support for Humidity setting/operation. (#1535 #1540) - TCL112AC: Add support for quiet/mute setting. (#1528 #1529) - LG2: Add Fan speed, Swing, & Light support for new `AKB74955603` model (#1513 #1530) - Add Mitsubishi AC "fan only" mode (#1527) **[Misc]** - Fix pylint issues due to pylint update. (#1569 #1570) - DAIKIN216: Update supported models. (#1552 #1567) - IRMQTTServer: Build a minimal OTA image via PlatformIO. (#1513 #1541) - Reduce memory fragmentation cause by String usage. (#1493 #1536) - Refactor `decodeMitsubishiAC()` (#1523 #1532) - Fix incorrect comment. - Migrate from Travis to GitHub Actions (#1522 #1526) - Documentation update with additional supported Panasonic AC models (#1525)

_v2.7.20 (20210828)_ **[Bug Fixes]** - Make `strToSwingH()` match "Right Max" (#1550 #1551) **[Features]** - Experimental Bose remote support (#1579) - Added MitsubishiAC VaneLeft (#1572 #1576) - HAIER_AC176: Add experimental detailed support (#1480 #1571) - Detailed support for Tornado/Sanyo 88-bit A/C protocol (#1503 #1568) - Add support for new `TROTEC_3550` A/C protocol (#1563 #1566 #1507) - SamsungAc: Use `sendExtended()` going forward. (#1484 #1562) - SamsungAc: Redo/fix checksum calculations. (#1538 #1554) - LG: Add support for `AKB73757604` model (#1531 #1545) - Daikin176: Add support for Unit Id. (#1543 #1544) - Daikin2: Add support for Humidity setting/operation. (#1535 #1540) - TCL112AC: Add support for quiet/mute setting. (#1528 #1529) - LG2: Add Fan speed, Swing, & Light support for new `AKB74955603` model (#1513 #1530) - Add Mitsubishi AC "fan only" mode (#1527) **[Misc]** - Change when some github workflows run (#1583) - Add/update supported device info (#1580 #1581 #1585) - Fix pylint issues due to pylint update. (#1569 #1570) - DAIKIN216: Update supported models. (#1552 #1567) - IRMQTTServer: Build a minimal OTA image via PlatformIO. (#1513 #1541) - Reduce memory fragmentation cause by String usage. (#1493 #1536) - Refactor `decodeMitsubishiAC()` (#1523 #1532) - Fix incorrect comment. - Migrate from Travis to GitHub Actions (#1522 #1526) - Documentation update with additional supported Panasonic AC models (#1525)

## _v2.7.20 (20210828)_ **[Bug Fixes]** - Make `strToSwingH()` match "Right Max" (#1550 #1551) **[Features]** - Experimental Bose remote support (#1579) - Added MitsubishiAC VaneLeft (#1572 #1576) - HAIER_AC176: Add experimental detailed support (#1480 #1571) - Detailed support for Tornado/Sanyo 88-bit A/C protocol (#1503 #1568) - Add support for new `TROTEC_3550` A/C protocol (#1563 #1566 #1507) - SamsungAc: Use `sendExtended()` going forward. (#1484 #1562) - SamsungAc: Redo/fix checksum calculations. (#1538 #1554) - LG: Add support for `AKB73757604` model (#1531 #1545) - Daikin176: Add support for Unit Id. (#1543 #1544) - Daikin2: Add support for Humidity setting/operation. (#1535 #1540) - TCL112AC: Add support for quiet/mute setting. (#1528 #1529) - LG2: Add Fan speed, Swing, & Light support for new `AKB74955603` model (#1513 #1530) - Add Mitsubishi AC "fan only" mode (#1527) **[Misc]** - Change when some github workflows run (#1583) - Add/update supported device info (#1580 #1581 #1585) - Fix pylint issues due to pylint update. (#1569 #1570) - DAIKIN216: Update supported models. (#1552 #1567) - IRMQTTServer: Build a minimal OTA image via PlatformIO. (#1513 #1541) - Reduce memory fragmentation cause by String usage. (#1493 #1536) - Refactor `decodeMitsubishiAC()` (#1523 #1532) - Fix incorrect comment. - Migrate from Travis to GitHub Actions (#1522 #1526) - Documentation update with additional supported Panasonic AC models (#1525)

crankyoldgit · 2021-08-28T09:02:42Z

FYI, the changes mentioned above have been included in the just released v2.7.20 of the library.

Added MitsubishiAC VaneLeft

c0100ad

NiKiZe requested changes Aug 23, 2021

View reviewed changes

NiKiZe self-assigned this Aug 23, 2021

NiKiZe added the enhancement label Aug 23, 2021

refactored and fixed typo

0d9adca

crankyoldgit requested changes Aug 24, 2021

View reviewed changes

hideosasaki added 2 commits August 25, 2021 11:38

fixed duplicated name

17afe5c

fixed comment

7786f62

hideosasaki requested review from crankyoldgit and NiKiZe August 25, 2021 03:27

NiKiZe approved these changes Aug 25, 2021

View reviewed changes

excepted VaneLeft from the IR message test

d00dd63

crankyoldgit requested changes Aug 26, 2021

View reviewed changes

test/ir_Mitsubishi_test.cpp Outdated Show resolved Hide resolved

updated an assert data changed by adding VaneLeft

ee49fc0

crankyoldgit merged commit e507b60 into crankyoldgit:master Aug 27, 2021

crankyoldgit reviewed Aug 27, 2021

View reviewed changes

crankyoldgit added a commit that referenced this pull request Aug 27, 2021

MitsubishiAC: Fix code lint issue.

0f11cbe

* Fix a code linter warning. * Improve documentation. * Improve a unit test. Ref #1572

crankyoldgit mentioned this pull request Aug 27, 2021

MitsubishiAC: Fix code lint issue. #1576

Merged

hideosasaki deleted the Mitsubishi_VaneLeft branch August 27, 2021 06:07

crankyoldgit added a commit that referenced this pull request Aug 27, 2021

MitsubishiAC: Fix code lint issue. (#1576)

bdff667

* Fix a code linter warning. * Improve documentation. * Improve a unit test. Ref #1572

crankyoldgit mentioned this pull request Aug 28, 2021

v2.7.20 release #1582

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added MitsubishiAC VaneLeft #1572

Added MitsubishiAC VaneLeft #1572

hideosasaki commented Aug 23, 2021 •

edited

Loading

NiKiZe commented Aug 23, 2021

NiKiZe Aug 23, 2021

NiKiZe Aug 23, 2021

NiKiZe Aug 23, 2021

hideosasaki commented Aug 23, 2021 •

edited

Loading

crankyoldgit left a comment

crankyoldgit Aug 24, 2021

hideosasaki Aug 25, 2021

crankyoldgit Aug 24, 2021

hideosasaki Aug 25, 2021 •

edited

Loading

crankyoldgit Aug 24, 2021

crankyoldgit commented Aug 26, 2021

hideosasaki commented Aug 26, 2021

crankyoldgit left a comment

crankyoldgit commented Aug 28, 2021

		uint8_t pos = std::min(position, kMitsubishiAcVaneAutoMove); // bounds check
		_.VaneLeft = pos;

Added MitsubishiAC VaneLeft #1572

Added MitsubishiAC VaneLeft #1572

Conversation

hideosasaki commented Aug 23, 2021 • edited Loading

NiKiZe commented Aug 23, 2021

NiKiZe Aug 23, 2021

Choose a reason for hiding this comment

NiKiZe Aug 23, 2021

Choose a reason for hiding this comment

NiKiZe Aug 23, 2021

Choose a reason for hiding this comment

hideosasaki commented Aug 23, 2021 • edited Loading

crankyoldgit left a comment

Choose a reason for hiding this comment

crankyoldgit Aug 24, 2021

Choose a reason for hiding this comment

hideosasaki Aug 25, 2021

Choose a reason for hiding this comment

crankyoldgit Aug 24, 2021

Choose a reason for hiding this comment

hideosasaki Aug 25, 2021 • edited Loading

Choose a reason for hiding this comment

crankyoldgit Aug 24, 2021

Choose a reason for hiding this comment

crankyoldgit commented Aug 26, 2021

hideosasaki commented Aug 26, 2021

crankyoldgit left a comment

Choose a reason for hiding this comment

crankyoldgit commented Aug 28, 2021

hideosasaki commented Aug 23, 2021 •

edited

Loading

hideosasaki commented Aug 23, 2021 •

edited

Loading

hideosasaki Aug 25, 2021 •

edited

Loading