SwiCago / HeatPump

Arduino library to control Mitsubishi Heat Pumps via connector cn105
GNU General Public License v3.0
823 stars 228 forks source link

New Home Assistant Integration with Native MQTT Component #138

Closed unixko closed 4 years ago

unixko commented 5 years ago

Home Assistant updates continuously and frequently breaks custom components. To integrate Mitsubishi Electric heat pump without further hiccup in every HA updates, I propose to use native component instead. So I modified mitsubishi_heatpump_mqtt_esp8266_esp32 code to utilize MQTT HVAC component instead of custom component. If you replace my code to your esp-8266 board, you don't have to reconfigure HA. Then if it works fine you can remove both custom component file and configuration in climate: section from HA.

I'm testing it for a while and it survives 0.96 update. However it still has some known issues which I need help from you guys to advise. My code is here:

https://github.com/unixko/MitsuCon

@SwiCago Please kindly leave this non library-related topic for a month so I can communicate to other HA users. Thanks.


UPDATE: @dzungpv has another Enhanced code with more functions like WiFiManager and Web User Interface.

https://github.com/dzungpv/mitsubishi2MQTT

SwiCago commented 5 years ago

@unixko if your external integration is good and clean and you or others will maintain it, I rather delete the POS HA integration off my library and add a link to yours. I have done more code checking of pulls related to HA, then the library itself....which is sad.

dzungpv commented 5 years ago

This not need to rewrite any: https://gist.github.com/kmdm/29f740e5f36036fb23daba8f2109c359 And there is a lib already what you do: https://github.com/gysmo38/mitsubishi2MQTT

dzungpv commented 5 years ago

@unixko what is the current issue with your code?

kmdm commented 5 years ago

My gist is a stopgap to avoid reflashing (for whatever reason you may have).

Flashing a proper native solution would be much better!

dzungpv commented 5 years ago

@kmdm my PR has been approved https://github.com/SwiCago/HeatPump/pull/139. Hass make their change for the final 1.0 https://developers.home-assistant.io/blog/2019/07/03/climate-cleanup.html, so it will not change often in the future and i am still use custom component. It is the most stable so far (after update each breaking change)

unixko commented 5 years ago

@dzungpv Thanks for asking. I've tried stopgap solution before I decided to go native integration route. I also added HA MQTT discovery function to my code so user doesn't have to configure HA. Without native integration, you can't use Area feature. Also if you own many heat pump units configuration.yaml is really mess.

My current code is working with native HA 0.96 but after 2-3 weeks esp8266 hangs and need to restart. I suspect that I did something wrong in fundamental about declare variable type, process string or JSON, append instead of replace array, etc., that cause memory full on board. However I can't find it yet.

For PR https://github.com/SwiCago/HeatPump/pull/139 I have the same comment with https://github.com/SwiCago/HeatPump/pull/137#issuecomment-513530500

According to dev blog, they don't use AUTO mode as before. So we should map HA heat_cool to ME AUTO mode instead, like mapping HA fan_only to ME FAN.

for hvac_action, I think it means whether a heat pump compressor is actually running or not rather than just current_operation. A library already return Operating as True=running and False=idle. In COOL or HEAT mode we can return correct hvac_action by checking Operating parameter. But in AUTO (heat_cool) mode when Operating=True we don't know it is CURRENT_HVAC_HEAT or CURRENT_HVAC_COOL. I think we need to ask @SwiCago to update library to return more details, if possible. I've given up on updating custom component following HA. Would you consider migrating to MQTT HVAC component instead?

Edit: If we compare Temperature and RoomTemperature along with Operating parameter we can assume hvac_action for CURRENT_HVAC_HEAT, CURRENT_HVAC_COOL, CURRENT_HVAC_IDLE. However native support from library itself is better.

kmdm commented 5 years ago

@unixko I still need to handle that in my gist but I wanted to wait until I updated to 0.96 myself.

unixko commented 5 years ago

@SwiCago I agree that someone should help you maintain HA code instead. Recently it had pull request about HA every month which should be very annoying for you who don't even use HA. If no current code owner I can help you maintain current code (custom component + arduino sketch) along with my new native integration code. At least I can test on actual unit before merge. Anyone if you can help to maintain code instead I'm very welcome.

kmdm commented 5 years ago

@unixko This is a library project. The HA code and other integration code should not be here, IMHO.

dzungpv commented 5 years ago

@unixko with my aircon, the temperature/mode set by machine itself with AUTO, if in auto mode, when set temp below 29 C degree, it is cool and then from 29 and above it is heat, but i am not sure about other device, also i have 1 device that do not have heat mode, only cool. But i can rewrite that.

About native support, i think it is great, and you are right about Area, but software need to test and running long time before we make sure it stable, so that why i stay with stable first.

@kmdmit It is not wrong when store code for HA here, because it is example use, but he could say he will not support/maintain it.

dzungpv commented 5 years ago

@SwiCago about issue with HA, you can create an issue template say that: you not support it, and show the link to chat or other repo, so it will not annoy with issue relate HA integration.

kmdm commented 5 years ago

@dzungpv The example uses should just be links to other git repos, not storing the code.

For "example" use the existing sketches are sufficient and demonstrate how to bring the functionality out to MQTT, e.g.

SwiCago commented 5 years ago

@unixko, @kmdm is right, integrations do not belong in a library. @everyone_else I am going to leave this issue open for a bit, to let you guys continue with what is the best coarse of action. If you guys can not agree to work together on a single flavor of HA, I will just delete it all together and add a link to all 3 gists/gits I have so far seen. But I rather link to one, so as not to confuse noobs

kmdm commented 5 years ago

I'll leave this to @unixko, @Lopton and @gysmo38 as those who have the native mqtt implementations.

The gist is suboptimal but works with the current example sketch (to provide an easier transition to the mqtt component in HA for those not wanting to flash a new sketch).

dzungpv commented 5 years ago

@unixko I see the bug with default mqtt example, sometimes set temperature turn of the devices, it is not your fault.

Matt-PMCT commented 5 years ago

The problem I have with both @unixko and @gysmo38 implementations is that it only is for 8266 boards and I choose to use ESP32. I started with the native SwiCago code that supports both 8266 and ESP32 then integrated in the Native MQTT for Home assistant.

I also don't think that @SwiCago needs to be wasting his time with trying to manage the integration's, I also don't mind add people to my repository if they want to help keep it up to date.

In case no one has said it in a while to you, thanks @SwiCago for the library, it is a lifesaver :)

kmdm commented 5 years ago

@dzungpv I used to have that exact bug with the custom component but never had since switching to the MQTT component.

(The example sketch isn't the problem, IMHO)

dzungpv commented 5 years ago

@kmdm what code you use? unixko say he had the same problem with mqtt component

kmdm commented 5 years ago

@dzungpv All I saw @unixko say above was that his code crashes and needs to restart after a while (which isn't the same thing).

dzungpv commented 5 years ago

@kmdm He said here https://github.com/unixko/mehpha/blob/master/README.md

shampeon commented 5 years ago

The problem I have with both @unixko and @gysmo38 implementations is that it only is for 8266 boards and I choose to use ESP32. I started with the native SwiCago code that supports both 8266 and ESP32 then integrated in the Native MQTT for Home assistant.

Both of those projects could easily do the same as the SwiCago .ino file and just add some #ifdefs for ESP32 with the appropriate includes, and then you'd add #define ESP32 in the .h file.

shampeon commented 5 years ago

The key issue is in the MQTT component is the hang after 2 weeks (memory leak?). "Restarting the device" means cutting breaker power to the units, and that's not acceptable. Until that is addressed, I'm not going to move away from the custom component.

SwiCago commented 5 years ago

All I can tell you guys, is the only time my setup gets restarted is when there is a power outage. Which happens about once per year. I have my own custom automation system running on an openwrt reuter, which I use in combination with mqttdash if I want to override any automated settings. I am using the same code as the primary example in this library, so it isn't the code. My MQTT setup is busy too, it runs 6 HeatPumps, 2 remote temperature sensors, a 6 zone hydronic controller, a 4 outlet controller for a terrarium, a coffee machine controler, washer/dryer, a chicken coop door controller and a sauna LOL....so lots of TCP traffic all using esp8266. I also plan to install 10 RGBW LED bulbs with MQTT and steam generator controller LOL, I am confident in my firmwares and I am confident my setup can handle all the load. Not to sound snobby, but with that said, I know it isn't this code that has issues, but rather how people are using it. Either HA problems or poor wifi or some other network issues. @Lopton , thanks for the appreciation.

shampeon commented 5 years ago

@SwiCago I've been using your code for a long time now, and it's been rock solid, and so I should say I appreciate you maintaining it.

unixko commented 5 years ago

Thanks everyone for feedback. I have 5 heat pump units at home and 15 units at office so I really understand how pain to get a stair to climb up to reflash all 20 units. I want something that "just work" in long run too! I propose architectural change first so we can go to the same direction rather than patch up every month. IMHO, we have at least 3 models:

Good - MQTT example code <-> HA Custom Component Better - MQTT native code with discovery function <-> native HA MQTT HVAC component Best - code with API like REST <-> built-in HA Mitsubishi component without MQTT middleware (something like Daikin component)

The Good/Current model was really work for long time back when HA MQTT component can't send control command. So custom component was an only choice. However it is always broken by HA update which is a real pain for all users/patchers/maintainers.

So how about this plan:

  1. I will copy current HA code to a new repo as HA "Classic" code. Keep maintain it until have native code. So no more HA things to bother @SwiCago from now on. We will also have space to discuss and support users rather than opening non-library related issue here.
  2. Then when we have a really work repo from anyone I will link to that. We have about 4 months before HA turns 1.00 to test and handle more changes to come.

Sounds good?

unixko commented 5 years ago

@dzungpv @kmdm Sorry for confusion. I may mix up test results from custom and native code since I have a lot of units using both codes. I will test again to separate my 2 issues (turn off when change temp / crash after long run) and report back. However it is hard to reproduce both issues.

@Lopton @shampeon is right. We can add support to ESP32 easily by just few lines. In fact it was added to example code just few months ago.

unixko commented 5 years ago

Quick update: I confirm that my native code seldom DOES unit turn off when changing temperature. @dzungpv did you try @kmdm stopgap solution?

dzungpv commented 5 years ago

@unixko I am not testing it, but default mqtt example, so it is the bug in that mqtt code or library itself. And i also have stability issues when running for some week, i have open issue one, but the author close it because not relate to lib. So much be problem some where, we just not find it.

I add more detail about stability, when running for some week, wifi and mqtt still connected, i could ping or send mqtt to it, but esp8266 could not read or send the command or the aircon could not respond, I have discuss this once on the chat room.

ghost commented 5 years ago

@unixko Any update on this as to whether it will work properly on HA 0.97.x? There was some pretty major overhauls of the Climate component in 0.97 and it seems to break the integration. Also, does using the integrated MQTT Thermostat function retain all the same features that were available with Custom Component?

unixko commented 5 years ago

@austwhite Native code works with 0.97.2. Native code has all functions of Custom code plus it has Auto Discovery function hence it also support "Area" feature in HA Integration.

unixko commented 5 years ago

Last week I had a chance to test more. I installed 5 units with my native code.

ghost commented 5 years ago

@unixko Having to power cycle every 4 days would kill the WAF considering the older version didn't ever need a reboot at all. I'll keep an eye on this thread. Let me know if you work out the memory leakage issue. I am not that good with Arduino sketches, but I'll have a quick look.

gysmo38 commented 5 years ago

 @Lopton and @shampeon I have only ESP8266 but I will be happy if you want to add pull request on my github to add ESP32 support. It is very first stage of dev my module, I will try to find time to add more features.

ghost commented 5 years ago

@unixko I noticed you were using LWT in your sketch. I have found that unreliable in the past, in Tasmota, ESPEasy and other smaller sketches. I commented out the line rootDiscovery["avty_t"] = "~/tele/lwt"; and now I find it does not go to unavailable at all. Just thought it may be worth trying if you still have that issue.

dzungpv commented 5 years ago

@austwhite I use LWT before with esp8266 and it work fine, in the code he forgot to update "online" state frequently

unixko commented 5 years ago

@austwhite Thanks for your comment. I tried to comment out avty_t line and it really survived "unavailable" state. I'm very glad that we can scoped down issue.

@dzungpv Thanks for your help. I had thought that checking MQTT connection in mqttConnect() is enough for LWT. Currently I'm testing your code on another unit. On 5 units:

HP1 - old example code HP2 - my new code HP3 - my new code beta HP4 - my new code with LWT update @dzungpv HP5 - my new code without LWT @austwhite

I notice that in latest HA 0.98 the unavailable cycle is faster than before. Previously I can't debug easily because it takes weeks or months for error but now it takes only days which is good. Moreover I didn't freeze my testing environment. I upgraded HA, MQTT, router, access point, etc. so I can't compare result directly. I will report testing result back soon.

dzungpv commented 4 years ago

@unixko I have not try, but your native code support HVAC Action?

aqualx commented 4 years ago

@unixko: using your native HA code on ESP-01S + 5v to 3.3v board for 3 weeks already. No issues so far at all. Thanks!

unixko commented 4 years ago

@dzungpv not yet. We have to wait for HA MQTT HVAC component to support it first. Anyway I passed all parameters received from controller to HA as attributes for convenient uses and just renamed Operating to hvac_action for future mapping.

@SwiCago in AUTO mode, can we get information about which direction of reversing valve? So we can tell operating is heat/cool/idle rather than just compressor is working(true)/idle(false).

ghost commented 4 years ago

@Unixko The library already does show heating or cooling. The classic version with custom component already shows Idle, Heating or Cooling as action in HA 0.96.x I still use that on some devices

dzungpv commented 4 years ago

@ghost, @unixko I added native HVAC action support here https://github.com/dzungpv/mitsubishi2MQTT/commit/b08cf9629756072aed131759a0f413f37de219c3 But this native project still has some stability with Wemos D1. I like it over unixko one because it support config wifi and mqtt on the web, no need to fixed when flash the chip and support html control page

unixko commented 4 years ago

@dzungpv I think you use definition of hvac_action from behavior of HA <0.96 that will shade graph in green when unit is ON whether compressor is operating or idle. So your code both in custom component and especially in mitsubishi2MQTT you removed checking value of operating. In my understanding hvac_action means whether compressor is running or not which is directly related to energy consumption. Below is my definition. Which definition is better please discuss.

table

However, I have one problem in my definition. If MODE is auto or heat_cool and operating is true, we can't tell that heat pump is heating or cooling . One way to derive information without support from library, I guess, is to check value of [TARGET_TEMP - CURRENT_TEMP]. If value has plus sign it is heating and if value has minus sign it is cooling.

Anyway you are more advance than me in this issue. So I should wait and see your code first as a guideline before I start :-)

dzungpv commented 4 years ago

@unixko yes, i have remove operating because not all mode operating is true, so it always idle for fan_only mode for example, an the operating value change very fast from true to false (in cool, heat or dry mode) so it failed to update in mqtt climate, always show "idle" .

Additional I handle power state "ON" or "OFF" in native code combined with mode "OFF", it could prevent turn off the machine when you change the temperature.

In HA they defined CURRENT_HVAC_OFF so when it off it is "OFF". With your table we could improve with operating state, but need to check it more, to do it quite easy, just change the action_template

About the "auto" mode (Heat Cool), native mode i see it not work accurate event i use Mitsubishi remote control, When i change to "Auto" it often heat but not cool even my room temperature is 28 degree.

Updated:

I also test native Auto mode, set temperature to 24 degree, after it reach 24 (room temperature) i change it to 26, check the aircon, it show operating false and only fan working and not Heating, so [TARGET_TEMP - CURRENT_TEMP] useless in this case, I also find out that it let room reach 26 degree by heat from our body, after room reach 26 operating change to true and it is cooling.

Personally I never use Auto mode, if we use HA, there are many way to make Aircon more smart without using Auto mode

dzungpv commented 4 years ago

@unixko I read their manual about Auto mode here: http://www.mitsubishi-electric.com.au/assets/LEG/MSZ-GA22-25-35VA_MSZ-GB50VA-SG79Y404H03.pdf They say: "Description of “AUTO CHANGEOVER” (1) Initial mode: 1 When unit starts the operation with AUTO operation from off; • If the room temperature is higher than the set temperature, operation starts in COOL mode. • If the room temperature is equal to or lower than the set temperature, operation starts in HEAT mode. (2) Mode change: 1 COOL mode changes to HEAT mode when about 15 minutes have passed with the room temperature 2 degrees below the set temperature. 2 HEAT mode changes to COOL mode when about 15 minutes have passed with the room temperature 2 degrees above the set temperature. NOTE: If two or more indoor units are operating in multi system, there might be a case that the indoor unit, which is operating in (AUTO), cannot change over to the other operating mode (COOL↔↔↔↔↔HEAT) and becomes a state of standby. Refer to the detailed information for multi system air conditioner explained on the right"

So we can simulate this behavior in software or implement:

lekebob 0x09 stage info 
Byte 10:
Only in "Auto" mode is shows what the unit is doing
0x01 is cool
0x02 is heat
unixko commented 4 years ago

@dzungpv Thanks for information. Here is my version of template. I think it covers all cases avoiding nested if. I intentionally omit else since HA will display mode for blank action. I don't know how to refer to its own entity_id in template to get attribute so I have to pass device name in value_json.

{%set value_json={"name":"Office Air conditioner","operating":true}%} // for template testing only

{%set hp='climate.'+value_json.name|lower|replace(' ','_')%}
{%if states(hp)=='off'%}off
{%elif states(hp)=='fan'%}fan
{%elif value_json.operating==false%}idle
{%elif states(hp)=='cool'%}cooling
{%elif states(hp)=='heat'%}heating
{%elif states(hp)=='dry'%}drying
{%elif states(hp)=='heat_cool'and(state_attr(hp,'current_temperature')>state_attr(hp,'temperature'))%}cooling
{%elif states(hp)=='heat_cool'and(state_attr(hp,'current_temperature')<=state_attr(hp,'temperature'))%}heating
{%endif%}

About 0x09 package @SwiCago already has placeholder in his code. I'm studying.

https://github.com/SwiCago/HeatPump/blob/8fa2f159b5823fd107e513426e1d4284bfcbf01f/src/HeatPump.cpp#L671

dzungpv commented 4 years ago

@unixko use my template here: https://github.com/dzungpv/mitsubishi2MQTT/blob/cbde6010d3d77b930817e7f21ab2de9201f7dae0/src/mitsubishi2mqtt/mitsubishi2mqtt.ino#L852.

in that repos i add support for cooling and heating, not the Heat/Cool, don't get HA variable, you can just compare: value_json.temperature and value_json.RoomTemperature

dzungpv commented 4 years ago

@unixko You can test my code with Auto mode operation here: https://github.com/dzungpv/HeatPump/commit/daaac03ddf39a4e3ede1ff34e2b6e4bf98a8ee93

austwhite commented 4 years ago

I have temporarily reverted back to the Custom Component as I had a weird issue with one of my heatpumps with native code. I think it is unique to my end so I will investigate. Custom component works in latest 0.99 HA, but needs HVAC_MODE_AUTO to be changed to HVAC_MODE_HEAT_COOL to match the Home Assistant Climate component.
Home Assistant specify Auto is only for use with Scheduling and AI systems now and the old auto change over mode is now HVAC_MODE_HEAT_COOL.

From https://developers.home-assistant.io/blog/2019/07/03/climate-cleanup.html

We split HVAC mode auto into auto and heat_cool. If it's heat_cool, the user has set a temperature range the device has to use heating and cooling to stay within. Auto mode is now limited to devices that are running on a schedule or AI.

I noted @dzungpv made the last update and wondered if maybe could take a look at this? I don't know enough python to make the needed changes. This seems to be the only change needed to keep the Custom Component compliant with the latest HA for those still using it.

dzungpv commented 4 years ago

@austwhite use this native https://github.com/dzungpv/mitsubishi2MQTT/releases by @gysmo38 and improved by me. It work out of the box without custom component and HA 0.99, if you use wemos d1, just download the bin file and OTA with https://1st.bitbumper.de/ota-firmware-update-tool-for-esp8266/ . After update just delete custom component and relate climate settings. See detail guide here https://github.com/dzungpv/mitsubishi2MQTT/blob/master/README.md.

I have used it for weeks, note that if your mqtt offline over 15 minutes, just enter the hvac webpage and reboot it when mqtt server back online.

austwhite commented 4 years ago

@dzungpv I'll look into that version of native code.
I think long term we need to settle on just one native code that works and link to that :).

Was just reading your INO source and noticed a typing error. Only a silly cosmetic one, but you might want to see it.

haConfigDevice["mdl"] = "HVAC MITUBISHI";