gysmo38 / mitsubishi2MQTT

Mitsubishi to MQTT with ESP8266 module
GNU Lesser General Public License v2.1
390 stars 139 forks source link

Changing mode causes wild flickering of MQTT states #94

Closed melyux closed 3 years ago

melyux commented 4 years ago

When you change the mode from Cool to Off, or vice versa like below, the topic/state topic reports the new state, then goes back to the old state, then back again to the new state, and so on for a while until it settles down. This causes a lot of flickering of state and unsureness, especially in Home Assistant.

Setting the HVAC mode from OFF to Cool:

  1. We publish "ON" to topic/power/set
  2. We publish "cool" to topic/mode/set
  3. We receive the following sequence of MQTT messages in rapid succession:
    • topic/state: {"roomTemperature":74,"temperature":74,"fan":"AUTO","vane":"SWING","action":"cooling","mode":"cool"}
    • topic/state: {"roomTemperature":74,"temperature":74,"fan":"AUTO","vane":"SWING","action":"off","mode":"off"}
    • topic/settings: {"temperature":74,"fan":"AUTO","vane":"SWING","mode":"cool"}
    • topic/state: {"roomTemperature":74,"temperature":74,"fan":"AUTO","vane":"SWING","action":"cooling","mode":"cool"}

In this case, it only swung once, but this can happen several times.

I see the hpSendDummy function attempts to do this, but the program should also block further state updates from the HVAC for a certain amount of time, and only after that timeout start passing along the HVAC's information, by which time the new mode setting will have hopefully stuck.

EDIT: It's happening for any setting change. There's a moderately long delay after any setting change, and it's causing the race condition between the settings "sticking" and next regular interval state reporting. You can see this on the web server as well. Change the fan speed, and then after the page refreshes, refresh the page again and you'll see your old setting because it hasn't "stuck" yet.

dzungpv commented 3 years ago

I guess you are not using Home Assistant. With home assistant you just send ONE cool command and it work. There are some delay with the hp so it will respond to the command after few seconds. If you only use mqtt, better use default example https://github.com/SwiCago/HeatPump/blob/master/examples/mitsubishi_heatpump_mqtt_esp8266_esp32/mitsubishi_heatpump_mqtt_esp8266_esp32.ino , you can only send 1 command to turn on the hp.

melyux commented 3 years ago

I am using Home Assistant. I send the cool command, the ESP responds with a confirmation state (“dummy state”) MQTT message so HA will be happy. But the HVAC continues sending its old state for up to 30 more seconds, overwriting the dummy state soon after, causing the state to revert to the old one for a while.

So the existing dummy state mechanism doesn’t work very well because it’s not enough. It has to suppress the regular state updates for a while so all its hard work won’t be for nothing.

Unless I’m the only whose HVAC sends regular updates over MQTT instead of only when its status actually changes.

dzungpv commented 3 years ago

I am using Home Assistant. I send the cool command, the ESP responds with a confirmation state (“dummy state”) MQTT message so HA will be happy. But the HVAC continues sending its old state for up to 30 more seconds, overwriting the dummy state soon after, causing the state to revert to the old one for a while.

So the existing dummy state mechanism doesn’t work very well because it’s not enough. It has to suppress the regular state updates for a while so all its hard work won’t be for nothing.

Unless I’m the only whose HVAC sends regular updates over MQTT instead of only when its status actually changes.

Dummy message only make HA ui update immediately with your command, and after HP will send back the update. I have 4 and it not the problems. You can use MQTT.fx to re check the message. If it is automation, recheck it. Check message with command from the UI and from the automation/services

melyux commented 3 years ago

Sorry we’re not covering any new ground here. Not sure if you’re getting what I’m saying. I’ve checked many times and looked at the bare MQTT messages being sent and received.

First, I send the Home Assistant (or bare MQTT) command. Then, Mitsubishi2MQTT sends a dummy MQTT packet reflecting the changes I’ve made. After that, the HVAC responds immediately to the command.

Yet, for the next 20 to 30 seconds, the HVAC continues to periodically send status updates (once every few seconds) that reflect the old state. This undoes the work the dummy packet is doing. Now Home Assistant thinks my AC is turning on, off, and on again in the next 30 seconds.

This may be a new thing with my HVAC, which is a relatively new model. Again I have to ask: If you use an MQTT monitoring tool, do you see that you get /state messages periodically, or only when something changes? I get them periodically, which is strange because the method that generates them is called hpStatusChanged, which implies it should only send if the status actually changes.

dzungpv commented 3 years ago

Sorry we’re not covering any new ground here. Not sure if you’re getting what I’m saying. I’ve checked many times and looked at the bare MQTT messages being sent and received.

First, I send the Home Assistant (or bare MQTT) command. Then, Mitsubishi2MQTT sends a dummy MQTT packet reflecting the changes I’ve made. After that, the HVAC responds immediately to the command.

Yet, for the next 20 to 30 seconds, the HVAC continues to periodically send status updates (once every few seconds) that reflect the old state. This undoes the work the dummy packet is doing. Now Home Assistant thinks my AC is turning on, off, and on again in the next 30 seconds.

This may be a new thing with my HVAC, which is a relatively new model. Again I have to ask: If you use an MQTT monitoring tool, do you see that you get /state messages periodically, or only when something changes? I get them periodically, which is strange because the method that generates them is called hpStatusChanged, which implies it should only send if the status actually changes.

You can try clean the ESP and re flash it, for me state only update each 30 seconds https://github.com/gysmo38/mitsubishi2MQTT/blob/fc1dd771bbd50597b6257396ecef67ff8e9c3264/src/mitsubishi2mqtt/mitsubishi2mqtt.ino#L1765 and every time i change climate ui on the HA, not repeat like you.

melyux commented 3 years ago

Okay, so that confirms that the periodic update thing is for everyone. So there's a race condition there: if it hits that 30-second mark before the HVAC starts sending the correct state, then you will get the problem I'm describing. I'm not sure why you're not seeing it, since this race condition should resolve in the outdated state's favor a lot of the time. Maybe your HVAC acknowledges and starts sending the updated state very quickly compared to mine.

In any case, a solution could be to put another condition in that line to check if a dummy packet was sent recently, and not send the periodic update in that case. Maybe setting lastTempSend to the current time when a new command is sent. There's some further logic to be considered though (Should we use a set blocking time, or end the timeout early by checking the status every now and then so temperature updates in the meantime aren't ignored? What will happen if the user sends another command during the wait?).

ajobbins commented 3 years ago

Having this issue too in HA and is really bothering me. A while ago I was using a different variant of the SwiCago code, but don't recall which one, and this issue didn't happen - the state would update immediately in HA when a change was made and would not flap around at all

melyux commented 3 years ago

@ajobbins It would be really cool if you could find which solution you were using, it'd be invaluable seeing someone else's solution to this.

ajobbins commented 3 years ago

@ajobbins It would be really cool if you could find which solution you were using, it'd be invaluable seeing someone else's solution to this.

I tried to check but the files would have been on an only laptop that I don't have any more, so really not sure. It may even have been an earlier version of this library.

Would setting optimistic mode in HA help? That would assume the settings took until next update from the HP on MQTT. I'm pretty sure the HP isn't reporting back an incorrect state by serial once it's recieved the command - it must be the code of this library if incorrect statuses are being published back to MQTT after the change is made.

melyux commented 3 years ago

@ajobbins It looks like this code is based on SwiCago's existing MQTT example, which handles things the same way. You'd think it would work well...

But I did find that this code indeed does what I asked of it. It already sets lastTempSend = millis(); in the last line of the hpSendDummy() function, which is exactly what I thought needed to be done. It does this right after it publishes the dummy packet. I'm going to try moving this set to right before the publishing to avoid the race condition and test it, though I'm not holding my breath since the existing delay should have been like a few nanoseconds.

melyux commented 3 years ago

I've solved it through some code refactoring, testing it now. It works well and should be a good improvement for everyone. The main issues were these:

  1. The code sends dummy packets as last status from the HVAC + only the last changed setting changed, so if you make multiple changes within a span of about ~30 seconds, all but your last change will be erased by the very last dummy packet.

  2. It sometimes takes the HVAC just over 30 seconds (the default periodic time) to acknowledge commands.

  3. It takes the HVAC ~14 seconds to make subsequent changes if more than one command is sent at the same time, e.g. when mode is set to off → /power/set OFF and /mode/set off are sent at the same time, with each callback erasing the dummy packet and providing outdated information.

I've solved each of these by increasing the default periodic time from 30 to 45 seconds, instituting a local state instead of relying on the HVAC's state or simply sending a delta dummy packet, and disabling callbacks for the periodic timeout (i.e. 45 seconds) after commands are sent.

Will do a PR soon, testing it for (any more) potential bugs.