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.

Sign up for GitHub

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

Jump to bottom

Gree library doesn’t work properly (AC White Westinghouse and Coventry) #684

Closed
enriqueolivieri opened this issue Apr 26, 2019 · 2 comments · Fixed by #686
Closed

Gree library doesn’t work properly (AC White Westinghouse and Coventry) #684

enriqueolivieri opened this issue Apr 26, 2019 · 2 comments · Fixed by #686
Assignees
@crankyoldgit
Labels
bug

Comments

@enriqueolivieri
Copy link

Version/revison of the library used

IRremoteESP8266 v2.5.6

Expected behavior

Hi, I’ve done some tests in 2 air conditioners (a White Westinghouse and a Coventry). Both use Gree protocol and the library doesn’t work properly.

  1. I found that the method “IRsend::sendGree” has a mistake in the // Footer #1. dutycycle is false and it should be 50 (50%). At the same time, MSBfirst is true but it is better to set false (even though it isn’t necessary because data is 0b010).
  2. Other change to make it work better is to set kGreeHdrSpace = 4500. I read this value of the signal with “IRrecvDumpV2".
@crankyoldgit crankyoldgit self-assigned this Apr 27, 2019
crankyoldgit added a commit that referenced this issue Apr 27, 2019
@crankyoldgit
Fix the byte array version of sendGree()
fe80582 
As reported in #684 by @enriqueolivieri the duty cycle setting for the middle data footer was wrong.
I guess everyone was using the `uint64t` version of `sendGree()`.

Unit tests don't and won't catch this as they don't care about duty cycles for the tests we typically conduct.
@crankyoldgit crankyoldgit mentioned this issue Apr 27, 2019
Merged
@crankyoldgit
Copy link
Owner

Thanks for the report! You are indeed correct on both points. Historic data does suggest that a hdrspace of 4500 would be a better fit. I'll create a PR for that later.

I've created a PR already for your first point. I guess it seemed it was working fine because most people use the uint64_t version rather than the uint8_t * version and my tests didn't catch it. :-/

crankyoldgit added a commit that referenced this issue Apr 27, 2019
@crankyoldgit
Fix the byte array version of sendGree() (#685)
efc87ae 
As reported in #684 by @enriqueolivieri the duty cycle setting for the middle data footer was wrong.
I guess everyone was using the `uint64t` version of `sendGree()`.

Unit tests don't and won't catch this as they don't care about duty cycles for the tests we typically conduct.
crankyoldgit added a commit that referenced this issue Apr 27, 2019
@crankyoldgit
Adjust kGreeHdrSpace to 4500
849cb30 
As reported in #684 and data reported in #386 a value of 4500 is a much better
choice over 4000.

Adjust value and unit tests to match.

Fixes #684
@crankyoldgit crankyoldgit mentioned this issue Apr 27, 2019
Merged
crankyoldgit added a commit that referenced this issue Apr 27, 2019
@crankyoldgit
Adjust kGreeHdrSpace to 4500 (#686)
e1f499e 
As reported in #684 and data reported in #386 a value of 4500 is a much better
choice over 4000.

Adjust value and unit tests to match.

Fixes #684
crankyoldgit added a commit that referenced this issue Apr 27, 2019
@crankyoldgit
Add unit tests for Frequency and Duty cycle
08c55ff 
Extend `IRsendTest::outputStr()` to note freqency and duty cycle changes.
* Add unit test to confirm new features work as intended.
* Update all existing unit test cases.
* [BUG] Fixed a problem where LG protocol used wrong duty cycle for repeat section.

Yay for unit tests finding bugs!

Ref #684 & #685
@crankyoldgit crankyoldgit mentioned this issue Apr 27, 2019
Merged
crankyoldgit added a commit that referenced this issue Apr 29, 2019
@crankyoldgit
Add unit tests for Frequency and Duty cycle (#687)
ac0200f 
Extend `IRsendTest::outputStr()` to note freqency and duty cycle changes.
* Add unit test to confirm new features work as intended.
* Update all existing unit test cases.
* [BUG] Fixed a problem where LG protocol used wrong duty cycle for repeat section.

Yay for unit tests finding bugs!

Ref #684 & #685
crankyoldgit added a commit that referenced this issue Apr 30, 2019
@crankyoldgit
v2.6.0 release
19468bc 
_v2.6.0 (20190430)_

**[Bug Fixes]**
- Fixed problem where LG protocol used wrong duty cycle for repeat. (#687)
- Fix checksum calculation for Daikin protocols. (#678)
- Fix the byte array version of sendGree() (#684, #685)
- Fix artificial vs. real state creation on HaierAC. (#668, #671)
- Fix issues caused by having `MQTT_ENABLE` set to false. (#677)
- Fix compile problem when DEBUG is defined. (#673, #674)
- Fix Minor bug with MQTT_ENABLE False condition (#654)

**[Features]**
- Experimental support for DAIKIN216 (ARC433B69) (#690)
- Experimental support for Mitsubishi Heavy Industries A/Cs. (#660, #665, #667)
- Support more features of TCL A/C (#656)
- Add LEGO(TM) Power Functions IR protocol. (#655)
- Add Panasonic AC RKR model & Example (#649)
- DAIKIN/IRDaikinESP overhaul and add Comfort mode support. (#678)
  **WARNING**: Previous `sendDaikin()` calls may not work.
               Please recapture codes or use `kDaikinStateLengthShort` for
               `nbytes` in those calls.
- IRMQTTServer: Move MQTT server and other parameters to WifiManager. (#680)
  **WARNING**: Previous users may need to fully wipe/reset the
               SPIFFS/WifiManager settings by visiting
               `http://<your_esp8266's_ip_address>/reset` prior to or
               after update.
- Add Wifi filtering options to IRMQTTServer. (#679)
- Add advanced aircon/climate functionality to IRMQTTServer (#677)
- Initial prototype of a common interface for all A/Cs. (#664)
- Improve MQTT topic usage for feedback messages. (#663)
- Add multiple independent GPIO sending support via MQTT. (#661)

**[Misc]**
- Adjust kGreeHdrSpace to 4500 (#684, #686)
- Add Home Assistant mqtt climate instructions. (#682)
- Implement htmlEscape() to prevent XSS etc. (#681)
- Add F() Macros (#670)
- Update Daikin2's Cool mode min temp to 18C (#658)
- Change per byte bit-order in Electra protocol. (#648)
- Improve Daikin2 power on/off. (#647)
@crankyoldgit crankyoldgit mentioned this issue Apr 30, 2019
Merged
crankyoldgit added a commit that referenced this issue Apr 30, 2019
@crankyoldgit
v2.6.0 release (#692)
528849e 
_v2.6.0 (20190430)_

**[Bug Fixes]**
- Fixed problem where LG protocol used wrong duty cycle for repeat. (#687)
- Fix checksum calculation for Daikin protocols. (#678)
- Fix the byte array version of sendGree() (#684, #685)
- Fix artificial vs. real state creation on HaierAC. (#668, #671)
- Fix issues caused by having `MQTT_ENABLE` set to false. (#677)
- Fix compile problem when DEBUG is defined. (#673, #674)
- Fix Minor bug with MQTT_ENABLE False condition (#654)

**[Features]**
- Experimental support for DAIKIN216 (ARC433B69) (#690)
- Experimental support for Mitsubishi Heavy Industries A/Cs. (#660, #665, #667)
- Support more features of TCL A/C (#656)
- Add LEGO(TM) Power Functions IR protocol. (#655)
- Add Panasonic AC RKR model & Example (#649)
- DAIKIN/IRDaikinESP overhaul and add Comfort mode support. (#678)
  **WARNING**: Previous `sendDaikin()` calls may not work.
               Please recapture codes or use `kDaikinStateLengthShort` for
               `nbytes` in those calls.
- IRMQTTServer: Move MQTT server and other parameters to WifiManager. (#680)
  **WARNING**: Previous users may need to fully wipe/reset the
               SPIFFS/WifiManager settings by visiting
               `http://<your_esp8266's_ip_address>/reset` prior to or
               after update.
- Add Wifi filtering options to IRMQTTServer. (#679)
- Add advanced aircon/climate functionality to IRMQTTServer (#677)
- Initial prototype of a common interface for all A/Cs. (#664)
- Improve MQTT topic usage for feedback messages. (#663)
- Add multiple independent GPIO sending support via MQTT. (#661)

**[Misc]**
- Adjust kGreeHdrSpace to 4500 (#684, #686)
- Add Home Assistant mqtt climate instructions. (#682)
- Implement htmlEscape() to prevent XSS etc. (#681)
- Add F() Macros (#670)
- Update Daikin2's Cool mode min temp to 18C (#658)
- Change per byte bit-order in Electra protocol. (#648)
- Improve Daikin2 power on/off. (#647)
@crankyoldgit
Copy link
Owner

FYI, this has been included in the newly released v2.6.0 of the library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@crankyoldgit crankyoldgit

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Adjust kGreeHdrSpace to 4500 crankyoldgit/IRremoteESP8266
2 participants
@crankyoldgit @enriqueolivieri