No support for tracking power state when receiving signals for Airwell and Whirlpool ACs

[ayavilevich]

Version/revision of the library used

2.7.10

Expected behavior

When receiving IR signals from physical remote for Airwell AC (and others such as Whirlpool) the power that is reported should be power state of the AC. Whether it is on or off. Disregarding any issues that could arise from getting out of sync (as there is no feedback).

Actual behavior

The power that is reported in the state object is actually "power toggle" and not "power".

This seems to be like this because ac.toCommon() for these ACs doesn't accept a previous state despite these ACs having a _.PowerToggle field rather than Power. So IRAcUtils::decodeToState doesn't pass prev is available.

There are vendors that currently accept "prev" in their "toCommon". for example COOLIX.

Output of raw data from IRrecvDumpV2.ino (if applicable)

IRrecvDumpV2 behaves correctly because IRAirwellAc::toString reports the field as kPowerToggleStr. It doesn't report "power". It also doesn't try to generate state.

Steps to reproduce the behavior

Here is one example that you can transmit:

Protocol : AIRWELL
Code : 0x40580002 (34 Bits)
Mesg Desc.: Power Toggle: Off, Mode: 1 (Cool), Fan: 0 (Low), Temp: 26C
uint16_t rawData[99] = {2922, 2934, 972, 980, 994, 958, 964, 1966, 1942, 988, 966, 986, 998, 954, 998, 954, 968, 984, 968, 984, 1002, 1928, 1948, 1958, 972, 980, 1950, 978, 964, 988, 964, 988, 966, 986, 998, 956, 966, 986, 968, 984, 1000, 952, 968, 984, 970, 982, 970, 982,
972, 980, 972, 980, 974, 978, 964, 988, 964, 988, 966, 1964, 1942, 1108, 2924, 2934, 962, 990, 964, 988, 964, 1964, 1942, 986, 966, 986, 968, 984, 968, 984, 968, 984, 970, 984, 970, 1958, 1948, 1956, 974, 978, 1940, 988, 964, 988, 964, 988, 966, 986, 966, 986, 972}; //
AIRWELL 40580002
uint64_t data = 0x40580002;

I have followed the steps in the Troubleshooting Guide & read the FAQ

Yes

Has this library/code previously worked as expected for you?

No but it got better. Previously didn't detect this AC at all.

Other useful information

Related to discussion in #1228

There you explain that ac.sendAc and IRac::handleToggles address this issue for sending. Including for the ACs mentioned here.

[crankyoldgit]

@ayavilevich Can you please try out the code in this branch: https://github.com/crankyoldgit/IRremoteESP8266/tree/Issue1275 and let me know if it solves the issue you're describing.

Regarding:

When receiving IR signals from physical remote for Airwell AC (and others such as Whirlpool) the power that is reported should be power state of the AC. Whether it is on or off.

I disagree. The library should report a "toggle" rather than the ultimate state of the A/C, as that is exactly what the message contains/is/says etc. If we force down that level of abstraction onto the raw protocol, that removes flexibility & granularity of operation to a user of the library.

However, I do agree that adding the option for the library to do some of that (via a prev) for the user when using stuff in the cooked IRac class-level interface, hence I've added it.

TL;DR: I disagree with your wording of the "expected" behaviour, but I agree with the sentiment. ;-)

[ayavilevich]

@crankyoldgit thanks for the patch. I will check it and report. Might take me a few days as I will need to dive deeper into the code.

Regarding expectations, let me elaborate on what I meant.

I think the disagreement is because of naming.

I agree that the library should provide receiving and parsing of messages (messages/results/packets/events, etc) in their raw form. We should not hide that away. But we should not call that "state".

Once we call something "state", and once it has a property called "power" there is an expectation.

[ayavilevich]

@crankyoldgit

Another question. I noticed that handleToggles does
if (prev != NULL && desired.protocol == prev->protocol && desired.model == prev->model)

where decodeToState makes no check on prev.
should decodeToState be changed or should it be the responsibility of the caller to verify that prev matches?

[crankyoldgit]

Once we call something "state", and once it has a property called "power" there is an expectation.

Re: "state". No argument. The naming is less-than-perfect. But we live in a less-than perfect world. ;-P

As for "power", hey, there is no definition of what it means. It could me "is there or isn't there a power toggle bit set?" :-)

But seriously, there is no way to make a short & simple unified programmatic representation of all A/C operations, settings, & controls. IRac & state_t is an attempt to shoe-horn everything into a rough common-ish feature set. It is NOT an attempt to be a perfect translation.

[crankyoldgit]

@crankyoldgit

Another question. I noticed that handleToggles does
if (prev != NULL && desired.protocol == prev->protocol && desired.model == prev->model)

where decodeToState makes no check on prev.
should decodeToState be changed or should it be the responsibility of the caller to verify that prev matches?

TL;DR: The whole prev thing in the various .toCommon() and decodeToState() is a horrible hack to deal with odd-ball A/C designs. Realistically I should never have added/allowed it. Only use those if you really know what you're doing, because they can never be perfect.

The longer version:
The protocol & model check in handleToggles() is there because some protocols treat the state differently. As you've pointed out, power for most is "Is the A/C on/off?" etc, where as some use it as "Is a power toggle operation requested?", and some have an even more complicated non-binary power operation. (Don't ask! But it's one of the reasons the prev thing was introduced. Not for toggles, but some A/Cs send different power commands based on the previous/existing state)
Also, it's one single test in a routine that is shared/common/integral to all things in IRac. Where as the various .toCommon() methods belong to individual AC classes, thus require considerable code duplication (read: extra flash space used).

It's at this point I should point out that IRac is intended for controlling A/Cs using a common interface, not a common interface to decode all A/Cs.

I added the existing support for converting to "common" (e.g. .toCommon() and decodeToState()) because some users of IRMQTTServer wanted to be able to also use their physical remotes to control the ESP (and the actual A/C at the same time). I warned that this would be a horrible hack and full of problems, but they were happy to accept that and the limitations. I get the feeling my worries were justified. :-/

In summary, IRac is for user-directed creation and sending. It is not intended or designed to work for accepting & interpreting raw/real IR messages. The fact that it does a fairly good job in combination with the various .toCommon() methods is great, but it can never be perfect. There is just too much variability in design/implementation/properties of the various A/C IR protocols to do a seamless job of decoding into a usable IRac/state_t. e.g. A lot of native A/C formats don't include their entire state in a message, thus we never can perfectly guess what the real state of the remote is, let alone the real state of the physical A/C.

[ayavilevich]

@crankyoldgit Agree with what you wrote above.

This is not perfect but IMO it is the best library in this category so I am happy it does what it does and gives the flexibility for the user to implement variable logic.

Making some code changes, will test the blaster with the new firmware and will update.

[ayavilevich]

@crankyoldgit so far all good. Seems to work.

If you are curious you can take a look at my fork of tasmota:
https://github.com/ayavilevich/Tasmota
the relevant changes are in tasmota\xdrv_05_irremote_full.ino

Asked a few people to help test.

[crankyoldgit]

so far all good. Seems to work.

Thanks for the confirmation. PR #1276 should fix/close out this issue when reviewed/submitted.

[crankyoldgit]

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

[koalazak]

Hello @crankyoldgit!
Is there any way to read the current Whirlpool state from the AC so then decide if we should send the command with "power toggle" flag or without it?

How do you handle th state to keep it in sync using external remotes too?

thank you

[crankyoldgit]

@koalazak In future, please create a new issue rather than hijacking/adding to another long-closed issue.

This is a problem inherent with the one-way nature of typical IR remote communication. You can't verify the state of the AC unit, nor if it got a command.

IRMQTTServer has code to try to do what you're asking by using an IR receiver. You can look through the source code for how it tries to do it. However, inherently, there is no sure-fire way.