Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to monitor memory leak #12

Open
baqwas opened this issue Oct 6, 2020 · 4 comments
Open

How to monitor memory leak #12

baqwas opened this issue Oct 6, 2020 · 4 comments

Comments

@baqwas
Copy link

baqwas commented Oct 6, 2020

Hello,

I've been using the following libraries for some LAN telemetry exercises:

#include <Ethernet.h>
#include <PubSubClient.h>
#include <Arduino_JSON.h>
#include <Arduino_MKRENV.h>

char pubTopic[]   = "arduino/env/pub";
PubSubClient MQTTclient(MQTTserver, MQTTport, callbackSub, ETHclient);
JSONVar MQTTmsg;                      //!< MQTT message buffer

The following statement causes a problem after about ~72 hours of operation at 1 second interval:
MQTTclient.publish(pubTopic, ((String)JSON.stringify(MQTTmsg)).c_str());

Is my method to serialize the JSON object the culprit? The number of fields in the JSON object does not change with the iterations. All 10 fields except the first (which has a constant string value) are floats as in the example below:

MQTTmsg["Temperature"] = roundf(ENV.readTemperature(unitTemperature) * DECIMALS) / DECIMALS;

The two boards (MKR1000, UNO) where this failure occurs repeatedly require a manual RESET. (I disable the SD card for MKR1000 HATs during setup with SPI CS pin 4). Is there an alternate way to serialize the JSON object using the Arduino_JSON (and not the 3rd party JSON) library? Thanks.

Kind regards.

@tinxx
Copy link

tinxx commented Dec 28, 2020

Hey @baqwas, I might have experienced a similar problem.

I suggest you declare a variable for the buffer before passing it on to MQTT library.

I don't know if it should be necessary but I have also had some problems concerning buffers over at my project.
This is how I solved it for MiThermometer2MQTT:

// Process attached BME280 sensor data
JSONVar sensorData = formatBmeSensorData(BOARD_UID.c_str(), WiFi.RSSI());
String sensorDataString = JSON.stringify(sensorData);
const char *payload = sensorDataString.c_str();
//...
mqttClient.beginMessage(topic,
                        sensorDataString.length(), // size
                        false,                     // retain
                        0,                         // qos
                        false);                    // dup
mqttClient.print(payload);
mqttClient.endMessage();

I made it a habit to use the long form of mqttClient.beginMessage() because without passing size it limits payload length to 254 chars or so.

PS: I see you round your temperature values using rounf(). Have you experienced any strange numbers like 982.66100000000006? I am clipping decimals and JSONVar.stringify() does not seem to quite like it. (I posted over at #5.)

@baqwas
Copy link
Author

baqwas commented Dec 28, 2020

Hello @tinxx,

Thanks for letting me know. Much appreciated that you chose to follow up to help me out.

Your suggestion was one of the first things that I tried but not in the elegant manner that you have illustrated. I am not in a position to try out your suggestion for another fortnight but I will definitely give it a workout and let you know. A few other unrelated (but nevertheless self-inflicted) anomalies to resolve in other projects for now.

Perhaps, I was chasing the wrong library (viz. Ethernet) since it required a little extra code even with an empty SD card slot. I have to use LAN and cannot use Wi-Fi for this exercise.

Kind regards.

P.S. Thanks for the example! I'll use a Schenzen BME680 sensor eventually!
P.P.S. Don't want to get into a dissertation on IEEE floating point formats here but just to keep it simple, I do not have any rounding for the "value" column in the table in MariaDB (since the range is truly extensive for home automation sensors) and I didn't want all those trailing digits for temperature and humidity for DHT22 data.

@baqwas
Copy link
Author

baqwas commented Jan 15, 2021

Hello @tinxx,

Do you have few minutes to spare to teach me a little about C++. In trying to adapt to your suggestion, I converted your example to the following for my case. Unfortunately, I am ignorant about string conversions and hence the compiler is issuing an error message:

` String payload = (String)JSON.stringify(MQTTmsg).c_str();

byte[] b = bytes(payload.begin(), payload.end());

//bytes.push_back('\0');

//char *c = &bytes[0];

MQTTclient.beginPublish(pubTopic, payload.length(), false);

MQTTclient.write(bytes, payload.length());

MQTTclient.endPublish();`

The error message is:
decomposition declaration cannot be declared with type 'byte {aka unsigned char}'

I would like to stay with JSON payloads but clearly there is some ignorance on my part because my original code halts on two different boards with regularity (i.e. number of times the loop is executed). If I upload to both boards at the same time then the halts occur on both boards around the same time. Thanks for your guidance.

Kind regards.

P.S.
The libraries in use are:
`
#include <Ethernet.h>

#include <PubSubClient.h>

#include <Arduino_MKRENV.h>

#include <Arduino_JSON.h>
`

@tinxx
Copy link

tinxx commented Jan 15, 2021

Hi @baqwas,
I am not an C++ expert myself but let me try and help out with some suggestions:

You use the following expression: String payload = (String)JSON.stringify(MQTTmsg).c_str();

So what you tell the compiler is:

  1. String payload <- declares a variable of type String
  2. = (String) <- you initialize the variable and tell the compiler "whatever type you might think the following it, I tell you this is actually a String!"
  3. JSON.stringify(MQTTmsg) <- Then you tell the JSON library to convert MQTTmsg into a String.
  4. .c_str(); <- finally you convert the string into a C string (as opposed to different C++ strings -- see below).

So you tell the compiler that this C string is a C++ String object. That obviously does not work.

This is called type casting, it just works for uniform types like byte and char.
This is not a conversion.

Calling c_str() is a conversion.


So what you need to keep in mind is that there are three different kinds of strings you usually come across if you read or write Arduino code:

  1. String <- complex C++ data type provided by Arduino library
  2. std::string <- complex C++ data type provided by std standard library
  3. c_str that is actually a character array (char[] or char * - a pointer to the first element of the array).

The first two types of string in the list are complex data types that come with many handy methods.
See here for 1 and here for 2.
One methods that both String and std::string have in common is c_str() which actually returns a string of type 3.
Remember: This is raw data that is somewhere in memory.
You might call it an array or a buffer. They have a fixed length.
If you write more data to it than it is long, you overwrite arbitrary data in memory (buffer overflow).
Also keep in mind that C strings are terminated with a null byte '\0'.
If this stop marker is missing, functions that you don't provide with a length manually keep reading until there is – by chance – a null byte in memory.


Next you manually assign a buffer byte[] b = bytes(payload.begin(), payload.end());.
This is an array of bytes.
You would fill it with characters to send your JSON string payload, right?

That is not necessary because a string of type 3 from the list above is a raw memory array of characters. Just use such one.


So we first create the actual JSON string and assign it to a variable to keep it in memory until it goes out of scope.
It is of type String because that is what the library returns.

String payload = JSON.stringify(MQTTmsg);

So we go ahead and create our raw buffer by converting the String into an character array:

char buffer[] = payload.c_str();

So now we have the raw data in memory until the variable buffer goes out of scope.

Next, we want to send it via MQTT.
I assume you use pubsubclient while I use ArduinoMqttClient.

I have experienced that longer messages got cropped by ArduinoMqttClient.
So I looked into the header file and found that I could pass along the size of the buffer that I wanted to send.
We can either get the strlen(buffer) to get the length of the C string in the buffer or we ask the String payload for its length.
Recap: The complex strings come with some handy methods.

If we look into PubSubClient's header file we can find some explanation about arbitrary length payloads and no memory copy.
That sounds good because we already have our message buffer ready.

MQTTclient.beginPublish(pubTopic, payload.length(), false);

So now we use the write function.
It consumes an array of type uint8_t so we might have to type cast our array of char.
Note that uint8_t is an 8 bit integer value - 8 bit of data. A char should have the uniform length of 8 bit, so we can type cast here.
We assume that, bluntly put, MQTTclient just picks raw data from memory and dumps it into the network interface.
So it does not care if the data it reads is actually a string of numbers or characters or raw binary data.

MQTTclient.write((uint8_t *)buffer, payload.length());

Finally we tell MQTTclient that we finished stuffing it with data and it should tell the MQTT server that the message is complete:

MQTTclient.endPublish();`

Please note that I did not test any of the code in this comment and never used PubSubClient.
Feel free to ask again if something is wrong or unclear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants