Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SharpAc: Allow position control of SwingV #1594

Merged
merged 4 commits into from
Sep 6, 2021
Merged

SharpAc: Allow position control of SwingV #1594

merged 4 commits into from
Sep 6, 2021

Conversation

crankyoldgit
Copy link
Owner

  • Add the ability to set swingV positions via setSwingV()
    • e.g. Coanda setting.
    • Several other undocumented positions discovered via experimentation.
    • May not work on all models.
  • Update & add unit tests accordingly.

Fixes #1590

* Add the ability to set swingV positions via `setSwingV()`
  - e.g. Coanda setting.
  - Several other undocumented positions discovered via experimentation.
  - May not work on all models.
* Update & add unit tests accordingly.

Fixes #1590
@crankyoldgit crankyoldgit self-assigned this Sep 2, 2021
@crankyoldgit crankyoldgit added the Pending Confirmation Waiting for confirmation from user label Sep 2, 2021
src/ir_Sharp.h Outdated Show resolved Hide resolved
@menexttoday
Copy link

Forgive the layout below this is the first time I reply. Still trying to figure it all out.

For some reason when invoking ac.setSwingV(kSharpAcSwingVCoanda); the signal sent is 0b100 to the AC instead of 0b110.

In ir_Sharp.cpp line 558 may be changing the value. The OEM remote just sends out 0x0E or 0b110. The AC responds accordingly depending to the mode. I don't think this code is required.
// Only allowed in Heat mode. if (getMode() != kSharpAcHeat) { setSwingV(kSharpAcSwingVLow); // Use the next lowest setting. return; } // FALLTHRU

I removed the above lines and the code behaves as the original remote. I tried pushing the change I am not sure if it was sent properly.

@crankyoldgit
Copy link
Owner Author

For some reason when invoking ac.setSwingV(kSharpAcSwingVCoanda); the signal sent is 0b100 to the AC instead of 0b110.

I decided to implement it as kSharpAcSwingVCoanda = kSharpAcSwingVLowest and only available in Heat mode, and use the "feature" that kSharpAcSwingVHigh (0b001 [0b1001]) has the same effect as Cool's Coanda.
This is because there are interfaces to the library (via the IRac class) which don't have the concept of a "Coanda" mode. e.g. they choose a fixed-ish position, like highest, high, middle, low, lowest etc. It's unintuitive for someone to set vanes to the lowest position to get the high/highest one, if you catch my drift.

In short, in Cool mode, if you desire the Highest (aka Coanda) fan position, you should specify: ac.setSwingV(kSharpAcSwingVHigh);

Yes, it won't generate the exact same code as the remote, but in theory it should allow us to have a true high setting for all modes, and avoid people having to know to set "lowest" for "highest" in Sharp.

I can/could change the code as you described to allow creating the exact same state[] code but I fear it's going to lead to feedback/questions/issues saying "I found a bug, when setting my sharp a/c to Lowest pos, it went to the Highest. Please fix!"

An alternative is to add an optional extra parameter to setSwingV() that allows someone to force the setting, so the lib can create the exact same state[] code. Hmmm.

I tried pushing the change I am not sure if it was sent properly.

It didn't come through that I can see, but thanks for trying. ;-)

* Modify `setSwingV()` to take an optional parameter to override the heat mode check.
Ref: #1594 (comment)
@crankyoldgit
Copy link
Owner Author

In the latest commit to the branch, I've added the ability to force Coanda mode in cool, via:

ac.setSwingV(kSharpAcSwingVCoanda, true);  // Force Coanda setting regardless of operating mode.

@menexttoday
Copy link

menexttoday commented Sep 3, 2021 via email

@crankyoldgit
Copy link
Owner Author

Not sure what Sharp was thinking

Alas, I don't think they think about people a) reverse engineering their protocols, and b) Trying to control them in a universal manor. ;-P We do the best we can do.

Accessing things directly via the IRSharpAc class methods etc as you are doing gives you the greatest control and functionality. Also, it something you can "program around" if/as you need to.

I'm going to leave the "normal"/default behaviour the way it is to allow it to work with/for people who are using the generic IRac class methods (e.g. IRMQTTServer, Tasmota, EasyESP, etc) and it does as "expected" from that perspective.

[Q] What happens if you send two Auto/0b111/0b1111 messages back to back (after a suitable gap of course). Is it really a "Toggle" or is it a "make the device swing the vane back and forth" message?
i.e. Does sending it twice result in it starting to swing, then stopping. Or will it continue to swing after the second message?
TL;DR: Is it really a "Start swinging" message, or a "If you were swinging, stop. If you were stopped, swing"?

I ask because the "generic" IRac class will keep sending the Auto swing bits in each message. Do I need to tell it to send Off after the initial Auto code?

@menexttoday
Copy link

menexttoday commented Sep 3, 2021 via email

* Change name of some settings to better match their action/function.
* Use previous state to influence settings sent via `IRac` class.
* Update unit tests.
* Update `toString()` output.

Ref: #1594 (comment)
@crankyoldgit
Copy link
Owner Author

@menexttoday Please take another look/test etc of the branch. Hopefully this version is better.

@menexttoday
Copy link

menexttoday commented Sep 4, 2021 via email

@crankyoldgit
Copy link
Owner Author

It took me longer to figure out Gihub and how to get the code in my system in the right folder than to make the changes to my code and run the tests. It seems to work great. I ran through all the codes in dry, cooI and heat. Will run more tests tomorrow when I have more time in case I missed one in one of the settings.

On Sep 3, 2021, at 10:08 PM, David Conran @.***> wrote: @menexttoday Please take another look/test etc of the branch. Hopefully this version is better. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

Okay. I'll await your response in a day or so.

@crankyoldgit crankyoldgit requested a review from NiKiZe September 4, 2021 08:16
@crankyoldgit
Copy link
Owner Author

@NiKiZe Can you please cast another eye over this. It's changed a bit since your previous review.

@crankyoldgit crankyoldgit merged commit a359a43 into master Sep 6, 2021
@crankyoldgit crankyoldgit mentioned this pull request Sep 6, 2021
@crankyoldgit crankyoldgit deleted the SharpSwingV branch September 9, 2021 00:04
crankyoldgit added a commit that referenced this pull request Nov 19, 2021
_v2.8.0 (20211119)_

**[Bug Fixes]**
- Fix compilation issue when using old 8266 Arduino Frameworks. (#1639 #1640)
- Fix potential security issue with `scrape_supported_devices.py` (#1616 #1619)

**[Features]**
- SAMSUNG_AC
  - Change `clean` setting to a toggle. (#1676 #1677)
  - Highest fan speed is available without Powerful setting. (#1675 #1678)
  - Change `beep` setting to a toggle. (#1669 #1671)
  - Fix Beep for AR12TXEAAWKNEU (#1668 #1669)
  - Add support for Horizontal Swing & Econo (#1277 #1667)
  - Add support for On, Off, & Sleep Timers (#1277 #1662)
  - Fix power control. Clean-up code & bitmaps from Checksum changes. (#1277 #1648 #1650)
- HAIER_AC176/HAIER_AC_YRW02
  - Add support A/B unit setting (#1672)
  - Add support degree Fahrenheit (#1659)
  - Add support `Lock` function (#1652)
  - Implement horizontal swing feature (#1641)
  - Implement Quiet setting. (#1634 #1635)
- Basic support for Airton Protocol (#1670 #1681)
- HAIER_AC176: Add Turbo and Quiet settings (#1634)
- Gree: Add `SwingH` & `Econo` control. (#1587 #1653)
- MIRAGE
  - Add experimental detailed support. (#1573 #1615)
  - Experimental detailed support for KKG29A-C1 remote. (#1573 #1660)
- ELECTRA_AC: Add support for "IFeel" & Sensor settings. (#1644 #1645)
- Add Russian translation (#1649)
- Add Swedish translation (#1627)
- Reduce flash space used. (#1633)
- Strings finally in Flash! (#1493 #1614 #1623)
- Add support for Rhoss Idrowall MPCV 20-30-35-40 A/C protocol (#1630)
- Make `IRAc::opmodeToString()` output nicer for humans. (#1613)
- TCL112AC/TEKNOPOINT: Add support for `GZ055BE1` model (#1486 #1602)
- Support for Arris protocol. (#1598)
- SharpAc: Allow position control of SwingV (#1590 #1594)

**[Misc]**
- HAIER_AC176/HAIER_AC_YRW02
  - Replace some magic numbers with constants (#1679)
  - Small fix `Quiet` and `Turbo` test (#1674)
  - Fix `IRHaierAC176::getTemp()` return value description (#1663)
- Security Policy creation and changes. (#1616 #1617 #1618 #1621 #1680)
- IRrecvDumpV2/3: Update PlatformIO envs for missing languages (#1661)
- IRMQTTServer
  - Use the correct string for Fan mode in Home Assistant. (#1610 #1657)
  - Move a lot of the strings/text to flash. (#1638)
- Minor code style improvements. (#1656)
- Update Supported Devices
  - HAIER_AC176 (#1673)
  - LG A/C (#1651 #1655)
  - Symphony (#1603 #1605)
  - Epson (#1574 #1601)
  - GREE (#1587 #1588)
  - SharpAc (#1590 #1591)
- Add extra tests for LG2 protocol (#1654)
- Fix parameter expansion in several macros.
- Move some strings to `IRtext.cpp` & `locale/default.h` (#1637)
- RHOSS: Move include and defines to their correct places (#1636)
- Make makefile only build required files when running `run-%` target (#1632)
- Update Portuguese translation (#1628)
- Add possibility to run specific test case (#1625)
- Change `googletest` library ignore (#1626)
- Re-work "Fan Only" strings & matching. (#1610)
- Address `C0209` pylint warnings. (#1608)
@crankyoldgit crankyoldgit mentioned this pull request Nov 19, 2021
crankyoldgit added a commit that referenced this pull request Nov 19, 2021
## _v2.8.0 (20211119)_

**[Bug Fixes]**
- Fix compilation issue when using old 8266 Arduino Frameworks. (#1639 #1640)
- Fix potential security issue with `scrape_supported_devices.py` (#1616 #1619)

**[Features]**
- SAMSUNG_AC
  - Change `clean` setting to a toggle. (#1676 #1677)
  - Highest fan speed is available without Powerful setting. (#1675 #1678)
  - Change `beep` setting to a toggle. (#1669 #1671)
  - Fix Beep for AR12TXEAAWKNEU (#1668 #1669)
  - Add support for Horizontal Swing & Econo (#1277 #1667)
  - Add support for On, Off, & Sleep Timers (#1277 #1662)
  - Fix power control. Clean-up code & bitmaps from Checksum changes. (#1277 #1648 #1650)
- HAIER_AC176/HAIER_AC_YRW02
  - Add support A/B unit setting (#1672)
  - Add support degree Fahrenheit (#1659)
  - Add support `Lock` function (#1652)
  - Implement horizontal swing feature (#1641)
  - Implement Quiet setting. (#1634 #1635)
- Basic support for Airton Protocol (#1670 #1681)
- HAIER_AC176: Add Turbo and Quiet settings (#1634)
- Gree: Add `SwingH` & `Econo` control. (#1587 #1653)
- MIRAGE
  - Add experimental detailed support. (#1573 #1615)
  - Experimental detailed support for KKG29A-C1 remote. (#1573 #1660)
- ELECTRA_AC: Add support for "IFeel" & Sensor settings. (#1644 #1645)
- Add Russian translation (#1649)
- Add Swedish translation (#1627)
- Reduce flash space used. (#1633)
- Strings finally in Flash! (#1493 #1614 #1623)
- Add support for Rhoss Idrowall MPCV 20-30-35-40 A/C protocol (#1630)
- Make `IRAc::opmodeToString()` output nicer for humans. (#1613)
- TCL112AC/TEKNOPOINT: Add support for `GZ055BE1` model (#1486 #1602)
- Support for Arris protocol. (#1598)
- SharpAc: Allow position control of SwingV (#1590 #1594)

**[Misc]**
- HAIER_AC176/HAIER_AC_YRW02
  - Replace some magic numbers with constants (#1679)
  - Small fix `Quiet` and `Turbo` test (#1674)
  - Fix `IRHaierAC176::getTemp()` return value description (#1663)
- Security Policy creation and changes. (#1616 #1617 #1618 #1621 #1680)
- IRrecvDumpV2/3: Update PlatformIO envs for missing languages (#1661)
- IRMQTTServer
  - Use the correct string for Fan mode in Home Assistant. (#1610 #1657)
  - Move a lot of the strings/text to flash. (#1638)
- Minor code style improvements. (#1656)
- Update Supported Devices
  - HAIER_AC176 (#1673)
  - LG A/C (#1651 #1655)
  - Symphony (#1603 #1605)
  - Epson (#1574 #1601)
  - GREE (#1587 #1588)
  - SharpAc (#1590 #1591)
- Add extra tests for LG2 protocol (#1654)
- Fix parameter expansion in several macros.
- Move some strings to `IRtext.cpp` & `locale/default.h` (#1637)
- RHOSS: Move include and defines to their correct places (#1636)
- Make makefile only build required files when running `run-%` target (#1632)
- Update Portuguese translation (#1628)
- Add possibility to run specific test case (#1625)
- Change `googletest` library ignore (#1626)
- Re-work "Fan Only" strings & matching. (#1610)
- Address `C0209` pylint warnings. (#1608)
@crankyoldgit
Copy link
Owner Author

FYI, the changes mentioned above have now been included in the new v2.8.0 release of the library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Pending Confirmation Waiting for confirmation from user
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants