SwiCago / HeatPump

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

Question #11

Closed uronito closed 7 years ago

uronito commented 7 years ago

Hi al, kayno.

I'm Albertoo from nicegear.nz. Can you help me?

If i try add more mqtt_publish in hpSeetingsChanges the sketch dont' work (only reads temperature) not send commands to heatpump or mqtt /heatpump.

The code i use is:

void hpSettingsChanged() { const size_t bufferSize = JSON_OBJECT_SIZE(6); const size_t bufferSizeDomoticz_power = JSON_OBJECT_SIZE(3); const size_t bufferSizeDomoticz_mode = JSON_OBJECT_SIZE(3); const size_t bufferSizeDomoticz_fan = JSON_OBJECT_SIZE(3); const size_t bufferSizeDomoticz_temperature = JSON_OBJECT_SIZE(3);

DynamicJsonBuffer jsonBuffer(bufferSize);

DynamicJsonBuffer jsonBufferDomoticz_power(bufferSizeDomoticz_power);
DynamicJsonBuffer jsonBufferDomoticz_mode(bufferSizeDomoticz_mode);
DynamicJsonBuffer jsonBufferDomoticz_fan(bufferSizeDomoticz_fan);
DynamicJsonBuffer jsonBufferDomoticz_temperature(bufferSizeDomoticz_temperature);

JsonObject& root = jsonBuffer.createObject();
JsonObject& domoticz_power = jsonBufferDomoticz_power.createObject();
JsonObject& domoticz_mode = jsonBufferDomoticz_mode.createObject();
JsonObject& domoticz_fan = jsonBufferDomoticz_fan.createObject();
JsonObject& domoticz_target_temp = jsonBufferDomoticz_temperature.createObject();

heatpumpSettings currentSettings = hp.getSettings();

root["power"]       = currentSettings.power;
root["mode"]        = currentSettings.mode;
root["temperature"] = currentSettings.temperature;
root["fan"]         = currentSettings.fan;
root["vane"]        = currentSettings.vane;
root["wideVane"]    = currentSettings.wideVane;
char buffer[512];
root.printTo(buffer, sizeof(buffer));
bool retain = true;
mqtt_client.publish(heatpump_topic, buffer, retain);
domoticz_power["command"] = "setuservariable";
domoticz_power["idx"]     = 13;
domoticz_power["value"]   = currentSettings.power;
char buffer_domoticz_power[512];
domoticz_power.printTo(buffer_domoticz_power, sizeof(buffer_domoticz_power));
mqtt_client.publish(domoticz_mqtt, buffer_domoticz_power);
domoticz_mode["command"] = "setuservariable";
domoticz_mode["idx"]     = 12;
domoticz_mode["value"]   = currentSettings.mode;
char buffer_domoticz_mode[512];
domoticz_mode.printTo(buffer_domoticz_mode, sizeof(buffer_domoticz_mode));
mqtt_client.publish(domoticz_mqtt, buffer_domoticz_mode);
domoticz_fan["command"] = "setuservariable";
domoticz_fan["idx"]     = 15;
domoticz_fan["value"]   = currentSettings.fan;
char buffer_domoticz_fan[512];
domoticz_fan.printTo(buffer_domoticz_fan, sizeof(buffer_domoticz_fan));
mqtt_client.publish(domoticz_mqtt, buffer_domoticz_fan);
domoticz_target_temp["command"] = "setuservariable";
domoticz_target_temp["idx"]     = 14;
domoticz_target_temp["value"]   = currentSettings.temperature;
char buffer_domoticz_target_temp[512];
domoticz_target_temp.printTo(buffer_domoticz_target_temp, sizeof(buffer_domoticz_target_temp));
mqtt_client.publish(domoticz_mqtt, buffer_domoticz_target_temp);

}

why cant't publlish mqtt messages ?

Thanks in advance!!!!!!!

uronito commented 7 years ago

I already have it solved :),

One question, it is normal that when sending an order to the heat pump, in the heatpump mqtt topic comes out the repeated command twice.

Thanks in advance !!!!!

topic /heatpump: {"power":"OFF","mode":"FAN","temperature":21,"fan":"1","vane":"1","wideVane":"|"} {"power":"OFF","mode":"FAN","temperature":21,"fan":"1","vane":"1","wideVane":"<<"}

kayno commented 7 years ago

@uronito - how did you solve the problem?

regarding seeing that topic change twice - I am seeing this too. I think it's happening because the wideVane setting is not working - when you update wideVane the callback is called, but then the heatpump changes back to the last known wideVane setting because it was invalid, so the callback fires again.

We need to work out the wideVane codes, which is being covered in #7.

SwiCago commented 7 years ago

@uronito exactly what @kayno said, wideVane is at the moment unknown. As far as i recall, it did not work for hadley either and is on the list of things to discover. I know for a fact it is not an option for kumo cloud device, so i wonder if it can even be set???

uronito commented 7 years ago

Hi Kayno ,Al,

@kayno, here is my code:

void hpSettingsChanged() { const size_t bufferSize = JSON_OBJECT_SIZE(6); const size_t bufferSizeDomoticz = JSON_OBJECT_SIZE(3); DynamicJsonBuffer jsonBuffer(bufferSize); JsonObject& root = jsonBuffer.createObject(); DynamicJsonBuffer jsonBufferDomoticz(bufferSizeDomoticz); JsonObject& domoticz = jsonBufferDomoticz.createObject(); heatpumpSettings currentSettings = hp.getSettings(); root["power"] = currentSettings.power; root["mode"] = currentSettings.mode; root["temperature"] = currentSettings.temperature; root["fan"] = currentSettings.fan; root["vane"] = currentSettings.vane; root["wideVane"] = currentSettings.wideVane; char buffer[512]; root.printTo(buffer, sizeof(buffer)); bool retain = true; mqtt_client.publish(heatpump_topic, buffer, retain); domoticz["command"] = "setuservariable"; domoticz["idx"] = 13; domoticz["value"] = currentSettings.power; char buffer_domoticz[512]; domoticz.printTo(buffer_domoticz, sizeof(buffer_domoticz)); mqtt_client.publish(domoticz_mqtt, buffer_domoticz, retain); domoticz["command"] = "setuservariable"; domoticz["idx"] = 12; domoticz["value"] = currentSettings.mode;

domoticz.printTo(buffer_domoticz, sizeof(buffer_domoticz));
mqtt_client.publish(domoticz_mqtt, buffer_domoticz, retain);
domoticz["command"] = "setuservariable";
domoticz["idx"]     = 15;
domoticz["value"]   = currentSettings.fan;

domoticz.printTo(buffer_domoticz, sizeof(buffer_domoticz));
mqtt_client.publish(domoticz_mqtt, buffer_domoticz, retain);
domoticz["command"] = "setuservariable";
domoticz["idx"]     = 14;
domoticz["value"]   = String(currentSettings.temperature);

domoticz.printTo(buffer_domoticz, sizeof(buffer_domoticz));
mqtt_client.publish(domoticz_mqtt, buffer_domoticz, retain );

}

@kayno, @all ,

topic /heatpump: {"power":"OFF","mode":"FAN","temperature":21,"fan":"1","vane":"1","wideVane":"|"} {"power":"OFF","mode":"FAN","temperature":21,"fan":"1","vane":"1","wideVane":"<<"}

my heat pump dont have wideVame, always i send power or mode or temperature or fan in the heatpump mqtt topic comes out the repeated command twice.

uronito commented 7 years ago

i Have original mac-557-ife mitsubishi wifi adaptor for melcloud, but dont have wide vane in melcloud app.

Thanks

kayno commented 7 years ago

re the double publish - it will happen even though you are not setting it. It might actually be related to two issues - #7 and #9:

7 because we haven't discovered how to control the wideVane

9 is that there is a default wantedSetting for wideVane, and each time it gets pushed the heatpump (firing the callback) the heatpump then flicks it back to whatever it originally was, firing the callback again

does your heatpump have a horizontal vane (left/right swing) at all? or just a vertical (up/down vane) vane? This is where future versions of the library might be able to detect the heatpump type or what features it has and adjust accordingly.

@uronito - which version of the code are you running? do you have my commit from ~12hrs ago?

uronito commented 7 years ago

@kayno ,

i have a ducted pead rp100 jaa, dont have vane,widevane. I use last swicago version 23 hours ago. I havent see your last version. Tomorrow i will probe it. I just saw that the latest version of melcloud have to control widevane. Thanks @kayno, @SwiCago

SwiCago commented 7 years ago

@uronito kumo cloud doesnt have wideVane. If as you say melcloud does, please to a tcp dump and see if it talks to their web in plain http. New kumo talks https, so no way to se what payload it gets from server. If melcloud is http, then you can get dump the payload as it receives it. It should be an xml, as i have seen posted somewhere else...and in that xml os literally the hex code that the device sends the heatpump.

SwiCago commented 7 years ago

@kayno, maybe we should default wanted wideVane as soon as it comes in from current wideVane, at least until we figure it out

SwiCago commented 7 years ago

@kayno @uronito default wideaVane for HPs that don't have wideVane should be 0x00, so what ever that is represented in the map.

uronito commented 7 years ago

@SwiCago ,

I try to get a tcp dump of the data coming from melcloud but I did not get it, I think they are encrypted.

@kayno

i had probe now your last version.The callback fires again. I think that default position for ducted heatpumps is (|) not (<<). I will try remap widevane array.

Thanks!!!

uronito commented 7 years ago

Wowwwww!!!!

At least!!!!!!!

I have managed to not trigger the callback for units that do not have widevane

The error was here:

HeatPump.cpp:

  HeatpumpSettings defaultSettings = {POWER_MAP [0], MODE_MAP [3], TEMP_MAP [9], FAN_MAP [2], VANE_MAP [1], WIDEVANE_MAP [2]};

I've changed it to:

  HeatpumpSettings defaultSettings = {POWER_MAP [0], MODE_MAP [3], TEMP_MAP [9], FAN_MAP [2], VANE_MAP [1], WIDEVANE_MAP [0]};

And so it is always kept in the pump and in the code the same value for wide vane with which the callback is not triggered.

kayno commented 7 years ago

@uronito glad to hear that worked! In my latest commits, I have removed the wantedSettings defaults completely, and left it to be populated the first time settings are received from the heatpump. I am assuming your heatpump will send a 0x01 (which is what is in WIDEVANE_MAP[0]) and set it in wantedSettings as the default, which will achieve the same as you have done, but also remove the problem raised in #9.

Can you please test my fork and see if this is the case? https://github.com/kayno/HeatPump

I have been testing it and it seems fine, but it would be nice to have someone else try before I submit a pull request to push it up to @SwiCago's repo.

uronito commented 7 years ago

I'll prove it right away and I'll tell you, Kay, give me 10 minutes

uronito commented 7 years ago

@kayno last version give me a error

exit status 1 'class HeatPump' has no member named 'setPacketReceivedCallback'

kayno commented 7 years ago

sorry @uronito, forgot to update the example in the library, doing that now

uronito commented 7 years ago

Thanks @kayno :)

kayno commented 7 years ago

try now :)

uronito commented 7 years ago

now works perfect!!!!!!

but if i set '{"power":"OFF"}' heatpump tell me {"power":"OFF","mode":"HEAT","temperature":28,"fan":"AUTO","vane":"AUTO","wideVane":"<<"}

and my last seetings in my heat pump before was

{"power":"OFF","mode":"HEAT","temperature":24,"fan":"AUTO","vane":"AUTO","wideVane":"<<"}

the temperature changes from 24to 28

Only happens the first time

This happened in the early versions.

For the rest works perfect

uronito commented 7 years ago

I think it is a normal behavior of the heat pump. When it detects that a device is connected by the port cn105, the heat pump sends that package with the adjustments by deefcto that came from factory, in this case mode heat with 28º. It only happens the first time, so I think it's normal for the heat pump, I think with melcloud it was also happening. So I think the code is perfect

kayno commented 7 years ago

ok interesting. I haven't seen it occur on my heatpump, so maybe a model thing. if you wanted to dig further, you could enable debug by sending a message to heatpump/debug/set with a payload of just the string "on", and then subscribe to /heatpump/debug to see all the packets going back and forth. There is a lot of output in debug mode though, so it can take a while to debug!!

thanks for the feedback on the code. i will sort a pull request :)

uronito commented 7 years ago

@kayno ,

Glad to help, I'll take a look at debug mode to see if I find out anything.

Many thanks to you for your efforts

Regards :)

uronito commented 7 years ago

Could you try something?

Disconnect the pump from the power supply.

wait a few seconds

Connect to power supply.

And send the command off by mqtt

See if it happens to you about the temperature.

Thank you

kayno commented 7 years ago

I will have to try tomorrow, it's near midnight here!

uronito commented 7 years ago

Ok Kayno.

Thankssss!!!! Goodnight

uronito commented 7 years ago

@kayno I have been testing during the day and the truth is that it works quite well. Just one thing, I noticed that if I change fast mode with the remote mqtt does not detect the change, there must be a pause of 2 seconds at least for the code to detect the change and show it in the heatpump / set theme

Thanks

kayno commented 7 years ago

Is "fast mode" a button on your remote?

uronito commented 7 years ago

No, jejeje

I mean if I change fast, that is, by pressing the mode key quickly.

1 second or 2 for pulse

kayno commented 7 years ago

Oh ok, quick fingers!!

If you do press mode fast, does it always report on mqtt the final mode you set? Or does that get missed?

We may need to review the delay() calls in update()

uronito commented 7 years ago

@kayno always no, missed presses.

if presses are in 1-2 second , missed some changes.

tomorrow i will try adjust delay.

Regards

SwiCago commented 7 years ago

@uronito @kayno , great team work guys. And thanks for adding to the compatability list. When we have a bigger list, I will make a compatability list file. This library is really taking off. I honestly didn't think anyone would else would need it, but am very glad I shared it to github

uronito commented 7 years ago

@kayno , @swicago, Equally, it is a pleasure to be able to help you to get this project forward :)

@kayno, today i will probe your last version, I saw you did it a few minutes ago.

Thanks @kayno, @swicago

kayno commented 7 years ago

@uronito, would be great if you could test it, to make sure it's not just working on my heatpump! thanks :)

uronito commented 7 years ago

@kayno , i have try your last version.

Works like a charm!!!!.

Regards

uronito commented 7 years ago

@kayno,

Too fast i said it.

There is a problem.

Whenever I turn on the pump in heat mode per mqtt, it always turns on 28º, ie what in the previous version passed only the first time (default values of the pump) now happens whenever I send a command per mqtt.

I imagine that if I turn on the pump in cold mode the same will happen with the default values of the pump in cold mode

uronito commented 7 years ago

Something very strange happens.

I disconnect the pump from the power supply.

I connect the circuit.

I connect the pump to the power supply.

I command the heat order to the pump per mqtt and the pump is turned on at 28º when the last temperature position was 24º.

I set the temperature from the remote (par21maa) to 24º.

I shut off the pump from the remote (par21maa) or by mqtt.

I command the heat order to the pump by mqtt and the pump is turned on to 28 ° again.

I set the temperature command to 24º from mqtt and turn off the pump.

And then the temperature is well memorized and when I command mqtt the heat order, the pump is turned on 24º which was the stored temperature.

I hope I have explained myself well

uronito commented 7 years ago

Can you probe the same?

Thanks!!!!

kayno commented 7 years ago

@uronito, can you try these steps with @SwiCago's repo (in other words without my latest changes)?

I am unable to replicate, so I want to see if its just my changes that are causing this.

uronito commented 7 years ago

I have try swicago's repo last version.

Te same problem.

Until you set the temperature from mqtt when you turn on or off the pump by mqtt {"power": "ON"} or {"power": "OFF"} the pump will use the default temperatures, in this case for heat will be 28 °

Once the temperature is set by mqtt, the operation is correct and the pump stores the temperatures correctly

Regards

uronito commented 7 years ago

I think the problem is here:

if(!wantedSettings) { wantedSettings = receivedSettings; } i think that the firs time the heatpump connects, wantesssetings dont' set to receivedsettings because 28º is max of my heatpump (0x00) in the temperature map.

Regards

uronito commented 7 years ago

Wooooowwww!!!!

With this modification: (heatpump.cpp) -------if(!wantedSettings) {--------> delete this wantedSettings = receivedSettings; --------}----------> delte this

Now works perfect always in swicago's code and kayno's code

Regards!!!!

SwiCago commented 7 years ago

@uronito get latest merge, make the change. @kayno can you test his change, i am not using the mqtt feature yet. When confirmed we can do a pull merge again

uronito commented 7 years ago

@swicago, i have made modification.

Regards!!!!

SwiCago commented 7 years ago

@uronito mergeed. @kayno please confirm that it works as expected

SwiCago commented 7 years ago

@kayno, there was a reason you checked if it was null, now with this merge wanted will always become current. @uronito i am not sure that is the correct logic, as my wanted settings may not be what the heat pump is set as. In example i set heat via esp, but ir remote sets it to fan. Via esp i still want heat, not fan. It should execute a call back that values are different from wanted and then it is up to the controlling software to decide if it should change it or not. However i do agree that if the wanted and current differ, that the hp library should only notify of the change once. Better yet would be to creat a prior current settings and compare that with current settings, if it differs notify. Wanted settings should not be changed automatically by library

uronito commented 7 years ago

@swicago,

I've been testing the latest version of Kayno with the modification I've done before, and the truth is that it works perfectly. Whenever I want to make a change by esp makes it perfect and the esp perfectly stores the modifications that are made from the pump.

Regards

SwiCago commented 7 years ago

@uronito @kayno , I understand we want heatpump to store current settings, but current settings should never override wanted settings. Here is how I see it. ESP connects ESP sets its wantedSettings ESP send update command ESP receives updateConfirm ESP receives callBack, because current settings != received settings (received stored in current) IR makes change ESP receives callBack, because current settings != received settings (received stored in current) It then update the receiving end to decide to leave as is, or send update commend, which send the wanted settings it originally wanted...unless updated by code in mean time. Wanted settings should never be overwritten by current or received ever!

I just don't see why wantedSettings would ever be changed by library, user sketch decides it's values. If user wants to be in sync, maybe get currrentSettings in sketch and update wanted to it, via sketch as setup. Library should not do this.

urinto, can you remove the wantedSettings = receivedSettings completely and see if it still works for you. Also, make sure on your sketch to be sure to get currentSettings once and set to your wanted settings or at least once set all wantedSettings, before you allow MQTT to do shcnages. I think that is where the confusion comes. When update is called, all settings must be sent to heatpump, it cannot just send one change. I believe Kayno set the wantedSettings to receivedSettings for first run, so it at least has a defaulted value. However IMO it is upto the sketch to get the settings and set all wanted settings at least once prior to calling update and then there after individual settings can be made. So either kayno's change to set wanted at least once goes back in and requires more testing or your sketch has to set settings at least once.

kayno commented 7 years ago

@SwiCago - to answer your question about why i checked if wantedSettings is null and then set it if it is, as you alluded to this is done so that on the very first run (and only the first run) I believe that wantedSettings should be synced to currentSettings. Otherwise, it has to be set somehow else, such as:

Personally I think the original idea (i.e. the solution from #9) is what we want. Going back to @uronito's earlier post:

I think the problem is here:

if(!wantedSettings) { wantedSettings = receivedSettings; } i think that the firs time the heatpump connects, wantesssetings dont' set to receivedsettings because 28º is max of my heatpump (0x00) in the temperature map.

@uronito - can you explain this some more? 28 is in TEMP and maps to 0x03 in TEMP_MAP, yet you are saving you are getting 0x00 (which maps to 31)?

it would be great @uronito to see the debug topic when your heatpump starts. The MQTT example now has _debugMode set to true, so it starts up in debug mode and you can see the connect() packets, and also the very first settings packet that comes in. That might help us to debug what the heatpump is reporting and what wantedSettings should be. If you open a terminal and subscribe to the topic heatpump/debug, then turn the heatpump on and you will see all the output. Post the first 10-20 odd lines here and we should be able to see what we need!

I also would like to test that the operator! function for heatpumpSettings is working correctly, and confirm that wantedSettings is only being set to currentSettings on the very first boot. Maybe my logic is wrong here??

Finally, going back to what I said earlier about abstracting complexities from the user... I wonder if there is a case to make update() private, and call it from the library every time one of the setSettings() functions is called? I was thinking about this from the point of view that if we were documenting each function of the library, for each of the setSettings() functions (e.g. setPowerSetting(), setModeSetting(), etc) we would keep repeating "Remember, this does not make the change, it just prepares things for you to call update(). It is when you call update() that the setting is actually changed". The very name of these functions - setSettings(), setPowerSettings(), etc is already implying that it will "set the setting", so why not make it happen then and there? Doing this might help us with some of the complexity of keeping wantedSettings in sync. If the IR remote for my heatpump had a button on it that said "update" or "change" or "send to heatpump" that you had to press after pressing the mode or temp buttons then I might think otherwise, but it doesn't :)

SwiCago commented 7 years ago

@kayno, the reason for update was for webbased application where more than one setting is set. The IR remote send all settings at once, not just one. So if we make update private, then sync will have to see if things changed and update. Also it would have to make sure that all individual settings have completed before update, so there is no race condition. Lastly if it is private, no way of knowing it is success like now on packet 61...i agree, wanted should be set at first boot as current settings of HP. But still think update should be done after user program determines it should be sent to HP.

kayno commented 7 years ago

@swicago, so are you saying #16 should be reversed? if so, I agree. We then need to look at @uronito's debug output to see what is coming in and what wantedSettings is being set to.

with the update() call being private, I hadn't thought of that angle (web based app). If we are saying that sketches that use the library should mimic the remote and send all settings, then we probably need to remove the individual set functions (e.g. setPowerSetting()) as the remote never does this... However I don't think we should remove the individual set functions, as I don't think we are trying to mimic the remote...

Also it would have to make sure that all individual settings have completed before update, so there is no race condition.

How would this race occur? you would still send all settings at once if just setPowerSetting() was called and it called update() itself. It uses wantedSettings to achieve this, and all the other settings (other than power) will be whatever the currentSetting are....

And if you want to set all at once, as in the web based app, you have setSettings() to do this...

The return value for update() can simply be returned by the set function that is being called, so that's not a problem.