Missing Details

[schotime]

Is anyone seeing this?

I'm getting vane, fan, and mode coming through blank every now and then. Its very strange. When this happens it seems to never update the current settings so it fires the settings changed event every single time.

[lekobob]

I have seen that once, I didn't have a chance to look for the reason at the time. Does it happen on HA startup? Or does it randomly go blank after HA has been up for a while?

[schotime]

Its not HA.
The mqtt raw message is blank coming from the heatpump library it seems.

[schotime]

Here is what I see sometimes. One moment it reports AUTO then next blank.

[kmdm]

Just a thought looking at the code and if I'm reading it correctly maybe you could try and see if you can get the corresponding received packet from the heatpump on the debug topic for the mqtt blank message/field...

[SwiCago]

I have never had that, but then again I am using a different mqtt library. I am using the one that supports TLS

[schotime]

It has nothing to do with mqtt. It is not getting the data correctly from the unit it seems.

[SwiCago]

How could it not return all the bytes correctly, the library validates it with the ChkSum byte. If the chksum doesn't match it is tossed.
Or is the heatpump returning 0x00 on bytes you are expecting a different value?

BTW: I am using this version of pubsubclient, but as you suspect it shouldn't matter.
https://pubsubclient.knolleary.net/
and this version of JSON library
https://travis-ci.org/bblanchon/ArduinoJson
with mosquitto broker on openwrt router

[SwiCago]

Maybe you can adjust the send for info packets interval. I recently had an issue with one of my ducted units. it was throwing code 2502 (overflow sensor) and to rule out issues with calling HP every second, I adjusted it to delay 2 second after an info send(but retained the must be >1sec to send changes). It still threw code, so it was not that. Turns out it was installed out of level, dumb asses. Anyhow, I never changed it back. Maybe 1 second intervals is pissing it off a bit.

Here is that bit of code if you want to try it
void HeatPump::sync(byte packetType) {
if((!connected) || (millis() - (PACKET_SENT_INTERVAL_MS * 10) > lastRecv)) {
connect(NULL);
}
else if(autoUpdate && !firstRun && wantedSettings != currentSettings && packetType == PACKET_TYPE_DEFAULT) {
update();
}
else if(canSend()) {
byte packet[PACKET_LEN] = {};
createInfoPacket(packet, packetType);
writePacket(packet, PACKET_LEN);
delay(2000);
}

readPacket();
}

[schotime]

Thanks.
Might be on to something here @SwiCago .
I'm just remote over the air updating all 5 of my units and we'll see what happens over the next couple of days.
Fingers crossed.

[SwiCago]

Yeah, let me know...ideally i'd pass a flag to canSend, if it is a info packet and do a 2 second check there and 1 second check for set packets...this way no lock ups, but this is a good way to test and see. To be honest 1 sec intervals is a lot, do we really need to know in near realtime the state of the hp ? LoL

[SwiCago]

@schotime I updated code to use an 2000ms for info packet interval via canSend, instead of forced delay as above. This can be adjusted via .h file constant PACKET_INFO_INTERVAL_MS
canSend will now decide this. Also I writePacket will now do a 1000ms delay, before continuing. I saw we sent delays before reads, so why not remove redundant code.

Let me know how this works for you.

[schotime]

Ok so i'm still getting issues and they start to occur after a day so i'm wondering if there is a memory leak or something happening remembering that i'm using Nano's which have bugger all memory compared to the 8266 chips.

As you can see it should be sending 6 messages per node but starts only sending 3 and half of the data is just not there.

A reboot of the arduino fixes the issues.

Also on another note, the latest version in master doesn't boot at all for me. It must be exceptioning somewhere.

[kmdm]

I was wondering what the latest master would do with the onConnectCallback() call if it was never set.

I guess maybe it exceptions.

(My C++ isn't anywhere near good enough to know if C++ picks a sensible default for that case.)

[SwiCago]

@schotime @kmdm i look at that, in thoery not setting a callback, just doesnt use it. Like an empty function. The MQTT uses callbacks, but my web samples don't, so i don't see not setting a callback being an issue. Maybe the move for the delay causes an issue. I will test further and rollback if that is the case
I also don't see how moving the delay to end of write packet function could be an issue, instead of calling it i the function that calls writepackets!?! Maybe on some HPs the connect needs longer than 1000ms, originally we had 1100ms.

Edit: i see issue, should have been if(onConnectCallback) {onConnectCallback();}
I'll fix as soon as home...my bad

[SwiCago]

@schotime @kmdm added the check to make sure onConnectCallback is set, before trying to call it. Can believe I missed that.. apologies

[schotime]

So i think i've figured it out for sure. My nanos only have 2kb of dynamic memory. The use of String in the library costs ~500 bytes. This was only leaving my Nano with 198 bytes of dynamic memory. It also takes just under 200 bytes to send and receive a message to the heatpump.

So the reason it works for an day or so is that the strings keep fragmenting the memory where eventually there isn't enough to create a new struct for the received settings and all I get is the defaults.

I have refactored the heatpump library to use char* everywhere and removed the reference to <WString.h> also. This reduced my hex output file by 10kb and now leaves me 682 bytes of free memory. It also reuses the stored char*'s where doing comparisons so effectively once allocated it is completely allocation free (except for the updates/syncs, but they get cleaned up and the end of the method).

Here is the comparison to master that i'm running on my units.
master...schotime:switch_to_char_star

I would like to let it run for a few days before creating a pull request, but if you see anything that doesn't make sense or some improvements, let me know.

Wowser....hopefully this my whole issue and i can finally leave them in the units.
Thanks to all for participating.

[SwiCago]

@schotime Hopefully that works, did you get a chance and see if the fix I made on the master corrected the failed boot.
Crazy that String sucks so much memory, you think it was a psuedo of char* with some methods attached to it

[SwiCago]

Hey on a side note, why use arduino nano...it has no wifi, are you doing a nano and esp piggy backed for wifi comms?

[schotime]

Choose the nano cause its 5V so no crazy wiring and using mysensors.org and nrf2401+ radios back to a gateway. Has over the air updates and integrates well with other sensors.

[schotime]

So far so good.
All devices still reporting 682 B of free memory.
Definitely looks good.

[kmdm]

Half idea:
I wonder if this could help / is even applicable: https://www.arduino.cc/en/Reference/PROGMEM

[SwiCago]

@kmdm progmem is flash memory. Meant for more static/constant data. Using progmem over and over could possible kill the flash memory at some point in time. Seeing that most of our constants aren't very big, using progmem would not help much.
Now if it was say a webpage, then yes using progmem would save a lot.

[SwiCago]

@kayno @schotime , I am getting a different issue. I just re-flashed all my chips and a few of them won't publish to the topic
Ex: I have one unit set as follows
` const char* client_id = "heatpump-middle"; // Must be unique on the MQTT network
const char* heatpump_topic = "heatpump/middle";
const char* heatpump_set_topic = "heatpump/middle/set";
const char* heatpump_status_topic = "heatpump/middle/status";
const char* heatpump_timers_topic = "heatpump/middle/timers";

const char* heatpump_debug_topic = "heatpump/middle/debug";
const char* heatpump_debug_set_topic = "heatpump/middle/debug/set";`

I subscribe to topic
mosquitto_sub -h localhost -p 1883 -t "heatpump/middle/#"
All topic seem to be fine except the "heatpump_topic" , timers debug etc all fine.
As soon as I send something, the HP sets the setting, shortly after it wants to send back the confirmed settings (POWER,MODE,VANE, etc)
And it fails, then on debug topic the following is printed "failed to publish to heatpump topic"
Exactly as is in code
bool retain = true; if(!mqtt_client.publish(heatpump_topic, buffer, retain)) { mqtt_client.publish(heatpump_debug_topic, "failed to publish to heatpump topic"); }

Any idea what the issue could be?
I have 2 other zones (living and kitchen), they are fine. My 4 other zones fail. Each one has unique ID

Even changed the topic to ""heatpump/middle/settings", still fails. I do not know what is wrong with updating the current settings to topic

[SwiCago]

Ok got a bit further...If I send a raw string on the topic. It is fine
if(!mqtt_client.publish(heatpump_topic, "{"power":"ON","mode":"COOL","temperature":29.00,"fan":"AUTO","vane":"AUTO","wideVane":"<<","iSee":false}", retain)) {
mqtt_client.publish(heatpump_debug_topic, "failed to publish to heatpump topic");
}
But when I use the JSON library it fails
char buffer[512];
root.printTo(buffer, sizeof(buffer));

I think it has to do with the buffer...I reduced JSON object to 6 and removed iSee, then it worked. Very odd behavior.

So, I updated all my units that have iSee to not return the true/false back over topic and it is perfect. My other two units do not have iSee(ducted models).

On a side note: I did take pics of my new setup, install and also took video of install ;) Will post up as soon as I processed them.

[schotime]

@SwiCago That is a weird one. Not sure about that one. iSee is the last one and also the only boolean so either of those could be the reason. Does printTo start from the start of the buffer everytime?

I'm still good for my units also. 5 days running now and no dropped packets and memory sits at 676.

[Moxified]

This sounds similar to the issue I was having in #53?

I solved my issue by increasing the MQTT_MAX_PACKET_SIZE to 256 in pubsubclient.h

[SwiCago]

@Moxified based on #53 you think it may be a combo of topic length and payload length that may make it fail? I'll have to try the max size adjustment and see if that fixes it. Stupid that it even has a limit, but that makes sense as to why the failer happens at send.

I'll try when I get home and confirm. If thatbsolves it, we will need to make a note in the mqtt example!

[Moxified]

I think the [my] issue is primarily the payload length. The topic default topic from the example sketch was short enough to fit in the payload but once I added longer topics to fit my MQTT naming scheme I noticed they stopped working except for debug topic. I suspect if I had yanked the isee out like you did that it would have reduced the payload enough to help out.

[SwiCago]

@Moxified seems the MQTT library uses the sum of client id, topic and payload to decide if it can send or not. If all 3 exceed max length, then return false. So raising the limit with #define MQTT_MAX_PACKET_SIZE 256 in sketch should fi the issue I am having. Will confim tonight

[SwiCago]

@Moxified I tried #define MQTT_MAX_PACKET_SIZE 256 in the sketch header and it failed, but then I noticed that we read pubsubclient.h before we read the sketch header, so I changed the order and it also failed. Then I set the define in the sketch, that also failed! Go figure, so lastly I was like f-it, I'll modify pubsubclient.h like you did and that worked.
I don't know how raising the buffer size on other chips with effect memory usage. ESP is fine as it has enough....so I guess people will either have to shorten their topics or just only return what they need.

[SwiCago]

@schotime your library still working for you? If so, before you do a pull request, make sure you are using latest master. Then add your changes, then submit new pull. Thanks

[schotime]

I have pushed up the PR.
Last night I also moved all the "static" type variables to statics and this saved another 200B. I now have approx 900/2000 free for dynamic allocation. This is a great result.

[SwiCago]

Why did you move them out of the header file? Couldn't they have stayed in the header?

[schotime]

Statics must be intialized in the cpp file as far as I know and it was the only way I could get it to work.

[kmdm]

Is the compiler/toolchain C++11 compliant?

I've come across:

class A {
 private:
  static constexpr const char* STRING = "some useful string constant";
};

https://stackoverflow.com/a/24341128
http://en.cppreference.com/w/cpp/language/constexpr

[schotime]

I tried that but I missed the const when I put the constexpr in. Will have a look again.

[SwiCago]

Let's see if we can move statics back to the header again. It is cleaner to read, but if it is not possible, then so be it. My very first version of the library had them in the ccp as well, but was able to get them into the header in later iterations.
Other than that, awesome on the memory savings. I would also like to complete or at least partially complete #39 error codes wil be really useful in the future

[kmdm]

@SwiCago Yeah... I think at the moment error codes support is the only thing that'll make me want to take the 'risk' of OTA flashing my huzzahs.

[SwiCago]

@schotime had to revert your last pull. It did not play well with any of the examples. Please make sure to compile against all examples and make changes as necessary, so that they compile correctly, then do a new pull request or share your branch first and I will copy them to my host locally and test them as well.
Thanks

[schotime]

@SwiCago After downloading half the internet for the 8266 libraries i've made the necessary changes to the examples.

[SwiCago]

@schotime lol... i commented on you pr, i'll get to it as soon as my drywallers are done