After updating to 2.7.0, I've noticed some strange behaviour.
When starting up, a persistent MQTT topic sets the payload 'off' to the ESP8266, which triggers the following code:
if (newTopic == "climate/aircon") {
if ( newPayload == "off" ) {
ac.setPower(false);
ac.send();
return;
}
if ( newPayload == "on" ) {
ac.setPower(true);
ac.setMode(kCoolixCool);
ac.setTemp(aircon_temp);
ac.send();
return;
}
}
When ac.setPower(false) and ac.send() is executed, the aircon turns on and is set to the default mode "Auto / 25c / Auto Fan". Even at this point, ac.toString() will still return "Power: Off".
Wrapping the OFF code as follow works:
if ( newPayload == "off" ) {
if( ac.getPower() ) {
ac.setPower(false);
ac.send();
}
return;
}
However it is possible that if the A/C is already on, and the state is thought to be off, you can't turn the unit off.
Hey Steven (@CRCinAU),
As I think I alluded to in #944 Coolix is an annoying protocol where you have to assume certain things about it's previous state to be optimal with it.
Please see comment I asked: https://github.com/crankyoldgit/IRremoteESP8266/pull/944#discussion_r331738032
I need to know more about this protocol so we can emulate this properly once and for all. I wasn't strictly happy with #944 but I have to trust user data & feedback with how their devices respond as I don't have access to one of these A/C units to do tests myself. :)
As far as I recall/understand it. There is a special (i.e. un-changing) "off" command, and "every other possible command" is assumed to "power on" the device. Is that correct? Can you confirm this please?
In your first block of (existing) code, are there any earlier calls to set any of the state of the IRCoolix ac object?
Can you also provide me with a simple sequence of calls to the ac object (with details of what the operating state is of the A/C unit prior to any .send()s. e.g. "The A/C unit is actually off, and then I called ac.setPower(false); ac.send();" etc. THEN provide me with a the output/capture of IRrecvDumpV2 for the same intended operation but produce using the ACTUAL real IR remote for the a/c unit.
With that data, I can ensure the class emulates the remote given the same conditions etc etc. Do this for all the sequences you want to do, and I can make sure those are correct too.
TL;DR: If you give me the data and flow above, I'll create a series of unit test cases to ensure correct operation now, and going forward.
Hi mate,
Yes - there's a OFF state - of which everything else is an implied ON with parameters.
The full code in question is here:
https://git.crc.id.au/netwiz/ESP8266_Code/src/branch/master/ClimateControl/src/ClimateControl.ino#L154
What I think is happening is that the unit is being set to the default state - even when ac.setPower(false) is being set before the ac.send().
The only calls to ac.send() are within the callback function - which gets called when MQTT data changes. As 'off' and 'on' are persistent values, these get called when the unit gets powered up.
As the A/C is normally off, when I reboot the ESP8266, the topic climate/aircon value off is received, but the ac.send ends up sending the default state.
The default state is defined in ir_Coolix.h - line 84.
So whatever the bug, the library thinks that the state is supposed to be one thing - but ac.setPower(false) doesn't seem to affect that state.
In essence, I don't think this is a communication problem with the protocol - but a logic issue.
EDIT: For what its worth, this logic seemed to work fine in 2.6.6. I tried getting my head around the changes, but having not looked at the meat & gravy of this library for quite a while, I'm not quite sure of the logic changes to have an 'ah ha!' moment as yet.
EDIT 2: I should actually note that if I change the state of the unit (to say 22 / Auto etc), then tell the unit to turn off, the correct power off is sent.
In #944 @ribeirodanielf changed the _default_ state to "powered off". i.e. When the object is created, it assumes the current A/C (real) state is "powered off". Prior to #944 the default power state was assumed to be "Powered on". In order to reduce messages etc, @ribeirodanielf changed it such that if the library things the unit is off i.e. it's internal state is "off", then any subsequent calls to setPower(false)/off() should result in nothing happening. That's if I recall it correctly.
So, back in 2.6.6, the default was to assume the device (or the object's initial state) is already on.
I'm personally torn between the previous (<= 2.6.6) implementation of default "assume power on" v.s. the current (2.7.0+) default of "assume the power is off" initial state. I can see equal pros and cons for either side, and if anything leaning ever so slightly to "power off" as the default, as the probably likely case is the A/C unit is "off" the majority of the time. Dunno. Shrug. I can be swung either way.
If you are correct, then I see three obvious solutions:
a) You add 'ac.setPower(true)immediately after the IRCoolix AC object created in your code. This should mean the persistent "off" message should generate a real "off" message when.send()` is called. (Or you can alter your copy of the library to make the default state "on".)
b) We change the library default official back to "power is on" default (similar to above) and you don't change your personal code.
c) I hack in something that says, don't assume anything. Wait for the first setPower(), on(), or off() call. Always send that "power" message, then assume/use that state from then on. This is more complication and means all other previously written code will not perform as it did. But hey, they both the "default on" and "default off" camps are equally disadvantaged. :-/
Thoughts?
My confusion is about how this is working now...
If the initial state now equals the default state, ie 25 / auto etc when the ac object is created, why doesn't a ac.setPower(false) make the ac.send() send the power off command?
This is where I get stuck in the logic. Should there be a call to set the current state to the default state before I setPower(false)?
My thoughts were that I'm happy to still send an OFF message to the a/c on boot as that gives me a known state to begin with. If the home automation stuff sends an ON, then that will work just fine as well.
As such, I'm happy for the defaults to stay whatever makes more sense - as I only ever update things on a new message from the MQTT source.
Yet when an implied ON is set via a specific ON or a temp change, then setPower(false) works fine.
So, do I need to use ac.begin() to init the first state? Should I try calling something to make sure the default state is configured?
I'm happy for the library to do whatever as a default, but be able to reliably set a known state via code.
On 3 November 2019 6:33:33 pm AEDT, David Conran notifications@github.com wrote:
In #944 @ribeirodanielf changed the _default_ state to "powered off".
i.e. When the object is created, it assumes the current A/C (real)
state is "powered off". Prior to #944 the default power state was
assumed to be "Powered on". In order to reduce messages etc,
@ribeirodanielf changed it such that if the library things the unit is
off i.e. it's internal state is "off", then any subsequent calls to
setPower(false)/off()should result in nothing happening. That's if
I recall it correctly.So, back in 2.6.6, the default was to assume the device (or the
object's initial state) is already on.I'm personally torn between the previous (<= 2.6.6) implementation of
default "assume power on" v.s. the current (2.7.0+) default of "assume
the power is off" initial state. I can see equal pros and cons for
either side, and if anything leaning ever so slightly to "power off" as
the default, as the probably likely case is the A/C unit is "off" the
majority of the time. Dunno. Shrug. I can be swung either way.If you are correct, then I see three obvious solutions:
a) You add 'ac.setPower(true)
immediately after the IRCoolix AC object created in your code. This should mean the persistent "off" message should generate a real "off" message when.send()` is called.
(Or you can alter your copy of the library to make the default state
"on".)b) We change the library default official back to "power is on"
default (similar to above) and you don't change your personal code.c) I hack in something that says, don't assume anything. Wait for
the firstsetPower(),on(), oroff()call. Always send that
"power" message, then assume/use that state from then on. This is more
complication and means all other previously written code will not
perform as it did. But hey, they both the "default on" and "default
off" camps are equally disadvantaged. :-/Thoughts?
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
https://github.com/crankyoldgit/IRremoteESP8266/issues/985#issuecomment-549112362
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
I've experimented a bit more tonight - and this code seems to work as expected:
if (newTopic == "climate/aircon") {
if ( newPayload == "off" ) {
ac.setPower(true);
ac.setMode(kCoolixCool);
ac.setTemp(aircon_temp);
ac.setPower(false);
ac.send();
return;
}
if ( newPayload == "on" ) {
ac.setPower(true);
ac.setMode(kCoolixCool);
ac.setTemp(aircon_temp);
ac.send();
return;
}
}
Thanks. I'll try look into it some more soon (tomorrow). I've been a little busy for the last few days.
@CRCinAU I think I've fixed it. Change is now in the master branch. Please let me know if it doesn't fix it. If it isn't fixed, ping me here and I'll re-open the issue.
Checked out the master branch...
Altered the code to:
if (newTopic == "climate/aircon") {
if ( newPayload == "off" ) {
//ac.setPower(true);
//ac.setMode(kCoolixCool);
//ac.setTemp(aircon_temp);
ac.setPower(false);
ac.send();
return;
}
if ( newPayload == "on" ) {
ac.setPower(true);
ac.setMode(kCoolixCool);
ac.setTemp(aircon_temp);
ac.send();
return;
}
}
It doesn't seem to send anything with the ac.send() in the off case.
Is this because the assumed state is still off on startup? If so, I guess its still valid for me to set it as a power on state, then power off state to ensure that an off can be sent on booting the ESP8266...
Testing further - I believe there may have been an issue - as after testing a few more iterations, just using setPower(false) and send() seems to work on rebooting the unit.
I'm doing some testing with setting the LED high / low on starting the IR transmit - ie in the above function...
Is there a method already within this library to use a status LED that could make this more accurate?
No, there isn't. You could just replace the IR led with a normal one. Or put one in parallel. That will show if it is transmitting or not. I use that on my testing rig.
Ok, for now I've set my code to:
if (newTopic == "climate/aircon") {
if ( newPayload == "off" ) {
//ac.setPower(true);
//ac.setMode(kCoolixCool);
//ac.setTemp(aircon_temp);
digitalWrite(LED_BUILTIN, LOW);
ac.setPower(false);
ac.send();
digitalWrite(LED_BUILTIN, HIGH);
return;
}
if ( newPayload == "on" ) {
digitalWrite(LED_BUILTIN, LOW);
ac.setPower(true);
ac.setMode(kCoolixCool);
ac.setTemp(aircon_temp);
ac.send();
digitalWrite(LED_BUILTIN, HIGH);
return;
}
}
This is probably good enough as an indication.
I can see on reboot now that ac.send() is actually working - and the A/C unit beeps to ACK it. More importantly, the aircon doesn't turn on!
Testing via turning the unit on with the standard remote, then rebooting the ESP causes the system to be turned off again.
Looks like a proper fix! Thanks!
Thanks for the confirmation.
FYI, this has been included in the just released version 2.7.1 of the library.
Most helpful comment
FYI, this has been included in the just released version 2.7.1 of the library.